Skip to content

Commit d19980e

Browse files
borsbors
committed
Auto merge of rust-lang#117500 - RalfJung:aggregate-abi, r=davidtwco
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
2 parents 290fc68 + c7b8dd4 commit d19980e

21 files changed

+265
-77
lines changed

‎compiler/rustc_codegen_llvm/src/abi.rs

+3-40
Original file line numberDiff line numberDiff line change
@@ -348,50 +348,18 @@ impl<'ll, 'tcx> FnAbiLlvmExt<'ll, 'tcx> for FnAbi<'tcx, Ty<'tcx>> {
348348
PassMode::Direct(_) => {
349349
// ABI-compatible Rust types have the same `layout.abi` (up to validity ranges),
350350
// and for Scalar ABIs the LLVM type is fully determined by `layout.abi`,
351-
// guarnateeing that we generate ABI-compatible LLVM IR. Things get tricky for
352-
// aggregates...
353-
if matches!(arg.layout.abi, abi::Abi::Aggregate { .. }) {
354-
assert!(
355-
arg.layout.is_sized(),
356-
"`PassMode::Direct` for unsized type: {}",
357-
arg.layout.ty
358-
);
359-
// This really shouldn't happen, since `immediate_llvm_type` will use
360-
// `layout.fields` to turn this Rust type into an LLVM type. This means all
361-
// sorts of Rust type details leak into the ABI. However wasm sadly *does*
362-
// currently use this mode so we have to allow it -- but we absolutely
363-
// shouldn't let any more targets do that.
364-
// (Also see <https://github.com/rust-lang/rust/issues/115666>.)
365-
//
366-
// The unstable abi `PtxKernel` also uses Direct for now.
367-
// It needs to switch to something else before stabilization can happen.
368-
// (See issue: https://github.com/rust-lang/rust/issues/117271)
369-
assert!(
370-
matches!(&*cx.tcx.sess.target.arch, "wasm32" | "wasm64")
371-
|| self.conv == Conv::PtxKernel,
372-
"`PassMode::Direct` for aggregates only allowed on wasm and `extern \"ptx-kernel\"` fns\nProblematic type: {:#?}",
373-
arg.layout,
374-
);
375-
}
351+
// guaranteeing that we generate ABI-compatible LLVM IR.
376352
arg.layout.immediate_llvm_type(cx)
377353
}
378354
PassMode::Pair(..) => {
379355
// ABI-compatible Rust types have the same `layout.abi` (up to validity ranges),
380356
// so for ScalarPair we can easily be sure that we are generating ABI-compatible
381357
// LLVM IR.
382-
assert!(
383-
matches!(arg.layout.abi, abi::Abi::ScalarPair(..)),
384-
"PassMode::Pair for type {}",
385-
arg.layout.ty
386-
);
387358
llargument_tys.push(arg.layout.scalar_pair_element_llvm_type(cx, 0, true));
388359
llargument_tys.push(arg.layout.scalar_pair_element_llvm_type(cx, 1, true));
389360
continue;
390361
}
391-
PassMode::Indirect { attrs: _, meta_attrs: Some(_), on_stack } => {
392-
// `Indirect` with metadata is only for unsized types, and doesn't work with
393-
// on-stack passing.
394-
assert!(arg.layout.is_unsized() && !on_stack);
362+
PassMode::Indirect { attrs: _, meta_attrs: Some(_), on_stack: _ } => {
395363
// Construct the type of a (wide) pointer to `ty`, and pass its two fields.
396364
// Any two ABI-compatible unsized types have the same metadata type and
397365
// moreover the same metadata value leads to the same dynamic size and
@@ -402,13 +370,8 @@ impl<'ll, 'tcx> FnAbiLlvmExt<'ll, 'tcx> for FnAbi<'tcx, Ty<'tcx>> {
402370
llargument_tys.push(ptr_layout.scalar_pair_element_llvm_type(cx, 1, true));
403371
continue;
404372
}
405-
PassMode::Indirect { attrs: _, meta_attrs: None, on_stack: _ } => {
406-
assert!(arg.layout.is_sized());
407-
cx.type_ptr()
408-
}
373+
PassMode::Indirect { attrs: _, meta_attrs: None, on_stack: _ } => cx.type_ptr(),
409374
PassMode::Cast { cast, pad_i32 } => {
410-
// `Cast` means "transmute to `CastType`"; that only makes sense for sized types.
411-
assert!(arg.layout.is_sized());
412375
// add padding
413376
if *pad_i32 {
414377
llargument_tys.push(Reg::i32().llvm_type(cx));

‎compiler/rustc_target/src/abi/call/aarch64.rs

+8
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,10 @@ where
4040
Ty: TyAbiInterface<'a, C> + Copy,
4141
C: HasDataLayout,
4242
{
43+
if !ret.layout.is_sized() {
44+
// Not touching this...
45+
return;
46+
}
4347
if !ret.layout.is_aggregate() {
4448
if kind == AbiKind::DarwinPCS {
4549
// On Darwin, when returning an i8/i16, it must be sign-extended to 32 bits,
@@ -67,6 +71,10 @@ where
6771
Ty: TyAbiInterface<'a, C> + Copy,
6872
C: HasDataLayout,
6973
{
74+
if !arg.layout.is_sized() {
75+
// Not touching this...
76+
return;
77+
}
7078
if !arg.layout.is_aggregate() {
7179
if kind == AbiKind::DarwinPCS {
7280
// On Darwin, when passing an i8/i16, it must be sign-extended to 32 bits,

‎compiler/rustc_target/src/abi/call/arm.rs

+8
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,10 @@ where
3030
Ty: TyAbiInterface<'a, C> + Copy,
3131
C: HasDataLayout,
3232
{
33+
if !ret.layout.is_sized() {
34+
// Not touching this...
35+
return;
36+
}
3337
if !ret.layout.is_aggregate() {
3438
ret.extend_integer_width_to(32);
3539
return;
@@ -56,6 +60,10 @@ where
5660
Ty: TyAbiInterface<'a, C> + Copy,
5761
C: HasDataLayout,
5862
{
63+
if !arg.layout.is_sized() {
64+
// Not touching this...
65+
return;
66+
}
5967
if !arg.layout.is_aggregate() {
6068
arg.extend_integer_width_to(32);
6169
return;

‎compiler/rustc_target/src/abi/call/csky.rs

+8
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@
77
use crate::abi::call::{ArgAbi, FnAbi, Reg, Uniform};
88

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

2630
fn classify_arg<Ty>(arg: &mut ArgAbi<'_, Ty>) {
31+
if !arg.layout.is_sized() {
32+
// Not touching this...
33+
return;
34+
}
2735
// For argument type, the first 4*XLen parts of aggregate will be passed
2836
// in registers, and the rest will be passed in stack.
2937
// So we can coerce to integers directly and let backend handle it correctly.

‎compiler/rustc_target/src/abi/call/loongarch.rs

+8
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,10 @@ fn classify_ret<'a, Ty, C>(cx: &C, arg: &mut ArgAbi<'a, Ty>, xlen: u64, flen: u6
152152
where
153153
Ty: TyAbiInterface<'a, C> + Copy,
154154
{
155+
if !arg.layout.is_sized() {
156+
// Not touching this...
157+
return false; // I guess? return value of this function is not documented
158+
}
155159
if let Some(conv) = should_use_fp_conv(cx, &arg.layout, xlen, flen) {
156160
match conv {
157161
FloatConv::Float(f) => {
@@ -214,6 +218,10 @@ fn classify_arg<'a, Ty, C>(
214218
) where
215219
Ty: TyAbiInterface<'a, C> + Copy,
216220
{
221+
if !arg.layout.is_sized() {
222+
// Not touching this...
223+
return;
224+
}
217225
if !is_vararg {
218226
match should_use_fp_conv(cx, &arg.layout, xlen, flen) {
219227
Some(FloatConv::Float(f)) if *avail_fprs >= 1 => {

‎compiler/rustc_target/src/abi/call/m68k.rs

+4
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,10 @@ fn classify_ret<Ty>(ret: &mut ArgAbi<'_, Ty>) {
99
}
1010

1111
fn classify_arg<Ty>(arg: &mut ArgAbi<'_, Ty>) {
12+
if !arg.layout.is_sized() {
13+
// Not touching this...
14+
return;
15+
}
1216
if arg.layout.is_aggregate() {
1317
arg.make_indirect_byval(None);
1418
} else {

‎compiler/rustc_target/src/abi/call/mips.rs

+4
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,10 @@ fn classify_arg<Ty, C>(cx: &C, arg: &mut ArgAbi<'_, Ty>, offset: &mut Size)
1717
where
1818
C: HasDataLayout,
1919
{
20+
if !arg.layout.is_sized() {
21+
// Not touching this...
22+
return;
23+
}
2024
let dl = cx.data_layout();
2125
let size = arg.layout.size;
2226
let align = arg.layout.align.max(dl.i32_align).min(dl.i64_align).abi;

‎compiler/rustc_target/src/abi/call/mod.rs

+23-7
Original file line numberDiff line numberDiff line change
@@ -422,7 +422,7 @@ impl<'a, Ty> TyAndLayout<'a, Ty> {
422422
}))
423423
}
424424

425-
Abi::ScalarPair(..) | Abi::Aggregate { .. } => {
425+
Abi::ScalarPair(..) | Abi::Aggregate { sized: true } => {
426426
// Helper for computing `homogeneous_aggregate`, allowing a custom
427427
// starting offset (used below for handling variants).
428428
let from_fields_at =
@@ -520,6 +520,7 @@ impl<'a, Ty> TyAndLayout<'a, Ty> {
520520
Ok(result)
521521
}
522522
}
523+
Abi::Aggregate { sized: false } => Err(Heterogeneous),
523524
}
524525
}
525526
}
@@ -555,8 +556,7 @@ impl<'a, Ty> ArgAbi<'a, Ty> {
555556
scalar_attrs(&layout, b, a.size(cx).align_to(b.align(cx).abi)),
556557
),
557558
Abi::Vector { .. } => PassMode::Direct(ArgAttributes::new()),
558-
// The `Aggregate` ABI should always be adjusted later.
559-
Abi::Aggregate { .. } => PassMode::Direct(ArgAttributes::new()),
559+
Abi::Aggregate { .. } => Self::indirect_pass_mode(&layout),
560560
};
561561
ArgAbi { layout, mode }
562562
}
@@ -580,14 +580,30 @@ impl<'a, Ty> ArgAbi<'a, Ty> {
580580
PassMode::Indirect { attrs, meta_attrs, on_stack: false }
581581
}
582582

583+
/// Pass this argument directly instead. Should NOT be used!
584+
/// Only exists because of past ABI mistakes that will take time to fix
585+
/// (see <https://github.com/rust-lang/rust/issues/115666>).
586+
pub fn make_direct_deprecated(&mut self) {
587+
match self.mode {
588+
PassMode::Indirect { .. } => {
589+
self.mode = PassMode::Direct(ArgAttributes::new());
590+
}
591+
PassMode::Ignore | PassMode::Direct(_) | PassMode::Pair(_, _) => return, // already direct
592+
_ => panic!("Tried to make {:?} direct", self.mode),
593+
}
594+
}
595+
583596
pub fn make_indirect(&mut self) {
584597
match self.mode {
585-
PassMode::Direct(_) | PassMode::Pair(_, _) => {}
586-
PassMode::Indirect { attrs: _, meta_attrs: None, on_stack: false } => return,
598+
PassMode::Direct(_) | PassMode::Pair(_, _) => {
599+
self.mode = Self::indirect_pass_mode(&self.layout);
600+
}
601+
PassMode::Indirect { attrs: _, meta_attrs: _, on_stack: false } => {
602+
// already indirect
603+
return;
604+
}
587605
_ => panic!("Tried to make {:?} indirect", self.mode),
588606
}
589-
590-
self.mode = Self::indirect_pass_mode(&self.layout);
591607
}
592608

593609
pub fn make_indirect_byval(&mut self, byval_align: Option<Align>) {

‎compiler/rustc_target/src/abi/call/nvptx64.rs

+9
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,18 @@ use crate::abi::{HasDataLayout, TyAbiInterface};
44
fn classify_ret<Ty>(ret: &mut ArgAbi<'_, Ty>) {
55
if ret.layout.is_aggregate() && ret.layout.size.bits() > 64 {
66
ret.make_indirect();
7+
} else {
8+
// FIXME: this is wrong! Need to decide which ABI we really want here.
9+
ret.make_direct_deprecated();
710
}
811
}
912

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

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

‎compiler/rustc_target/src/abi/call/powerpc64.rs

+8
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,10 @@ where
4646
Ty: TyAbiInterface<'a, C> + Copy,
4747
C: HasDataLayout,
4848
{
49+
if !ret.layout.is_sized() {
50+
// Not touching this...
51+
return;
52+
}
4953
if !ret.layout.is_aggregate() {
5054
ret.extend_integer_width_to(64);
5155
return;
@@ -89,6 +93,10 @@ where
8993
Ty: TyAbiInterface<'a, C> + Copy,
9094
C: HasDataLayout,
9195
{
96+
if !arg.layout.is_sized() {
97+
// Not touching this...
98+
return;
99+
}
92100
if !arg.layout.is_aggregate() {
93101
arg.extend_integer_width_to(64);
94102
return;

‎compiler/rustc_target/src/abi/call/riscv.rs

+8
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,10 @@ fn classify_ret<'a, Ty, C>(cx: &C, arg: &mut ArgAbi<'a, Ty>, xlen: u64, flen: u6
158158
where
159159
Ty: TyAbiInterface<'a, C> + Copy,
160160
{
161+
if !arg.layout.is_sized() {
162+
// Not touching this...
163+
return false; // I guess? return value of this function is not documented
164+
}
161165
if let Some(conv) = should_use_fp_conv(cx, &arg.layout, xlen, flen) {
162166
match conv {
163167
FloatConv::Float(f) => {
@@ -220,6 +224,10 @@ fn classify_arg<'a, Ty, C>(
220224
) where
221225
Ty: TyAbiInterface<'a, C> + Copy,
222226
{
227+
if !arg.layout.is_sized() {
228+
// Not touching this...
229+
return;
230+
}
223231
if !is_vararg {
224232
match should_use_fp_conv(cx, &arg.layout, xlen, flen) {
225233
Some(FloatConv::Float(f)) if *avail_fprs >= 1 => {

‎compiler/rustc_target/src/abi/call/s390x.rs

+4
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,10 @@ where
1717
Ty: TyAbiInterface<'a, C> + Copy,
1818
C: HasDataLayout,
1919
{
20+
if !arg.layout.is_sized() {
21+
// Not touching this...
22+
return;
23+
}
2024
if !arg.layout.is_aggregate() && arg.layout.size.bits() <= 64 {
2125
arg.extend_integer_width_to(64);
2226
return;

‎compiler/rustc_target/src/abi/call/sparc.rs

+4
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,10 @@ fn classify_arg<Ty, C>(cx: &C, arg: &mut ArgAbi<'_, Ty>, offset: &mut Size)
1717
where
1818
C: HasDataLayout,
1919
{
20+
if !arg.layout.is_sized() {
21+
// Not touching this...
22+
return;
23+
}
2024
let dl = cx.data_layout();
2125
let size = arg.layout.size;
2226
let align = arg.layout.align.max(dl.i32_align).min(dl.i64_align).abi;

‎compiler/rustc_target/src/abi/call/wasm.rs

+20-4
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,10 @@ where
3434
Ty: TyAbiInterface<'a, C> + Copy,
3535
C: HasDataLayout,
3636
{
37+
if !arg.layout.is_sized() {
38+
// Not touching this...
39+
return;
40+
}
3741
arg.extend_integer_width_to(32);
3842
if arg.layout.is_aggregate() && !unwrap_trivial_aggregate(cx, arg) {
3943
arg.make_indirect_byval(None);
@@ -67,21 +71,33 @@ where
6771
/// Also see <https://github.com/rust-lang/rust/issues/115666>.
6872
pub fn compute_wasm_abi_info<Ty>(fn_abi: &mut FnAbi<'_, Ty>) {
6973
if !fn_abi.ret.is_ignore() {
70-
classify_ret(&mut fn_abi.ret);
74+
classify_ret_wasm_abi(&mut fn_abi.ret);
7175
}
7276

7377
for arg in fn_abi.args.iter_mut() {
7478
if arg.is_ignore() {
7579
continue;
7680
}
77-
classify_arg(arg);
81+
classify_arg_wasm_abi(arg);
7882
}
7983

80-
fn classify_ret<Ty>(ret: &mut ArgAbi<'_, Ty>) {
84+
fn classify_ret_wasm_abi<Ty>(ret: &mut ArgAbi<'_, Ty>) {
85+
if !ret.layout.is_sized() {
86+
// Not touching this...
87+
return;
88+
}
89+
// FIXME: this is bad! https://github.com/rust-lang/rust/issues/115666
90+
ret.make_direct_deprecated();
8191
ret.extend_integer_width_to(32);
8292
}
8393

84-
fn classify_arg<Ty>(arg: &mut ArgAbi<'_, Ty>) {
94+
fn classify_arg_wasm_abi<Ty>(arg: &mut ArgAbi<'_, Ty>) {
95+
if !arg.layout.is_sized() {
96+
// Not touching this...
97+
return;
98+
}
99+
// FIXME: this is bad! https://github.com/rust-lang/rust/issues/115666
100+
arg.make_direct_deprecated();
85101
arg.extend_integer_width_to(32);
86102
}
87103
}

0 commit comments

Comments
 (0)