From c3e14edd8b7cbb502002a2390bf0ca5c6d09a315 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 2 Sep 2023 11:59:31 +0200 Subject: [PATCH] accept some differences for rustc_abi(assert_eq), so that we can test more things to be compatible --- compiler/rustc_abi/src/lib.rs | 17 + .../src/interpret/terminator.rs | 70 +---- compiler/rustc_passes/src/abi_test.rs | 4 +- compiler/rustc_target/src/abi/call/mod.rs | 48 +++ tests/ui/abi/compatibility.rs | 76 +++++ tests/ui/abi/debug.rs | 7 + tests/ui/abi/debug.stderr | 291 +++++++++++++++++- 7 files changed, 449 insertions(+), 64 deletions(-) create mode 100644 tests/ui/abi/compatibility.rs diff --git a/compiler/rustc_abi/src/lib.rs b/compiler/rustc_abi/src/lib.rs index 571aaf631bd..f9393539ea4 100644 --- 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 index bc4edf1c4b6..6312dc7414d 100644 --- 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,40 +322,6 @@ 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 @@ -381,7 +329,9 @@ 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 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 index 48c18912703..0c707b4ef9c 100644 --- 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 index 9a905dbc806..42483b7c6a5 100644 --- 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, 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 new file mode 100644 index 00000000000..0bf218730ba --- /dev/null +++ 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); +#[repr(C)] +struct ReprC2Int(i32, T); +#[repr(C)] +struct ReprC2Float(f32, T); +#[repr(C)] +struct ReprC4(T, T, T, T); +#[repr(C)] +struct ReprC4Mixed(T, f32, i32, T); +#[repr(C)] +enum ReprCEnum { + Variant1, + Variant2(T), +} +#[repr(C)] +union ReprCUnion { + 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); +test_abi_compatible!(ref_mut, &i32, &mut i32); +test_abi_compatible!(ref_ptr, &i32, *const i32); +test_abi_compatible!(box_ptr, Box, *const i32); +test_abi_compatible!(nonnull_ptr, NonNull, *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 index 058c205abd8..d08945ce3c2 100644 --- 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 index 321ec8715c6..dffeef444f5 100644 --- a/tests/ui/abi/debug.stderr +++ b/tests/ui/abi/debug.stderr @@ -508,5 +508,294 @@ error: ABIs are not compatible LL | type TestAbiNe = (fn(u8), fn(u32)); | ^^^^^^^^^^^^^^ -error: aborting due to 7 previous errors +error: ABIs are not compatible + left ABI = FnAbi { + args: [ + ArgAbi { + layout: TyAndLayout { + ty: f32, + layout: Layout { + size: $SOME_SIZE, + align: AbiAndPrefAlign { + abi: $SOME_ALIGN, + pref: $SOME_ALIGN, + }, + abi: Scalar( + Initialized { + value: F32, + valid_range: $FULL, + }, + ), + fields: Primitive, + largest_niche: None, + variants: Single { + index: 0, + }, + max_repr_align: None, + unadjusted_abi_align: $SOME_ALIGN, + }, + }, + mode: Direct( + ArgAttributes { + regular: NoUndef, + arg_ext: None, + pointee_size: Size(0 bytes), + pointee_align: None, + }, + ), + }, + ], + ret: ArgAbi { + layout: TyAndLayout { + ty: (), + layout: Layout { + size: Size(0 bytes), + align: AbiAndPrefAlign { + abi: $SOME_ALIGN, + pref: $SOME_ALIGN, + }, + abi: Aggregate { + sized: true, + }, + fields: Arbitrary { + offsets: [], + memory_index: [], + }, + largest_niche: None, + variants: Single { + index: 0, + }, + max_repr_align: None, + unadjusted_abi_align: $SOME_ALIGN, + }, + }, + mode: Ignore, + }, + c_variadic: false, + fixed_count: 1, + conv: Rust, + can_unwind: $SOME_BOOL, + } + right ABI = FnAbi { + args: [ + ArgAbi { + layout: TyAndLayout { + ty: u32, + layout: Layout { + size: $SOME_SIZE, + align: AbiAndPrefAlign { + abi: $SOME_ALIGN, + pref: $SOME_ALIGN, + }, + abi: Scalar( + Initialized { + value: Int( + I32, + false, + ), + valid_range: $FULL, + }, + ), + fields: Primitive, + largest_niche: None, + variants: Single { + index: 0, + }, + max_repr_align: None, + unadjusted_abi_align: $SOME_ALIGN, + }, + }, + mode: Direct( + ArgAttributes { + regular: NoUndef, + arg_ext: None, + pointee_size: Size(0 bytes), + pointee_align: None, + }, + ), + }, + ], + ret: ArgAbi { + layout: TyAndLayout { + ty: (), + layout: Layout { + size: Size(0 bytes), + align: AbiAndPrefAlign { + abi: $SOME_ALIGN, + pref: $SOME_ALIGN, + }, + abi: Aggregate { + sized: true, + }, + fields: Arbitrary { + offsets: [], + memory_index: [], + }, + largest_niche: None, + variants: Single { + index: 0, + }, + max_repr_align: None, + unadjusted_abi_align: $SOME_ALIGN, + }, + }, + mode: Ignore, + }, + c_variadic: false, + fixed_count: 1, + conv: Rust, + can_unwind: $SOME_BOOL, + } + --> $DIR/debug.rs:43:1 + | +LL | type TestAbiNeFloat = (fn(f32), fn(u32)); + | ^^^^^^^^^^^^^^^^^^^ + +error: ABIs are not compatible + left ABI = FnAbi { + args: [ + ArgAbi { + layout: TyAndLayout { + ty: i32, + layout: Layout { + size: $SOME_SIZE, + align: AbiAndPrefAlign { + abi: $SOME_ALIGN, + pref: $SOME_ALIGN, + }, + abi: Scalar( + Initialized { + value: Int( + I32, + true, + ), + valid_range: $FULL, + }, + ), + fields: Primitive, + largest_niche: None, + variants: Single { + index: 0, + }, + max_repr_align: None, + unadjusted_abi_align: $SOME_ALIGN, + }, + }, + mode: Direct( + ArgAttributes { + regular: NoUndef, + arg_ext: None, + pointee_size: Size(0 bytes), + pointee_align: None, + }, + ), + }, + ], + ret: ArgAbi { + layout: TyAndLayout { + ty: (), + layout: Layout { + size: Size(0 bytes), + align: AbiAndPrefAlign { + abi: $SOME_ALIGN, + pref: $SOME_ALIGN, + }, + abi: Aggregate { + sized: true, + }, + fields: Arbitrary { + offsets: [], + memory_index: [], + }, + largest_niche: None, + variants: Single { + index: 0, + }, + max_repr_align: None, + unadjusted_abi_align: $SOME_ALIGN, + }, + }, + mode: Ignore, + }, + c_variadic: false, + fixed_count: 1, + conv: Rust, + can_unwind: $SOME_BOOL, + } + right ABI = FnAbi { + args: [ + ArgAbi { + layout: TyAndLayout { + ty: u32, + layout: Layout { + size: $SOME_SIZE, + align: AbiAndPrefAlign { + abi: $SOME_ALIGN, + pref: $SOME_ALIGN, + }, + abi: Scalar( + Initialized { + value: Int( + I32, + false, + ), + valid_range: $FULL, + }, + ), + fields: Primitive, + largest_niche: None, + variants: Single { + index: 0, + }, + max_repr_align: None, + unadjusted_abi_align: $SOME_ALIGN, + }, + }, + mode: Direct( + ArgAttributes { + regular: NoUndef, + arg_ext: None, + pointee_size: Size(0 bytes), + pointee_align: None, + }, + ), + }, + ], + ret: ArgAbi { + layout: TyAndLayout { + ty: (), + layout: Layout { + size: Size(0 bytes), + align: AbiAndPrefAlign { + abi: $SOME_ALIGN, + pref: $SOME_ALIGN, + }, + abi: Aggregate { + sized: true, + }, + fields: Arbitrary { + offsets: [], + memory_index: [], + }, + largest_niche: None, + variants: Single { + index: 0, + }, + max_repr_align: None, + unadjusted_abi_align: $SOME_ALIGN, + }, + }, + mode: Ignore, + }, + c_variadic: false, + fixed_count: 1, + conv: Rust, + can_unwind: $SOME_BOOL, + } + --> $DIR/debug.rs:47:1 + | +LL | type TestAbiNeSign = (fn(i32), fn(u32)); + | ^^^^^^^^^^^^^^^^^^ + +error: aborting due to 9 previous errors