From 897a65804d7891c2d4518d6c6061e7baedaa745b Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 9 Sep 2023 10:35:26 +0200 Subject: [PATCH 1/7] interpret: change ABI-compat test to be type-based, so the test is consistent across targets --- .../src/interpret/terminator.rs | 175 +++++++++++++----- .../tests/pass/function_calls/abi_compat.rs | 39 ++-- 2 files changed, 146 insertions(+), 68 deletions(-) diff --git a/compiler/rustc_const_eval/src/interpret/terminator.rs b/compiler/rustc_const_eval/src/interpret/terminator.rs index eb4673c0edcd5..eeeec97936b33 100644 --- a/compiler/rustc_const_eval/src/interpret/terminator.rs +++ b/compiler/rustc_const_eval/src/interpret/terminator.rs @@ -6,12 +6,16 @@ use rustc_middle::{ mir, ty::{ self, - layout::{FnAbiOf, LayoutOf, TyAndLayout}, + layout::{FnAbiOf, IntegerExt, LayoutOf, TyAndLayout}, Instance, Ty, }, }; -use rustc_target::abi::call::{ArgAbi, FnAbi, PassMode}; -use rustc_target::abi::{self, FieldIdx}; +use rustc_span::sym; +use rustc_target::abi::FieldIdx; +use rustc_target::abi::{ + call::{ArgAbi, FnAbi, PassMode}, + Integer, +}; use rustc_target::spec::abi::Abi; use super::{ @@ -255,6 +259,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { /// Find the wrapped inner type of a transparent wrapper. /// Must not be called on 1-ZST (as they don't have a uniquely defined "wrapped field"). + /// + /// We work with `TyAndLayout` here since that makes it much easier to iterate over all fields. fn unfold_transparent(&self, layout: TyAndLayout<'tcx>) -> TyAndLayout<'tcx> { match layout.ty.kind() { ty::Adt(adt_def, _) if adt_def.repr().transparent() => { @@ -278,6 +284,37 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { } } + /// Unwrap types that are guaranteed a null-pointer-optimization + fn unfold_npo(&self, ty: Ty<'tcx>) -> InterpResult<'tcx, Ty<'tcx>> { + // Check if this is `Option` wrapping some type. + let inner_ty = match ty.kind() { + ty::Adt(def, args) if self.tcx.is_diagnostic_item(sym::Option, def.did()) => { + args[0].as_type().unwrap() + } + _ => { + // Not an `Option`. + return Ok(ty); + } + }; + // Check if the inner type is one of the NPO-guaranteed ones. + Ok(match inner_ty.kind() { + ty::Ref(..) => { + // Option<&T> behaves like &T + inner_ty + } + ty::Adt(def, _) + if self.tcx.has_attr(def.did(), sym::rustc_nonnull_optimization_guaranteed) => + { + // For non-null-guaranteed structs, unwrap newtypes. + self.unfold_transparent(self.layout_of(inner_ty)?).ty + } + _ => { + // Everything else we do not unfold. + ty + } + }) + } + /// Check if these two layouts look like they are fn-ABI-compatible. /// (We also compare the `PassMode`, so this doesn't have to check everything. But it turns out /// that only checking the `PassMode` is insufficient.) @@ -285,63 +322,86 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { &self, caller_layout: TyAndLayout<'tcx>, callee_layout: TyAndLayout<'tcx>, - ) -> bool { + ) -> InterpResult<'tcx, bool> { + // Fast path: equal types are definitely compatible. if caller_layout.ty == callee_layout.ty { - // Fast path: equal types are definitely compatible. - return true; + return Ok(true); } - - match caller_layout.abi { - // For Scalar/Vector/ScalarPair ABI, we directly compare them. - // NOTE: this is *not* a stable guarantee! It just reflects a property of our current - // ABIs. It's also fragile; the same pair of types might be considered ABI-compatible - // when used directly by-value but not considered compatible as a struct field or array - // element. - abi::Abi::Scalar(..) | abi::Abi::ScalarPair(..) | abi::Abi::Vector { .. } => { - caller_layout.abi.eq_up_to_validity(&callee_layout.abi) - } - _ => { - // Everything else is compatible only if they newtype-wrap the same type, or if they are both 1-ZST. - // (The latter part is needed to ensure e.g. that `struct Zst` is compatible with `struct Wrap((), Zst)`.) - // This is conservative, but also means that our check isn't quite so heavily dependent on the `PassMode`, - // which means having ABI-compatibility on one target is much more likely to imply compatibility for other targets. - if caller_layout.is_1zst() || callee_layout.is_1zst() { - // If either is a 1-ZST, both must be. - caller_layout.is_1zst() && callee_layout.is_1zst() - } else { - // Neither is a 1-ZST, so we can check what they are wrapping. - self.unfold_transparent(caller_layout).ty - == self.unfold_transparent(callee_layout).ty + // 1-ZST are compatible with all 1-ZST (and with nothing else). + if caller_layout.is_1zst() || callee_layout.is_1zst() { + return Ok(caller_layout.is_1zst() && callee_layout.is_1zst()); + } + // Unfold newtypes and NPO optimizations. + let caller_ty = self.unfold_npo(self.unfold_transparent(caller_layout).ty)?; + let callee_ty = self.unfold_npo(self.unfold_transparent(callee_layout).ty)?; + // Now see if these inner types are compatible. + + // Compatible pointer types. + let pointee_ty = |ty: Ty<'tcx>| { + // We cannot use `builtin_deref` here since we need to reject `Box`. + Some(match ty.kind() { + ty::Ref(_, ty, _) => *ty, + ty::RawPtr(mt) => mt.ty, + // We should only accept `Box` with the default allocator. + // It's hard to test for that though so we accept every 1-ZST allocator. + ty::Adt(def, args) + if def.is_box() + && self.layout_of(args[1].expect_ty()).is_ok_and(|l| l.is_1zst()) => + { + args[0].expect_ty() } - } + _ => return None, + }) + }; + if let (Some(left), Some(right)) = (pointee_ty(caller_ty), pointee_ty(callee_ty)) { + // This is okay if they have the same metadata type. + let meta_ty = |ty: Ty<'tcx>| { + let (meta, only_if_sized) = ty.ptr_metadata_ty(*self.tcx, |ty| ty); + assert!( + !only_if_sized, + "there should be no more 'maybe has that metadata' types during interpretation" + ); + meta + }; + return Ok(meta_ty(left) == meta_ty(right)); } + + // Compatible integer types (in particular, usize vs ptr-sized-u32/u64). + let int_ty = |ty: Ty<'tcx>| { + Some(match ty.kind() { + ty::Int(ity) => (Integer::from_int_ty(&self.tcx, *ity), /* signed */ true), + ty::Uint(uty) => (Integer::from_uint_ty(&self.tcx, *uty), /* signed */ false), + _ => return None, + }) + }; + if let (Some(left), Some(right)) = (int_ty(caller_ty), int_ty(callee_ty)) { + // This is okay if they are the same integer type. + return Ok(left == right); + } + + // Fall back to exact equality. + // FIXME: We are missing the rules for "repr(C) wrapping compatible types". + Ok(caller_ty == callee_ty) } fn check_argument_compat( &self, caller_abi: &ArgAbi<'tcx, Ty<'tcx>>, callee_abi: &ArgAbi<'tcx, Ty<'tcx>>, - ) -> bool { - // Ideally `PassMode` would capture everything there is about argument passing, but that is - // not the case: in `FnAbi::llvm_type`, also parts of the layout and type information are - // used. So we need to check that *both* sufficiently agree to ensures the arguments are - // compatible. - // For instance, `layout_compat` is needed to reject `i32` vs `f32`, which is not reflected - // in `PassMode`. `mode_compat` is needed to reject `u8` vs `bool`, which have the same - // `abi::Primitive` but different `arg_ext`. - if self.layout_compat(caller_abi.layout, callee_abi.layout) - && caller_abi.mode.eq_abi(&callee_abi.mode) - { - // Something went very wrong if our checks don't imply layout ABI compatibility. - assert!(caller_abi.layout.eq_abi(&callee_abi.layout)); - return true; + ) -> InterpResult<'tcx, bool> { + // We do not want to accept things as ABI-compatible that just "happen to be" compatible on the current target, + // so we implement a type-based check that reflects the guaranteed rules for ABI compatibility. + if self.layout_compat(caller_abi.layout, callee_abi.layout)? { + // Ensure that our checks imply actual ABI compatibility for this concrete call. + assert!(caller_abi.eq_abi(&callee_abi)); + return Ok(true); } else { trace!( "check_argument_compat: incompatible ABIs:\ncaller: {:?}\ncallee: {:?}", caller_abi, callee_abi ); - return false; + return Ok(false); } } @@ -360,6 +420,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { 'tcx: 'x, 'tcx: 'y, { + assert_eq!(callee_ty, callee_abi.layout.ty); if matches!(callee_abi.mode, PassMode::Ignore) { // This one is skipped. Still must be made live though! if !already_live { @@ -371,8 +432,13 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { let Some((caller_arg, caller_abi)) = caller_args.next() else { throw_ub_custom!(fluent::const_eval_not_enough_caller_args); }; + assert_eq!(caller_arg.layout().layout, caller_abi.layout.layout); + // Sadly we cannot assert that `caller_arg.layout().ty` and `caller_abi.layout.ty` are + // equal; in closures the types sometimes differ. We just hope that `caller_abi` is the + // right type to print to the user. + // Check compatibility - if !self.check_argument_compat(caller_abi, callee_abi) { + if !self.check_argument_compat(caller_abi, callee_abi)? { let callee_ty = format!("{}", callee_ty); let caller_ty = format!("{}", caller_arg.layout().ty); throw_ub_custom!( @@ -583,7 +649,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { // taking into account the `spread_arg`. If we could write // this is a single iterator (that handles `spread_arg`), then // `pass_argument` would be the loop body. It takes care to - // not advance `caller_iter` for ZSTs. + // not advance `caller_iter` for ignored arguments. let mut callee_args_abis = callee_fn_abi.args.iter(); for local in body.args_iter() { // Construct the destination place for this argument. At this point all @@ -645,7 +711,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { throw_ub_custom!(fluent::const_eval_too_many_caller_args); } // Don't forget to check the return type! - if !self.check_argument_compat(&caller_fn_abi.ret, &callee_fn_abi.ret) { + if !self.check_argument_compat(&caller_fn_abi.ret, &callee_fn_abi.ret)? { let callee_ty = format!("{}", callee_fn_abi.ret.layout.ty); let caller_ty = format!("{}", caller_fn_abi.ret.layout.ty); throw_ub_custom!( @@ -674,7 +740,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { Ok(()) => Ok(()), } } - // cannot use the shim here, because that will only result in infinite recursion + // `InstanceDef::Virtual` does not have callable MIR. Calls to `Virtual` instances must be + // codegen'd / interpreted as virtual calls through the vtable. ty::InstanceDef::Virtual(def_id, idx) => { let mut args = args.to_vec(); // We have to implement all "object safe receivers". So we have to go search for a @@ -798,18 +865,26 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { } // Adjust receiver argument. Layout can be any (thin) ptr. + let receiver_ty = Ty::new_mut_ptr(self.tcx.tcx, dyn_ty); args[0] = FnArg::Copy( ImmTy::from_immediate( Scalar::from_maybe_pointer(adjusted_receiver, self).into(), - self.layout_of(Ty::new_mut_ptr(self.tcx.tcx, dyn_ty))?, + self.layout_of(receiver_ty)?, ) .into(), ); trace!("Patched receiver operand to {:#?}", args[0]); + // Need to also adjust the type in the ABI. Strangely, the layout there is actually + // already fine! Just the type is bogus. This is due to what `force_thin_self_ptr` + // does in `fn_abi_new_uncached`; supposedly, codegen relies on having the bogus + // type, so we just patch this up locally. + let mut caller_fn_abi = caller_fn_abi.clone(); + caller_fn_abi.args[0].layout.ty = receiver_ty; + // recurse with concrete function self.eval_fn_call( FnVal::Instance(fn_inst), - (caller_abi, caller_fn_abi), + (caller_abi, &caller_fn_abi), &args, with_caller_location, destination, diff --git a/src/tools/miri/tests/pass/function_calls/abi_compat.rs b/src/tools/miri/tests/pass/function_calls/abi_compat.rs index 08be29115ca0b..714d5c43b491a 100644 --- a/src/tools/miri/tests/pass/function_calls/abi_compat.rs +++ b/src/tools/miri/tests/pass/function_calls/abi_compat.rs @@ -1,10 +1,11 @@ use std::mem; use std::num; +use std::ptr; #[derive(Copy, Clone, Default)] struct Zst; -fn test_abi_compat(t: T, u: U) { +fn test_abi_compat(t: T, u: U) { fn id(x: T) -> T { x } @@ -16,10 +17,10 @@ fn test_abi_compat(t: T, u: U) { // in both directions. let f: fn(T) -> T = id; let f: fn(U) -> U = unsafe { std::mem::transmute(f) }; - let _val = f(u); + let _val = f(u.clone()); let f: fn(U) -> U = id; let f: fn(T) -> T = unsafe { std::mem::transmute(f) }; - let _val = f(t); + let _val = f(t.clone()); // And then we do the same for `extern "C"`. let f: extern "C" fn(T) -> T = id_c; @@ -54,23 +55,25 @@ fn test_abi_newtype() { } fn main() { - // Here we check: - // - u32 vs char is allowed - // - u32 vs NonZeroU32/Option is allowed - // - reference vs raw pointer is allowed - // - references to things of the same size and alignment are allowed - // These are very basic tests that should work on all ABIs. However it is not clear that any of - // these would be stably guaranteed. Code that relies on this is equivalent to code that relies - // on the layout of `repr(Rust)` types. They are also fragile: the same mismatches in the fields - // of a struct (even with `repr(C)`) will not always be accepted by Miri. - // Note that `bool` and `u8` are *not* compatible, at least on x86-64! - // One of them has `arg_ext: Zext`, the other does not. - // Similarly, `i32` and `u32` are not compatible on s390x due to different `arg_ext`. - test_abi_compat(0u32, 'x'); - test_abi_compat(42u32, num::NonZeroU32::new(1).unwrap()); - test_abi_compat(0u32, Some(num::NonZeroU32::new(1).unwrap())); + // Here we check some of the guaranteed ABI compatibilities. + // Different integer types of the same size and sign. + if cfg!(target_pointer_width = "32") { + test_abi_compat(0usize, 0u32); + test_abi_compat(0isize, 0i32); + } else { + test_abi_compat(0usize, 0u64); + test_abi_compat(0isize, 0i64); + } + // Reference/pointer types with the same pointee. test_abi_compat(&0u32, &0u32 as *const u32); + test_abi_compat(&mut 0u32 as *mut u32, Box::new(0u32)); + test_abi_compat(&(), ptr::NonNull::<()>::dangling()); + // Reference/pointer types with different but sized pointees. test_abi_compat(&0u32, &([true; 4], [0u32; 0])); + // Guaranteed null-pointer-optimizations. + test_abi_compat(&0u32 as *const u32, Some(&0u32)); + test_abi_compat(42u32, num::NonZeroU32::new(1).unwrap()); + test_abi_compat(0u32, Some(num::NonZeroU32::new(1).unwrap())); // These must work for *any* type, since we guarantee that `repr(transparent)` is ABI-compatible // with the wrapped field. From f993ddc079112bc0b630b1fb38e141239b36cca6 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 9 Sep 2023 10:50:14 +0200 Subject: [PATCH 2/7] give extra context to ABI mismatch errors --- compiler/rustc_const_eval/src/errors.rs | 27 ++++++++++++------- .../src/interpret/terminator.rs | 22 ++++++--------- .../rustc_middle/src/mir/interpret/error.rs | 19 ++++++++----- src/tools/miri/src/diagnostics.rs | 24 +++++++++++------ .../abi_mismatch_array_vs_struct.stderr | 2 ++ .../abi_mismatch_int_vs_float.stderr | 2 ++ .../abi_mismatch_raw_pointer.stderr | 2 ++ .../abi_mismatch_return_type.stderr | 2 ++ .../abi_mismatch_simple.stderr | 2 ++ .../abi_mismatch_vector.stderr | 2 ++ 10 files changed, 66 insertions(+), 38 deletions(-) diff --git a/compiler/rustc_const_eval/src/errors.rs b/compiler/rustc_const_eval/src/errors.rs index c74fed0e47f52..c3c6cbe399109 100644 --- a/compiler/rustc_const_eval/src/errors.rs +++ b/compiler/rustc_const_eval/src/errors.rs @@ -482,6 +482,9 @@ impl<'a> ReportErrorExt for UndefinedBehaviorInfo<'a> { use UndefinedBehaviorInfo::*; match self { Ub(msg) => msg.clone().into(), + Custom(x) => (x.msg)(), + ValidationError(e) => e.diagnostic_message(), + Unreachable => const_eval_unreachable, BoundsCheckFailed { .. } => const_eval_bounds_check_failed, DivisionByZero => const_eval_division_by_zero, @@ -513,8 +516,8 @@ impl<'a> ReportErrorExt for UndefinedBehaviorInfo<'a> { ScalarSizeMismatch(_) => const_eval_scalar_size_mismatch, UninhabitedEnumVariantWritten(_) => const_eval_uninhabited_enum_variant_written, UninhabitedEnumVariantRead(_) => const_eval_uninhabited_enum_variant_read, - ValidationError(e) => e.diagnostic_message(), - Custom(x) => (x.msg)(), + AbiMismatchArgument { .. } => const_eval_incompatible_types, + AbiMismatchReturn { .. } => const_eval_incompatible_return_types, } } @@ -525,8 +528,15 @@ impl<'a> ReportErrorExt for UndefinedBehaviorInfo<'a> { ) { use UndefinedBehaviorInfo::*; match self { - Ub(_) - | Unreachable + Ub(_) => {} + Custom(custom) => { + (custom.add_args)(&mut |name, value| { + builder.set_arg(name, value); + }); + } + ValidationError(e) => e.add_args(handler, builder), + + Unreachable | DivisionByZero | RemainderByZero | DivisionOverflow @@ -593,11 +603,10 @@ impl<'a> ReportErrorExt for UndefinedBehaviorInfo<'a> { builder.set_arg("target_size", info.target_size); builder.set_arg("data_size", info.data_size); } - ValidationError(e) => e.add_args(handler, builder), - Custom(custom) => { - (custom.add_args)(&mut |name, value| { - builder.set_arg(name, value); - }); + AbiMismatchArgument { caller_ty, callee_ty } + | AbiMismatchReturn { caller_ty, callee_ty } => { + builder.set_arg("caller_ty", caller_ty.to_string()); + builder.set_arg("callee_ty", callee_ty.to_string()); } } } diff --git a/compiler/rustc_const_eval/src/interpret/terminator.rs b/compiler/rustc_const_eval/src/interpret/terminator.rs index eeeec97936b33..726a4e6f8a03c 100644 --- a/compiler/rustc_const_eval/src/interpret/terminator.rs +++ b/compiler/rustc_const_eval/src/interpret/terminator.rs @@ -439,13 +439,10 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { // Check compatibility if !self.check_argument_compat(caller_abi, callee_abi)? { - let callee_ty = format!("{}", callee_ty); - let caller_ty = format!("{}", caller_arg.layout().ty); - throw_ub_custom!( - fluent::const_eval_incompatible_types, - callee_ty = callee_ty, - caller_ty = caller_ty, - ) + throw_ub!(AbiMismatchArgument { + caller_ty: caller_abi.layout.ty, + callee_ty: callee_abi.layout.ty + }); } // We work with a copy of the argument for now; if this is in-place argument passing, we // will later protect the source it comes from. This means the callee cannot observe if we @@ -712,13 +709,10 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { } // Don't forget to check the return type! if !self.check_argument_compat(&caller_fn_abi.ret, &callee_fn_abi.ret)? { - let callee_ty = format!("{}", callee_fn_abi.ret.layout.ty); - let caller_ty = format!("{}", caller_fn_abi.ret.layout.ty); - throw_ub_custom!( - fluent::const_eval_incompatible_return_types, - callee_ty = callee_ty, - caller_ty = caller_ty, - ) + throw_ub!(AbiMismatchReturn { + caller_ty: caller_fn_abi.ret.layout.ty, + callee_ty: callee_fn_abi.ret.layout.ty + }); } // Ensure the return place is aligned and dereferenceable, and protect it for // in-place return value passing. diff --git a/compiler/rustc_middle/src/mir/interpret/error.rs b/compiler/rustc_middle/src/mir/interpret/error.rs index 577cd20b74ca7..76ac2b2ac607a 100644 --- a/compiler/rustc_middle/src/mir/interpret/error.rs +++ b/compiler/rustc_middle/src/mir/interpret/error.rs @@ -255,9 +255,16 @@ impl_into_diagnostic_arg_through_debug! { /// Error information for when the program caused Undefined Behavior. #[derive(Debug)] -pub enum UndefinedBehaviorInfo<'a> { +pub enum UndefinedBehaviorInfo<'tcx> { /// Free-form case. Only for errors that are never caught! Used by miri Ub(String), + // FIXME(fee1-dead) these should all be actual variants of the enum instead of dynamically + // dispatched + /// A custom (free-form) fluent-translated error, created by `err_ub_custom!`. + Custom(crate::error::CustomSubdiagnostic<'tcx>), + /// Validation error. + ValidationError(ValidationErrorInfo<'tcx>), + /// Unreachable code was executed. Unreachable, /// A slice/array index projection went out-of-bounds. @@ -319,12 +326,10 @@ pub enum UndefinedBehaviorInfo<'a> { UninhabitedEnumVariantWritten(VariantIdx), /// An uninhabited enum variant is projected. UninhabitedEnumVariantRead(VariantIdx), - /// Validation error. - ValidationError(ValidationErrorInfo<'a>), - // FIXME(fee1-dead) these should all be actual variants of the enum instead of dynamically - // dispatched - /// A custom (free-form) error, created by `err_ub_custom!`. - Custom(crate::error::CustomSubdiagnostic<'a>), + /// ABI-incompatible argument types. + AbiMismatchArgument { caller_ty: Ty<'tcx>, callee_ty: Ty<'tcx> }, + /// ABI-incompatible return types. + AbiMismatchReturn { caller_ty: Ty<'tcx>, callee_ty: Ty<'tcx> }, } #[derive(Debug, Clone, Copy)] diff --git a/src/tools/miri/src/diagnostics.rs b/src/tools/miri/src/diagnostics.rs index 4f249acda1df1..cb095f94f3598 100644 --- a/src/tools/miri/src/diagnostics.rs +++ b/src/tools/miri/src/diagnostics.rs @@ -199,6 +199,7 @@ pub fn report_error<'tcx, 'mir>( e: InterpErrorInfo<'tcx>, ) -> Option<(i64, bool)> { use InterpError::*; + use UndefinedBehaviorInfo::*; let mut msg = vec![]; @@ -271,7 +272,7 @@ pub fn report_error<'tcx, 'mir>( (title, helps) } else { let title = match e.kind() { - UndefinedBehavior(UndefinedBehaviorInfo::ValidationError(validation_err)) + UndefinedBehavior(ValidationError(validation_err)) if matches!( validation_err.kind, ValidationErrorKind::PointerAsInt { .. } | ValidationErrorKind::PartialPointer @@ -299,7 +300,7 @@ pub fn report_error<'tcx, 'mir>( let helps = match e.kind() { Unsupported(_) => vec![(None, format!("this is likely not a bug in the program; it indicates that the program performed an operation that the interpreter does not support"))], - UndefinedBehavior(UndefinedBehaviorInfo::AlignmentCheckFailed { .. }) + UndefinedBehavior(AlignmentCheckFailed { .. }) if ecx.machine.check_alignment == AlignmentCheck::Symbolic => vec![ @@ -311,13 +312,20 @@ pub fn report_error<'tcx, 'mir>( (None, format!("this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior")), (None, format!("see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information")), ]; - if let UndefinedBehaviorInfo::PointerUseAfterFree(alloc_id, _) | UndefinedBehaviorInfo::PointerOutOfBounds { alloc_id, .. } = info { - if let Some(span) = ecx.machine.allocated_span(*alloc_id) { - helps.push((Some(span), format!("{:?} was allocated here:", alloc_id))); + match info { + PointerUseAfterFree(alloc_id, _) | PointerOutOfBounds { alloc_id, .. } => { + if let Some(span) = ecx.machine.allocated_span(*alloc_id) { + helps.push((Some(span), format!("{:?} was allocated here:", alloc_id))); + } + if let Some(span) = ecx.machine.deallocated_span(*alloc_id) { + helps.push((Some(span), format!("{:?} was deallocated here:", alloc_id))); + } } - if let Some(span) = ecx.machine.deallocated_span(*alloc_id) { - helps.push((Some(span), format!("{:?} was deallocated here:", alloc_id))); + AbiMismatchArgument { .. } | AbiMismatchReturn { .. } => { + helps.push((None, format!("this means these two types are not *guaranteed* to be ABI-compatible across all targets"))); + helps.push((None, format!("if you think this code should be accepted anyway, please report an issue"))); } + _ => {}, } helps } @@ -339,7 +347,7 @@ pub fn report_error<'tcx, 'mir>( // We want to dump the allocation if this is `InvalidUninitBytes`. Since `format_error` consumes `e`, we compute the outut early. let mut extra = String::new(); match e.kind() { - UndefinedBehavior(UndefinedBehaviorInfo::InvalidUninitBytes(Some((alloc_id, access)))) => { + UndefinedBehavior(InvalidUninitBytes(Some((alloc_id, access)))) => { writeln!( extra, "Uninitialized memory occurred at {alloc_id:?}{range:?}, in this allocation:", diff --git a/src/tools/miri/tests/fail/function_pointers/abi_mismatch_array_vs_struct.stderr b/src/tools/miri/tests/fail/function_pointers/abi_mismatch_array_vs_struct.stderr index 50d4228c11111..d1ccaf89974cc 100644 --- a/src/tools/miri/tests/fail/function_pointers/abi_mismatch_array_vs_struct.stderr +++ b/src/tools/miri/tests/fail/function_pointers/abi_mismatch_array_vs_struct.stderr @@ -6,6 +6,8 @@ LL | g(Default::default()) | = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information + = help: this means these two types are not *guaranteed* to be ABI-compatible across all targets + = help: if you think this code should be accepted anyway, please report an issue = note: BACKTRACE: = note: inside `main` at $DIR/abi_mismatch_array_vs_struct.rs:LL:CC diff --git a/src/tools/miri/tests/fail/function_pointers/abi_mismatch_int_vs_float.stderr b/src/tools/miri/tests/fail/function_pointers/abi_mismatch_int_vs_float.stderr index a53126c733eca..3875c2617bb59 100644 --- a/src/tools/miri/tests/fail/function_pointers/abi_mismatch_int_vs_float.stderr +++ b/src/tools/miri/tests/fail/function_pointers/abi_mismatch_int_vs_float.stderr @@ -6,6 +6,8 @@ LL | g(42) | = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information + = help: this means these two types are not *guaranteed* to be ABI-compatible across all targets + = help: if you think this code should be accepted anyway, please report an issue = note: BACKTRACE: = note: inside `main` at $DIR/abi_mismatch_int_vs_float.rs:LL:CC diff --git a/src/tools/miri/tests/fail/function_pointers/abi_mismatch_raw_pointer.stderr b/src/tools/miri/tests/fail/function_pointers/abi_mismatch_raw_pointer.stderr index 6eacfeece1497..6d1bdfee0073f 100644 --- a/src/tools/miri/tests/fail/function_pointers/abi_mismatch_raw_pointer.stderr +++ b/src/tools/miri/tests/fail/function_pointers/abi_mismatch_raw_pointer.stderr @@ -6,6 +6,8 @@ LL | g(&42 as *const i32) | = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information + = help: this means these two types are not *guaranteed* to be ABI-compatible across all targets + = help: if you think this code should be accepted anyway, please report an issue = note: BACKTRACE: = note: inside `main` at $DIR/abi_mismatch_raw_pointer.rs:LL:CC diff --git a/src/tools/miri/tests/fail/function_pointers/abi_mismatch_return_type.stderr b/src/tools/miri/tests/fail/function_pointers/abi_mismatch_return_type.stderr index eedc1235773e2..07d76c90e5e56 100644 --- a/src/tools/miri/tests/fail/function_pointers/abi_mismatch_return_type.stderr +++ b/src/tools/miri/tests/fail/function_pointers/abi_mismatch_return_type.stderr @@ -6,6 +6,8 @@ LL | g() | = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information + = help: this means these two types are not *guaranteed* to be ABI-compatible across all targets + = help: if you think this code should be accepted anyway, please report an issue = note: BACKTRACE: = note: inside `main` at $DIR/abi_mismatch_return_type.rs:LL:CC diff --git a/src/tools/miri/tests/fail/function_pointers/abi_mismatch_simple.stderr b/src/tools/miri/tests/fail/function_pointers/abi_mismatch_simple.stderr index bc500a90b7772..7ac2bc2739fed 100644 --- a/src/tools/miri/tests/fail/function_pointers/abi_mismatch_simple.stderr +++ b/src/tools/miri/tests/fail/function_pointers/abi_mismatch_simple.stderr @@ -6,6 +6,8 @@ LL | g(42) | = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information + = help: this means these two types are not *guaranteed* to be ABI-compatible across all targets + = help: if you think this code should be accepted anyway, please report an issue = note: BACKTRACE: = note: inside `main` at $DIR/abi_mismatch_simple.rs:LL:CC diff --git a/src/tools/miri/tests/fail/function_pointers/abi_mismatch_vector.stderr b/src/tools/miri/tests/fail/function_pointers/abi_mismatch_vector.stderr index 7dcca1e85b870..e082eb9e3e3d4 100644 --- a/src/tools/miri/tests/fail/function_pointers/abi_mismatch_vector.stderr +++ b/src/tools/miri/tests/fail/function_pointers/abi_mismatch_vector.stderr @@ -6,6 +6,8 @@ LL | g(Default::default()) | = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information + = help: this means these two types are not *guaranteed* to be ABI-compatible across all targets + = help: if you think this code should be accepted anyway, please report an issue = note: BACKTRACE: = note: inside `main` at $DIR/abi_mismatch_vector.rs:LL:CC From b5bab2b1cc731302fe515fa434cd30f8d0a18a17 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 9 Sep 2023 11:17:42 +0200 Subject: [PATCH 3/7] implement and test fn ptr ABI compatibility rules --- compiler/rustc_const_eval/src/interpret/terminator.rs | 9 +++++++-- src/tools/miri/tests/pass/function_calls/abi_compat.rs | 5 +++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_const_eval/src/interpret/terminator.rs b/compiler/rustc_const_eval/src/interpret/terminator.rs index 726a4e6f8a03c..3ea7e77197e0b 100644 --- a/compiler/rustc_const_eval/src/interpret/terminator.rs +++ b/compiler/rustc_const_eval/src/interpret/terminator.rs @@ -298,8 +298,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { }; // Check if the inner type is one of the NPO-guaranteed ones. Ok(match inner_ty.kind() { - ty::Ref(..) => { - // Option<&T> behaves like &T + ty::Ref(..) | ty::FnPtr(..) => { + // Option<&T> behaves like &T, and same for fn() inner_ty } ty::Adt(def, _) @@ -366,6 +366,11 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { return Ok(meta_ty(left) == meta_ty(right)); } + // Compatible function pointer types. + if let (ty::FnPtr(..), ty::FnPtr(..)) = (caller_ty.kind(), callee_ty.kind()) { + return Ok(true); + } + // Compatible integer types (in particular, usize vs ptr-sized-u32/u64). let int_ty = |ty: Ty<'tcx>| { Some(match ty.kind() { diff --git a/src/tools/miri/tests/pass/function_calls/abi_compat.rs b/src/tools/miri/tests/pass/function_calls/abi_compat.rs index 714d5c43b491a..90dc73d19cb2f 100644 --- a/src/tools/miri/tests/pass/function_calls/abi_compat.rs +++ b/src/tools/miri/tests/pass/function_calls/abi_compat.rs @@ -5,6 +5,8 @@ use std::ptr; #[derive(Copy, Clone, Default)] struct Zst; +fn id(x: T) -> T { x } + fn test_abi_compat(t: T, u: U) { fn id(x: T) -> T { x @@ -70,8 +72,11 @@ fn main() { test_abi_compat(&(), ptr::NonNull::<()>::dangling()); // Reference/pointer types with different but sized pointees. test_abi_compat(&0u32, &([true; 4], [0u32; 0])); + // `fn` types + test_abi_compat(main as fn(), id:: as fn(i32) -> i32); // Guaranteed null-pointer-optimizations. test_abi_compat(&0u32 as *const u32, Some(&0u32)); + test_abi_compat(main as fn(), Some(main as fn())); test_abi_compat(42u32, num::NonZeroU32::new(1).unwrap()); test_abi_compat(0u32, Some(num::NonZeroU32::new(1).unwrap())); From a38a3bfc6dcc01de2b6e54f28a766997a1fda304 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 9 Sep 2023 11:36:35 +0200 Subject: [PATCH 4/7] implement and test ABI compatibility for transparent wrappers around NPO types --- .../src/interpret/terminator.rs | 41 ++++++++++++------- .../tests/pass/function_calls/abi_compat.rs | 13 +++--- 2 files changed, 35 insertions(+), 19 deletions(-) diff --git a/compiler/rustc_const_eval/src/interpret/terminator.rs b/compiler/rustc_const_eval/src/interpret/terminator.rs index 3ea7e77197e0b..437a035d08211 100644 --- a/compiler/rustc_const_eval/src/interpret/terminator.rs +++ b/compiler/rustc_const_eval/src/interpret/terminator.rs @@ -7,7 +7,7 @@ use rustc_middle::{ ty::{ self, layout::{FnAbiOf, IntegerExt, LayoutOf, TyAndLayout}, - Instance, Ty, + AdtDef, Instance, Ty, }, }; use rustc_span::sym; @@ -261,9 +261,13 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { /// Must not be called on 1-ZST (as they don't have a uniquely defined "wrapped field"). /// /// We work with `TyAndLayout` here since that makes it much easier to iterate over all fields. - fn unfold_transparent(&self, layout: TyAndLayout<'tcx>) -> TyAndLayout<'tcx> { + fn unfold_transparent( + &self, + layout: TyAndLayout<'tcx>, + may_unfold: impl Fn(AdtDef<'tcx>) -> bool, + ) -> TyAndLayout<'tcx> { match layout.ty.kind() { - ty::Adt(adt_def, _) if adt_def.repr().transparent() => { + ty::Adt(adt_def, _) if adt_def.repr().transparent() && may_unfold(*adt_def) => { assert!(!adt_def.is_enum()); // Find the non-1-ZST field. let mut non_1zst_fields = (0..layout.fields.count()).filter_map(|idx| { @@ -277,7 +281,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { ); // Found it! - self.unfold_transparent(first) + self.unfold_transparent(first, may_unfold) } // Not a transparent type, no further unfolding. _ => layout, @@ -287,7 +291,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { /// Unwrap types that are guaranteed a null-pointer-optimization fn unfold_npo(&self, ty: Ty<'tcx>) -> InterpResult<'tcx, Ty<'tcx>> { // Check if this is `Option` wrapping some type. - let inner_ty = match ty.kind() { + let inner = match ty.kind() { ty::Adt(def, args) if self.tcx.is_diagnostic_item(sym::Option, def.did()) => { args[0].as_type().unwrap() } @@ -297,16 +301,23 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { } }; // Check if the inner type is one of the NPO-guaranteed ones. - Ok(match inner_ty.kind() { + // For that we first unpeel transparent *structs* (but not unions). + let is_npo = |def: AdtDef<'tcx>| { + self.tcx.has_attr(def.did(), sym::rustc_nonnull_optimization_guaranteed) + }; + let inner = self.unfold_transparent(self.layout_of(inner)?, /* may_unfold */ |def| { + // Stop at NPO tpyes so that we don't miss that attribute in the check below! + def.is_struct() && !is_npo(def) + }); + Ok(match inner.ty.kind() { ty::Ref(..) | ty::FnPtr(..) => { // Option<&T> behaves like &T, and same for fn() - inner_ty + inner.ty } - ty::Adt(def, _) - if self.tcx.has_attr(def.did(), sym::rustc_nonnull_optimization_guaranteed) => - { - // For non-null-guaranteed structs, unwrap newtypes. - self.unfold_transparent(self.layout_of(inner_ty)?).ty + ty::Adt(def, _) if is_npo(*def) => { + // Once we found a `nonnull_optimization_guaranteed` type, further strip off + // newtype structs from it to find the underlying ABI type. + self.unfold_transparent(inner, /* may_unfold */ |def| def.is_struct()).ty } _ => { // Everything else we do not unfold. @@ -332,8 +343,10 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { return Ok(caller_layout.is_1zst() && callee_layout.is_1zst()); } // Unfold newtypes and NPO optimizations. - let caller_ty = self.unfold_npo(self.unfold_transparent(caller_layout).ty)?; - let callee_ty = self.unfold_npo(self.unfold_transparent(callee_layout).ty)?; + let caller_ty = + self.unfold_npo(self.unfold_transparent(caller_layout, /* may_unfold */ |_| true).ty)?; + let callee_ty = + self.unfold_npo(self.unfold_transparent(callee_layout, /* may_unfold */ |_| true).ty)?; // Now see if these inner types are compatible. // Compatible pointer types. diff --git a/src/tools/miri/tests/pass/function_calls/abi_compat.rs b/src/tools/miri/tests/pass/function_calls/abi_compat.rs index 90dc73d19cb2f..2e1ab58db7a39 100644 --- a/src/tools/miri/tests/pass/function_calls/abi_compat.rs +++ b/src/tools/miri/tests/pass/function_calls/abi_compat.rs @@ -5,6 +5,10 @@ use std::ptr; #[derive(Copy, Clone, Default)] struct Zst; +#[repr(transparent)] +#[derive(Copy, Clone)] +struct Wrapper(T); + fn id(x: T) -> T { x } fn test_abi_compat(t: T, u: U) { @@ -35,9 +39,6 @@ fn test_abi_compat(t: T, u: U) { /// Ensure that `T` is compatible with various repr(transparent) wrappers around `T`. fn test_abi_newtype() { - #[repr(transparent)] - #[derive(Copy, Clone)] - struct Wrapper1(T); #[repr(transparent)] #[derive(Copy, Clone)] struct Wrapper2(T, ()); @@ -49,7 +50,7 @@ fn test_abi_newtype() { struct Wrapper3(Zst, T, [u8; 0]); let t = T::default(); - test_abi_compat(t, Wrapper1(t)); + test_abi_compat(t, Wrapper(t)); test_abi_compat(t, Wrapper2(t, ())); test_abi_compat(t, Wrapper2a((), t)); test_abi_compat(t, Wrapper3(Zst, t, [])); @@ -66,6 +67,7 @@ fn main() { test_abi_compat(0usize, 0u64); test_abi_compat(0isize, 0i64); } + test_abi_compat(42u32, num::NonZeroU32::new(1).unwrap()); // Reference/pointer types with the same pointee. test_abi_compat(&0u32, &0u32 as *const u32); test_abi_compat(&mut 0u32 as *mut u32, Box::new(0u32)); @@ -77,8 +79,9 @@ fn main() { // Guaranteed null-pointer-optimizations. test_abi_compat(&0u32 as *const u32, Some(&0u32)); test_abi_compat(main as fn(), Some(main as fn())); - test_abi_compat(42u32, num::NonZeroU32::new(1).unwrap()); test_abi_compat(0u32, Some(num::NonZeroU32::new(1).unwrap())); + test_abi_compat(&0u32 as *const u32, Some(Wrapper(&0u32))); + test_abi_compat(0u32, Some(Wrapper(num::NonZeroU32::new(1).unwrap()))); // These must work for *any* type, since we guarantee that `repr(transparent)` is ABI-compatible // with the wrapped field. From 4999000da1a315ac85246d1218809820b12520c6 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 9 Sep 2023 11:47:05 +0200 Subject: [PATCH 5/7] fix ptr_metadata_ty for DynStar type --- compiler/rustc_middle/src/ty/sty.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_middle/src/ty/sty.rs b/compiler/rustc_middle/src/ty/sty.rs index fbbdfe5f14f92..6a918383de3dd 100644 --- a/compiler/rustc_middle/src/ty/sty.rs +++ b/compiler/rustc_middle/src/ty/sty.rs @@ -2735,6 +2735,8 @@ impl<'tcx> Ty<'tcx> { | ty::Error(_) // Extern types have metadata = (). | ty::Foreign(..) + // `dyn*` has no metadata + | ty::Dynamic(_, _, DynKind::DynStar) // If returned by `struct_tail_without_normalization` this is a unit struct // without any fields, or not a struct, and therefore is Sized. | ty::Adt(..) @@ -2743,7 +2745,7 @@ impl<'tcx> Ty<'tcx> { | ty::Tuple(..) => (tcx.types.unit, false), ty::Str | ty::Slice(_) => (tcx.types.usize, false), - ty::Dynamic(..) => { + ty::Dynamic(_, _, DynKind::Dyn) => { let dyn_metadata = tcx.require_lang_item(LangItem::DynMetadata, None); (tcx.type_of(dyn_metadata).instantiate(tcx, &[tail.into()]), false) }, From e00120906e6a00408c45d941b34f20c5e3277d95 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 9 Sep 2023 12:08:49 +0200 Subject: [PATCH 6/7] handle/hack for arbitrary-self dyn receivers --- .../src/interpret/terminator.rs | 76 +++++++++++-------- 1 file changed, 44 insertions(+), 32 deletions(-) diff --git a/compiler/rustc_const_eval/src/interpret/terminator.rs b/compiler/rustc_const_eval/src/interpret/terminator.rs index 437a035d08211..7e22981542e51 100644 --- a/compiler/rustc_const_eval/src/interpret/terminator.rs +++ b/compiler/rustc_const_eval/src/interpret/terminator.rs @@ -11,7 +11,7 @@ use rustc_middle::{ }, }; use rustc_span::sym; -use rustc_target::abi::FieldIdx; +use rustc_target::abi::{self, FieldIdx}; use rustc_target::abi::{ call::{ArgAbi, FnAbi, PassMode}, Integer, @@ -289,39 +289,40 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { } /// Unwrap types that are guaranteed a null-pointer-optimization - fn unfold_npo(&self, ty: Ty<'tcx>) -> InterpResult<'tcx, Ty<'tcx>> { + fn unfold_npo(&self, layout: TyAndLayout<'tcx>) -> InterpResult<'tcx, TyAndLayout<'tcx>> { // Check if this is `Option` wrapping some type. - let inner = match ty.kind() { + let inner = match layout.ty.kind() { ty::Adt(def, args) if self.tcx.is_diagnostic_item(sym::Option, def.did()) => { args[0].as_type().unwrap() } _ => { // Not an `Option`. - return Ok(ty); + return Ok(layout); } }; + let inner = self.layout_of(inner)?; // Check if the inner type is one of the NPO-guaranteed ones. // For that we first unpeel transparent *structs* (but not unions). let is_npo = |def: AdtDef<'tcx>| { self.tcx.has_attr(def.did(), sym::rustc_nonnull_optimization_guaranteed) }; - let inner = self.unfold_transparent(self.layout_of(inner)?, /* may_unfold */ |def| { + let inner = self.unfold_transparent(inner, /* may_unfold */ |def| { // Stop at NPO tpyes so that we don't miss that attribute in the check below! def.is_struct() && !is_npo(def) }); Ok(match inner.ty.kind() { ty::Ref(..) | ty::FnPtr(..) => { // Option<&T> behaves like &T, and same for fn() - inner.ty + inner } ty::Adt(def, _) if is_npo(*def) => { // Once we found a `nonnull_optimization_guaranteed` type, further strip off // newtype structs from it to find the underlying ABI type. - self.unfold_transparent(inner, /* may_unfold */ |def| def.is_struct()).ty + self.unfold_transparent(inner, /* may_unfold */ |def| def.is_struct()) } _ => { // Everything else we do not unfold. - ty + layout } }) } @@ -331,28 +332,44 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { /// that only checking the `PassMode` is insufficient.) fn layout_compat( &self, - caller_layout: TyAndLayout<'tcx>, - callee_layout: TyAndLayout<'tcx>, + caller: TyAndLayout<'tcx>, + callee: TyAndLayout<'tcx>, ) -> InterpResult<'tcx, bool> { // Fast path: equal types are definitely compatible. - if caller_layout.ty == callee_layout.ty { + if caller.ty == callee.ty { return Ok(true); } // 1-ZST are compatible with all 1-ZST (and with nothing else). - if caller_layout.is_1zst() || callee_layout.is_1zst() { - return Ok(caller_layout.is_1zst() && callee_layout.is_1zst()); + if caller.is_1zst() || callee.is_1zst() { + return Ok(caller.is_1zst() && callee.is_1zst()); } // Unfold newtypes and NPO optimizations. - let caller_ty = - self.unfold_npo(self.unfold_transparent(caller_layout, /* may_unfold */ |_| true).ty)?; - let callee_ty = - self.unfold_npo(self.unfold_transparent(callee_layout, /* may_unfold */ |_| true).ty)?; + let unfold = |layout: TyAndLayout<'tcx>| { + self.unfold_npo(self.unfold_transparent(layout, /* may_unfold */ |_def| true)) + }; + let caller = unfold(caller)?; + let callee = unfold(callee)?; // Now see if these inner types are compatible. - // Compatible pointer types. - let pointee_ty = |ty: Ty<'tcx>| { + // Compatible pointer types. For thin pointers, we have to accept even non-`repr(transparent)` + // things as compatible due to `DispatchFromDyn`. For instance, `Rc` and `*mut i32` + // must be compatible. So we just accept everything with Pointer ABI as compatible, + // even if this will accept some code that is not stably guaranteed to work. + // This also handles function pointers. + let thin_pointer = |layout: TyAndLayout<'tcx>| match layout.abi { + abi::Abi::Scalar(s) => match s.primitive() { + abi::Primitive::Pointer(addr_space) => Some(addr_space), + _ => None, + }, + _ => None, + }; + if let (Some(caller), Some(callee)) = (thin_pointer(caller), thin_pointer(callee)) { + return Ok(caller == callee); + } + // For wide pointers we have to get the pointee type. + let pointee_ty = |ty: Ty<'tcx>| -> InterpResult<'tcx, Option>> { // We cannot use `builtin_deref` here since we need to reject `Box`. - Some(match ty.kind() { + Ok(Some(match ty.kind() { ty::Ref(_, ty, _) => *ty, ty::RawPtr(mt) => mt.ty, // We should only accept `Box` with the default allocator. @@ -363,10 +380,10 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { { args[0].expect_ty() } - _ => return None, - }) + _ => return Ok(None), + })) }; - if let (Some(left), Some(right)) = (pointee_ty(caller_ty), pointee_ty(callee_ty)) { + if let (Some(caller), Some(callee)) = (pointee_ty(caller.ty)?, pointee_ty(callee.ty)?) { // This is okay if they have the same metadata type. let meta_ty = |ty: Ty<'tcx>| { let (meta, only_if_sized) = ty.ptr_metadata_ty(*self.tcx, |ty| ty); @@ -376,12 +393,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { ); meta }; - return Ok(meta_ty(left) == meta_ty(right)); - } - - // Compatible function pointer types. - if let (ty::FnPtr(..), ty::FnPtr(..)) = (caller_ty.kind(), callee_ty.kind()) { - return Ok(true); + return Ok(meta_ty(caller) == meta_ty(callee)); } // Compatible integer types (in particular, usize vs ptr-sized-u32/u64). @@ -392,14 +404,14 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { _ => return None, }) }; - if let (Some(left), Some(right)) = (int_ty(caller_ty), int_ty(callee_ty)) { + if let (Some(caller), Some(callee)) = (int_ty(caller.ty), int_ty(callee.ty)) { // This is okay if they are the same integer type. - return Ok(left == right); + return Ok(caller == callee); } // Fall back to exact equality. // FIXME: We are missing the rules for "repr(C) wrapping compatible types". - Ok(caller_ty == callee_ty) + Ok(caller == callee) } fn check_argument_compat( From e68e9d4a14a487bd109ec654ee3e2da1432edbd0 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 11 Sep 2023 10:24:53 +0200 Subject: [PATCH 7/7] explain why DispatchFromDyn does the check it does --- compiler/rustc_hir_analysis/src/coherence/builtin.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/compiler/rustc_hir_analysis/src/coherence/builtin.rs b/compiler/rustc_hir_analysis/src/coherence/builtin.rs index 67aef0a7c4349..94f3e8706fce3 100644 --- a/compiler/rustc_hir_analysis/src/coherence/builtin.rs +++ b/compiler/rustc_hir_analysis/src/coherence/builtin.rs @@ -157,6 +157,14 @@ fn visit_implementation_of_dispatch_from_dyn(tcx: TyCtxt<'_>, impl_did: LocalDef let infcx = tcx.infer_ctxt().build(); let cause = ObligationCause::misc(span, impl_did); + // Later parts of the compiler rely on all DispatchFromDyn types to be ABI-compatible with raw + // pointers. This is enforced here: we only allow impls for references, raw pointers, and things + // that are effectively repr(transparent) newtypes around types that already hav a + // DispatchedFromDyn impl. We cannot literally use repr(transparent) on those tpyes since some + // of them support an allocator, but we ensure that for the cases where the type implements this + // trait, they *do* satisfy the repr(transparent) rules, and then we assume that everything else + // in the compiler (in particular, all the call ABI logic) will treat them as repr(transparent) + // even if they do not carry that attribute. use rustc_type_ir::sty::TyKind::*; match (source.kind(), target.kind()) { (&Ref(r_a, _, mutbl_a), Ref(r_b, _, mutbl_b))