Auto merge of #97025 - ouz-a:mini-derefer-generator, r=davidtwco

Add validation layer for Derefer

_Follow up work to #96549 #96116 #95857 #95649_

This adds validation for Derefer making sure it is always the first projection.

r? rust-lang/mir-opt

Author: bors

compiler/rustc_const_eval/src/transform/validate.rs

@@ -4,6 +4,7 @@ use rustc_data_structures::fx::FxHashSet;
 use rustc_index::bit_set::BitSet;
 use rustc_infer::infer::TyCtxtInferExt;
 use rustc_middle::mir::interpret::Scalar;
+use rustc_middle::mir::visit::NonUseContext::VarDebugInfo;
 use rustc_middle::mir::visit::{PlaceContext, Visitor};
 use rustc_middle::mir::{
     traversal, AggregateKind, BasicBlock, BinOp, Body, BorrowKind, Local, Location, MirPass,
@@ -302,9 +303,17 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
         self.super_projection_elem(local, proj_base, elem, context, location);
     }
 
-    fn visit_place(&mut self, place: &Place<'tcx>, _: PlaceContext, _: Location) {
+    fn visit_place(&mut self, place: &Place<'tcx>, cntxt: PlaceContext, location: Location) {
         // Set off any `bug!`s in the type computation code
         let _ = place.ty(&self.body.local_decls, self.tcx);
+
+        if self.mir_phase >= MirPhase::Derefered
+            && place.projection.len() > 1
+            && cntxt != PlaceContext::NonUse(VarDebugInfo)
+            && place.projection[1..].contains(&ProjectionElem::Deref)
+        {
+            self.fail(location, format!("{:?}, has deref at the wrong place", place));
+        }
     }
 
     fn visit_rvalue(&mut self, rvalue: &Rvalue<'tcx>, location: Location) {

compiler/rustc_middle/src/mir/mod.rs

@@ -172,12 +172,14 @@ pub enum MirPhase {
     /// terminator means that the auto-generated drop glue will be invoked. Also, `Copy` operands
     /// are allowed for non-`Copy` types.
     DropsLowered = 3,
+    /// After this projections may only contain deref projections as the first element.
+    Derefered = 4,
     /// Beginning with this phase, the following variant is disallowed:
     /// * [`Rvalue::Aggregate`] for any `AggregateKind` except `Array`
     ///
     /// And the following variant is allowed:
     /// * [`StatementKind::SetDiscriminant`]
-    Deaggregated = 4,
+    Deaggregated = 5,
     /// Before this phase, generators are in the "source code" form, featuring `yield` statements
     /// and such. With this phase change, they are transformed into a proper state machine. Running
     /// optimizations before this change can be potentially dangerous because the source code is to
@@ -191,8 +193,8 @@ pub enum MirPhase {
     /// Beginning with this phase, the following variants are disallowed:
     /// * [`TerminatorKind::Yield`](terminator::TerminatorKind::Yield)
     /// * [`TerminatorKind::GeneratorDrop`](terminator::TerminatorKind::GeneratorDrop)
-    GeneratorsLowered = 5,
-    Optimized = 6,
+    GeneratorsLowered = 6,
+    Optimized = 7,
 }
 
 impl MirPhase {

compiler/rustc_mir_transform/src/deref_separator.rs

@@ -1,9 +1,11 @@
 use crate::MirPass;
 use rustc_index::vec::IndexVec;
 use rustc_middle::mir::patch::MirPatch;
+use rustc_middle::mir::visit::NonUseContext::VarDebugInfo;
 use rustc_middle::mir::visit::{MutVisitor, PlaceContext};
 use rustc_middle::mir::*;
 use rustc_middle::ty::TyCtxt;
+
 pub struct Derefer;
 
 pub struct DerefChecker<'tcx> {
@@ -17,63 +19,68 @@ impl<'tcx> MutVisitor<'tcx> for DerefChecker<'tcx> {
         self.tcx
     }
 
-    fn visit_place(&mut self, place: &mut Place<'tcx>, _: PlaceContext, loc: Location) {
-        let mut place_local = place.local;
-        let mut last_len = 0;
-        let mut last_deref_idx = 0;
+    fn visit_place(&mut self, place: &mut Place<'tcx>, cntxt: PlaceContext, loc: Location) {
+        if !place.projection.is_empty()
+            && cntxt != PlaceContext::NonUse(VarDebugInfo)
+            && place.projection[1..].contains(&ProjectionElem::Deref)
+        {
+            let mut place_local = place.local;
+            let mut last_len = 0;
+            let mut last_deref_idx = 0;
 
-        let mut prev_temp: Option<Local> = None;
+            let mut prev_temp: Option<Local> = None;
 
-        for (idx, (p_ref, p_elem)) in place.iter_projections().enumerate() {
-            if p_elem == ProjectionElem::Deref && !p_ref.projection.is_empty() {
-                last_deref_idx = idx;
-            }
-        }
-
-        for (idx, (p_ref, p_elem)) in place.iter_projections().enumerate() {
-            if p_elem == ProjectionElem::Deref && !p_ref.projection.is_empty() {
-                let ty = p_ref.ty(&self.local_decls, self.tcx).ty;
-                let temp = self.patcher.new_local_with_info(
-                    ty,
-                    self.local_decls[p_ref.local].source_info.span,
-                    Some(Box::new(LocalInfo::DerefTemp)),
-                );
-
-                self.patcher.add_statement(loc, StatementKind::StorageLive(temp));
-
-                // We are adding current p_ref's projections to our
-                // temp value, excluding projections we already covered.
-                let deref_place = Place::from(place_local)
-                    .project_deeper(&p_ref.projection[last_len..], self.tcx);
-
-                self.patcher.add_assign(
-                    loc,
-                    Place::from(temp),
-                    Rvalue::Use(Operand::Move(deref_place)),
-                );
-                place_local = temp;
-                last_len = p_ref.projection.len();
-
-                // Change `Place` only if we are actually at the Place's last deref
-                if idx == last_deref_idx {
-                    let temp_place =
-                        Place::from(temp).project_deeper(&place.projection[idx..], self.tcx);
-                    *place = temp_place;
+            for (idx, elem) in place.projection[0..].iter().enumerate() {
+                if *elem == ProjectionElem::Deref {
+                    last_deref_idx = idx;
                 }
-
-                // We are destroying the previous temp since it's no longer used.
-                if let Some(prev_temp) = prev_temp {
-                    self.patcher.add_statement(loc, StatementKind::StorageDead(prev_temp));
+            }
+            for (idx, (p_ref, p_elem)) in place.iter_projections().enumerate() {
+                if !p_ref.projection.is_empty() && p_elem == ProjectionElem::Deref {
+                    let ty = p_ref.ty(&self.local_decls, self.tcx).ty;
+                    let temp = self.patcher.new_local_with_info(
+                        ty,
+                        self.local_decls[p_ref.local].source_info.span,
+                        Some(Box::new(LocalInfo::DerefTemp)),
+                    );
+
+                    self.patcher.add_statement(loc, StatementKind::StorageLive(temp));
+
+                    // We are adding current p_ref's projections to our
+                    // temp value, excluding projections we already covered.
+                    let deref_place = Place::from(place_local)
+                        .project_deeper(&p_ref.projection[last_len..], self.tcx);
+
+                    self.patcher.add_assign(
+                        loc,
+                        Place::from(temp),
+                        Rvalue::Use(Operand::Move(deref_place)),
+                    );
+                    place_local = temp;
+                    last_len = p_ref.projection.len();
+
+                    // Change `Place` only if we are actually at the Place's last deref
+                    if idx == last_deref_idx {
+                        let temp_place =
+                            Place::from(temp).project_deeper(&place.projection[idx..], self.tcx);
+                        *place = temp_place;
+                    }
+
+                    // We are destroying the previous temp since it's no longer used.
+                    if let Some(prev_temp) = prev_temp {
+                        self.patcher.add_statement(loc, StatementKind::StorageDead(prev_temp));
+                    }
+
+                    prev_temp = Some(temp);
                 }
-
-                prev_temp = Some(temp);
             }
-        }
 
-        // Since we won't be able to reach final temp, we destroy it outside the loop.
-        if let Some(prev_temp) = prev_temp {
-            let last_loc = Location { block: loc.block, statement_index: loc.statement_index + 1 };
-            self.patcher.add_statement(last_loc, StatementKind::StorageDead(prev_temp));
+            // Since we won't be able to reach final temp, we destroy it outside the loop.
+            if let Some(prev_temp) = prev_temp {
+                let last_loc =
+                    Location { block: loc.block, statement_index: loc.statement_index + 1 };
+                self.patcher.add_statement(last_loc, StatementKind::StorageDead(prev_temp));
+            }
         }
     }
 }
@@ -92,5 +99,6 @@ pub fn deref_finder<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
 impl<'tcx> MirPass<'tcx> for Derefer {
     fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
         deref_finder(tcx, body);
+        body.phase = MirPhase::Derefered;
     }
 }

compiler/rustc_mir_transform/src/generator.rs

@@ -49,6 +49,7 @@
 //! For generators with state 1 (returned) and state 2 (poisoned) it does nothing.
 //! Otherwise it drops all the values in scope at the last suspension point.
 
+use crate::deref_separator::deref_finder;
 use crate::simplify;
 use crate::util::expand_aggregate;
 use crate::MirPass;
@@ -1368,6 +1369,9 @@ impl<'tcx> MirPass<'tcx> for StateTransform {
 
         // Create the Generator::resume function
         create_generator_resume_function(tcx, transform, body, can_return);
+
+        // Run derefer to fix Derefs that are not in the first place
+        deref_finder(tcx, body);
     }
 }
 

compiler/rustc_mir_transform/src/inline.rs

@@ -1,5 +1,5 @@
 //! Inlining pass for MIR functions
-
+use crate::deref_separator::deref_finder;
 use rustc_attr::InlineAttr;
 use rustc_index::bit_set::BitSet;
 use rustc_index::vec::Idx;
@@ -53,6 +53,7 @@ impl<'tcx> MirPass<'tcx> for Inline {
             debug!("running simplify cfg on {:?}", body.source);
             CfgSimplifier::new(body).simplify();
             remove_dead_blocks(tcx, body);
+            deref_finder(tcx, body);
         }
     }
 }

src/test/mir-opt/derefer_inline_test.main.Derefer.diff

@@ -0,0 +1,69 @@
+- // MIR for `main` before Derefer
++ // MIR for `main` after Derefer
+  
+  fn main() -> () {
+      let mut _0: ();                      // return place in scope 0 at $DIR/derefer_inline_test.rs:9:11: 9:11
+      let _1: std::boxed::Box<std::boxed::Box<u32>>; // in scope 0 at $DIR/derefer_inline_test.rs:10:5: 10:12
+      let mut _2: usize;                   // in scope 0 at $DIR/derefer_inline_test.rs:10:5: 10:12
+      let mut _3: usize;                   // in scope 0 at $DIR/derefer_inline_test.rs:10:5: 10:12
+      let mut _4: *mut u8;                 // in scope 0 at $DIR/derefer_inline_test.rs:10:5: 10:12
+      let mut _5: std::boxed::Box<std::boxed::Box<u32>>; // in scope 0 at $DIR/derefer_inline_test.rs:10:5: 10:12
+      let mut _6: ();                      // in scope 0 at $DIR/derefer_inline_test.rs:10:11: 10:12
+      scope 1 {
+      }
+  
+      bb0: {
+          StorageLive(_1);                 // scope 0 at $DIR/derefer_inline_test.rs:10:5: 10:12
+          _2 = SizeOf(std::boxed::Box<u32>); // scope 1 at $DIR/derefer_inline_test.rs:10:5: 10:12
+          _3 = AlignOf(std::boxed::Box<u32>); // scope 1 at $DIR/derefer_inline_test.rs:10:5: 10:12
+          _4 = alloc::alloc::exchange_malloc(move _2, move _3) -> bb1; // scope 1 at $DIR/derefer_inline_test.rs:10:5: 10:12
+                                           // mir::Constant
+                                           // + span: $DIR/derefer_inline_test.rs:10:5: 10:12
+                                           // + literal: Const { ty: unsafe fn(usize, usize) -> *mut u8 {alloc::alloc::exchange_malloc}, val: Value(Scalar(<ZST>)) }
+      }
+  
+      bb1: {
+          StorageLive(_5);                 // scope 0 at $DIR/derefer_inline_test.rs:10:5: 10:12
+          _5 = ShallowInitBox(move _4, std::boxed::Box<u32>); // scope 0 at $DIR/derefer_inline_test.rs:10:5: 10:12
+          (*_5) = f() -> [return: bb2, unwind: bb5]; // scope 0 at $DIR/derefer_inline_test.rs:10:9: 10:12
+                                           // mir::Constant
+                                           // + span: $DIR/derefer_inline_test.rs:10:9: 10:10
+                                           // + literal: Const { ty: fn() -> Box<u32> {f}, val: Value(Scalar(<ZST>)) }
+      }
+  
+      bb2: {
+          _1 = move _5;                    // scope 0 at $DIR/derefer_inline_test.rs:10:5: 10:12
+          goto -> bb3;                     // scope 0 at $DIR/derefer_inline_test.rs:10:11: 10:12
+      }
+  
+      bb3: {
+          StorageDead(_5);                 // scope 0 at $DIR/derefer_inline_test.rs:10:11: 10:12
+          drop(_1) -> [return: bb4, unwind: bb6]; // scope 0 at $DIR/derefer_inline_test.rs:10:12: 10:13
+      }
+  
+      bb4: {
+          StorageDead(_1);                 // scope 0 at $DIR/derefer_inline_test.rs:10:12: 10:13
+          _0 = const ();                   // scope 0 at $DIR/derefer_inline_test.rs:9:11: 11:2
+          return;                          // scope 0 at $DIR/derefer_inline_test.rs:11:2: 11:2
+      }
+  
+      bb5 (cleanup): {
+          goto -> bb8;                     // scope 0 at $DIR/derefer_inline_test.rs:10:11: 10:12
+      }
+  
+      bb6 (cleanup): {
+          resume;                          // scope 0 at $DIR/derefer_inline_test.rs:9:1: 11:2
+      }
+  
+      bb7 (cleanup): {
+          _6 = alloc::alloc::box_free::<Box<u32>, std::alloc::Global>(move (_5.0: std::ptr::Unique<std::boxed::Box<u32>>), move (_5.1: std::alloc::Global)) -> bb6; // scope 0 at $DIR/derefer_inline_test.rs:10:11: 10:12
+                                           // mir::Constant
+                                           // + span: $DIR/derefer_inline_test.rs:10:11: 10:12
+                                           // + literal: Const { ty: unsafe fn(Unique<Box<u32>>, std::alloc::Global) {alloc::alloc::box_free::<Box<u32>, std::alloc::Global>}, val: Value(Scalar(<ZST>)) }
+      }
+  
+      bb8 (cleanup): {
+          goto -> bb7;                     // scope 0 at $DIR/derefer_inline_test.rs:10:11: 10:12
+      }
+  }
+  

src/test/mir-opt/derefer_inline_test.rs

@@ -0,0 +1,11 @@
+// EMIT_MIR derefer_inline_test.main.Derefer.diff
+// ignore-wasm32 compiled with panic=abort by default
+
+#![feature(box_syntax)]
+#[inline]
+fn f() -> Box<u32> {
+    box 0
+}
+fn main() {
+    box f();
+}

src/test/mir-opt/inline/dyn_trait.get_query.Inline.diff

@@ -57,6 +57,10 @@
           StorageDead(_4);                 // scope 1 at $DIR/dyn-trait.rs:34:24: 34:25
           StorageDead(_2);                 // scope 0 at $DIR/dyn-trait.rs:35:1: 35:2
           return;                          // scope 0 at $DIR/dyn-trait.rs:35:2: 35:2
++     }
++ 
++     bb3 (cleanup): {
++         resume;                          // scope 0 at $DIR/dyn-trait.rs:32:1: 35:2
       }
   }
 

src/test/mir-opt/inline/dyn_trait.try_execute_query.Inline.diff

@@ -32,6 +32,10 @@
 +         StorageDead(_4);                 // scope 1 at $DIR/dyn-trait.rs:21:21: 21:22
           StorageDead(_2);                 // scope 0 at $DIR/dyn-trait.rs:27:15: 27:16
           return;                          // scope 0 at $DIR/dyn-trait.rs:28:2: 28:2
++     }
++ 
++     bb2 (cleanup): {
++         resume;                          // scope 0 at $DIR/dyn-trait.rs:26:1: 28:2
       }
   }
 

src/test/mir-opt/inline/inline_any_operand.bar.Inline.after.mir

@@ -41,4 +41,8 @@ fn bar() -> bool {
         StorageDead(_1);                 // scope 0 at $DIR/inline-any-operand.rs:13:1: 13:2
         return;                          // scope 0 at $DIR/inline-any-operand.rs:13:2: 13:2
     }
+
+    bb1 (cleanup): {
+        resume;                          // scope 0 at $DIR/inline-any-operand.rs:10:1: 13:2
+    }
 }