Auto merge of #95161 - JakobDegen:fix-early-otherwise-branch, r=wesleywiser

Disable almost certainly unsound early otherwise branch MIR opt

Originally thought this was just an MIR semantics issue, but it's not.

r? rust-lang/mir-opt

1 files changed, 32 insertions, 1 deletions

diff --git a/compiler/rustc_mir_transform/src/early_otherwise_branch.rs b/compiler/rustc_mir_transform/src/early_otherwise_branch.rs
index 2bf97e5d43c..d72e8d16105 100644
--- a/compiler/rustc_mir_transform/src/early_otherwise_branch.rs
+++ b/compiler/rustc_mir_transform/src/early_otherwise_branch.rs
@@ -95,7 +95,7 @@ pub struct EarlyOtherwiseBranch;
 
 impl<'tcx> MirPass<'tcx> for EarlyOtherwiseBranch {
     fn is_enabled(&self, sess: &rustc_session::Session) -> bool {
-        sess.mir_opt_level() >= 2
+        sess.mir_opt_level() >= 3 && sess.opts.debugging_opts.unsound_mir_opts
     }
 
     fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
@@ -226,6 +226,37 @@ impl<'tcx> MirPass<'tcx> for EarlyOtherwiseBranch {
 
 /// Returns true if computing the discriminant of `place` may be hoisted out of the branch
 fn may_hoist<'tcx>(tcx: TyCtxt<'tcx>, body: &Body<'tcx>, place: Place<'tcx>) -> bool {
+    // FIXME(JakobDegen): This is unsound. Someone could write code like this:
+    // ```rust
+    // let Q = val;
+    // if discriminant(P) == otherwise {
+    //     let ptr = &mut Q as *mut _ as *mut u8;
+    //     unsafe { *ptr = 10; } // Any invalid value for the type
+    // }
+    //
+    // match P {
+    //    A => match Q {
+    //        A => {
+    //            // code
+    //        }
+    //        _ => {
+    //            // don't use Q
+    //        }
+    //    }
+    //    _ => {
+    //        // don't use Q
+    //    }
+    // };
+    // ```
+    //
+    // Hoisting the `discriminant(Q)` out of the `A` arm causes us to compute the discriminant of an
+    // invalid value, which is UB.
+    //
+    // In order to fix this, we would either need to show that the discriminant computation of
+    // `place` is computed in all branches, including the `otherwise` branch, or we would need
+    // another analysis pass to determine that the place is fully initialized. It might even be best
+    // to have the hoisting be performed in a different pass and just do the CFG changing in this
+    // pass.
     for (place, proj) in place.iter_projections() {
         match proj {
             // Dereferencing in the computation of `place` might cause issues from one of two