Skip to content

Commit fdef093

Browse files
authored
Rollup merge of #139042 - compiler-errors:do-not-optimize-switchint, r=saethlin
Do not remove trivial `SwitchInt` in analysis MIR This PR ensures that we don't prematurely remove trivial `SwitchInt` terminators which affects both the borrow-checking and runtime semantics (i.e. UB) of the code. Previously the `SimplifyCfg` optimization was removing `SwitchInt` terminators when they was "trivial", i.e. when all arms branched to the same basic block, even if that `SwitchInt` terminator had the side-effect of reading an operand which (for example) may not be initialized or may point to an invalid place in memory. This behavior is unlike all other optimizations, which are only applied after "analysis" (i.e. borrow-checking) is finished, and which Miri disables to make sure the compiler doesn't silently remove UB. Fixing this code "breaks" (i.e. unmasks) code that used to borrow-check but no longer does, like: ```rust fn foo() { let x; let (0 | _) = x; } ``` This match expression should perform a read because `_` does not shadow the `0` literal pattern, and the compiler should have to read the match scrutinee to compare it to 0. I've checked that this behavior does not actually manifest in practice via a crater run which came back clean: rust-lang/rust#139042 (comment) As a side-note, it may be tempting to suggest that this is actually a good thing or that we should preserve this behavior. If we wanted to make this work (i.e. trivially optimize out reads from matches that are redundant like `0 | _`), then we should be enabling this behavior *after* fixing this. However, I think it's kinda unprincipled, and for example other variations of the code don't even work today, e.g.: ```rust fn foo() { let x; let (0.. | _) = x; } ```
2 parents 610263b + c5d9ae7 commit fdef093

File tree

3 files changed

+30
-1
lines changed

3 files changed

+30
-1
lines changed

src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ pub const MIRI_DEFAULT_ARGS: &[&str] = &[
169169
"-Zalways-encode-mir",
170170
"-Zextra-const-ub-checks",
171171
"-Zmir-emit-retag",
172-
"-Zmir-keep-place-mention",
172+
"-Zmir-preserve-ub",
173173
"-Zmir-opt-level=0",
174174
"-Zmir-enable-passes=-CheckAlignment,-CheckNull",
175175
// Deduplicating diagnostics means we miss events when tracking what happens during an
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// Ensure that we don't optimize out `SwitchInt` reads even if that terminator
2+
// branches to the same basic block on every target, since the operand may have
3+
// side-effects that affect analysis of the MIR.
4+
//
5+
// See <https://github.com/rust-lang/miri/issues/4237>.
6+
7+
use std::mem::MaybeUninit;
8+
9+
fn main() {
10+
let uninit: MaybeUninit<i32> = MaybeUninit::uninit();
11+
let bad_ref: &i32 = unsafe { uninit.assume_init_ref() };
12+
let &(0 | _) = bad_ref;
13+
//~^ ERROR: Undefined Behavior: using uninitialized data, but this operation requires initialized memory
14+
}
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
error: Undefined Behavior: using uninitialized data, but this operation requires initialized memory
2+
--> tests/fail/read_from_trivial_switch.rs:LL:CC
3+
|
4+
LL | let &(0 | _) = bad_ref;
5+
| ^^^^^^^^ using uninitialized data, but this operation requires initialized memory
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 tests/fail/read_from_trivial_switch.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 1 previous error
15+

0 commit comments

Comments
 (0)