about summary refs log tree commit diff
diff options
context:
space:
mode:
authorRalf Jung <post@ralfj.de>2023-08-30 11:11:02 +0200
committerRalf Jung <post@ralfj.de>2023-08-30 17:04:54 +0200
commit1e95aa0c4920b9f780e27da6933052a3417646be (patch)
tree6b3cf050b2944a48389440a5ffe0780c4f8f2834
parent26089ba0a2d9dab8381ccb0d7b99e704bc5cb3ed (diff)
downloadrust-1e95aa0c4920b9f780e27da6933052a3417646be.tar.gz
rust-1e95aa0c4920b9f780e27da6933052a3417646be.zip
interpret: make sure we accept transparent newtypes as ABI-compatible
also we were missing the case for Vector arguments, so handle those as well
-rw-r--r--compiler/rustc_const_eval/src/interpret/terminator.rs52
-rw-r--r--src/tools/miri/tests/pass/function_calls/abi_compat.rs24
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 279a06d3868..c61e30d86ad 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.
@@ -289,13 +283,40 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                     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())
                 }
-                // 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 67b87b46bd9..1be29992f25 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, U>(t: T, u: U) {
     fn id<T>(x: T) -> T { x }
@@ -15,6 +17,20 @@ fn test_abi_compat<T, U>(t: T, u: U) {
     drop(f(t));
 }
 
+/// Ensure that `T` is compatible with various repr(transparent) wrappers around `T`.
+fn test_abi_newtype<T: Copy>(t: T) {
+    #[repr(transparent)]
+    struct Wrapper1<T>(T);
+    #[repr(transparent)]
+    struct Wrapper2<T>(T, ());
+    #[repr(transparent)]
+    struct Wrapper3<T>(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]);
 }