Skip to content

Commit

Permalink
Rollup merge of rust-lang#97025 - ouz-a:mini-derefer-generator, r=dav…
Browse files Browse the repository at this point in the history
…idtwco

Add validation layer for Derefer

_Follow up work to rust-lang#96549 rust-lang#96116 rust-lang#95857 #95649_

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

r? rust-lang/mir-opt
  • Loading branch information
GuillaumeGomez authored May 28, 2022
2 parents 1385c15 + 1c048eb commit 59d2231
Show file tree
Hide file tree
Showing 39 changed files with 297 additions and 59 deletions.
11 changes: 10 additions & 1 deletion compiler/rustc_const_eval/src/transform/validate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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) {
Expand Down
8 changes: 5 additions & 3 deletions compiler/rustc_middle/src/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand Down
110 changes: 59 additions & 51 deletions compiler/rustc_mir_transform/src/deref_separator.rs
Original file line number Diff line number Diff line change
@@ -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> {
Expand All @@ -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));
}
}
}
}
Expand All @@ -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;
}
}
4 changes: 4 additions & 0 deletions compiler/rustc_mir_transform/src/generator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
}

Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_mir_transform/src/inline.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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);
}
}
}
Expand Down
69 changes: 69 additions & 0 deletions src/test/mir-opt/derefer_inline_test.main.Derefer.diff
Original file line number Diff line number Diff line change
@@ -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:7:11: 7:11
let _1: std::boxed::Box<std::boxed::Box<i32>>; // in scope 0 at $DIR/derefer_inline_test.rs:8:5: 8:12
let mut _2: usize; // in scope 0 at $DIR/derefer_inline_test.rs:8:5: 8:12
let mut _3: usize; // in scope 0 at $DIR/derefer_inline_test.rs:8:5: 8:12
let mut _4: *mut u8; // in scope 0 at $DIR/derefer_inline_test.rs:8:5: 8:12
let mut _5: std::boxed::Box<std::boxed::Box<i32>>; // in scope 0 at $DIR/derefer_inline_test.rs:8:5: 8:12
let mut _6: (); // in scope 0 at $DIR/derefer_inline_test.rs:8:11: 8:12
scope 1 {
}

bb0: {
StorageLive(_1); // scope 0 at $DIR/derefer_inline_test.rs:8:5: 8:12
_2 = SizeOf(std::boxed::Box<i32>); // scope 1 at $DIR/derefer_inline_test.rs:8:5: 8:12
_3 = AlignOf(std::boxed::Box<i32>); // scope 1 at $DIR/derefer_inline_test.rs:8:5: 8:12
_4 = alloc::alloc::exchange_malloc(move _2, move _3) -> bb1; // scope 1 at $DIR/derefer_inline_test.rs:8:5: 8:12
// mir::Constant
// + span: $DIR/derefer_inline_test.rs:8:5: 8: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:8:5: 8:12
_5 = ShallowInitBox(move _4, std::boxed::Box<i32>); // scope 0 at $DIR/derefer_inline_test.rs:8:5: 8:12
(*_5) = f() -> [return: bb2, unwind: bb5]; // scope 0 at $DIR/derefer_inline_test.rs:8:9: 8:12
// mir::Constant
// + span: $DIR/derefer_inline_test.rs:8:9: 8:10
// + literal: Const { ty: fn() -> Box<i32> {f}, val: Value(Scalar(<ZST>)) }
}

bb2: {
_1 = move _5; // scope 0 at $DIR/derefer_inline_test.rs:8:5: 8:12
goto -> bb3; // scope 0 at $DIR/derefer_inline_test.rs:8:11: 8:12
}

bb3: {
StorageDead(_5); // scope 0 at $DIR/derefer_inline_test.rs:8:11: 8:12
drop(_1) -> [return: bb4, unwind: bb6]; // scope 0 at $DIR/derefer_inline_test.rs:8:12: 8:13
}

bb4: {
StorageDead(_1); // scope 0 at $DIR/derefer_inline_test.rs:8:12: 8:13
_0 = const (); // scope 0 at $DIR/derefer_inline_test.rs:7:11: 9:2
return; // scope 0 at $DIR/derefer_inline_test.rs:9:2: 9:2
}

bb5 (cleanup): {
goto -> bb8; // scope 0 at $DIR/derefer_inline_test.rs:8:11: 8:12
}

bb6 (cleanup): {
resume; // scope 0 at $DIR/derefer_inline_test.rs:7:1: 9:2
}

bb7 (cleanup): {
_6 = alloc::alloc::box_free::<Box<i32>, std::alloc::Global>(move (_5.0: std::ptr::Unique<std::boxed::Box<i32>>), move (_5.1: std::alloc::Global)) -> bb6; // scope 0 at $DIR/derefer_inline_test.rs:8:11: 8:12
// mir::Constant
// + span: $DIR/derefer_inline_test.rs:8:11: 8:12
// + literal: Const { ty: unsafe fn(Unique<Box<i32>>, std::alloc::Global) {alloc::alloc::box_free::<Box<i32>, std::alloc::Global>}, val: Value(Scalar(<ZST>)) }
}

bb8 (cleanup): {
goto -> bb7; // scope 0 at $DIR/derefer_inline_test.rs:8:11: 8:12
}
}

9 changes: 9 additions & 0 deletions src/test/mir-opt/derefer_inline_test.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// EMIT_MIR derefer_inline_test.main.Derefer.diff
#![feature(box_syntax)]
#[inline]
fn f() -> Box<i32> {
box 0
}
fn main() {
box f();
}
4 changes: 4 additions & 0 deletions src/test/mir-opt/inline/dyn_trait.get_query.Inline.diff
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
4 changes: 4 additions & 0 deletions src/test/mir-opt/inline/inline_closure.foo.Inline.after.mir
Original file line number Diff line number Diff line change
Expand Up @@ -46,4 +46,8 @@ fn foo(_1: T, _2: i32) -> i32 {
StorageDead(_3); // scope 0 at $DIR/inline-closure.rs:13:1: 13:2
return; // scope 0 at $DIR/inline-closure.rs:13:2: 13:2
}

bb1 (cleanup): {
resume; // scope 0 at $DIR/inline-closure.rs:10:1: 13:2
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -53,4 +53,8 @@ fn foo(_1: T, _2: &i32) -> i32 {
StorageDead(_3); // scope 0 at $DIR/inline-closure-borrows-arg.rs:17:1: 17:2
return; // scope 0 at $DIR/inline-closure-borrows-arg.rs:17:2: 17:2
}

bb1 (cleanup): {
resume; // scope 0 at $DIR/inline-closure-borrows-arg.rs:11:1: 17:2
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -66,4 +66,8 @@ fn foo(_1: T, _2: i32) -> (i32, T) {
StorageDead(_3); // scope 0 at $DIR/inline-closure-captures.rs:13:1: 13:2
return; // scope 0 at $DIR/inline-closure-captures.rs:13:2: 13:2
}

bb1 (cleanup): {
resume; // scope 0 at $DIR/inline-closure-captures.rs:10:1: 13:2
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@
StorageDead(_1); // scope 0 at $DIR/inline-compatibility.rs:24:18: 24:19
_0 = const (); // scope 0 at $DIR/inline-compatibility.rs:23:37: 25:2
return; // scope 0 at $DIR/inline-compatibility.rs:25:2: 25:2
+ }
+
+ bb1 (cleanup): {
+ resume; // scope 0 at $DIR/inline-compatibility.rs:23:1: 25:2
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@
StorageDead(_1); // scope 0 at $DIR/inline-compatibility.rs:13:21: 13:22
_0 = const (); // scope 0 at $DIR/inline-compatibility.rs:12:40: 14:2
return; // scope 0 at $DIR/inline-compatibility.rs:14:2: 14:2
+ }
+
+ bb1 (cleanup): {
+ resume; // scope 0 at $DIR/inline-compatibility.rs:12:1: 14:2
}
}

4 changes: 4 additions & 0 deletions src/test/mir-opt/inline/inline_cycle.one.Inline.diff
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@
StorageDead(_1); // scope 0 at $DIR/inline-cycle.rs:14:24: 14:25
_0 = const (); // scope 0 at $DIR/inline-cycle.rs:13:10: 15:2
return; // scope 0 at $DIR/inline-cycle.rs:15:2: 15:2
+ }
+
+ bb2 (cleanup): {
+ resume; // scope 0 at $DIR/inline-cycle.rs:13:1: 15:2
}
}

Loading

0 comments on commit 59d2231

Please sign in to comment.