Skip to content

Commit 7e2b2f3

Browse files
authored
Rollup merge of rust-lang#39682 - solson:fix-unaligned-read, r=eddyb
Fix unsafe unaligned loads in test. r? @eddyb cc @Aatch @nikomatsakis The `#[derive(PartialEq, Debug)]` impls on a packed struct contain undefined behaviour. Both generated impls take references to unaligned fields, which will fail to compile once we correctly treat that as unsafe (see rust-lang#27060). This UB was found by running the test under [Miri](https://github.com/solson/miri/) which rejects these unsafe unaligned loads. 😄 Here's a simpler example: ```rust struct Packed { a: u8, b: u64, } ``` It expands to: ```rust fn fmt(&self, __arg_0: &mut ::std::fmt::Formatter) -> ::std::fmt::Result { match *self { Packed { a: ref __self_0_0, b: ref __self_0_1 } => { // BAD: these patterns are unsafe let mut builder = __arg_0.debug_struct("Packed"); let _ = builder.field("a", &&(*__self_0_0)); let _ = builder.field("b", &&(*__self_0_1)); builder.finish() } } } ``` and ```rust fn eq(&self, __arg_0: &Packed) -> bool { match *__arg_0 { Packed { a: ref __self_1_0, b: ref __self_1_1 } => // BAD: these patterns are unsafe match *self { Packed { a: ref __self_0_0, b: ref __self_0_1 } => // BAD: these patterns are unsafe true && (*__self_0_0) == (*__self_1_0) && (*__self_0_1) == (*__self_1_1), }, } } ```
2 parents 7bd0da7 + 2589f4a commit 7e2b2f3

File tree

1 file changed

+46
-15
lines changed

1 file changed

+46
-15
lines changed

src/test/run-pass/mir_adt_construction.rs

+46-15
Original file line numberDiff line numberDiff line change
@@ -8,23 +8,24 @@
88
// option. This file may not be copied, modified, or distributed
99
// except according to those terms.
1010

11+
use std::fmt;
12+
1113
#[repr(C)]
1214
enum CEnum {
1315
Hello = 30,
1416
World = 60
1517
}
1618

1719
fn test1(c: CEnum) -> i32 {
18-
let c2 = CEnum::Hello;
19-
match (c, c2) {
20-
(CEnum::Hello, CEnum::Hello) => 42,
21-
(CEnum::World, CEnum::Hello) => 0,
22-
_ => 1
23-
}
20+
let c2 = CEnum::Hello;
21+
match (c, c2) {
22+
(CEnum::Hello, CEnum::Hello) => 42,
23+
(CEnum::World, CEnum::Hello) => 0,
24+
_ => 1
25+
}
2426
}
2527

2628
#[repr(packed)]
27-
#[derive(PartialEq, Debug)]
2829
struct Pakd {
2930
a: u64,
3031
b: u32,
@@ -33,6 +34,36 @@ struct Pakd {
3334
e: ()
3435
}
3536

37+
// It is unsafe to use #[derive(Debug)] on a packed struct because the code generated by the derive
38+
// macro takes references to the fields instead of accessing them directly.
39+
impl fmt::Debug for Pakd {
40+
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
41+
// It's important that we load the fields into locals by-value here. This will do safe
42+
// unaligned loads into the locals, then pass references to the properly-aligned locals to
43+
// the formatting code.
44+
let Pakd { a, b, c, d, e } = *self;
45+
f.debug_struct("Pakd")
46+
.field("a", &a)
47+
.field("b", &b)
48+
.field("c", &c)
49+
.field("d", &d)
50+
.field("e", &e)
51+
.finish()
52+
}
53+
}
54+
55+
// It is unsafe to use #[derive(PartialEq)] on a packed struct because the code generated by the
56+
// derive macro takes references to the fields instead of accessing them directly.
57+
impl PartialEq for Pakd {
58+
fn eq(&self, other: &Pakd) -> bool {
59+
self.a == other.a &&
60+
self.b == other.b &&
61+
self.c == other.c &&
62+
self.d == other.d &&
63+
self.e == other.e
64+
}
65+
}
66+
3667
impl Drop for Pakd {
3768
fn drop(&mut self) {}
3869
}
@@ -59,12 +90,12 @@ fn test5(x: fn(u32) -> Option<u32>) -> (Option<u32>, Option<u32>) {
5990
}
6091

6192
fn main() {
62-
assert_eq!(test1(CEnum::Hello), 42);
63-
assert_eq!(test1(CEnum::World), 0);
64-
assert_eq!(test2(), Pakd { a: 42, b: 42, c: 42, d: 42, e: () });
65-
assert_eq!(test3(), TupleLike(42, 42));
66-
let t4 = test4(TupleLike);
67-
assert_eq!(t4.0, t4.1);
68-
let t5 = test5(Some);
69-
assert_eq!(t5.0, t5.1);
93+
assert_eq!(test1(CEnum::Hello), 42);
94+
assert_eq!(test1(CEnum::World), 0);
95+
assert_eq!(test2(), Pakd { a: 42, b: 42, c: 42, d: 42, e: () });
96+
assert_eq!(test3(), TupleLike(42, 42));
97+
let t4 = test4(TupleLike);
98+
assert_eq!(t4.0, t4.1);
99+
let t5 = test5(Some);
100+
assert_eq!(t5.0, t5.1);
70101
}

0 commit comments

Comments
 (0)