Skip to content

Commit

Permalink
Correctly determine which terminators can have side effects
Browse files Browse the repository at this point in the history
The previous code ignored that `Drop` terminators could execute custom
`Drop` implementations which might mutate the environment. Now we
correctly implement `has_side_effects` and tests that custom `Drop`
impls are recorded as an indirect definition.
  • Loading branch information
ecstatic-morse committed Aug 8, 2019
1 parent cfd9320 commit 047e7ca
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 8 deletions.
43 changes: 36 additions & 7 deletions src/librustc_mir/dataflow/impls/reaching_defs.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
use rustc::mir::visit::{PlaceContext, Visitor};
use rustc::mir::visit::{MutatingUseContext, NonMutatingUseContext, NonUseContext};
use rustc::mir::{self, Local, Location, Place, PlaceBase};
use rustc::ty::{self, TyCtxt, ParamEnv};
use rustc_data_structures::bit_set::BitSet;
use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::indexed_vec::{Idx, IndexVec};
use rustc_target::spec::abi::Abi;

use smallvec::SmallVec;
use syntax::symbol::sym;

use super::{BitDenotation, GenKillSet, BottomValue};

Expand Down Expand Up @@ -74,16 +78,41 @@ pub struct ReachingDefinitions {
at_location: FxHashMap<Location, SmallVec<[DefIndex; 4]>>,
}

fn is_call_with_side_effects(terminator: &mir::Terminator<'tcx>) -> bool {
if let mir::TerminatorKind::Call { .. } = terminator.kind {
true
} else {
false
fn has_side_effects(
tcx: TyCtxt<'tcx>,
body: &mir::Body<'tcx>,
param_env: ParamEnv<'tcx>,
terminator: &mir::Terminator<'tcx>,
) -> bool {
match &terminator.kind {
mir::TerminatorKind::Call { .. } => true,

// Types with special drop glue may mutate their environment.
| mir::TerminatorKind::Drop { location: place, .. }
| mir::TerminatorKind::DropAndReplace { location: place, .. }
=> place.ty(body, tcx).ty.needs_drop(tcx, param_env),

| mir::TerminatorKind::Goto { .. }
| mir::TerminatorKind::SwitchInt { .. }
| mir::TerminatorKind::Resume
| mir::TerminatorKind::Abort
| mir::TerminatorKind::Return
| mir::TerminatorKind::Unreachable
| mir::TerminatorKind::Assert { .. }
| mir::TerminatorKind::FalseEdges { .. }
| mir::TerminatorKind::FalseUnwind { .. }
=> false,

// FIXME: I don't know the semantics around these so assume that they may mutate their
// environment.
| mir::TerminatorKind::Yield { .. }
| mir::TerminatorKind::GeneratorDrop
=> true,
}
}

impl ReachingDefinitions {
pub fn new(body: &mir::Body<'_>) -> Self {
pub fn new<'tcx>(tcx: TyCtxt<'tcx>, body: &mir::Body<'tcx>, param_env: ParamEnv<'tcx>) -> Self {
let mut ret = ReachingDefinitions {
all: IndexVec::new(),
for_local_direct: IndexVec::from_elem(Vec::new(), &body.local_decls),
Expand All @@ -102,7 +131,7 @@ impl ReachingDefinitions {
let blocks_with_side_effects = body
.basic_blocks()
.iter_enumerated()
.filter(|(_, data)| is_call_with_side_effects(data.terminator()));
.filter(|(_, data)| has_side_effects(tcx, body, param_env, data.terminator()));

for (block, data) in blocks_with_side_effects {
let term_loc = Location { block, statement_index: data.statements.len() };
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/transform/rustc_peek.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ impl MirPass for SanityCheck {
|_bd, i| DebugFormatted::new(&i));
let flow_reaching_defs =
do_dataflow(tcx, body, def_id, &attributes, &dead_unwinds,
ReachingDefinitions::new(body),
ReachingDefinitions::new(tcx, body, tcx.param_env(def_id)),
|bd, i| DebugFormatted::new(&bd.get(i).location));
let flow_use_def_chain =
UseDefChain::new(body, &flow_reaching_defs, &flow_borrowed_locals);
Expand Down
26 changes: 26 additions & 0 deletions src/test/ui/mir-dataflow/reaching-defs-drop.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
#![feature(core_intrinsics, rustc_attrs)]
use std::intrinsics::rustc_peek;

struct NoSideEffectsInDrop<'a>(&'a mut u32);
struct SideEffectsInDrop<'a>(&'a mut u32);

impl Drop for SideEffectsInDrop<'_> {
fn drop(&mut self) {
*self.0 = 42
}
}

#[rustc_mir(rustc_peek_use_def_chain, stop_after_dataflow, borrowck_graphviz_postflow="flow.dot")]
fn main() {
let mut x;
x=0;

NoSideEffectsInDrop(&mut x);
SideEffectsInDrop(&mut x);

// The ";" on line 19 is the point at which `SideEffectsInDrop` is dropped.
unsafe { rustc_peek(x); }
//~^ ERROR rustc_peek: [16: "x=0", 19: ";"]

assert_eq!(x, 42);
}
10 changes: 10 additions & 0 deletions src/test/ui/mir-dataflow/reaching-defs-drop.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
error: rustc_peek: [16: "x=0", 19: ";"]
--> $DIR/reaching-defs-drop.rs:22:14
|
LL | unsafe { rustc_peek(x); }
| ^^^^^^^^^^^^^

error: stop_after_dataflow ended compilation

error: aborting due to 2 previous errors

0 comments on commit 047e7ca

Please sign in to comment.