about summary refs log tree commit diff
diff options
context:
space:
mode:
authorCorey Farwell <coreyf@rwell.org>2017-02-09 12:14:23 -0500
committerGitHub <noreply@github.com>2017-02-09 12:14:23 -0500
commit7e2b2f30cd55cf2c506852a14ad01c4214406f52 (patch)
tree78bfc4f4adc6f417fd17156809faeddb49a00741
parent7bd0da7e89faea251e77215ec70f7a5b2c26a3f8 (diff)
parent2589f4a7511ab04a3325b0372cd2ae174940c6c8 (diff)
downloadrust-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.rs61
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);
 }