Distinguish repr(C) ZSTs from others in ABI computation

Co-Authored-By: Ralf Jung <post@ralfj.de>

Author: Jules-Bertholet

compiler/rustc_abi/src/layout.rs

@@ -197,6 +197,7 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
             size,
             max_repr_align: None,
             unadjusted_abi_align: element.align.abi,
+            repr_c: false,
             randomization_seed: element.randomization_seed.wrapping_add(Hash64::new(count)),
         })
     }
@@ -498,6 +499,10 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
             return Err(LayoutCalculatorError::EmptyUnion);
         };
 
+        // We also need to check for a `repr(transparent)` wrapper around `repr(C)`
+        let repr_c =
+            repr.c() || (repr.transparent() && only_variant.iter().any(|field| field.repr_c));
+
         let combined_seed = only_variant
             .iter()
             .map(|v| v.randomization_seed)
@@ -513,6 +518,7 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
             size: size.align_to(align),
             max_repr_align,
             unadjusted_abi_align,
+            repr_c,
             randomization_seed: combined_seed,
         })
     }
@@ -736,6 +742,7 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
                 align: AbiAlign::new(align),
                 max_repr_align,
                 unadjusted_abi_align,
+                repr_c: repr.c(),
                 randomization_seed: combined_seed,
             };
 
@@ -1067,6 +1074,7 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
             size,
             max_repr_align,
             unadjusted_abi_align,
+            repr_c: repr.c(),
             randomization_seed: combined_seed,
         };
 
@@ -1416,6 +1424,9 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
             unadjusted_abi_align
         };
 
+        // We also need to check for a `repr(transparent)` wrapper around `repr(C)`
+        let repr_c = repr.c() || (repr.transparent() && fields.iter().any(|field| field.repr_c));
+
         let seed = field_seed.wrapping_add(repr.field_shuffle_seed);
 
         Ok(LayoutData {
@@ -1428,6 +1439,7 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
             size,
             max_repr_align,
             unadjusted_abi_align,
+            repr_c,
             randomization_seed: seed,
         })
     }
@@ -1530,6 +1542,7 @@ where
         align: AbiAlign::new(align),
         max_repr_align: None,
         unadjusted_abi_align: elt.align.abi,
+        repr_c: false,
         randomization_seed: elt.randomization_seed.wrapping_add(Hash64::new(count)),
     })
 }

compiler/rustc_abi/src/layout/coroutine.rs

@@ -310,6 +310,7 @@ pub(super) fn layout<
         align,
         max_repr_align: None,
         unadjusted_abi_align: align.abi,
+        repr_c: false,
         randomization_seed: Default::default(),
     })
 }

compiler/rustc_abi/src/layout/simple.rs

@@ -25,6 +25,7 @@ impl<FieldIdx: Idx, VariantIdx: Idx> LayoutData<FieldIdx, VariantIdx> {
             size: Size::ZERO,
             max_repr_align: None,
             unadjusted_abi_align: dl.i8_align,
+            repr_c: false,
             randomization_seed: Hash64::new(0),
         }
     }
@@ -42,6 +43,7 @@ impl<FieldIdx: Idx, VariantIdx: Idx> LayoutData<FieldIdx, VariantIdx> {
             size: Size::ZERO,
             max_repr_align: None,
             unadjusted_abi_align: dl.i8_align,
+            repr_c: false,
             randomization_seed: Hash64::ZERO,
         }
     }
@@ -84,6 +86,7 @@ impl<FieldIdx: Idx, VariantIdx: Idx> LayoutData<FieldIdx, VariantIdx> {
             align,
             max_repr_align: None,
             unadjusted_abi_align: align.abi,
+            repr_c: false,
             randomization_seed: Hash64::new(randomization_seed),
         }
     }
@@ -117,6 +120,7 @@ impl<FieldIdx: Idx, VariantIdx: Idx> LayoutData<FieldIdx, VariantIdx> {
             size,
             max_repr_align: None,
             unadjusted_abi_align: align,
+            repr_c: false,
             randomization_seed: Hash64::new(combined_seed),
         }
     }
@@ -143,6 +147,7 @@ impl<FieldIdx: Idx, VariantIdx: Idx> LayoutData<FieldIdx, VariantIdx> {
             size: Size::ZERO,
             max_repr_align: None,
             unadjusted_abi_align: dl.i8_align,
+            repr_c: false,
             // Variant layouts never flow back into actual layout computations,
             // so dummy values are fine here.
             randomization_seed: Hash64::ZERO,
@@ -169,6 +174,7 @@ impl<FieldIdx: Idx, VariantIdx: Idx> LayoutData<FieldIdx, VariantIdx> {
             align: parent.align,
             max_repr_align: parent.max_repr_align,
             unadjusted_abi_align: parent.unadjusted_abi_align,
+            repr_c: parent.repr_c,
             // Variant layouts never flow back into actual layout computations,
             // so dummy values are fine here.
             randomization_seed: Hash64::ZERO,

compiler/rustc_abi/src/lib.rs

@@ -2143,6 +2143,11 @@ pub struct LayoutData<FieldIdx: Idx, VariantIdx: Idx> {
     /// in some cases.
     pub unadjusted_abi_align: Align,
 
+    /// Whether this type is `repr(C)`, or a`repr(transparent)` wrapper around such.
+    /// Some C ABIs pass `repr(C)` ZSTs by pointer, but `repr(Rust)` ZSTs should always
+    /// be ignored.
+    pub repr_c: bool,
+
     /// The randomization seed based on this type's own repr and its fields.
     ///
     /// Since randomization is toggled on a per-crate basis even crates that do not have randomization
@@ -2171,6 +2176,12 @@ impl<FieldIdx: Idx, VariantIdx: Idx> LayoutData<FieldIdx, VariantIdx> {
     pub fn is_uninhabited(&self) -> bool {
         self.uninhabited
     }
+
+    /// Returns `true` if this is a `repr(C)` type,
+    /// or an array of such, or a `repr(transparent)` wrapper around such
+    pub fn is_repr_c(&self) -> bool {
+        self.repr_c
+    }
 }
 
 impl<FieldIdx: Idx, VariantIdx: Idx> fmt::Debug for LayoutData<FieldIdx, VariantIdx>
@@ -2192,6 +2203,7 @@ where
             variants,
             max_repr_align,
             unadjusted_abi_align,
+            repr_c,
             randomization_seed,
         } = self;
         f.debug_struct("Layout")
@@ -2204,6 +2216,7 @@ where
             .field("variants", variants)
             .field("max_repr_align", max_repr_align)
             .field("unadjusted_abi_align", unadjusted_abi_align)
+            .field("repr_c", repr_c)
             .field("randomization_seed", randomization_seed)
             .finish()
     }

compiler/rustc_target/src/callconv/powerpc.rs

@@ -19,7 +19,7 @@ where
         // powerpc-unknown-linux-{gnu,musl,uclibc} doesn't ignore ZSTs.
         if cx.target_spec().os == Os::Linux
             && matches!(cx.target_spec().env, Env::Gnu | Env::Musl | Env::Uclibc)
-            && arg.layout.is_zst()
+            && arg.layout.is_repr_c()
         {
             arg.make_indirect_from_ignore();
         }

compiler/rustc_target/src/callconv/s390x.rs

@@ -31,7 +31,7 @@ where
         // s390x-unknown-linux-{gnu,musl,uclibc} doesn't ignore ZSTs.
         if cx.target_spec().os == Os::Linux
             && matches!(cx.target_spec().env, Env::Gnu | Env::Musl | Env::Uclibc)
-            && arg.layout.is_zst()
+            && arg.layout.is_repr_c()
         {
             arg.make_indirect_from_ignore();
         }

compiler/rustc_target/src/callconv/sparc64.rs

@@ -212,7 +212,7 @@ where
             continue;
         }
         if arg.is_ignore() {
-            if passes_zsts && arg.layout.is_zst() {
+            if passes_zsts && arg.layout.is_repr_c() {
                 arg.make_indirect_from_ignore();
                 double_word_count += 1;
             }

compiler/rustc_target/src/callconv/x86_win64.rs

@@ -48,18 +48,25 @@ where
         }
     };
 
-    if !fn_abi.ret.is_ignore() {
+    // Windows ABIs do not talk about ZST since such types do not exist in MSVC.
+    // However, clang and gcc allow ZST in their windows-gnu targets, and pass them by pointer indirection.
+    // We follow that for `repr(C)` ZSTs (and `repr(transparent)` wrappers around them),
+    // but `repr(Rust)` ones are always ignored (ensuring that `()` matches C `void`).
+
+    if fn_abi.ret.is_ignore() {
+        if fn_abi.ret.layout.is_repr_c() {
+            fn_abi.ret.make_indirect_from_ignore();
+        }
+    } else {
         fixup(&mut fn_abi.ret, true);
     }
 
     for arg in fn_abi.args.iter_mut() {
-        if arg.is_ignore() && arg.layout.is_zst() {
-            // Windows ABIs do not talk about ZST since such types do not exist in MSVC.
-            // In that sense we can do whatever we want here, and maybe we should throw an error
-            // (but of course that would be a massive breaking change now).
-            // We try to match clang and gcc (which allow ZST is their windows-gnu targets), so we
-            // pass ZST via pointer indirection.
-            arg.make_indirect_from_ignore();
+        if arg.is_ignore() {
+            if arg.layout.is_repr_c() {
+                arg.make_indirect_from_ignore();
+            }
+
             continue;
         }
         if arg.layout.pass_indirectly_in_non_rustic_abis(cx) {
@@ -68,7 +75,4 @@ where
         }
         fixup(arg, false);
     }
-    // FIXME: We should likely also do something about ZST return types, similar to above.
-    // However, that's non-trivial due to `()`.
-    // See <https://github.com/rust-lang/unsafe-code-guidelines/issues/552>.
 }

library/core/src/primitive_docs.rs

@@ -1841,9 +1841,14 @@ mod prim_ref {}
 ///   call will be valid ABI-wise. The callee receives the result of transmuting the function pointer
 ///   from `fn()` to `fn(i32)`; that transmutation is itself a well-defined operation, it's just
 ///   almost certainly UB to later call that function pointer.)
-/// - Any two types with size 0 and alignment 1 are ABI-compatible.
-/// - A `repr(transparent)` type `T` is ABI-compatible with its unique non-trivial field, i.e., the
-///   unique field that doesn't have size 0 and alignment 1 (if there is such a field).
+/// - Any two types fulfilling all the following conditions are ABI-compatible;
+///   such types are said to have "trivial ABI":
+///   - Size 0
+///   - Alignment 1
+///   - Not `repr(C)`
+///   - Not a `repr(transparent)` wrapper around a type that fails to satisfy these conditions
+/// - A `repr(transparent)` type is ABI-compatible with its unique field that does not have trivial ABI
+///   (as defined above). If there is no such field, the type has trivial ABI.
 /// - `i32` is ABI-compatible with `NonZero<i32>`, and similar for all other integer types.
 /// - If `T` is guaranteed to be subject to the [null pointer
 ///   optimization](option/index.html#representation), and `E` is an enum satisfying the following

tests/codegen-llvm/abi-win64-zst.rs

@@ -21,33 +21,129 @@
 extern crate minicore;
 use minicore::*;
 
-// Make sure the argument is always passed when explicitly requesting a Windows ABI.
+#[repr(C)]
+struct CMaybeZst;
+
+#[repr(transparent)]
+struct CMaybeZst2((), CMaybeZst, ());
+
+// Make sure the argument is always passed when explicitly requesting a Windows ABI,
+// and it is `repr(C)` - but not if it is `repr(Rust)`.
 // Our goal here is to match clang: <https://clang.godbolt.org/z/Wr4jMWq3P>.
 
-// CHECK: define win64cc void @pass_zst_win64(ptr {{[^,]*}})
+// CHECK: define win64cc void @pass_rust_zst_win64()
+#[no_mangle]
+extern "win64" fn pass_rust_zst_win64(_: ()) {}
+
+// CHECK: define win64cc void @pass_c_maybezst_win64(ptr {{[^,]*}})
+#[no_mangle]
+extern "win64" fn pass_c_maybezst_win64(_: CMaybeZst) {}
+
+// CHECK: define win64cc void @pass_c_maybezst_2_win64(ptr {{[^,]*}})
+#[no_mangle]
+extern "win64" fn pass_c_maybezst_2_win64(_: CMaybeZst2) {}
+
+// CHECK: define x86_vectorcallcc void @pass_rust_zst_vectorcall()
+#[no_mangle]
+extern "vectorcall" fn pass_rust_zst_vectorcall(_: ()) {}
+
+// CHECK: define x86_vectorcallcc void @pass_c_maybezst_vectorcall(ptr {{[^,]*}})
 #[no_mangle]
-extern "win64" fn pass_zst_win64(_: ()) {}
+extern "vectorcall" fn pass_c_maybezst_vectorcall(_: CMaybeZst) {}
 
-// CHECK: define x86_vectorcallcc void @pass_zst_vectorcall(ptr {{[^,]*}})
+// CHECK: define x86_vectorcallcc void @pass_c_maybezst_2_vectorcall(ptr {{[^,]*}})
 #[no_mangle]
-extern "vectorcall" fn pass_zst_vectorcall(_: ()) {}
+extern "vectorcall" fn pass_c_maybezst_2_vectorcall(_: CMaybeZst2) {}
 
-// windows-gnu: define void @pass_zst_fastcall(ptr {{[^,]*}})
-// windows-msvc: define void @pass_zst_fastcall(ptr {{[^,]*}})
+// windows-gnu: define void @pass_rust_zst_fastcall()
+// windows-msvc: define void @pass_rust_zst_fastcall()
 #[no_mangle]
 #[cfg(windows)] // "fastcall" is not valid on 64bit Linux
-extern "fastcall" fn pass_zst_fastcall(_: ()) {}
+extern "fastcall" fn pass_rust_zst_fastcall(_: ()) {}
+
+// windows-gnu: define void @pass_c_maybezst_fastcall(ptr {{[^,]*}})
+// windows-msvc: define void @pass_c_maybezst_fastcall(ptr {{[^,]*}})
+#[no_mangle]
+#[cfg(windows)] // "fastcall" is not valid on 64bit Linux
+extern "fastcall" fn pass_c_maybezst_fastcall(_: CMaybeZst) {}
+
+// windows-gnu: define void @pass_c_maybezst_2_fastcall(ptr {{[^,]*}})
+// windows-msvc: define void @pass_c_maybezst_2_fastcall(ptr {{[^,]*}})
+#[no_mangle]
+#[cfg(windows)] // "fastcall" is not valid on 64bit Linux
+extern "fastcall" fn pass_c_maybezst_2_fastcall(_: CMaybeZst2) {}
 
 // The sysv64 ABI ignores ZST.
 
-// CHECK: define x86_64_sysvcc void @pass_zst_sysv64()
+// CHECK: define x86_64_sysvcc void @pass_rust_zst_sysv64()
+#[no_mangle]
+extern "sysv64" fn pass_rust_zst_sysv64(_: ()) {}
+
+// CHECK: define x86_64_sysvcc void @pass_c_maybezst_sysv64()
+#[no_mangle]
+extern "sysv64" fn pass_c_maybezst_sysv64(_: CMaybeZst) {}
+
+// CHECK: define x86_64_sysvcc void @pass_c_maybezst_2_sysv64()
 #[no_mangle]
-extern "sysv64" fn pass_zst_sysv64(_: ()) {}
+extern "sysv64" fn pass_c_maybezst_2_sysv64(_: CMaybeZst2) {}
 
 // For `extern "C"` functions, ZST are ignored on Linux put passed on Windows.
 
-// linux: define void @pass_zst_c()
-// windows-msvc: define void @pass_zst_c(ptr {{[^,]*}})
-// windows-gnu: define void @pass_zst_c(ptr {{[^,]*}})
+// linux: define void @pass_rust_zst_c()
+// windows-msvc: define void @pass_rust_zst_c()
+// windows-gnu: define void @pass_rust_zst_c()
+#[no_mangle]
+extern "C" fn pass_rust_zst_c(_: ()) {}
+
+// linux: define void @pass_c_maybezst_c()
+// windows-msvc: define void @pass_c_maybezst_c(ptr {{[^,]*}})
+// windows-gnu: define void @pass_c_maybezst_c(ptr {{[^,]*}})
+#[no_mangle]
+extern "C" fn pass_c_maybezst_c(_: CMaybeZst) {}
+
+// linux: define void @pass_c_maybezst_2_c()
+// windows-msvc: define void @pass_c_maybezst_2_c(ptr {{[^,]*}})
+// windows-gnu: define void @pass_c_maybezst_2_c(ptr {{[^,]*}})
+#[no_mangle]
+extern "C" fn pass_c_maybezst_2_c(_: CMaybeZst2) {}
+
+// Now check `repr(C)` return types.
+// Again, we seek to match clang: <https://clang.godbolt.org/z/hKv74ThnE>
+
+// CHECK: define win64cc void @ret_c_maybezst_win64(ptr {{[^,]*}})
+#[no_mangle]
+extern "win64" fn ret_c_maybezst_win64() -> CMaybeZst {
+    CMaybeZst
+}
+
+// CHECK: define x86_vectorcallcc void @ret_c_maybezst_vectorcall(ptr {{[^,]*}})
+#[no_mangle]
+extern "vectorcall" fn ret_c_maybezst_vectorcall() -> CMaybeZst {
+    CMaybeZst
+}
+
+// windows-gnu: define void @ret_c_maybezst_fastcall(ptr {{[^,]*}})
+// windows-msvc: define void @ret_c_maybezst_fastcall(ptr {{[^,]*}})
+#[no_mangle]
+#[cfg(windows)] // "fastcall" is not valid on 64bit Linux
+extern "fastcall" fn ret_c_maybezst_fastcall() -> CMaybeZst {
+    CMaybeZst
+}
+
+// The sysv64 ABI ignores ZST.
+
+// CHECK: define x86_64_sysvcc void @ret_c_maybezst_sysv64()
+#[no_mangle]
+extern "sysv64" fn ret_c_maybezst_sysv64() -> CMaybeZst {
+    CMaybeZst
+}
+
+// For `extern "C"` functions, ZST are ignored on Linux but returned via pointer on Windows.
+
+// linux: define void @ret_c_maybezst_c()
+// windows-msvc: define void @ret_c_maybezst_c(ptr {{[^,]*}})
+// windows-gnu: define void @ret_c_maybezst_c(ptr {{[^,]*}})
 #[no_mangle]
-extern "C" fn pass_zst_c(_: ()) {}
+extern "C" fn ret_c_maybezst_c() -> CMaybeZst {
+    CMaybeZst
+}