[derive] Fix handling of repr(packed)

While we're here, improve error messages and add tests for deriving
`IntoBytes` on unions.

Closes #1763

zerocopy-derive/src/lib.rs

@@ -753,22 +753,21 @@ fn derive_into_bytes_struct(ast: &DeriveInput, strct: &DataStruct) -> Result<Tok
  let is_packed_1 = repr.is_packed_1();
  let num_fields = strct.fields().len();
 
- let (padding_check, require_unaligned_fields) = if is_transparent || is_packed_1 {
+ let (padding_check, require_unaligned_fields) = if is_transparent || (is_c && is_packed_1) {
  // No padding check needed.
  // - repr(transparent): The layout and ABI of the whole struct is the
  // same as its only non-ZST field (meaning there's no padding outside
  // of that field) and we require that field to be `IntoBytes` (meaning
  // there's no padding in that field).
- // - repr(packed): Any inter-field padding bytes are removed, meaning
+ // - repr(C, packed): Any inter-field padding bytes are removed, meaning
  // that any padding bytes would need to come from the fields, all of
  // which we require to be `IntoBytes` (meaning they don't have any
  // padding).
  (None, false)
- } else if is_c && num_fields <= 1 {
+ } else if is_c && !repr.is_align_gt_1() && num_fields <= 1 {
  // No padding check needed. A repr(C) struct with zero or one field has
- // no padding.
- //
- // TODO(#1763): This is probably unsound! Fix it.
+ // no padding unless #[repr(align)] explicitly adds padding, which we
+ // check for in this branch's condition.
  (None, false)
  } else if ast.generics.params.is_empty() {
  // Since there are no generics, we can emit a padding check. This is
@@ -777,16 +776,15 @@ fn derive_into_bytes_struct(ast: &DeriveInput, strct: &DataStruct) -> Result<Tok
  //
  // TODO(#1764): This is probably unsound! Fix it.
  (Some(PaddingCheck::Struct), false)
- } else if is_c {
+ } else if is_c && !repr.is_align_gt_1() {
  // We can't use a padding check since there are generic type arguments.
  // Instead, we require all field types to implement `Unaligned`. This
  // ensures that the `repr(C)` layout algorithm will not insert any
- // padding.
+ // padding unless #[repr(align)] explicitly adds padding, which we check
+ // for in this branch's condition.
  //
  // TODO(#10): Support type parameters for non-transparent, non-packed
  // structs without requiring `Unaligned`.
- //
- // TODO(#1763): This is probably unsound! Fix it.
  (None, true)
  } else {
  return Err(Error::new(Span::call_site(), "must have a non-align #[repr(...)] attribute or #[repr(packed)] in order to guarantee this type's memory layout"));
@@ -842,9 +840,16 @@ fn derive_into_bytes_union(ast: &DeriveInput, unn: &DataUnion) -> Result<TokenSt
  return Err(Error::new(Span::call_site(), "unsupported on types with type parameters"));
  }
 
+ // Because we don't support generics, we don't need to worry about
+ // special-casing different reprs. So long as there is *some* repr which
+ // guarantees the layout, our `PaddingCheck::Union` guarantees that there is
+ // no padding.
  let repr = StructUnionRepr::from_attrs(&ast.attrs)?;
  if !repr.is_c() && !repr.is_transparent() && !repr.is_packed_1() {
- return Err(Error::new(Span::call_site(), "must have a non-align #[repr(...)] attribute in order to guarantee this type's memory layout"));
+ return Err(Error::new(
+ Span::call_site(),
+ "must be #[repr(C)], #[repr(packed)], or #[repr(transparent)]",
+ ));
  }
 
  Ok(impl_block(

zerocopy-derive/src/repr.rs

@@ -132,9 +132,11 @@ impl<Prim, Packed> Repr<Prim, Packed> {
  None
  }
  }
-}
 
-impl<Prim, Packed> Repr<Prim, Packed> {
+ pub(crate) fn is_align_gt_1(&self) -> bool {
+ self.get_align().map(|n| n.t.get() > 1).unwrap_or(false)
+ }
+
  /// When deriving `Unaligned`, validate that the decorated type has no
  /// `#[repr(align(N))]` attribute where `N > 1`. If no such attribute exists
  /// (including if `N == 1`), this returns `Ok(())`, and otherwise it returns

zerocopy-derive/tests/ui-msrv/msrv_specific.rs

@@ -0,0 +1,37 @@
+// Copyright 2024 The Fuchsia Authors
+//
+// Licensed under a BSD-style license <LICENSE-BSD>, Apache License, Version 2.0
+// <LICENSE-APACHE or https://www.apache.org/licenses/LICENSE-2.0>, or the MIT
+// license <LICENSE-MIT or https://opensource.org/licenses/MIT>, at your option.
+// This file may not be copied, modified, or distributed except according to
+// those terms.
+
+// This file contains tests which trigger errors on MSRV during a different
+// compiler pass compared to the stable or nightly toolchains.
+
+#[macro_use]
+extern crate zerocopy;
+
+#[path = "../include.rs"]
+mod util;
+
+use self::util::util::AU16;
+use zerocopy::IntoBytes;
+
+fn main() {}
+
+// `repr(C, packed(2))` is not equivalent to `repr(C, packed)`.
+#[derive(IntoBytes)]
+#[repr(C, packed(2))]
+struct IntoBytes1<T> {
+ t0: T,
+ // Add a second field to avoid triggering the "repr(C) struct with one
+ // field" special case.
+ t1: T,
+}
+
+fn is_into_bytes_1<T: IntoBytes>() {
+ if false {
+ is_into_bytes_1::<IntoBytes1<AU16>>();
+ }
+}

zerocopy-derive/tests/ui-msrv/msrv_specific.stderr

@@ -0,0 +1,25 @@
+warning: unused `#[macro_use]` import
+ --> tests/ui-msrv/msrv_specific.rs:12:1
+ |
+12 | #[macro_use]
+ | ^^^^^^^^^^^^
+ |
+ = note: `#[warn(unused_imports)]` on by default
+
+error[E0277]: the trait bound `AU16: Unaligned` is not satisfied
+ --> tests/ui-msrv/msrv_specific.rs:35:9
+ |
+35 | is_into_bytes_1::<IntoBytes1<AU16>>();
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Unaligned` is not implemented for `AU16`
+ |
+note: required because of the requirements on the impl of `zerocopy::IntoBytes` for `IntoBytes1<AU16>`
+ --> tests/ui-msrv/msrv_specific.rs:24:10
+ |
+24 | #[derive(IntoBytes)]
+ | ^^^^^^^^^
+note: required by a bound in `is_into_bytes_1`
+ --> tests/ui-msrv/msrv_specific.rs:33:23
+ |
+33 | fn is_into_bytes_1<T: IntoBytes>() {
+ | ^^^^^^^^^ required by this bound in `is_into_bytes_1`
+ = note: this error originates in the derive macro `IntoBytes` (in Nightly builds, run with -Z macro-backtrace for more info)

zerocopy-derive/tests/ui-msrv/struct.stderr

@@ -20,62 +20,78 @@ error: must have a non-align #[repr(...)] attribute or #[repr(packed)] in order
  |
  = note: this error originates in the derive macro `IntoBytes` (in Nightly builds, run with -Z macro-backtrace for more info)
 
+error: must have a non-align #[repr(...)] attribute or #[repr(packed)] in order to guarantee this type's memory layout
+ --> tests/ui-msrv/struct.rs:164:10
+ |
+164 | #[derive(IntoBytes)]
+ | ^^^^^^^^^
+ |
+ = note: this error originates in the derive macro `IntoBytes` (in Nightly builds, run with -Z macro-backtrace for more info)
+
+error: must have a non-align #[repr(...)] attribute or #[repr(packed)] in order to guarantee this type's memory layout
+ --> tests/ui-msrv/struct.rs:187:10
+ |
+187 | #[derive(IntoBytes)]
+ | ^^^^^^^^^
+ |
+ = note: this error originates in the derive macro `IntoBytes` (in Nightly builds, run with -Z macro-backtrace for more info)
+
 error: cannot derive `Unaligned` on type with alignment greater than 1
- --> tests/ui-msrv/struct.rs:167:11
+ --> tests/ui-msrv/struct.rs:198:11
  |
-167 | #[repr(C, align(2))]
+198 | #[repr(C, align(2))]
  | ^^^^^
 
 error: this conflicts with another representation hint
- --> tests/ui-msrv/struct.rs:171:8
+ --> tests/ui-msrv/struct.rs:202:8
  |
-171 | #[repr(transparent, align(2))]
+202 | #[repr(transparent, align(2))]
  | ^^^^^^^^^^^
 
 error: this conflicts with another representation hint
- --> tests/ui-msrv/struct.rs:177:16
+ --> tests/ui-msrv/struct.rs:208:16
  |
-177 | #[repr(packed, align(2))]
+208 | #[repr(packed, align(2))]
  | ^^^^^
 
 error: this conflicts with another representation hint
- --> tests/ui-msrv/struct.rs:181:18
+ --> tests/ui-msrv/struct.rs:212:18
  |
-181 | #[repr(align(1), align(2))]
+212 | #[repr(align(1), align(2))]
  | ^^^^^
 
 error: this conflicts with another representation hint
- --> tests/ui-msrv/struct.rs:185:18
+ --> tests/ui-msrv/struct.rs:216:18
  |
-185 | #[repr(align(2), align(4))]
+216 | #[repr(align(2), align(4))]
  | ^^^^^
 
 error: must have #[repr(C)], #[repr(transparent)], or #[repr(packed)] attribute in order to guarantee this type's alignment
- --> tests/ui-msrv/struct.rs:188:10
+ --> tests/ui-msrv/struct.rs:219:10
  |
-188 | #[derive(Unaligned)]
+219 | #[derive(Unaligned)]
  | ^^^^^^^^^
  |
  = note: this error originates in the derive macro `Unaligned` (in Nightly builds, run with -Z macro-backtrace for more info)
 
 error: must have #[repr(C)], #[repr(transparent)], or #[repr(packed)] attribute in order to guarantee this type's alignment
- --> tests/ui-msrv/struct.rs:191:10
+ --> tests/ui-msrv/struct.rs:222:10
  |
-191 | #[derive(Unaligned)]
+222 | #[derive(Unaligned)]
  | ^^^^^^^^^
  |
  = note: this error originates in the derive macro `Unaligned` (in Nightly builds, run with -Z macro-backtrace for more info)
 
 error: this conflicts with another representation hint
- --> tests/ui-msrv/struct.rs:201:8
+ --> tests/ui-msrv/struct.rs:232:8
  |
-201 | #[repr(C, packed(2))]
+232 | #[repr(C, packed(2))]
  | ^
 
 error[E0692]: transparent struct cannot have other repr hints
- --> tests/ui-msrv/struct.rs:171:8
+ --> tests/ui-msrv/struct.rs:202:8
  |
-171 | #[repr(transparent, align(2))]
+202 | #[repr(transparent, align(2))]
  | ^^^^^^^^^^^ ^^^^^^^^
 
 error[E0277]: the size for values of type `[u8]` cannot be known at compilation time

zerocopy-derive/tests/ui-msrv/union.stderr

@@ -6,45 +6,61 @@ error: unsupported on types with type parameters
  |
  = note: this error originates in the derive macro `IntoBytes` (in Nightly builds, run with -Z macro-backtrace for more info)
 
+error: must be #[repr(C)], #[repr(packed)], or #[repr(transparent)]
+ --> tests/ui-msrv/union.rs:47:10
+ |
+47 | #[derive(IntoBytes)]
+ | ^^^^^^^^^
+ |
+ = note: this error originates in the derive macro `IntoBytes` (in Nightly builds, run with -Z macro-backtrace for more info)
+
+error: must be #[repr(C)], #[repr(packed)], or #[repr(transparent)]
+ --> tests/ui-msrv/union.rs:53:10
+ |
+53 | #[derive(IntoBytes)]
+ | ^^^^^^^^^
+ |
+ = note: this error originates in the derive macro `IntoBytes` (in Nightly builds, run with -Z macro-backtrace for more info)
+
 error: cannot derive `Unaligned` on type with alignment greater than 1
- --> tests/ui-msrv/union.rs:51:11
+ --> tests/ui-msrv/union.rs:64:11
  |
-51 | #[repr(C, align(2))]
+64 | #[repr(C, align(2))]
  | ^^^^^
 
 error: this conflicts with another representation hint
- --> tests/ui-msrv/union.rs:67:16
+ --> tests/ui-msrv/union.rs:80:16
  |
-67 | #[repr(packed, align(2))]
+80 | #[repr(packed, align(2))]
  | ^^^^^
 
 error: this conflicts with another representation hint
- --> tests/ui-msrv/union.rs:73:18
+ --> tests/ui-msrv/union.rs:86:18
  |
-73 | #[repr(align(1), align(2))]
+86 | #[repr(align(1), align(2))]
  | ^^^^^
 
 error: this conflicts with another representation hint
- --> tests/ui-msrv/union.rs:79:18
+ --> tests/ui-msrv/union.rs:92:18
  |
-79 | #[repr(align(2), align(4))]
+92 | #[repr(align(2), align(4))]
  | ^^^^^
 
 error: must have #[repr(C)], #[repr(transparent)], or #[repr(packed)] attribute in order to guarantee this type's alignment
- --> tests/ui-msrv/union.rs:84:10
+ --> tests/ui-msrv/union.rs:97:10
  |
-84 | #[derive(Unaligned)]
+97 | #[derive(Unaligned)]
  | ^^^^^^^^^
  |
  = note: this error originates in the derive macro `Unaligned` (in Nightly builds, run with -Z macro-backtrace for more info)
 
 error: must have #[repr(C)], #[repr(transparent)], or #[repr(packed)] attribute in order to guarantee this type's alignment
- --> tests/ui-msrv/union.rs:90:10
- |
-90 | #[derive(Unaligned)]
- | ^^^^^^^^^
- |
- = note: this error originates in the derive macro `Unaligned` (in Nightly builds, run with -Z macro-backtrace for more info)
+  --> tests/ui-msrv/union.rs:103:10
+  |
+103 | #[derive(Unaligned)]
+  | ^^^^^^^^^
+  |
+  = note: this error originates in the derive macro `Unaligned` (in Nightly builds, run with -Z macro-backtrace for more info)
 
 error[E0277]: the trait bound `UnsafeCell<()>: zerocopy::Immutable` is not satisfied
  --> tests/ui-msrv/union.rs:24:10

zerocopy-derive/tests/ui-nightly/struct.rs

@@ -12,7 +12,7 @@ extern crate zerocopy;
 #[path = "../include.rs"]
 mod util;
 
-use zerocopy::KnownLayout;
+use zerocopy::{IntoBytes, KnownLayout};
 
 use self::util::util::AU16;
 
@@ -159,6 +159,37 @@ struct IntoBytes9<T> {
  t: T,
 }
 
+// `repr(packed)` without `repr(C)` is not considered sufficient to guarantee
+// layout.
+#[derive(IntoBytes)]
+#[repr(packed)]
+struct IntoBytes10<T> {
+ t: T,
+}
+
+// `repr(C, packed(2))` is not equivalent to `repr(C, packed)`.
+#[derive(IntoBytes)]
+#[repr(C, packed(2))]
+struct IntoBytes11<T> {
+ t0: T,
+ // Add a second field to avoid triggering the "repr(C) struct with one
+ // field" special case.
+ t1: T,
+}
+
+fn is_into_bytes_11<T: IntoBytes>() {
+ if false {
+ is_into_bytes_11::<IntoBytes11<AU16>>();
+ }
+}
+
+// `repr(C, align(2))` is not sufficient to guarantee the layout of this type.
+#[derive(IntoBytes)]
+#[repr(C, align(2))]
+struct IntoBytes12<T> {
+ t: T,
+}
+
 //
 // Unaligned errors
 //