[derive] Fix AsBytes for #[repr(C, packed(N))] (#672)

maurer · joshlf · web-flow · commit 941c83fb4cea · 2023-12-05T18:11:34.000Z
`packed(N)` does not gaurantee no padding, but it doesn't prevent it
either. This was previously supported.

Co-authored-by: Joshua Liebow-Feeser &lt;joshlf@users.noreply.github.com&gt;
diff --git a/zerocopy-derive/src/repr.rs b/zerocopy-derive/src/repr.rs
@@ -106,23 +106,24 @@ pub trait KindRepr: 'static + Sized + Ord {
 // etc), and provide implementations of `KindRepr`, `Ord`, and `Display`, and
 // those traits' super-traits.
 macro_rules! define_kind_specific_repr {
-    ($type_name:expr, $repr_name:ident, $($repr_variant:ident),*) => {
+    ($type_name:expr, $repr_name:ident, [ $($repr_variant:ident),* ] , [ $($repr_variant_aligned:ident),* ]) => {
         #[derive(Copy, Clone, Debug, Eq, PartialEq)]
         pub enum $repr_name {
             $($repr_variant,)*
-            Align(u64),
+            $($repr_variant_aligned(u64),)*
         }
 
         impl KindRepr for $repr_name {
             fn is_align(&self) -> bool {
                 match self {
-                    $repr_name::Align(_) => true,
+                    $($repr_name::$repr_variant_aligned(_) => true,)*
                     _ => false,
                 }
             }
 
             fn is_align_gt_one(&self) -> bool {
                 match self {
+                    // `packed(n)` only lowers alignment
                     $repr_name::Align(n) => n > &1,
                     _ => false,
                 }
@@ -131,7 +132,7 @@ macro_rules! define_kind_specific_repr {
             fn parse(meta: &Meta) -> syn::Result<$repr_name> {
                 match Repr::from_meta(meta)? {
                     $(Repr::$repr_variant => Ok($repr_name::$repr_variant),)*
-                    Repr::Align(u) => Ok($repr_name::Align(u)),
+                    $(Repr::$repr_variant_aligned(u) => Ok($repr_name::$repr_variant_aligned(u)),)*
                     _ => Err(Error::new_spanned(meta, concat!("unsupported representation for deriving FromBytes, AsBytes, or Unaligned on ", $type_name)))
                 }
             }
@@ -155,16 +156,19 @@ macro_rules! define_kind_specific_repr {
             fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
                 match self {
                     $($repr_name::$repr_variant => Repr::$repr_variant,)*
-                    $repr_name::Align(u) => Repr::Align(*u),
+                    $($repr_name::$repr_variant_aligned(u) => Repr::$repr_variant_aligned(*u),)*
                 }.fmt(f)
             }
         }
     }
 }
 
-define_kind_specific_repr!("a struct", StructRepr, C, Transparent, Packed);
+define_kind_specific_repr!("a struct", StructRepr, [C, Transparent, Packed], [Align, PackedN]);
 define_kind_specific_repr!(
-    "an enum", EnumRepr, C, U8, U16, U32, U64, Usize, I8, I16, I32, I64, Isize
+    "an enum",
+    EnumRepr,
+    [C, U8, U16, U32, U64, Usize, I8, I16, I32, I64, Isize],
+    [Align]
 );
 
 // All representations known to Rust.
diff --git a/zerocopy-derive/tests/struct_as_bytes.rs b/zerocopy-derive/tests/struct_as_bytes.rs
@@ -79,6 +79,18 @@ struct CPacked {
 
 assert_impl_all!(CPacked: AsBytes);
 
+#[derive(AsBytes)]
+#[repr(C, packed(2))]
+// The same caveats as for CPacked apply - we're assuming u64 is at least
+// 4-byte aligned by default. Without packed(2), this should fail, as there
+// would be padding between a/b assuming u64 is 4+ byte aligned.
+struct CPacked2 {
+    a: u16,
+    b: u64,
+}
+
+assert_impl_all!(CPacked2: AsBytes);
+
 #[derive(AsBytes)]
 #[repr(C, packed)]
 struct CPackedGeneric<T, U: ?Sized> {
diff --git a/zerocopy-derive/tests/ui-msrv/struct.stderr b/zerocopy-derive/tests/ui-msrv/struct.stderr
@@ -7,39 +7,39 @@ error: unsupported on generic structs that are not repr(transparent) or repr(pac
    = note: this error originates in the derive macro `AsBytes` (in Nightly builds, run with -Z macro-backtrace for more info)
 
 error: cannot derive Unaligned with repr(align(N > 1))
-  --> tests/ui-msrv/struct.rs:71:11
+  --> tests/ui-msrv/struct.rs:80:11
    |
-71 | #[repr(C, align(2))]
+80 | #[repr(C, align(2))]
    |           ^^^^^^^^
 
 error: cannot derive Unaligned with repr(align(N > 1))
-  --> tests/ui-msrv/struct.rs:75:21
+  --> tests/ui-msrv/struct.rs:84:21
    |
-75 | #[repr(transparent, align(2))]
+84 | #[repr(transparent, align(2))]
    |                     ^^^^^^^^
 
 error: cannot derive Unaligned with repr(align(N > 1))
-  --> tests/ui-msrv/struct.rs:81:16
+  --> tests/ui-msrv/struct.rs:90:16
    |
-81 | #[repr(packed, align(2))]
+90 | #[repr(packed, align(2))]
    |                ^^^^^^^^
 
 error: cannot derive Unaligned with repr(align(N > 1))
-  --> tests/ui-msrv/struct.rs:85:18
+  --> tests/ui-msrv/struct.rs:94:18
    |
-85 | #[repr(align(1), align(2))]
+94 | #[repr(align(1), align(2))]
    |                  ^^^^^^^^
 
 error: cannot derive Unaligned with repr(align(N > 1))
-  --> tests/ui-msrv/struct.rs:89:8
+  --> tests/ui-msrv/struct.rs:98:8
    |
-89 | #[repr(align(2), align(4))]
+98 | #[repr(align(2), align(4))]
    |        ^^^^^^^^
 
 error[E0692]: transparent struct cannot have other repr hints
-  --> tests/ui-msrv/struct.rs:75:8
+  --> tests/ui-msrv/struct.rs:84:8
    |
-75 | #[repr(transparent, align(2))]
+84 | #[repr(transparent, align(2))]
    |        ^^^^^^^^^^^  ^^^^^^^^
 
 error[E0277]: the size for values of type `[u8]` cannot be known at compilation time
@@ -100,3 +100,14 @@ error[E0277]: the trait bound `HasPadding<AsBytes2, true>: ShouldBe<false>` is n
              <HasPadding<T, VALUE> as ShouldBe<VALUE>>
    = help: see issue #48214
    = note: this error originates in the derive macro `AsBytes` (in Nightly builds, run with -Z macro-backtrace for more info)
+
+error[E0277]: the trait bound `HasPadding<AsBytes3, true>: ShouldBe<false>` is not satisfied
+  --> tests/ui-msrv/struct.rs:66:10
+   |
+66 | #[derive(AsBytes)]
+   |          ^^^^^^^ the trait `ShouldBe<false>` is not implemented for `HasPadding<AsBytes3, true>`
+   |
+   = help: the following implementations were found:
+             <HasPadding<T, VALUE> as ShouldBe<VALUE>>
+   = help: see issue #48214
+   = note: this error originates in the derive macro `AsBytes` (in Nightly builds, run with -Z macro-backtrace for more info)
diff --git a/zerocopy-derive/tests/ui-nightly/struct.rs b/zerocopy-derive/tests/ui-nightly/struct.rs
@@ -63,6 +63,15 @@ struct AsBytes2 {
     bar: AU16,
 }
 
+#[derive(AsBytes)]
+#[repr(C, packed(2))]
+struct AsBytes3 {
+    foo: u8,
+    // We'd prefer to use AU64 here, but you can't use aligned types in
+    // packed structs.
+    bar: u64,
+}
+
 //
 // Unaligned errors
 //
diff --git a/zerocopy-derive/tests/ui-nightly/struct.stderr b/zerocopy-derive/tests/ui-nightly/struct.stderr
@@ -7,39 +7,39 @@ error: unsupported on generic structs that are not repr(transparent) or repr(pac
    = note: this error originates in the derive macro `AsBytes` (in Nightly builds, run with -Z macro-backtrace for more info)
 
 error: cannot derive Unaligned with repr(align(N > 1))
-  --> tests/ui-nightly/struct.rs:71:11
+  --> tests/ui-nightly/struct.rs:80:11
    |
-71 | #[repr(C, align(2))]
+80 | #[repr(C, align(2))]
    |           ^^^^^^^^
 
 error: cannot derive Unaligned with repr(align(N > 1))
-  --> tests/ui-nightly/struct.rs:75:21
+  --> tests/ui-nightly/struct.rs:84:21
    |
-75 | #[repr(transparent, align(2))]
+84 | #[repr(transparent, align(2))]
    |                     ^^^^^^^^
 
 error: cannot derive Unaligned with repr(align(N > 1))
-  --> tests/ui-nightly/struct.rs:81:16
+  --> tests/ui-nightly/struct.rs:90:16
    |
-81 | #[repr(packed, align(2))]
+90 | #[repr(packed, align(2))]
    |                ^^^^^^^^
 
 error: cannot derive Unaligned with repr(align(N > 1))
-  --> tests/ui-nightly/struct.rs:85:18
+  --> tests/ui-nightly/struct.rs:94:18
    |
-85 | #[repr(align(1), align(2))]
+94 | #[repr(align(1), align(2))]
    |                  ^^^^^^^^
 
 error: cannot derive Unaligned with repr(align(N > 1))
-  --> tests/ui-nightly/struct.rs:89:8
+  --> tests/ui-nightly/struct.rs:98:8
    |
-89 | #[repr(align(2), align(4))]
+98 | #[repr(align(2), align(4))]
    |        ^^^^^^^^
 
 error[E0692]: transparent struct cannot have other repr hints
-  --> tests/ui-nightly/struct.rs:75:8
+  --> tests/ui-nightly/struct.rs:84:8
    |
-75 | #[repr(transparent, align(2))]
+84 | #[repr(transparent, align(2))]
    |        ^^^^^^^^^^^  ^^^^^^^^
 
 error[E0277]: the size for values of type `[u8]` cannot be known at compilation time
@@ -125,8 +125,19 @@ error[E0277]: the trait bound `HasPadding<AsBytes2, true>: ShouldBe<false>` is n
    = help: add `#![feature(trivial_bounds)]` to the crate attributes to enable
    = note: this error originates in the derive macro `AsBytes` (in Nightly builds, run with -Z macro-backtrace for more info)
 
+error[E0277]: the trait bound `HasPadding<AsBytes3, true>: ShouldBe<false>` is not satisfied
+  --> tests/ui-nightly/struct.rs:66:10
+   |
+66 | #[derive(AsBytes)]
+   |          ^^^^^^^ the trait `ShouldBe<false>` is not implemented for `HasPadding<AsBytes3, true>`
+   |
+   = help: the trait `ShouldBe<true>` is implemented for `HasPadding<AsBytes3, true>`
+   = help: see issue #48214
+   = help: add `#![feature(trivial_bounds)]` to the crate attributes to enable
+   = note: this error originates in the derive macro `AsBytes` (in Nightly builds, run with -Z macro-backtrace for more info)
+
 error[E0587]: type has conflicting packed and align representation hints
-  --> tests/ui-nightly/struct.rs:82:1
+  --> tests/ui-nightly/struct.rs:91:1
    |
-82 | struct Unaligned3;
+91 | struct Unaligned3;
    | ^^^^^^^^^^^^^^^^^
diff --git a/zerocopy-derive/tests/ui-stable/struct.stderr b/zerocopy-derive/tests/ui-stable/struct.stderr
@@ -7,39 +7,39 @@ error: unsupported on generic structs that are not repr(transparent) or repr(pac
    = note: this error originates in the derive macro `AsBytes` (in Nightly builds, run with -Z macro-backtrace for more info)
 
 error: cannot derive Unaligned with repr(align(N > 1))
-  --> tests/ui-stable/struct.rs:71:11
+  --> tests/ui-stable/struct.rs:80:11
    |
-71 | #[repr(C, align(2))]
+80 | #[repr(C, align(2))]
    |           ^^^^^^^^
 
 error: cannot derive Unaligned with repr(align(N > 1))
-  --> tests/ui-stable/struct.rs:75:21
+  --> tests/ui-stable/struct.rs:84:21
    |
-75 | #[repr(transparent, align(2))]
+84 | #[repr(transparent, align(2))]
    |                     ^^^^^^^^
 
 error: cannot derive Unaligned with repr(align(N > 1))
-  --> tests/ui-stable/struct.rs:81:16
+  --> tests/ui-stable/struct.rs:90:16
    |
-81 | #[repr(packed, align(2))]
+90 | #[repr(packed, align(2))]
    |                ^^^^^^^^
 
 error: cannot derive Unaligned with repr(align(N > 1))
-  --> tests/ui-stable/struct.rs:85:18
+  --> tests/ui-stable/struct.rs:94:18
    |
-85 | #[repr(align(1), align(2))]
+94 | #[repr(align(1), align(2))]
    |                  ^^^^^^^^
 
 error: cannot derive Unaligned with repr(align(N > 1))
-  --> tests/ui-stable/struct.rs:89:8
+  --> tests/ui-stable/struct.rs:98:8
    |
-89 | #[repr(align(2), align(4))]
+98 | #[repr(align(2), align(4))]
    |        ^^^^^^^^
 
 error[E0692]: transparent struct cannot have other repr hints
-  --> tests/ui-stable/struct.rs:75:8
+  --> tests/ui-stable/struct.rs:84:8
    |
-75 | #[repr(transparent, align(2))]
+84 | #[repr(transparent, align(2))]
    |        ^^^^^^^^^^^  ^^^^^^^^
 
 error[E0277]: the size for values of type `[u8]` cannot be known at compilation time
@@ -119,3 +119,13 @@ error[E0277]: the trait bound `HasPadding<AsBytes2, true>: ShouldBe<false>` is n
    = help: the trait `ShouldBe<VALUE>` is implemented for `HasPadding<T, VALUE>`
    = help: see issue #48214
    = note: this error originates in the derive macro `AsBytes` (in Nightly builds, run with -Z macro-backtrace for more info)
+
+error[E0277]: the trait bound `HasPadding<AsBytes3, true>: ShouldBe<false>` is not satisfied
+  --> tests/ui-stable/struct.rs:66:10
+   |
+66 | #[derive(AsBytes)]
+   |          ^^^^^^^ the trait `ShouldBe<false>` is not implemented for `HasPadding<AsBytes3, true>`
+   |
+   = help: the trait `ShouldBe<VALUE>` is implemented for `HasPadding<T, VALUE>`
+   = help: see issue #48214
+   = note: this error originates in the derive macro `AsBytes` (in Nightly builds, run with -Z macro-backtrace for more info)

Original file line number	Diff line number	Diff line change
`@@ -106,23 +106,24 @@ pub trait KindRepr: 'static + Sized + Ord {`
`106`	`106`	// etc), and provide implementations of `KindRepr`, `Ord`, and `Display`, and
`107`	`107`	`// those traits' super-traits.`
`108`	`108`	`macro_rules! define_kind_specific_repr {`
`109`		`- ($type_name:expr, $repr_name:ident, $($repr_variant:ident),*) => {`
	`109`	`+ ($type_name:expr, $repr_name:ident, [ $($repr_variant:ident),* ] , [ $($repr_variant_aligned:ident),* ]) => {`
`110`	`110`	`#[derive(Copy, Clone, Debug, Eq, PartialEq)]`
`111`	`111`	`pub enum $repr_name {`
`112`	`112`	`$($repr_variant,)*`
`113`		`- Align(u64),`
	`113`	`+ $($repr_variant_aligned(u64),)*`
`114`	`114`	`}`
`115`	`115`
`116`	`116`	`impl KindRepr for $repr_name {`
`117`	`117`	`fn is_align(&self) -> bool {`
`118`	`118`	`match self {`
`119`		`- $repr_name::Align(_) => true,`
	`119`	`+ $($repr_name::$repr_variant_aligned(_) => true,)*`
`120`	`120`	`_ => false,`
`121`	`121`	`}`
`122`	`122`	`}`
`123`	`123`
`124`	`124`	`fn is_align_gt_one(&self) -> bool {`
`125`	`125`	`match self {`
	`126`	+ // `packed(n)` only lowers alignment
`126`	`127`	`$repr_name::Align(n) => n > &1,`
`127`	`128`	`_ => false,`
`128`	`129`	`}`
`@@ -131,7 +132,7 @@ macro_rules! define_kind_specific_repr {`
`131`	`132`	`fn parse(meta: &Meta) -> syn::Result<$repr_name> {`
`132`	`133`	`match Repr::from_meta(meta)? {`
`133`	`134`	`$(Repr::$repr_variant => Ok($repr_name::$repr_variant),)*`
`134`		`- Repr::Align(u) => Ok($repr_name::Align(u)),`
	`135`	`+ $(Repr::$repr_variant_aligned(u) => Ok($repr_name::$repr_variant_aligned(u)),)*`
`135`	`136`	`_ => Err(Error::new_spanned(meta, concat!("unsupported representation for deriving FromBytes, AsBytes, or Unaligned on ", $type_name)))`
`136`	`137`	`}`
`137`	`138`	`}`
`@@ -155,16 +156,19 @@ macro_rules! define_kind_specific_repr {`
`155`	`156`	`fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {`
`156`	`157`	`match self {`
`157`	`158`	`$($repr_name::$repr_variant => Repr::$repr_variant,)*`
`158`		`- $repr_name::Align(u) => Repr::Align(*u),`
	`159`	`+ $($repr_name::$repr_variant_aligned(u) => Repr::$repr_variant_aligned(u),)`
`159`	`160`	`}.fmt(f)`
`160`	`161`	`}`
`161`	`162`	`}`
`162`	`163`	`}`
`163`	`164`	`}`
`164`	`165`
`165`		`-define_kind_specific_repr!("a struct", StructRepr, C, Transparent, Packed);`
	`166`	`+define_kind_specific_repr!("a struct", StructRepr, [C, Transparent, Packed], [Align, PackedN]);`
`166`	`167`	`define_kind_specific_repr!(`
`167`		`- "an enum", EnumRepr, C, U8, U16, U32, U64, Usize, I8, I16, I32, I64, Isize`
	`168`	`+ "an enum",`
	`169`	`+ EnumRepr,`
	`170`	`+ [C, U8, U16, U32, U64, Usize, I8, I16, I32, I64, Isize],`
	`171`	`+ [Align]`
`168`	`172`	`);`
`169`	`173`
`170`	`174`	`// All representations known to Rust.`