From 1e95aa0c4920b9f780e27da6933052a3417646be Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 30 Aug 2023 11:11:02 +0200 Subject: [PATCH 1/3] interpret: make sure we accept transparent newtypes as ABI-compatible also we were missing the case for Vector arguments, so handle those as well --- .../src/interpret/terminator.rs | 52 +++++++++++++------ .../tests/pass/function_calls/abi_compat.rs | 24 ++++++++- 2 files changed, 59 insertions(+), 17 deletions(-) diff --git a/compiler/rustc_const_eval/src/interpret/terminator.rs b/compiler/rustc_const_eval/src/interpret/terminator.rs index 279a06d386861..c61e30d86ad59 100644 --- a/compiler/rustc_const_eval/src/interpret/terminator.rs +++ b/compiler/rustc_const_eval/src/interpret/terminator.rs @@ -269,15 +269,9 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { // Heuristic for type comparison. let layout_compat = || { if caller_abi.layout.ty == callee_abi.layout.ty { - // No question + // Fast path: definitely compatible. return true; } - if caller_abi.layout.is_unsized() || callee_abi.layout.is_unsized() { - // No, no, no. We require the types to *exactly* match for unsized arguments. If - // these are somehow unsized "in a different way" (say, `dyn Trait` vs `[i32]`), - // then who knows what happens. - return false; - } // This is tricky. Some ABIs split aggregates up into multiple registers etc, so we have // to be super careful here. For the scalar ABIs we conveniently already have all the // newtypes unwrapped etc, so in those cases we can just compare the scalar components. @@ -288,6 +282,13 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { (abi::Abi::Scalar(caller), abi::Abi::Scalar(callee)) => { primitive_abi_compat(caller.primitive(), callee.primitive()) } + ( + abi::Abi::Vector { element: caller_element, count: caller_count }, + abi::Abi::Vector { element: callee_element, count: callee_count }, + ) => { + primitive_abi_compat(caller_element.primitive(), callee_element.primitive()) + && caller_count == callee_count + } ( abi::Abi::ScalarPair(caller1, caller2), abi::Abi::ScalarPair(callee1, callee2), @@ -295,7 +296,27 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { primitive_abi_compat(caller1.primitive(), callee1.primitive()) && primitive_abi_compat(caller2.primitive(), callee2.primitive()) } - // Be conservative. + ( + abi::Abi::Aggregate { sized: caller_sized }, + abi::Abi::Aggregate { sized: callee_sized }, + ) => { + // For these we rely on all the information being encoded in the `PassMode`, so + // here we only habe to check in-memory compatibility. + // FIXME: unwrap transparent newtype wrappers instead. + if !caller_sized || !callee_sized { + // No, no, no. We require the types to *exactly* match for unsized arguments. If + // these are somehow unsized "in a different way" (say, `dyn Trait` vs `[i32]`), + // then who knows what happens. + // FIXME: ideally we'd support newtyped around unized types, but that requires ensuring + // that for all values of the metadata, both types will compute the same dynamic size... + // not an easy thing to check. + return false; + } + caller_abi.layout.size == callee_abi.layout.size + && caller_abi.layout.align.abi == callee_abi.layout.align.abi + } + // What remains is `Abi::Uninhabited` (which can never be passed anyway) and + // mismatching ABIs, that should all be rejected. _ => false, } }; @@ -333,15 +354,14 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { _ => false, }; - // We have to check both. `layout_compat` is needed to reject e.g. `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`. + // 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 layout_compat() && mode_compat() { - // Something went very wrong if our checks don't even imply that the layout is the same. - assert!( - caller_abi.layout.size == callee_abi.layout.size - && caller_abi.layout.align.abi == callee_abi.layout.align.abi - ); return true; } trace!( 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 67b87b46bd9bc..1be29992f252e 100644 --- a/src/tools/miri/tests/pass/function_calls/abi_compat.rs +++ b/src/tools/miri/tests/pass/function_calls/abi_compat.rs @@ -1,5 +1,7 @@ +#![feature(portable_simd)] use std::num; use std::mem; +use std::simd; fn test_abi_compat(t: T, u: U) { fn id(x: T) -> T { x } @@ -15,6 +17,20 @@ fn test_abi_compat(t: T, u: U) { drop(f(t)); } +/// Ensure that `T` is compatible with various repr(transparent) wrappers around `T`. +fn test_abi_newtype(t: T) { + #[repr(transparent)] + struct Wrapper1(T); + #[repr(transparent)] + struct Wrapper2(T, ()); + #[repr(transparent)] + struct Wrapper3(T, [u8; 0]); + + test_abi_compat(t, Wrapper1(t)); + test_abi_compat(t, Wrapper2(t, ())); + test_abi_compat(t, Wrapper3(t, [])); +} + fn main() { test_abi_compat(0u32, 'x'); test_abi_compat(&0u32, &([true; 4], [0u32; 0])); @@ -22,6 +38,12 @@ fn main() { test_abi_compat(42u32, num::NonZeroU32::new(1).unwrap()); test_abi_compat(0u32, Some(num::NonZeroU32::new(1).unwrap())); test_abi_compat(0u32, 0i32); - // Note that `bool` and `u8` are *not* compatible! + test_abi_compat(simd::u32x8::splat(1), simd::i32x8::splat(1)); + // Note that `bool` and `u8` are *not* compatible, at least on x86-64! // One of them has `arg_ext: Zext`, the other does not. + + test_abi_newtype(0u32); + test_abi_newtype(0f32); + test_abi_newtype((0u32, 1u32, 2u32)); + test_abi_newtype([0u32, 1u32, 2u32]); } From c1a34729e1a9e35af053c1c7ac76de6f2de3dfaf Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 30 Aug 2023 08:43:20 +0200 Subject: [PATCH 2/3] organize failing ABI compat tests and add some more --- .../abi_mismatch_int_vs_float.rs | 7 +++++++ .../abi_mismatch_int_vs_float.stderr | 15 +++++++++++++++ ...ast_fn_ptr4.rs => abi_mismatch_raw_pointer.rs} | 0 ...tr4.stderr => abi_mismatch_raw_pointer.stderr} | 4 ++-- ...ast_fn_ptr5.rs => abi_mismatch_return_type.rs} | 0 ...tr5.stderr => abi_mismatch_return_type.stderr} | 4 ++-- .../{cast_fn_ptr2.rs => abi_mismatch_simple.rs} | 0 ..._fn_ptr2.stderr => abi_mismatch_simple.stderr} | 4 ++-- ...st_fn_ptr3.rs => abi_mismatch_too_few_args.rs} | 0 ...r3.stderr => abi_mismatch_too_few_args.stderr} | 4 ++-- ...t_fn_ptr1.rs => abi_mismatch_too_many_args.rs} | 0 ...1.stderr => abi_mismatch_too_many_args.stderr} | 4 ++-- .../fail/function_pointers/abi_mismatch_vector.rs | 11 +++++++++++ .../function_pointers/abi_mismatch_vector.stderr | 15 +++++++++++++++ 14 files changed, 58 insertions(+), 10 deletions(-) create mode 100644 src/tools/miri/tests/fail/function_pointers/abi_mismatch_int_vs_float.rs create mode 100644 src/tools/miri/tests/fail/function_pointers/abi_mismatch_int_vs_float.stderr rename src/tools/miri/tests/fail/function_pointers/{cast_fn_ptr4.rs => abi_mismatch_raw_pointer.rs} (100%) rename src/tools/miri/tests/fail/function_pointers/{cast_fn_ptr4.stderr => abi_mismatch_raw_pointer.stderr} (85%) rename src/tools/miri/tests/fail/function_pointers/{cast_fn_ptr5.rs => abi_mismatch_return_type.rs} (100%) rename src/tools/miri/tests/fail/function_pointers/{cast_fn_ptr5.stderr => abi_mismatch_return_type.stderr} (84%) rename src/tools/miri/tests/fail/function_pointers/{cast_fn_ptr2.rs => abi_mismatch_simple.rs} (100%) rename src/tools/miri/tests/fail/function_pointers/{cast_fn_ptr2.stderr => abi_mismatch_simple.stderr} (85%) rename src/tools/miri/tests/fail/function_pointers/{cast_fn_ptr3.rs => abi_mismatch_too_few_args.rs} (100%) rename src/tools/miri/tests/fail/function_pointers/{cast_fn_ptr3.stderr => abi_mismatch_too_few_args.stderr} (83%) rename src/tools/miri/tests/fail/function_pointers/{cast_fn_ptr1.rs => abi_mismatch_too_many_args.rs} (100%) rename src/tools/miri/tests/fail/function_pointers/{cast_fn_ptr1.stderr => abi_mismatch_too_many_args.stderr} (83%) create mode 100644 src/tools/miri/tests/fail/function_pointers/abi_mismatch_vector.rs create mode 100644 src/tools/miri/tests/fail/function_pointers/abi_mismatch_vector.stderr diff --git a/src/tools/miri/tests/fail/function_pointers/abi_mismatch_int_vs_float.rs b/src/tools/miri/tests/fail/function_pointers/abi_mismatch_int_vs_float.rs new file mode 100644 index 0000000000000..a1fda329e8d05 --- /dev/null +++ b/src/tools/miri/tests/fail/function_pointers/abi_mismatch_int_vs_float.rs @@ -0,0 +1,7 @@ +fn main() { + fn f(_: f32) {} + + let g = unsafe { std::mem::transmute::(f) }; + + g(42) //~ ERROR: calling a function with argument of type f32 passing data of type i32 +} 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 new file mode 100644 index 0000000000000..a53126c733eca --- /dev/null +++ b/src/tools/miri/tests/fail/function_pointers/abi_mismatch_int_vs_float.stderr @@ -0,0 +1,15 @@ +error: Undefined Behavior: calling a function with argument of type f32 passing data of type i32 + --> $DIR/abi_mismatch_int_vs_float.rs:LL:CC + | +LL | g(42) + | ^^^^^ calling a function with argument of type f32 passing data of type 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 + = note: BACKTRACE: + = note: inside `main` at $DIR/abi_mismatch_int_vs_float.rs:LL:CC + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to previous error + diff --git a/src/tools/miri/tests/fail/function_pointers/cast_fn_ptr4.rs b/src/tools/miri/tests/fail/function_pointers/abi_mismatch_raw_pointer.rs similarity index 100% rename from src/tools/miri/tests/fail/function_pointers/cast_fn_ptr4.rs rename to src/tools/miri/tests/fail/function_pointers/abi_mismatch_raw_pointer.rs diff --git a/src/tools/miri/tests/fail/function_pointers/cast_fn_ptr4.stderr b/src/tools/miri/tests/fail/function_pointers/abi_mismatch_raw_pointer.stderr similarity index 85% rename from src/tools/miri/tests/fail/function_pointers/cast_fn_ptr4.stderr rename to src/tools/miri/tests/fail/function_pointers/abi_mismatch_raw_pointer.stderr index 610425658fe1f..6eacfeece1497 100644 --- a/src/tools/miri/tests/fail/function_pointers/cast_fn_ptr4.stderr +++ b/src/tools/miri/tests/fail/function_pointers/abi_mismatch_raw_pointer.stderr @@ -1,5 +1,5 @@ error: Undefined Behavior: calling a function with argument of type *const [i32] passing data of type *const i32 - --> $DIR/cast_fn_ptr4.rs:LL:CC + --> $DIR/abi_mismatch_raw_pointer.rs:LL:CC | LL | g(&42 as *const i32) | ^^^^^^^^^^^^^^^^^^^^ calling a function with argument of type *const [i32] passing data of type *const i32 @@ -7,7 +7,7 @@ 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 = note: BACKTRACE: - = note: inside `main` at $DIR/cast_fn_ptr4.rs:LL:CC + = note: inside `main` at $DIR/abi_mismatch_raw_pointer.rs:LL:CC note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace diff --git a/src/tools/miri/tests/fail/function_pointers/cast_fn_ptr5.rs b/src/tools/miri/tests/fail/function_pointers/abi_mismatch_return_type.rs similarity index 100% rename from src/tools/miri/tests/fail/function_pointers/cast_fn_ptr5.rs rename to src/tools/miri/tests/fail/function_pointers/abi_mismatch_return_type.rs diff --git a/src/tools/miri/tests/fail/function_pointers/cast_fn_ptr5.stderr b/src/tools/miri/tests/fail/function_pointers/abi_mismatch_return_type.stderr similarity index 84% rename from src/tools/miri/tests/fail/function_pointers/cast_fn_ptr5.stderr rename to src/tools/miri/tests/fail/function_pointers/abi_mismatch_return_type.stderr index c4e08b58430a2..eedc1235773e2 100644 --- a/src/tools/miri/tests/fail/function_pointers/cast_fn_ptr5.stderr +++ b/src/tools/miri/tests/fail/function_pointers/abi_mismatch_return_type.stderr @@ -1,5 +1,5 @@ error: Undefined Behavior: calling a function with return type u32 passing return place of type () - --> $DIR/cast_fn_ptr5.rs:LL:CC + --> $DIR/abi_mismatch_return_type.rs:LL:CC | LL | g() | ^^^ calling a function with return type u32 passing return place of type () @@ -7,7 +7,7 @@ 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 = note: BACKTRACE: - = note: inside `main` at $DIR/cast_fn_ptr5.rs:LL:CC + = note: inside `main` at $DIR/abi_mismatch_return_type.rs:LL:CC note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace diff --git a/src/tools/miri/tests/fail/function_pointers/cast_fn_ptr2.rs b/src/tools/miri/tests/fail/function_pointers/abi_mismatch_simple.rs similarity index 100% rename from src/tools/miri/tests/fail/function_pointers/cast_fn_ptr2.rs rename to src/tools/miri/tests/fail/function_pointers/abi_mismatch_simple.rs diff --git a/src/tools/miri/tests/fail/function_pointers/cast_fn_ptr2.stderr b/src/tools/miri/tests/fail/function_pointers/abi_mismatch_simple.stderr similarity index 85% rename from src/tools/miri/tests/fail/function_pointers/cast_fn_ptr2.stderr rename to src/tools/miri/tests/fail/function_pointers/abi_mismatch_simple.stderr index 086712e0d13bd..bc500a90b7772 100644 --- a/src/tools/miri/tests/fail/function_pointers/cast_fn_ptr2.stderr +++ b/src/tools/miri/tests/fail/function_pointers/abi_mismatch_simple.stderr @@ -1,5 +1,5 @@ error: Undefined Behavior: calling a function with argument of type (i32, i32) passing data of type i32 - --> $DIR/cast_fn_ptr2.rs:LL:CC + --> $DIR/abi_mismatch_simple.rs:LL:CC | LL | g(42) | ^^^^^ calling a function with argument of type (i32, i32) passing data of type i32 @@ -7,7 +7,7 @@ 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 = note: BACKTRACE: - = note: inside `main` at $DIR/cast_fn_ptr2.rs:LL:CC + = note: inside `main` at $DIR/abi_mismatch_simple.rs:LL:CC note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace diff --git a/src/tools/miri/tests/fail/function_pointers/cast_fn_ptr3.rs b/src/tools/miri/tests/fail/function_pointers/abi_mismatch_too_few_args.rs similarity index 100% rename from src/tools/miri/tests/fail/function_pointers/cast_fn_ptr3.rs rename to src/tools/miri/tests/fail/function_pointers/abi_mismatch_too_few_args.rs diff --git a/src/tools/miri/tests/fail/function_pointers/cast_fn_ptr3.stderr b/src/tools/miri/tests/fail/function_pointers/abi_mismatch_too_few_args.stderr similarity index 83% rename from src/tools/miri/tests/fail/function_pointers/cast_fn_ptr3.stderr rename to src/tools/miri/tests/fail/function_pointers/abi_mismatch_too_few_args.stderr index 55fd7d6072089..558d83bcfd247 100644 --- a/src/tools/miri/tests/fail/function_pointers/cast_fn_ptr3.stderr +++ b/src/tools/miri/tests/fail/function_pointers/abi_mismatch_too_few_args.stderr @@ -1,5 +1,5 @@ error: Undefined Behavior: calling a function with fewer arguments than it requires - --> $DIR/cast_fn_ptr3.rs:LL:CC + --> $DIR/abi_mismatch_too_few_args.rs:LL:CC | LL | g() | ^^^ calling a function with fewer arguments than it requires @@ -7,7 +7,7 @@ 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 = note: BACKTRACE: - = note: inside `main` at $DIR/cast_fn_ptr3.rs:LL:CC + = note: inside `main` at $DIR/abi_mismatch_too_few_args.rs:LL:CC note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace diff --git a/src/tools/miri/tests/fail/function_pointers/cast_fn_ptr1.rs b/src/tools/miri/tests/fail/function_pointers/abi_mismatch_too_many_args.rs similarity index 100% rename from src/tools/miri/tests/fail/function_pointers/cast_fn_ptr1.rs rename to src/tools/miri/tests/fail/function_pointers/abi_mismatch_too_many_args.rs diff --git a/src/tools/miri/tests/fail/function_pointers/cast_fn_ptr1.stderr b/src/tools/miri/tests/fail/function_pointers/abi_mismatch_too_many_args.stderr similarity index 83% rename from src/tools/miri/tests/fail/function_pointers/cast_fn_ptr1.stderr rename to src/tools/miri/tests/fail/function_pointers/abi_mismatch_too_many_args.stderr index bb2a263795980..dc12073952f86 100644 --- a/src/tools/miri/tests/fail/function_pointers/cast_fn_ptr1.stderr +++ b/src/tools/miri/tests/fail/function_pointers/abi_mismatch_too_many_args.stderr @@ -1,5 +1,5 @@ error: Undefined Behavior: calling a function with more arguments than it expected - --> $DIR/cast_fn_ptr1.rs:LL:CC + --> $DIR/abi_mismatch_too_many_args.rs:LL:CC | LL | g(42) | ^^^^^ calling a function with more arguments than it expected @@ -7,7 +7,7 @@ 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 = note: BACKTRACE: - = note: inside `main` at $DIR/cast_fn_ptr1.rs:LL:CC + = note: inside `main` at $DIR/abi_mismatch_too_many_args.rs:LL:CC note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace diff --git a/src/tools/miri/tests/fail/function_pointers/abi_mismatch_vector.rs b/src/tools/miri/tests/fail/function_pointers/abi_mismatch_vector.rs new file mode 100644 index 0000000000000..80f357b61badd --- /dev/null +++ b/src/tools/miri/tests/fail/function_pointers/abi_mismatch_vector.rs @@ -0,0 +1,11 @@ +#![feature(portable_simd)] +use std::simd; + +fn main() { + fn f(_: simd::u32x8) {} + + // These two vector types have the same size but are still not compatible. + let g = unsafe { std::mem::transmute::(f) }; + + g(Default::default()) //~ ERROR: calling a function with argument of type std::simd::Simd passing data of type std::simd::Simd +} 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 new file mode 100644 index 0000000000000..7dcca1e85b870 --- /dev/null +++ b/src/tools/miri/tests/fail/function_pointers/abi_mismatch_vector.stderr @@ -0,0 +1,15 @@ +error: Undefined Behavior: calling a function with argument of type std::simd::Simd passing data of type std::simd::Simd + --> $DIR/abi_mismatch_vector.rs:LL:CC + | +LL | g(Default::default()) + | ^^^^^^^^^^^^^^^^^^^^^ calling a function with argument of type std::simd::Simd passing data of type std::simd::Simd + | + = 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 + = note: BACKTRACE: + = note: inside `main` at $DIR/abi_mismatch_vector.rs:LL:CC + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to previous error + From c37bd09d88fb720eb5172a833fea2d74a3d14b9a Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 30 Aug 2023 12:26:17 +0200 Subject: [PATCH 3/3] miri function ABI check: specifically look for repr(transparent) --- .../src/interpret/terminator.rs | 173 +++++++++++------- .../abi_mismatch_array_vs_struct.rs | 16 ++ .../abi_mismatch_array_vs_struct.stderr | 15 ++ .../tests/pass/function_calls/abi_compat.rs | 4 + 4 files changed, 137 insertions(+), 71 deletions(-) create mode 100644 src/tools/miri/tests/fail/function_pointers/abi_mismatch_array_vs_struct.rs create mode 100644 src/tools/miri/tests/fail/function_pointers/abi_mismatch_array_vs_struct.stderr diff --git a/compiler/rustc_const_eval/src/interpret/terminator.rs b/compiler/rustc_const_eval/src/interpret/terminator.rs index c61e30d86ad59..a016bfa5cf8b3 100644 --- a/compiler/rustc_const_eval/src/interpret/terminator.rs +++ b/compiler/rustc_const_eval/src/interpret/terminator.rs @@ -2,12 +2,13 @@ use std::borrow::Cow; use either::Either; use rustc_ast::ast::InlineAsmOptions; -use rustc_middle::mir::ProjectionElem; -use rustc_middle::ty::layout::{FnAbiOf, LayoutOf, TyAndLayout}; -use rustc_middle::ty::Instance; use rustc_middle::{ mir, - ty::{self, Ty}, + ty::{ + self, + layout::{FnAbiOf, LayoutOf, TyAndLayout}, + Instance, Ty, + }, }; use rustc_target::abi::call::{ArgAbi, ArgAttribute, ArgAttributes, FnAbi, PassMode}; use rustc_target::abi::{self, FieldIdx}; @@ -252,11 +253,43 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { .collect() } - fn check_argument_compat( - caller_abi: &ArgAbi<'tcx, Ty<'tcx>>, - callee_abi: &ArgAbi<'tcx, Ty<'tcx>>, + /// Find the wrapped inner type of a transparent wrapper. + fn unfold_transparent(&self, layout: TyAndLayout<'tcx>) -> TyAndLayout<'tcx> { + match layout.ty.kind() { + ty::Adt(adt_def, _) if adt_def.repr().transparent() => { + assert!(!adt_def.is_enum()); + // Find the non-1-ZST field. + let mut non_1zst_fields = (0..layout.fields.count()).filter_map(|idx| { + let field = layout.field(self, idx); + if field.is_1zst() { None } else { Some(field) } + }); + let Some(first) = non_1zst_fields.next() else { + // All fields are 1-ZST, so this is basically the same as `()`. + // (We still also compare the `PassMode`, so if this target does something strange with 1-ZST there, we'll know.) + return self.layout_of(self.tcx.types.unit).unwrap(); + }; + assert!( + non_1zst_fields.next().is_none(), + "more than one non-1-ZST field in a transparent type" + ); + + // Found it! + self.unfold_transparent(first) + } + // Not a transparent type, no further unfolding. + _ => layout, + } + } + + /// 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.) + fn layout_compat( + &self, + caller_layout: TyAndLayout<'tcx>, + callee_layout: TyAndLayout<'tcx>, ) -> bool { - let primitive_abi_compat = |a1: abi::Primitive, a2: abi::Primitive| -> bool { + fn primitive_abi_compat(a1: abi::Primitive, a2: abi::Primitive) -> bool { match (a1, a2) { // For integers, ignore the sign. (abi::Primitive::Int(int_ty1, _sign1), abi::Primitive::Int(int_ty2, _sign2)) => { @@ -265,61 +298,49 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { // For everything else we require full equality. _ => a1 == a2, } - }; - // Heuristic for type comparison. - let layout_compat = || { - if caller_abi.layout.ty == callee_abi.layout.ty { - // Fast path: definitely compatible. - return true; + } + + if caller_layout.ty == callee_layout.ty { + // Fast path: equal types are definitely compatible. + return true; + } + + match (caller_layout.abi, callee_layout.abi) { + // If both sides have Scalar/Vector/ScalarPair ABI, we can easily directly compare them. + // Different valid ranges are okay (the validity check will complain if this leads to + // invalid transmutes). + (abi::Abi::Scalar(caller), abi::Abi::Scalar(callee)) => { + primitive_abi_compat(caller.primitive(), callee.primitive()) } - // This is tricky. Some ABIs split aggregates up into multiple registers etc, so we have - // to be super careful here. For the scalar ABIs we conveniently already have all the - // newtypes unwrapped etc, so in those cases we can just compare the scalar components. - // Everything else we just reject for now. - match (caller_abi.layout.abi, callee_abi.layout.abi) { - // Different valid ranges are okay (the validity check will complain if this leads - // to invalid transmutes). - (abi::Abi::Scalar(caller), abi::Abi::Scalar(callee)) => { - primitive_abi_compat(caller.primitive(), callee.primitive()) - } - ( - abi::Abi::Vector { element: caller_element, count: caller_count }, - abi::Abi::Vector { element: callee_element, count: callee_count }, - ) => { - primitive_abi_compat(caller_element.primitive(), callee_element.primitive()) - && caller_count == callee_count - } - ( - abi::Abi::ScalarPair(caller1, caller2), - abi::Abi::ScalarPair(callee1, callee2), - ) => { - primitive_abi_compat(caller1.primitive(), callee1.primitive()) - && primitive_abi_compat(caller2.primitive(), callee2.primitive()) - } - ( - abi::Abi::Aggregate { sized: caller_sized }, - abi::Abi::Aggregate { sized: callee_sized }, - ) => { - // For these we rely on all the information being encoded in the `PassMode`, so - // here we only habe to check in-memory compatibility. - // FIXME: unwrap transparent newtype wrappers instead. - if !caller_sized || !callee_sized { - // No, no, no. We require the types to *exactly* match for unsized arguments. If - // these are somehow unsized "in a different way" (say, `dyn Trait` vs `[i32]`), - // then who knows what happens. - // FIXME: ideally we'd support newtyped around unized types, but that requires ensuring - // that for all values of the metadata, both types will compute the same dynamic size... - // not an easy thing to check. - return false; - } - caller_abi.layout.size == callee_abi.layout.size - && caller_abi.layout.align.abi == callee_abi.layout.align.abi - } - // What remains is `Abi::Uninhabited` (which can never be passed anyway) and - // mismatching ABIs, that should all be rejected. - _ => false, + ( + abi::Abi::Vector { element: caller_element, count: caller_count }, + abi::Abi::Vector { element: callee_element, count: callee_count }, + ) => { + primitive_abi_compat(caller_element.primitive(), callee_element.primitive()) + && caller_count == callee_count } - }; + (abi::Abi::ScalarPair(caller1, caller2), abi::Abi::ScalarPair(callee1, callee2)) => { + primitive_abi_compat(caller1.primitive(), callee1.primitive()) + && primitive_abi_compat(caller2.primitive(), callee2.primitive()) + } + (abi::Abi::Aggregate { .. }, abi::Abi::Aggregate { .. }) => { + // Aggregates are compatible only if they newtype-wrap the same type. + // 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. + self.unfold_transparent(caller_layout).ty + == self.unfold_transparent(callee_layout).ty + } + // What remains is `Abi::Uninhabited` (which can never be passed anyway) and + // mismatching ABIs, that should all be rejected. + _ => false, + } + } + + fn check_argument_compat( + &self, + caller_abi: &ArgAbi<'tcx, Ty<'tcx>>, + callee_abi: &ArgAbi<'tcx, Ty<'tcx>>, + ) -> bool { // When comparing the PassMode, we have to be smart about comparing the attributes. let arg_attr_compat = |a1: &ArgAttributes, a2: &ArgAttributes| { // There's only one regular attribute that matters for the call ABI: InReg. @@ -361,15 +382,22 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { // 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 layout_compat() && mode_compat() { + if self.layout_compat(caller_abi.layout, callee_abi.layout) && mode_compat() { + // Something went very wrong if our checks don't even imply that the layout is the same. + assert!( + caller_abi.layout.size == callee_abi.layout.size + && caller_abi.layout.align.abi == callee_abi.layout.align.abi + && caller_abi.layout.is_sized() == callee_abi.layout.is_sized() + ); return true; + } else { + trace!( + "check_argument_compat: incompatible ABIs:\ncaller: {:?}\ncallee: {:?}", + caller_abi, + callee_abi + ); + return false; } - trace!( - "check_argument_compat: incompatible ABIs:\ncaller: {:?}\ncallee: {:?}", - caller_abi, - callee_abi - ); - return false; } /// Initialize a single callee argument, checking the types for compatibility. @@ -399,7 +427,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { throw_ub_custom!(fluent::const_eval_not_enough_caller_args); }; // 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!( @@ -632,7 +660,10 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { }; for (i, field_ty) in fields.iter().enumerate() { let dest = dest.project_deeper( - &[ProjectionElem::Field(FieldIdx::from_usize(i), field_ty)], + &[mir::ProjectionElem::Field( + FieldIdx::from_usize(i), + field_ty, + )], *self.tcx, ); let callee_abi = callee_args_abis.next().unwrap(); @@ -669,7 +700,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!( diff --git a/src/tools/miri/tests/fail/function_pointers/abi_mismatch_array_vs_struct.rs b/src/tools/miri/tests/fail/function_pointers/abi_mismatch_array_vs_struct.rs new file mode 100644 index 0000000000000..415e91b250fc6 --- /dev/null +++ b/src/tools/miri/tests/fail/function_pointers/abi_mismatch_array_vs_struct.rs @@ -0,0 +1,16 @@ +#![feature(portable_simd)] + +// Some targets treat arrays and structs very differently. We would probably catch that on those +// targets since we check the `PassMode`; here we ensure that we catch it on *all* targets +// (in particular, on x86-64 the pass mode is `Indirect` for both of these). +struct S(i32, i32, i32, i32); +type A = [i32; 4]; + +fn main() { + fn f(_: S) {} + + // These two types have the same size but are still not compatible. + let g = unsafe { std::mem::transmute::(f) }; + + g(Default::default()) //~ ERROR: calling a function with argument of type S passing data of type [i32; 4] +} 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 new file mode 100644 index 0000000000000..50d4228c11111 --- /dev/null +++ b/src/tools/miri/tests/fail/function_pointers/abi_mismatch_array_vs_struct.stderr @@ -0,0 +1,15 @@ +error: Undefined Behavior: calling a function with argument of type S passing data of type [i32; 4] + --> $DIR/abi_mismatch_array_vs_struct.rs:LL:CC + | +LL | g(Default::default()) + | ^^^^^^^^^^^^^^^^^^^^^ calling a function with argument of type S passing data of type [i32; 4] + | + = 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 + = note: BACKTRACE: + = note: inside `main` at $DIR/abi_mismatch_array_vs_struct.rs:LL:CC + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to previous error + 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 1be29992f252e..0786450f75117 100644 --- a/src/tools/miri/tests/pass/function_calls/abi_compat.rs +++ b/src/tools/miri/tests/pass/function_calls/abi_compat.rs @@ -24,10 +24,13 @@ fn test_abi_newtype(t: T) { #[repr(transparent)] struct Wrapper2(T, ()); #[repr(transparent)] + struct Wrapper2a((), T); + #[repr(transparent)] struct Wrapper3(T, [u8; 0]); test_abi_compat(t, Wrapper1(t)); test_abi_compat(t, Wrapper2(t, ())); + test_abi_compat(t, Wrapper2a((), t)); test_abi_compat(t, Wrapper3(t, [])); } @@ -46,4 +49,5 @@ fn main() { test_abi_newtype(0f32); test_abi_newtype((0u32, 1u32, 2u32)); test_abi_newtype([0u32, 1u32, 2u32]); + test_abi_newtype([0i32; 0]); }