Skip to content

Commit d6b96b6

Browse files
committed
Auto merge of #100064 - RalfJung:disaligned, r=petrochenkov
fix is_disaligned logic for nested packed structs #83605 broke the `is_disaligned` logic by bailing out of the loop in `is_within_packed` early. This PR fixes that problem and adds suitable tests. Fixes #99838
2 parents 04f72f9 + 9097ce9 commit d6b96b6

File tree

4 files changed

+162
-17
lines changed

4 files changed

+162
-17
lines changed

Diff for: compiler/rustc_const_eval/src/util/alignment.rs

+12-16
Original file line numberDiff line numberDiff line change
@@ -48,20 +48,16 @@ fn is_within_packed<'tcx, L>(
4848
where
4949
L: HasLocalDecls<'tcx>,
5050
{
51-
for (place_base, elem) in place.iter_projections().rev() {
52-
match elem {
53-
// encountered a Deref, which is ABI-aligned
54-
ProjectionElem::Deref => break,
55-
ProjectionElem::Field(..) => {
56-
let ty = place_base.ty(local_decls, tcx).ty;
57-
match ty.kind() {
58-
ty::Adt(def, _) => return def.repr().pack,
59-
_ => {}
60-
}
61-
}
62-
_ => {}
63-
}
64-
}
65-
66-
None
51+
place
52+
.iter_projections()
53+
.rev()
54+
// Stop at `Deref`; standard ABI alignment applies there.
55+
.take_while(|(_base, elem)| !matches!(elem, ProjectionElem::Deref))
56+
// Consider the packed alignments at play here...
57+
.filter_map(|(base, _elem)| {
58+
base.ty(local_decls, tcx).ty.ty_adt_def().and_then(|adt| adt.repr().pack)
59+
})
60+
// ... and compute their minimum.
61+
// The overall smallest alignment is what matters.
62+
.min()
6763
}

Diff for: src/test/ui/issues/issue-99838.rs

+40
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
// run-pass
2+
#![feature(bench_black_box)]
3+
use std::hint;
4+
5+
struct U16(u16);
6+
7+
impl Drop for U16 {
8+
fn drop(&mut self) {
9+
// Prevent LLVM from optimizing away our alignment check.
10+
assert!(hint::black_box(self as *mut U16 as usize) % 2 == 0);
11+
}
12+
}
13+
14+
struct HasDrop;
15+
16+
impl Drop for HasDrop {
17+
fn drop(&mut self) {}
18+
}
19+
20+
struct Wrapper {
21+
_a: U16,
22+
b: HasDrop,
23+
}
24+
25+
#[repr(packed)]
26+
struct Misalign(u8, Wrapper);
27+
28+
fn main() {
29+
let m = Misalign(
30+
0,
31+
Wrapper {
32+
_a: U16(10),
33+
b: HasDrop,
34+
},
35+
);
36+
// Put it somewhere definitely even (so the `a` field is definitely at an odd address).
37+
let m: ([u16; 0], Misalign) = ([], m);
38+
// Move out one field, so we run custom per-field drop logic below.
39+
let _x = m.1.1.b;
40+
}

Diff for: src/test/ui/lint/unaligned_references.rs

+53
Original file line numberDiff line numberDiff line change
@@ -47,4 +47,57 @@ fn main() {
4747
let _ = &packed2.y; // ok, has align 2 in packed(2) struct
4848
let _ = &packed2.z; // ok, has align 1
4949
}
50+
51+
unsafe {
52+
struct U16(u16);
53+
54+
impl Drop for U16 {
55+
fn drop(&mut self) {
56+
println!("{:p}", self);
57+
}
58+
}
59+
60+
struct HasDrop;
61+
62+
impl Drop for HasDrop {
63+
fn drop(&mut self) {}
64+
}
65+
66+
#[allow(unused)]
67+
struct Wrapper {
68+
a: U16,
69+
b: HasDrop,
70+
}
71+
#[allow(unused)]
72+
#[repr(packed(2))]
73+
struct Wrapper2 {
74+
a: U16,
75+
b: HasDrop,
76+
}
77+
78+
// An outer struct with more restrictive packing than the inner struct -- make sure we
79+
// notice that!
80+
#[repr(packed)]
81+
struct Misalign<T>(u8, T);
82+
83+
let m1 = Misalign(
84+
0,
85+
Wrapper {
86+
a: U16(10),
87+
b: HasDrop,
88+
},
89+
);
90+
let _ref = &m1.1.a; //~ ERROR reference to packed field
91+
//~^ previously accepted
92+
93+
let m2 = Misalign(
94+
0,
95+
Wrapper2 {
96+
a: U16(10),
97+
b: HasDrop,
98+
},
99+
);
100+
let _ref = &m2.1.a; //~ ERROR reference to packed field
101+
//~^ previously accepted
102+
}
50103
}

Diff for: src/test/ui/lint/unaligned_references.stderr

+57-1
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,29 @@ LL | let _ = &packed2.x;
8080
= note: fields of packed structs are not properly aligned, and creating a misaligned reference is undefined behavior (even if that reference is never dereferenced)
8181
= help: copy the field contents to a local variable, or replace the reference with a raw pointer and use `read_unaligned`/`write_unaligned` (loads and stores via `*p` must be properly aligned even when using raw pointers)
8282

83-
error: aborting due to 7 previous errors
83+
error: reference to packed field is unaligned
84+
--> $DIR/unaligned_references.rs:90:20
85+
|
86+
LL | let _ref = &m1.1.a;
87+
| ^^^^^^^
88+
|
89+
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
90+
= note: for more information, see issue #82523 <https://github.com/rust-lang/rust/issues/82523>
91+
= note: fields of packed structs are not properly aligned, and creating a misaligned reference is undefined behavior (even if that reference is never dereferenced)
92+
= help: copy the field contents to a local variable, or replace the reference with a raw pointer and use `read_unaligned`/`write_unaligned` (loads and stores via `*p` must be properly aligned even when using raw pointers)
93+
94+
error: reference to packed field is unaligned
95+
--> $DIR/unaligned_references.rs:100:20
96+
|
97+
LL | let _ref = &m2.1.a;
98+
| ^^^^^^^
99+
|
100+
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
101+
= note: for more information, see issue #82523 <https://github.com/rust-lang/rust/issues/82523>
102+
= note: fields of packed structs are not properly aligned, and creating a misaligned reference is undefined behavior (even if that reference is never dereferenced)
103+
= help: copy the field contents to a local variable, or replace the reference with a raw pointer and use `read_unaligned`/`write_unaligned` (loads and stores via `*p` must be properly aligned even when using raw pointers)
104+
105+
error: aborting due to 9 previous errors
84106

85107
Future incompatibility report: Future breakage diagnostic:
86108
error: reference to packed field is unaligned
@@ -201,3 +223,37 @@ LL | #![deny(unaligned_references)]
201223
= note: fields of packed structs are not properly aligned, and creating a misaligned reference is undefined behavior (even if that reference is never dereferenced)
202224
= help: copy the field contents to a local variable, or replace the reference with a raw pointer and use `read_unaligned`/`write_unaligned` (loads and stores via `*p` must be properly aligned even when using raw pointers)
203225

226+
Future breakage diagnostic:
227+
error: reference to packed field is unaligned
228+
--> $DIR/unaligned_references.rs:90:20
229+
|
230+
LL | let _ref = &m1.1.a;
231+
| ^^^^^^^
232+
|
233+
note: the lint level is defined here
234+
--> $DIR/unaligned_references.rs:1:9
235+
|
236+
LL | #![deny(unaligned_references)]
237+
| ^^^^^^^^^^^^^^^^^^^^
238+
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
239+
= note: for more information, see issue #82523 <https://github.com/rust-lang/rust/issues/82523>
240+
= note: fields of packed structs are not properly aligned, and creating a misaligned reference is undefined behavior (even if that reference is never dereferenced)
241+
= help: copy the field contents to a local variable, or replace the reference with a raw pointer and use `read_unaligned`/`write_unaligned` (loads and stores via `*p` must be properly aligned even when using raw pointers)
242+
243+
Future breakage diagnostic:
244+
error: reference to packed field is unaligned
245+
--> $DIR/unaligned_references.rs:100:20
246+
|
247+
LL | let _ref = &m2.1.a;
248+
| ^^^^^^^
249+
|
250+
note: the lint level is defined here
251+
--> $DIR/unaligned_references.rs:1:9
252+
|
253+
LL | #![deny(unaligned_references)]
254+
| ^^^^^^^^^^^^^^^^^^^^
255+
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
256+
= note: for more information, see issue #82523 <https://github.com/rust-lang/rust/issues/82523>
257+
= note: fields of packed structs are not properly aligned, and creating a misaligned reference is undefined behavior (even if that reference is never dereferenced)
258+
= help: copy the field contents to a local variable, or replace the reference with a raw pointer and use `read_unaligned`/`write_unaligned` (loads and stores via `*p` must be properly aligned even when using raw pointers)
259+

0 commit comments

Comments
 (0)