[WIP] Don't assume primitive type alignments

TODO: Commit message Closes #116
google · Nov 3, 2022 · d9f1004 · d9f1004
1 parent 6bd9d7d
commit d9f1004
Show file tree

Hide file tree

Showing 10 changed files with 228 additions and 144 deletions.
diff --git a/src/lib.rs b/src/lib.rs
diff --git a/zerocopy-derive/tests/struct_as_bytes.rs b/zerocopy-derive/tests/struct_as_bytes.rs
@@ -25,6 +25,14 @@ macro_rules! is_as_bytes {
 //   - no padding (size of struct equals sum of size of field types)
 // - `repr(packed)`
 
+// A `u16` with alignment 2.
+//
+// Though `u16` has alignment 2 on some platforms, it's not guaranteed.
+// By contrast, `AU16` is guaranteed to have alignment 2.
+#[derive(AsBytes)]
+#[repr(C, align(2))]
+struct AU16(u16);
+
 #[derive(AsBytes)]
 #[repr(C)]
 struct CZst;
@@ -36,7 +44,7 @@ is_as_bytes!(CZst);
 struct C {
     a: u8,
     b: u8,
-    c: u16,
+    c: AU16,
 }
 
 is_as_bytes!(C);
@@ -60,6 +68,13 @@ is_as_bytes!(CZstPacked);
 #[repr(C, packed)]
 struct CPacked {
     a: u8,
+    // NOTE: The `u16` type is not guaranteed to have alignment 2, although it
+    // does on many platforms. However, to fix this would require a custom type
+    // with a `#[repr(align(2))]` attribute, and `#[repr(packed)]` types are not
+    // allowed to transitively contain `#[repr(align(...))]` types. Thus, we
+    // have no choice but to use `u16` here. Luckily, these tests run in CI on
+    // platforms on which `u16` has alignment 2, so this isn't that big of a
+    // deal.
     b: u16,
 }
 

diff --git a/zerocopy-derive/tests/struct_unaligned.rs b/zerocopy-derive/tests/struct_unaligned.rs
@@ -44,6 +44,13 @@ is_unaligned!(Bar);
 #[derive(Unaligned)]
 #[repr(packed)]
 struct Baz {
+    // NOTE: The `u16` type is not guaranteed to have alignment 2, although it
+    // does on many platforms. However, to fix this would require a custom type
+    // with a `#[repr(align(2))]` attribute, and `#[repr(packed)]` types are not
+    // allowed to transitively contain `#[repr(align(...))]` types. Thus, we
+    // have no choice but to use `u16` here. Luckily, these tests run in CI on
+    // platforms on which `u16` has alignment 2, so this isn't that big of a
+    // deal.
     a: u16,
 }
 

diff --git a/zerocopy-derive/tests/ui/late_compile_pass.rs b/zerocopy-derive/tests/ui/late_compile_pass.rs
@@ -37,10 +37,18 @@ struct AsBytes1 {
 // Unaligned errors
 //
 
+// A `u16` with alignment 2.
+//
+// Though `u16` has alignment 2 on some platforms, it's not guaranteed.
+// By contrast, `AU16` is guaranteed to have alignment 2.
+#[derive(AsBytes)]
+#[repr(C, align(2))]
+struct AU16(u16);
+
 #[derive(Unaligned)]
 #[repr(C)]
 struct Unaligned1 {
-    aligned: u16,
+    aligned: AU16,
 }
 
 // This specifically tests a bug we had in an old version of the code in which
@@ -49,11 +57,11 @@ struct Unaligned1 {
 #[repr(C)]
 struct Unaligned2 {
     unaligned: u8,
-    aligned: u16,
+    aligned: AU16,
 }
 
 #[derive(Unaligned)]
 #[repr(transparent)]
 struct Unaligned3 {
-    aligned: u16,
+    aligned: AU16,
 }
diff --git a/zerocopy-derive/tests/ui/late_compile_pass.stderr b/zerocopy-derive/tests/ui/late_compile_pass.stderr
@@ -13,7 +13,7 @@ error[E0277]: the trait bound `&'static str: FromBytes` is not satisfied
              I16<O>
              I32<O>
              I64<O>
-           and $N others
+           and 36 others
 note: required by a bound in `ImplementsFromBytes`
   --> tests/ui/late_compile_pass.rs:18:10
    |
@@ -29,65 +29,86 @@ error[E0277]: the trait bound `NotAsBytes: AsBytes` is not satisfied
    |
    = help: the following other types implement trait `AsBytes`:
              ()
+             AU16
              AsBytes1
              F32<O>
              F64<O>
              I128<O>
              I16<O>
              I32<O>
-             I64<O>
-           and $N others
-note: required by a bound in `ImplementsAsBytes`
+           and 51 others
+note: required by a bound in `<AsBytes1 as AsBytes>::only_derive_is_allowed_to_implement_this_trait::ImplementsAsBytes`
   --> tests/ui/late_compile_pass.rs:30:10
    |
 30 | #[derive(AsBytes)]
-   |          ^^^^^^^ required by this bound in `ImplementsAsBytes`
+   |          ^^^^^^^ required by this bound in `<AsBytes1 as AsBytes>::only_derive_is_allowed_to_implement_this_trait::ImplementsAsBytes`
    = 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 `u16: Unaligned` is not satisfied
-  --> tests/ui/late_compile_pass.rs:40:10
+error[E0277]: the trait bound `AU16: Unaligned` is not satisfied
+  --> tests/ui/late_compile_pass.rs:48:10
    |
-40 | #[derive(Unaligned)]
-   |          ^^^^^^^^^ the trait `Unaligned` is not implemented for `u16`
+48 | #[derive(Unaligned)]
+   |          ^^^^^^^^^ the trait `Unaligned` is not implemented for `AU16`
    |
    = help: the following other types implement trait `Unaligned`:
-             i8
-             u8
+             ()
+             F32<O>
+             F64<O>
+             I128<O>
+             I16<O>
+             I32<O>
+             I64<O>
+             PhantomData<T>
+           and 14 others
 note: required by a bound in `<Unaligned1 as Unaligned>::only_derive_is_allowed_to_implement_this_trait::ImplementsUnaligned`
-  --> tests/ui/late_compile_pass.rs:40:10
+  --> tests/ui/late_compile_pass.rs:48:10
    |
-40 | #[derive(Unaligned)]
+48 | #[derive(Unaligned)]
    |          ^^^^^^^^^ required by this bound in `<Unaligned1 as Unaligned>::only_derive_is_allowed_to_implement_this_trait::ImplementsUnaligned`
    = 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 `u16: Unaligned` is not satisfied
-  --> tests/ui/late_compile_pass.rs:48:10
+error[E0277]: the trait bound `AU16: Unaligned` is not satisfied
+  --> tests/ui/late_compile_pass.rs:56:10
    |
-48 | #[derive(Unaligned)]
-   |          ^^^^^^^^^ the trait `Unaligned` is not implemented for `u16`
+56 | #[derive(Unaligned)]
+   |          ^^^^^^^^^ the trait `Unaligned` is not implemented for `AU16`
    |
    = help: the following other types implement trait `Unaligned`:
-             i8
-             u8
+             ()
+             F32<O>
+             F64<O>
+             I128<O>
+             I16<O>
+             I32<O>
+             I64<O>
+             PhantomData<T>
+           and 14 others
 note: required by a bound in `<Unaligned2 as Unaligned>::only_derive_is_allowed_to_implement_this_trait::ImplementsUnaligned`
-  --> tests/ui/late_compile_pass.rs:48:10
+  --> tests/ui/late_compile_pass.rs:56:10
    |
-48 | #[derive(Unaligned)]
+56 | #[derive(Unaligned)]
    |          ^^^^^^^^^ required by this bound in `<Unaligned2 as Unaligned>::only_derive_is_allowed_to_implement_this_trait::ImplementsUnaligned`
    = 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 `u16: Unaligned` is not satisfied
-  --> tests/ui/late_compile_pass.rs:55:10
+error[E0277]: the trait bound `AU16: Unaligned` is not satisfied
+  --> tests/ui/late_compile_pass.rs:63:10
    |
-55 | #[derive(Unaligned)]
-   |          ^^^^^^^^^ the trait `Unaligned` is not implemented for `u16`
+63 | #[derive(Unaligned)]
+   |          ^^^^^^^^^ the trait `Unaligned` is not implemented for `AU16`
    |
    = help: the following other types implement trait `Unaligned`:
-             i8
-             u8
+             ()
+             F32<O>
+             F64<O>
+             I128<O>
+             I16<O>
+             I32<O>
+             I64<O>
+             PhantomData<T>
+           and 14 others
 note: required by a bound in `<Unaligned3 as Unaligned>::only_derive_is_allowed_to_implement_this_trait::ImplementsUnaligned`
-  --> tests/ui/late_compile_pass.rs:55:10
+  --> tests/ui/late_compile_pass.rs:63:10
    |
-55 | #[derive(Unaligned)]
+63 | #[derive(Unaligned)]
    |          ^^^^^^^^^ required by this bound in `<Unaligned3 as Unaligned>::only_derive_is_allowed_to_implement_this_trait::ImplementsUnaligned`
    = note: this error originates in the derive macro `Unaligned` (in Nightly builds, run with -Z macro-backtrace for more info)
diff --git a/zerocopy-derive/tests/ui/struct.rs b/zerocopy-derive/tests/ui/struct.rs
@@ -15,15 +15,19 @@ fn main() {}
 #[repr(C)]
 struct AsBytes1<T>(T);
 
+// A `u16` with alignment 2.
+//
+// Though `u16` has alignment 2 on some platforms, it's not guaranteed.
+// By contrast, `AU16` is guaranteed to have alignment 2.
 #[derive(AsBytes)]
-#[repr(C, align(4))]
-struct AlignedU32(u32);
+#[repr(C, align(2))]
+struct AU16(u16);
 
 #[derive(AsBytes)]
 #[repr(C)]
 struct AsBytes2 {
     foo: u8,
-    bar: AlignedU32,
+    bar: AU16,
 }
 
 //

diff --git a/zerocopy-derive/tests/ui/struct.stderr b/zerocopy-derive/tests/ui/struct.stderr
@@ -7,45 +7,45 @@ error: unsupported on types with type parameters
    = 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/struct.rs:34:11
+  --> tests/ui/struct.rs:38:11
    |
-34 | #[repr(C, align(2))]
+38 | #[repr(C, align(2))]
    |           ^^^^^^^^
 
 error: cannot derive Unaligned with repr(align(N > 1))
-  --> tests/ui/struct.rs:38:21
+  --> tests/ui/struct.rs:42:21
    |
-38 | #[repr(transparent, align(2))]
+42 | #[repr(transparent, align(2))]
    |                     ^^^^^^^^
 
 error: cannot derive Unaligned with repr(align(N > 1))
-  --> tests/ui/struct.rs:44:16
+  --> tests/ui/struct.rs:48:16
    |
-44 | #[repr(packed, align(2))]
+48 | #[repr(packed, align(2))]
    |                ^^^^^^^^
 
 error: cannot derive Unaligned with repr(align(N > 1))
-  --> tests/ui/struct.rs:48:18
+  --> tests/ui/struct.rs:52:18
    |
-48 | #[repr(align(1), align(2))]
+52 | #[repr(align(1), align(2))]
    |                  ^^^^^^^^
 
 error: cannot derive Unaligned with repr(align(N > 1))
-  --> tests/ui/struct.rs:52:8
+  --> tests/ui/struct.rs:56:8
    |
-52 | #[repr(align(2), align(4))]
+56 | #[repr(align(2), align(4))]
    |        ^^^^^^^^
 
 error[E0692]: transparent struct cannot have other repr hints
-  --> tests/ui/struct.rs:38:8
+  --> tests/ui/struct.rs:42:8
    |
-38 | #[repr(transparent, align(2))]
+42 | #[repr(transparent, align(2))]
    |        ^^^^^^^^^^^  ^^^^^^^^
 
 error[E0277]: the trait bound `HasPadding<AsBytes2, true>: ShouldBe<false>` is not satisfied
-  --> tests/ui/struct.rs:22:10
+  --> tests/ui/struct.rs:26:10
    |
-22 | #[derive(AsBytes)]
+26 | #[derive(AsBytes)]
    |          ^^^^^^^ the trait `ShouldBe<false>` is not implemented for `HasPadding<AsBytes2, true>`
    |
    = help: the trait `ShouldBe<VALUE>` is implemented for `HasPadding<T, VALUE>`

diff --git a/zerocopy-derive/tests/ui/union.rs b/zerocopy-derive/tests/ui/union.rs
@@ -30,11 +30,19 @@ union AsBytes2 {
 // Unaligned errors
 //
 
+// A `u16` with alignment 2.
+//
+// Though `u16` has alignment 2 on some platforms, it's not guaranteed.
+// By contrast, `AU16` is guaranteed to have alignment 2.
+#[derive(AsBytes)]
+#[repr(C, align(2))]
+struct AU16(u16);
+
 #[derive(Unaligned)]
 #[repr(C, align(2))]
 union Unaligned1 {
     foo: i16,
-    bar: u16,
+    bar: AU16,
 }
 
 // Transparent unions are unstable; see issue #60405

diff --git a/zerocopy-derive/tests/ui/union.stderr b/zerocopy-derive/tests/ui/union.stderr
@@ -7,27 +7,27 @@ error: unsupported on types with type parameters
    = 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/union.rs:34:11
+  --> tests/ui/union.rs:42:11
    |
-34 | #[repr(C, align(2))]
+42 | #[repr(C, align(2))]
    |           ^^^^^^^^
 
 error: cannot derive Unaligned with repr(align(N > 1))
-  --> tests/ui/union.rs:50:16
+  --> tests/ui/union.rs:58:16
    |
-50 | #[repr(packed, align(2))]
+58 | #[repr(packed, align(2))]
    |                ^^^^^^^^
 
 error: cannot derive Unaligned with repr(align(N > 1))
-  --> tests/ui/union.rs:56:18
+  --> tests/ui/union.rs:64:18
    |
-56 | #[repr(align(1), align(2))]
+64 | #[repr(align(1), align(2))]
    |                  ^^^^^^^^
 
 error: cannot derive Unaligned with repr(align(N > 1))
-  --> tests/ui/union.rs:62:8
+  --> tests/ui/union.rs:70:8
    |
-62 | #[repr(align(2), align(4))]
+70 | #[repr(align(2), align(4))]
    |        ^^^^^^^^
 
 error[E0277]: the trait bound `HasPadding<AsBytes2, true>: ShouldBe<false>` is not satisfied

diff --git a/zerocopy-derive/tests/union_unaligned.rs b/zerocopy-derive/tests/union_unaligned.rs
@@ -47,6 +47,13 @@ is_unaligned!(Foo);
 #[derive(Unaligned)]
 #[repr(packed)]
 union Baz {
+    // NOTE: The `u16` type is not guaranteed to have alignment 2, although it
+    // does on many platforms. However, to fix this would require a custom type
+    // with a `#[repr(align(2))]` attribute, and `#[repr(packed)]` types are not
+    // allowed to transitively contain `#[repr(align(...))]` types. Thus, we
+    // have no choice but to use `u16` here. Luckily, these tests run in CI on
+    // platforms on which `u16` has alignment 2, so this isn't that big of a
+    // deal.
     a: u16,
 }