Auto merge of #3046 - RalfJung:rustup, r=RalfJung

Rustup

also more ABI compat tests

compiler/rustc_const_eval/src/interpret/terminator.rs

@@ -2,12 +2,13 @@
 
 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 @@ pub(super) fn eval_fn_call_arguments(
             .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,40 +298,49 @@ fn check_argument_compat(
                 // 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 {
-                // No question
+        }
+
+        if caller_layout.ty == callee_layout.ty {
+            // Fast path: equal types are 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.
-            // 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).
+
+        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())
             }
             (
-                    abi::Abi::ScalarPair(caller1, caller2),
-                    abi::Abi::ScalarPair(callee1, callee2),
+                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())
             }
-                // Be conservative.
+            (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.
@@ -333,17 +375,22 @@ fn check_argument_compat(
             _ => 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`.
-        if layout_compat() && mode_compat() {
+        // 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) && 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,
@@ -351,6 +398,7 @@ fn check_argument_compat(
             );
             return false;
         }
+    }
 
     /// Initialize a single callee argument, checking the types for compatibility.
     fn pass_argument<'x, 'y>(
@@ -379,7 +427,7 @@ fn pass_argument<'x, 'y>(
             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!(
@@ -612,7 +660,10 @@ pub(crate) fn eval_fn_call(
                             };
                             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();
@@ -649,7 +700,7 @@ pub(crate) fn eval_fn_call(
                         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!(

src/tools/miri/rust-version

@@ -1 +1 @@
-008c21c9779fd1e3632d9fe908b8afc0c421b26c
+dca2d1ff00bf96d244b1bb9a2117a92ec50ac71d

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::<fn(S), fn(A)>(f) };
+
+    g(Default::default()) //~ ERROR: calling a function with argument of type S passing data of type [i32; 4]
+}

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
+

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::<fn(f32), fn(i32)>(f) };
+
+    g(42) //~ ERROR: calling a function with argument of type f32 passing data of type i32
+}

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
+

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
 

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
 

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
 

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
 

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
 

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::<fn(simd::u32x8), fn(simd::u64x4)>(f) };
+
+    g(Default::default()) //~ ERROR: calling a function with argument of type std::simd::Simd<u32, 8> passing data of type std::simd::Simd<u64, 4>
+}

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<u32, 8> passing data of type std::simd::Simd<u64, 4>
+  --> $DIR/abi_mismatch_vector.rs:LL:CC
+   |
+LL |     g(Default::default())
+   |     ^^^^^^^^^^^^^^^^^^^^^ calling a function with argument of type std::simd::Simd<u32, 8> passing data of type std::simd::Simd<u64, 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_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
+

src/tools/miri/tests/pass/function_calls/abi_compat.rs

@@ -1,29 +1,91 @@
+#![feature(portable_simd)]
 use std::mem;
 use std::num;
+use std::simd;
 
-fn test_abi_compat<T, U>(t: T, u: U) {
+#[derive(Copy, Clone)]
+struct Zst;
+
+fn test_abi_compat<T: Copy, U: Copy>(t: T, u: U) {
     fn id<T>(x: T) -> T {
         x
     }
+    extern "C" fn id_c<T>(x: T) -> T {
+        x
+    }
 
     // This checks ABI compatibility both for arguments and return values,
     // in both directions.
     let f: fn(T) -> T = id;
     let f: fn(U) -> U = unsafe { std::mem::transmute(f) };
-    drop(f(u));
-
+    let _val = f(u);
     let f: fn(U) -> U = id;
     let f: fn(T) -> T = unsafe { std::mem::transmute(f) };
-    drop(f(t));
+    let _val = f(t);
+
+    // And then we do the same for `extern "C"`.
+    let f: extern "C" fn(T) -> T = id_c;
+    let f: extern "C" fn(U) -> U = unsafe { std::mem::transmute(f) };
+    let _val = f(u);
+    let f: extern "C" fn(U) -> U = id_c;
+    let f: extern "C" fn(T) -> T = unsafe { std::mem::transmute(f) };
+    let _val = f(t);
+}
+
+/// Ensure that `T` is compatible with various repr(transparent) wrappers around `T`.
+fn test_abi_newtype<T: Copy>(t: T) {
+    #[repr(transparent)]
+    #[derive(Copy, Clone)]
+    struct Wrapper1<T>(T);
+    #[repr(transparent)]
+    #[derive(Copy, Clone)]
+    struct Wrapper2<T>(T, ());
+    #[repr(transparent)]
+    #[derive(Copy, Clone)]
+    struct Wrapper2a<T>((), T);
+    #[repr(transparent)]
+    #[derive(Copy, Clone)]
+    struct Wrapper3<T>(Zst, 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(Zst, t, []));
+    test_abi_compat(t, mem::MaybeUninit::new(t)); // MaybeUninit is `repr(transparent)`
 }
 
 fn main() {
+    // Here we check:
+    // - unsigned vs signed integer is allowed
+    // - u32/i32 vs char is allowed
+    // - u32 vs NonZeroU32/Option<NonZeroU32> 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.
+    test_abi_compat(0u32, 0i32);
+    test_abi_compat(simd::u32x8::splat(1), simd::i32x8::splat(1));
     test_abi_compat(0u32, 'x');
-    test_abi_compat(&0u32, &([true; 4], [0u32; 0]));
-    test_abi_compat(0u32, mem::MaybeUninit::new(0u32));
+    test_abi_compat(0i32, 'x');
     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(&0u32, &0u32 as *const u32);
+    test_abi_compat(&0u32, &([true; 4], [0u32; 0]));
+    // Note that `bool` and `u8` are *not* compatible, at least on x86-64!
     // One of them has `arg_ext: Zext`, the other does not.
+
+    // These must work for *any* type, since we guarantee that `repr(transparent)` is ABI-compatible
+    // with the wrapped field.
+    test_abi_newtype(());
+    // FIXME: this still fails! test_abi_newtype(Zst);
+    test_abi_newtype(0u32);
+    test_abi_newtype(0f32);
+    test_abi_newtype((0u32, 1u32, 2u32));
+    // FIXME: skipping the array tests on mips64 due to https://github.com/rust-lang/rust/issues/115404
+    if !cfg!(target_arch = "mips64") {
+        test_abi_newtype([0u32, 1u32, 2u32]);
+        test_abi_newtype([0i32; 0]);
+    }
 }