From 5264103de4085d61a9e47c97de3a31b1f36b2dd3 Mon Sep 17 00:00:00 2001
From: Ralf Jung <post@ralfj.de>
Date: Tue, 11 Jul 2017 14:01:07 -0700
Subject: [PATCH 01/30] add new instructions for asserting when values are
valid, and to describe when we can rely on them being locked in memory
---
src/librustc/ich/impls_mir.rs | 10 ++++++++--
src/librustc/mir/mod.rs | 18 ++++++++++++++++++
src/librustc/mir/visit.rs | 15 +++++++++++++--
src/librustc_mir/dataflow/drop_flag_effects.rs | 1 +
src/librustc_mir/dataflow/impls/mod.rs | 1 +
src/librustc_mir/dataflow/move_paths/mod.rs | 1 +
src/librustc_mir/transform/qualify_consts.rs | 1 +
src/librustc_mir/transform/rustc_peek.rs | 1 +
src/librustc_mir/transform/type_check.rs | 1 +
src/librustc_passes/mir_stats.rs | 1 +
src/librustc_trans/mir/analyze.rs | 1 +
src/librustc_trans/mir/constant.rs | 1 +
src/librustc_trans/mir/statement.rs | 1 +
13 files changed, 49 insertions(+), 4 deletions(-)
diff --git a/src/librustc/ich/impls_mir.rs b/src/librustc/ich/impls_mir.rs
index 6dadb702b9f..eb0c62a1161 100644
--- a/src/librustc/ich/impls_mir.rs
+++ b/src/librustc/ich/impls_mir.rs
@@ -226,8 +226,12 @@ for mir::StatementKind<'tcx> {
mir::StatementKind::StorageDead(ref lvalue) => {
lvalue.hash_stable(hcx, hasher);
}
- mir::StatementKind::EndRegion(ref extents) => {
- extents.hash_stable(hcx, hasher);
+ mir::StatementKind::EndRegion(ref extent) => {
+ extent.hash_stable(hcx, hasher);
+ }
+ mir::StatementKind::Validate(ref op, ref lvalues) => {
+ op.hash_stable(hcx, hasher);
+ lvalues.hash_stable(hcx, hasher);
}
mir::StatementKind::Nop => {}
mir::StatementKind::InlineAsm { ref asm, ref outputs, ref inputs } => {
@@ -239,6 +243,8 @@ for mir::StatementKind<'tcx> {
}
}
+impl_stable_hash_for!(enum mir::ValidationOp { Acquire, Release, Suspend(extent) });
+
impl<'a, 'gcx, 'tcx> HashStable<StableHashingContext<'a, 'gcx, 'tcx>> for mir::Lvalue<'tcx> {
fn hash_stable<W: StableHasherResult>(&self,
hcx: &mut StableHashingContext<'a, 'gcx, 'tcx>,
diff --git a/src/librustc/mir/mod.rs b/src/librustc/mir/mod.rs
index 3dcd64af2ed..c7be58c13f8 100644
--- a/src/librustc/mir/mod.rs
+++ b/src/librustc/mir/mod.rs
@@ -818,12 +818,16 @@ pub enum StatementKind<'tcx> {
/// End the current live range for the storage of the local.
StorageDead(Lvalue<'tcx>),
+ /// Execute a piece of inline Assembly.
InlineAsm {
asm: Box<InlineAsm>,
outputs: Vec<Lvalue<'tcx>>,
inputs: Vec<Operand<'tcx>>
},
+ /// Assert the given lvalues to be valid inhabitants of their type.
+ Validate(ValidationOp, Vec<(Ty<'tcx>, Lvalue<'tcx>)>),
+
/// Mark one terminating point of an extent (i.e. static region).
/// (The starting point(s) arise implicitly from borrows.)
EndRegion(CodeExtent),
@@ -832,6 +836,13 @@ pub enum StatementKind<'tcx> {
Nop,
}
+#[derive(Copy, Clone, Debug, RustcEncodable, RustcDecodable, PartialEq, Eq)]
+pub enum ValidationOp {
+ Acquire,
+ Release,
+ Suspend(CodeExtent),
+}
+
impl<'tcx> Debug for Statement<'tcx> {
fn fmt(&self, fmt: &mut Formatter) -> fmt::Result {
use self::StatementKind::*;
@@ -839,6 +850,7 @@ impl<'tcx> Debug for Statement<'tcx> {
Assign(ref lv, ref rv) => write!(fmt, "{:?} = {:?}", lv, rv),
// (reuse lifetime rendering policy from ppaux.)
EndRegion(ref ce) => write!(fmt, "EndRegion({})", ty::ReScope(*ce)),
+ Validate(ref op, ref lvalues) => write!(fmt, "Validate({:?}, {:?})", op, lvalues),
StorageLive(ref lv) => write!(fmt, "StorageLive({:?})", lv),
StorageDead(ref lv) => write!(fmt, "StorageDead({:?})", lv),
SetDiscriminant{lvalue: ref lv, variant_index: index} => {
@@ -1505,6 +1517,10 @@ impl<'tcx> TypeFoldable<'tcx> for Statement<'tcx> {
// trait with a `fn fold_extent`.
EndRegion(ref extent) => EndRegion(extent.clone()),
+ Validate(ref op, ref lvals) =>
+ Validate(op.clone(),
+ lvals.iter().map(|ty_and_lval| ty_and_lval.fold_with(folder)).collect()),
+
Nop => Nop,
};
Statement {
@@ -1530,6 +1546,8 @@ impl<'tcx> TypeFoldable<'tcx> for Statement<'tcx> {
// trait with a `fn visit_extent`.
EndRegion(ref _extent) => false,
+ Validate(ref _op, ref lvalues) => lvalues.iter().any(|ty_and_lvalue| ty_and_lvalue.visit_with(visitor)),
+
Nop => false,
}
}
diff --git a/src/librustc/mir/visit.rs b/src/librustc/mir/visit.rs
index fd3a9f8cd2d..5284a613239 100644
--- a/src/librustc/mir/visit.rs
+++ b/src/librustc/mir/visit.rs
@@ -333,6 +333,12 @@ macro_rules! make_mir_visitor {
self.visit_assign(block, lvalue, rvalue, location);
}
StatementKind::EndRegion(_) => {}
+ StatementKind::Validate(_, ref $($mutability)* lvalues) => {
+ for & $($mutability)* (ref $($mutability)* ty, ref $($mutability)* lvalue) in lvalues {
+ self.visit_ty(ty, Lookup::Loc(location));
+ self.visit_lvalue(lvalue, LvalueContext::Validate, location);
+ }
+ }
StatementKind::SetDiscriminant{ ref $($mutability)* lvalue, .. } => {
self.visit_lvalue(lvalue, LvalueContext::Store, location);
}
@@ -784,6 +790,9 @@ pub enum LvalueContext<'tcx> {
// Starting and ending a storage live range
StorageLive,
StorageDead,
+
+ // Validation command
+ Validate,
}
impl<'tcx> LvalueContext<'tcx> {
@@ -830,7 +839,8 @@ impl<'tcx> LvalueContext<'tcx> {
LvalueContext::Borrow { kind: BorrowKind::Shared, .. } |
LvalueContext::Borrow { kind: BorrowKind::Unique, .. } |
LvalueContext::Projection(Mutability::Not) | LvalueContext::Consume |
- LvalueContext::StorageLive | LvalueContext::StorageDead => false,
+ LvalueContext::StorageLive | LvalueContext::StorageDead |
+ LvalueContext::Validate => false,
}
}
@@ -842,7 +852,8 @@ impl<'tcx> LvalueContext<'tcx> {
LvalueContext::Projection(Mutability::Not) | LvalueContext::Consume => true,
LvalueContext::Borrow { kind: BorrowKind::Mut, .. } | LvalueContext::Store |
LvalueContext::Call | LvalueContext::Projection(Mutability::Mut) |
- LvalueContext::Drop | LvalueContext::StorageLive | LvalueContext::StorageDead => false,
+ LvalueContext::Drop | LvalueContext::StorageLive | LvalueContext::StorageDead |
+ LvalueContext::Validate => false,
}
}
diff --git a/src/librustc_mir/dataflow/drop_flag_effects.rs b/src/librustc_mir/dataflow/drop_flag_effects.rs
index daafbecc5df..24d5aa9e46b 100644
--- a/src/librustc_mir/dataflow/drop_flag_effects.rs
+++ b/src/librustc_mir/dataflow/drop_flag_effects.rs
@@ -289,6 +289,7 @@ pub(crate) fn drop_flag_effects_for_location<'a, 'tcx, F>(
mir::StatementKind::StorageDead(_) |
mir::StatementKind::InlineAsm { .. } |
mir::StatementKind::EndRegion(_) |
+ mir::StatementKind::Validate(..) |
mir::StatementKind::Nop => {}
},
None => {
diff --git a/src/librustc_mir/dataflow/impls/mod.rs b/src/librustc_mir/dataflow/impls/mod.rs
index 97c996dea68..d5bdc71a705 100644
--- a/src/librustc_mir/dataflow/impls/mod.rs
+++ b/src/librustc_mir/dataflow/impls/mod.rs
@@ -486,6 +486,7 @@ impl<'a, 'tcx> BitDenotation for MovingOutStatements<'a, 'tcx> {
mir::StatementKind::StorageDead(_) |
mir::StatementKind::InlineAsm { .. } |
mir::StatementKind::EndRegion(_) |
+ mir::StatementKind::Validate(..) |
mir::StatementKind::Nop => {}
}
}
diff --git a/src/librustc_mir/dataflow/move_paths/mod.rs b/src/librustc_mir/dataflow/move_paths/mod.rs
index fbf977b98f9..c2945d46592 100644
--- a/src/librustc_mir/dataflow/move_paths/mod.rs
+++ b/src/librustc_mir/dataflow/move_paths/mod.rs
@@ -416,6 +416,7 @@ impl<'a, 'tcx> MoveDataBuilder<'a, 'tcx> {
}
StatementKind::InlineAsm { .. } |
StatementKind::EndRegion(_) |
+ StatementKind::Validate(..) |
StatementKind::Nop => {}
}
}
diff --git a/src/librustc_mir/transform/qualify_consts.rs b/src/librustc_mir/transform/qualify_consts.rs
index 9bb0f07aa68..9d01f8294e4 100644
--- a/src/librustc_mir/transform/qualify_consts.rs
+++ b/src/librustc_mir/transform/qualify_consts.rs
@@ -908,6 +908,7 @@ impl<'a, 'tcx> Visitor<'tcx> for Qualifier<'a, 'tcx, 'tcx> {
StatementKind::StorageDead(_) |
StatementKind::InlineAsm {..} |
StatementKind::EndRegion(_) |
+ StatementKind::Validate(..) |
StatementKind::Nop => {}
}
});
diff --git a/src/librustc_mir/transform/rustc_peek.rs b/src/librustc_mir/transform/rustc_peek.rs
index 5918de0c688..268e7a4c185 100644
--- a/src/librustc_mir/transform/rustc_peek.rs
+++ b/src/librustc_mir/transform/rustc_peek.rs
@@ -161,6 +161,7 @@ fn each_block<'a, 'tcx, O>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
mir::StatementKind::StorageDead(_) |
mir::StatementKind::InlineAsm { .. } |
mir::StatementKind::EndRegion(_) |
+ mir::StatementKind::Validate(..) |
mir::StatementKind::Nop => continue,
mir::StatementKind::SetDiscriminant{ .. } =>
span_bug!(stmt.source_info.span,
diff --git a/src/librustc_mir/transform/type_check.rs b/src/librustc_mir/transform/type_check.rs
index 7e6fccf3019..1c7899a46d1 100644
--- a/src/librustc_mir/transform/type_check.rs
+++ b/src/librustc_mir/transform/type_check.rs
@@ -414,6 +414,7 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> {
}
StatementKind::InlineAsm { .. } |
StatementKind::EndRegion(_) |
+ StatementKind::Validate(..) |
StatementKind::Nop => {}
}
}
diff --git a/src/librustc_passes/mir_stats.rs b/src/librustc_passes/mir_stats.rs
index 9895802700e..d5e477ff0c7 100644
--- a/src/librustc_passes/mir_stats.rs
+++ b/src/librustc_passes/mir_stats.rs
@@ -126,6 +126,7 @@ impl<'a, 'tcx> mir_visit::Visitor<'tcx> for StatCollector<'a, 'tcx> {
self.record(match statement.kind {
StatementKind::Assign(..) => "StatementKind::Assign",
StatementKind::EndRegion(..) => "StatementKind::EndRegion",
+ StatementKind::Validate(..) => "StatementKind::Validate",
StatementKind::SetDiscriminant { .. } => "StatementKind::SetDiscriminant",
StatementKind::StorageLive(..) => "StatementKind::StorageLive",
StatementKind::StorageDead(..) => "StatementKind::StorageDead",
diff --git a/src/librustc_trans/mir/analyze.rs b/src/librustc_trans/mir/analyze.rs
index 45afcf51b52..598af1cda91 100644
--- a/src/librustc_trans/mir/analyze.rs
+++ b/src/librustc_trans/mir/analyze.rs
@@ -158,6 +158,7 @@ impl<'mir, 'a, 'tcx> Visitor<'tcx> for LocalAnalyzer<'mir, 'a, 'tcx> {
LvalueContext::StorageLive |
LvalueContext::StorageDead |
+ LvalueContext::Validate |
LvalueContext::Inspect |
LvalueContext::Consume => {}
diff --git a/src/librustc_trans/mir/constant.rs b/src/librustc_trans/mir/constant.rs
index 98e774a2987..c90382673a4 100644
--- a/src/librustc_trans/mir/constant.rs
+++ b/src/librustc_trans/mir/constant.rs
@@ -285,6 +285,7 @@ impl<'a, 'tcx> MirConstContext<'a, 'tcx> {
}
mir::StatementKind::StorageLive(_) |
mir::StatementKind::StorageDead(_) |
+ mir::StatementKind::Validate(..) |
mir::StatementKind::EndRegion(_) |
mir::StatementKind::Nop => {}
mir::StatementKind::InlineAsm { .. } |
diff --git a/src/librustc_trans/mir/statement.rs b/src/librustc_trans/mir/statement.rs
index 170a76a4949..52dfc8dc4de 100644
--- a/src/librustc_trans/mir/statement.rs
+++ b/src/librustc_trans/mir/statement.rs
@@ -87,6 +87,7 @@ impl<'a, 'tcx> MirContext<'a, 'tcx> {
bcx
}
mir::StatementKind::EndRegion(_) |
+ mir::StatementKind::Validate(..) |
mir::StatementKind::Nop => bcx,
}
}
From 735ace977c75405084cb41b3b0613d14b55c811d Mon Sep 17 00:00:00 2001
From: Ralf Jung <post@ralfj.de>
Date: Tue, 11 Jul 2017 14:10:38 -0700
Subject: [PATCH 02/30] add a pass for validation commands; for now just emit
the initial AcquireValid
---
src/librustc_driver/driver.rs | 4 ++
src/librustc_mir/transform/add_validation.rs | 43 ++++++++++++++++++++
src/librustc_mir/transform/mod.rs | 1 +
3 files changed, 48 insertions(+)
create mode 100644 src/librustc_mir/transform/add_validation.rs
diff --git a/src/librustc_driver/driver.rs b/src/librustc_driver/driver.rs
index c592882a1e4..68e6b0f50d1 100644
--- a/src/librustc_driver/driver.rs
+++ b/src/librustc_driver/driver.rs
@@ -925,6 +925,10 @@ pub fn phase_3_run_analysis_passes<'tcx, F, R>(sess: &'tcx Session,
let mut passes = Passes::new();
passes.push_hook(mir::transform::dump_mir::DumpMir);
+ // Insert AcquireValid and ReleaseValid calls. Conceptually, this
+ // pass is actually part of MIR building.
+ passes.push_pass(MIR_CONST, mir::transform::add_validation::AddValidation);
+
// Remove all `EndRegion` statements that are not involved in borrows.
passes.push_pass(MIR_CONST, mir::transform::clean_end_regions::CleanEndRegions);
diff --git a/src/librustc_mir/transform/add_validation.rs b/src/librustc_mir/transform/add_validation.rs
new file mode 100644
index 00000000000..6934ec7a74f
--- /dev/null
+++ b/src/librustc_mir/transform/add_validation.rs
@@ -0,0 +1,43 @@
+// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+//! This pass adds validation calls (AcquireValid, ReleaseValid) where appropriate.
+//! It has to be run really early, before transformations like inlining, because
+//! introducing these calls *adds* UB -- so, conceptually, this pass is actually part
+//! of MIR building, and only after this pass we think of the program has having the
+//! normal MIR semantics.
+
+use rustc::ty::TyCtxt;
+use rustc::mir::*;
+use rustc::mir::transform::{MirPass, MirSource};
+
+pub struct AddValidation;
+
+impl MirPass for AddValidation {
+ fn run_pass<'a, 'tcx>(&self,
+ _tcx: TyCtxt<'a, 'tcx, 'tcx>,
+ _: MirSource,
+ mir: &mut Mir<'tcx>) {
+ // Add an AcquireValid at the beginning of the start block
+ if mir.arg_count > 0 {
+ let acquire_stmt = Statement {
+ source_info: SourceInfo {
+ scope: ARGUMENT_VISIBILITY_SCOPE,
+ span: mir.span,
+ },
+ kind: StatementKind::Validate(ValidationOp::Acquire,
+ // Skip return value, go over all the arguments
+ mir.local_decls.iter_enumerated().skip(1).take(mir.arg_count)
+ .map(|(local, local_decl)| (local_decl.ty, Lvalue::Local(local))).collect())
+ };
+ mir.basic_blocks_mut()[START_BLOCK].statements.insert(0, acquire_stmt);
+ }
+ }
+}
diff --git a/src/librustc_mir/transform/mod.rs b/src/librustc_mir/transform/mod.rs
index c9c8ad0e0eb..a247ce2231e 100644
--- a/src/librustc_mir/transform/mod.rs
+++ b/src/librustc_mir/transform/mod.rs
@@ -24,6 +24,7 @@ use syntax::ast;
use syntax_pos::{DUMMY_SP, Span};
use transform;
+pub mod add_validation;
pub mod clean_end_regions;
pub mod simplify_branches;
pub mod simplify;
From 33585f4fe11968ce652815c8a3debfdf97df6baa Mon Sep 17 00:00:00 2001
From: Ralf Jung <post@ralfj.de>
Date: Tue, 11 Jul 2017 14:30:30 -0700
Subject: [PATCH 03/30] CleanEndRegions: do not clean regions that occur in
types in validation statements
---
.../transform/clean_end_regions.rs | 33 +++++++++++++++++--
src/librustc_mir/transform/erase_regions.rs | 20 +++++++----
2 files changed, 44 insertions(+), 9 deletions(-)
diff --git a/src/librustc_mir/transform/clean_end_regions.rs b/src/librustc_mir/transform/clean_end_regions.rs
index 36125f94543..28311a5e68c 100644
--- a/src/librustc_mir/transform/clean_end_regions.rs
+++ b/src/librustc_mir/transform/clean_end_regions.rs
@@ -24,13 +24,14 @@ use rustc_data_structures::fx::FxHashSet;
use rustc::middle::region::CodeExtent;
use rustc::mir::transform::{MirPass, MirSource};
use rustc::mir::{BasicBlock, Location, Mir, Rvalue, Statement, StatementKind};
-use rustc::mir::visit::{MutVisitor, Visitor};
-use rustc::ty::{RegionKind, TyCtxt};
+use rustc::mir::visit::{MutVisitor, Visitor, Lookup};
+use rustc::ty::{Ty, RegionKind, TyCtxt};
pub struct CleanEndRegions;
struct GatherBorrowedRegions {
seen_regions: FxHashSet<CodeExtent>,
+ in_validation_statement: bool,
}
struct DeleteTrivialEndRegions<'a> {
@@ -42,7 +43,7 @@ impl MirPass for CleanEndRegions {
_tcx: TyCtxt<'a, 'tcx, 'tcx>,
_source: MirSource,
mir: &mut Mir<'tcx>) {
- let mut gather = GatherBorrowedRegions { seen_regions: FxHashSet() };
+ let mut gather = GatherBorrowedRegions { seen_regions: FxHashSet(), in_validation_statement: false };
gather.visit_mir(mir);
let mut delete = DeleteTrivialEndRegions { seen_regions: &mut gather.seen_regions };
@@ -54,6 +55,7 @@ impl<'tcx> Visitor<'tcx> for GatherBorrowedRegions {
fn visit_rvalue(&mut self,
rvalue: &Rvalue<'tcx>,
location: Location) {
+ // Gather regions that are used for borrows
if let Rvalue::Ref(r, _, _) = *rvalue {
if let RegionKind::ReScope(ce) = *r {
self.seen_regions.insert(ce);
@@ -61,6 +63,31 @@ impl<'tcx> Visitor<'tcx> for GatherBorrowedRegions {
}
self.super_rvalue(rvalue, location);
}
+
+ fn visit_statement(&mut self,
+ block: BasicBlock,
+ statement: &Statement<'tcx>,
+ location: Location) {
+ self.in_validation_statement = match statement.kind {
+ StatementKind::Validate(..) => true,
+ _ => false,
+ };
+ self.super_statement(block, statement, location);
+ self.in_validation_statement = false;
+ }
+
+ fn visit_ty(&mut self, ty: &Ty<'tcx>, _: Lookup) {
+ // Gather regions that occur in types inside AcquireValid/ReleaseValid statements
+ if self.in_validation_statement {
+ for re in ty.walk().flat_map(|t| t.regions()) {
+ match *re {
+ RegionKind::ReScope(ce) => { self.seen_regions.insert(ce); }
+ _ => {},
+ }
+ }
+ }
+ self.super_ty(ty);
+ }
}
impl<'a, 'tcx> MutVisitor<'tcx> for DeleteTrivialEndRegions<'a> {
diff --git a/src/librustc_mir/transform/erase_regions.rs b/src/librustc_mir/transform/erase_regions.rs
index da9032685e0..12b1c549ffe 100644
--- a/src/librustc_mir/transform/erase_regions.rs
+++ b/src/librustc_mir/transform/erase_regions.rs
@@ -11,6 +11,8 @@
//! This pass erases all early-bound regions from the types occuring in the MIR.
//! We want to do this once just before trans, so trans does not have to take
//! care erasing regions all over the place.
+//! NOTE: We do NOT erase regions of statements that are relevant for "types-as-contracts"-validation,
+//! namely, AcquireValid, ReleaseValid, and EndRegion.
use rustc::ty::subst::Substs;
use rustc::ty::{Ty, TyCtxt, ClosureSubsts};
@@ -20,20 +22,24 @@ use rustc::mir::transform::{MirPass, MirSource};
struct EraseRegionsVisitor<'a, 'tcx: 'a> {
tcx: TyCtxt<'a, 'tcx, 'tcx>,
+ in_validation_statement: bool,
}
impl<'a, 'tcx> EraseRegionsVisitor<'a, 'tcx> {
pub fn new(tcx: TyCtxt<'a, 'tcx, 'tcx>) -> Self {
EraseRegionsVisitor {
- tcx: tcx
+ tcx: tcx,
+ in_validation_statement: false,
}
}
}
impl<'a, 'tcx> MutVisitor<'tcx> for EraseRegionsVisitor<'a, 'tcx> {
fn visit_ty(&mut self, ty: &mut Ty<'tcx>, _: Lookup) {
- let old_ty = *ty;
- *ty = self.tcx.erase_regions(&old_ty);
+ if !self.in_validation_statement {
+ *ty = self.tcx.erase_regions(&{*ty});
+ }
+ self.super_ty(ty);
}
fn visit_substs(&mut self, substs: &mut &'tcx Substs<'tcx>, _: Location) {
@@ -71,10 +77,12 @@ impl<'a, 'tcx> MutVisitor<'tcx> for EraseRegionsVisitor<'a, 'tcx> {
block: BasicBlock,
statement: &mut Statement<'tcx>,
location: Location) {
- if let StatementKind::EndRegion(_) = statement.kind {
- statement.kind = StatementKind::Nop;
- }
+ self.in_validation_statement = match statement.kind {
+ StatementKind::Validate(..) => true,
+ _ => false,
+ };
self.super_statement(block, statement, location);
+ self.in_validation_statement = false;
}
}
From 82786b2fe12fcc0fc9d5b2e9d069460e05310787 Mon Sep 17 00:00:00 2001
From: Ralf Jung <post@ralfj.de>
Date: Tue, 11 Jul 2017 16:31:30 -0700
Subject: [PATCH 04/30] emit validation for function calls and Ref
---
src/librustc_mir/transform/add_validation.rs | 93 ++++++++++++++++++--
1 file changed, 88 insertions(+), 5 deletions(-)
diff --git a/src/librustc_mir/transform/add_validation.rs b/src/librustc_mir/transform/add_validation.rs
index 6934ec7a74f..0c9848de8fc 100644
--- a/src/librustc_mir/transform/add_validation.rs
+++ b/src/librustc_mir/transform/add_validation.rs
@@ -14,7 +14,7 @@
//! of MIR building, and only after this pass we think of the program has having the
//! normal MIR semantics.
-use rustc::ty::TyCtxt;
+use rustc::ty::{TyCtxt, RegionKind};
use rustc::mir::*;
use rustc::mir::transform::{MirPass, MirSource};
@@ -22,22 +22,105 @@ pub struct AddValidation;
impl MirPass for AddValidation {
fn run_pass<'a, 'tcx>(&self,
- _tcx: TyCtxt<'a, 'tcx, 'tcx>,
+ tcx: TyCtxt<'a, 'tcx, 'tcx>,
_: MirSource,
mir: &mut Mir<'tcx>) {
- // Add an AcquireValid at the beginning of the start block
+ // PART 1
+ // Add an AcquireValid at the beginning of the start block.
if mir.arg_count > 0 {
let acquire_stmt = Statement {
source_info: SourceInfo {
scope: ARGUMENT_VISIBILITY_SCOPE,
- span: mir.span,
+ span: mir.span, // TODO: Consider using just the span covering the function argument declaration
},
kind: StatementKind::Validate(ValidationOp::Acquire,
// Skip return value, go over all the arguments
mir.local_decls.iter_enumerated().skip(1).take(mir.arg_count)
- .map(|(local, local_decl)| (local_decl.ty, Lvalue::Local(local))).collect())
+ .map(|(local, local_decl)| (local_decl.ty, Lvalue::Local(local))).collect()
+ )
};
mir.basic_blocks_mut()[START_BLOCK].statements.insert(0, acquire_stmt);
}
+
+ // PART 2
+ // Add ReleaseValid/AcquireValid around function call terminators. We don't use a visitor because
+ // we need to access the block that a Call jumps to.
+ let mut returns : Vec<(SourceInfo, Lvalue<'tcx>, BasicBlock)> = Vec::new(); // Here we collect the destinations.
+ let local_decls = mir.local_decls.clone(); // TODO: Find a way to get rid of this clone.
+ for block_data in mir.basic_blocks_mut() {
+ match block_data.terminator {
+ Some(Terminator { kind: TerminatorKind::Call { ref args, ref destination, .. }, source_info }) => {
+ // Before the call: Release all arguments
+ let release_stmt = Statement {
+ source_info,
+ kind: StatementKind::Validate(ValidationOp::Release,
+ args.iter().filter_map(|op| {
+ match op {
+ &Operand::Consume(ref lval) => {
+ let ty = lval.ty(&local_decls, tcx).to_ty(tcx);
+ Some((ty, lval.clone()))
+ },
+ &Operand::Constant(..) => { None },
+ }
+ }).collect())
+ };
+ block_data.statements.push(release_stmt);
+ // Remember the return destination for later
+ if let &Some(ref destination) = destination {
+ returns.push((source_info, destination.0.clone(), destination.1));
+ }
+ }
+ _ => {
+ // Not a block ending in a Call -> ignore.
+ // TODO: Handle drop.
+ }
+ }
+ }
+ // Now we go over the returns we collected to acquire the return values.
+ for (source_info, dest_lval, dest_block) in returns {
+ let ty = dest_lval.ty(&local_decls, tcx).to_ty(tcx);
+ let acquire_stmt = Statement {
+ source_info,
+ kind: StatementKind::Validate(ValidationOp::Acquire, vec![(ty, dest_lval)])
+ };
+ mir.basic_blocks_mut()[dest_block].statements.insert(0, acquire_stmt);
+ }
+
+ // PART 3
+ // Add ReleaseValid/AcquireValid around Ref. Again an iterator does not seem very suited as
+ // we need to add new statements before and after each Ref.
+ for block_data in mir.basic_blocks_mut() {
+ // We want to insert statements around Ref commands as we iterate. To this end, we iterate backwards
+ // using indices.
+ for i in (0..block_data.statements.len()).rev() {
+ let (dest_lval, re, src_lval) = match block_data.statements[i].kind {
+ StatementKind::Assign(ref dest_lval, Rvalue::Ref(re, _, ref src_lval)) => {
+ (dest_lval.clone(), re, src_lval.clone())
+ },
+ _ => continue,
+ };
+ // So this is a ref, and we got all the data we wanted.
+ let dest_ty = dest_lval.ty(&local_decls, tcx).to_ty(tcx);
+ let acquire_stmt = Statement {
+ source_info: block_data.statements[i].source_info,
+ kind: StatementKind::Validate(ValidationOp::Acquire, vec![(dest_ty, dest_lval)]),
+ };
+ block_data.statements.insert(i+1, acquire_stmt);
+
+ // The source is released until the region of the borrow ends.
+ // FIXME: We have to check whether the source path was writable.
+ let src_ty = src_lval.ty(&local_decls, tcx).to_ty(tcx);
+ let op = match re {
+ &RegionKind::ReScope(ce) => ValidationOp::Suspend(ce),
+ &RegionKind::ReErased => bug!("AddValidation pass must be run before erasing lifetimes"),
+ _ => ValidationOp::Release,
+ };
+ let release_stmt = Statement {
+ source_info: block_data.statements[i].source_info,
+ kind: StatementKind::Validate(op, vec![(src_ty, src_lval)]),
+ };
+ block_data.statements.insert(i, release_stmt);
+ }
+ }
}
}
From 24a2ac9e468a18913e5238475db57f7404a37bc5 Mon Sep 17 00:00:00 2001
From: Ralf Jung <post@ralfj.de>
Date: Thu, 13 Jul 2017 22:10:14 -0700
Subject: [PATCH 05/30] add_validation: handle drop
---
src/librustc_mir/transform/add_validation.rs | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/src/librustc_mir/transform/add_validation.rs b/src/librustc_mir/transform/add_validation.rs
index 0c9848de8fc..dacc267612f 100644
--- a/src/librustc_mir/transform/add_validation.rs
+++ b/src/librustc_mir/transform/add_validation.rs
@@ -70,9 +70,19 @@ impl MirPass for AddValidation {
returns.push((source_info, destination.0.clone(), destination.1));
}
}
+ Some(Terminator { kind: TerminatorKind::Drop { location: ref lval, .. }, source_info }) |
+ Some(Terminator { kind: TerminatorKind::DropAndReplace { location: ref lval, .. }, source_info }) => {
+ // Before the call: Release all arguments
+ let ty = lval.ty(&local_decls, tcx).to_ty(tcx);
+ let release_stmt = Statement {
+ source_info,
+ kind: StatementKind::Validate(ValidationOp::Release, vec![(ty, lval.clone())])
+ };
+ block_data.statements.push(release_stmt);
+ // drop doesn't return anything, so we need no acquire.
+ }
_ => {
// Not a block ending in a Call -> ignore.
- // TODO: Handle drop.
}
}
}
@@ -108,7 +118,6 @@ impl MirPass for AddValidation {
block_data.statements.insert(i+1, acquire_stmt);
// The source is released until the region of the borrow ends.
- // FIXME: We have to check whether the source path was writable.
let src_ty = src_lval.ty(&local_decls, tcx).to_ty(tcx);
let op = match re {
&RegionKind::ReScope(ce) => ValidationOp::Suspend(ce),
From 511b88cdce696d04522aa27a741c4033d54e0bef Mon Sep 17 00:00:00 2001
From: Ralf Jung <post@ralfj.de>
Date: Thu, 20 Jul 2017 15:27:03 -0700
Subject: [PATCH 06/30] only emit Suspend validation for mutable paths
---
src/librustc_mir/transform/add_validation.rs | 53 +++++++++++++++-----
1 file changed, 41 insertions(+), 12 deletions(-)
diff --git a/src/librustc_mir/transform/add_validation.rs b/src/librustc_mir/transform/add_validation.rs
index dacc267612f..b79c1a2d6fd 100644
--- a/src/librustc_mir/transform/add_validation.rs
+++ b/src/librustc_mir/transform/add_validation.rs
@@ -20,6 +20,33 @@ use rustc::mir::transform::{MirPass, MirSource};
pub struct AddValidation;
+
+fn is_lvalue_shared<'a, 'tcx, D>(lval: &Lvalue<'tcx>, local_decls: &D, tcx: TyCtxt<'a, 'tcx, 'tcx>) -> bool
+ where D: HasLocalDecls<'tcx>
+{
+ use rustc::mir::Lvalue::*;
+
+ match *lval {
+ Local { .. } => false,
+ Static(_) => true,
+ Projection(ref proj) => {
+ // If the base is shared, things stay shared
+ if is_lvalue_shared(&proj.base, local_decls, tcx) {
+ return true;
+ }
+ // A Deref projection may make things shared
+ match proj.elem {
+ ProjectionElem::Deref => {
+ // Computing the inside the recursion makes this quadratic. We don't expect deep paths though.
+ let ty = proj.base.ty(local_decls, tcx).to_ty(tcx);
+ !ty.is_mutable_pointer()
+ }
+ _ => false,
+ }
+ }
+ }
+}
+
impl MirPass for AddValidation {
fn run_pass<'a, 'tcx>(&self,
tcx: TyCtxt<'a, 'tcx, 'tcx>,
@@ -117,18 +144,20 @@ impl MirPass for AddValidation {
};
block_data.statements.insert(i+1, acquire_stmt);
- // The source is released until the region of the borrow ends.
- let src_ty = src_lval.ty(&local_decls, tcx).to_ty(tcx);
- let op = match re {
- &RegionKind::ReScope(ce) => ValidationOp::Suspend(ce),
- &RegionKind::ReErased => bug!("AddValidation pass must be run before erasing lifetimes"),
- _ => ValidationOp::Release,
- };
- let release_stmt = Statement {
- source_info: block_data.statements[i].source_info,
- kind: StatementKind::Validate(op, vec![(src_ty, src_lval)]),
- };
- block_data.statements.insert(i, release_stmt);
+ // The source is released until the region of the borrow ends -- but not if it is shared.
+ if !is_lvalue_shared(&src_lval, &local_decls, tcx) {
+ let src_ty = src_lval.ty(&local_decls, tcx).to_ty(tcx);
+ let op = match re {
+ &RegionKind::ReScope(ce) => ValidationOp::Suspend(ce),
+ &RegionKind::ReErased => bug!("AddValidation pass must be run before erasing lifetimes"),
+ _ => ValidationOp::Release,
+ };
+ let release_stmt = Statement {
+ source_info: block_data.statements[i].source_info,
+ kind: StatementKind::Validate(op, vec![(src_ty, src_lval)]),
+ };
+ block_data.statements.insert(i, release_stmt);
+ }
}
}
}
From a233afa794763846a8d970ecedf763350cc2c067 Mon Sep 17 00:00:00 2001
From: Ralf Jung <post@ralfj.de>
Date: Thu, 20 Jul 2017 15:27:26 -0700
Subject: [PATCH 07/30] respect lifetime rendering when rendering Suspend
validation op
---
src/librustc/mir/mod.rs | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/src/librustc/mir/mod.rs b/src/librustc/mir/mod.rs
index c7be58c13f8..dcab476ec23 100644
--- a/src/librustc/mir/mod.rs
+++ b/src/librustc/mir/mod.rs
@@ -836,13 +836,25 @@ pub enum StatementKind<'tcx> {
Nop,
}
-#[derive(Copy, Clone, Debug, RustcEncodable, RustcDecodable, PartialEq, Eq)]
+#[derive(Copy, Clone, RustcEncodable, RustcDecodable, PartialEq, Eq)]
pub enum ValidationOp {
Acquire,
Release,
Suspend(CodeExtent),
}
+impl Debug for ValidationOp {
+ fn fmt(&self, fmt: &mut Formatter) -> fmt::Result {
+ use self::ValidationOp::*;
+ match *self {
+ Acquire => write!(fmt, "Acquire"),
+ Release => write!(fmt, "Release"),
+ // (reuse lifetime rendering policy from ppaux.)
+ Suspend(ref ce) => write!(fmt, "Suspend({})", ty::ReScope(*ce)),
+ }
+ }
+}
+
impl<'tcx> Debug for Statement<'tcx> {
fn fmt(&self, fmt: &mut Formatter) -> fmt::Result {
use self::StatementKind::*;
From 60096b9e8259ba227a0a85fc1a16dca5d3fd2217 Mon Sep 17 00:00:00 2001
From: Ralf Jung <post@ralfj.de>
Date: Fri, 21 Jul 2017 12:43:09 -0700
Subject: [PATCH 08/30] when suspending, we need to specify for which lifetime
to recover
This matters if the lvalues that is suspended involves Deref'ing a reference --
that reference's lifetime will then not be in the type any more
---
src/librustc/hir/mod.rs | 10 ++
src/librustc/ich/impls_mir.rs | 2 +
src/librustc/mir/mod.rs | 43 +++++-
src/librustc/mir/visit.rs | 7 +-
src/librustc_mir/transform/add_validation.rs | 141 ++++++++++++-------
5 files changed, 147 insertions(+), 56 deletions(-)
diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs
index a3a133daa09..cc0d49c1a36 100644
--- a/src/librustc/hir/mod.rs
+++ b/src/librustc/hir/mod.rs
@@ -684,6 +684,16 @@ pub enum Mutability {
MutImmutable,
}
+impl Mutability {
+ /// Return MutMutable only if both arguments are mutable.
+ pub fn and(self, other: Self) -> Self {
+ match self {
+ MutMutable => other,
+ MutImmutable => MutImmutable,
+ }
+ }
+}
+
#[derive(Clone, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug, Copy)]
pub enum BinOp_ {
/// The `+` operator (addition)
diff --git a/src/librustc/ich/impls_mir.rs b/src/librustc/ich/impls_mir.rs
index eb0c62a1161..dc41f981ed5 100644
--- a/src/librustc/ich/impls_mir.rs
+++ b/src/librustc/ich/impls_mir.rs
@@ -243,6 +243,8 @@ for mir::StatementKind<'tcx> {
}
}
+impl_stable_hash_for!(struct mir::ValidationOperand<'tcx> { lval, ty, re, mutbl });
+
impl_stable_hash_for!(enum mir::ValidationOp { Acquire, Release, Suspend(extent) });
impl<'a, 'gcx, 'tcx> HashStable<StableHashingContext<'a, 'gcx, 'tcx>> for mir::Lvalue<'tcx> {
diff --git a/src/librustc/mir/mod.rs b/src/librustc/mir/mod.rs
index dcab476ec23..4655f8a9c15 100644
--- a/src/librustc/mir/mod.rs
+++ b/src/librustc/mir/mod.rs
@@ -25,7 +25,7 @@ use ty::{self, AdtDef, ClosureSubsts, Region, Ty};
use ty::fold::{TypeFoldable, TypeFolder, TypeVisitor};
use util::ppaux;
use rustc_back::slice;
-use hir::InlineAsm;
+use hir::{self, InlineAsm};
use std::ascii;
use std::borrow::{Cow};
use std::cell::Ref;
@@ -826,7 +826,7 @@ pub enum StatementKind<'tcx> {
},
/// Assert the given lvalues to be valid inhabitants of their type.
- Validate(ValidationOp, Vec<(Ty<'tcx>, Lvalue<'tcx>)>),
+ Validate(ValidationOp, Vec<ValidationOperand<'tcx>>),
/// Mark one terminating point of an extent (i.e. static region).
/// (The starting point(s) arise implicitly from borrows.)
@@ -855,6 +855,28 @@ impl Debug for ValidationOp {
}
}
+#[derive(Clone, RustcEncodable, RustcDecodable)]
+pub struct ValidationOperand<'tcx> {
+ pub lval: Lvalue<'tcx>,
+ pub ty: Ty<'tcx>,
+ pub re: Option<CodeExtent>,
+ pub mutbl: hir::Mutability,
+}
+
+impl<'tcx> Debug for ValidationOperand<'tcx> {
+ fn fmt(&self, fmt: &mut Formatter) -> fmt::Result {
+ write!(fmt, "{:?}@{:?}", self.lval, self.ty)?;
+ if let Some(ce) = self.re {
+ // (reuse lifetime rendering policy from ppaux.)
+ write!(fmt, "/{}", ty::ReScope(ce))?;
+ }
+ if let hir::MutImmutable = self.mutbl {
+ write!(fmt, " (imm)")?;
+ }
+ Ok(())
+ }
+}
+
impl<'tcx> Debug for Statement<'tcx> {
fn fmt(&self, fmt: &mut Formatter) -> fmt::Result {
use self::StatementKind::*;
@@ -1505,6 +1527,21 @@ impl<'tcx> TypeFoldable<'tcx> for BasicBlockData<'tcx> {
}
}
+impl<'tcx> TypeFoldable<'tcx> for ValidationOperand<'tcx> {
+ fn super_fold_with<'gcx: 'tcx, F: TypeFolder<'gcx, 'tcx>>(&self, folder: &mut F) -> Self {
+ ValidationOperand {
+ lval: self.lval.fold_with(folder),
+ ty: self.ty.fold_with(folder),
+ re: self.re,
+ mutbl: self.mutbl,
+ }
+ }
+
+ fn super_visit_with<V: TypeVisitor<'tcx>>(&self, visitor: &mut V) -> bool {
+ self.lval.visit_with(visitor) || self.ty.visit_with(visitor)
+ }
+}
+
impl<'tcx> TypeFoldable<'tcx> for Statement<'tcx> {
fn super_fold_with<'gcx: 'tcx, F: TypeFolder<'gcx, 'tcx>>(&self, folder: &mut F) -> Self {
use mir::StatementKind::*;
@@ -1531,7 +1568,7 @@ impl<'tcx> TypeFoldable<'tcx> for Statement<'tcx> {
Validate(ref op, ref lvals) =>
Validate(op.clone(),
- lvals.iter().map(|ty_and_lval| ty_and_lval.fold_with(folder)).collect()),
+ lvals.iter().map(|operand| operand.fold_with(folder)).collect()),
Nop => Nop,
};
diff --git a/src/librustc/mir/visit.rs b/src/librustc/mir/visit.rs
index 5284a613239..a05007503ce 100644
--- a/src/librustc/mir/visit.rs
+++ b/src/librustc/mir/visit.rs
@@ -334,9 +334,10 @@ macro_rules! make_mir_visitor {
}
StatementKind::EndRegion(_) => {}
StatementKind::Validate(_, ref $($mutability)* lvalues) => {
- for & $($mutability)* (ref $($mutability)* ty, ref $($mutability)* lvalue) in lvalues {
- self.visit_ty(ty, Lookup::Loc(location));
- self.visit_lvalue(lvalue, LvalueContext::Validate, location);
+ for operand in lvalues {
+ self.visit_lvalue(& $($mutability)* operand.lval,
+ LvalueContext::Validate, location);
+ self.visit_ty(& $($mutability)* operand.ty, Lookup::Loc(location));
}
}
StatementKind::SetDiscriminant{ ref $($mutability)* lvalue, .. } => {
diff --git a/src/librustc_mir/transform/add_validation.rs b/src/librustc_mir/transform/add_validation.rs
index b79c1a2d6fd..1fe16fb98f2 100644
--- a/src/librustc_mir/transform/add_validation.rs
+++ b/src/librustc_mir/transform/add_validation.rs
@@ -14,34 +14,67 @@
//! of MIR building, and only after this pass we think of the program has having the
//! normal MIR semantics.
-use rustc::ty::{TyCtxt, RegionKind};
+use rustc::ty::{self, TyCtxt, RegionKind};
+use rustc::hir;
use rustc::mir::*;
use rustc::mir::transform::{MirPass, MirSource};
+use rustc::middle::region::CodeExtent;
pub struct AddValidation;
-
-fn is_lvalue_shared<'a, 'tcx, D>(lval: &Lvalue<'tcx>, local_decls: &D, tcx: TyCtxt<'a, 'tcx, 'tcx>) -> bool
+/// Determine the "context" of the lval: Mutability and region.
+fn lval_context<'a, 'tcx, D>(
+ lval: &Lvalue<'tcx>,
+ local_decls: &D,
+ tcx: TyCtxt<'a, 'tcx, 'tcx>
+) -> (Option<CodeExtent>, hir::Mutability)
where D: HasLocalDecls<'tcx>
{
use rustc::mir::Lvalue::*;
match *lval {
- Local { .. } => false,
- Static(_) => true,
+ Local { .. } => (None, hir::MutMutable),
+ Static(_) => (None, hir::MutImmutable),
Projection(ref proj) => {
- // If the base is shared, things stay shared
- if is_lvalue_shared(&proj.base, local_decls, tcx) {
- return true;
- }
- // A Deref projection may make things shared
match proj.elem {
ProjectionElem::Deref => {
- // Computing the inside the recursion makes this quadratic. We don't expect deep paths though.
+ // Computing the inside the recursion makes this quadratic.
+ // We don't expect deep paths though.
let ty = proj.base.ty(local_decls, tcx).to_ty(tcx);
- !ty.is_mutable_pointer()
+ // A Deref projection may restrict the context, this depends on the type
+ // being deref'd.
+ let context = match ty.sty {
+ ty::TyRef(re, tam) => {
+ let re = match re {
+ &RegionKind::ReScope(ce) => Some(ce),
+ &RegionKind::ReErased =>
+ bug!("AddValidation pass must be run before erasing lifetimes"),
+ _ => None
+ };
+ (re, tam.mutbl)
+ }
+ ty::TyRawPtr(_) =>
+ // There is no guarantee behind even a mutable raw pointer,
+ // no write locks are acquired there, so we also don't want to
+ // release any.
+ (None, hir::MutImmutable),
+ ty::TyAdt(adt, _) if adt.is_box() => (None, hir::MutMutable),
+ _ => bug!("Deref on a non-pointer type {:?}", ty),
+ };
+ // "Intersect" this restriction with proj.base.
+ if let (Some(_), hir::MutImmutable) = context {
+ // This is already as restricted as it gets, no need to even recurse
+ context
+ } else {
+ let base_context = lval_context(&proj.base, local_decls, tcx);
+ // The region of the outermost Deref is always most restrictive.
+ let re = context.0.or(base_context.0);
+ let mutbl = context.1.and(base_context.1);
+ (re, mutbl)
+ }
+
}
- _ => false,
+ _ => lval_context(&proj.base, local_decls, tcx),
}
}
}
@@ -52,41 +85,49 @@ impl MirPass for AddValidation {
tcx: TyCtxt<'a, 'tcx, 'tcx>,
_: MirSource,
mir: &mut Mir<'tcx>) {
+ let local_decls = mir.local_decls.clone(); // TODO: Find a way to get rid of this clone.
+
+ /// Convert an lvalue to a validation operand.
+ let lval_to_operand = |lval: Lvalue<'tcx>| -> ValidationOperand<'tcx> {
+ let (re, mutbl) = lval_context(&lval, &local_decls, tcx);
+ let ty = lval.ty(&local_decls, tcx).to_ty(tcx);
+ ValidationOperand { lval, ty, re, mutbl }
+ };
+
// PART 1
// Add an AcquireValid at the beginning of the start block.
if mir.arg_count > 0 {
let acquire_stmt = Statement {
source_info: SourceInfo {
scope: ARGUMENT_VISIBILITY_SCOPE,
- span: mir.span, // TODO: Consider using just the span covering the function argument declaration
+ span: mir.span, // TODO: Consider using just the span covering the function
+ // argument declaration.
},
kind: StatementKind::Validate(ValidationOp::Acquire,
// Skip return value, go over all the arguments
mir.local_decls.iter_enumerated().skip(1).take(mir.arg_count)
- .map(|(local, local_decl)| (local_decl.ty, Lvalue::Local(local))).collect()
+ .map(|(local, _)| lval_to_operand(Lvalue::Local(local))).collect()
)
};
mir.basic_blocks_mut()[START_BLOCK].statements.insert(0, acquire_stmt);
}
// PART 2
- // Add ReleaseValid/AcquireValid around function call terminators. We don't use a visitor because
- // we need to access the block that a Call jumps to.
- let mut returns : Vec<(SourceInfo, Lvalue<'tcx>, BasicBlock)> = Vec::new(); // Here we collect the destinations.
- let local_decls = mir.local_decls.clone(); // TODO: Find a way to get rid of this clone.
+ // Add ReleaseValid/AcquireValid around function call terminators. We don't use a visitor
+ // because we need to access the block that a Call jumps to.
+ let mut returns : Vec<(SourceInfo, Lvalue<'tcx>, BasicBlock)> = Vec::new();
for block_data in mir.basic_blocks_mut() {
match block_data.terminator {
- Some(Terminator { kind: TerminatorKind::Call { ref args, ref destination, .. }, source_info }) => {
+ Some(Terminator { kind: TerminatorKind::Call { ref args, ref destination, .. },
+ source_info }) => {
// Before the call: Release all arguments
let release_stmt = Statement {
source_info,
kind: StatementKind::Validate(ValidationOp::Release,
args.iter().filter_map(|op| {
match op {
- &Operand::Consume(ref lval) => {
- let ty = lval.ty(&local_decls, tcx).to_ty(tcx);
- Some((ty, lval.clone()))
- },
+ &Operand::Consume(ref lval) =>
+ Some(lval_to_operand(lval.clone())),
&Operand::Constant(..) => { None },
}
}).collect())
@@ -97,13 +138,15 @@ impl MirPass for AddValidation {
returns.push((source_info, destination.0.clone(), destination.1));
}
}
- Some(Terminator { kind: TerminatorKind::Drop { location: ref lval, .. }, source_info }) |
- Some(Terminator { kind: TerminatorKind::DropAndReplace { location: ref lval, .. }, source_info }) => {
+ Some(Terminator { kind: TerminatorKind::Drop { location: ref lval, .. },
+ source_info }) |
+ Some(Terminator { kind: TerminatorKind::DropAndReplace { location: ref lval, .. },
+ source_info }) => {
// Before the call: Release all arguments
- let ty = lval.ty(&local_decls, tcx).to_ty(tcx);
let release_stmt = Statement {
source_info,
- kind: StatementKind::Validate(ValidationOp::Release, vec![(ty, lval.clone())])
+ kind: StatementKind::Validate(ValidationOp::Release,
+ vec![lval_to_operand(lval.clone())]),
};
block_data.statements.push(release_stmt);
// drop doesn't return anything, so we need no acquire.
@@ -115,20 +158,20 @@ impl MirPass for AddValidation {
}
// Now we go over the returns we collected to acquire the return values.
for (source_info, dest_lval, dest_block) in returns {
- let ty = dest_lval.ty(&local_decls, tcx).to_ty(tcx);
let acquire_stmt = Statement {
source_info,
- kind: StatementKind::Validate(ValidationOp::Acquire, vec![(ty, dest_lval)])
+ kind: StatementKind::Validate(ValidationOp::Acquire,
+ vec![lval_to_operand(dest_lval)]),
};
mir.basic_blocks_mut()[dest_block].statements.insert(0, acquire_stmt);
}
// PART 3
- // Add ReleaseValid/AcquireValid around Ref. Again an iterator does not seem very suited as
- // we need to add new statements before and after each Ref.
+ // Add ReleaseValid/AcquireValid around Ref. Again an iterator does not seem very suited
+ // as we need to add new statements before and after each Ref.
for block_data in mir.basic_blocks_mut() {
- // We want to insert statements around Ref commands as we iterate. To this end, we iterate backwards
- // using indices.
+ // We want to insert statements around Ref commands as we iterate. To this end, we
+ // iterate backwards using indices.
for i in (0..block_data.statements.len()).rev() {
let (dest_lval, re, src_lval) = match block_data.statements[i].kind {
StatementKind::Assign(ref dest_lval, Rvalue::Ref(re, _, ref src_lval)) => {
@@ -137,27 +180,25 @@ impl MirPass for AddValidation {
_ => continue,
};
// So this is a ref, and we got all the data we wanted.
- let dest_ty = dest_lval.ty(&local_decls, tcx).to_ty(tcx);
let acquire_stmt = Statement {
source_info: block_data.statements[i].source_info,
- kind: StatementKind::Validate(ValidationOp::Acquire, vec![(dest_ty, dest_lval)]),
+ kind: StatementKind::Validate(ValidationOp::Acquire,
+ vec![lval_to_operand(dest_lval)]),
};
block_data.statements.insert(i+1, acquire_stmt);
- // The source is released until the region of the borrow ends -- but not if it is shared.
- if !is_lvalue_shared(&src_lval, &local_decls, tcx) {
- let src_ty = src_lval.ty(&local_decls, tcx).to_ty(tcx);
- let op = match re {
- &RegionKind::ReScope(ce) => ValidationOp::Suspend(ce),
- &RegionKind::ReErased => bug!("AddValidation pass must be run before erasing lifetimes"),
- _ => ValidationOp::Release,
- };
- let release_stmt = Statement {
- source_info: block_data.statements[i].source_info,
- kind: StatementKind::Validate(op, vec![(src_ty, src_lval)]),
- };
- block_data.statements.insert(i, release_stmt);
- }
+ // The source is released until the region of the borrow ends.
+ let op = match re {
+ &RegionKind::ReScope(ce) => ValidationOp::Suspend(ce),
+ &RegionKind::ReErased =>
+ bug!("AddValidation pass must be run before erasing lifetimes"),
+ _ => ValidationOp::Release,
+ };
+ let release_stmt = Statement {
+ source_info: block_data.statements[i].source_info,
+ kind: StatementKind::Validate(op, vec![lval_to_operand(src_lval)]),
+ };
+ block_data.statements.insert(i, release_stmt);
}
}
}
From e869cf2be74372db55b64eb549f4dc0e6b5a667b Mon Sep 17 00:00:00 2001
From: Ralf Jung <post@ralfj.de>
Date: Fri, 21 Jul 2017 14:49:01 -0700
Subject: [PATCH 09/30] make ValidationOperand generic so that we can reuse it
in miri with a different Lvalue type
---
src/librustc/ich/impls_mir.rs | 14 +++++++++++++-
src/librustc/mir/mod.rs | 11 ++++++-----
src/librustc_mir/transform/add_validation.rs | 2 +-
3 files changed, 20 insertions(+), 7 deletions(-)
diff --git a/src/librustc/ich/impls_mir.rs b/src/librustc/ich/impls_mir.rs
index dc41f981ed5..bef35fdc257 100644
--- a/src/librustc/ich/impls_mir.rs
+++ b/src/librustc/ich/impls_mir.rs
@@ -243,7 +243,19 @@ for mir::StatementKind<'tcx> {
}
}
-impl_stable_hash_for!(struct mir::ValidationOperand<'tcx> { lval, ty, re, mutbl });
+impl<'a, 'gcx, 'tcx, T> HashStable<StableHashingContext<'a, 'gcx, 'tcx>> for mir::ValidationOperand<'tcx, T>
+ where T: HashStable<StableHashingContext<'a, 'gcx, 'tcx>>
+{
+ fn hash_stable<W: StableHasherResult>(&self,
+ hcx: &mut StableHashingContext<'a, 'gcx, 'tcx>,
+ hasher: &mut StableHasher<W>)
+ {
+ self.lval.hash_stable(hcx, hasher);
+ self.ty.hash_stable(hcx, hasher);
+ self.re.hash_stable(hcx, hasher);
+ self.mutbl.hash_stable(hcx, hasher);
+ }
+}
impl_stable_hash_for!(enum mir::ValidationOp { Acquire, Release, Suspend(extent) });
diff --git a/src/librustc/mir/mod.rs b/src/librustc/mir/mod.rs
index 4655f8a9c15..f8261f80629 100644
--- a/src/librustc/mir/mod.rs
+++ b/src/librustc/mir/mod.rs
@@ -826,7 +826,7 @@ pub enum StatementKind<'tcx> {
},
/// Assert the given lvalues to be valid inhabitants of their type.
- Validate(ValidationOp, Vec<ValidationOperand<'tcx>>),
+ Validate(ValidationOp, Vec<ValidationOperand<'tcx, Lvalue<'tcx>>>),
/// Mark one terminating point of an extent (i.e. static region).
/// (The starting point(s) arise implicitly from borrows.)
@@ -855,15 +855,16 @@ impl Debug for ValidationOp {
}
}
+// This is generic so that it can be reused by miri
#[derive(Clone, RustcEncodable, RustcDecodable)]
-pub struct ValidationOperand<'tcx> {
- pub lval: Lvalue<'tcx>,
+pub struct ValidationOperand<'tcx, T> {
+ pub lval: T,
pub ty: Ty<'tcx>,
pub re: Option<CodeExtent>,
pub mutbl: hir::Mutability,
}
-impl<'tcx> Debug for ValidationOperand<'tcx> {
+impl<'tcx, T: Debug> Debug for ValidationOperand<'tcx, T> {
fn fmt(&self, fmt: &mut Formatter) -> fmt::Result {
write!(fmt, "{:?}@{:?}", self.lval, self.ty)?;
if let Some(ce) = self.re {
@@ -1527,7 +1528,7 @@ impl<'tcx> TypeFoldable<'tcx> for BasicBlockData<'tcx> {
}
}
-impl<'tcx> TypeFoldable<'tcx> for ValidationOperand<'tcx> {
+impl<'tcx> TypeFoldable<'tcx> for ValidationOperand<'tcx, Lvalue<'tcx>> {
fn super_fold_with<'gcx: 'tcx, F: TypeFolder<'gcx, 'tcx>>(&self, folder: &mut F) -> Self {
ValidationOperand {
lval: self.lval.fold_with(folder),
diff --git a/src/librustc_mir/transform/add_validation.rs b/src/librustc_mir/transform/add_validation.rs
index 1fe16fb98f2..005d793cd8b 100644
--- a/src/librustc_mir/transform/add_validation.rs
+++ b/src/librustc_mir/transform/add_validation.rs
@@ -88,7 +88,7 @@ impl MirPass for AddValidation {
let local_decls = mir.local_decls.clone(); // TODO: Find a way to get rid of this clone.
/// Convert an lvalue to a validation operand.
- let lval_to_operand = |lval: Lvalue<'tcx>| -> ValidationOperand<'tcx> {
+ let lval_to_operand = |lval: Lvalue<'tcx>| -> ValidationOperand<'tcx, Lvalue<'tcx>> {
let (re, mutbl) = lval_context(&lval, &local_decls, tcx);
let ty = lval.ty(&local_decls, tcx).to_ty(tcx);
ValidationOperand { lval, ty, re, mutbl }
From 23cd90ed41b6f1299d51da80cc6481f28a4b0f1f Mon Sep 17 00:00:00 2001
From: Ralf Jung <post@ralfj.de>
Date: Fri, 21 Jul 2017 23:18:34 -0700
Subject: [PATCH 10/30] add -Z flag for AddValidation pass
---
src/librustc/session/config.rs | 2 ++
src/librustc_mir/transform/add_validation.rs | 4 ++++
src/librustc_mir/transform/erase_regions.rs | 6 ++++++
3 files changed, 12 insertions(+)
diff --git a/src/librustc/session/config.rs b/src/librustc/session/config.rs
index 8b55eb4c099..c5ddcb597cb 100644
--- a/src/librustc/session/config.rs
+++ b/src/librustc/session/config.rs
@@ -1025,6 +1025,8 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options,
"the directory the MIR is dumped into"),
dump_mir_exclude_pass_number: bool = (false, parse_bool, [UNTRACKED],
"if set, exclude the pass number when dumping MIR (used in tests)"),
+ mir_emit_validate: bool = (false, parse_bool, [TRACKED],
+ "emit Validate MIR statements, interpreted e.g. by miri"),
perf_stats: bool = (false, parse_bool, [UNTRACKED],
"print some performance-related statistics"),
hir_stats: bool = (false, parse_bool, [UNTRACKED],
diff --git a/src/librustc_mir/transform/add_validation.rs b/src/librustc_mir/transform/add_validation.rs
index 005d793cd8b..e400683e8b4 100644
--- a/src/librustc_mir/transform/add_validation.rs
+++ b/src/librustc_mir/transform/add_validation.rs
@@ -85,6 +85,10 @@ impl MirPass for AddValidation {
tcx: TyCtxt<'a, 'tcx, 'tcx>,
_: MirSource,
mir: &mut Mir<'tcx>) {
+ if !tcx.sess.opts.debugging_opts.mir_emit_validate {
+ return;
+ }
+
let local_decls = mir.local_decls.clone(); // TODO: Find a way to get rid of this clone.
/// Convert an lvalue to a validation operand.
diff --git a/src/librustc_mir/transform/erase_regions.rs b/src/librustc_mir/transform/erase_regions.rs
index 12b1c549ffe..05376ff3d52 100644
--- a/src/librustc_mir/transform/erase_regions.rs
+++ b/src/librustc_mir/transform/erase_regions.rs
@@ -77,6 +77,12 @@ impl<'a, 'tcx> MutVisitor<'tcx> for EraseRegionsVisitor<'a, 'tcx> {
block: BasicBlock,
statement: &mut Statement<'tcx>,
location: Location) {
+ if !self.tcx.sess.opts.debugging_opts.mir_emit_validate {
+ if let StatementKind::EndRegion(_) = statement.kind {
+ statement.kind = StatementKind::Nop;
+ }
+ }
+
self.in_validation_statement = match statement.kind {
StatementKind::Validate(..) => true,
_ => false,
From b6816b2b56f5e1044f100e0ab7da0d9540d8f9cf Mon Sep 17 00:00:00 2001
From: Ralf Jung <post@ralfj.de>
Date: Sat, 22 Jul 2017 01:04:16 -0700
Subject: [PATCH 11/30] please the tidy
---
src/librustc/ich/impls_mir.rs | 3 ++-
src/librustc/mir/mod.rs | 3 ++-
src/librustc_mir/transform/add_validation.rs | 4 ++--
src/librustc_mir/transform/clean_end_regions.rs | 5 ++++-
src/librustc_mir/transform/erase_regions.rs | 4 ++--
5 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/src/librustc/ich/impls_mir.rs b/src/librustc/ich/impls_mir.rs
index bef35fdc257..c20864183f4 100644
--- a/src/librustc/ich/impls_mir.rs
+++ b/src/librustc/ich/impls_mir.rs
@@ -243,7 +243,8 @@ for mir::StatementKind<'tcx> {
}
}
-impl<'a, 'gcx, 'tcx, T> HashStable<StableHashingContext<'a, 'gcx, 'tcx>> for mir::ValidationOperand<'tcx, T>
+impl<'a, 'gcx, 'tcx, T> HashStable<StableHashingContext<'a, 'gcx, 'tcx>>
+ for mir::ValidationOperand<'tcx, T>
where T: HashStable<StableHashingContext<'a, 'gcx, 'tcx>>
{
fn hash_stable<W: StableHasherResult>(&self,
diff --git a/src/librustc/mir/mod.rs b/src/librustc/mir/mod.rs
index f8261f80629..3ee86dbdc84 100644
--- a/src/librustc/mir/mod.rs
+++ b/src/librustc/mir/mod.rs
@@ -1596,7 +1596,8 @@ impl<'tcx> TypeFoldable<'tcx> for Statement<'tcx> {
// trait with a `fn visit_extent`.
EndRegion(ref _extent) => false,
- Validate(ref _op, ref lvalues) => lvalues.iter().any(|ty_and_lvalue| ty_and_lvalue.visit_with(visitor)),
+ Validate(ref _op, ref lvalues) =>
+ lvalues.iter().any(|ty_and_lvalue| ty_and_lvalue.visit_with(visitor)),
Nop => false,
}
diff --git a/src/librustc_mir/transform/add_validation.rs b/src/librustc_mir/transform/add_validation.rs
index e400683e8b4..d91db41d20d 100644
--- a/src/librustc_mir/transform/add_validation.rs
+++ b/src/librustc_mir/transform/add_validation.rs
@@ -89,7 +89,7 @@ impl MirPass for AddValidation {
return;
}
- let local_decls = mir.local_decls.clone(); // TODO: Find a way to get rid of this clone.
+ let local_decls = mir.local_decls.clone(); // FIXME: Find a way to get rid of this clone.
/// Convert an lvalue to a validation operand.
let lval_to_operand = |lval: Lvalue<'tcx>| -> ValidationOperand<'tcx, Lvalue<'tcx>> {
@@ -104,7 +104,7 @@ impl MirPass for AddValidation {
let acquire_stmt = Statement {
source_info: SourceInfo {
scope: ARGUMENT_VISIBILITY_SCOPE,
- span: mir.span, // TODO: Consider using just the span covering the function
+ span: mir.span, // FIXME: Consider using just the span covering the function
// argument declaration.
},
kind: StatementKind::Validate(ValidationOp::Acquire,
diff --git a/src/librustc_mir/transform/clean_end_regions.rs b/src/librustc_mir/transform/clean_end_regions.rs
index 28311a5e68c..1a31bf97530 100644
--- a/src/librustc_mir/transform/clean_end_regions.rs
+++ b/src/librustc_mir/transform/clean_end_regions.rs
@@ -43,7 +43,10 @@ impl MirPass for CleanEndRegions {
_tcx: TyCtxt<'a, 'tcx, 'tcx>,
_source: MirSource,
mir: &mut Mir<'tcx>) {
- let mut gather = GatherBorrowedRegions { seen_regions: FxHashSet(), in_validation_statement: false };
+ let mut gather = GatherBorrowedRegions {
+ seen_regions: FxHashSet(),
+ in_validation_statement: false
+ };
gather.visit_mir(mir);
let mut delete = DeleteTrivialEndRegions { seen_regions: &mut gather.seen_regions };
diff --git a/src/librustc_mir/transform/erase_regions.rs b/src/librustc_mir/transform/erase_regions.rs
index 05376ff3d52..f01d71fde26 100644
--- a/src/librustc_mir/transform/erase_regions.rs
+++ b/src/librustc_mir/transform/erase_regions.rs
@@ -11,8 +11,8 @@
//! This pass erases all early-bound regions from the types occuring in the MIR.
//! We want to do this once just before trans, so trans does not have to take
//! care erasing regions all over the place.
-//! NOTE: We do NOT erase regions of statements that are relevant for "types-as-contracts"-validation,
-//! namely, AcquireValid, ReleaseValid, and EndRegion.
+//! NOTE: We do NOT erase regions of statements that are relevant for
+//! "types-as-contracts"-validation, namely, AcquireValid, ReleaseValid, and EndRegion.
use rustc::ty::subst::Substs;
use rustc::ty::{Ty, TyCtxt, ClosureSubsts};
From 04f962adc39c2632da8f712b2cd38eb6109ae5a1 Mon Sep 17 00:00:00 2001
From: Ralf Jung <post@ralfj.de>
Date: Mon, 24 Jul 2017 19:19:39 -0700
Subject: [PATCH 12/30] after a Ref, only acquire the Deref'd destination
---
src/librustc_mir/transform/add_validation.rs | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/src/librustc_mir/transform/add_validation.rs b/src/librustc_mir/transform/add_validation.rs
index d91db41d20d..4edcab738c3 100644
--- a/src/librustc_mir/transform/add_validation.rs
+++ b/src/librustc_mir/transform/add_validation.rs
@@ -184,6 +184,10 @@ impl MirPass for AddValidation {
_ => continue,
};
// So this is a ref, and we got all the data we wanted.
+ // Do an acquire of the result -- but only what it points to, so add a Deref
+ // projection.
+ let dest_lval = Projection { base: dest_lval, elem: ProjectionElem::Deref };
+ let dest_lval = Lvalue::Projection(Box::new(dest_lval));
let acquire_stmt = Statement {
source_info: block_data.statements[i].source_info,
kind: StatementKind::Validate(ValidationOp::Acquire,
From b934506e681e00b803ca886122062916b41e0fbe Mon Sep 17 00:00:00 2001
From: Ralf Jung <post@ralfj.de>
Date: Tue, 25 Jul 2017 16:44:49 -0700
Subject: [PATCH 13/30] Reorder passes so that AddValidation can run after
ElaborateDrops
---
src/librustc_driver/driver.rs | 23 +++++++++++--------
.../transform/clean_end_regions.rs | 21 +++++------------
2 files changed, 19 insertions(+), 25 deletions(-)
diff --git a/src/librustc_driver/driver.rs b/src/librustc_driver/driver.rs
index 68e6b0f50d1..2b667d83e35 100644
--- a/src/librustc_driver/driver.rs
+++ b/src/librustc_driver/driver.rs
@@ -925,10 +925,6 @@ pub fn phase_3_run_analysis_passes<'tcx, F, R>(sess: &'tcx Session,
let mut passes = Passes::new();
passes.push_hook(mir::transform::dump_mir::DumpMir);
- // Insert AcquireValid and ReleaseValid calls. Conceptually, this
- // pass is actually part of MIR building.
- passes.push_pass(MIR_CONST, mir::transform::add_validation::AddValidation);
-
// Remove all `EndRegion` statements that are not involved in borrows.
passes.push_pass(MIR_CONST, mir::transform::clean_end_regions::CleanEndRegions);
@@ -937,6 +933,8 @@ pub fn phase_3_run_analysis_passes<'tcx, F, R>(sess: &'tcx Session,
passes.push_pass(MIR_CONST, mir::transform::type_check::TypeckMir);
passes.push_pass(MIR_CONST, mir::transform::rustc_peek::SanityCheck);
+ // We compute "constant qualifications" betwen MIR_CONST and MIR_VALIDATED.
+
// What we need to run borrowck etc.
passes.push_pass(MIR_VALIDATED, mir::transform::qualify_consts::QualifyAndPromoteConstants);
passes.push_pass(MIR_VALIDATED,
@@ -944,18 +942,23 @@ pub fn phase_3_run_analysis_passes<'tcx, F, R>(sess: &'tcx Session,
passes.push_pass(MIR_VALIDATED, mir::transform::simplify::SimplifyCfg::new("qualify-consts"));
passes.push_pass(MIR_VALIDATED, mir::transform::nll::NLL);
- // Optimizations begin.
- passes.push_pass(MIR_OPTIMIZED, mir::transform::no_landing_pads::NoLandingPads);
- passes.push_pass(MIR_OPTIMIZED, mir::transform::simplify::SimplifyCfg::new("no-landing-pads"));
+ // borrowck runs between MIR_VALIDATED and MIR_OPTIMIZED.
- // From here on out, regions are gone.
- passes.push_pass(MIR_OPTIMIZED, mir::transform::erase_regions::EraseRegions);
+ // These next passes must be executed together
+ passes.push_pass(MIR_OPTIMIZED, mir::transform::no_landing_pads::NoLandingPads);
passes.push_pass(MIR_OPTIMIZED, mir::transform::add_call_guards::AddCallGuards);
passes.push_pass(MIR_OPTIMIZED, mir::transform::elaborate_drops::ElaborateDrops);
passes.push_pass(MIR_OPTIMIZED, mir::transform::no_landing_pads::NoLandingPads);
passes.push_pass(MIR_OPTIMIZED, mir::transform::simplify::SimplifyCfg::new("elaborate-drops"));
-
// No lifetime analysis based on borrowing can be done from here on out.
+
+ // AddValidation needs to run after ElaborateDrops and before EraseRegions.
+ passes.push_pass(MIR_OPTIMIZED, mir::transform::add_validation::AddValidation);
+
+ // From here on out, regions are gone.
+ passes.push_pass(MIR_OPTIMIZED, mir::transform::erase_regions::EraseRegions);
+
+ // Optimizations begin.
passes.push_pass(MIR_OPTIMIZED, mir::transform::inline::Inline);
passes.push_pass(MIR_OPTIMIZED, mir::transform::instcombine::InstCombine);
passes.push_pass(MIR_OPTIMIZED, mir::transform::deaggregator::Deaggregator);
diff --git a/src/librustc_mir/transform/clean_end_regions.rs b/src/librustc_mir/transform/clean_end_regions.rs
index 1a31bf97530..d7ec58384a4 100644
--- a/src/librustc_mir/transform/clean_end_regions.rs
+++ b/src/librustc_mir/transform/clean_end_regions.rs
@@ -31,7 +31,6 @@ pub struct CleanEndRegions;
struct GatherBorrowedRegions {
seen_regions: FxHashSet<CodeExtent>,
- in_validation_statement: bool,
}
struct DeleteTrivialEndRegions<'a> {
@@ -44,8 +43,7 @@ impl MirPass for CleanEndRegions {
_source: MirSource,
mir: &mut Mir<'tcx>) {
let mut gather = GatherBorrowedRegions {
- seen_regions: FxHashSet(),
- in_validation_statement: false
+ seen_regions: FxHashSet()
};
gather.visit_mir(mir);
@@ -71,22 +69,15 @@ impl<'tcx> Visitor<'tcx> for GatherBorrowedRegions {
block: BasicBlock,
statement: &Statement<'tcx>,
location: Location) {
- self.in_validation_statement = match statement.kind {
- StatementKind::Validate(..) => true,
- _ => false,
- };
self.super_statement(block, statement, location);
- self.in_validation_statement = false;
}
fn visit_ty(&mut self, ty: &Ty<'tcx>, _: Lookup) {
- // Gather regions that occur in types inside AcquireValid/ReleaseValid statements
- if self.in_validation_statement {
- for re in ty.walk().flat_map(|t| t.regions()) {
- match *re {
- RegionKind::ReScope(ce) => { self.seen_regions.insert(ce); }
- _ => {},
- }
+ // Gather regions that occur in types
+ for re in ty.walk().flat_map(|t| t.regions()) {
+ match *re {
+ RegionKind::ReScope(ce) => { self.seen_regions.insert(ce); }
+ _ => {},
}
}
self.super_ty(ty);
From 7ec50dfee3f2c2562586a59d80d3d1e9d2d0c0cd Mon Sep 17 00:00:00 2001
From: Ralf Jung <post@ralfj.de>
Date: Wed, 26 Jul 2017 20:51:36 -0700
Subject: [PATCH 14/30] also release/validate around non-Misc casts
---
src/librustc_mir/transform/add_validation.rs | 101 +++++++++++++------
1 file changed, 71 insertions(+), 30 deletions(-)
diff --git a/src/librustc_mir/transform/add_validation.rs b/src/librustc_mir/transform/add_validation.rs
index 4edcab738c3..70ef08cf2d1 100644
--- a/src/librustc_mir/transform/add_validation.rs
+++ b/src/librustc_mir/transform/add_validation.rs
@@ -171,42 +171,83 @@ impl MirPass for AddValidation {
}
// PART 3
- // Add ReleaseValid/AcquireValid around Ref. Again an iterator does not seem very suited
+ // Add ReleaseValid/AcquireValid around Ref and Cast. Again an iterator does not seem very
+ // suited
// as we need to add new statements before and after each Ref.
for block_data in mir.basic_blocks_mut() {
// We want to insert statements around Ref commands as we iterate. To this end, we
// iterate backwards using indices.
for i in (0..block_data.statements.len()).rev() {
- let (dest_lval, re, src_lval) = match block_data.statements[i].kind {
- StatementKind::Assign(ref dest_lval, Rvalue::Ref(re, _, ref src_lval)) => {
- (dest_lval.clone(), re, src_lval.clone())
- },
- _ => continue,
- };
- // So this is a ref, and we got all the data we wanted.
- // Do an acquire of the result -- but only what it points to, so add a Deref
- // projection.
- let dest_lval = Projection { base: dest_lval, elem: ProjectionElem::Deref };
- let dest_lval = Lvalue::Projection(Box::new(dest_lval));
- let acquire_stmt = Statement {
- source_info: block_data.statements[i].source_info,
- kind: StatementKind::Validate(ValidationOp::Acquire,
- vec![lval_to_operand(dest_lval)]),
- };
- block_data.statements.insert(i+1, acquire_stmt);
+ match block_data.statements[i].kind {
+ // When the borrow of this ref expires, we need to recover validation.
+ StatementKind::Assign(_, Rvalue::Ref(_, _, _)) => {
+ // Due to a lack of NLL; we can't capture anything directly here.
+ // Instead, we have to re-match and clone there.
+ let (dest_lval, re, src_lval) = match block_data.statements[i].kind {
+ StatementKind::Assign(ref dest_lval,
+ Rvalue::Ref(re, _, ref src_lval)) => {
+ (dest_lval.clone(), re, src_lval.clone())
+ },
+ _ => bug!("We already matched this."),
+ };
+ // So this is a ref, and we got all the data we wanted.
+ // Do an acquire of the result -- but only what it points to, so add a Deref
+ // projection.
+ let dest_lval = Projection { base: dest_lval, elem: ProjectionElem::Deref };
+ let dest_lval = Lvalue::Projection(Box::new(dest_lval));
+ let acquire_stmt = Statement {
+ source_info: block_data.statements[i].source_info,
+ kind: StatementKind::Validate(ValidationOp::Acquire,
+ vec![lval_to_operand(dest_lval)]),
+ };
+ block_data.statements.insert(i+1, acquire_stmt);
- // The source is released until the region of the borrow ends.
- let op = match re {
- &RegionKind::ReScope(ce) => ValidationOp::Suspend(ce),
- &RegionKind::ReErased =>
- bug!("AddValidation pass must be run before erasing lifetimes"),
- _ => ValidationOp::Release,
- };
- let release_stmt = Statement {
- source_info: block_data.statements[i].source_info,
- kind: StatementKind::Validate(op, vec![lval_to_operand(src_lval)]),
- };
- block_data.statements.insert(i, release_stmt);
+ // The source is released until the region of the borrow ends.
+ let op = match re {
+ &RegionKind::ReScope(ce) => ValidationOp::Suspend(ce),
+ &RegionKind::ReErased =>
+ bug!("AddValidation pass must be run before erasing lifetimes"),
+ _ => ValidationOp::Release,
+ };
+ let release_stmt = Statement {
+ source_info: block_data.statements[i].source_info,
+ kind: StatementKind::Validate(op, vec![lval_to_operand(src_lval)]),
+ };
+ block_data.statements.insert(i, release_stmt);
+ }
+ // Casts can change what validation does (e.g. unsizing)
+ StatementKind::Assign(_, Rvalue::Cast(kind, Operand::Consume(_), _))
+ if kind != CastKind::Misc =>
+ {
+ // Due to a lack of NLL; we can't capture anything directly here.
+ // Instead, we have to re-match and clone there.
+ let (dest_lval, src_lval) = match block_data.statements[i].kind {
+ StatementKind::Assign(ref dest_lval,
+ Rvalue::Cast(_, Operand::Consume(ref src_lval), _)) =>
+ {
+ (dest_lval.clone(), src_lval.clone())
+ },
+ _ => bug!("We already matched this."),
+ };
+
+ // Acquire of the result
+ let acquire_stmt = Statement {
+ source_info: block_data.statements[i].source_info,
+ kind: StatementKind::Validate(ValidationOp::Acquire,
+ vec![lval_to_operand(dest_lval)]),
+ };
+ block_data.statements.insert(i+1, acquire_stmt);
+
+ // Release of the input
+ let release_stmt = Statement {
+ source_info: block_data.statements[i].source_info,
+ kind: StatementKind::Validate(ValidationOp::Release,
+ vec![lval_to_operand(src_lval)]),
+ };
+ block_data.statements.insert(i, release_stmt);
+ }
+ _ => {},
+ }
}
}
}
From 57958d1a04d12dbd3ea51904c2f38fbc3b40d246 Mon Sep 17 00:00:00 2001
From: Ralf Jung <post@ralfj.de>
Date: Fri, 28 Jul 2017 15:16:17 -0700
Subject: [PATCH 15/30] Add tests for emitting validation statements
---
src/test/mir-opt/README.md | 11 ++------
src/test/mir-opt/validate_1.rs | 48 ++++++++++++++++++++++++++++++++++
src/test/mir-opt/validate_2.rs | 26 ++++++++++++++++++
src/test/mir-opt/validate_3.rs | 45 +++++++++++++++++++++++++++++++
4 files changed, 121 insertions(+), 9 deletions(-)
create mode 100644 src/test/mir-opt/validate_1.rs
create mode 100644 src/test/mir-opt/validate_2.rs
create mode 100644 src/test/mir-opt/validate_3.rs
diff --git a/src/test/mir-opt/README.md b/src/test/mir-opt/README.md
index 28a124e3c61..d999ff97551 100644
--- a/src/test/mir-opt/README.md
+++ b/src/test/mir-opt/README.md
@@ -57,13 +57,6 @@ the lines being too long.
compiletest handles dumping the MIR before and after every pass for you. The
test writer only has to specify the file names of the dumped files (not the
-full path to the file) and what lines to expect. I added an option to rustc
+full path to the file) and what lines to expect. There is an option to rustc
that tells it to dump the mir into some directly (rather then always dumping to
-the current directory).
-
-Lines match ignoring whitespace, and the prefix "//" is removed of course.
-
-It also currently strips trailing comments -- partly because the full file path
-in "scope comments" is unpredictable and partly because tidy complains about
-the lines being too long.
-
+the current directory).
diff --git a/src/test/mir-opt/validate_1.rs b/src/test/mir-opt/validate_1.rs
new file mode 100644
index 00000000000..0059fc93062
--- /dev/null
+++ b/src/test/mir-opt/validate_1.rs
@@ -0,0 +1,48 @@
+// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// compile-flags: -Z verbose -Z mir-emit-validate
+
+fn foo(_x: &mut i32) {}
+
+fn main() {
+ let mut x = 0;
+ foo(&mut x);
+}
+
+// END RUST SOURCE
+// START rustc.node4.EraseRegions.after.mir
+// fn foo(_1: &ReErased mut i32) -> () {
+// bb0: {
+// Validate(Acquire, [_1@&ReFree(DefId { krate: CrateNum(0), node: DefIndex(3) => validate_1/8cd878b::foo[0] }, BrAnon(0)) mut i32]);
+// return;
+// }
+// }
+// END rustc.node4.EraseRegions.after.mir
+// START rustc.node11.EraseRegions.after.mir
+// fn main() -> () {
+// bb0: {
+// Validate(Suspend(ReScope(Misc(NodeId(20)))), [_1@i32]);
+// _4 = &ReErased mut _1;
+// Validate(Acquire, [(*_4)@i32/ReScope(Misc(NodeId(20)))]);
+// Validate(Suspend(ReScope(Misc(NodeId(20)))), [(*_4)@i32/ReScope(Misc(NodeId(20)))]);
+// _3 = &ReErased mut (*_4);
+// Validate(Acquire, [(*_3)@i32/ReScope(Misc(NodeId(20)))]);
+// Validate(Release, [_3@&ReScope(Misc(NodeId(20))) mut i32]);
+// _2 = const foo(_3) -> bb1;
+// }
+//
+// bb1: {
+// Validate(Acquire, [_2@()]);
+// EndRegion(ReScope(Misc(NodeId(20))));
+// return;
+// }
+// }
+// END rustc.node11.EraseRegions.after.mir
diff --git a/src/test/mir-opt/validate_2.rs b/src/test/mir-opt/validate_2.rs
new file mode 100644
index 00000000000..f1562c8c34c
--- /dev/null
+++ b/src/test/mir-opt/validate_2.rs
@@ -0,0 +1,26 @@
+// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// compile-flags: -Z verbose -Z mir-emit-validate
+
+fn main() {
+ let _x : Box<[i32]> = Box::new([1, 2, 3]);
+}
+
+// END RUST SOURCE
+// START rustc.node4.EraseRegions.after.mir
+// fn main() -> () {
+// bb1: {
+// Validate(Release, [_2@std::boxed::Box<[i32; 3]>]);
+// _1 = _2 as std::boxed::Box<[i32]> (Unsize);
+// Validate(Acquire, [_1@std::boxed::Box<[i32]>]);
+// }
+// }
+// END rustc.node4.EraseRegions.after.mir
diff --git a/src/test/mir-opt/validate_3.rs b/src/test/mir-opt/validate_3.rs
new file mode 100644
index 00000000000..6990167b4e1
--- /dev/null
+++ b/src/test/mir-opt/validate_3.rs
@@ -0,0 +1,45 @@
+// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// compile-flags: -Z verbose -Z mir-emit-validate
+
+struct Test {
+ x: i32
+}
+
+fn foo(_x: &i32) {}
+
+fn main() {
+ let t = Test { x: 0 };
+ let t = &t;
+ foo(&t.x);
+}
+
+// END RUST SOURCE
+// START rustc.node16.EraseRegions.after.mir
+// fn main() -> () {
+// let mut _5: &ReErased i32;
+// bb0: {
+// Validate(Suspend(ReScope(Misc(NodeId(31)))), [((*_2).0: i32)@i32/ReScope(Remainder(BlockRemainder { block: NodeId(18), first_statement_index: 1 })) (imm)]);
+// _5 = &ReErased ((*_2).0: i32);
+// Validate(Acquire, [(*_5)@i32/ReScope(Misc(NodeId(31))) (imm)]);
+// Validate(Suspend(ReScope(Misc(NodeId(31)))), [(*_5)@i32/ReScope(Misc(NodeId(31))) (imm)]);
+// _4 = &ReErased (*_5);
+// Validate(Acquire, [(*_4)@i32/ReScope(Misc(NodeId(31))) (imm)]);
+// Validate(Release, [_4@&ReScope(Misc(NodeId(31))) i32]);
+// _3 = const foo(_4) -> bb1;
+// }
+// bb1: {
+// EndRegion(ReScope(Misc(NodeId(31))));
+// EndRegion(ReScope(Remainder(BlockRemainder { block: NodeId(18), first_statement_index: 1 })));
+// return;
+// }
+// }
+// END rustc.node16.EraseRegions.after.mir
From 29ed317ecb88458e74cd78003205368ba7d04cfb Mon Sep 17 00:00:00 2001
From: Ralf Jung <post@ralfj.de>
Date: Fri, 28 Jul 2017 15:42:11 -0700
Subject: [PATCH 16/30] silence tidy
---
src/test/mir-opt/validate_1.rs | 1 +
src/test/mir-opt/validate_2.rs | 1 +
src/test/mir-opt/validate_3.rs | 1 +
3 files changed, 3 insertions(+)
diff --git a/src/test/mir-opt/validate_1.rs b/src/test/mir-opt/validate_1.rs
index 0059fc93062..868d23b03c2 100644
--- a/src/test/mir-opt/validate_1.rs
+++ b/src/test/mir-opt/validate_1.rs
@@ -8,6 +8,7 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.
+// ignore-tidy-linelength
// compile-flags: -Z verbose -Z mir-emit-validate
fn foo(_x: &mut i32) {}
diff --git a/src/test/mir-opt/validate_2.rs b/src/test/mir-opt/validate_2.rs
index f1562c8c34c..a219c5fc78e 100644
--- a/src/test/mir-opt/validate_2.rs
+++ b/src/test/mir-opt/validate_2.rs
@@ -8,6 +8,7 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.
+// ignore-tidy-linelength
// compile-flags: -Z verbose -Z mir-emit-validate
fn main() {
diff --git a/src/test/mir-opt/validate_3.rs b/src/test/mir-opt/validate_3.rs
index 6990167b4e1..78957115f50 100644
--- a/src/test/mir-opt/validate_3.rs
+++ b/src/test/mir-opt/validate_3.rs
@@ -8,6 +8,7 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.
+// ignore-tidy-linelength
// compile-flags: -Z verbose -Z mir-emit-validate
struct Test {
From 6641415e8744e3f82f464eba270296536b1edaa1 Mon Sep 17 00:00:00 2001
From: Ralf Jung <post@ralfj.de>
Date: Sun, 30 Jul 2017 10:29:15 -0700
Subject: [PATCH 17/30] do not use doc comments inside functions
---
src/librustc_mir/transform/add_validation.rs | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/librustc_mir/transform/add_validation.rs b/src/librustc_mir/transform/add_validation.rs
index 70ef08cf2d1..ee472c616f6 100644
--- a/src/librustc_mir/transform/add_validation.rs
+++ b/src/librustc_mir/transform/add_validation.rs
@@ -91,7 +91,7 @@ impl MirPass for AddValidation {
let local_decls = mir.local_decls.clone(); // FIXME: Find a way to get rid of this clone.
- /// Convert an lvalue to a validation operand.
+ // Convert an lvalue to a validation operand.
let lval_to_operand = |lval: Lvalue<'tcx>| -> ValidationOperand<'tcx, Lvalue<'tcx>> {
let (re, mutbl) = lval_context(&lval, &local_decls, tcx);
let ty = lval.ty(&local_decls, tcx).to_ty(tcx);
From 6ff7c8fa047f98bfc6f1d7c9abdd64bc557add32 Mon Sep 17 00:00:00 2001
From: Ralf Jung <post@ralfj.de>
Date: Mon, 31 Jul 2017 13:20:37 -0700
Subject: [PATCH 18/30] more documentation
---
src/librustc/mir/mod.rs | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/src/librustc/mir/mod.rs b/src/librustc/mir/mod.rs
index 3ee86dbdc84..f7ef542544c 100644
--- a/src/librustc/mir/mod.rs
+++ b/src/librustc/mir/mod.rs
@@ -825,7 +825,9 @@ pub enum StatementKind<'tcx> {
inputs: Vec<Operand<'tcx>>
},
- /// Assert the given lvalues to be valid inhabitants of their type.
+ /// Assert the given lvalues to be valid inhabitants of their type. These statements are
+ /// currently only interpreted by miri and only generated when "-Z mir-emit-validate" is passed.
+ /// See <https://internals.rust-lang.org/t/types-as-contracts/5562/73> for more details.
Validate(ValidationOp, Vec<ValidationOperand<'tcx, Lvalue<'tcx>>>),
/// Mark one terminating point of an extent (i.e. static region).
@@ -836,10 +838,19 @@ pub enum StatementKind<'tcx> {
Nop,
}
+/// The `ValidationOp` describes what happens with each of the operands of a
+/// `Validate` statement.
#[derive(Copy, Clone, RustcEncodable, RustcDecodable, PartialEq, Eq)]
pub enum ValidationOp {
+ /// Recursively traverse the lvalue following the type and validate that all type
+ /// invariants are maintained. Furthermore, acquire exclusive/read-only access to the
+ /// memory reachable from the lvalue.
Acquire,
+ /// Recursive traverse the *mutable* part of the type and relinquish all exclusive
+ /// access.
Release,
+ /// Recursive traverse the *mutable* part of the type and relinquish all exclusive
+ /// access *until* the given region ends. Then, access will be recovered.
Suspend(CodeExtent),
}
From 6135461f9a44732a61e2422af20030ebd31486e8 Mon Sep 17 00:00:00 2001
From: Ralf Jung <post@ralfj.de>
Date: Mon, 31 Jul 2017 13:21:10 -0700
Subject: [PATCH 19/30] CleanEndRegions: use default impl where possible
---
src/librustc_mir/transform/clean_end_regions.rs | 7 -------
1 file changed, 7 deletions(-)
diff --git a/src/librustc_mir/transform/clean_end_regions.rs b/src/librustc_mir/transform/clean_end_regions.rs
index d7ec58384a4..f06b88551d1 100644
--- a/src/librustc_mir/transform/clean_end_regions.rs
+++ b/src/librustc_mir/transform/clean_end_regions.rs
@@ -65,13 +65,6 @@ impl<'tcx> Visitor<'tcx> for GatherBorrowedRegions {
self.super_rvalue(rvalue, location);
}
- fn visit_statement(&mut self,
- block: BasicBlock,
- statement: &Statement<'tcx>,
- location: Location) {
- self.super_statement(block, statement, location);
- }
-
fn visit_ty(&mut self, ty: &Ty<'tcx>, _: Lookup) {
// Gather regions that occur in types
for re in ty.walk().flat_map(|t| t.regions()) {
From 5e426e10683d851a530e17d33bf6454d958b7d46 Mon Sep 17 00:00:00 2001
From: Ralf Jung <post@ralfj.de>
Date: Mon, 31 Jul 2017 15:46:36 -0700
Subject: [PATCH 20/30] optionally only emit basic validation for functions
containing unsafe block / unsafe function
---
src/librustc/hir/mod.rs | 6 +-
src/librustc/session/config.rs | 5 +-
src/librustc_mir/transform/add_validation.rs | 174 ++++++++++++++-----
src/librustc_mir/transform/erase_regions.rs | 4 +-
src/test/mir-opt/validate_1.rs | 2 +-
src/test/mir-opt/validate_2.rs | 2 +-
src/test/mir-opt/validate_3.rs | 20 ++-
7 files changed, 155 insertions(+), 58 deletions(-)
diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs
index cc0d49c1a36..85d9745246f 100644
--- a/src/librustc/hir/mod.rs
+++ b/src/librustc/hir/mod.rs
@@ -49,7 +49,7 @@ use rustc_data_structures::indexed_vec;
use std::collections::BTreeMap;
use std::fmt;
-/// HIR doesn't commit to a concrete storage type and have its own alias for a vector.
+/// HIR doesn't commit to a concrete storage type and has its own alias for a vector.
/// It can be `Vec`, `P<[T]>` or potentially `Box<[T]>`, or some other container with similar
/// behavior. Unlike AST, HIR is mostly a static structure, so we can use an owned slice instead
/// of `Vec` to avoid keeping extra capacity.
@@ -76,14 +76,14 @@ pub mod pat_util;
pub mod print;
pub mod svh;
-/// A HirId uniquely identifies a node in the HIR of then current crate. It is
+/// A HirId uniquely identifies a node in the HIR of the current crate. It is
/// composed of the `owner`, which is the DefIndex of the directly enclosing
/// hir::Item, hir::TraitItem, or hir::ImplItem (i.e. the closest "item-like"),
/// and the `local_id` which is unique within the given owner.
///
/// This two-level structure makes for more stable values: One can move an item
/// around within the source code, or add or remove stuff before it, without
-/// the local_id part of the HirId changing, which is a very useful property
+/// the local_id part of the HirId changing, which is a very useful property in
/// incremental compilation where we have to persist things through changes to
/// the code base.
#[derive(Copy, Clone, PartialEq, Eq, Hash, PartialOrd, Ord, Debug,
diff --git a/src/librustc/session/config.rs b/src/librustc/session/config.rs
index c5ddcb597cb..c8b9412c566 100644
--- a/src/librustc/session/config.rs
+++ b/src/librustc/session/config.rs
@@ -1025,8 +1025,9 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options,
"the directory the MIR is dumped into"),
dump_mir_exclude_pass_number: bool = (false, parse_bool, [UNTRACKED],
"if set, exclude the pass number when dumping MIR (used in tests)"),
- mir_emit_validate: bool = (false, parse_bool, [TRACKED],
- "emit Validate MIR statements, interpreted e.g. by miri"),
+ mir_emit_validate: usize = (0, parse_uint, [TRACKED],
+ "emit Validate MIR statements, interpreted e.g. by miri (0: do not emit; 1: if function \
+ contains unsafe block, only validate arguments; 2: always emit full validation)"),
perf_stats: bool = (false, parse_bool, [UNTRACKED],
"print some performance-related statistics"),
hir_stats: bool = (false, parse_bool, [UNTRACKED],
diff --git a/src/librustc_mir/transform/add_validation.rs b/src/librustc_mir/transform/add_validation.rs
index ee472c616f6..1329378fbef 100644
--- a/src/librustc_mir/transform/add_validation.rs
+++ b/src/librustc_mir/transform/add_validation.rs
@@ -14,6 +14,8 @@
//! of MIR building, and only after this pass we think of the program has having the
//! normal MIR semantics.
+use syntax_pos::Span;
+use syntax::ast::NodeId;
use rustc::ty::{self, TyCtxt, RegionKind};
use rustc::hir;
use rustc::mir::*;
@@ -80,15 +82,78 @@ fn lval_context<'a, 'tcx, D>(
}
}
+/// Check if this function contains an unsafe block or is an unsafe function.
+fn fn_contains_unsafe<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, src: MirSource) -> bool {
+ use rustc::hir::intravisit::{self, Visitor};
+
+ let fn_node_id = match src {
+ MirSource::Fn(node_id) => node_id,
+ _ => return false, // only functions can have unsafe
+ };
+ let fn_item = tcx.hir.expect_item(fn_node_id);
+
+ struct FindUnsafe<'b, 'tcx> where 'tcx : 'b {
+ map: &'b hir::map::Map<'tcx>,
+ found_unsafe: bool,
+ }
+ let mut finder = FindUnsafe { map: &tcx.hir, found_unsafe: false };
+ finder.visit_item(fn_item);
+
+ impl<'b, 'tcx> Visitor<'tcx> for FindUnsafe<'b, 'tcx> {
+ fn nested_visit_map<'this>(&'this mut self) -> intravisit::NestedVisitorMap<'this, 'tcx> {
+ intravisit::NestedVisitorMap::OnlyBodies(self.map)
+ }
+
+ fn visit_fn(&mut self, fk: intravisit::FnKind<'tcx>, fd: &'tcx hir::FnDecl,
+ b: hir::BodyId, s: Span, id: NodeId)
+ {
+ assert!(!self.found_unsafe, "We should never see more than one fn");
+ let is_unsafe = match fk {
+ intravisit::FnKind::ItemFn(_, _, unsafety, ..) => unsafety == hir::Unsafety::Unsafe,
+ intravisit::FnKind::Method(_, sig, ..) => sig.unsafety == hir::Unsafety::Unsafe,
+ intravisit::FnKind::Closure(_) => false,
+ };
+ if is_unsafe {
+ // This is unsafe, and we are done.
+ self.found_unsafe = true;
+ } else {
+ // Go on searching.
+ intravisit::walk_fn(self, fk, fd, b, s, id)
+ }
+ }
+
+ fn visit_block(&mut self, b: &'tcx hir::Block) {
+ use rustc::hir::BlockCheckMode::*;
+
+ if self.found_unsafe { return; } // short-circuit
+
+ match b.rules {
+ UnsafeBlock(_) | PushUnsafeBlock(_) => {
+ // We found an unsafe block.
+ self.found_unsafe = true;
+ }
+ DefaultBlock | PopUnsafeBlock(_) => {
+ // No unsafe block here, go on searching.
+ intravisit::walk_block(self, b);
+ }
+ };
+ }
+ }
+
+ finder.found_unsafe
+}
+
impl MirPass for AddValidation {
fn run_pass<'a, 'tcx>(&self,
tcx: TyCtxt<'a, 'tcx, 'tcx>,
- _: MirSource,
- mir: &mut Mir<'tcx>) {
- if !tcx.sess.opts.debugging_opts.mir_emit_validate {
+ src: MirSource,
+ mir: &mut Mir<'tcx>)
+ {
+ let emit_validate = tcx.sess.opts.debugging_opts.mir_emit_validate;
+ if emit_validate == 0 {
return;
}
-
+ let restricted_validation = emit_validate == 1 && fn_contains_unsafe(tcx, src);
let local_decls = mir.local_decls.clone(); // FIXME: Find a way to get rid of this clone.
// Convert an lvalue to a validation operand.
@@ -98,22 +163,40 @@ impl MirPass for AddValidation {
ValidationOperand { lval, ty, re, mutbl }
};
+ // Emit an Acquire at the beginning of the given block. If we are in restricted emission mode
+ // (mir_emit_validate=1), also emit a Release immediately after the Acquire.
+ let emit_acquire = |block: &mut BasicBlockData<'tcx>, source_info, operands: Vec<_>| {
+ if operands.len() == 0 {
+ return; // Nothing to do
+ }
+ // Emit the release first, to avoid cloning if we do not emit it
+ if restricted_validation {
+ let release_stmt = Statement {
+ source_info,
+ kind: StatementKind::Validate(ValidationOp::Release, operands.clone()),
+ };
+ block.statements.insert(0, release_stmt);
+ }
+ // Now, the acquire
+ let acquire_stmt = Statement {
+ source_info,
+ kind: StatementKind::Validate(ValidationOp::Acquire, operands),
+ };
+ block.statements.insert(0, acquire_stmt);
+ };
+
// PART 1
// Add an AcquireValid at the beginning of the start block.
- if mir.arg_count > 0 {
- let acquire_stmt = Statement {
- source_info: SourceInfo {
- scope: ARGUMENT_VISIBILITY_SCOPE,
- span: mir.span, // FIXME: Consider using just the span covering the function
- // argument declaration.
- },
- kind: StatementKind::Validate(ValidationOp::Acquire,
- // Skip return value, go over all the arguments
- mir.local_decls.iter_enumerated().skip(1).take(mir.arg_count)
- .map(|(local, _)| lval_to_operand(Lvalue::Local(local))).collect()
- )
+ {
+ let source_info = SourceInfo {
+ scope: ARGUMENT_VISIBILITY_SCOPE,
+ span: mir.span, // FIXME: Consider using just the span covering the function
+ // argument declaration.
};
- mir.basic_blocks_mut()[START_BLOCK].statements.insert(0, acquire_stmt);
+ // Gather all arguments, skip return value.
+ let operands = mir.local_decls.iter_enumerated().skip(1).take(mir.arg_count)
+ .map(|(local, _)| lval_to_operand(Lvalue::Local(local))).collect();
+ emit_acquire(&mut mir.basic_blocks_mut()[START_BLOCK], source_info, operands);
}
// PART 2
@@ -125,18 +208,20 @@ impl MirPass for AddValidation {
Some(Terminator { kind: TerminatorKind::Call { ref args, ref destination, .. },
source_info }) => {
// Before the call: Release all arguments
- let release_stmt = Statement {
- source_info,
- kind: StatementKind::Validate(ValidationOp::Release,
- args.iter().filter_map(|op| {
- match op {
- &Operand::Consume(ref lval) =>
- Some(lval_to_operand(lval.clone())),
- &Operand::Constant(..) => { None },
- }
- }).collect())
- };
- block_data.statements.push(release_stmt);
+ if !restricted_validation {
+ let release_stmt = Statement {
+ source_info,
+ kind: StatementKind::Validate(ValidationOp::Release,
+ args.iter().filter_map(|op| {
+ match op {
+ &Operand::Consume(ref lval) =>
+ Some(lval_to_operand(lval.clone())),
+ &Operand::Constant(..) => { None },
+ }
+ }).collect())
+ };
+ block_data.statements.push(release_stmt);
+ }
// Remember the return destination for later
if let &Some(ref destination) = destination {
returns.push((source_info, destination.0.clone(), destination.1));
@@ -147,12 +232,14 @@ impl MirPass for AddValidation {
Some(Terminator { kind: TerminatorKind::DropAndReplace { location: ref lval, .. },
source_info }) => {
// Before the call: Release all arguments
- let release_stmt = Statement {
- source_info,
- kind: StatementKind::Validate(ValidationOp::Release,
- vec![lval_to_operand(lval.clone())]),
- };
- block_data.statements.push(release_stmt);
+ if !restricted_validation {
+ let release_stmt = Statement {
+ source_info,
+ kind: StatementKind::Validate(ValidationOp::Release,
+ vec![lval_to_operand(lval.clone())]),
+ };
+ block_data.statements.push(release_stmt);
+ }
// drop doesn't return anything, so we need no acquire.
}
_ => {
@@ -162,18 +249,21 @@ impl MirPass for AddValidation {
}
// Now we go over the returns we collected to acquire the return values.
for (source_info, dest_lval, dest_block) in returns {
- let acquire_stmt = Statement {
+ emit_acquire(
+ &mut mir.basic_blocks_mut()[dest_block],
source_info,
- kind: StatementKind::Validate(ValidationOp::Acquire,
- vec![lval_to_operand(dest_lval)]),
- };
- mir.basic_blocks_mut()[dest_block].statements.insert(0, acquire_stmt);
+ vec![lval_to_operand(dest_lval)]
+ );
+ }
+
+ if restricted_validation {
+ // No part 3 for us.
+ return;
}
// PART 3
// Add ReleaseValid/AcquireValid around Ref and Cast. Again an iterator does not seem very
- // suited
- // as we need to add new statements before and after each Ref.
+ // suited as we need to add new statements before and after each Ref.
for block_data in mir.basic_blocks_mut() {
// We want to insert statements around Ref commands as we iterate. To this end, we
// iterate backwards using indices.
diff --git a/src/librustc_mir/transform/erase_regions.rs b/src/librustc_mir/transform/erase_regions.rs
index f01d71fde26..baf0522896c 100644
--- a/src/librustc_mir/transform/erase_regions.rs
+++ b/src/librustc_mir/transform/erase_regions.rs
@@ -77,7 +77,9 @@ impl<'a, 'tcx> MutVisitor<'tcx> for EraseRegionsVisitor<'a, 'tcx> {
block: BasicBlock,
statement: &mut Statement<'tcx>,
location: Location) {
- if !self.tcx.sess.opts.debugging_opts.mir_emit_validate {
+ // Do NOT delete EndRegion if validation statements are emitted.
+ // Validation needs EndRegion.
+ if self.tcx.sess.opts.debugging_opts.mir_emit_validate == 0 {
if let StatementKind::EndRegion(_) = statement.kind {
statement.kind = StatementKind::Nop;
}
diff --git a/src/test/mir-opt/validate_1.rs b/src/test/mir-opt/validate_1.rs
index 868d23b03c2..558426fcde1 100644
--- a/src/test/mir-opt/validate_1.rs
+++ b/src/test/mir-opt/validate_1.rs
@@ -9,7 +9,7 @@
// except according to those terms.
// ignore-tidy-linelength
-// compile-flags: -Z verbose -Z mir-emit-validate
+// compile-flags: -Z verbose -Z mir-emit-validate=1
fn foo(_x: &mut i32) {}
diff --git a/src/test/mir-opt/validate_2.rs b/src/test/mir-opt/validate_2.rs
index a219c5fc78e..21723739ca1 100644
--- a/src/test/mir-opt/validate_2.rs
+++ b/src/test/mir-opt/validate_2.rs
@@ -9,7 +9,7 @@
// except according to those terms.
// ignore-tidy-linelength
-// compile-flags: -Z verbose -Z mir-emit-validate
+// compile-flags: -Z verbose -Z mir-emit-validate=1
fn main() {
let _x : Box<[i32]> = Box::new([1, 2, 3]);
diff --git a/src/test/mir-opt/validate_3.rs b/src/test/mir-opt/validate_3.rs
index 78957115f50..88ae114c579 100644
--- a/src/test/mir-opt/validate_3.rs
+++ b/src/test/mir-opt/validate_3.rs
@@ -9,7 +9,7 @@
// except according to those terms.
// ignore-tidy-linelength
-// compile-flags: -Z verbose -Z mir-emit-validate
+// compile-flags: -Z verbose -Z mir-emit-validate=1
struct Test {
x: i32
@@ -18,6 +18,10 @@ struct Test {
fn foo(_x: &i32) {}
fn main() {
+ // These internal unsafe functions should have no effect on the code generation.
+ unsafe fn _unused1() {}
+ fn _unused2(x: *const i32) -> i32 { unsafe { *x }}
+
let t = Test { x: 0 };
let t = &t;
foo(&t.x);
@@ -28,18 +32,18 @@ fn main() {
// fn main() -> () {
// let mut _5: &ReErased i32;
// bb0: {
-// Validate(Suspend(ReScope(Misc(NodeId(31)))), [((*_2).0: i32)@i32/ReScope(Remainder(BlockRemainder { block: NodeId(18), first_statement_index: 1 })) (imm)]);
+// Validate(Suspend(ReScope(Misc(NodeId(46)))), [((*_2).0: i32)@i32/ReScope(Remainder(BlockRemainder { block: NodeId(18), first_statement_index: 3 })) (imm)]);
// _5 = &ReErased ((*_2).0: i32);
-// Validate(Acquire, [(*_5)@i32/ReScope(Misc(NodeId(31))) (imm)]);
-// Validate(Suspend(ReScope(Misc(NodeId(31)))), [(*_5)@i32/ReScope(Misc(NodeId(31))) (imm)]);
+// Validate(Acquire, [(*_5)@i32/ReScope(Misc(NodeId(46))) (imm)]);
+// Validate(Suspend(ReScope(Misc(NodeId(46)))), [(*_5)@i32/ReScope(Misc(NodeId(46))) (imm)]);
// _4 = &ReErased (*_5);
-// Validate(Acquire, [(*_4)@i32/ReScope(Misc(NodeId(31))) (imm)]);
-// Validate(Release, [_4@&ReScope(Misc(NodeId(31))) i32]);
+// Validate(Acquire, [(*_4)@i32/ReScope(Misc(NodeId(46))) (imm)]);
+// Validate(Release, [_4@&ReScope(Misc(NodeId(46))) i32]);
// _3 = const foo(_4) -> bb1;
// }
// bb1: {
-// EndRegion(ReScope(Misc(NodeId(31))));
-// EndRegion(ReScope(Remainder(BlockRemainder { block: NodeId(18), first_statement_index: 1 })));
+// EndRegion(ReScope(Misc(NodeId(46))));
+// EndRegion(ReScope(Remainder(BlockRemainder { block: NodeId(18), first_statement_index: 3 })));
// return;
// }
// }
From 09cbe588c3a73b06ce800732ce122d8357c1d0cc Mon Sep 17 00:00:00 2001
From: Ralf Jung <post@ralfj.de>
Date: Mon, 31 Jul 2017 15:59:29 -0700
Subject: [PATCH 21/30] more readable printing of validation operands
---
src/librustc/mir/mod.rs | 2 +-
src/test/mir-opt/validate_1.rs | 14 +++++++-------
src/test/mir-opt/validate_2.rs | 4 ++--
src/test/mir-opt/validate_3.rs | 10 +++++-----
4 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/src/librustc/mir/mod.rs b/src/librustc/mir/mod.rs
index f7ef542544c..1e8dda0addf 100644
--- a/src/librustc/mir/mod.rs
+++ b/src/librustc/mir/mod.rs
@@ -877,7 +877,7 @@ pub struct ValidationOperand<'tcx, T> {
impl<'tcx, T: Debug> Debug for ValidationOperand<'tcx, T> {
fn fmt(&self, fmt: &mut Formatter) -> fmt::Result {
- write!(fmt, "{:?}@{:?}", self.lval, self.ty)?;
+ write!(fmt, "{:?}: {:?}", self.lval, self.ty)?;
if let Some(ce) = self.re {
// (reuse lifetime rendering policy from ppaux.)
write!(fmt, "/{}", ty::ReScope(ce))?;
diff --git a/src/test/mir-opt/validate_1.rs b/src/test/mir-opt/validate_1.rs
index 558426fcde1..4a143c4cee9 100644
--- a/src/test/mir-opt/validate_1.rs
+++ b/src/test/mir-opt/validate_1.rs
@@ -22,7 +22,7 @@ fn main() {
// START rustc.node4.EraseRegions.after.mir
// fn foo(_1: &ReErased mut i32) -> () {
// bb0: {
-// Validate(Acquire, [_1@&ReFree(DefId { krate: CrateNum(0), node: DefIndex(3) => validate_1/8cd878b::foo[0] }, BrAnon(0)) mut i32]);
+// Validate(Acquire, [_1: &ReFree(DefId { krate: CrateNum(0), node: DefIndex(3) => validate_1/8cd878b::foo[0] }, BrAnon(0)) mut i32]);
// return;
// }
// }
@@ -30,18 +30,18 @@ fn main() {
// START rustc.node11.EraseRegions.after.mir
// fn main() -> () {
// bb0: {
-// Validate(Suspend(ReScope(Misc(NodeId(20)))), [_1@i32]);
+// Validate(Suspend(ReScope(Misc(NodeId(20)))), [_1: i32]);
// _4 = &ReErased mut _1;
-// Validate(Acquire, [(*_4)@i32/ReScope(Misc(NodeId(20)))]);
-// Validate(Suspend(ReScope(Misc(NodeId(20)))), [(*_4)@i32/ReScope(Misc(NodeId(20)))]);
+// Validate(Acquire, [(*_4): i32/ReScope(Misc(NodeId(20)))]);
+// Validate(Suspend(ReScope(Misc(NodeId(20)))), [(*_4): i32/ReScope(Misc(NodeId(20)))]);
// _3 = &ReErased mut (*_4);
-// Validate(Acquire, [(*_3)@i32/ReScope(Misc(NodeId(20)))]);
-// Validate(Release, [_3@&ReScope(Misc(NodeId(20))) mut i32]);
+// Validate(Acquire, [(*_3): i32/ReScope(Misc(NodeId(20)))]);
+// Validate(Release, [_3: &ReScope(Misc(NodeId(20))) mut i32]);
// _2 = const foo(_3) -> bb1;
// }
//
// bb1: {
-// Validate(Acquire, [_2@()]);
+// Validate(Acquire, [_2: ()]);
// EndRegion(ReScope(Misc(NodeId(20))));
// return;
// }
diff --git a/src/test/mir-opt/validate_2.rs b/src/test/mir-opt/validate_2.rs
index 21723739ca1..37ebd720d52 100644
--- a/src/test/mir-opt/validate_2.rs
+++ b/src/test/mir-opt/validate_2.rs
@@ -19,9 +19,9 @@ fn main() {
// START rustc.node4.EraseRegions.after.mir
// fn main() -> () {
// bb1: {
-// Validate(Release, [_2@std::boxed::Box<[i32; 3]>]);
+// Validate(Release, [_2: std::boxed::Box<[i32; 3]>]);
// _1 = _2 as std::boxed::Box<[i32]> (Unsize);
-// Validate(Acquire, [_1@std::boxed::Box<[i32]>]);
+// Validate(Acquire, [_1: std::boxed::Box<[i32]>]);
// }
// }
// END rustc.node4.EraseRegions.after.mir
diff --git a/src/test/mir-opt/validate_3.rs b/src/test/mir-opt/validate_3.rs
index 88ae114c579..100fae5c678 100644
--- a/src/test/mir-opt/validate_3.rs
+++ b/src/test/mir-opt/validate_3.rs
@@ -32,13 +32,13 @@ fn main() {
// fn main() -> () {
// let mut _5: &ReErased i32;
// bb0: {
-// Validate(Suspend(ReScope(Misc(NodeId(46)))), [((*_2).0: i32)@i32/ReScope(Remainder(BlockRemainder { block: NodeId(18), first_statement_index: 3 })) (imm)]);
+// Validate(Suspend(ReScope(Misc(NodeId(46)))), [((*_2).0: i32): i32/ReScope(Remainder(BlockRemainder { block: NodeId(18), first_statement_index: 3 })) (imm)]);
// _5 = &ReErased ((*_2).0: i32);
-// Validate(Acquire, [(*_5)@i32/ReScope(Misc(NodeId(46))) (imm)]);
-// Validate(Suspend(ReScope(Misc(NodeId(46)))), [(*_5)@i32/ReScope(Misc(NodeId(46))) (imm)]);
+// Validate(Acquire, [(*_5): i32/ReScope(Misc(NodeId(46))) (imm)]);
+// Validate(Suspend(ReScope(Misc(NodeId(46)))), [(*_5): i32/ReScope(Misc(NodeId(46))) (imm)]);
// _4 = &ReErased (*_5);
-// Validate(Acquire, [(*_4)@i32/ReScope(Misc(NodeId(46))) (imm)]);
-// Validate(Release, [_4@&ReScope(Misc(NodeId(46))) i32]);
+// Validate(Acquire, [(*_4): i32/ReScope(Misc(NodeId(46))) (imm)]);
+// Validate(Release, [_4: &ReScope(Misc(NodeId(46))) i32]);
// _3 = const foo(_4) -> bb1;
// }
// bb1: {
From 26ca0d1b3ac8046662e1a6d976c9fdfba2c118dc Mon Sep 17 00:00:00 2001
From: Ralf Jung <post@ralfj.de>
Date: Mon, 31 Jul 2017 16:15:37 -0700
Subject: [PATCH 22/30] tidy
---
src/librustc_mir/transform/add_validation.rs | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/librustc_mir/transform/add_validation.rs b/src/librustc_mir/transform/add_validation.rs
index 1329378fbef..86a86f4934c 100644
--- a/src/librustc_mir/transform/add_validation.rs
+++ b/src/librustc_mir/transform/add_validation.rs
@@ -163,8 +163,8 @@ impl MirPass for AddValidation {
ValidationOperand { lval, ty, re, mutbl }
};
- // Emit an Acquire at the beginning of the given block. If we are in restricted emission mode
- // (mir_emit_validate=1), also emit a Release immediately after the Acquire.
+ // Emit an Acquire at the beginning of the given block. If we are in restricted emission
+ // mode (mir_emit_validate=1), also emit a Release immediately after the Acquire.
let emit_acquire = |block: &mut BasicBlockData<'tcx>, source_info, operands: Vec<_>| {
if operands.len() == 0 {
return; // Nothing to do
From e73d3145f57817ff91468107fc8cad3c6d6616e1 Mon Sep 17 00:00:00 2001
From: Ralf Jung <post@ralfj.de>
Date: Mon, 31 Jul 2017 18:33:45 -0700
Subject: [PATCH 23/30] fix AddValidation on methods
---
src/librustc_mir/transform/add_validation.rs | 10 ++++-
src/test/mir-opt/validate_1.rs | 39 +++++++++++---------
2 files changed, 29 insertions(+), 20 deletions(-)
diff --git a/src/librustc_mir/transform/add_validation.rs b/src/librustc_mir/transform/add_validation.rs
index 86a86f4934c..a3ec6af7603 100644
--- a/src/librustc_mir/transform/add_validation.rs
+++ b/src/librustc_mir/transform/add_validation.rs
@@ -85,19 +85,25 @@ fn lval_context<'a, 'tcx, D>(
/// Check if this function contains an unsafe block or is an unsafe function.
fn fn_contains_unsafe<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, src: MirSource) -> bool {
use rustc::hir::intravisit::{self, Visitor};
+ use rustc::hir::map::Node;
let fn_node_id = match src {
MirSource::Fn(node_id) => node_id,
_ => return false, // only functions can have unsafe
};
- let fn_item = tcx.hir.expect_item(fn_node_id);
struct FindUnsafe<'b, 'tcx> where 'tcx : 'b {
map: &'b hir::map::Map<'tcx>,
found_unsafe: bool,
}
let mut finder = FindUnsafe { map: &tcx.hir, found_unsafe: false };
- finder.visit_item(fn_item);
+ // Run the visitor on the NodeId we got. Seems like there is no uniform way to do that.
+ match tcx.hir.find(fn_node_id) {
+ Some(Node::NodeItem(item)) => finder.visit_item(item),
+ Some(Node::NodeImplItem(item)) => finder.visit_impl_item(item),
+ Some(_) | None =>
+ bug!("Expected method or function, found {}", tcx.hir.node_to_string(fn_node_id)),
+ };
impl<'b, 'tcx> Visitor<'tcx> for FindUnsafe<'b, 'tcx> {
fn nested_visit_map<'this>(&'this mut self) -> intravisit::NestedVisitorMap<'this, 'tcx> {
diff --git a/src/test/mir-opt/validate_1.rs b/src/test/mir-opt/validate_1.rs
index 4a143c4cee9..c8ea2bc2544 100644
--- a/src/test/mir-opt/validate_1.rs
+++ b/src/test/mir-opt/validate_1.rs
@@ -11,39 +11,42 @@
// ignore-tidy-linelength
// compile-flags: -Z verbose -Z mir-emit-validate=1
-fn foo(_x: &mut i32) {}
+struct Test;
+
+impl Test {
+ // Make sure we run the pass on a method, not just on bare functions.
+ fn foo(&self, _x: &mut i32) {}
+}
fn main() {
let mut x = 0;
- foo(&mut x);
+ Test.foo(&mut x);
}
// END RUST SOURCE
-// START rustc.node4.EraseRegions.after.mir
-// fn foo(_1: &ReErased mut i32) -> () {
+// START rustc.node10.EraseRegions.after.mir
// bb0: {
-// Validate(Acquire, [_1: &ReFree(DefId { krate: CrateNum(0), node: DefIndex(3) => validate_1/8cd878b::foo[0] }, BrAnon(0)) mut i32]);
+// Validate(Acquire, [_1: &ReFree(DefId { krate: CrateNum(0), node: DefIndex(5) => validate_1/8cd878b::{{impl}}[0]::foo[0] }, BrAnon(0)) Test, _2: &ReFree(DefId { krate: CrateNum(0), node: DefIndex(5) => validate_1/8cd878b::{{impl}}[0]::foo[0] }, BrAnon(1)) mut i32]);
// return;
// }
-// }
-// END rustc.node4.EraseRegions.after.mir
-// START rustc.node11.EraseRegions.after.mir
+// END rustc.node10.EraseRegions.after.mir
+// START rustc.node21.EraseRegions.after.mir
// fn main() -> () {
// bb0: {
-// Validate(Suspend(ReScope(Misc(NodeId(20)))), [_1: i32]);
-// _4 = &ReErased mut _1;
-// Validate(Acquire, [(*_4): i32/ReScope(Misc(NodeId(20)))]);
-// Validate(Suspend(ReScope(Misc(NodeId(20)))), [(*_4): i32/ReScope(Misc(NodeId(20)))]);
-// _3 = &ReErased mut (*_4);
-// Validate(Acquire, [(*_3): i32/ReScope(Misc(NodeId(20)))]);
-// Validate(Release, [_3: &ReScope(Misc(NodeId(20))) mut i32]);
-// _2 = const foo(_3) -> bb1;
+// Validate(Suspend(ReScope(Misc(NodeId(30)))), [_1: i32]);
+// _6 = &ReErased mut _1;
+// Validate(Acquire, [(*_6): i32/ReScope(Misc(NodeId(30)))]);
+// Validate(Suspend(ReScope(Misc(NodeId(30)))), [(*_6): i32/ReScope(Misc(NodeId(30)))]);
+// _5 = &ReErased mut (*_6);
+// Validate(Acquire, [(*_5): i32/ReScope(Misc(NodeId(30)))]);
+// Validate(Release, [_3: &ReScope(Misc(NodeId(30))) Test, _5: &ReScope(Misc(NodeId(30))) mut i32]);
+// _2 = const Test::foo(_3, _5) -> bb1;
// }
//
// bb1: {
// Validate(Acquire, [_2: ()]);
-// EndRegion(ReScope(Misc(NodeId(20))));
+// EndRegion(ReScope(Misc(NodeId(30))));
// return;
// }
// }
-// END rustc.node11.EraseRegions.after.mir
+// END rustc.node21.EraseRegions.after.mir
From 584d823bf2c309cb7b40aadd9d55ecc75f7eb9fc Mon Sep 17 00:00:00 2001
From: Ralf Jung <post@ralfj.de>
Date: Mon, 31 Jul 2017 19:51:10 -0700
Subject: [PATCH 24/30] Handle closures. Add some more tests.
---
src/librustc_mir/transform/add_validation.rs | 61 +++++++++++++++-----
src/test/mir-opt/validate_1.rs | 7 +++
src/test/mir-opt/validate_4.rs | 58 +++++++++++++++++++
src/test/mir-opt/validate_5.rs | 44 ++++++++++++++
4 files changed, 156 insertions(+), 14 deletions(-)
create mode 100644 src/test/mir-opt/validate_4.rs
create mode 100644 src/test/mir-opt/validate_5.rs
diff --git a/src/librustc_mir/transform/add_validation.rs b/src/librustc_mir/transform/add_validation.rs
index a3ec6af7603..6f136624f0a 100644
--- a/src/librustc_mir/transform/add_validation.rs
+++ b/src/librustc_mir/transform/add_validation.rs
@@ -87,6 +87,17 @@ fn fn_contains_unsafe<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, src: MirSource) ->
use rustc::hir::intravisit::{self, Visitor};
use rustc::hir::map::Node;
+ fn block_is_unsafe(block: &hir::Block) -> bool {
+ use rustc::hir::BlockCheckMode::*;
+
+ match block.rules {
+ UnsafeBlock(_) | PushUnsafeBlock(_) => true,
+ // For PopUnsafeBlock, we don't actually know -- but we will always also check all
+ // parent blocks, so we can safely declare the PopUnsafeBlock to not be unsafe.
+ DefaultBlock | PopUnsafeBlock(_) => false,
+ }
+ }
+
let fn_node_id = match src {
MirSource::Fn(node_id) => node_id,
_ => return false, // only functions can have unsafe
@@ -101,8 +112,35 @@ fn fn_contains_unsafe<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, src: MirSource) ->
match tcx.hir.find(fn_node_id) {
Some(Node::NodeItem(item)) => finder.visit_item(item),
Some(Node::NodeImplItem(item)) => finder.visit_impl_item(item),
+ Some(Node::NodeExpr(item)) => {
+ // This is a closure.
+ // We also have to walk up the parents and check that there is no unsafe block
+ // there.
+ let mut cur = fn_node_id;
+ loop {
+ // Go further upwards.
+ let parent = tcx.hir.get_parent_node(cur);
+ if cur == parent {
+ break;
+ }
+ cur = parent;
+ // Check if this is a a block
+ match tcx.hir.find(cur) {
+ Some(Node::NodeExpr(&hir::Expr { node: hir::ExprBlock(ref block), ..})) => {
+ if block_is_unsafe(&*block) {
+ // We can bail out here.
+ return true;
+ }
+ }
+ _ => {},
+ }
+ }
+ // Finally, visit the closure itself.
+ finder.visit_expr(item);
+ }
Some(_) | None =>
- bug!("Expected method or function, found {}", tcx.hir.node_to_string(fn_node_id)),
+ bug!("Expected function, method or closure, found {}",
+ tcx.hir.node_to_string(fn_node_id)),
};
impl<'b, 'tcx> Visitor<'tcx> for FindUnsafe<'b, 'tcx> {
@@ -113,7 +151,7 @@ fn fn_contains_unsafe<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, src: MirSource) ->
fn visit_fn(&mut self, fk: intravisit::FnKind<'tcx>, fd: &'tcx hir::FnDecl,
b: hir::BodyId, s: Span, id: NodeId)
{
- assert!(!self.found_unsafe, "We should never see more than one fn");
+ assert!(!self.found_unsafe, "We should never see a fn when we already saw unsafe");
let is_unsafe = match fk {
intravisit::FnKind::ItemFn(_, _, unsafety, ..) => unsafety == hir::Unsafety::Unsafe,
intravisit::FnKind::Method(_, sig, ..) => sig.unsafety == hir::Unsafety::Unsafe,
@@ -129,20 +167,15 @@ fn fn_contains_unsafe<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, src: MirSource) ->
}
fn visit_block(&mut self, b: &'tcx hir::Block) {
- use rustc::hir::BlockCheckMode::*;
-
if self.found_unsafe { return; } // short-circuit
- match b.rules {
- UnsafeBlock(_) | PushUnsafeBlock(_) => {
- // We found an unsafe block.
- self.found_unsafe = true;
- }
- DefaultBlock | PopUnsafeBlock(_) => {
- // No unsafe block here, go on searching.
- intravisit::walk_block(self, b);
- }
- };
+ if block_is_unsafe(b) {
+ // We found an unsafe block. We can stop searching.
+ self.found_unsafe = true;
+ } else {
+ // No unsafe block here, go on searching.
+ intravisit::walk_block(self, b);
+ }
}
}
diff --git a/src/test/mir-opt/validate_1.rs b/src/test/mir-opt/validate_1.rs
index c8ea2bc2544..b85d9261e4a 100644
--- a/src/test/mir-opt/validate_1.rs
+++ b/src/test/mir-opt/validate_1.rs
@@ -21,8 +21,15 @@ impl Test {
fn main() {
let mut x = 0;
Test.foo(&mut x);
+
+ // Also test closures
+ let c = |x: &mut i32| { let y = &*x; *y };
+ c(&mut x);
}
+// FIXME: Also test code generated inside the closure, make sure it has validation. Unfortunately,
+// the interesting lines of code also contain name of the source file, so we cannot test for it.
+
// END RUST SOURCE
// START rustc.node10.EraseRegions.after.mir
// bb0: {
diff --git a/src/test/mir-opt/validate_4.rs b/src/test/mir-opt/validate_4.rs
new file mode 100644
index 00000000000..49acaccd86a
--- /dev/null
+++ b/src/test/mir-opt/validate_4.rs
@@ -0,0 +1,58 @@
+// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// ignore-tidy-linelength
+// compile-flags: -Z verbose -Z mir-emit-validate=1
+
+// Make sure unsafe fns and fns with an unsafe block only get restricted validation.
+
+unsafe fn write_42(x: *mut i32) -> bool {
+ *x = 42;
+ true
+}
+
+fn test(x: &mut i32) {
+ unsafe { write_42(x) };
+}
+
+fn main() {
+ test(&mut 0);
+
+ let test_closure = unsafe { |x: &mut i32| write_42(x) };
+ test_closure(&mut 0);
+}
+
+// FIXME: Also test code generated inside the closure, make sure it only does restricted validation
+// because it is entirely inside an unsafe block. Unfortunately, the interesting lines of code also
+// contain name of the source file, so we cannot test for it.
+
+// END RUST SOURCE
+// START rustc.node4.EraseRegions.after.mir
+// fn write_42(_1: *mut i32) -> bool {
+// bb0: {
+// Validate(Acquire, [_1: *mut i32]);
+// Validate(Release, [_1: *mut i32]);
+// return;
+// }
+// }
+// END rustc.node4.EraseRegions.after.mir
+// START rustc.node17.EraseRegions.after.mir
+// fn test(_1: &ReErased mut i32) -> () {
+// bb0: {
+// Validate(Acquire, [_1: &ReFree(DefId { krate: CrateNum(0), node: DefIndex(4) => validate_4/8cd878b::test[0] }, BrAnon(0)) mut i32]);
+// Validate(Release, [_1: &ReFree(DefId { krate: CrateNum(0), node: DefIndex(4) => validate_4/8cd878b::test[0] }, BrAnon(0)) mut i32]);
+// _3 = const write_42(_4) -> bb1;
+// }
+// bb1: {
+// Validate(Acquire, [_3: bool]);
+// Validate(Release, [_3: bool]);
+// }
+// }
+// END rustc.node17.EraseRegions.after.mir
diff --git a/src/test/mir-opt/validate_5.rs b/src/test/mir-opt/validate_5.rs
new file mode 100644
index 00000000000..1831f9dd713
--- /dev/null
+++ b/src/test/mir-opt/validate_5.rs
@@ -0,0 +1,44 @@
+// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// ignore-tidy-linelength
+// compile-flags: -Z verbose -Z mir-emit-validate=2
+
+// Make sure unsafe fns and fns with an unsafe block only get full validation.
+
+unsafe fn write_42(x: *mut i32) -> bool {
+ *x = 42;
+ true
+}
+
+fn test(x: &mut i32) {
+ unsafe { write_42(x) };
+}
+
+fn main() {
+ test(&mut 0);
+
+ let test_closure = unsafe { |x: &mut i32| write_42(x) };
+ test_closure(&mut 0);
+}
+
+// FIXME: Also test code generated inside the closure, make sure it has validation. Unfortunately,
+// the interesting lines of code also contain name of the source file, so we cannot test for it.
+
+// END RUST SOURCE
+// START rustc.node17.EraseRegions.after.mir
+// fn test(_1: &ReErased mut i32) -> () {
+// bb0: {
+// Validate(Acquire, [_1: &ReFree(DefId { krate: CrateNum(0), node: DefIndex(4) => validate_5/8cd878b::test[0] }, BrAnon(0)) mut i32]);
+// Validate(Release, [_4: *mut i32]);
+// _3 = const write_42(_4) -> bb1;
+// }
+// }
+// END rustc.node17.EraseRegions.after.mir
From 4310edb4cb106e725bb63e05b2cba21d6bb2a85f Mon Sep 17 00:00:00 2001
From: Ralf Jung <post@ralfj.de>
Date: Tue, 1 Aug 2017 08:48:28 -0700
Subject: [PATCH 25/30] handle tuple struct ctors
---
src/librustc_mir/transform/add_validation.rs | 6 ++++-
src/test/mir-opt/validate_1.rs | 24 ++++++++++----------
2 files changed, 17 insertions(+), 13 deletions(-)
diff --git a/src/librustc_mir/transform/add_validation.rs b/src/librustc_mir/transform/add_validation.rs
index 6f136624f0a..578a63e44b0 100644
--- a/src/librustc_mir/transform/add_validation.rs
+++ b/src/librustc_mir/transform/add_validation.rs
@@ -128,7 +128,7 @@ fn fn_contains_unsafe<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, src: MirSource) ->
match tcx.hir.find(cur) {
Some(Node::NodeExpr(&hir::Expr { node: hir::ExprBlock(ref block), ..})) => {
if block_is_unsafe(&*block) {
- // We can bail out here.
+ // Found an unsafe block, we can bail out here.
return true;
}
}
@@ -138,6 +138,10 @@ fn fn_contains_unsafe<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, src: MirSource) ->
// Finally, visit the closure itself.
finder.visit_expr(item);
}
+ Some(Node::NodeStructCtor(_)) => {
+ // Auto-generated tuple struct ctor. Cannot contain unsafe code.
+ return false;
+ },
Some(_) | None =>
bug!("Expected function, method or closure, found {}",
tcx.hir.node_to_string(fn_node_id)),
diff --git a/src/test/mir-opt/validate_1.rs b/src/test/mir-opt/validate_1.rs
index b85d9261e4a..542ba87fef4 100644
--- a/src/test/mir-opt/validate_1.rs
+++ b/src/test/mir-opt/validate_1.rs
@@ -11,7 +11,7 @@
// ignore-tidy-linelength
// compile-flags: -Z verbose -Z mir-emit-validate=1
-struct Test;
+struct Test(i32);
impl Test {
// Make sure we run the pass on a method, not just on bare functions.
@@ -20,7 +20,7 @@ impl Test {
fn main() {
let mut x = 0;
- Test.foo(&mut x);
+ Test(0).foo(&mut x);
// Also test closures
let c = |x: &mut i32| { let y = &*x; *y };
@@ -31,29 +31,29 @@ fn main() {
// the interesting lines of code also contain name of the source file, so we cannot test for it.
// END RUST SOURCE
-// START rustc.node10.EraseRegions.after.mir
+// START rustc.node12.EraseRegions.after.mir
// bb0: {
// Validate(Acquire, [_1: &ReFree(DefId { krate: CrateNum(0), node: DefIndex(5) => validate_1/8cd878b::{{impl}}[0]::foo[0] }, BrAnon(0)) Test, _2: &ReFree(DefId { krate: CrateNum(0), node: DefIndex(5) => validate_1/8cd878b::{{impl}}[0]::foo[0] }, BrAnon(1)) mut i32]);
// return;
// }
-// END rustc.node10.EraseRegions.after.mir
-// START rustc.node21.EraseRegions.after.mir
+// END rustc.node12.EraseRegions.after.mir
+// START rustc.node23.EraseRegions.after.mir
// fn main() -> () {
// bb0: {
-// Validate(Suspend(ReScope(Misc(NodeId(30)))), [_1: i32]);
+// Validate(Suspend(ReScope(Misc(NodeId(34)))), [_1: i32]);
// _6 = &ReErased mut _1;
-// Validate(Acquire, [(*_6): i32/ReScope(Misc(NodeId(30)))]);
-// Validate(Suspend(ReScope(Misc(NodeId(30)))), [(*_6): i32/ReScope(Misc(NodeId(30)))]);
+// Validate(Acquire, [(*_6): i32/ReScope(Misc(NodeId(34)))]);
+// Validate(Suspend(ReScope(Misc(NodeId(34)))), [(*_6): i32/ReScope(Misc(NodeId(34)))]);
// _5 = &ReErased mut (*_6);
-// Validate(Acquire, [(*_5): i32/ReScope(Misc(NodeId(30)))]);
-// Validate(Release, [_3: &ReScope(Misc(NodeId(30))) Test, _5: &ReScope(Misc(NodeId(30))) mut i32]);
+// Validate(Acquire, [(*_5): i32/ReScope(Misc(NodeId(34)))]);
+// Validate(Release, [_3: &ReScope(Misc(NodeId(34))) Test, _5: &ReScope(Misc(NodeId(34))) mut i32]);
// _2 = const Test::foo(_3, _5) -> bb1;
// }
//
// bb1: {
// Validate(Acquire, [_2: ()]);
-// EndRegion(ReScope(Misc(NodeId(30))));
+// EndRegion(ReScope(Misc(NodeId(34))));
// return;
// }
// }
-// END rustc.node21.EraseRegions.after.mir
+// END rustc.node23.EraseRegions.after.mir
From 8f910bcbbcbd47b372670e0a347fcde340d25e5e Mon Sep 17 00:00:00 2001
From: Ralf Jung <post@ralfj.de>
Date: Tue, 1 Aug 2017 09:22:58 -0700
Subject: [PATCH 26/30] handle trait items as well
---
src/librustc_mir/transform/add_validation.rs | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/librustc_mir/transform/add_validation.rs b/src/librustc_mir/transform/add_validation.rs
index 578a63e44b0..374b658dfc7 100644
--- a/src/librustc_mir/transform/add_validation.rs
+++ b/src/librustc_mir/transform/add_validation.rs
@@ -112,6 +112,7 @@ fn fn_contains_unsafe<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, src: MirSource) ->
match tcx.hir.find(fn_node_id) {
Some(Node::NodeItem(item)) => finder.visit_item(item),
Some(Node::NodeImplItem(item)) => finder.visit_impl_item(item),
+ Some(Node::NodeTraitItem(item)) => finder.visit_trait_item(item),
Some(Node::NodeExpr(item)) => {
// This is a closure.
// We also have to walk up the parents and check that there is no unsafe block
From c5154d036d09b5274a9e7bcf9f343520728d4c07 Mon Sep 17 00:00:00 2001
From: Ralf Jung <post@ralfj.de>
Date: Tue, 1 Aug 2017 13:07:45 -0700
Subject: [PATCH 27/30] use FnLike to recognize functions for us
---
src/librustc/hir/map/blocks.rs | 12 ++
src/librustc_mir/transform/add_validation.rs | 125 +++++++++----------
2 files changed, 73 insertions(+), 64 deletions(-)
diff --git a/src/librustc/hir/map/blocks.rs b/src/librustc/hir/map/blocks.rs
index 661798a8250..1b7eb158567 100644
--- a/src/librustc/hir/map/blocks.rs
+++ b/src/librustc/hir/map/blocks.rs
@@ -192,6 +192,18 @@ impl<'a> FnLikeNode<'a> {
}
}
+ pub fn unsafety(self) -> ast::Unsafety {
+ match self.kind() {
+ FnKind::ItemFn(_, _, unsafety, ..) => {
+ unsafety
+ }
+ FnKind::Method(_, m, ..) => {
+ m.unsafety
+ }
+ _ => ast::Unsafety::Normal
+ }
+ }
+
pub fn kind(self) -> FnKind<'a> {
let item = |p: ItemFnParts<'a>| -> FnKind<'a> {
FnKind::ItemFn(p.name, p.generics, p.unsafety, p.constness, p.abi, p.vis, p.attrs)
diff --git a/src/librustc_mir/transform/add_validation.rs b/src/librustc_mir/transform/add_validation.rs
index 374b658dfc7..2afaa070118 100644
--- a/src/librustc_mir/transform/add_validation.rs
+++ b/src/librustc_mir/transform/add_validation.rs
@@ -14,8 +14,6 @@
//! of MIR building, and only after this pass we think of the program has having the
//! normal MIR semantics.
-use syntax_pos::Span;
-use syntax::ast::NodeId;
use rustc::ty::{self, TyCtxt, RegionKind};
use rustc::hir;
use rustc::mir::*;
@@ -84,9 +82,11 @@ fn lval_context<'a, 'tcx, D>(
/// Check if this function contains an unsafe block or is an unsafe function.
fn fn_contains_unsafe<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, src: MirSource) -> bool {
- use rustc::hir::intravisit::{self, Visitor};
+ use rustc::hir::intravisit::{self, Visitor, FnKind};
+ use rustc::hir::map::blocks::FnLikeNode;
use rustc::hir::map::Node;
+ /// Decide if this is an unsafe block
fn block_is_unsafe(block: &hir::Block) -> bool {
use rustc::hir::BlockCheckMode::*;
@@ -98,77 +98,74 @@ fn fn_contains_unsafe<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, src: MirSource) ->
}
}
- let fn_node_id = match src {
- MirSource::Fn(node_id) => node_id,
+ /// Decide if this FnLike is a closure
+ fn fn_is_closure<'a>(fn_like: FnLikeNode<'a>) -> bool {
+ match fn_like.kind() {
+ FnKind::Closure(_) => true,
+ FnKind::Method(..) | FnKind::ItemFn(..) => false,
+ }
+ }
+
+ let fn_like = match src {
+ MirSource::Fn(node_id) => {
+ match FnLikeNode::from_node(tcx.hir.get(node_id)) {
+ Some(fn_like) => fn_like,
+ None => return false, // e.g. struct ctor shims -- such auto-generated code cannot
+ // contain unsafe.
+ }
+ },
_ => return false, // only functions can have unsafe
};
- struct FindUnsafe<'b, 'tcx> where 'tcx : 'b {
- map: &'b hir::map::Map<'tcx>,
- found_unsafe: bool,
+ // Test if the function is marked unsafe.
+ if fn_like.unsafety() == hir::Unsafety::Unsafe {
+ return true;
}
- let mut finder = FindUnsafe { map: &tcx.hir, found_unsafe: false };
- // Run the visitor on the NodeId we got. Seems like there is no uniform way to do that.
- match tcx.hir.find(fn_node_id) {
- Some(Node::NodeItem(item)) => finder.visit_item(item),
- Some(Node::NodeImplItem(item)) => finder.visit_impl_item(item),
- Some(Node::NodeTraitItem(item)) => finder.visit_trait_item(item),
- Some(Node::NodeExpr(item)) => {
- // This is a closure.
- // We also have to walk up the parents and check that there is no unsafe block
- // there.
- let mut cur = fn_node_id;
- loop {
- // Go further upwards.
- let parent = tcx.hir.get_parent_node(cur);
- if cur == parent {
+
+ // For closures, we need to walk up the parents and see if we are inside an unsafe fn or
+ // unsafe block.
+ if fn_is_closure(fn_like) {
+ let mut cur = fn_like.id();
+ loop {
+ // Go further upwards.
+ let parent = tcx.hir.get_parent_node(cur);
+ if cur == parent {
+ bug!("Closures muts be inside a non-closure fn_like");
+ }
+ cur = parent;
+ // Check if this is an unsafe block
+ match tcx.hir.find(cur) {
+ Some(Node::NodeExpr(&hir::Expr { node: hir::ExprBlock(ref block), ..})) => {
+ if block_is_unsafe(&*block) {
+ // Found an unsafe block, we can bail out here.
+ return true;
+ }
+ }
+ _ => {},
+ }
+ // Check if this is a non-closure fn_like, at which point we have to stop moving up
+ if let Some(fn_like) = FnLikeNode::from_node(tcx.hir.get(cur)) {
+ if !fn_is_closure(fn_like) {
+ if fn_like.unsafety() == hir::Unsafety::Unsafe {
+ return true;
+ }
break;
}
- cur = parent;
- // Check if this is a a block
- match tcx.hir.find(cur) {
- Some(Node::NodeExpr(&hir::Expr { node: hir::ExprBlock(ref block), ..})) => {
- if block_is_unsafe(&*block) {
- // Found an unsafe block, we can bail out here.
- return true;
- }
- }
- _ => {},
- }
}
- // Finally, visit the closure itself.
- finder.visit_expr(item);
}
- Some(Node::NodeStructCtor(_)) => {
- // Auto-generated tuple struct ctor. Cannot contain unsafe code.
- return false;
- },
- Some(_) | None =>
- bug!("Expected function, method or closure, found {}",
- tcx.hir.node_to_string(fn_node_id)),
- };
+ }
- impl<'b, 'tcx> Visitor<'tcx> for FindUnsafe<'b, 'tcx> {
+ // Visit the entire body of the function and check for unsafe blocks in there
+ struct FindUnsafe {
+ found_unsafe: bool,
+ }
+ let mut finder = FindUnsafe { found_unsafe: false };
+ // Run the visitor on the NodeId we got. Seems like there is no uniform way to do that.
+ finder.visit_body(tcx.hir.body(fn_like.body()));
+
+ impl<'tcx> Visitor<'tcx> for FindUnsafe {
fn nested_visit_map<'this>(&'this mut self) -> intravisit::NestedVisitorMap<'this, 'tcx> {
- intravisit::NestedVisitorMap::OnlyBodies(self.map)
- }
-
- fn visit_fn(&mut self, fk: intravisit::FnKind<'tcx>, fd: &'tcx hir::FnDecl,
- b: hir::BodyId, s: Span, id: NodeId)
- {
- assert!(!self.found_unsafe, "We should never see a fn when we already saw unsafe");
- let is_unsafe = match fk {
- intravisit::FnKind::ItemFn(_, _, unsafety, ..) => unsafety == hir::Unsafety::Unsafe,
- intravisit::FnKind::Method(_, sig, ..) => sig.unsafety == hir::Unsafety::Unsafe,
- intravisit::FnKind::Closure(_) => false,
- };
- if is_unsafe {
- // This is unsafe, and we are done.
- self.found_unsafe = true;
- } else {
- // Go on searching.
- intravisit::walk_fn(self, fk, fd, b, s, id)
- }
+ intravisit::NestedVisitorMap::None
}
fn visit_block(&mut self, b: &'tcx hir::Block) {
From a8129d128c314975d4d34a47e9cb7127de0d0dbc Mon Sep 17 00:00:00 2001
From: Ralf Jung <post@ralfj.de>
Date: Tue, 1 Aug 2017 13:14:32 -0700
Subject: [PATCH 28/30] add a closure inside an unsafe fn to the tests
---
src/test/mir-opt/validate_4.rs | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/test/mir-opt/validate_4.rs b/src/test/mir-opt/validate_4.rs
index 49acaccd86a..591de975740 100644
--- a/src/test/mir-opt/validate_4.rs
+++ b/src/test/mir-opt/validate_4.rs
@@ -14,6 +14,8 @@
// Make sure unsafe fns and fns with an unsafe block only get restricted validation.
unsafe fn write_42(x: *mut i32) -> bool {
+ let test_closure = |x: *mut i32| *x = 23;
+ test_closure(x);
*x = 42;
true
}
@@ -43,7 +45,7 @@ fn main() {
// }
// }
// END rustc.node4.EraseRegions.after.mir
-// START rustc.node17.EraseRegions.after.mir
+// START rustc.node31.EraseRegions.after.mir
// fn test(_1: &ReErased mut i32) -> () {
// bb0: {
// Validate(Acquire, [_1: &ReFree(DefId { krate: CrateNum(0), node: DefIndex(4) => validate_4/8cd878b::test[0] }, BrAnon(0)) mut i32]);
@@ -55,4 +57,4 @@ fn main() {
// Validate(Release, [_3: bool]);
// }
// }
-// END rustc.node17.EraseRegions.after.mir
+// END rustc.node31.EraseRegions.after.mir
From 321a72c1c1329415c6b94cc18b42b05c1ce8b59d Mon Sep 17 00:00:00 2001
From: Ralf Jung <post@ralfj.de>
Date: Tue, 1 Aug 2017 18:26:28 -0700
Subject: [PATCH 29/30] closure unsafety check: stop moving up when we hit an
item
---
src/librustc_mir/transform/add_validation.rs | 30 ++++++++++----------
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/src/librustc_mir/transform/add_validation.rs b/src/librustc_mir/transform/add_validation.rs
index 2afaa070118..bbd2829c303 100644
--- a/src/librustc_mir/transform/add_validation.rs
+++ b/src/librustc_mir/transform/add_validation.rs
@@ -128,29 +128,29 @@ fn fn_contains_unsafe<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, src: MirSource) ->
let mut cur = fn_like.id();
loop {
// Go further upwards.
- let parent = tcx.hir.get_parent_node(cur);
- if cur == parent {
- bug!("Closures muts be inside a non-closure fn_like");
+ cur = tcx.hir.get_parent_node(cur);
+ let node = tcx.hir.get(cur);
+ // Check if this is an unsafe function
+ if let Some(fn_like) = FnLikeNode::from_node(node) {
+ if !fn_is_closure(fn_like) {
+ if fn_like.unsafety() == hir::Unsafety::Unsafe {
+ return true;
+ }
+ }
}
- cur = parent;
- // Check if this is an unsafe block
- match tcx.hir.find(cur) {
- Some(Node::NodeExpr(&hir::Expr { node: hir::ExprBlock(ref block), ..})) => {
+ // Check if this is an unsafe block, or an item
+ match node {
+ Node::NodeExpr(&hir::Expr { node: hir::ExprBlock(ref block), ..}) => {
if block_is_unsafe(&*block) {
// Found an unsafe block, we can bail out here.
return true;
}
}
- _ => {},
- }
- // Check if this is a non-closure fn_like, at which point we have to stop moving up
- if let Some(fn_like) = FnLikeNode::from_node(tcx.hir.get(cur)) {
- if !fn_is_closure(fn_like) {
- if fn_like.unsafety() == hir::Unsafety::Unsafe {
- return true;
- }
+ Node::NodeItem(..) => {
+ // No walking up beyond items. This makes sure the loop always terminates.
break;
}
+ _ => {},
}
}
}
From 7d8dc7a979975ab6d8aab29cfa0b69e8a64f1280 Mon Sep 17 00:00:00 2001
From: Ralf Jung <post@ralfj.de>
Date: Tue, 1 Aug 2017 21:06:33 -0700
Subject: [PATCH 30/30] also release-validate return value before a call
---
src/librustc_mir/transform/add_validation.rs | 21 ++++++++++++--------
src/test/mir-opt/validate_1.rs | 2 +-
src/test/mir-opt/validate_3.rs | 2 +-
src/test/mir-opt/validate_5.rs | 2 +-
4 files changed, 16 insertions(+), 11 deletions(-)
diff --git a/src/librustc_mir/transform/add_validation.rs b/src/librustc_mir/transform/add_validation.rs
index bbd2829c303..52c2eaa7cb6 100644
--- a/src/librustc_mir/transform/add_validation.rs
+++ b/src/librustc_mir/transform/add_validation.rs
@@ -248,18 +248,23 @@ impl MirPass for AddValidation {
match block_data.terminator {
Some(Terminator { kind: TerminatorKind::Call { ref args, ref destination, .. },
source_info }) => {
- // Before the call: Release all arguments
+ // Before the call: Release all arguments *and* the return value.
+ // The callee may write into the return value! Note that this relies
+ // on "release of uninitialized" to be a NOP.
if !restricted_validation {
let release_stmt = Statement {
source_info,
kind: StatementKind::Validate(ValidationOp::Release,
- args.iter().filter_map(|op| {
- match op {
- &Operand::Consume(ref lval) =>
- Some(lval_to_operand(lval.clone())),
- &Operand::Constant(..) => { None },
- }
- }).collect())
+ destination.iter().map(|dest| lval_to_operand(dest.0.clone()))
+ .chain(
+ args.iter().filter_map(|op| {
+ match op {
+ &Operand::Consume(ref lval) =>
+ Some(lval_to_operand(lval.clone())),
+ &Operand::Constant(..) => { None },
+ }
+ })
+ ).collect())
};
block_data.statements.push(release_stmt);
}
diff --git a/src/test/mir-opt/validate_1.rs b/src/test/mir-opt/validate_1.rs
index 542ba87fef4..9ac76a5f4ea 100644
--- a/src/test/mir-opt/validate_1.rs
+++ b/src/test/mir-opt/validate_1.rs
@@ -46,7 +46,7 @@ fn main() {
// Validate(Suspend(ReScope(Misc(NodeId(34)))), [(*_6): i32/ReScope(Misc(NodeId(34)))]);
// _5 = &ReErased mut (*_6);
// Validate(Acquire, [(*_5): i32/ReScope(Misc(NodeId(34)))]);
-// Validate(Release, [_3: &ReScope(Misc(NodeId(34))) Test, _5: &ReScope(Misc(NodeId(34))) mut i32]);
+// Validate(Release, [_2: (), _3: &ReScope(Misc(NodeId(34))) Test, _5: &ReScope(Misc(NodeId(34))) mut i32]);
// _2 = const Test::foo(_3, _5) -> bb1;
// }
//
diff --git a/src/test/mir-opt/validate_3.rs b/src/test/mir-opt/validate_3.rs
index 100fae5c678..9140cf5768f 100644
--- a/src/test/mir-opt/validate_3.rs
+++ b/src/test/mir-opt/validate_3.rs
@@ -38,7 +38,7 @@ fn main() {
// Validate(Suspend(ReScope(Misc(NodeId(46)))), [(*_5): i32/ReScope(Misc(NodeId(46))) (imm)]);
// _4 = &ReErased (*_5);
// Validate(Acquire, [(*_4): i32/ReScope(Misc(NodeId(46))) (imm)]);
-// Validate(Release, [_4: &ReScope(Misc(NodeId(46))) i32]);
+// Validate(Release, [_3: (), _4: &ReScope(Misc(NodeId(46))) i32]);
// _3 = const foo(_4) -> bb1;
// }
// bb1: {
diff --git a/src/test/mir-opt/validate_5.rs b/src/test/mir-opt/validate_5.rs
index 1831f9dd713..e9919af9fd3 100644
--- a/src/test/mir-opt/validate_5.rs
+++ b/src/test/mir-opt/validate_5.rs
@@ -37,7 +37,7 @@ fn main() {
// fn test(_1: &ReErased mut i32) -> () {
// bb0: {
// Validate(Acquire, [_1: &ReFree(DefId { krate: CrateNum(0), node: DefIndex(4) => validate_5/8cd878b::test[0] }, BrAnon(0)) mut i32]);
-// Validate(Release, [_4: *mut i32]);
+// Validate(Release, [_3: bool, _4: *mut i32]);
// _3 = const write_42(_4) -> bb1;
// }
// }