Auto merge of #48092 - eddyb:discriminate-the-void, r=nikomatsakis

rustc_mir: insert a dummy access to places being matched on, when building MIR. Fixes #47412 by adding a `_dummy = Discriminant(place)` before each `match place {...}`. r? @nikomatsakis
rust-lang · Feb 11, 2018 · 7f2baba · 7f2baba
2 parents d0f1f42 + 8af134e
commit 7f2baba
Show file tree

Hide file tree

Showing 10 changed files with 154 additions and 74 deletions.
diff --git a/src/librustc/mir/tcx.rs b/src/librustc/mir/tcx.rs
@@ -182,9 +182,8 @@ impl<'tcx> Rvalue<'tcx> {
                 if let ty::TyAdt(adt_def, _) = ty.sty {
                     adt_def.repr.discr_type().to_ty(tcx)
                 } else {
-                    // Undefined behaviour, bug for now; may want to return something for
-                    // the `discriminant` intrinsic later.
-                    bug!("Rvalue::Discriminant on Place of type {:?}", ty);
+                    // This can only be `0`, for now, so `u8` will suffice.
+                    tcx.types.u8
                 }
             }
             Rvalue::NullaryOp(NullOp::Box, t) => tcx.mk_box(t),

diff --git a/src/librustc_mir/build/matches/mod.rs b/src/librustc_mir/build/matches/mod.rs
@@ -38,6 +38,22 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
                       -> BlockAnd<()> {
         let discriminant_place = unpack!(block = self.as_place(block, discriminant));
 
+        // Matching on a `discriminant_place` with an uninhabited type doesn't
+        // generate any memory reads by itself, and so if the place "expression"
+        // contains unsafe operations like raw pointer dereferences or union
+        // field projections, we wouldn't know to require an `unsafe` block
+        // around a `match` equivalent to `std::intrinsics::unreachable()`.
+        // See issue #47412 for this hole being discovered in the wild.
+        //
+        // HACK(eddyb) Work around the above issue by adding a dummy inspection
+        // of `discriminant_place`, specifically by applying `Rvalue::Discriminant`
+        // (which will work regardless of type) and storing the result in a temp.
+        let dummy_source_info = self.source_info(span);
+        let dummy_access = Rvalue::Discriminant(discriminant_place.clone());
+        let dummy_ty = dummy_access.ty(&self.local_decls, self.hir.tcx());
+        let dummy_temp = self.temp(dummy_ty, dummy_source_info.span);
+        self.cfg.push_assign(block, dummy_source_info, &dummy_temp, dummy_access);
+
         let mut arm_blocks = ArmBlocks {
             blocks: arms.iter()
                         .map(|_| self.cfg.start_new_block())

diff --git a/src/test/compile-fail/borrowck/borrowck-describe-lvalue.rs b/src/test/compile-fail/borrowck/borrowck-describe-lvalue.rs
@@ -72,7 +72,7 @@ fn main() {
     {
         let mut e = Baz::X(2);
         let _e0 = e.x();
-        match e {
+        match e { //[mir]~ ERROR cannot use `e` because it was mutably borrowed
             Baz::X(value) => value
             //[ast]~^ ERROR cannot use `e.0` because it was mutably borrowed
             //[mir]~^^ ERROR cannot use `e.0` because it was mutably borrowed
@@ -110,7 +110,7 @@ fn main() {
     {
         let mut e = Box::new(Baz::X(3));
         let _e0 = e.x();
-        match *e {
+        match *e { //[mir]~ ERROR cannot use `*e` because it was mutably borrowed
             Baz::X(value) => value
             //[ast]~^ ERROR cannot use `e.0` because it was mutably borrowed
             //[mir]~^^ ERROR cannot use `e.0` because it was mutably borrowed
@@ -127,25 +127,25 @@ fn main() {
     {
         let mut v = &[1, 2, 3, 4, 5, 6, 7, 8, 9, 10];
         let _v = &mut v;
-        match v {
+        match v { //[mir]~ ERROR cannot use `v` because it was mutably borrowed
             &[x, _, .., _, _] => println!("{}", x),
                 //[ast]~^ ERROR cannot use `v[..]` because it was mutably borrowed
                 //[mir]~^^ ERROR cannot use `v[..]` because it was mutably borrowed
                             _ => panic!("other case"),
         }
-        match v {
+        match v { //[mir]~ ERROR cannot use `v` because it was mutably borrowed
             &[_, x, .., _, _] => println!("{}", x),
                 //[ast]~^ ERROR cannot use `v[..]` because it was mutably borrowed
                 //[mir]~^^ ERROR cannot use `v[..]` because it was mutably borrowed
                             _ => panic!("other case"),
         }
-        match v {
+        match v { //[mir]~ ERROR cannot use `v` because it was mutably borrowed
             &[_, _, .., x, _] => println!("{}", x),
                 //[ast]~^ ERROR cannot use `v[..]` because it was mutably borrowed
                 //[mir]~^^ ERROR cannot use `v[..]` because it was mutably borrowed
                             _ => panic!("other case"),
         }
-        match v {
+        match v { //[mir]~ ERROR cannot use `v` because it was mutably borrowed
             &[_, _, .., _, x] => println!("{}", x),
                 //[ast]~^ ERROR cannot use `v[..]` because it was mutably borrowed
                 //[mir]~^^ ERROR cannot use `v[..]` because it was mutably borrowed
@@ -156,25 +156,25 @@ fn main() {
     {
         let mut v = &[1, 2, 3, 4, 5];
         let _v = &mut v;
-        match v {
+        match v { //[mir]~ ERROR cannot use `v` because it was mutably borrowed
             &[x..] => println!("{:?}", x),
                 //[ast]~^ ERROR cannot use `v[..]` because it was mutably borrowed
                 //[mir]~^^ ERROR cannot use `v[..]` because it was mutably borrowed
             _ => panic!("other case"),
         }
-        match v {
+        match v { //[mir]~ ERROR cannot use `v` because it was mutably borrowed
             &[_, x..] => println!("{:?}", x),
                 //[ast]~^ ERROR cannot use `v[..]` because it was mutably borrowed
                 //[mir]~^^ ERROR cannot use `v[..]` because it was mutably borrowed
             _ => panic!("other case"),
         }
-        match v {
+        match v { //[mir]~ ERROR cannot use `v` because it was mutably borrowed
             &[x.., _] => println!("{:?}", x),
                 //[ast]~^ ERROR cannot use `v[..]` because it was mutably borrowed
                 //[mir]~^^ ERROR cannot use `v[..]` because it was mutably borrowed
             _ => panic!("other case"),
         }
-        match v {
+        match v { //[mir]~ ERROR cannot use `v` because it was mutably borrowed
             &[_, x.., _] => println!("{:?}", x),
                 //[ast]~^ ERROR cannot use `v[..]` because it was mutably borrowed
                 //[mir]~^^ ERROR cannot use `v[..]` because it was mutably borrowed
@@ -187,7 +187,7 @@ fn main() {
 
         let mut e = E::A(3);
         let _e = &mut e;
-        match e {
+        match e { //[mir]~ ERROR cannot use `e` because it was mutably borrowed
             E::A(ref ax) =>
                 //[ast]~^ ERROR cannot borrow `e.0` as immutable because `e` is also borrowed as mutable
                 //[mir]~^^ ERROR cannot borrow `e.0` as immutable because it is also borrowed as mutable
@@ -205,14 +205,14 @@ fn main() {
         struct S { x: F, y: (u32, u32), };
         let mut s = S { x: F { x: 1, y: 2}, y: (999, 998) };
         let _s = &mut s;
-        match s {
+        match s { //[mir]~ ERROR cannot use `s` because it was mutably borrowed
             S  { y: (ref y0, _), .. } =>
                 //[ast]~^ ERROR cannot borrow `s.y.0` as immutable because `s` is also borrowed as mutable
                 //[mir]~^^ ERROR cannot borrow `s.y.0` as immutable because it is also borrowed as mutable
                 println!("y0: {:?}", y0),
             _ => panic!("other case"),
         }
-        match s {
+        match s { //[mir]~ ERROR cannot use `s` because it was mutably borrowed
             S  { x: F { y: ref x0, .. }, .. } =>
                 //[ast]~^ ERROR cannot borrow `s.x.y` as immutable because `s` is also borrowed as mutable
                 //[mir]~^^ ERROR cannot borrow `s.x.y` as immutable because it is also borrowed as mutable
@@ -263,7 +263,7 @@ fn main() {
         struct F {x: u32, y: u32};
         let mut v = &[F{x: 1, y: 2}, F{x: 3, y: 4}];
         let _v = &mut v;
-        match v {
+        match v { //[mir]~ ERROR cannot use `v` because it was mutably borrowed
             &[_, F {x: ref xf, ..}] => println!("{}", xf),
             //[mir]~^ ERROR cannot borrow `v[..].x` as immutable because it is also borrowed as mutable
             // No errors in AST

diff --git a/src/test/compile-fail/borrowck/borrowck-match-already-borrowed.rs b/src/test/compile-fail/borrowck/borrowck-match-already-borrowed.rs
@@ -19,7 +19,7 @@ enum Foo {
 fn match_enum() {
     let mut foo = Foo::B;
     let p = &mut foo;
-    let _ = match foo {
+    let _ = match foo { //[mir]~ ERROR [E0503]
         Foo::B => 1, //[mir]~ ERROR [E0503]
         _ => 2,
         Foo::A(x) => x //[ast]~ ERROR [E0503]
@@ -31,7 +31,7 @@ fn match_enum() {
 fn main() {
     let mut x = 1;
     let _x = &mut x;
-    let _ = match x {
+    let _ = match x { //[mir]~ ERROR [E0503]
         x => x + 1, //[ast]~ ERROR [E0503]
                     //[mir]~^ ERROR [E0503]
         y => y + 2, //[ast]~ ERROR [E0503]

diff --git a/src/test/compile-fail/issue-47412.rs b/src/test/compile-fail/issue-47412.rs
@@ -0,0 +1,31 @@
+// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+#[derive(Copy, Clone)]
+enum Void {}
+
+// Tests that we detect unsafe places (specifically, union fields and
+// raw pointer dereferences), even when they're matched on while having
+// an uninhabited type (equivalent to `std::intrinsics::unreachable()`).
+
+fn union_field() {
+    union Union { unit: (), void: Void }
+    let u = Union { unit: () };
+    match u.void {}
+    //~^ ERROR access to union field requires unsafe function or block
+}
+
+fn raw_ptr_deref() {
+    let ptr = std::ptr::null::<Void>();
+    match *ptr {}
+    //~^ ERROR dereference of raw pointer requires unsafe function or block
+}
+
+fn main() {}
diff --git a/src/test/mir-opt/match_false_edges.rs b/src/test/mir-opt/match_false_edges.rs
@@ -53,17 +53,18 @@ fn main() {
 //  bb0: {
 //      ...
 //      _2 = std::option::Option<i32>::Some(const 42i32,);
-//      _5 = discriminant(_2);
-//      switchInt(move _5) -> [0isize: bb6, 1isize: bb4, otherwise: bb8];
+//      _3 = discriminant(_2);
+//      _6 = discriminant(_2);
+//      switchInt(move _6) -> [0isize: bb6, 1isize: bb4, otherwise: bb8];
 //  }
 //  bb1: {
 //      resume;
 //  }
 //  bb2: {  // arm1
-//      StorageLive(_7);
-//      _7 = _3;
-//      _1 = (const 1i32, move _7);
-//      StorageDead(_7);
+//      StorageLive(_8);
+//      _8 = _4;
+//      _1 = (const 1i32, move _8);
+//      StorageDead(_8);
 //      goto -> bb13;
 //  }
 //  bb3: { // binding3(empty) and arm3
@@ -86,24 +87,24 @@ fn main() {
 //      unreachable;
 //  }
 //  bb9: { // binding1 and guard
-//      StorageLive(_3);
-//      _3 = ((_2 as Some).0: i32);
-//      StorageLive(_6);
-//      _6 = const guard() -> [return: bb10, unwind: bb1];
+//      StorageLive(_4);
+//      _4 = ((_2 as Some).0: i32);
+//      StorageLive(_7);
+//      _7 = const guard() -> [return: bb10, unwind: bb1];
 //  }
 //  bb10: { // end of guard
-//      switchInt(move _6) -> [0u8: bb11, otherwise: bb2];
+//      switchInt(move _7) -> [0u8: bb11, otherwise: bb2];
 //  }
 //  bb11: { // to pre_binding2
 //      falseEdges -> [real: bb5, imaginary: bb5];
 //  }
 //  bb12: { // bindingNoLandingPads.before.mir2 and arm2
-//      StorageLive(_4);
-//      _4 = ((_2 as Some).0: i32);
-//      StorageLive(_8);
-//      _8 = _4;
-//      _1 = (const 2i32, move _8);
-//      StorageDead(_8);
+//      StorageLive(_5);
+//      _5 = ((_2 as Some).0: i32);
+//      StorageLive(_9);
+//      _9 = _5;
+//      _1 = (const 2i32, move _9);
+//      StorageDead(_9);
 //      goto -> bb13;
 //  }
 //  bb13: {
@@ -116,17 +117,18 @@ fn main() {
 //  bb0: {
 //      ...
 //      _2 = std::option::Option<i32>::Some(const 42i32,);
-//      _5 = discriminant(_2);
-//      switchInt(move _5) -> [0isize: bb5, 1isize: bb4, otherwise: bb8];
+//      _3 = discriminant(_2);
+//      _6 = discriminant(_2);
+//      switchInt(move _6) -> [0isize: bb5, 1isize: bb4, otherwise: bb8];
 //  }
 //  bb1: {
 //      resume;
 //  }
 //  bb2: { // arm1
-//      StorageLive(_7);
-//      _7 = _3;
-//      _1 = (const 1i32, move _7);
-//      StorageDead(_7);
+//      StorageLive(_8);
+//      _8 = _4;
+//      _1 = (const 1i32, move _8);
+//      StorageDead(_8);
 //      goto -> bb13;
 //  }
 //  bb3: { // binding3(empty) and arm3
@@ -149,24 +151,24 @@ fn main() {
 //      unreachable;
 //  }
 //  bb9: { // binding1 and guard
-//      StorageLive(_3);
-//      _3 = ((_2 as Some).0: i32);
-//      StorageLive(_6);
-//      _6 = const guard() -> [return: bb10, unwind: bb1];
+//      StorageLive(_4);
+//      _4 = ((_2 as Some).0: i32);
+//      StorageLive(_7);
+//      _7 = const guard() -> [return: bb10, unwind: bb1];
 //  }
 //  bb10: { // end of guard
-//      switchInt(move _6) -> [0u8: bb11, otherwise: bb2];
+//      switchInt(move _7) -> [0u8: bb11, otherwise: bb2];
 //  }
 //  bb11: { // to pre_binding2
 //      falseEdges -> [real: bb6, imaginary: bb5];
 //  }
 //  bb12: { // binding2 and arm2
-//      StorageLive(_4);
-//      _4 = ((_2 as Some).0: i32);
-//      StorageLive(_8);
-//      _8 = _4;
-//      _1 = (const 2i32, move _8);
-//      StorageDead(_8);
+//      StorageLive(_5);
+//      _5 = ((_2 as Some).0: i32);
+//      StorageLive(_9);
+//      _9 = _5;
+//      _1 = (const 2i32, move _9);
+//      StorageDead(_9);
 //      goto -> bb13;
 //  }
 //  bb13: {
@@ -179,8 +181,9 @@ fn main() {
 // bb0: {
 //     ...
 //     _2 = std::option::Option<i32>::Some(const 1i32,);
-//     _7 = discriminant(_2);
-//     switchInt(move _7) -> [1isize: bb4, otherwise: bb5];
+//     _3 = discriminant(_2);
+//     _8 = discriminant(_2);
+//     switchInt(move _8) -> [1isize: bb4, otherwise: bb5];
 // }
 // bb1: {
 //     resume;
@@ -210,41 +213,41 @@ fn main() {
 //     unreachable;
 // }
 // bb9: { // binding1: Some(w) if guard()
-//     StorageLive(_3);
-//     _3 = ((_2 as Some).0: i32);
-//     StorageLive(_8);
-//     _8 = const guard() -> [return: bb10, unwind: bb1];
+//     StorageLive(_4);
+//     _4 = ((_2 as Some).0: i32);
+//     StorageLive(_9);
+//     _9 = const guard() -> [return: bb10, unwind: bb1];
 // }
 // bb10: { //end of guard
-//    switchInt(move _8) -> [0u8: bb11, otherwise: bb2];
+//    switchInt(move _9) -> [0u8: bb11, otherwise: bb2];
 // }
 // bb11: { // to pre_binding2
 //     falseEdges -> [real: bb5, imaginary: bb5];
 // }
 // bb12: { // binding2 & arm2
-//     StorageLive(_4);
-//     _4 = _2;
+//     StorageLive(_5);
+//     _5 = _2;
 //     _1 = const 2i32;
 //     goto -> bb17;
 // }
 // bb13: { // binding3: Some(y) if guard2(y)
-//     StorageLive(_5);
-//     _5 = ((_2 as Some).0: i32);
-//     StorageLive(_10);
+//     StorageLive(_6);
+//     _6 = ((_2 as Some).0: i32);
 //     StorageLive(_11);
-//     _11 = _5;
-//     _10 = const guard2(move _11) -> [return: bb14, unwind: bb1];
+//     StorageLive(_12);
+//     _12 = _6;
+//     _11 = const guard2(move _12) -> [return: bb14, unwind: bb1];
 // }
 // bb14: { // end of guard2
-//     StorageDead(_11);
-//     switchInt(move _10) -> [0u8: bb15, otherwise: bb3];
+//     StorageDead(_12);
+//     switchInt(move _11) -> [0u8: bb15, otherwise: bb3];
 // }
 // bb15: { // to pre_binding4
 //     falseEdges -> [real: bb7, imaginary: bb7];
 // }
 // bb16: { // binding4 & arm4
-//     StorageLive(_6);
-//     _6 = _2;
+//     StorageLive(_7);
+//     _7 = _2;
 //     _1 = const 4i32;
 //     goto -> bb17;
 // }

diff --git a/src/test/ui/borrowck/issue-41962.rs b/src/test/ui/borrowck/issue-41962.rs
@@ -18,6 +18,7 @@ pub fn main(){
         //~^ ERROR use of partially moved value: `maybe` (Ast) [E0382]
         //~| ERROR use of moved value: `(maybe as std::prelude::v1::Some).0` (Ast) [E0382]
         //~| ERROR use of moved value: `maybe` (Mir) [E0382]
+        //~| ERROR use of moved value: `maybe` (Mir) [E0382]
         //~| ERROR use of moved value: `maybe.0` (Mir) [E0382]
         }
     }