diff --git a/compiler/rustc_middle/src/ty/instance.rs b/compiler/rustc_middle/src/ty/instance.rs index e9c19331e4a0b..98ca71b86be07 100644 --- a/compiler/rustc_middle/src/ty/instance.rs +++ b/compiler/rustc_middle/src/ty/instance.rs @@ -111,8 +111,9 @@ pub enum InstanceKind<'tcx> { /// Dynamic dispatch to `::fn`. /// - /// This `InstanceKind` does not have callable MIR. Calls to `Virtual` instances must be - /// codegen'd as virtual calls through the vtable. + /// This `InstanceKind` may have a callable MIR as the default implementation. + /// Calls to `Virtual` instances must be codegen'd as virtual calls through the vtable. + /// *This means we might not know exactly what is being called.* /// /// If this is reified to a `fn` pointer, a `ReifyShim` is used (see `ReifyShim` above for more /// details on that). diff --git a/compiler/rustc_ty_utils/src/abi.rs b/compiler/rustc_ty_utils/src/abi.rs index 0ff82f0c256de..74ab65b4f58b7 100644 --- a/compiler/rustc_ty_utils/src/abi.rs +++ b/compiler/rustc_ty_utils/src/abi.rs @@ -309,15 +309,11 @@ fn fn_abi_of_fn_ptr<'tcx>( query: ty::PseudoCanonicalInput<'tcx, (ty::PolyFnSig<'tcx>, &'tcx ty::List>)>, ) -> Result<&'tcx FnAbi<'tcx, Ty<'tcx>>, &'tcx FnAbiError<'tcx>> { let ty::PseudoCanonicalInput { typing_env, value: (sig, extra_args) } = query; - - let cx = LayoutCx::new(tcx, typing_env); fn_abi_new_uncached( - &cx, + &LayoutCx::new(tcx, typing_env), tcx.instantiate_bound_regions_with_erased(sig), extra_args, None, - None, - false, ) } @@ -326,19 +322,11 @@ fn fn_abi_of_instance<'tcx>( query: ty::PseudoCanonicalInput<'tcx, (ty::Instance<'tcx>, &'tcx ty::List>)>, ) -> Result<&'tcx FnAbi<'tcx, Ty<'tcx>>, &'tcx FnAbiError<'tcx>> { let ty::PseudoCanonicalInput { typing_env, value: (instance, extra_args) } = query; - - let sig = fn_sig_for_fn_abi(tcx, instance, typing_env); - - let caller_location = - instance.def.requires_caller_location(tcx).then(|| tcx.caller_location_ty()); - fn_abi_new_uncached( &LayoutCx::new(tcx, typing_env), - sig, + fn_sig_for_fn_abi(tcx, instance, typing_env), extra_args, - caller_location, - Some(instance.def_id()), - matches!(instance.def, ty::InstanceKind::Virtual(..)), + Some(instance), ) } @@ -547,19 +535,25 @@ fn fn_abi_sanity_check<'tcx>( fn_arg_sanity_check(cx, fn_abi, spec_abi, &fn_abi.ret); } -// FIXME(eddyb) perhaps group the signature/type-containing (or all of them?) -// arguments of this method, into a separate `struct`. -#[tracing::instrument(level = "debug", skip(cx, caller_location, fn_def_id, force_thin_self_ptr))] +#[tracing::instrument(level = "debug", skip(cx, instance))] fn fn_abi_new_uncached<'tcx>( cx: &LayoutCx<'tcx>, sig: ty::FnSig<'tcx>, extra_args: &[Ty<'tcx>], - caller_location: Option>, - fn_def_id: Option, - // FIXME(eddyb) replace this with something typed, like an `enum`. - force_thin_self_ptr: bool, + instance: Option>, ) -> Result<&'tcx FnAbi<'tcx, Ty<'tcx>>, &'tcx FnAbiError<'tcx>> { let tcx = cx.tcx(); + let (caller_location, determined_fn_def_id, is_virtual_call) = if let Some(instance) = instance + { + let is_virtual_call = matches!(instance.def, ty::InstanceKind::Virtual(..)); + ( + instance.def.requires_caller_location(tcx).then(|| tcx.caller_location_ty()), + if is_virtual_call { None } else { Some(instance.def_id()) }, + is_virtual_call, + ) + } else { + (None, None, false) + }; let sig = tcx.normalize_erasing_regions(cx.typing_env, sig); let conv = conv_from_spec_abi(cx.tcx(), sig.abi, sig.c_variadic); @@ -568,16 +562,11 @@ fn fn_abi_new_uncached<'tcx>( let extra_args = if sig.abi == ExternAbi::RustCall { assert!(!sig.c_variadic && extra_args.is_empty()); - if let Some(input) = sig.inputs().last() { - if let ty::Tuple(tupled_arguments) = input.kind() { - inputs = &sig.inputs()[0..sig.inputs().len() - 1]; - tupled_arguments - } else { - bug!( - "argument to function with \"rust-call\" ABI \ - is not a tuple" - ); - } + if let Some(input) = sig.inputs().last() + && let ty::Tuple(tupled_arguments) = input.kind() + { + inputs = &sig.inputs()[0..sig.inputs().len() - 1]; + tupled_arguments } else { bug!( "argument to function with \"rust-call\" ABI \ @@ -590,7 +579,7 @@ fn fn_abi_new_uncached<'tcx>( }; let is_drop_in_place = - fn_def_id.is_some_and(|def_id| tcx.is_lang_item(def_id, LangItem::DropInPlace)); + determined_fn_def_id.is_some_and(|def_id| tcx.is_lang_item(def_id, LangItem::DropInPlace)); let arg_of = |ty: Ty<'tcx>, arg_idx: Option| -> Result<_, &'tcx FnAbiError<'tcx>> { let span = tracing::debug_span!("arg_of"); @@ -603,7 +592,7 @@ fn fn_abi_new_uncached<'tcx>( }); let layout = cx.layout_of(ty).map_err(|err| &*tcx.arena.alloc(FnAbiError::Layout(*err)))?; - let layout = if force_thin_self_ptr && arg_idx == Some(0) { + let layout = if is_virtual_call && arg_idx == Some(0) { // Don't pass the vtable, it's not an argument of the virtual fn. // Instead, pass just the data pointer, but give it the type `*const/mut dyn Trait` // or `&/&mut dyn Trait` because this is special-cased elsewhere in codegen @@ -646,9 +635,22 @@ fn fn_abi_new_uncached<'tcx>( c_variadic: sig.c_variadic, fixed_count: inputs.len() as u32, conv, - can_unwind: fn_can_unwind(cx.tcx(), fn_def_id, sig.abi), + can_unwind: fn_can_unwind( + tcx, + // Since `#[rustc_nounwind]` can change unwinding, we cannot infer unwinding by `fn_def_id` for a virtual call. + determined_fn_def_id, + sig.abi, + ), }; - fn_abi_adjust_for_abi(cx, &mut fn_abi, sig.abi, fn_def_id); + fn_abi_adjust_for_abi( + cx, + &mut fn_abi, + sig.abi, + // If this is a virtual call, we cannot pass the `fn_def_id`, as it might call other + // functions from vtable. Internally, `deduced_param_attrs` attempts to infer attributes by + // visit the function body. + determined_fn_def_id, + ); debug!("fn_abi_new_uncached = {:?}", fn_abi); fn_abi_sanity_check(cx, &fn_abi, sig.abi); Ok(tcx.arena.alloc(fn_abi)) diff --git a/tests/codegen/virtual-call-attrs-issue-137646.rs b/tests/codegen/virtual-call-attrs-issue-137646.rs new file mode 100644 index 0000000000000..5e453947f27db --- /dev/null +++ b/tests/codegen/virtual-call-attrs-issue-137646.rs @@ -0,0 +1,37 @@ +//! Regression test for https://github.com/rust-lang/rust/issues/137646. +//! Since we don't know the exact implementation of the virtual call, +//! it might write to parameters, we can't infer the readonly attribute. +//@ compile-flags: -C opt-level=3 -C no-prepopulate-passes + +#![crate_type = "lib"] +#![feature(rustc_attrs)] + +pub trait Trait { + #[rustc_nounwind] + fn m(&self, _: (i32, i32, i32)) {} +} + +#[no_mangle] +pub fn foo(trait_: &dyn Trait) { + // CHECK-LABEL: @foo( + // CHECK: call void + // CHECK-NOT: readonly + trait_.m((1, 1, 1)); +} + +#[no_mangle] +#[rustc_nounwind] +pub fn foo_nounwind(trait_: &dyn Trait) { + // CHECK-LABEL: @foo_nounwind( + // FIXME: Here should be invoke. + // COM: CHECK: invoke + trait_.m((1, 1, 1)); +} + +#[no_mangle] +pub extern "C" fn c_nounwind(trait_: &dyn Trait) { + // CHECK-LABEL: @c_nounwind( + // FIXME: Here should be invoke. + // COM: CHECK: invoke + trait_.m((1, 1, 1)); +} diff --git a/tests/ui/virtual-call-attrs-issue-137646.rs b/tests/ui/virtual-call-attrs-issue-137646.rs new file mode 100644 index 0000000000000..e80bd5768a429 --- /dev/null +++ b/tests/ui/virtual-call-attrs-issue-137646.rs @@ -0,0 +1,45 @@ +//! Regression test for https://github.com/rust-lang/rust/issues/137646. +//! The parameter value at all calls to `check` should be `(1, 1, 1)`. + +//@ run-pass + +use std::hint::black_box; + +type T = (i32, i32, i32); + +pub trait Trait { + fn m(&self, _: T, _: T) {} +} + +impl Trait for () { + fn m(&self, mut _v1: T, v2: T) { + _v1 = (0, 0, 0); + check(v2); + } +} + +pub fn run_1(trait_: &dyn Trait) { + let v1 = (1, 1, 1); + let v2 = (1, 1, 1); + trait_.m(v1, v2); +} + +pub fn run_2(trait_: &dyn Trait) { + let v1 = (1, 1, 1); + let v2 = (1, 1, 1); + trait_.m(v1, v2); + check(v1); + check(v2); +} + +#[inline(never)] +fn check(v: T) { + assert_eq!(v, (1, 1, 1)); +} + +fn main() { + black_box(run_1 as fn(&dyn Trait)); + black_box(run_2 as fn(&dyn Trait)); + run_1(&()); + run_2(&()); +}