mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Auto merge of #2711 - RalfJung:btrack, r=RalfJung

Browse Source 
slight simplifications for borrow tracking

and some renaming for consistency
This commit is contained in:
bors 2022-12-02 13:48:14 +00:00
commit 4a64902a9d
9 changed files with 62 additions and 84 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

52
src/tools/miri/src/borrow_tracker/mod.rs
View File

@ -55,9 +55,9 @@ impl fmt::Debug for BorTag {
}
}
/// Per-frame data for borrow tracking
/// Per-call-stack-frame data for borrow tracking
#[derive(Debug)]
pub struct FrameExtra {
pub struct FrameState {
/// The ID of the call this frame corresponds to.
pub call_id: CallId,
@ -72,7 +72,7 @@ pub struct FrameExtra {
pub protected_tags: SmallVec<[BorTag; 2]>,
}
impl VisitTags for FrameExtra {
impl VisitTags for FrameState {
fn visit_tags(&self, _visit: &mut dyn FnMut(BorTag)) {
// `protected_tags` are fine to GC.
}
@ -190,17 +190,17 @@ impl GlobalStateInner {
id
}
pub fn new_frame(&mut self, machine: &MiriMachine<'_, '_>) -> FrameExtra {
pub fn new_frame(&mut self, machine: &MiriMachine<'_, '_>) -> FrameState {
let call_id = self.next_call_id;
trace!("new_frame: Assigning call ID {}", call_id);
if self.tracked_call_ids.contains(&call_id) {
machine.emit_diagnostic(NonHaltingDiagnostic::CreatedCallId(call_id));
}
self.next_call_id = NonZeroU64::new(call_id.get() + 1).unwrap();
FrameExtra { call_id, protected_tags: SmallVec::new() }
FrameState { call_id, protected_tags: SmallVec::new() }
}
pub fn end_call(&mut self, frame: &machine::FrameData<'_>) {
pub fn end_call(&mut self, frame: &machine::FrameExtra<'_>) {
for tag in &frame
.borrow_tracker
.as_ref()
@ -253,10 +253,10 @@ impl GlobalStateInner {
alloc_size: Size,
kind: MemoryKind<machine::MiriMemoryKind>,
machine: &MiriMachine<'_, '_>,
) -> AllocExtra {
) -> AllocState {
match self.borrow_tracker_method {
BorrowTrackerMethod::StackedBorrows =>
AllocExtra::StackedBorrows(Box::new(RefCell::new(Stacks::new_allocation(
AllocState::StackedBorrows(Box::new(RefCell::new(Stacks::new_allocation(
id, alloc_size, self, kind, machine,
)))),
}
@ -292,24 +292,30 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
/// Extra per-allocation data for borrow tracking
#[derive(Debug, Clone)]
pub enum AllocExtra {
pub enum AllocState {
/// Data corresponding to Stacked Borrows
StackedBorrows(Box<RefCell<stacked_borrows::AllocExtra>>),
StackedBorrows(Box<RefCell<stacked_borrows::AllocState>>),
}
impl AllocExtra {
pub fn assert_sb(&self) -> &RefCell<stacked_borrows::AllocExtra> {
match self {
AllocExtra::StackedBorrows(ref sb) => sb,
impl machine::AllocExtra {
#[track_caller]
pub fn borrow_tracker_sb(&self) -> &RefCell<stacked_borrows::AllocState> {
match self.borrow_tracker {
Some(AllocState::StackedBorrows(ref sb)) => sb,
_ => panic!("expected Stacked Borrows borrow tracking, got something else"),
}
}
pub fn assert_sb_mut(&mut self) -> &mut RefCell<stacked_borrows::AllocExtra> {
match self {
AllocExtra::StackedBorrows(ref mut sb) => sb,
#[track_caller]
pub fn borrow_tracker_sb_mut(&mut self) -> &mut RefCell<stacked_borrows::AllocState> {
match self.borrow_tracker {
Some(AllocState::StackedBorrows(ref mut sb)) => sb,
_ => panic!("expected Stacked Borrows borrow tracking, got something else"),
}
}
}
impl AllocState {
pub fn before_memory_read<'tcx>(
&self,
alloc_id: AllocId,
@ -318,7 +324,7 @@ impl AllocExtra {
machine: &MiriMachine<'_, 'tcx>,
) -> InterpResult<'tcx> {
match self {
AllocExtra::StackedBorrows(sb) =>
AllocState::StackedBorrows(sb) =>
sb.borrow_mut().before_memory_read(alloc_id, prov_extra, range, machine),
}
}
@ -331,7 +337,7 @@ impl AllocExtra {
machine: &mut MiriMachine<'_, 'tcx>,
) -> InterpResult<'tcx> {
match self {
AllocExtra::StackedBorrows(sb) =>
AllocState::StackedBorrows(sb) =>
sb.get_mut().before_memory_write(alloc_id, prov_extra, range, machine),
}
}
@ -344,22 +350,22 @@ impl AllocExtra {
machine: &mut MiriMachine<'_, 'tcx>,
) -> InterpResult<'tcx> {
match self {
AllocExtra::StackedBorrows(sb) =>
AllocState::StackedBorrows(sb) =>
sb.get_mut().before_memory_deallocation(alloc_id, prov_extra, range, machine),
}
}
pub fn remove_unreachable_tags(&self, tags: &FxHashSet<BorTag>) {
match self {
AllocExtra::StackedBorrows(sb) => sb.borrow_mut().remove_unreachable_tags(tags),
AllocState::StackedBorrows(sb) => sb.borrow_mut().remove_unreachable_tags(tags),
}
}
}
impl VisitTags for AllocExtra {
impl VisitTags for AllocState {
fn visit_tags(&self, visit: &mut dyn FnMut(BorTag)) {
match self {
AllocExtra::StackedBorrows(sb) => sb.visit_tags(visit),
AllocState::StackedBorrows(sb) => sb.visit_tags(visit),
}
}
}

40
src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs
View File

@ -25,7 +25,7 @@ mod stack;
pub use stack::Stack;
pub mod diagnostics;
pub type AllocExtra = Stacks;
pub type AllocState = Stacks;
/// Extra per-allocation state.
#[derive(Clone, Debug)]
@ -500,10 +500,6 @@ impl Stacks {
})?;
Ok(())
}
fn expose_tag(&mut self, tag: BorTag) {
self.exposed_tags.insert(tag);
}
}
/// Retagging/reborrowing. There is some policy in here, such as which permissions
@ -567,10 +563,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<'
// uncovers a non-supported `extern static`.
let extra = this.get_alloc_extra(alloc_id)?;
let mut stacked_borrows = extra
.borrow_tracker
.as_ref()
.expect("We should have borrow tracking data")
.assert_sb()
.borrow_tracker_sb()
.borrow_mut();
// Note that we create a *second* `DiagnosticCxBuilder` below for the actual retag.
// FIXME: can this be done cleaner?
@ -681,12 +674,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<'
// We have to use shared references to alloc/memory_extra here since
// `visit_freeze_sensitive` needs to access the global state.
let alloc_extra = this.get_alloc_extra(alloc_id)?;
let mut stacked_borrows = alloc_extra
.borrow_tracker
.as_ref()
.expect("We should have borrow tracking data")
.assert_sb()
.borrow_mut();
let mut stacked_borrows = alloc_extra.borrow_tracker_sb().borrow_mut();
this.visit_freeze_sensitive(place, size, |mut range, frozen| {
// Adjust range.
range.start += base_offset;
@ -736,12 +724,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<'
// Note that this asserts that the allocation is mutable -- but since we are creating a
// mutable pointer, that seems reasonable.
let (alloc_extra, machine) = this.get_alloc_extra_mut(alloc_id)?;
let stacked_borrows = alloc_extra
.borrow_tracker
.as_mut()
.expect("We should have borrow tracking data")
.assert_sb_mut()
.get_mut();
let stacked_borrows = alloc_extra.borrow_tracker_sb_mut().get_mut();
let item = Item::new(new_tag, perm, protect.is_some());
let range = alloc_range(base_offset, size);
let global = machine.borrow_tracker.as_ref().unwrap().borrow();
@ -993,13 +976,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
// uncovers a non-supported `extern static`.
let alloc_extra = this.get_alloc_extra(alloc_id)?;
trace!("Stacked Borrows tag {tag:?} exposed in {alloc_id:?}");
alloc_extra
.borrow_tracker
.as_ref()
.expect("We should have borrow tracking data")
.assert_sb()
.borrow_mut()
.expose_tag(tag);
alloc_extra.borrow_tracker_sb().borrow_mut().exposed_tags.insert(tag);
}
AllocKind::Function | AllocKind::VTable | AllocKind::Dead => {
// No stacked borrows on these allocations.
@ -1011,12 +988,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
fn print_stacks(&mut self, alloc_id: AllocId) -> InterpResult<'tcx> {
let this = self.eval_context_mut();
let alloc_extra = this.get_alloc_extra(alloc_id)?;
let stacks = alloc_extra
.borrow_tracker
.as_ref()
.expect("We should have borrow tracking data")
.assert_sb()
.borrow();
let stacks = alloc_extra.borrow_tracker_sb().borrow();
for (range, stack) in stacks.stacks.iter_all() {
print!("{range:?}: [");
if let Some(bottom) = stack.unknown_bottom() {

2
src/tools/miri/src/concurrency/data_race.rs
View File

@ -59,7 +59,7 @@ use super::{
weak_memory::EvalContextExt as _,
};
pub type AllocExtra = VClockAlloc;
pub type AllocState = VClockAlloc;
/// Valid atomic read-write orderings, alias of atomic::Ordering (not non-exhaustive).
#[derive(Copy, Clone, PartialEq, Eq, Debug)]

14
src/tools/miri/src/concurrency/thread.rs
View File

@ -113,7 +113,7 @@ pub struct Thread<'mir, 'tcx> {
thread_name: Option<Vec<u8>>,
/// The virtual call stack.
stack: Vec<Frame<'mir, 'tcx, Provenance, FrameData<'tcx>>>,
stack: Vec<Frame<'mir, 'tcx, Provenance, FrameExtra<'tcx>>>,
/// The function to call when the stack ran empty, to figure out what to do next.
/// Conceptually, this is the interpreter implementation of the things that happen 'after' the
@ -232,7 +232,7 @@ impl VisitTags for Thread<'_, '_> {
}
}
impl VisitTags for Frame<'_, '_, Provenance, FrameData<'_>> {
impl VisitTags for Frame<'_, '_, Provenance, FrameExtra<'_>> {
fn visit_tags(&self, visit: &mut dyn FnMut(BorTag)) {
let Frame {
return_place,
@ -385,20 +385,20 @@ impl<'mir, 'tcx: 'mir> ThreadManager<'mir, 'tcx> {
}
/// Borrow the stack of the active thread.
pub fn active_thread_stack(&self) -> &[Frame<'mir, 'tcx, Provenance, FrameData<'tcx>>] {
pub fn active_thread_stack(&self) -> &[Frame<'mir, 'tcx, Provenance, FrameExtra<'tcx>>] {
&self.threads[self.active_thread].stack
}
/// Mutably borrow the stack of the active thread.
fn active_thread_stack_mut(
&mut self,
) -> &mut Vec<Frame<'mir, 'tcx, Provenance, FrameData<'tcx>>> {
) -> &mut Vec<Frame<'mir, 'tcx, Provenance, FrameExtra<'tcx>>> {
&mut self.threads[self.active_thread].stack
}
pub fn all_stacks(
&self,
) -> impl Iterator<Item = &[Frame<'mir, 'tcx, Provenance, FrameData<'tcx>>]> {
) -> impl Iterator<Item = &[Frame<'mir, 'tcx, Provenance, FrameExtra<'tcx>>]> {
self.threads.iter().map(|t| &t.stack[..])
}
@ -921,7 +921,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
}
#[inline]
fn active_thread_stack(&self) -> &[Frame<'mir, 'tcx, Provenance, FrameData<'tcx>>] {
fn active_thread_stack(&self) -> &[Frame<'mir, 'tcx, Provenance, FrameExtra<'tcx>>] {
let this = self.eval_context_ref();
this.machine.threads.active_thread_stack()
}
@ -929,7 +929,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
#[inline]
fn active_thread_stack_mut(
&mut self,
) -> &mut Vec<Frame<'mir, 'tcx, Provenance, FrameData<'tcx>>> {
) -> &mut Vec<Frame<'mir, 'tcx, Provenance, FrameExtra<'tcx>>> {
let this = self.eval_context_mut();
this.machine.threads.active_thread_stack_mut()
}

2
src/tools/miri/src/concurrency/weak_memory.rs
View File

@ -93,7 +93,7 @@ use super::{
vector_clock::{VClock, VTimestamp, VectorIdx},
};
pub type AllocExtra = StoreBufferAlloc;
pub type AllocState = StoreBufferAlloc;
// Each store buffer must be bounded otherwise it will grow indefinitely.
// However, bounding the store buffer means restricting the amount of weak

2
src/tools/miri/src/helpers.rs
View File

@ -988,7 +988,7 @@ impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> {
self.stack()[frame_idx].current_span()
}
fn stack(&self) -> &[Frame<'mir, 'tcx, Provenance, machine::FrameData<'tcx>>] {
fn stack(&self) -> &[Frame<'mir, 'tcx, Provenance, machine::FrameExtra<'tcx>>] {
self.threads.active_thread_stack()
}

2
src/tools/miri/src/lib.rs
View File

@ -106,7 +106,7 @@ pub use crate::eval::{
pub use crate::helpers::EvalContextExt as _;
pub use crate::intptrcast::ProvenanceMode;
pub use crate::machine::{
AllocExtra, FrameData, MiriInterpCx, MiriInterpCxExt, MiriMachine, MiriMemoryKind,
AllocExtra, FrameExtra, MiriInterpCx, MiriInterpCxExt, MiriMachine, MiriMemoryKind,
PrimitiveLayouts, Provenance, ProvenanceExtra, PAGE_SIZE, STACK_ADDR, STACK_SIZE,
};
pub use crate::mono_hash_map::MonoHashMap;

30
src/tools/miri/src/machine.rs
View File

@ -37,9 +37,9 @@ pub const STACK_ADDR: u64 = 32 * PAGE_SIZE; // not really about the "stack", but
pub const STACK_SIZE: u64 = 16 * PAGE_SIZE; // whatever
/// Extra data stored with each stack frame
pub struct FrameData<'tcx> {
pub struct FrameExtra<'tcx> {
/// Extra data for Stacked Borrows.
pub borrow_tracker: Option<borrow_tracker::FrameExtra>,
pub borrow_tracker: Option<borrow_tracker::FrameState>,
/// If this is Some(), then this is a special "catch unwind" frame (the frame of `try_fn`
/// called by `try`). When this frame is popped during unwinding a panic,
@ -58,10 +58,10 @@ pub struct FrameData<'tcx> {
pub is_user_relevant: bool,
}
impl<'tcx> std::fmt::Debug for FrameData<'tcx> {
impl<'tcx> std::fmt::Debug for FrameExtra<'tcx> {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
// Omitting `timing`, it does not support `Debug`.
let FrameData { borrow_tracker, catch_unwind, timing: _, is_user_relevant: _ } = self;
let FrameExtra { borrow_tracker, catch_unwind, timing: _, is_user_relevant: _ } = self;
f.debug_struct("FrameData")
.field("borrow_tracker", borrow_tracker)
.field("catch_unwind", catch_unwind)
@ -69,9 +69,9 @@ impl<'tcx> std::fmt::Debug for FrameData<'tcx> {
}
}
impl VisitTags for FrameData<'_> {
impl VisitTags for FrameExtra<'_> {
fn visit_tags(&self, visit: &mut dyn FnMut(BorTag)) {
let FrameData { catch_unwind, borrow_tracker, timing: _, is_user_relevant: _ } = self;
let FrameExtra { catch_unwind, borrow_tracker, timing: _, is_user_relevant: _ } = self;
catch_unwind.visit_tags(visit);
borrow_tracker.visit_tags(visit);
@ -255,13 +255,13 @@ impl ProvenanceExtra {
#[derive(Debug, Clone)]
pub struct AllocExtra {
/// Global state of the borrow tracker, if enabled.
pub borrow_tracker: Option<borrow_tracker::AllocExtra>,
pub borrow_tracker: Option<borrow_tracker::AllocState>,
/// Data race detection via the use of a vector-clock,
/// this is only added if it is enabled.
pub data_race: Option<data_race::AllocExtra>,
pub data_race: Option<data_race::AllocState>,
/// Weak memory emulation via the use of store buffers,
/// this is only added if it is enabled.
pub weak_memory: Option<weak_memory::AllocExtra>,
pub weak_memory: Option<weak_memory::AllocState>,
}
impl VisitTags for AllocExtra {
@ -736,7 +736,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {
type MemoryKind = MiriMemoryKind;
type ExtraFnVal = Dlsym;
type FrameExtra = FrameData<'tcx>;
type FrameExtra = FrameExtra<'tcx>;
type AllocExtra = AllocExtra;
type Provenance = Provenance;
@ -908,14 +908,14 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {
.map(|bt| bt.borrow_mut().new_allocation(id, alloc.size(), kind, &ecx.machine));
let race_alloc = ecx.machine.data_race.as_ref().map(|data_race| {
data_race::AllocExtra::new_allocation(
data_race::AllocState::new_allocation(
data_race,
&ecx.machine.threads,
alloc.size(),
kind,
)
});
let buffer_alloc = ecx.machine.weak_memory.then(weak_memory::AllocExtra::new_allocation);
let buffer_alloc = ecx.machine.weak_memory.then(weak_memory::AllocState::new_allocation);
let alloc: Allocation<Provenance, Self::AllocExtra> = alloc.adjust_from_tcx(
&ecx.tcx,
AllocExtra { borrow_tracker, data_race: race_alloc, weak_memory: buffer_alloc },
@ -1070,7 +1070,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {
fn init_frame_extra(
ecx: &mut InterpCx<'mir, 'tcx, Self>,
frame: Frame<'mir, 'tcx, Provenance>,
) -> InterpResult<'tcx, Frame<'mir, 'tcx, Provenance, FrameData<'tcx>>> {
) -> InterpResult<'tcx, Frame<'mir, 'tcx, Provenance, FrameExtra<'tcx>>> {
// Start recording our event before doing anything else
let timing = if let Some(profiler) = ecx.machine.profiler.as_ref() {
let fn_name = frame.instance.to_string();
@ -1088,7 +1088,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {
let borrow_tracker = ecx.machine.borrow_tracker.as_ref();
let extra = FrameData {
let extra = FrameExtra {
borrow_tracker: borrow_tracker.map(|bt| bt.borrow_mut().new_frame(&ecx.machine)),
catch_unwind: None,
timing,
@ -1157,7 +1157,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {
#[inline(always)]
fn after_stack_pop(
ecx: &mut InterpCx<'mir, 'tcx, Self>,
mut frame: Frame<'mir, 'tcx, Provenance, FrameData<'tcx>>,
mut frame: Frame<'mir, 'tcx, Provenance, FrameExtra<'tcx>>,
unwinding: bool,
) -> InterpResult<'tcx, StackPopJump> {
if frame.extra.is_user_relevant {

2
src/tools/miri/src/shims/panic.rs
View File

@ -125,7 +125,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
fn handle_stack_pop_unwind(
&mut self,
mut extra: FrameData<'tcx>,
mut extra: FrameExtra<'tcx>,
unwinding: bool,
) -> InterpResult<'tcx, StackPopJump> {
let this = self.eval_context_mut();