Skip to content

Commit

Permalink
Auto merge of rust-lang#117500 - RalfJung:aggregate-abi, r=davidtwco
Browse files Browse the repository at this point in the history
Ensure sanity of all computed ABIs

This moves the ABI sanity assertions from the codegen backend to the ABI computation logic. Sadly, due to past mistakes, we [have to](rust-lang#117351 (comment)) be able to compute a sane ABI for nonsensical function types like `extern "C" fn(str) -> str`.  So to make the sanity check pass we first need to make all ABI adjustment deal with unsized types... and we have no shared infrastructure for those adjustments, so that's a bunch of copy-paste. At least we have assertions failing loudly when one accidentally sets a different mode for an unsized argument.

To achieve this, this re-lands the parts of rust-lang#80594 that got reverted in rust-lang#81388.  To avoid breaking wasm ABI again, that ABI now explicitly opts-in to the (wrong, broken) ABI that we currently keep for backwards compatibility. That's still better than having *every* ABI use the wrong broken default!

Cc `@bjorn3`
Fixes rust-lang#115845
  • Loading branch information
bors committed Nov 19, 2023
2 parents 097261f + cfb47ca commit 7f172a1
Show file tree
Hide file tree
Showing 21 changed files with 256 additions and 74 deletions.
43 changes: 3 additions & 40 deletions compiler/rustc_codegen_llvm/src/abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -348,50 +348,18 @@ impl<'ll, 'tcx> FnAbiLlvmExt<'ll, 'tcx> for FnAbi<'tcx, Ty<'tcx>> {
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 { .. }) {
assert!(
arg.layout.is_sized(),
"`PassMode::Direct` for unsized type: {}",
arg.layout.ty
);
// 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>.)
//
// The unstable abi `PtxKernel` also uses Direct for now.
// It needs to switch to something else before stabilization can happen.
// (See issue: https://github.com/rust-lang/rust/issues/117271)
assert!(
matches!(&*cx.tcx.sess.target.arch, "wasm32" | "wasm64")
|| self.conv == Conv::PtxKernel,
"`PassMode::Direct` for aggregates only allowed on wasm and `extern \"ptx-kernel\"` fns\nProblematic type: {:#?}",
arg.layout,
);
}
// guaranteeing that we generate ABI-compatible LLVM IR.
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: _, meta_attrs: Some(_), on_stack } => {
// `Indirect` with metadata is only for unsized types, and doesn't work with
// on-stack passing.
assert!(arg.layout.is_unsized() && !on_stack);
PassMode::Indirect { attrs: _, meta_attrs: Some(_), on_stack: _ } => {
// 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
Expand All @@ -402,13 +370,8 @@ impl<'ll, 'tcx> FnAbiLlvmExt<'ll, 'tcx> for FnAbi<'tcx, Ty<'tcx>> {
llargument_tys.push(ptr_layout.scalar_pair_element_llvm_type(cx, 1, true));
continue;
}
PassMode::Indirect { attrs: _, meta_attrs: None, on_stack: _ } => {
assert!(arg.layout.is_sized());
cx.type_ptr()
}
PassMode::Indirect { attrs: _, meta_attrs: None, on_stack: _ } => cx.type_ptr(),
PassMode::Cast { cast, pad_i32 } => {
// `Cast` means "transmute to `CastType`"; that only makes sense for sized types.
assert!(arg.layout.is_sized());
// add padding
if *pad_i32 {
llargument_tys.push(Reg::i32().llvm_type(cx));
Expand Down
8 changes: 8 additions & 0 deletions compiler/rustc_target/src/abi/call/aarch64.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,10 @@ where
Ty: TyAbiInterface<'a, C> + Copy,
C: HasDataLayout,
{
if !ret.layout.is_sized() {
// Not touching this...
return;
}
if !ret.layout.is_aggregate() {
if kind == AbiKind::DarwinPCS {
// On Darwin, when returning an i8/i16, it must be sign-extended to 32 bits,
Expand Down Expand Up @@ -67,6 +71,10 @@ where
Ty: TyAbiInterface<'a, C> + Copy,
C: HasDataLayout,
{
if !arg.layout.is_sized() {
// Not touching this...
return;
}
if !arg.layout.is_aggregate() {
if kind == AbiKind::DarwinPCS {
// On Darwin, when passing an i8/i16, it must be sign-extended to 32 bits,
Expand Down
8 changes: 8 additions & 0 deletions compiler/rustc_target/src/abi/call/arm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ where
Ty: TyAbiInterface<'a, C> + Copy,
C: HasDataLayout,
{
if !ret.layout.is_sized() {
// Not touching this...
return;
}
if !ret.layout.is_aggregate() {
ret.extend_integer_width_to(32);
return;
Expand All @@ -56,6 +60,10 @@ where
Ty: TyAbiInterface<'a, C> + Copy,
C: HasDataLayout,
{
if !arg.layout.is_sized() {
// Not touching this...
return;
}
if !arg.layout.is_aggregate() {
arg.extend_integer_width_to(32);
return;
Expand Down
8 changes: 8 additions & 0 deletions compiler/rustc_target/src/abi/call/csky.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@
use crate::abi::call::{ArgAbi, FnAbi, Reg, Uniform};

fn classify_ret<Ty>(arg: &mut ArgAbi<'_, Ty>) {
if !arg.layout.is_sized() {
// Not touching this...
return;
}
// For return type, aggregate which <= 2*XLen will be returned in registers.
// Otherwise, aggregate will be returned indirectly.
if arg.layout.is_aggregate() {
Expand All @@ -24,6 +28,10 @@ fn classify_ret<Ty>(arg: &mut ArgAbi<'_, Ty>) {
}

fn classify_arg<Ty>(arg: &mut ArgAbi<'_, Ty>) {
if !arg.layout.is_sized() {
// Not touching this...
return;
}
// For argument type, the first 4*XLen parts of aggregate will be passed
// in registers, and the rest will be passed in stack.
// So we can coerce to integers directly and let backend handle it correctly.
Expand Down
8 changes: 8 additions & 0 deletions compiler/rustc_target/src/abi/call/loongarch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,10 @@ fn classify_ret<'a, Ty, C>(cx: &C, arg: &mut ArgAbi<'a, Ty>, xlen: u64, flen: u6
where
Ty: TyAbiInterface<'a, C> + Copy,
{
if !arg.layout.is_sized() {
// Not touching this...
return false; // I guess? return value of this function is not documented
}
if let Some(conv) = should_use_fp_conv(cx, &arg.layout, xlen, flen) {
match conv {
FloatConv::Float(f) => {
Expand Down Expand Up @@ -214,6 +218,10 @@ fn classify_arg<'a, Ty, C>(
) where
Ty: TyAbiInterface<'a, C> + Copy,
{
if !arg.layout.is_sized() {
// Not touching this...
return;
}
if !is_vararg {
match should_use_fp_conv(cx, &arg.layout, xlen, flen) {
Some(FloatConv::Float(f)) if *avail_fprs >= 1 => {
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_target/src/abi/call/m68k.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ fn classify_ret<Ty>(ret: &mut ArgAbi<'_, Ty>) {
}

fn classify_arg<Ty>(arg: &mut ArgAbi<'_, Ty>) {
if !arg.layout.is_sized() {
// Not touching this...
return;
}
if arg.layout.is_aggregate() {
arg.make_indirect_byval(None);
} else {
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_target/src/abi/call/mips.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ fn classify_arg<Ty, C>(cx: &C, arg: &mut ArgAbi<'_, Ty>, offset: &mut Size)
where
C: HasDataLayout,
{
if !arg.layout.is_sized() {
// Not touching this...
return;
}
let dl = cx.data_layout();
let size = arg.layout.size;
let align = arg.layout.align.max(dl.i32_align).min(dl.i64_align).abi;
Expand Down
18 changes: 14 additions & 4 deletions compiler/rustc_target/src/abi/call/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -422,7 +422,7 @@ impl<'a, Ty> TyAndLayout<'a, Ty> {
}))
}

Abi::ScalarPair(..) | Abi::Aggregate { .. } => {
Abi::ScalarPair(..) | Abi::Aggregate { sized: true } => {
// Helper for computing `homogeneous_aggregate`, allowing a custom
// starting offset (used below for handling variants).
let from_fields_at =
Expand Down Expand Up @@ -520,6 +520,7 @@ impl<'a, Ty> TyAndLayout<'a, Ty> {
Ok(result)
}
}
Abi::Aggregate { sized: false } => Err(Heterogeneous),
}
}
}
Expand Down Expand Up @@ -555,8 +556,7 @@ impl<'a, Ty> ArgAbi<'a, Ty> {
scalar_attrs(&layout, b, a.size(cx).align_to(b.align(cx).abi)),
),
Abi::Vector { .. } => PassMode::Direct(ArgAttributes::new()),
// The `Aggregate` ABI should always be adjusted later.
Abi::Aggregate { .. } => PassMode::Direct(ArgAttributes::new()),
Abi::Aggregate { .. } => Self::indirect_pass_mode(&layout),
};
ArgAbi { layout, mode }
}
Expand All @@ -580,10 +580,20 @@ impl<'a, Ty> ArgAbi<'a, Ty> {
PassMode::Indirect { attrs, meta_attrs, on_stack: false }
}

/// Pass this argument directly instead. Should NOT be used!
/// Only exists because of past ABI mistakes that will take time to fix
/// (see <https://github.com/rust-lang/rust/issues/115666>).
pub fn make_direct_deprecated(&mut self) {
self.mode = PassMode::Direct(ArgAttributes::new());
}

pub fn make_indirect(&mut self) {
match self.mode {
PassMode::Direct(_) | PassMode::Pair(_, _) => {}
PassMode::Indirect { attrs: _, meta_attrs: None, on_stack: false } => return,
PassMode::Indirect { attrs: _, meta_attrs: _, on_stack: false } => {
// already indirect
return;
}
_ => panic!("Tried to make {:?} indirect", self.mode),
}

Expand Down
9 changes: 9 additions & 0 deletions compiler/rustc_target/src/abi/call/nvptx64.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,18 @@ use crate::abi::{HasDataLayout, TyAbiInterface};
fn classify_ret<Ty>(ret: &mut ArgAbi<'_, Ty>) {
if ret.layout.is_aggregate() && ret.layout.size.bits() > 64 {
ret.make_indirect();
} else {
// FIXME: this is wrong! Need to decide which ABI we really want here.
ret.make_direct_deprecated();
}
}

fn classify_arg<Ty>(arg: &mut ArgAbi<'_, Ty>) {
if arg.layout.is_aggregate() && arg.layout.size.bits() > 64 {
arg.make_indirect();
} else {
// FIXME: this is wrong! Need to decide which ABI we really want here.
arg.make_direct_deprecated();
}
}

Expand All @@ -30,6 +36,9 @@ where
_ => unreachable!("Align is given as power of 2 no larger than 16 bytes"),
};
arg.cast_to(Uniform { unit, total: Size::from_bytes(2 * align_bytes) });
} else {
// FIXME: find a better way to do this. See https://github.com/rust-lang/rust/issues/117271.
arg.make_direct_deprecated();
}
}

Expand Down
8 changes: 8 additions & 0 deletions compiler/rustc_target/src/abi/call/powerpc64.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@ where
Ty: TyAbiInterface<'a, C> + Copy,
C: HasDataLayout,
{
if !ret.layout.is_sized() {
// Not touching this...
return;
}
if !ret.layout.is_aggregate() {
ret.extend_integer_width_to(64);
return;
Expand Down Expand Up @@ -89,6 +93,10 @@ where
Ty: TyAbiInterface<'a, C> + Copy,
C: HasDataLayout,
{
if !arg.layout.is_sized() {
// Not touching this...
return;
}
if !arg.layout.is_aggregate() {
arg.extend_integer_width_to(64);
return;
Expand Down
8 changes: 8 additions & 0 deletions compiler/rustc_target/src/abi/call/riscv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,10 @@ fn classify_ret<'a, Ty, C>(cx: &C, arg: &mut ArgAbi<'a, Ty>, xlen: u64, flen: u6
where
Ty: TyAbiInterface<'a, C> + Copy,
{
if !arg.layout.is_sized() {
// Not touching this...
return false; // I guess? return value of this function is not documented
}
if let Some(conv) = should_use_fp_conv(cx, &arg.layout, xlen, flen) {
match conv {
FloatConv::Float(f) => {
Expand Down Expand Up @@ -220,6 +224,10 @@ fn classify_arg<'a, Ty, C>(
) where
Ty: TyAbiInterface<'a, C> + Copy,
{
if !arg.layout.is_sized() {
// Not touching this...
return;
}
if !is_vararg {
match should_use_fp_conv(cx, &arg.layout, xlen, flen) {
Some(FloatConv::Float(f)) if *avail_fprs >= 1 => {
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_target/src/abi/call/s390x.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ where
Ty: TyAbiInterface<'a, C> + Copy,
C: HasDataLayout,
{
if !arg.layout.is_sized() {
// Not touching this...
return;
}
if !arg.layout.is_aggregate() && arg.layout.size.bits() <= 64 {
arg.extend_integer_width_to(64);
return;
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_target/src/abi/call/sparc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ fn classify_arg<Ty, C>(cx: &C, arg: &mut ArgAbi<'_, Ty>, offset: &mut Size)
where
C: HasDataLayout,
{
if !arg.layout.is_sized() {
// Not touching this...
return;
}
let dl = cx.data_layout();
let size = arg.layout.size;
let align = arg.layout.align.max(dl.i32_align).min(dl.i64_align).abi;
Expand Down
24 changes: 20 additions & 4 deletions compiler/rustc_target/src/abi/call/wasm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ where
Ty: TyAbiInterface<'a, C> + Copy,
C: HasDataLayout,
{
if !arg.layout.is_sized() {
// Not touching this...
return;
}
arg.extend_integer_width_to(32);
if arg.layout.is_aggregate() && !unwrap_trivial_aggregate(cx, arg) {
arg.make_indirect_byval(None);
Expand Down Expand Up @@ -67,21 +71,33 @@ where
/// Also see <https://github.com/rust-lang/rust/issues/115666>.
pub fn compute_wasm_abi_info<Ty>(fn_abi: &mut FnAbi<'_, Ty>) {
if !fn_abi.ret.is_ignore() {
classify_ret(&mut fn_abi.ret);
classify_ret_wasm_abi(&mut fn_abi.ret);
}

for arg in fn_abi.args.iter_mut() {
if arg.is_ignore() {
continue;
}
classify_arg(arg);
classify_arg_wasm_abi(arg);
}

fn classify_ret<Ty>(ret: &mut ArgAbi<'_, Ty>) {
fn classify_ret_wasm_abi<Ty>(ret: &mut ArgAbi<'_, Ty>) {
if !ret.layout.is_sized() {
// Not touching this...
return;
}
// FIXME: this is bad! https://github.com/rust-lang/rust/issues/115666
ret.make_direct_deprecated();
ret.extend_integer_width_to(32);
}

fn classify_arg<Ty>(arg: &mut ArgAbi<'_, Ty>) {
fn classify_arg_wasm_abi<Ty>(arg: &mut ArgAbi<'_, Ty>) {
if !arg.layout.is_sized() {
// Not touching this...
return;
}
// FIXME: this is bad! https://github.com/rust-lang/rust/issues/115666
arg.make_direct_deprecated();
arg.extend_integer_width_to(32);
}
}
4 changes: 2 additions & 2 deletions compiler/rustc_target/src/abi/call/x86.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ where
C: HasDataLayout + HasTargetSpec,
{
if !fn_abi.ret.is_ignore() {
if fn_abi.ret.layout.is_aggregate() {
if fn_abi.ret.layout.is_aggregate() && fn_abi.ret.layout.is_sized() {
// Returning a structure. Most often, this will use
// a hidden first argument. On some platforms, though,
// small structs are returned as integers.
Expand Down Expand Up @@ -50,7 +50,7 @@ where
}

for arg in fn_abi.args.iter_mut() {
if arg.is_ignore() {
if arg.is_ignore() || !arg.layout.is_sized() {
continue;
}

Expand Down
Loading

0 comments on commit 7f172a1

Please sign in to comment.