diff --git a/compiler/rustc_codegen_ssa/src/lib.rs b/compiler/rustc_codegen_ssa/src/lib.rs index 31854c7f4c4e0cb6501d9b3631c9266cf0243e18..c26a7422fdd241ade10c110dbe35d208219cbaae 100644 --- a/compiler/rustc_codegen_ssa/src/lib.rs +++ b/compiler/rustc_codegen_ssa/src/lib.rs @@ -4,6 +4,7 @@ #![feature(if_let_guard)] #![feature(int_roundings)] #![feature(let_chains)] +#![feature(negative_impls)] #![feature(never_type)] #![feature(strict_provenance)] #![feature(try_blocks)] diff --git a/compiler/rustc_codegen_ssa/src/mir/block.rs b/compiler/rustc_codegen_ssa/src/mir/block.rs index 0cec560ba4518480ff84bdc00c5247948d8f06ca..5e40b672866bfd1ac9b13ee069e74106df0dfd75 100644 --- a/compiler/rustc_codegen_ssa/src/mir/block.rs +++ b/compiler/rustc_codegen_ssa/src/mir/block.rs @@ -1729,7 +1729,7 @@ fn store_return( IndirectOperand(tmp, index) => { let op = bx.load_operand(tmp); tmp.storage_dead(bx); - self.locals[index] = LocalRef::Operand(op); + self.overwrite_local(index, LocalRef::Operand(op)); self.debug_introduce_local(bx, index); } DirectOperand(index) => { @@ -1744,7 +1744,7 @@ fn store_return( } else { OperandRef::from_immediate_or_packed_pair(bx, llval, ret_abi.layout) }; - self.locals[index] = LocalRef::Operand(op); + self.overwrite_local(index, LocalRef::Operand(op)); self.debug_introduce_local(bx, index); } } diff --git a/compiler/rustc_codegen_ssa/src/mir/debuginfo.rs b/compiler/rustc_codegen_ssa/src/mir/debuginfo.rs index 4f79c6a3d823c83e912348e0e192a08a754020b0..c6589a40392d71dc631b9e8479cad59c330b2472 100644 --- a/compiler/rustc_codegen_ssa/src/mir/debuginfo.rs +++ b/compiler/rustc_codegen_ssa/src/mir/debuginfo.rs @@ -248,7 +248,7 @@ fn adjust_span_for_debugging(&self, mut span: Span) -> Span { } fn spill_operand_to_stack( - operand: &OperandRef<'tcx, Bx::Value>, + operand: OperandRef<'tcx, Bx::Value>, name: Option, bx: &mut Bx, ) -> PlaceRef<'tcx, Bx::Value> { @@ -375,7 +375,7 @@ pub fn debug_introduce_local(&self, bx: &mut Bx, local: mir::Local) { return; } - Self::spill_operand_to_stack(operand, name, bx) + Self::spill_operand_to_stack(*operand, name, bx) } LocalRef::Place(place) => *place, @@ -550,7 +550,7 @@ pub fn compute_per_local_var_debug_info( if let Ok(operand) = self.eval_mir_constant_to_operand(bx, &c) { self.set_debug_loc(bx, var.source_info); let base = Self::spill_operand_to_stack( - &operand, + operand, Some(var.name.to_string()), bx, ); diff --git a/compiler/rustc_codegen_ssa/src/mir/locals.rs b/compiler/rustc_codegen_ssa/src/mir/locals.rs new file mode 100644 index 0000000000000000000000000000000000000000..da8bf5e7916a74c96a2c0436bcac1b100a7f511d --- /dev/null +++ b/compiler/rustc_codegen_ssa/src/mir/locals.rs @@ -0,0 +1,75 @@ +//! Locals are in a private module as updating `LocalRef::Operand` has to +//! be careful wrt to subtyping. To deal with this we only allow updates by using +//! `FunctionCx::overwrite_local` which handles it automatically. +use crate::mir::{FunctionCx, LocalRef}; +use crate::traits::BuilderMethods; +use rustc_index::IndexVec; +use rustc_middle::mir; +use rustc_middle::ty::print::with_no_trimmed_paths; +use std::ops::{Index, IndexMut}; + +pub(super) struct Locals<'tcx, V> { + values: IndexVec>, +} + +impl<'tcx, V> Index for Locals<'tcx, V> { + type Output = LocalRef<'tcx, V>; + #[inline] + fn index(&self, index: mir::Local) -> &LocalRef<'tcx, V> { + &self.values[index] + } +} + +/// To mutate locals, use `FunctionCx::overwrite_local` instead. +impl<'tcx, V, Idx: ?Sized> !IndexMut for Locals<'tcx, V> {} + +impl<'tcx, V> Locals<'tcx, V> { + pub(super) fn empty() -> Locals<'tcx, V> { + Locals { values: IndexVec::default() } + } + + pub(super) fn indices(&self) -> impl DoubleEndedIterator + Clone + 'tcx { + self.values.indices() + } +} + +impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { + pub(super) fn initialize_locals(&mut self, values: Vec>) { + assert!(self.locals.values.is_empty()); + + for (local, value) in values.into_iter().enumerate() { + match value { + LocalRef::Place(_) | LocalRef::UnsizedPlace(_) | LocalRef::PendingOperand => (), + LocalRef::Operand(op) => { + let local = mir::Local::from_usize(local); + let expected_ty = self.monomorphize(self.mir.local_decls[local].ty); + assert_eq!(expected_ty, op.layout.ty, "unexpected initial operand type"); + } + } + + self.locals.values.push(value); + } + } + + pub(super) fn overwrite_local( + &mut self, + local: mir::Local, + mut value: LocalRef<'tcx, Bx::Value>, + ) { + match value { + LocalRef::Place(_) | LocalRef::UnsizedPlace(_) | LocalRef::PendingOperand => (), + LocalRef::Operand(ref mut op) => { + let local_ty = self.monomorphize(self.mir.local_decls[local].ty); + if local_ty != op.layout.ty { + // FIXME(#112651): This can be changed to an ICE afterwards. + debug!("updating type of operand due to subtyping"); + with_no_trimmed_paths!(debug!(?op.layout.ty)); + with_no_trimmed_paths!(debug!(?local_ty)); + op.layout.ty = local_ty; + } + } + }; + + self.locals.values[local] = value; + } +} diff --git a/compiler/rustc_codegen_ssa/src/mir/mod.rs b/compiler/rustc_codegen_ssa/src/mir/mod.rs index 2809ec2deb550c1926265bc6557aa7290a8f8c59..15b0e34b8e4d26cc0fa3de9420c855fee8af59c6 100644 --- a/compiler/rustc_codegen_ssa/src/mir/mod.rs +++ b/compiler/rustc_codegen_ssa/src/mir/mod.rs @@ -1,21 +1,31 @@ use crate::base; use crate::traits::*; +use rustc_index::bit_set::BitSet; +use rustc_index::IndexVec; use rustc_middle::mir; use rustc_middle::mir::interpret::ErrorHandled; +use rustc_middle::mir::traversal; use rustc_middle::ty::layout::{FnAbiOf, HasTyCtxt, TyAndLayout}; use rustc_middle::ty::{self, Instance, Ty, TyCtxt, TypeFoldable, TypeVisitableExt}; use rustc_target::abi::call::{FnAbi, PassMode}; use std::iter; -use rustc_index::bit_set::BitSet; -use rustc_index::IndexVec; +mod analyze; +mod block; +pub mod constant; +pub mod coverageinfo; +pub mod debuginfo; +mod intrinsic; +mod locals; +pub mod operand; +pub mod place; +mod rvalue; +mod statement; use self::debuginfo::{FunctionDebugContext, PerLocalVarDebugInfo}; -use self::place::PlaceRef; -use rustc_middle::mir::traversal; - use self::operand::{OperandRef, OperandValue}; +use self::place::PlaceRef; // Used for tracking the state of generated basic blocks. enum CachedLlbb { @@ -91,7 +101,7 @@ pub struct FunctionCx<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> { /// /// Avoiding allocs can also be important for certain intrinsics, /// notably `expect`. - locals: IndexVec>, + locals: locals::Locals<'tcx, Bx::Value>, /// All `VarDebugInfo` from the MIR body, partitioned by `Local`. /// This is `None` if no var`#[non_exhaustive]`iable debuginfo/names are needed. @@ -192,7 +202,7 @@ pub fn codegen_mir<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>( cleanup_kinds, landing_pads: IndexVec::from_elem(None, &mir.basic_blocks), funclets: IndexVec::from_fn_n(|_| None, mir.basic_blocks.len()), - locals: IndexVec::new(), + locals: locals::Locals::empty(), debug_context, per_local_var_debug_info: None, caller_location: None, @@ -223,7 +233,7 @@ pub fn codegen_mir<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>( let memory_locals = analyze::non_ssa_locals(&fx); // Allocate variable and temp allocas - fx.locals = { + let local_values = { let args = arg_local_refs(&mut start_bx, &mut fx, &memory_locals); let mut allocate_local = |local| { @@ -256,6 +266,7 @@ pub fn codegen_mir<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>( .chain(mir.vars_and_temps_iter().map(allocate_local)) .collect() }; + fx.initialize_locals(local_values); // Apply debuginfo to the newly allocated locals. fx.debug_introduce_locals(&mut start_bx); @@ -289,14 +300,13 @@ fn arg_local_refs<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>( .enumerate() .map(|(arg_index, local)| { let arg_decl = &mir.local_decls[local]; + let arg_ty = fx.monomorphize(arg_decl.ty); if Some(local) == mir.spread_arg { // This argument (e.g., the last argument in the "rust-call" ABI) // is a tuple that was spread at the ABI level and now we have // to reconstruct it into a tuple local variable, from multiple // individual LLVM function arguments. - - let arg_ty = fx.monomorphize(arg_decl.ty); let ty::Tuple(tupled_arg_tys) = arg_ty.kind() else { bug!("spread argument isn't a tuple?!"); }; @@ -331,8 +341,6 @@ fn arg_local_refs<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>( } if fx.fn_abi.c_variadic && arg_index == fx.fn_abi.args.len() { - let arg_ty = fx.monomorphize(arg_decl.ty); - let va_list = PlaceRef::alloca(bx, bx.layout_of(arg_ty)); bx.va_start(va_list.llval); @@ -429,14 +437,3 @@ fn arg_local_refs<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>( args } - -mod analyze; -mod block; -pub mod constant; -pub mod coverageinfo; -pub mod debuginfo; -mod intrinsic; -pub mod operand; -pub mod place; -mod rvalue; -mod statement; diff --git a/compiler/rustc_codegen_ssa/src/mir/statement.rs b/compiler/rustc_codegen_ssa/src/mir/statement.rs index 3fd7397ad38651c21ce8f6d93ac6c1ba7abe6955..314d364c0c2a342ed5b8bc59bc1fc89641ae0133 100644 --- a/compiler/rustc_codegen_ssa/src/mir/statement.rs +++ b/compiler/rustc_codegen_ssa/src/mir/statement.rs @@ -20,7 +20,7 @@ pub fn codegen_statement(&mut self, bx: &mut Bx, statement: &mir::Statement<'tcx } LocalRef::PendingOperand => { let operand = self.codegen_rvalue_operand(bx, rvalue); - self.locals[index] = LocalRef::Operand(operand); + self.overwrite_local(index, LocalRef::Operand(operand)); self.debug_introduce_local(bx, index); } LocalRef::Operand(op) => { diff --git a/compiler/rustc_mir_transform/src/dest_prop.rs b/compiler/rustc_mir_transform/src/dest_prop.rs index 78758e2db28abfdb7ad77277ac6c5c65d83a9b16..a31551cf6199c75f8fc275124da49904235d0b9f 100644 --- a/compiler/rustc_mir_transform/src/dest_prop.rs +++ b/compiler/rustc_mir_transform/src/dest_prop.rs @@ -37,6 +37,10 @@ //! if they do not consistently refer to the same place in memory. This is satisfied if they do //! not contain any indirection through a pointer or any indexing projections. //! +//! * `p` and `q` must have the **same type**. If we replace a local with a subtype or supertype, +//! we may end up with a differnet vtable for that local. See the `subtyping-impacts-selection` +//! tests for an example where that causes issues. +//! //! * We need to make sure that the goal of "merging the memory" is actually structurally possible //! in MIR. For example, even if all the other conditions are satisfied, there is no way to //! "merge" `_5.foo` and `_6.bar`. For now, we ensure this by requiring that both `p` and `q` are @@ -134,6 +138,7 @@ use rustc_data_structures::fx::FxHashMap; use rustc_index::bit_set::BitSet; use rustc_middle::mir::visit::{MutVisitor, PlaceContext, Visitor}; +use rustc_middle::mir::HasLocalDecls; use rustc_middle::mir::{dump_mir, PassWhere}; use rustc_middle::mir::{ traversal, Body, InlineAsmOperand, Local, LocalKind, Location, Operand, Place, Rvalue, @@ -763,12 +768,22 @@ fn visit_statement(&mut self, statement: &Statement<'tcx>, _: Location) { return; }; - // As described at the top of the file, we do not go near things that have their address - // taken. + // As described at the top of the file, we do not go near things that have + // their address taken. if self.borrowed.contains(src) || self.borrowed.contains(dest) { return; } + // As described at the top of this file, we do not touch locals which have + // different types. + let src_ty = self.body.local_decls()[src].ty; + let dest_ty = self.body.local_decls()[dest].ty; + if src_ty != dest_ty { + // FIXME(#112651): This can be removed afterwards. Also update the module description. + trace!("skipped `{src:?} = {dest:?}` due to subtyping: {src_ty} != {dest_ty}"); + return; + } + // Also, we need to make sure that MIR actually allows the `src` to be removed if is_local_required(src, self.body) { return; diff --git a/compiler/rustc_mir_transform/src/shim.rs b/compiler/rustc_mir_transform/src/shim.rs index 2ae5ec4d9e6c83ee3bb6cf5b1d3ca6be4858e042..500595e9f507e1866c191de48645cd51bf2185e5 100644 --- a/compiler/rustc_mir_transform/src/shim.rs +++ b/compiler/rustc_mir_transform/src/shim.rs @@ -32,13 +32,15 @@ fn make_shim<'tcx>(tcx: TyCtxt<'tcx>, instance: ty::InstanceDef<'tcx>) -> Body<' let mut result = match instance { ty::InstanceDef::Item(..) => bug!("item {:?} passed to make_shim", instance), ty::InstanceDef::VTableShim(def_id) => { - build_call_shim(tcx, instance, Some(Adjustment::Deref), CallKind::Direct(def_id)) + let adjustment = Adjustment::Deref { source: DerefSource::MutPtr }; + build_call_shim(tcx, instance, Some(adjustment), CallKind::Direct(def_id)) } ty::InstanceDef::FnPtrShim(def_id, ty) => { let trait_ = tcx.trait_of_item(def_id).unwrap(); let adjustment = match tcx.fn_trait_kind_from_def_id(trait_) { Some(ty::ClosureKind::FnOnce) => Adjustment::Identity, - Some(ty::ClosureKind::FnMut | ty::ClosureKind::Fn) => Adjustment::Deref, + Some(ty::ClosureKind::Fn) => Adjustment::Deref { source: DerefSource::ImmRef }, + Some(ty::ClosureKind::FnMut) => Adjustment::Deref { source: DerefSource::MutRef }, None => bug!("fn pointer {:?} is not an fn", ty), }; @@ -107,16 +109,26 @@ fn make_shim<'tcx>(tcx: TyCtxt<'tcx>, instance: ty::InstanceDef<'tcx>) -> Body<' result } +#[derive(Copy, Clone, Debug, PartialEq)] +enum DerefSource { + /// `fn shim(&self) { inner(*self )}`. + ImmRef, + /// `fn shim(&mut self) { inner(*self )}`. + MutRef, + /// `fn shim(*mut self) { inner(*self )}`. + MutPtr, +} + #[derive(Copy, Clone, Debug, PartialEq)] enum Adjustment { /// Pass the receiver as-is. Identity, - /// We get passed `&[mut] self` and call the target with `*self`. + /// We get passed a reference or a raw pointer to `self` and call the target with `*self`. /// /// This either copies `self` (if `Self: Copy`, eg. for function items), or moves out of it /// (for `VTableShim`, which effectively is passed `&own Self`). - Deref, + Deref { source: DerefSource }, /// We get passed `self: Self` and call the target with `&mut self`. /// @@ -667,8 +679,12 @@ fn build_call_shim<'tcx>( let self_arg = &mut inputs_and_output[0]; *self_arg = match rcvr_adjustment.unwrap() { Adjustment::Identity => fnty, - Adjustment::Deref => tcx.mk_imm_ptr(fnty), - Adjustment::RefMut => tcx.mk_mut_ptr(fnty), + Adjustment::Deref { source } => match source { + DerefSource::ImmRef => tcx.mk_imm_ref(tcx.lifetimes.re_erased, fnty), + DerefSource::MutRef => tcx.mk_mut_ref(tcx.lifetimes.re_erased, fnty), + DerefSource::MutPtr => tcx.mk_mut_ptr(fnty), + }, + Adjustment::RefMut => bug!("`RefMut` is never used with indirect calls: {instance:?}"), }; sig.inputs_and_output = tcx.mk_type_list(&inputs_and_output); } @@ -699,7 +715,7 @@ fn build_call_shim<'tcx>( let rcvr = rcvr_adjustment.map(|rcvr_adjustment| match rcvr_adjustment { Adjustment::Identity => Operand::Move(rcvr_place()), - Adjustment::Deref => Operand::Move(tcx.mk_place_deref(rcvr_place())), + Adjustment::Deref { source: _ } => Operand::Move(tcx.mk_place_deref(rcvr_place())), Adjustment::RefMut => { // let rcvr = &mut rcvr; let ref_rcvr = local_decls.push( diff --git a/compiler/rustc_mir_transform/src/ssa.rs b/compiler/rustc_mir_transform/src/ssa.rs index 7a0d3a025f3652c1608d5c997588e25b782ec6fc..8dc2dfe13bd3b6be8b2f4a92e74ca9ce9372165e 100644 --- a/compiler/rustc_mir_transform/src/ssa.rs +++ b/compiler/rustc_mir_transform/src/ssa.rs @@ -271,6 +271,14 @@ fn compute_copy_classes(ssa: &mut SsaLocals, body: &Body<'_>) { else { continue }; let Some(rhs) = place.as_local() else { continue }; + let local_ty = body.local_decls()[local].ty; + let rhs_ty = body.local_decls()[rhs].ty; + if local_ty != rhs_ty { + // FIXME(#112651): This can be removed afterwards. + trace!("skipped `{local:?} = {rhs:?}` due to subtyping: {local_ty} != {rhs_ty}"); + continue; + } + if !ssa.is_ssa(rhs) { continue; } diff --git a/tests/mir-opt/fn_ptr_shim.core.ops-function-Fn-call.AddMovesForPackedDrops.before.mir b/tests/mir-opt/fn_ptr_shim.core.ops-function-Fn-call.AddMovesForPackedDrops.before.mir index b15a634256f641ff39c4357d10ff3c2ac23c7961..5fddfd494eadf16a050aa17da6d1150d23428763 100644 --- a/tests/mir-opt/fn_ptr_shim.core.ops-function-Fn-call.AddMovesForPackedDrops.before.mir +++ b/tests/mir-opt/fn_ptr_shim.core.ops-function-Fn-call.AddMovesForPackedDrops.before.mir @@ -1,6 +1,6 @@ // MIR for `std::ops::Fn::call` before AddMovesForPackedDrops -fn std::ops::Fn::call(_1: *const fn(), _2: ()) -> >::Output { +fn std::ops::Fn::call(_1: &fn(), _2: ()) -> >::Output { let mut _0: >::Output; bb0: { diff --git a/tests/ui/codegen/subtyping-impacts-selection-1.rs b/tests/ui/codegen/subtyping-impacts-selection-1.rs new file mode 100644 index 0000000000000000000000000000000000000000..09e06f6d6843b9e0145078b7480739212565f912 --- /dev/null +++ b/tests/ui/codegen/subtyping-impacts-selection-1.rs @@ -0,0 +1,44 @@ +// run-pass +// revisions: mir codegen +//[mir] compile-flags: -Zmir-opt-level=3 +//[codegen] compile-flags: -Zmir-opt-level=0 + +// A regression test for #107205 +#![allow(coherence_leak_check)] +struct Foo(T); + +fn useful<'a>(_: &'a u8) {} + +trait GetInner { + type Assoc; + fn muahaha(&mut self) -> Self::Assoc; +} + +impl GetInner for Foo { + type Assoc = String; + fn muahaha(&mut self) -> String { + String::from("I am a string") + } +} + +impl GetInner for Foo fn(&'a u8)> { + type Assoc = [usize; 3]; + fn muahaha(&mut self) -> [usize; 3] { + [100; 3] + } +} + +fn break_me(hr_fnptr: Box fn(&'a u8)>>) -> Box> { + let lr_fnptr = hr_fnptr as Box>; + lr_fnptr as Box> +} + +fn main() { + drop(Box::new(Foo(useful as fn(&'static u8))) as Box>); + drop(Box::new(Foo(useful as fn(&u8))) as Box>); + + let mut any = break_me(Box::new(Foo(useful))); + + let evil_string = any.muahaha(); + assert_eq!(evil_string, "I am a string"); +} diff --git a/tests/ui/codegen/subtyping-impacts-selection-2.rs b/tests/ui/codegen/subtyping-impacts-selection-2.rs new file mode 100644 index 0000000000000000000000000000000000000000..921136775b7fd950cb5d0534cbff8c7522efdb3c --- /dev/null +++ b/tests/ui/codegen/subtyping-impacts-selection-2.rs @@ -0,0 +1,12 @@ +// run-pass +// revisions: mir codegen +//[mir] compile-flags: -Zmir-opt-level=3 +//[codegen] compile-flags: -Zmir-opt-level=0 + +// A regression test for #107205 + +const X: for<'b> fn(&'b ()) = |&()| (); +fn main() { + let dyn_debug = Box::new(X) as Box as Box; + drop(dyn_debug) +}