Skip to content

Commit cc6597a

Browse files
authored
[derive] Support IntoBytes on repr(Rust) structs (#1796)
The Reference guarantees that, even in `repr(Rust)` structs, fields cannot overlap. This is sufficient to guarantee the soundness of implementing `IntoBytes` on some `repr(Rust)` structs. Closes #1794
1 parent d6fb61c commit cc6597a

File tree

6 files changed

+138
-199
lines changed

6 files changed

+138
-199
lines changed

zerocopy-derive/src/lib.rs

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -853,27 +853,42 @@ fn derive_into_bytes_struct(ast: &DeriveInput, strct: &DataStruct) -> Result<Tok
853853
let is_packed_1 = repr.is_packed_1();
854854
let num_fields = strct.fields().len();
855855

856-
let (padding_check, require_unaligned_fields) = if is_transparent || (is_c && is_packed_1) {
856+
let (padding_check, require_unaligned_fields) = if is_transparent || is_packed_1 {
857857
// No padding check needed.
858858
// - repr(transparent): The layout and ABI of the whole struct is the
859859
// same as its only non-ZST field (meaning there's no padding outside
860860
// of that field) and we require that field to be `IntoBytes` (meaning
861861
// there's no padding in that field).
862-
// - repr(C, packed): Any inter-field padding bytes are removed, meaning
862+
// - repr(packed): Any inter-field padding bytes are removed, meaning
863863
// that any padding bytes would need to come from the fields, all of
864864
// which we require to be `IntoBytes` (meaning they don't have any
865-
// padding).
865+
// padding). Note that this holds regardless of other `repr`
866+
// attributes, including `repr(Rust)`. [1]
867+
//
868+
// [1] Per https://doc.rust-lang.org/1.81.0/reference/type-layout.html#the-alignment-modifiers:
869+
//
870+
// An important consequence of these rules is that a type with
871+
// `#[repr(packed(1))]`` (or `#[repr(packed)]``) will have no
872+
// inter-field padding.
866873
(None, false)
867874
} else if is_c && !repr.is_align_gt_1() && num_fields <= 1 {
868875
// No padding check needed. A repr(C) struct with zero or one field has
869876
// no padding unless #[repr(align)] explicitly adds padding, which we
870877
// check for in this branch's condition.
871878
(None, false)
872-
} else if is_c && ast.generics.params.is_empty() {
873-
// Since there are no generics, we can emit a padding check. `repr(C)`
874-
// guarantees that fields won't overlap, so the padding check is sound.
875-
// This is more permissive than the next case, which requires that all
876-
// field types implement `Unaligned`.
879+
} else if ast.generics.params.is_empty() {
880+
// Since there are no generics, we can emit a padding check. All reprs
881+
// guarantee that fields won't overlap [1], so the padding check is
882+
// sound. This is more permissive than the next case, which requires
883+
// that all field types implement `Unaligned`.
884+
//
885+
// [1] Per https://doc.rust-lang.org/1.81.0/reference/type-layout.html#the-rust-representation:
886+
//
887+
// The only data layout guarantees made by [`repr(Rust)`] are those
888+
// required for soundness. They are:
889+
// ...
890+
// 2. The fields do not overlap.
891+
// ...
877892
(Some(PaddingCheck::Struct), false)
878893
} else if is_c && !repr.is_align_gt_1() {
879894
// We can't use a padding check since there are generic type arguments.

zerocopy-derive/tests/struct_to_bytes.rs

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,31 @@ struct CPackedGeneric<T, U: ?imp::Sized> {
102102
util_assert_impl_all!(CPackedGeneric<u8, util::AU16>: imp::IntoBytes);
103103
util_assert_impl_all!(CPackedGeneric<u8, [util::AU16]>: imp::IntoBytes);
104104

105+
#[derive(imp::IntoBytes)]
106+
#[repr(packed)]
107+
struct PackedGeneric<T, U: ?imp::Sized> {
108+
t: T,
109+
// Unsized types stored in `repr(packed)` structs must not be dropped
110+
// because dropping them in-place might be unsound depending on the
111+
// alignment of the outer struct. Sized types can be dropped by first being
112+
// moved to an aligned stack variable, but this isn't possible with unsized
113+
// types.
114+
u: imp::ManuallyDrop<U>,
115+
}
116+
117+
util_assert_impl_all!(PackedGeneric<u8, util::AU16>: imp::IntoBytes);
118+
util_assert_impl_all!(PackedGeneric<u8, [util::AU16]>: imp::IntoBytes);
119+
120+
// This test is non-portable, but works so long as Rust happens to lay this
121+
// struct out with no padding.
122+
#[derive(imp::IntoBytes)]
123+
struct Unpacked {
124+
a: u8,
125+
b: u8,
126+
}
127+
128+
util_assert_impl_all!(Unpacked: imp::IntoBytes);
129+
105130
#[derive(imp::IntoBytes)]
106131
#[repr(C)]
107132
struct ReprCGenericOneField<T: ?imp::Sized> {

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

Lines changed: 24 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -5,109 +5,85 @@ error: this conflicts with another representation hint
55
| ^
66

77
error: must have a non-align #[repr(...)] attribute in order to guarantee this type's memory layout
8-
--> tests/ui-msrv/struct.rs:140:10
8+
--> tests/ui-msrv/struct.rs:138:10
99
|
10-
140 | #[derive(IntoBytes)]
10+
138 | #[derive(IntoBytes)]
1111
| ^^^^^^^^^
1212
|
1313
= note: this error originates in the derive macro `IntoBytes` (in Nightly builds, run with -Z macro-backtrace for more info)
1414

1515
error: must have a non-align #[repr(...)] attribute in order to guarantee this type's memory layout
16-
--> tests/ui-msrv/struct.rs:150:10
16+
--> tests/ui-msrv/struct.rs:143:10
1717
|
18-
150 | #[derive(IntoBytes)]
18+
143 | #[derive(IntoBytes)]
1919
| ^^^^^^^^^
2020
|
2121
= note: this error originates in the derive macro `IntoBytes` (in Nightly builds, run with -Z macro-backtrace for more info)
2222

2323
error: must have a non-align #[repr(...)] attribute in order to guarantee this type's memory layout
24-
--> tests/ui-msrv/struct.rs:159:10
24+
--> tests/ui-msrv/struct.rs:166:10
2525
|
26-
159 | #[derive(IntoBytes)]
27-
| ^^^^^^^^^
28-
|
29-
= note: this error originates in the derive macro `IntoBytes` (in Nightly builds, run with -Z macro-backtrace for more info)
30-
31-
error: must have a non-align #[repr(...)] attribute in order to guarantee this type's memory layout
32-
--> tests/ui-msrv/struct.rs:164:10
33-
|
34-
164 | #[derive(IntoBytes)]
35-
| ^^^^^^^^^
36-
|
37-
= note: this error originates in the derive macro `IntoBytes` (in Nightly builds, run with -Z macro-backtrace for more info)
38-
39-
error: must have a non-align #[repr(...)] attribute in order to guarantee this type's memory layout
40-
--> tests/ui-msrv/struct.rs:172:10
41-
|
42-
172 | #[derive(IntoBytes)]
43-
| ^^^^^^^^^
44-
|
45-
= note: this error originates in the derive macro `IntoBytes` (in Nightly builds, run with -Z macro-backtrace for more info)
46-
47-
error: must have a non-align #[repr(...)] attribute in order to guarantee this type's memory layout
48-
--> tests/ui-msrv/struct.rs:195:10
49-
|
50-
195 | #[derive(IntoBytes)]
26+
166 | #[derive(IntoBytes)]
5127
| ^^^^^^^^^
5228
|
5329
= note: this error originates in the derive macro `IntoBytes` (in Nightly builds, run with -Z macro-backtrace for more info)
5430

5531
error: cannot derive `Unaligned` on type with alignment greater than 1
56-
--> tests/ui-msrv/struct.rs:206:11
32+
--> tests/ui-msrv/struct.rs:177:11
5733
|
58-
206 | #[repr(C, align(2))]
34+
177 | #[repr(C, align(2))]
5935
| ^^^^^
6036

6137
error: this conflicts with another representation hint
62-
--> tests/ui-msrv/struct.rs:210:8
38+
--> tests/ui-msrv/struct.rs:181:8
6339
|
64-
210 | #[repr(transparent, align(2))]
40+
181 | #[repr(transparent, align(2))]
6541
| ^^^^^^^^^^^
6642

6743
error: this conflicts with another representation hint
68-
--> tests/ui-msrv/struct.rs:216:16
44+
--> tests/ui-msrv/struct.rs:187:16
6945
|
70-
216 | #[repr(packed, align(2))]
46+
187 | #[repr(packed, align(2))]
7147
| ^^^^^
7248

7349
error: this conflicts with another representation hint
74-
--> tests/ui-msrv/struct.rs:220:18
50+
--> tests/ui-msrv/struct.rs:191:18
7551
|
76-
220 | #[repr(align(1), align(2))]
52+
191 | #[repr(align(1), align(2))]
7753
| ^^^^^
7854

7955
error: this conflicts with another representation hint
80-
--> tests/ui-msrv/struct.rs:224:18
56+
--> tests/ui-msrv/struct.rs:195:18
8157
|
82-
224 | #[repr(align(2), align(4))]
58+
195 | #[repr(align(2), align(4))]
8359
| ^^^^^
8460

8561
error: must have #[repr(C)], #[repr(transparent)], or #[repr(packed)] attribute in order to guarantee this type's alignment
86-
--> tests/ui-msrv/struct.rs:227:10
62+
--> tests/ui-msrv/struct.rs:198:10
8763
|
88-
227 | #[derive(Unaligned)]
64+
198 | #[derive(Unaligned)]
8965
| ^^^^^^^^^
9066
|
9167
= note: this error originates in the derive macro `Unaligned` (in Nightly builds, run with -Z macro-backtrace for more info)
9268

9369
error: must have #[repr(C)], #[repr(transparent)], or #[repr(packed)] attribute in order to guarantee this type's alignment
94-
--> tests/ui-msrv/struct.rs:230:10
70+
--> tests/ui-msrv/struct.rs:201:10
9571
|
96-
230 | #[derive(Unaligned)]
72+
201 | #[derive(Unaligned)]
9773
| ^^^^^^^^^
9874
|
9975
= note: this error originates in the derive macro `Unaligned` (in Nightly builds, run with -Z macro-backtrace for more info)
10076

10177
error: this conflicts with another representation hint
102-
--> tests/ui-msrv/struct.rs:240:8
78+
--> tests/ui-msrv/struct.rs:211:8
10379
|
104-
240 | #[repr(C, packed(2))]
80+
211 | #[repr(C, packed(2))]
10581
| ^
10682

10783
error[E0692]: transparent struct cannot have other repr hints
108-
--> tests/ui-msrv/struct.rs:210:8
84+
--> tests/ui-msrv/struct.rs:181:8
10985
|
110-
210 | #[repr(transparent, align(2))]
86+
181 | #[repr(transparent, align(2))]
11187
| ^^^^^^^^^^^ ^^^^^^^^
11288

11389
error[E0277]: the size for values of type `[u8]` cannot be known at compilation time

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

Lines changed: 0 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -135,27 +135,6 @@ struct IntoBytes5 {
135135
a: u8,
136136
}
137137

138-
// We don't emit a padding check unless there's a repr that can guarantee that
139-
// fields don't overlap.
140-
#[derive(IntoBytes)]
141-
struct IntoBytes6 {
142-
a: u8,
143-
// Add a second field to avoid triggering the "repr(C) struct with one
144-
// field" special case.
145-
b: u8,
146-
}
147-
148-
// We don't emit a padding check unless there's a repr that can guarantee that
149-
// fields don't overlap. `repr(packed)` on its own doesn't guarantee this.
150-
#[derive(IntoBytes)]
151-
#[repr(packed(2))]
152-
struct IntoBytes7 {
153-
a: u8,
154-
// Add a second field to avoid triggering the "repr(C) struct with one
155-
// field" special case.
156-
b: u8,
157-
}
158-
159138
#[derive(IntoBytes)]
160139
struct IntoBytes8<T> {
161140
t: T,
@@ -167,14 +146,6 @@ struct IntoBytes9<T> {
167146
t: T,
168147
}
169148

170-
// `repr(packed)` without `repr(C)` is not considered sufficient to guarantee
171-
// layout.
172-
#[derive(IntoBytes)]
173-
#[repr(packed)]
174-
struct IntoBytes10<T> {
175-
t: T,
176-
}
177-
178149
// `repr(C, packed(2))` is not equivalent to `repr(C, packed)`.
179150
#[derive(IntoBytes)]
180151
#[repr(C, packed(2))]

0 commit comments

Comments
 (0)