default Aggregate ABI to Indirect, and make sure it's never used for unsized

RalfJung · RalfJung · commit eaaa03faf77b · 2023-11-03T07:14:27.000+01:00
diff --git a/compiler/rustc_target/src/abi/call/aarch64.rs b/compiler/rustc_target/src/abi/call/aarch64.rs
@@ -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,
@@ -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,
diff --git a/compiler/rustc_target/src/abi/call/arm.rs b/compiler/rustc_target/src/abi/call/arm.rs
@@ -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;
@@ -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;
diff --git a/compiler/rustc_target/src/abi/call/loongarch.rs b/compiler/rustc_target/src/abi/call/loongarch.rs
@@ -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) => {
@@ -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 => {
diff --git a/compiler/rustc_target/src/abi/call/mips.rs b/compiler/rustc_target/src/abi/call/mips.rs
@@ -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;
diff --git a/compiler/rustc_target/src/abi/call/mod.rs b/compiler/rustc_target/src/abi/call/mod.rs
@@ -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 =
@@ -520,6 +520,7 @@ impl<'a, Ty> TyAndLayout<'a, Ty> {
                     Ok(result)
                 }
             }
+            Abi::Aggregate { sized: false } => Err(Heterogeneous),
         }
     }
 }
@@ -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 }
     }
@@ -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),
         }
 
diff --git a/compiler/rustc_target/src/abi/call/nvptx64.rs b/compiler/rustc_target/src/abi/call/nvptx64.rs
@@ -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();
     }
 }
 
@@ -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();
     }
 }
 
diff --git a/compiler/rustc_target/src/abi/call/powerpc64.rs b/compiler/rustc_target/src/abi/call/powerpc64.rs
@@ -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;
@@ -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;
diff --git a/compiler/rustc_target/src/abi/call/riscv.rs b/compiler/rustc_target/src/abi/call/riscv.rs
@@ -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) => {
@@ -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 => {
diff --git a/compiler/rustc_target/src/abi/call/s390x.rs b/compiler/rustc_target/src/abi/call/s390x.rs
@@ -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;
diff --git a/compiler/rustc_target/src/abi/call/sparc.rs b/compiler/rustc_target/src/abi/call/sparc.rs
@@ -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;
diff --git a/compiler/rustc_target/src/abi/call/wasm.rs b/compiler/rustc_target/src/abi/call/wasm.rs
@@ -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);
@@ -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);
     }
 }
diff --git a/compiler/rustc_target/src/abi/call/x86.rs b/compiler/rustc_target/src/abi/call/x86.rs
@@ -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.
@@ -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;
         }
 
diff --git a/compiler/rustc_target/src/abi/call/x86_64.rs b/compiler/rustc_target/src/abi/call/x86_64.rs
@@ -153,9 +153,9 @@ fn reg_component(cls: &[Option<Class>], i: &mut usize, size: Size) -> Option<Reg
     }
 }
 
-fn cast_target(cls: &[Option<Class>], size: Size) -> Option<CastTarget> {
+fn cast_target(cls: &[Option<Class>], size: Size) -> CastTarget {
     let mut i = 0;
-    let lo = reg_component(cls, &mut i, size)?;
+    let lo = reg_component(cls, &mut i, size).unwrap();
     let offset = Size::from_bytes(8) * (i as u64);
     let mut target = CastTarget::from(lo);
     if size > offset {
@@ -164,7 +164,7 @@ fn cast_target(cls: &[Option<Class>], size: Size) -> Option<CastTarget> {
         }
     }
     assert_eq!(reg_component(cls, &mut i, Size::ZERO), None);
-    Some(target)
+    target
 }
 
 const MAX_INT_REGS: usize = 6; // RDI, RSI, RDX, RCX, R8, R9
@@ -179,6 +179,10 @@ where
     let mut sse_regs = MAX_SSE_REGS;
 
     let mut x86_64_arg_or_ret = |arg: &mut ArgAbi<'a, Ty>, is_arg: bool| {
+        if !arg.layout.is_sized() {
+            // Not touching this...
+            return;
+        }
         let mut cls_or_mem = classify_arg(cx, arg);
 
         if is_arg {
@@ -227,9 +231,7 @@ where
                 // split into sized chunks passed individually
                 if arg.layout.is_aggregate() {
                     let size = arg.layout.size;
-                    if let Some(cast_target) = cast_target(cls, size) {
-                        arg.cast_to(cast_target);
-                    }
+                    arg.cast_to(cast_target(cls, size));
                 } else {
                     arg.extend_integer_width_to(32);
                 }
diff --git a/compiler/rustc_target/src/abi/call/x86_win64.rs b/compiler/rustc_target/src/abi/call/x86_win64.rs
@@ -6,8 +6,8 @@ use crate::abi::Abi;
 pub fn compute_abi_info<Ty>(fn_abi: &mut FnAbi<'_, Ty>) {
     let fixup = |a: &mut ArgAbi<'_, Ty>| {
         match a.layout.abi {
-            Abi::Uninhabited => {}
-            Abi::ScalarPair(..) | Abi::Aggregate { .. } => match a.layout.size.bits() {
+            Abi::Uninhabited | Abi::Aggregate { sized: false } => {}
+            Abi::ScalarPair(..) | Abi::Aggregate { sized: true } => match a.layout.size.bits() {
                 8 => a.cast_to(Reg::i8()),
                 16 => a.cast_to(Reg::i16()),
                 32 => a.cast_to(Reg::i32()),
diff --git a/compiler/rustc_ty_utils/src/abi.rs b/compiler/rustc_ty_utils/src/abi.rs
@@ -591,13 +591,14 @@ fn fn_abi_adjust_for_abi<'tcx>(
 
                 _ => return,
             }
-            // `Aggregate` ABI must be adjusted to ensure that ABI-compatible Rust types are passed
-            // the same way.
+            // Compute `Aggregate` ABI.
+
+            let is_indirect_not_on_stack =
+                matches!(arg.mode, PassMode::Indirect { on_stack: false, .. });
+            assert!(is_indirect_not_on_stack, "{:?}", arg);
 
             let size = arg.layout.size;
-            if arg.layout.is_unsized() || size > Pointer(AddressSpace::DATA).size(cx) {
-                arg.make_indirect();
-            } else {
+            if !arg.layout.is_unsized() && size <= Pointer(AddressSpace::DATA).size(cx) {
                 // We want to pass small aggregates as immediates, but using
                 // an LLVM aggregate type for this leads to bad optimizations,
                 // so we pick an appropriately sized integer type instead.
diff --git a/tests/ui/abi/compatibility.rs b/tests/ui/abi/compatibility.rs
@@ -1,5 +1,14 @@
 // check-pass
 // revisions: host
+// revisions: i686
+//[i686] compile-flags: --target i686-unknown-linux-gnu
+//[i686] needs-llvm-components: x86
+// revisions: x86-64
+//[x86-64] compile-flags: --target x86_64-unknown-linux-gnu
+//[x86-64] needs-llvm-components: x86
+// revisions: x86-64-win
+//[x86-64-win] compile-flags: --target x86_64-pc-windows-msvc
+//[x86-64-win] needs-llvm-components: x86
 // revisions: arm
 //[arm] compile-flags: --target arm-unknown-linux-gnueabi
 //[arm] needs-llvm-components: arm
@@ -38,6 +47,7 @@
 //[wasi] compile-flags: --target wasm32-wasi
 //[wasi] needs-llvm-components: webassembly
 // FIXME: disabled on nvptx64 since the target ABI fails the sanity check
+// see https://github.com/rust-lang/rust/issues/117480
 /* revisions: nvptx64
   [nvptx64] compile-flags: --target nvptx64-nvidia-cuda
   [nvptx64] needs-llvm-components: nvptx
@@ -326,6 +336,7 @@ mod unsized_ {
     use super::*;
     test_transparent_unsized!(str_, str);
     test_transparent_unsized!(slice, [u8]);
+    test_transparent_unsized!(slice_with_prefix, (usize, [u8]));
     test_transparent_unsized!(dyn_trait, dyn Any);
 }
 

Original file line number	Diff line number	Diff line change
`@@ -422,7 +422,7 @@ impl<'a, Ty> TyAndLayout<'a, Ty> {`
`422`	`422`	`}))`
`423`	`423`	`}`
`424`	`424`
`425`		`- Abi::ScalarPair(..) \| Abi::Aggregate { .. } => {`
	`425`	`+ Abi::ScalarPair(..) \| Abi::Aggregate { sized: true } => {`
`426`	`426`	// Helper for computing `homogeneous_aggregate`, allowing a custom
`427`	`427`	`// starting offset (used below for handling variants).`
`428`	`428`	`let from_fields_at =`
`@@ -520,6 +520,7 @@ impl<'a, Ty> TyAndLayout<'a, Ty> {`
`520`	`520`	`Ok(result)`
`521`	`521`	`}`
`522`	`522`	`}`
	`523`	`+ Abi::Aggregate { sized: false } => Err(Heterogeneous),`
`523`	`524`	`}`
`524`	`525`	`}`
`525`	`526`	`}`
`@@ -555,8 +556,7 @@ impl<'a, Ty> ArgAbi<'a, Ty> {`
`555`	`556`	`scalar_attrs(&layout, b, a.size(cx).align_to(b.align(cx).abi)),`
`556`	`557`	`),`
`557`	`558`	`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),`
`560`	`560`	`};`
`561`	`561`	`ArgAbi { layout, mode }`
`562`	`562`	`}`
`@@ -580,10 +580,20 @@ impl<'a, Ty> ArgAbi<'a, Ty> {`
`580`	`580`	`PassMode::Indirect { attrs, meta_attrs, on_stack: false }`
`581`	`581`	`}`
`582`	`582`
	`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`	`+ self.mode = PassMode::Direct(ArgAttributes::new());`
	`588`	`+ }`
	`589`	`+`
`583`	`590`	`pub fn make_indirect(&mut self) {`
`584`	`591`	`match self.mode {`
`585`	`592`	`PassMode::Direct(_) \| PassMode::Pair(_, _) => {}`
`586`		`- PassMode::Indirect { attrs: _, meta_attrs: None, on_stack: false } => return,`
	`593`	`+ PassMode::Indirect { attrs: _, meta_attrs: _, on_stack: false } => {`
	`594`	`+ // already indirect`
	`595`	`+ return;`
	`596`	`+ }`
`587`	`597`	`_ => panic!("Tried to make {:?} indirect", self.mode),`
`588`	`598`	`}`
`589`	`599`
Original file line number	Diff line number	Diff line change
`@@ -4,12 +4,18 @@ use crate::abi::{HasDataLayout, TyAbiInterface};`
`4`	`4`	`fn classify_ret<Ty>(ret: &mut ArgAbi<'_, Ty>) {`
`5`	`5`	`if ret.layout.is_aggregate() && ret.layout.size.bits() > 64 {`
`6`	`6`	`ret.make_indirect();`
	`7`	`+ } else {`
	`8`	`+ // FIXME: this is wrong! Need to decide which ABI we really want here.`
	`9`	`+ ret.make_direct_deprecated();`
`7`	`10`	`}`
`8`	`11`	`}`
`9`	`12`
`10`	`13`	`fn classify_arg<Ty>(arg: &mut ArgAbi<'_, Ty>) {`
`11`	`14`	`if arg.layout.is_aggregate() && arg.layout.size.bits() > 64 {`
`12`	`15`	`arg.make_indirect();`
	`16`	`+ } else {`
	`17`	`+ // FIXME: this is wrong! Need to decide which ABI we really want here.`
	`18`	`+ arg.make_direct_deprecated();`
`13`	`19`	`}`
`14`	`20`	`}`
`15`	`21`
`@@ -30,6 +36,9 @@ where`
`30`	`36`	`_ => unreachable!("Align is given as power of 2 no larger than 16 bytes"),`
`31`	`37`	`};`
`32`	`38`	`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();`
`33`	`42`	`}`
`34`	`43`	`}`
`35`	`44`