Skip to content

Commit

Permalink
Auto merge of #122243 - RalfJung:local-place-sanity-check, r=oli-obk
Browse files Browse the repository at this point in the history
interpret: ensure that Place is never used for a different frame

We store the address where the stack frame stores its `locals`. The idea is that even if we pop and push, or switch to a different thread with a larger number of frames, then the `locals` address will most likely change so we'll notice that problem. This is made possible by some recent changes by `@WaffleLapkin,` where we no longer use `Place` across things that change the number of stack frames.

I made these debug assertions for now, just to make sure this can't cost us any perf.

The first commit is unrelated but it's a one-line comment change so it didn't warrant a separate PR...

r? `@oli-obk`
  • Loading branch information
bors committed Mar 14, 2024
2 parents b62ba94 + 4a349d2 commit 1a92777
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 3 deletions.
2 changes: 1 addition & 1 deletion src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
.ok_or_else(|| err_ub_format!("callee has fewer arguments than expected"))?;
// Make the local live, and insert the initial value.
this.storage_live(local)?;
let callee_arg = this.local_to_place(this.frame_idx(), local)?;
let callee_arg = this.local_to_place(local)?;
this.write_immediate(*arg, &callee_arg)?;
}
if callee_args.next().is_some() {
Expand Down
3 changes: 1 addition & 2 deletions src/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1488,14 +1488,13 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {

fn after_local_allocated(
ecx: &mut InterpCx<'mir, 'tcx, Self>,
frame: usize,
local: mir::Local,
mplace: &MPlaceTy<'tcx, Provenance>,
) -> InterpResult<'tcx> {
let Some(Provenance::Concrete { alloc_id, .. }) = mplace.ptr().provenance else {
panic!("after_local_allocated should only be called on fresh allocations");
};
let local_decl = &ecx.active_thread_stack()[frame].body.local_decls[local];
let local_decl = &ecx.frame().body.local_decls[local];
let span = local_decl.source_info.span;
ecx.machine.allocation_spans.borrow_mut().insert(alloc_id, (span, None));
Ok(())
Expand Down

0 comments on commit 1a92777

Please sign in to comment.