Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
48 commits
Select commit Hold shift + click to select a range
cd81b60
fix bug in `debug!` output from `rustc::middle::dataflow`
pnkfelix Apr 28, 2016
bfe789c
fixes to `librustc_borrowck::bitslice::bits_to_string`, used for grap…
pnkfelix Apr 28, 2016
306ca4c
`rustc_mir::pretty`: factor out scope entry/exit annotation computation.
pnkfelix Apr 28, 2016
90b7a86
`rustc_mir::pretty` refactoring: break `fn write_fn_intro` into two r…
pnkfelix Apr 28, 2016
4446e79
Expose pretty print routines that accept just `mir` (no need for a `N…
pnkfelix Apr 28, 2016
129b371
One-line doc clarification for representation of unit type `()`.
pnkfelix Apr 28, 2016
b4972b0
Add helper method for getting the dataflow results at exit from a bas…
pnkfelix Apr 28, 2016
79ab855
Remove `&self` parameter from `DataflowOperator::initial_value`.
pnkfelix Apr 28, 2016
d03b341
Unit struct defns for 3 dataflow analyses for `borrowck::mir::dataflow`.
pnkfelix Apr 28, 2016
6c72c5f
`borrowck::mir::gather_moves`: create MovePaths for lvalues even if u…
pnkfelix May 2, 2016
c73f351
Revised mir-dataflow.
pnkfelix May 2, 2016
3bb5984
Adding magic `rustc_peek` intrinsic that other code can repurpose to
pnkfelix May 11, 2016
5839e6b
Add ability to unit-test dataflow results via `rustc_peek` intrinsic.
pnkfelix May 11, 2016
8956789
Little unit tests for MIR dataflow analysis.
pnkfelix May 6, 2016
ee44f7e
`DefinitelyInitializedLvals` dataflow op (goal: move away from `Maybe…
pnkfelix May 13, 2016
cd71b0d
core::intrinsics: fix typo noted during review.
pnkfelix May 16, 2016
b0b1f4d
Fix comments in `mir::dataflow::sanity_check`.
pnkfelix May 17, 2016
90a6526
Escape asterix in markdown file to side-step it being interpreted as …
pnkfelix May 17, 2016
8999e87
`mir::dataflow::sanity_check`: removed hackish `tmp = val` propagatio…
pnkfelix May 17, 2016
582f060
markdown fix suggested during review.
pnkfelix May 20, 2016
aaad8a2
`mir::dataflow::sanity_check`: Factor out `fn is_rustc_peek` helper r…
pnkfelix May 20, 2016
9fcbe2a
fix comment in `impl DataflowOperator for MaybeUninitializedLvals`.
pnkfelix May 20, 2016
011c37d
`borrowck::mir`: alpha-renamed DropFlagState variant names.
pnkfelix May 20, 2016
9c468f4
Added comment pointing out somewhat subtle initialization in `fn star…
pnkfelix May 20, 2016
a7e3204
`mir::dataflow` arielb1 review feedback
pnkfelix May 20, 2016
5811950
Review feedback.
pnkfelix May 20, 2016
59008cb
review feedback: fix some index-mismatch bugs pointed out by arielb1.
pnkfelix May 20, 2016
f18bafd
Refactor `bitslice`: distinguish `usize` for indexing vs word type be…
pnkfelix May 20, 2016
0796ee7
add `indexed_set` mod for typed wrappers around bitarrays representin…
pnkfelix May 23, 2016
71af40b
revised mir-dataflow so bitvectors carry a phantom type for their ind…
pnkfelix May 23, 2016
c48650d
bug fix to `borrowck::indexed_set`: wanted bit-count not byte-count.
pnkfelix May 24, 2016
b8c6d1c
Fix comment within sanity_check.
pnkfelix May 24, 2016
ede2958
`mir::dataflow::sanity_check`: extract an `fn each_block` to simplify…
pnkfelix May 24, 2016
ae09c5e
Moved the four impls of `BitDenotation` to their own module, `mod imp…
pnkfelix May 24, 2016
221cce9
move the `tcx` and `mir` parts of associated `Ctxt` onto each `BitDen…
pnkfelix May 24, 2016
0cf5f10
Replaced use of `interpret` method in `mir::dataflow::graphviz` with …
pnkfelix May 24, 2016
fdf80bc
Removed `type Bit` and `fn interpret` items from `trait BitDenotation`.
pnkfelix May 24, 2016
ad1294d
threaded a `ty::ParameterEnvironment` for the current node id via the…
pnkfelix May 24, 2016
a82676e
placate tidy in `mir::dataflow::graphviz`.
pnkfelix May 24, 2016
ac6ea44
placate tidy in `mir::dataflow`.
pnkfelix May 24, 2016
fe49b41
placate tidy in `mir::gather_moves`.
pnkfelix May 24, 2016
4412c7a
placate tidy in compile-fail test.
pnkfelix May 24, 2016
d9680f5
Fix some comments.
pnkfelix May 25, 2016
25f37fd
Add notes that data-structures should potentially move to different c…
pnkfelix May 25, 2016
58f1a49
remove unnecessary use of `indexed_set::Indexed` trait.
pnkfelix May 25, 2016
a28771c
remove `indexed_set::Indexed` trait.
pnkfelix May 25, 2016
ad0e6ad
fixes to `indexed_set`: add comments and fix `PhantomData` def'n.
pnkfelix May 25, 2016
df5c116
Alpha rename `OwnIdxSet` to `IdxSetBuf`.
pnkfelix May 25, 2016
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
193 changes: 193 additions & 0 deletions src/librustc_borrowck/borrowck/mir/dataflow/sanity_check.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,193 @@
// Copyright 2012-2016 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.

use syntax::abi::{Abi};
use syntax::ast;

use rustc::ty::{self, TyCtxt};
use rustc::mir::repr::{self, Mir};

use bitslice::BitSlice;

use super::super::gather_moves::MovePath;
use super::{bitwise, Union, Subtract};
use super::BitDenotation;
use super::DataflowResults;
use super::HasMoveData;

pub fn sanity_check_via_rustc_peek<'a, 'tcx, O>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
mir: &Mir<'tcx>,
id: ast::NodeId,
_attributes: &[ast::Attribute],
flow_ctxt: &O::Ctxt,
results: &DataflowResults<O>)
where O: BitDenotation<Bit=MovePath<'tcx>>, O::Ctxt: HasMoveData<'tcx>
{
debug!("sanity_check_via_rustc_peek id: {:?}", id);
// FIXME: this is not DRY. Figure out way to abstract this and
// `dataflow::build_sets`. (But see note below about how the
// behavior of this traversal is a bit different than that
// performed by `build_sets`.)

let blocks = mir.all_basic_blocks();
'next_block: for bb in blocks {
let bb_data = mir.basic_block_data(bb);
let &repr::BasicBlockData { ref statements,
ref terminator,
is_cleanup: _ } = bb_data;

let (args, span) = if let Some(repr::Terminator { ref kind, span, .. }) = *terminator {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: I'd prefer to see this "is this rust_peek code" in a helper fn :)

if let repr::TerminatorKind::Call { func: ref oper, ref args, .. } = *kind
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extract-function please

{
if let repr::Operand::Constant(ref func) = *oper
{
if let ty::TyFnDef(def_id, _, &ty::BareFnTy { abi, .. }) = func.ty.sty
{
let name = tcx.item_name(def_id);
if abi == Abi::RustIntrinsic || abi == Abi::PlatformIntrinsic {
if name.as_str() == "rustc_peek" {
(args, span)
} else {
continue;
}
} else {
continue;
}
} else {
continue;
}
} else {
continue;
}
} else {
continue;
}
} else {
continue;
};
assert!(args.len() == 1);
let peek_arg_lval = match args[0] {
repr::Operand::Consume(ref lval @ repr::Lvalue::Temp(_)) => {
lval
}
repr::Operand::Consume(_) => {
Copy link
Contributor

@arielb1 arielb1 May 18, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: unify arms and print the operand in the bug make this an err.

bug!("dataflow::sanity_check cannot feed a non-temp to rustc_peek.");
}
repr::Operand::Constant(_) => {
bug!("dataflow::sanity_check cannot feed a constant to rustc_peek.");
}
};

let mut entry = results.0.sets.on_entry_set_for(bb.index()).to_owned();
let mut gen = results.0.sets.gen_set_for(bb.index()).to_owned();
let mut kill = results.0.sets.kill_set_for(bb.index()).to_owned();

let move_data = flow_ctxt.move_data();

// Emulate effect of all statements in the block up to (but
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thou shalt not have comments that lie. This abstract-interprets to the borrow, not to the call.

// not including) the assignment to `peek_arg_lval`. Do *not*
// include terminator (since we are peeking the state of the
// argument at time immediate preceding Call to `rustc_peek`).

let mut sets = super::BlockSets { on_entry: &mut entry[..],
gen_set: &mut gen[..],
kill_set: &mut kill[..] };
// Unfortunately if we just re-do the same thing that dataflow does, then
// it will always appear like Lvalues are initialized; e.g. in
// a case like:
//
// <bitset maps var1 to 0>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this talking about? rustc_peek(&var1) should be translated to

        tmp1 = &var0;
        tmp0 = rustc_peek(tmp1) -> bb1;

If it copies the lvalue before taking a reference, that would be totally broken (consider Cell). So it doesn't. Unless something funny is going on here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh you're right; I didn't update that comment correctly (it was originally authored for a different version of rustc_peek that looked like rustc_peek(var1) (instead of rustc_peek(&var1)), and I erroneously translated its content without thinking about what the actual translation of rustc_peek(&var1) is).

(The original version of rustc_peek had a more fragile API, where one would write rustc_peek(expr) (instead of rustc_peek(&expr)).)


The main point I'm trying to make is this: even with the corrected translation that you wrote:

tmp1 = &var0;
tmp0 = rustc_peek(tmp1) -> bb1;

The normal (naive) dataflow GEN/KILL sets are going to say that tmp1 is initialized. If the semantics of rustc_peek were a naive "look directly at your argument's dataflow bit state", then rustc_peek would query the state of tmp1, which would always indicate that it has been initialized under the naive rules.

So I had written a comment about a work-around I was using for the naive system.


With the new API the story is somewhat related, but the example I gave in the comment isn't helping explain things.

I will update it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guessed. Yeah, you want the state of var0, not of tmp1. I think you could just go to the initialization of tmp1 and use that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I also expected to find the definition of tmp1 when @pnkfelix and I discussed in the work week; but this solution works too.)

Copy link
Contributor

@nikomatsakis nikomatsakis May 17, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the reason not to do rustc_peek(&x) is that this code runs before borrowck anyway, so e.g. moves don't matter? never mind, I see you are doing &x

// tmp13 = var1;
// tmp14 = &tmp13;
// rustc_peek(tmp14)
//
// The gen_set for normal dataflow would treat tmp13 as
// initialized, even though it's source expression is
// uninitialized.
//
// Work around this for rustc_peek by explicitly propagating
// the relevant bitvector state when computing the effect of a
// statement.

for (j, stmt) in statements.iter().enumerate() {
debug!("rustc_peek: ({:?},{}) {:?}", bb, j, stmt);
let (lvalue, rvalue) = match stmt.kind {
repr::StatementKind::Assign(ref lvalue, ref rvalue) => {
(lvalue, rvalue)
}
};

if lvalue == peek_arg_lval {
if let repr::Rvalue::Ref(_,
repr::BorrowKind::Shared,
ref peeking_at_lval) = *rvalue {
// Okay, our search is over.
let peek_mpi = move_data.rev_lookup.find(peeking_at_lval);
let bit_state = sets.on_entry.get_bit(peek_mpi.idx());
debug!("rustc_peek({:?} = &{:?}) bit_state: {}",
lvalue, peeking_at_lval, bit_state);
if !bit_state {
tcx.sess.span_err(span, &format!("rustc_peek: bit not set"));
}
continue 'next_block;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extract-function the whole thing so you can use return.

} else {
// Our search should have been over, but the input
// does not match expectations of `rustc_peek` for
// this sanity_check.
tcx.sess.span_err(span, &format!("rustc_peek: argument expression \
must be immediate borrow of form `&expr`"));
}
}

enum Effect<'a, 'tcx:'a> { Propagate(&'a repr::Lvalue<'tcx>), Compute }
let lvalue_effect: Effect = match *rvalue {
// tmp = rhs
repr::Rvalue::Use(repr::Operand::Consume(ref rhs_lval)) =>
Effect::Propagate(rhs_lval),

repr::Rvalue::Use(repr::Operand::Constant(_)) =>
Effect::Compute,

_ => {
// (fall back to BitDenotation for all other kinds of Rvalues
Effect::Compute
}
};

let lhs_mpi = move_data.rev_lookup.find(lvalue);

if let Effect::Propagate(rhs_lval) = lvalue_effect {
let rhs_mpi = move_data.rev_lookup.find(rhs_lval);
let state = sets.on_entry.get_bit(rhs_mpi.idx());
debug!("rustc_peek: propagate into lvalue {:?} ({:?}) from rhs: {:?} state: {}",
lvalue, lhs_mpi, rhs_lval, state);
if state {
sets.on_entry.set_bit(lhs_mpi.idx());
} else {
sets.on_entry.clear_bit(lhs_mpi.idx());
}
} else {
debug!("rustc_peek: computing effect on lvalue: {:?} ({:?}) in stmt: {:?}",
lvalue, lhs_mpi, stmt);
// reset GEN and KILL sets before emulating their effect.
for e in &mut sets.gen_set[..] { *e = 0; }
for e in &mut sets.kill_set[..] { *e = 0; }
results.0.operator.statement_effect(flow_ctxt, &mut sets, bb, j);
bitwise(sets.on_entry, sets.gen_set, &Union);
bitwise(sets.on_entry, sets.kill_set, &Subtract);
}
}

tcx.sess.span_err(span, &format!("rustc_peek: MIR did not match \
anticipated pattern; note that \
rustc_peek expects input of \
form `&expr`"));
}
}
18 changes: 15 additions & 3 deletions src/librustc_borrowck/borrowck/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,21 @@ pub fn borrowck_mir<'a, 'tcx: 'a>(
let move_data = MoveData::gather_moves(mir, tcx);
let ctxt = (tcx, mir, move_data);
let (ctxt, flow_inits) =
do_dataflow(bcx, mir, id, attributes, ctxt, MaybeInitializedLvals::default());
let ((_, _, move_data), flow_uninits) =
do_dataflow(bcx, mir, id, attributes, ctxt, MaybeUninitializedLvals::default());
do_dataflow(tcx, mir, id, attributes, ctxt, MaybeInitializedLvals::default());
let (ctxt, flow_uninits) =
do_dataflow(tcx, mir, id, attributes, ctxt, MaybeUninitializedLvals::default());

if has_rustc_mir_with(attributes, "rustc_peek_maybe_init").is_some() {
dataflow::sanity_check_via_rustc_peek(bcx.tcx, mir, id, attributes, &ctxt, &flow_inits);
}
if has_rustc_mir_with(attributes, "rustc_peek_maybe_uninit").is_some() {
dataflow::sanity_check_via_rustc_peek(bcx.tcx, mir, id, attributes, &ctxt, &flow_uninits);
}
let move_data = ctxt.2;

if has_rustc_mir_with(attributes, "stop_after_dataflow").is_some() {
bcx.tcx.sess.fatal("stop_after_dataflow ended compilation");
}

let mut mbcx = MirBorrowckCtxt {
bcx: bcx,
Expand Down