accept some differences for rustc_abi(assert_eq), so that we can test more things to be compatible

RalfJung · RalfJung · commit c3e14edd8b7c · 2023-09-08T08:59:55.000+02:00
diff --git a/compiler/rustc_abi/src/lib.rs b/compiler/rustc_abi/src/lib.rs
@@ -1348,6 +1348,23 @@ impl Abi {
             Abi::Uninhabited | Abi::Aggregate { .. } => Abi::Aggregate { sized: true },
         }
     }
+
+    pub fn eq_up_to_validity(&self, other: &Self) -> bool {
+        match (self, other) {
+            // Scalar, Vector, ScalarPair have `Scalar` in them where we ignore validity ranges.
+            // We do *not* ignore the sign since it matters for some ABIs (e.g. s390x).
+            (Abi::Scalar(l), Abi::Scalar(r)) => l.primitive() == r.primitive(),
+            (
+                Abi::Vector { element: element_l, count: count_l },
+                Abi::Vector { element: element_r, count: count_r },
+            ) => element_l.primitive() == element_r.primitive() && count_l == count_r,
+            (Abi::ScalarPair(l1, l2), Abi::ScalarPair(r1, r2)) => {
+                l1.primitive() == r1.primitive() && l2.primitive() == r2.primitive()
+            }
+            // Everything else must be strictly identical.
+            _ => self == other,
+        }
+    }
 }
 
 #[derive(PartialEq, Eq, Hash, Clone, Debug)]
diff --git a/compiler/rustc_const_eval/src/interpret/terminator.rs b/compiler/rustc_const_eval/src/interpret/terminator.rs
@@ -10,7 +10,7 @@ use rustc_middle::{
         Instance, Ty,
     },
 };
-use rustc_target::abi::call::{ArgAbi, ArgAttribute, ArgAttributes, FnAbi, PassMode};
+use rustc_target::abi::call::{ArgAbi, FnAbi, PassMode};
 use rustc_target::abi::{self, FieldIdx};
 use rustc_target::spec::abi::Abi;
 
@@ -291,32 +291,17 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
             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). Different signs are *not* okay on some targets (e.g. `extern
-            // "C"` on `s390x` where small integers are passed zero/sign-extended in large
-            // registers), so we generally reject them to increase portability.
+        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(caller), abi::Abi::Scalar(callee)) => {
-                caller.primitive() == callee.primitive()
+            abi::Abi::Scalar(..) | abi::Abi::ScalarPair(..) | abi::Abi::Vector { .. } => {
+                caller_layout.abi.eq_up_to_validity(&callee_layout.abi)
             }
-            (
-                abi::Abi::Vector { element: caller_element, count: caller_count },
-                abi::Abi::Vector { element: callee_element, count: callee_count },
-            ) => {
-                caller_element.primitive() == callee_element.primitive()
-                    && caller_count == callee_count
-            }
-            (abi::Abi::ScalarPair(caller1, caller2), abi::Abi::ScalarPair(callee1, callee2)) => {
-                caller1.primitive() == callee1.primitive()
-                    && caller2.primitive() == callee2.primitive()
-            }
-            (abi::Abi::Aggregate { .. }, abi::Abi::Aggregate { .. }) => {
-                // Aggregates are compatible only if they newtype-wrap the same type, or if they are both 1-ZST.
+            _ => {
+                // 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.
@@ -329,9 +314,6 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                         == 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,
         }
     }
 
@@ -340,48 +322,16 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         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.
-            // Everything else is things like noalias, dereferenceable, nonnull, ...
-            // (This also applies to pointee_size, pointee_align.)
-            if a1.regular.contains(ArgAttribute::InReg) != a2.regular.contains(ArgAttribute::InReg)
-            {
-                return false;
-            }
-            // We also compare the sign extension mode -- this could let the callee make assumptions
-            // about bits that conceptually were not even passed.
-            if a1.arg_ext != a2.arg_ext {
-                return false;
-            }
-            return true;
-        };
-        let mode_compat = || match (&caller_abi.mode, &callee_abi.mode) {
-            (PassMode::Ignore, PassMode::Ignore) => true, // can still be reached for the return type
-            (PassMode::Direct(a1), PassMode::Direct(a2)) => arg_attr_compat(a1, a2),
-            (PassMode::Pair(a1, b1), PassMode::Pair(a2, b2)) => {
-                arg_attr_compat(a1, a2) && arg_attr_compat(b1, b2)
-            }
-            (PassMode::Cast(c1, pad1), PassMode::Cast(c2, pad2)) => c1 == c2 && pad1 == pad2,
-            (
-                PassMode::Indirect { attrs: a1, extra_attrs: None, on_stack: s1 },
-                PassMode::Indirect { attrs: a2, extra_attrs: None, on_stack: s2 },
-            ) => arg_attr_compat(a1, a2) && s1 == s2,
-            (
-                PassMode::Indirect { attrs: a1, extra_attrs: Some(e1), on_stack: s1 },
-                PassMode::Indirect { attrs: a2, extra_attrs: Some(e2), on_stack: s2 },
-            ) => arg_attr_compat(a1, a2) && arg_attr_compat(e1, e2) && s1 == s2,
-            _ => false,
-        };
-
         // 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() {
+        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 even imply that the layout is the same.
             assert!(
                 caller_abi.layout.size == callee_abi.layout.size
diff --git a/compiler/rustc_passes/src/abi_test.rs b/compiler/rustc_passes/src/abi_test.rs
@@ -121,9 +121,7 @@ fn test_arg_abi_eq<'tcx>(
     // Ideally we'd just compare the `mode`, but that is not enough -- for some modes LLVM will look
     // at the type. Comparing the `mode` and `layout.abi` should catch basically everything though
     // (except for tricky cases around unized types).
-    // This *is* overly strict (e.g. we compare the sign of integer `Primitive`s, or parts of `ArgAttributes` that do not affect ABI),
-    // but for the purpose of ensuring repr(transparent) ABI compatibility that is fine.
-    abi1.mode == abi2.mode && abi1.layout.abi == abi2.layout.abi
+    abi1.mode.eq_abi(&abi2.mode) && abi1.layout.abi.eq_up_to_validity(&abi2.layout.abi)
 }
 
 fn test_abi_eq<'tcx>(abi1: &'tcx FnAbi<'tcx, Ty<'tcx>>, abi2: &'tcx FnAbi<'tcx, Ty<'tcx>>) -> bool {
diff --git a/compiler/rustc_target/src/abi/call/mod.rs b/compiler/rustc_target/src/abi/call/mod.rs
@@ -55,6 +55,28 @@ pub enum PassMode {
     Indirect { attrs: ArgAttributes, extra_attrs: Option<ArgAttributes>, on_stack: bool },
 }
 
+impl PassMode {
+    /// Checks if these two `PassMode` are equal enough to be considered "the same for all
+    /// function call ABIs".
+    pub fn eq_abi(&self, other: &Self) -> bool {
+        match (self, other) {
+            (PassMode::Ignore, PassMode::Ignore) => true, // can still be reached for the return type
+            (PassMode::Direct(a1), PassMode::Direct(a2)) => a1.eq_abi(a2),
+            (PassMode::Pair(a1, b1), PassMode::Pair(a2, b2)) => a1.eq_abi(a2) && b1.eq_abi(b2),
+            (PassMode::Cast(c1, pad1), PassMode::Cast(c2, pad2)) => c1.eq_abi(c2) && pad1 == pad2,
+            (
+                PassMode::Indirect { attrs: a1, extra_attrs: None, on_stack: s1 },
+                PassMode::Indirect { attrs: a2, extra_attrs: None, on_stack: s2 },
+            ) => a1.eq_abi(a2) && s1 == s2,
+            (
+                PassMode::Indirect { attrs: a1, extra_attrs: Some(e1), on_stack: s1 },
+                PassMode::Indirect { attrs: a2, extra_attrs: Some(e2), on_stack: s2 },
+            ) => a1.eq_abi(a2) && e1.eq_abi(e2) && s1 == s2,
+            _ => false,
+        }
+    }
+}
+
 // Hack to disable non_upper_case_globals only for the bitflags! and not for the rest
 // of this module
 pub use attr_impl::ArgAttribute;
@@ -127,6 +149,24 @@ impl ArgAttributes {
     pub fn contains(&self, attr: ArgAttribute) -> bool {
         self.regular.contains(attr)
     }
+
+    /// Checks if these two `ArgAttributes` are equal enough to be considered "the same for all
+    /// function call ABIs".
+    pub fn eq_abi(&self, other: &Self) -> bool {
+        // There's only one regular attribute that matters for the call ABI: InReg.
+        // Everything else is things like noalias, dereferenceable, nonnull, ...
+        // (This also applies to pointee_size, pointee_align.)
+        if self.regular.contains(ArgAttribute::InReg) != other.regular.contains(ArgAttribute::InReg)
+        {
+            return false;
+        }
+        // We also compare the sign extension mode -- this could let the callee make assumptions
+        // about bits that conceptually were not even passed.
+        if self.arg_ext != other.arg_ext {
+            return false;
+        }
+        return true;
+    }
 }
 
 #[derive(Copy, Clone, PartialEq, Eq, Hash, Debug, HashStable_Generic)]
@@ -272,6 +312,14 @@ impl CastTarget {
                 acc.max(align)
             })
     }
+
+    /// Checks if these two `CastTarget` are equal enough to be considered "the same for all
+    /// function call ABIs".
+    pub fn eq_abi(&self, other: &Self) -> bool {
+        let CastTarget { prefix: prefix_l, rest: rest_l, attrs: attrs_l } = self;
+        let CastTarget { prefix: prefix_r, rest: rest_r, attrs: attrs_r } = other;
+        prefix_l == prefix_r && rest_l == rest_r && attrs_l.eq_abi(attrs_r)
+    }
 }
 
 /// Return value from the `homogeneous_aggregate` test function.
diff --git a/tests/ui/abi/compatibility.rs b/tests/ui/abi/compatibility.rs
@@ -0,0 +1,76 @@
+// check-pass
+#![feature(rustc_attrs)]
+#![allow(unused, improper_ctypes_definitions)]
+use std::num::NonZeroI32;
+use std::ptr::NonNull;
+
+macro_rules! assert_abi_compatible {
+    ($name:ident, $t1:ty, $t2:ty) => {
+        mod $name {
+            use super::*;
+            // Test argument and return value, `Rust` and `C` ABIs.
+            #[rustc_abi(assert_eq)]
+            type TestRust = (fn($t1) -> $t1, fn($t2) -> $t2);
+            #[rustc_abi(assert_eq)]
+            type TestC = (extern "C" fn($t1) -> $t1, extern "C" fn($t2) -> $t2);
+        }
+    };
+}
+
+#[derive(Copy, Clone)]
+struct Zst;
+
+#[repr(C)]
+struct ReprC1<T>(T);
+#[repr(C)]
+struct ReprC2Int<T>(i32, T);
+#[repr(C)]
+struct ReprC2Float<T>(f32, T);
+#[repr(C)]
+struct ReprC4<T>(T, T, T, T);
+#[repr(C)]
+struct ReprC4Mixed<T>(T, f32, i32, T);
+#[repr(C)]
+enum ReprCEnum<T> {
+    Variant1,
+    Variant2(T),
+}
+#[repr(C)]
+union ReprCUnion<T: Copy> {
+    nothing: (),
+    something: T,
+}
+
+macro_rules! test_abi_compatible {
+    ($name:ident, $t1:ty, $t2:ty) => {
+        mod $name {
+            use super::*;
+            assert_abi_compatible!(plain, $t1, $t2);
+            // We also do some tests with differences in fields of `repr(C)` types.
+            assert_abi_compatible!(repr_c_1, ReprC1<$t1>, ReprC1<$t2>);
+            assert_abi_compatible!(repr_c_2_int, ReprC2Int<$t1>, ReprC2Int<$t2>);
+            assert_abi_compatible!(repr_c_2_float, ReprC2Float<$t1>, ReprC2Float<$t2>);
+            assert_abi_compatible!(repr_c_4, ReprC4<$t1>, ReprC4<$t2>);
+            assert_abi_compatible!(repr_c_4mixed, ReprC4Mixed<$t1>, ReprC4Mixed<$t2>);
+            assert_abi_compatible!(repr_c_enum, ReprCEnum<$t1>, ReprCEnum<$t2>);
+            assert_abi_compatible!(repr_c_union, ReprCUnion<$t1>, ReprCUnion<$t2>);
+        }
+    };
+}
+
+// Compatibility of pointers is probably de-facto guaranteed,
+// but that does not seem to be documented.
+test_abi_compatible!(ptr_mut, *const i32, *mut i32);
+test_abi_compatible!(ptr_pointee, *const i32, *const Vec<i32>);
+test_abi_compatible!(ref_mut, &i32, &mut i32);
+test_abi_compatible!(ref_ptr, &i32, *const i32);
+test_abi_compatible!(box_ptr, Box<i32>, *const i32);
+test_abi_compatible!(nonnull_ptr, NonNull<i32>, *const i32);
+test_abi_compatible!(fn_fn, fn(), fn(i32) -> i32);
+
+// Some further guarantees we will likely (have to) make.
+test_abi_compatible!(zst_unit, Zst, ());
+test_abi_compatible!(zst_array, Zst, [u8; 0]);
+test_abi_compatible!(nonzero_int, NonZeroI32, i32);
+
+fn main() {}
diff --git a/tests/ui/abi/debug.rs b/tests/ui/abi/debug.rs
@@ -38,3 +38,10 @@ type TestAbiEq = (fn(bool), fn(bool));
 
 #[rustc_abi(assert_eq)]
 type TestAbiNe = (fn(u8), fn(u32)); //~ ERROR: ABIs are not compatible
+
+#[rustc_abi(assert_eq)]
+type TestAbiNeFloat = (fn(f32), fn(u32)); //~ ERROR: ABIs are not compatible
+
+// Sign matters on some targets (such as s390x), so let's make sure we never accept this.
+#[rustc_abi(assert_eq)]
+type TestAbiNeSign = (fn(i32), fn(u32)); //~ ERROR: ABIs are not compatible
diff --git a/tests/ui/abi/debug.stderr b/tests/ui/abi/debug.stderr
