Skip to content

Commit

Permalink
Auto merge of rust-lang#115372 - RalfJung:abi-assert-eq, r=davidtwco
Browse files Browse the repository at this point in the history
add rustc_abi(assert_eq) to test some guaranteed or at least highly expected ABI compatibility guarantees

This new repr(transparent) test is super useful, it would have found rust-lang#115336 and found rust-lang#115404, rust-lang#115481, rust-lang#115509.
  • Loading branch information
bors committed Sep 8, 2023
2 parents de4cba3 + bd13b26 commit 643ed81
Show file tree
Hide file tree
Showing 17 changed files with 1,300 additions and 170 deletions.
41 changes: 40 additions & 1 deletion compiler/rustc_abi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1300,12 +1300,18 @@ impl Abi {
matches!(*self, Abi::Uninhabited)
}

/// Returns `true` is this is a scalar type
/// Returns `true` if this is a scalar type
#[inline]
pub fn is_scalar(&self) -> bool {
matches!(*self, Abi::Scalar(_))
}

/// Returns `true` if this is a bool
#[inline]
pub fn is_bool(&self) -> bool {
matches!(*self, Abi::Scalar(s) if s.is_bool())
}

/// Returns the fixed alignment of this ABI, if any is mandated.
pub fn inherent_align<C: HasDataLayout>(&self, cx: &C) -> Option<AbiAndPrefAlign> {
Some(match *self {
Expand Down Expand Up @@ -1348,6 +1354,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)]
Expand Down Expand Up @@ -1686,6 +1709,22 @@ impl LayoutS {
Abi::Aggregate { sized } => sized && self.size.bytes() == 0,
}
}

/// Checks if these two `Layout` are equal enough to be considered "the same for all function
/// call ABIs". Note however that real ABIs depend on more details that are not reflected in the
/// `Layout`; the `PassMode` need to be compared as well.
pub fn eq_abi(&self, other: &Self) -> bool {
// The one thing that we are not capturing here is that for unsized types, the metadata must
// also have the same ABI, and moreover that the same metadata leads to the same size. The
// 2nd point is quite hard to check though.
self.size == other.size
&& self.is_sized() == other.is_sized()
&& self.abi.eq_up_to_validity(&other.abi)
&& self.abi.is_bool() == other.abi.is_bool()
&& self.align.abi == other.align.abi
&& self.max_repr_align == other.max_repr_align
&& self.unadjusted_abi_align == other.unadjusted_abi_align
}
}

#[derive(Copy, Clone, Debug)]
Expand Down
39 changes: 38 additions & 1 deletion compiler/rustc_codegen_llvm/src/abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -340,15 +340,50 @@ impl<'ll, 'tcx> FnAbiLlvmExt<'ll, 'tcx> for FnAbi<'tcx, Ty<'tcx>> {
};

for arg in args {
// Note that the exact number of arguments pushed here is carefully synchronized with
// code all over the place, both in the codegen_llvm and codegen_ssa crates. That's how
// other code then knows which LLVM argument(s) correspond to the n-th Rust argument.
let llarg_ty = match &arg.mode {
PassMode::Ignore => continue,
PassMode::Direct(_) => arg.layout.immediate_llvm_type(cx),
PassMode::Direct(_) => {
// ABI-compatible Rust types have the same `layout.abi` (up to validity ranges),
// and for Scalar ABIs the LLVM type is fully determined by `layout.abi`,
// guarnateeing that we generate ABI-compatible LLVM IR. Things get tricky for
// aggregates...
if matches!(arg.layout.abi, abi::Abi::Aggregate { .. }) {
// This really shouldn't happen, since `immediate_llvm_type` will use
// `layout.fields` to turn this Rust type into an LLVM type. This means all
// sorts of Rust type details leak into the ABI. However wasm sadly *does*
// currently use this mode so we have to allow it -- but we absolutely
// shouldn't let any more targets do that.
// (Also see <https://github.com/rust-lang/rust/issues/115666>.)
assert!(
matches!(&*cx.tcx.sess.target.arch, "wasm32" | "wasm64"),
"`PassMode::Direct` for aggregates only allowed on wasm targets\nProblematic type: {:#?}",
arg.layout,
);
}
arg.layout.immediate_llvm_type(cx)
}
PassMode::Pair(..) => {
// ABI-compatible Rust types have the same `layout.abi` (up to validity ranges),
// so for ScalarPair we can easily be sure that we are generating ABI-compatible
// LLVM IR.
assert!(
matches!(arg.layout.abi, abi::Abi::ScalarPair(..)),
"PassMode::Pair for type {}",
arg.layout.ty
);
llargument_tys.push(arg.layout.scalar_pair_element_llvm_type(cx, 0, true));
llargument_tys.push(arg.layout.scalar_pair_element_llvm_type(cx, 1, true));
continue;
}
PassMode::Indirect { attrs: _, extra_attrs: Some(_), on_stack: _ } => {
assert!(arg.layout.is_unsized());
// Construct the type of a (wide) pointer to `ty`, and pass its two fields.
// Any two ABI-compatible unsized types have the same metadata type and
// moreover the same metadata value leads to the same dynamic size and
// alignment, so this respects ABI compatibility.
let ptr_ty = Ty::new_mut_ptr(cx.tcx, arg.layout.ty);
let ptr_layout = cx.layout_of(ptr_ty);
llargument_tys.push(ptr_layout.scalar_pair_element_llvm_type(cx, 0, true));
Expand All @@ -360,6 +395,8 @@ impl<'ll, 'tcx> FnAbiLlvmExt<'ll, 'tcx> for FnAbi<'tcx, Ty<'tcx>> {
if *pad_i32 {
llargument_tys.push(Reg::i32().llvm_type(cx));
}
// Compute the LLVM type we use for this function from the cast type.
// We assume here that ABI-compatible Rust types have the same cast type.
cast.llvm_type(cx)
}
PassMode::Indirect { attrs: _, extra_attrs: None, on_stack: _ } => cx.type_ptr(),
Expand Down
78 changes: 12 additions & 66 deletions compiler/rustc_const_eval/src/interpret/terminator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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.
Expand All @@ -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,
}
}

Expand All @@ -340,54 +322,18 @@ 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() {
// 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()
);
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 imply layout ABI compatibility.
assert!(caller_abi.layout.eq_abi(&callee_abi.layout));
return true;
} else {
trace!(
Expand Down
31 changes: 17 additions & 14 deletions compiler/rustc_passes/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,14 @@
-passes_see_issue =
see issue #{$issue} <https://github.com/rust-lang/rust/issues/{$issue}> for more information
passes_abi =
abi: {$abi}
passes_abi_invalid_attribute =
`#[rustc_abi]` can only be applied to function items, type aliases, and associated functions
passes_abi_ne =
ABIs are not compatible
left ABI = {$left}
right ABI = {$right}
passes_abi_of =
fn_abi_of_instance({$fn_name}) = {$fn_abi}
passes_align =
align: {$align}
fn_abi_of({$fn_name}) = {$fn_abi}
passes_allow_incoherent_impl =
`rustc_allow_incoherent_impl` attribute should be applied to impl items.
Expand Down Expand Up @@ -318,9 +318,6 @@ passes_has_incoherent_inherent_impl =
`rustc_has_incoherent_inherent_impls` attribute should be applied to types or traits.
.label = only adts, extern types and traits are supported
passes_homogeneous_aggregate =
homogeneous_aggregate: {$homogeneous_aggregate}
passes_ignored_attr =
`#[{$sym}]` is ignored on struct fields and match arms
.warn = {-passes_previously_accepted}
Expand Down Expand Up @@ -404,9 +401,18 @@ passes_lang_item_on_incorrect_target =
passes_layout =
layout error: {$layout_error}
passes_layout_abi =
abi: {$abi}
passes_layout_align =
align: {$align}
passes_layout_homogeneous_aggregate =
homogeneous_aggregate: {$homogeneous_aggregate}
passes_layout_invalid_attribute =
`#[rustc_layout]` can only be applied to `struct`/`enum`/`union` declarations and type aliases
passes_layout_of =
layout_of({$normalized_ty}) = {$ty_layout}
passes_layout_size =
size: {$size}
passes_link =
attribute should be applied to an `extern` block with non-Rust ABI
Expand Down Expand Up @@ -662,9 +668,6 @@ passes_should_be_applied_to_trait =
attribute should be applied to a trait
.label = not a trait
passes_size =
size: {$size}
passes_skipping_const_checks = skipping const checks
passes_stability_promotable =
Expand Down
Loading

0 comments on commit 643ed81

Please sign in to comment.