Skip to content

Commit 59bb950

Browse files
borsbors
committed
Auto merge of #103208 - cjgillot:match-fake-read, r=oli-obk,RalfJung
Allow partially moved values in match This PR attempts to unify the behaviour between `let _ = PLACE`, `let _: TY = PLACE;` and `match PLACE { _ => {} }`. The logical conclusion is that the `match` version should not check for uninitialised places nor check that borrows are still live. The `match PLACE {}` case is handled by keeping a `FakeRead` in the unreachable fallback case to verify that `PLACE` has a legal value. Schematically, `match PLACE { arms }` in surface rust becomes in MIR: ```rust PlaceMention(PLACE) match PLACE { // Decision tree for the explicit arms arms, // An extra fallback arm _ => { FakeRead(ForMatchedPlace, PLACE); unreachable } } ``` `match *borrow { _ => {} }` continues to check that `*borrow` is live, but does not read the value. `match *borrow {}` both checks that `*borrow` is live, and fake-reads the value. Continuation of ~#102256 ~#104844 Fixes #99180 #53114
2 parents 10143e7 + 479fb4b commit 59bb950

File tree

64 files changed

+394
-506
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

64 files changed

+394
-506
lines changed

‎compiler/rustc_mir_build/src/build/matches/mod.rs

+39-29
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
157157
/// [ 0. Pre-match ]
158158
/// |
159159
/// [ 1. Evaluate Scrutinee (expression being matched on) ]
160-
/// [ (fake read of scrutinee) ]
160+
/// [ (PlaceMention of scrutinee) ]
161161
/// |
162162
/// [ 2. Decision tree -- check discriminants ] <--------+
163163
/// | |
@@ -184,7 +184,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
184184
///
185185
/// We generate MIR in the following steps:
186186
///
187-
/// 1. Evaluate the scrutinee and add the fake read of it ([Builder::lower_scrutinee]).
187+
/// 1. Evaluate the scrutinee and add the PlaceMention of it ([Builder::lower_scrutinee]).
188188
/// 2. Create the decision tree ([Builder::lower_match_tree]).
189189
/// 3. Determine the fake borrows that are needed from the places that were
190190
/// matched against and create the required temporaries for them
@@ -223,6 +223,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
223223
let fake_borrow_temps = self.lower_match_tree(
224224
block,
225225
scrutinee_span,
226+
&scrutinee_place,
226227
match_start_span,
227228
match_has_guard,
228229
&mut candidates,
@@ -238,34 +239,17 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
238239
)
239240
}
240241

241-
/// Evaluate the scrutinee and add the fake read of it.
242+
/// Evaluate the scrutinee and add the PlaceMention for it.
242243
fn lower_scrutinee(
243244
&mut self,
244245
mut block: BasicBlock,
245246
scrutinee: &Expr<'tcx>,
246247
scrutinee_span: Span,
247248
) -> BlockAnd<PlaceBuilder<'tcx>> {
248249
let scrutinee_place_builder = unpack!(block = self.as_place_builder(block, scrutinee));
249-
// Matching on a `scrutinee_place` with an uninhabited type doesn't
250-
// generate any memory reads by itself, and so if the place "expression"
251-
// contains unsafe operations like raw pointer dereferences or union
252-
// field projections, we wouldn't know to require an `unsafe` block
253-
// around a `match` equivalent to `std::intrinsics::unreachable()`.
254-
// See issue #47412 for this hole being discovered in the wild.
255-
//
256-
// HACK(eddyb) Work around the above issue by adding a dummy inspection
257-
// of `scrutinee_place`, specifically by applying `ReadForMatch`.
258-
//
259-
// NOTE: ReadForMatch also checks that the scrutinee is initialized.
260-
// This is currently needed to not allow matching on an uninitialized,
261-
// uninhabited value. If we get never patterns, those will check that
262-
// the place is initialized, and so this read would only be used to
263-
// check safety.
264-
let cause_matched_place = FakeReadCause::ForMatchedPlace(None);
265-
let source_info = self.source_info(scrutinee_span);
266-
267250
if let Some(scrutinee_place) = scrutinee_place_builder.try_to_place(self) {
268-
self.cfg.push_fake_read(block, source_info, cause_matched_place, scrutinee_place);
251+
let source_info = self.source_info(scrutinee_span);
252+
self.cfg.push_place_mention(block, source_info, scrutinee_place);
269253
}
270254

271255
block.and(scrutinee_place_builder)
@@ -304,6 +288,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
304288
&mut self,
305289
block: BasicBlock,
306290
scrutinee_span: Span,
291+
scrutinee_place_builder: &PlaceBuilder<'tcx>,
307292
match_start_span: Span,
308293
match_has_guard: bool,
309294
candidates: &mut [&mut Candidate<'pat, 'tcx>],
@@ -331,6 +316,33 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
331316
// otherwise block. Match checking will ensure this is actually
332317
// unreachable.
333318
let source_info = self.source_info(scrutinee_span);
319+
320+
// Matching on a `scrutinee_place` with an uninhabited type doesn't
321+
// generate any memory reads by itself, and so if the place "expression"
322+
// contains unsafe operations like raw pointer dereferences or union
323+
// field projections, we wouldn't know to require an `unsafe` block
324+
// around a `match` equivalent to `std::intrinsics::unreachable()`.
325+
// See issue #47412 for this hole being discovered in the wild.
326+
//
327+
// HACK(eddyb) Work around the above issue by adding a dummy inspection
328+
// of `scrutinee_place`, specifically by applying `ReadForMatch`.
329+
//
330+
// NOTE: ReadForMatch also checks that the scrutinee is initialized.
331+
// This is currently needed to not allow matching on an uninitialized,
332+
// uninhabited value. If we get never patterns, those will check that
333+
// the place is initialized, and so this read would only be used to
334+
// check safety.
335+
let cause_matched_place = FakeReadCause::ForMatchedPlace(None);
336+
337+
if let Some(scrutinee_place) = scrutinee_place_builder.try_to_place(self) {
338+
self.cfg.push_fake_read(
339+
otherwise_block,
340+
source_info,
341+
cause_matched_place,
342+
scrutinee_place,
343+
);
344+
}
345+
334346
self.cfg.terminate(otherwise_block, source_info, TerminatorKind::Unreachable);
335347
}
336348

@@ -599,13 +611,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
599611
}
600612

601613
_ => {
602-
let place_builder = unpack!(block = self.as_place_builder(block, initializer));
603-
604-
if let Some(place) = place_builder.try_to_place(self) {
605-
let source_info = self.source_info(initializer.span);
606-
self.cfg.push_place_mention(block, source_info, place);
607-
}
608-
614+
let place_builder =
615+
unpack!(block = self.lower_scrutinee(block, initializer, initializer.span));
609616
self.place_into_pattern(block, &irrefutable_pat, place_builder, true)
610617
}
611618
}
@@ -622,6 +629,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
622629
let fake_borrow_temps = self.lower_match_tree(
623630
block,
624631
irrefutable_pat.span,
632+
&initializer,
625633
irrefutable_pat.span,
626634
false,
627635
&mut [&mut candidate],
@@ -1841,6 +1849,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
18411849
let fake_borrow_temps = self.lower_match_tree(
18421850
block,
18431851
pat.span,
1852+
&expr_place_builder,
18441853
pat.span,
18451854
false,
18461855
&mut [&mut guard_candidate, &mut otherwise_candidate],
@@ -2342,6 +2351,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
23422351
let fake_borrow_temps = this.lower_match_tree(
23432352
block,
23442353
initializer_span,
2354+
&scrutinee,
23452355
pattern.span,
23462356
false,
23472357
&mut [&mut candidate, &mut wildcard],

‎src/tools/miri/tests/fail/dangling_pointers/dangling_pointer_deref_match_never.rs

+17
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// Make sure we find these even with many checks disabled.
2+
//@compile-flags: -Zmiri-disable-alignment-check -Zmiri-disable-stacked-borrows -Zmiri-disable-validation
3+
4+
#![allow(unreachable_code)]
5+
#![feature(never_type)]
6+
7+
fn main() {
8+
let p = {
9+
let b = Box::new(42);
10+
&*b as *const i32 as *const !
11+
};
12+
unsafe {
13+
match *p {} //~ ERROR: entering unreachable code
14+
}
15+
panic!("this should never print");
16+
}
17+

‎src/tools/miri/tests/fail/dangling_pointers/dangling_pointer_deref_match_never.stderr

+15
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
error: Undefined Behavior: entering unreachable code
2+
--> $DIR/dangling_pointer_deref_match_never.rs:LL:CC
3+
|
4+
LL | match *p {}
5+
| ^^ entering unreachable code
6+
|
7+
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
8+
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
9+
= note: BACKTRACE:
10+
= note: inside `main` at $DIR/dangling_pointer_deref_match_never.rs:LL:CC
11+
12+
note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
13+
14+
error: aborting due to previous error
15+

‎src/tools/miri/tests/fail/never_match_never.rs

+10
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
// This should fail even without validation
2+
//@compile-flags: -Zmiri-disable-validation
3+
4+
#![feature(never_type)]
5+
#![allow(unreachable_code)]
6+
7+
fn main() {
8+
let ptr: *const (i32, !) = &0i32 as *const i32 as *const _;
9+
unsafe { match (*ptr).1 {} } //~ ERROR: entering unreachable code
10+
}

‎src/tools/miri/tests/fail/never_match_never.stderr

+15
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
error: Undefined Behavior: entering unreachable code
2+
--> $DIR/never_match_never.rs:LL:CC
3+
|
4+
LL | unsafe { match (*ptr).1 {} }
5+
| ^^^^^^^^ entering unreachable code
6+
|
7+
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
8+
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
9+
= note: BACKTRACE:
10+
= note: inside `main` at $DIR/never_match_never.rs:LL:CC
11+
12+
note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
13+
14+
error: aborting due to previous error
15+

‎src/tools/miri/tests/pass/dangling_pointer_deref_match_underscore.rs

+14
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// A `_` binding in a match is a nop, so we do not detect that the pointer is dangling.
2+
//@compile-flags: -Zmiri-disable-alignment-check -Zmiri-disable-stacked-borrows -Zmiri-disable-validation
3+
4+
fn main() {
5+
let p = {
6+
let b = Box::new(42);
7+
&*b as *const i32
8+
};
9+
unsafe {
10+
match *p {
11+
_ => {}
12+
}
13+
}
14+
}

‎src/tools/miri/tests/pass/union-uninhabited-match-underscore.rs

+17
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
fn main() {
2+
#[derive(Copy, Clone)]
3+
enum Void {}
4+
union Uninit<T: Copy> {
5+
value: T,
6+
uninit: (),
7+
}
8+
unsafe {
9+
let x: Uninit<Void> = Uninit { uninit: () };
10+
match x.value {
11+
// rustc warns about un unreachable pattern,
12+
// but is wrong in unsafe code.
13+
#[allow(unreachable_patterns)]
14+
_ => println!("hi from the void!"),
15+
}
16+
}
17+
}

‎src/tools/miri/tests/pass/union-uninhabited-match-underscore.stdout

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
hi from the void!

‎tests/mir-opt/building/async_await.b-{closure#0}.coroutine_resume.0.mir

+4
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,7 @@ fn b::{closure#0}(_1: Pin<&mut {async fn body@$DIR/async_await.rs:15:18: 18:2}>,
127127

128128
bb3: {
129129
StorageDead(_5);
130+
PlaceMention(_4);
130131
nop;
131132
(((*(_1.0: &mut {async fn body@$DIR/async_await.rs:15:18: 18:2})) as variant#3).0: {async fn body@$DIR/async_await.rs:12:14: 12:16}) = move _4;
132133
goto -> bb4;
@@ -162,6 +163,7 @@ fn b::{closure#0}(_1: Pin<&mut {async fn body@$DIR/async_await.rs:15:18: 18:2}>,
162163
bb7: {
163164
StorageDead(_13);
164165
StorageDead(_10);
166+
PlaceMention(_9);
165167
_16 = discriminant(_9);
166168
switchInt(move _16) -> [0: bb10, 1: bb8, otherwise: bb9];
167169
}
@@ -223,6 +225,7 @@ fn b::{closure#0}(_1: Pin<&mut {async fn body@$DIR/async_await.rs:15:18: 18:2}>,
223225

224226
bb15: {
225227
StorageDead(_22);
228+
PlaceMention(_21);
226229
nop;
227230
(((*(_1.0: &mut {async fn body@$DIR/async_await.rs:15:18: 18:2})) as variant#4).0: {async fn body@$DIR/async_await.rs:12:14: 12:16}) = move _21;
228231
goto -> bb16;
@@ -258,6 +261,7 @@ fn b::{closure#0}(_1: Pin<&mut {async fn body@$DIR/async_await.rs:15:18: 18:2}>,
258261
bb19: {
259262
StorageDead(_29);
260263
StorageDead(_26);
264+
PlaceMention(_25);
261265
_32 = discriminant(_25);
262266
switchInt(move _32) -> [0: bb21, 1: bb20, otherwise: bb9];
263267
}

‎tests/mir-opt/building/issue_101867.main.built.after.mir

+1-1
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ fn main() -> () {
2525
FakeRead(ForLet(None), _1);
2626
AscribeUserType(_1, o, UserTypeProjection { base: UserType(1), projs: [] });
2727
StorageLive(_5);
28-
FakeRead(ForMatchedPlace(None), _1);
28+
PlaceMention(_1);
2929
_6 = discriminant(_1);
3030
switchInt(move _6) -> [1: bb4, otherwise: bb3];
3131
}

‎tests/mir-opt/building/issue_49232.main.built.after.mir

+1-1
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ fn main() -> () {
2424
StorageLive(_2);
2525
StorageLive(_3);
2626
_3 = const true;
27-
FakeRead(ForMatchedPlace(None), _3);
27+
PlaceMention(_3);
2828
switchInt(_3) -> [0: bb3, otherwise: bb4];
2929
}
3030

‎tests/mir-opt/building/logical_or_in_conditional.test_complex.built.after.mir

+2-2
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ fn test_complex() -> () {
2323
}
2424

2525
bb1: {
26-
FakeRead(ForMatchedPlace(None), _2);
26+
PlaceMention(_2);
2727
_3 = discriminant(_2);
2828
switchInt(move _3) -> [0: bb2, otherwise: bb3];
2929
}
@@ -151,7 +151,7 @@ fn test_complex() -> () {
151151
}
152152

153153
bb25: {
154-
FakeRead(ForMatchedPlace(None), _12);
154+
PlaceMention(_12);
155155
_13 = discriminant(_12);
156156
switchInt(move _13) -> [1: bb27, otherwise: bb26];
157157
}

‎tests/mir-opt/building/match_false_edges.full_tested_match.built.after.mir

+2-1
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ fn full_tested_match() -> () {
2626
StorageLive(_1);
2727
StorageLive(_2);
2828
_2 = Option::<i32>::Some(const 42_i32);
29-
FakeRead(ForMatchedPlace(None), _2);
29+
PlaceMention(_2);
3030
_3 = discriminant(_2);
3131
switchInt(move _3) -> [0: bb1, 1: bb2, otherwise: bb4];
3232
}
@@ -45,6 +45,7 @@ fn full_tested_match() -> () {
4545
}
4646

4747
bb4: {
48+
FakeRead(ForMatchedPlace(None), _2);
4849
unreachable;
4950
}
5051

‎tests/mir-opt/building/match_false_edges.full_tested_match2.built.after.mir

+2-1
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ fn full_tested_match2() -> () {
2626
StorageLive(_1);
2727
StorageLive(_2);
2828
_2 = Option::<i32>::Some(const 42_i32);
29-
FakeRead(ForMatchedPlace(None), _2);
29+
PlaceMention(_2);
3030
_3 = discriminant(_2);
3131
switchInt(move _3) -> [0: bb1, 1: bb2, otherwise: bb4];
3232
}
@@ -51,6 +51,7 @@ fn full_tested_match2() -> () {
5151
}
5252

5353
bb4: {
54+
FakeRead(ForMatchedPlace(None), _2);
5455
unreachable;
5556
}
5657

‎tests/mir-opt/building/match_false_edges.main.built.after.mir

+1-1
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ fn main() -> () {
3737
StorageLive(_1);
3838
StorageLive(_2);
3939
_2 = Option::<i32>::Some(const 1_i32);
40-
FakeRead(ForMatchedPlace(None), _2);
40+
PlaceMention(_2);
4141
_4 = discriminant(_2);
4242
switchInt(move _4) -> [1: bb2, otherwise: bb1];
4343
}

‎tests/mir-opt/building/simple_match.match_bool.built.after.mir

+1-1
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ fn match_bool(_1: bool) -> usize {
55
let mut _0: usize;
66

77
bb0: {
8-
FakeRead(ForMatchedPlace(None), _1);
8+
PlaceMention(_1);
99
switchInt(_1) -> [0: bb2, otherwise: bb1];
1010
}
1111

‎tests/mir-opt/const_prop/transmute.unreachable_box.ConstProp.32bit.diff

+3
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
fn unreachable_box() -> ! {
55
let mut _0: !;
66
let _1: std::boxed::Box<Never>;
7+
let mut _2: *const Never;
78
scope 1 {
89
debug x => _1;
910
}
@@ -13,7 +14,9 @@
1314
bb0: {
1415
StorageLive(_1);
1516
- _1 = const 1_usize as std::boxed::Box<Never> (Transmute);
17+
- _2 = (((_1.0: std::ptr::Unique<Never>).0: std::ptr::NonNull<Never>).0: *const Never);
1618
+ _1 = const Box::<Never>(Unique::<Never> {{ pointer: NonNull::<Never> {{ pointer: {0x1 as *const Never} }}, _marker: PhantomData::<Never> }}, std::alloc::Global);
19+
+ _2 = const {0x1 as *const Never};
1720
unreachable;
1821
}
1922
}

‎tests/mir-opt/const_prop/transmute.unreachable_box.ConstProp.64bit.diff

+3
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
fn unreachable_box() -> ! {
55
let mut _0: !;
66
let _1: std::boxed::Box<Never>;
7+
let mut _2: *const Never;
78
scope 1 {
89
debug x => _1;
910
}
@@ -13,7 +14,9 @@
1314
bb0: {
1415
StorageLive(_1);
1516
- _1 = const 1_usize as std::boxed::Box<Never> (Transmute);
17+
- _2 = (((_1.0: std::ptr::Unique<Never>).0: std::ptr::NonNull<Never>).0: *const Never);
1618
+ _1 = const Box::<Never>(Unique::<Never> {{ pointer: NonNull::<Never> {{ pointer: {0x1 as *const Never} }}, _marker: PhantomData::<Never> }}, std::alloc::Global);
19+
+ _2 = const {0x1 as *const Never};
1720
unreachable;
1821
}
1922
}

0 commit comments

Comments
 (0)