Skip to content

Commit

Permalink
Rollup merge of #71475 - RalfJung:miri-frame-loc, r=ecstatic-morse
Browse files Browse the repository at this point in the history
Miri Frame: use mir::Location to represent position in function

I only recently learned that `Location` exists, and it seems to perfectly fit what Miri needs to represent which statement we are currently executing. :)

r? @ecstatic-morse
  • Loading branch information
Dylan-DPC authored Apr 24, 2020
2 parents 715948e + 90f1131 commit aa9053a
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 41 deletions.
50 changes: 20 additions & 30 deletions src/librustc_mir/interpret/eval_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,14 +81,9 @@ pub struct Frame<'mir, 'tcx, Tag = (), Extra = ()> {
////////////////////////////////////////////////////////////////////////////////
// Current position within the function
////////////////////////////////////////////////////////////////////////////////
/// The block that is currently executed (or will be executed after the above call stacks
/// return).
/// If this is `None`, we are unwinding and this function doesn't need any clean-up.
/// Just continue the same as with `Resume`.
pub block: Option<mir::BasicBlock>,

/// The index of the currently evaluated statement.
pub stmt: usize,
pub loc: Option<mir::Location>,
}

#[derive(Clone, Eq, PartialEq, Debug, HashStable)] // Miri debug-prints these
Expand Down Expand Up @@ -168,8 +163,7 @@ impl<'mir, 'tcx, Tag> Frame<'mir, 'tcx, Tag> {
return_to_block: self.return_to_block,
return_place: self.return_place,
locals: self.locals,
block: self.block,
stmt: self.stmt,
loc: self.loc,
extra,
}
}
Expand All @@ -178,10 +172,10 @@ impl<'mir, 'tcx, Tag> Frame<'mir, 'tcx, Tag> {
impl<'mir, 'tcx, Tag, Extra> Frame<'mir, 'tcx, Tag, Extra> {
/// Return the `SourceInfo` of the current instruction.
pub fn current_source_info(&self) -> Option<mir::SourceInfo> {
self.block.map(|block| {
let block = &self.body.basic_blocks()[block];
if self.stmt < block.statements.len() {
block.statements[self.stmt].source_info
self.loc.map(|loc| {
let block = &self.body.basic_blocks()[loc.block];
if loc.statement_index < block.statements.len() {
block.statements[loc.statement_index].source_info
} else {
block.terminator().source_info
}
Expand Down Expand Up @@ -615,14 +609,13 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
// first push a stack frame so we have access to the local substs
let pre_frame = Frame {
body,
block: Some(mir::START_BLOCK),
loc: Some(mir::Location::START),
return_to_block,
return_place,
// empty local array, we fill it in below, after we are inside the stack frame and
// all methods actually know about the frame
locals: IndexVec::new(),
instance,
stmt: 0,
extra: (),
};
let frame = M::init_frame_extra(self, pre_frame)?;
Expand Down Expand Up @@ -666,9 +659,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
/// Jump to the given block.
#[inline]
pub fn go_to_block(&mut self, target: mir::BasicBlock) {
let frame = self.frame_mut();
frame.block = Some(target);
frame.stmt = 0;
self.frame_mut().loc = Some(mir::Location { block: target, statement_index: 0 });
}

/// *Return* to the given `target` basic block.
Expand All @@ -690,9 +681,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
/// If `target` is `None`, that indicates the function does not need cleanup during
/// unwinding, and we will just keep propagating that upwards.
pub fn unwind_to_block(&mut self, target: Option<mir::BasicBlock>) {
let frame = self.frame_mut();
frame.block = target;
frame.stmt = 0;
self.frame_mut().loc = target.map(|block| mir::Location { block, statement_index: 0 });
}

/// Pops the current frame from the stack, deallocating the
Expand All @@ -719,9 +708,9 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
// Sanity check `unwinding`.
assert_eq!(
unwinding,
match self.frame().block {
match self.frame().loc {
None => true,
Some(block) => self.body().basic_blocks()[block].is_cleanup,
Some(loc) => self.body().basic_blocks()[loc.block].is_cleanup,
}
);

Expand Down Expand Up @@ -973,13 +962,14 @@ where
Tag: HashStable<StableHashingContext<'ctx>>,
{
fn hash_stable(&self, hcx: &mut StableHashingContext<'ctx>, hasher: &mut StableHasher) {
self.body.hash_stable(hcx, hasher);
self.instance.hash_stable(hcx, hasher);
self.return_to_block.hash_stable(hcx, hasher);
self.return_place.as_ref().map(|r| &**r).hash_stable(hcx, hasher);
self.locals.hash_stable(hcx, hasher);
self.block.hash_stable(hcx, hasher);
self.stmt.hash_stable(hcx, hasher);
self.extra.hash_stable(hcx, hasher);
// Exhaustive match on fields to make sure we forget no field.
let Frame { body, instance, return_to_block, return_place, locals, loc, extra } = self;
body.hash_stable(hcx, hasher);
instance.hash_stable(hcx, hasher);
return_to_block.hash_stable(hcx, hasher);
return_place.as_ref().map(|r| &**r).hash_stable(hcx, hasher);
locals.hash_stable(hcx, hasher);
loc.hash_stable(hcx, hasher);
extra.hash_stable(hcx, hasher);
}
}
16 changes: 7 additions & 9 deletions src/librustc_mir/interpret/step.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
return Ok(false);
}

let block = match self.frame().block {
Some(block) => block,
let loc = match self.frame().loc {
Some(loc) => loc,
None => {
// We are unwinding and this fn has no cleanup code.
// Just go on unwinding.
Expand All @@ -56,13 +56,11 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
return Ok(true);
}
};
let stmt_id = self.frame().stmt;
let body = self.body();
let basic_block = &body.basic_blocks()[block];
let basic_block = &self.body().basic_blocks()[loc.block];

let old_frames = self.frame_idx();

if let Some(stmt) = basic_block.statements.get(stmt_id) {
if let Some(stmt) = basic_block.statements.get(loc.statement_index) {
assert_eq!(old_frames, self.frame_idx());
self.statement(stmt)?;
return Ok(true);
Expand Down Expand Up @@ -126,7 +124,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
LlvmInlineAsm { .. } => throw_unsup_format!("inline assembly is not supported"),
}

self.stack_mut()[frame_idx].stmt += 1;
self.stack_mut()[frame_idx].loc.as_mut().unwrap().statement_index += 1;
Ok(())
}

Expand Down Expand Up @@ -279,8 +277,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {

self.eval_terminator(terminator)?;
if !self.stack().is_empty() {
if let Some(block) = self.frame().block {
info!("// executing {:?}", block);
if let Some(loc) = self.frame().loc {
info!("// executing {:?}", loc.block);
}
}
Ok(())
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_mir/interpret/terminator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {

Call { ref func, ref args, destination, ref cleanup, .. } => {
let old_stack = self.frame_idx();
let old_bb = self.frame().block;
let old_loc = self.frame().loc;
let func = self.eval_operand(func, None)?;
let (fn_val, abi) = match func.layout.ty.kind {
ty::FnPtr(sig) => {
Expand All @@ -79,7 +79,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
self.eval_fn_call(fn_val, abi, &args[..], ret, *cleanup)?;
// Sanity-check that `eval_fn_call` either pushed a new frame or
// did a jump to another block.
if self.frame_idx() == old_stack && self.frame().block == old_bb {
if self.frame_idx() == old_stack && self.frame().loc == old_loc {
span_bug!(terminator.source_info.span, "evaluating this call made no progress");
}
}
Expand Down

0 comments on commit aa9053a

Please sign in to comment.