diff options
| author | Corey Farwell <coreyf@rwell.org> | 2017-02-09 12:14:23 -0500 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2017-02-09 12:14:23 -0500 |
| commit | 7e2b2f30cd55cf2c506852a14ad01c4214406f52 (patch) | |
| tree | 78bfc4f4adc6f417fd17156809faeddb49a00741 | |
| parent | 7bd0da7e89faea251e77215ec70f7a5b2c26a3f8 (diff) | |
| parent | 2589f4a7511ab04a3325b0372cd2ae174940c6c8 (diff) | |
| download | rust-7e2b2f30cd55cf2c506852a14ad01c4214406f52.tar.gz rust-7e2b2f30cd55cf2c506852a14ad01c4214406f52.zip | |
Rollup merge of #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 https://github.com/rust-lang/rust/issues/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),
},
}
}
```
| -rw-r--r-- | src/test/run-pass/mir_adt_construction.rs | 61 |
1 files changed, 46 insertions, 15 deletions
diff --git a/src/test/run-pass/mir_adt_construction.rs b/src/test/run-pass/mir_adt_construction.rs index 6b47721ab4b..eb96d94efec 100644 --- a/src/test/run-pass/mir_adt_construction.rs +++ b/src/test/run-pass/mir_adt_construction.rs @@ -8,6 +8,8 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +use std::fmt; + #[repr(C)] enum CEnum { Hello = 30, @@ -15,16 +17,15 @@ enum CEnum { } fn test1(c: CEnum) -> i32 { - let c2 = CEnum::Hello; - match (c, c2) { - (CEnum::Hello, CEnum::Hello) => 42, - (CEnum::World, CEnum::Hello) => 0, - _ => 1 - } + let c2 = CEnum::Hello; + match (c, c2) { + (CEnum::Hello, CEnum::Hello) => 42, + (CEnum::World, CEnum::Hello) => 0, + _ => 1 + } } #[repr(packed)] -#[derive(PartialEq, Debug)] struct Pakd { a: u64, b: u32, @@ -33,6 +34,36 @@ struct Pakd { e: () } +// It is unsafe to use #[derive(Debug)] on a packed struct because the code generated by the derive +// macro takes references to the fields instead of accessing them directly. +impl fmt::Debug for Pakd { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + // It's important that we load the fields into locals by-value here. This will do safe + // unaligned loads into the locals, then pass references to the properly-aligned locals to + // the formatting code. + let Pakd { a, b, c, d, e } = *self; + f.debug_struct("Pakd") + .field("a", &a) + .field("b", &b) + .field("c", &c) + .field("d", &d) + .field("e", &e) + .finish() + } +} + +// It is unsafe to use #[derive(PartialEq)] on a packed struct because the code generated by the +// derive macro takes references to the fields instead of accessing them directly. +impl PartialEq for Pakd { + fn eq(&self, other: &Pakd) -> bool { + self.a == other.a && + self.b == other.b && + self.c == other.c && + self.d == other.d && + self.e == other.e + } +} + impl Drop for Pakd { fn drop(&mut self) {} } @@ -59,12 +90,12 @@ fn test5(x: fn(u32) -> Option<u32>) -> (Option<u32>, Option<u32>) { } fn main() { - assert_eq!(test1(CEnum::Hello), 42); - assert_eq!(test1(CEnum::World), 0); - assert_eq!(test2(), Pakd { a: 42, b: 42, c: 42, d: 42, e: () }); - assert_eq!(test3(), TupleLike(42, 42)); - let t4 = test4(TupleLike); - assert_eq!(t4.0, t4.1); - let t5 = test5(Some); - assert_eq!(t5.0, t5.1); + assert_eq!(test1(CEnum::Hello), 42); + assert_eq!(test1(CEnum::World), 0); + assert_eq!(test2(), Pakd { a: 42, b: 42, c: 42, d: 42, e: () }); + assert_eq!(test3(), TupleLike(42, 42)); + let t4 = test4(TupleLike); + assert_eq!(t4.0, t4.1); + let t5 = test5(Some); + assert_eq!(t5.0, t5.1); } |
