Skip to content

Commit 4a64902

Browse files
committed
Auto merge of rust-lang#2711 - RalfJung:btrack, r=RalfJung
slight simplifications for borrow tracking and some renaming for consistency
2 parents 89dd322 + b12ce55 commit 4a64902

File tree

9 files changed

+62
-84
lines changed

9 files changed

+62
-84
lines changed

src/tools/miri/src/borrow_tracker/mod.rs

+29-23
Original file line numberDiff line numberDiff line change
@@ -55,9 +55,9 @@ impl fmt::Debug for BorTag {
5555
}
5656
}
5757

58-
/// Per-frame data for borrow tracking
58+
/// Per-call-stack-frame data for borrow tracking
5959
#[derive(Debug)]
60-
pub struct FrameExtra {
60+
pub struct FrameState {
6161
/// The ID of the call this frame corresponds to.
6262
pub call_id: CallId,
6363

@@ -72,7 +72,7 @@ pub struct FrameExtra {
7272
pub protected_tags: SmallVec<[BorTag; 2]>,
7373
}
7474

75-
impl VisitTags for FrameExtra {
75+
impl VisitTags for FrameState {
7676
fn visit_tags(&self, _visit: &mut dyn FnMut(BorTag)) {
7777
// `protected_tags` are fine to GC.
7878
}
@@ -190,17 +190,17 @@ impl GlobalStateInner {
190190
id
191191
}
192192

193-
pub fn new_frame(&mut self, machine: &MiriMachine<'_, '_>) -> FrameExtra {
193+
pub fn new_frame(&mut self, machine: &MiriMachine<'_, '_>) -> FrameState {
194194
let call_id = self.next_call_id;
195195
trace!("new_frame: Assigning call ID {}", call_id);
196196
if self.tracked_call_ids.contains(&call_id) {
197197
machine.emit_diagnostic(NonHaltingDiagnostic::CreatedCallId(call_id));
198198
}
199199
self.next_call_id = NonZeroU64::new(call_id.get() + 1).unwrap();
200-
FrameExtra { call_id, protected_tags: SmallVec::new() }
200+
FrameState { call_id, protected_tags: SmallVec::new() }
201201
}
202202

203-
pub fn end_call(&mut self, frame: &machine::FrameData<'_>) {
203+
pub fn end_call(&mut self, frame: &machine::FrameExtra<'_>) {
204204
for tag in &frame
205205
.borrow_tracker
206206
.as_ref()
@@ -253,10 +253,10 @@ impl GlobalStateInner {
253253
alloc_size: Size,
254254
kind: MemoryKind<machine::MiriMemoryKind>,
255255
machine: &MiriMachine<'_, '_>,
256-
) -> AllocExtra {
256+
) -> AllocState {
257257
match self.borrow_tracker_method {
258258
BorrowTrackerMethod::StackedBorrows =>
259-
AllocExtra::StackedBorrows(Box::new(RefCell::new(Stacks::new_allocation(
259+
AllocState::StackedBorrows(Box::new(RefCell::new(Stacks::new_allocation(
260260
id, alloc_size, self, kind, machine,
261261
)))),
262262
}
@@ -292,24 +292,30 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
292292

293293
/// Extra per-allocation data for borrow tracking
294294
#[derive(Debug, Clone)]
295-
pub enum AllocExtra {
295+
pub enum AllocState {
296296
/// Data corresponding to Stacked Borrows
297-
StackedBorrows(Box<RefCell<stacked_borrows::AllocExtra>>),
297+
StackedBorrows(Box<RefCell<stacked_borrows::AllocState>>),
298298
}
299299

300-
impl AllocExtra {
301-
pub fn assert_sb(&self) -> &RefCell<stacked_borrows::AllocExtra> {
302-
match self {
303-
AllocExtra::StackedBorrows(ref sb) => sb,
300+
impl machine::AllocExtra {
301+
#[track_caller]
302+
pub fn borrow_tracker_sb(&self) -> &RefCell<stacked_borrows::AllocState> {
303+
match self.borrow_tracker {
304+
Some(AllocState::StackedBorrows(ref sb)) => sb,
305+
_ => panic!("expected Stacked Borrows borrow tracking, got something else"),
304306
}
305307
}
306308

307-
pub fn assert_sb_mut(&mut self) -> &mut RefCell<stacked_borrows::AllocExtra> {
308-
match self {
309-
AllocExtra::StackedBorrows(ref mut sb) => sb,
309+
#[track_caller]
310+
pub fn borrow_tracker_sb_mut(&mut self) -> &mut RefCell<stacked_borrows::AllocState> {
311+
match self.borrow_tracker {
312+
Some(AllocState::StackedBorrows(ref mut sb)) => sb,
313+
_ => panic!("expected Stacked Borrows borrow tracking, got something else"),
310314
}
311315
}
316+
}
312317

318+
impl AllocState {
313319
pub fn before_memory_read<'tcx>(
314320
&self,
315321
alloc_id: AllocId,
@@ -318,7 +324,7 @@ impl AllocExtra {
318324
machine: &MiriMachine<'_, 'tcx>,
319325
) -> InterpResult<'tcx> {
320326
match self {
321-
AllocExtra::StackedBorrows(sb) =>
327+
AllocState::StackedBorrows(sb) =>
322328
sb.borrow_mut().before_memory_read(alloc_id, prov_extra, range, machine),
323329
}
324330
}
@@ -331,7 +337,7 @@ impl AllocExtra {
331337
machine: &mut MiriMachine<'_, 'tcx>,
332338
) -> InterpResult<'tcx> {
333339
match self {
334-
AllocExtra::StackedBorrows(sb) =>
340+
AllocState::StackedBorrows(sb) =>
335341
sb.get_mut().before_memory_write(alloc_id, prov_extra, range, machine),
336342
}
337343
}
@@ -344,22 +350,22 @@ impl AllocExtra {
344350
machine: &mut MiriMachine<'_, 'tcx>,
345351
) -> InterpResult<'tcx> {
346352
match self {
347-
AllocExtra::StackedBorrows(sb) =>
353+
AllocState::StackedBorrows(sb) =>
348354
sb.get_mut().before_memory_deallocation(alloc_id, prov_extra, range, machine),
349355
}
350356
}
351357

352358
pub fn remove_unreachable_tags(&self, tags: &FxHashSet<BorTag>) {
353359
match self {
354-
AllocExtra::StackedBorrows(sb) => sb.borrow_mut().remove_unreachable_tags(tags),
360+
AllocState::StackedBorrows(sb) => sb.borrow_mut().remove_unreachable_tags(tags),
355361
}
356362
}
357363
}
358364

359-
impl VisitTags for AllocExtra {
365+
impl VisitTags for AllocState {
360366
fn visit_tags(&self, visit: &mut dyn FnMut(BorTag)) {
361367
match self {
362-
AllocExtra::StackedBorrows(sb) => sb.visit_tags(visit),
368+
AllocState::StackedBorrows(sb) => sb.visit_tags(visit),
363369
}
364370
}
365371
}

src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs

+6-34
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ mod stack;
2525
pub use stack::Stack;
2626
pub mod diagnostics;
2727

28-
pub type AllocExtra = Stacks;
28+
pub type AllocState = Stacks;
2929

3030
/// Extra per-allocation state.
3131
#[derive(Clone, Debug)]
@@ -500,10 +500,6 @@ impl Stacks {
500500
})?;
501501
Ok(())
502502
}
503-
504-
fn expose_tag(&mut self, tag: BorTag) {
505-
self.exposed_tags.insert(tag);
506-
}
507503
}
508504

509505
/// 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<'
567563
// uncovers a non-supported `extern static`.
568564
let extra = this.get_alloc_extra(alloc_id)?;
569565
let mut stacked_borrows = extra
570-
.borrow_tracker
571-
.as_ref()
572-
.expect("We should have borrow tracking data")
573-
.assert_sb()
566+
.borrow_tracker_sb()
574567
.borrow_mut();
575568
// Note that we create a *second* `DiagnosticCxBuilder` below for the actual retag.
576569
// FIXME: can this be done cleaner?
@@ -681,12 +674,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<'
681674
// We have to use shared references to alloc/memory_extra here since
682675
// `visit_freeze_sensitive` needs to access the global state.
683676
let alloc_extra = this.get_alloc_extra(alloc_id)?;
684-
let mut stacked_borrows = alloc_extra
685-
.borrow_tracker
686-
.as_ref()
687-
.expect("We should have borrow tracking data")
688-
.assert_sb()
689-
.borrow_mut();
677+
let mut stacked_borrows = alloc_extra.borrow_tracker_sb().borrow_mut();
690678
this.visit_freeze_sensitive(place, size, |mut range, frozen| {
691679
// Adjust range.
692680
range.start += base_offset;
@@ -736,12 +724,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<'
736724
// Note that this asserts that the allocation is mutable -- but since we are creating a
737725
// mutable pointer, that seems reasonable.
738726
let (alloc_extra, machine) = this.get_alloc_extra_mut(alloc_id)?;
739-
let stacked_borrows = alloc_extra
740-
.borrow_tracker
741-
.as_mut()
742-
.expect("We should have borrow tracking data")
743-
.assert_sb_mut()
744-
.get_mut();
727+
let stacked_borrows = alloc_extra.borrow_tracker_sb_mut().get_mut();
745728
let item = Item::new(new_tag, perm, protect.is_some());
746729
let range = alloc_range(base_offset, size);
747730
let global = machine.borrow_tracker.as_ref().unwrap().borrow();
@@ -993,13 +976,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
993976
// uncovers a non-supported `extern static`.
994977
let alloc_extra = this.get_alloc_extra(alloc_id)?;
995978
trace!("Stacked Borrows tag {tag:?} exposed in {alloc_id:?}");
996-
alloc_extra
997-
.borrow_tracker
998-
.as_ref()
999-
.expect("We should have borrow tracking data")
1000-
.assert_sb()
1001-
.borrow_mut()
1002-
.expose_tag(tag);
979+
alloc_extra.borrow_tracker_sb().borrow_mut().exposed_tags.insert(tag);
1003980
}
1004981
AllocKind::Function | AllocKind::VTable | AllocKind::Dead => {
1005982
// No stacked borrows on these allocations.
@@ -1011,12 +988,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
1011988
fn print_stacks(&mut self, alloc_id: AllocId) -> InterpResult<'tcx> {
1012989
let this = self.eval_context_mut();
1013990
let alloc_extra = this.get_alloc_extra(alloc_id)?;
1014-
let stacks = alloc_extra
1015-
.borrow_tracker
1016-
.as_ref()
1017-
.expect("We should have borrow tracking data")
1018-
.assert_sb()
1019-
.borrow();
991+
let stacks = alloc_extra.borrow_tracker_sb().borrow();
1020992
for (range, stack) in stacks.stacks.iter_all() {
1021993
print!("{range:?}: [");
1022994
if let Some(bottom) = stack.unknown_bottom() {

src/tools/miri/src/concurrency/data_race.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ use super::{
5959
weak_memory::EvalContextExt as _,
6060
};
6161

62-
pub type AllocExtra = VClockAlloc;
62+
pub type AllocState = VClockAlloc;
6363

6464
/// Valid atomic read-write orderings, alias of atomic::Ordering (not non-exhaustive).
6565
#[derive(Copy, Clone, PartialEq, Eq, Debug)]

src/tools/miri/src/concurrency/thread.rs

+7-7
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ pub struct Thread<'mir, 'tcx> {
113113
thread_name: Option<Vec<u8>>,
114114

115115
/// The virtual call stack.
116-
stack: Vec<Frame<'mir, 'tcx, Provenance, FrameData<'tcx>>>,
116+
stack: Vec<Frame<'mir, 'tcx, Provenance, FrameExtra<'tcx>>>,
117117

118118
/// The function to call when the stack ran empty, to figure out what to do next.
119119
/// Conceptually, this is the interpreter implementation of the things that happen 'after' the
@@ -232,7 +232,7 @@ impl VisitTags for Thread<'_, '_> {
232232
}
233233
}
234234

235-
impl VisitTags for Frame<'_, '_, Provenance, FrameData<'_>> {
235+
impl VisitTags for Frame<'_, '_, Provenance, FrameExtra<'_>> {
236236
fn visit_tags(&self, visit: &mut dyn FnMut(BorTag)) {
237237
let Frame {
238238
return_place,
@@ -385,20 +385,20 @@ impl<'mir, 'tcx: 'mir> ThreadManager<'mir, 'tcx> {
385385
}
386386

387387
/// Borrow the stack of the active thread.
388-
pub fn active_thread_stack(&self) -> &[Frame<'mir, 'tcx, Provenance, FrameData<'tcx>>] {
388+
pub fn active_thread_stack(&self) -> &[Frame<'mir, 'tcx, Provenance, FrameExtra<'tcx>>] {
389389
&self.threads[self.active_thread].stack
390390
}
391391

392392
/// Mutably borrow the stack of the active thread.
393393
fn active_thread_stack_mut(
394394
&mut self,
395-
) -> &mut Vec<Frame<'mir, 'tcx, Provenance, FrameData<'tcx>>> {
395+
) -> &mut Vec<Frame<'mir, 'tcx, Provenance, FrameExtra<'tcx>>> {
396396
&mut self.threads[self.active_thread].stack
397397
}
398398

399399
pub fn all_stacks(
400400
&self,
401-
) -> impl Iterator<Item = &[Frame<'mir, 'tcx, Provenance, FrameData<'tcx>>]> {
401+
) -> impl Iterator<Item = &[Frame<'mir, 'tcx, Provenance, FrameExtra<'tcx>>]> {
402402
self.threads.iter().map(|t| &t.stack[..])
403403
}
404404

@@ -921,15 +921,15 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
921921
}
922922

923923
#[inline]
924-
fn active_thread_stack(&self) -> &[Frame<'mir, 'tcx, Provenance, FrameData<'tcx>>] {
924+
fn active_thread_stack(&self) -> &[Frame<'mir, 'tcx, Provenance, FrameExtra<'tcx>>] {
925925
let this = self.eval_context_ref();
926926
this.machine.threads.active_thread_stack()
927927
}
928928

929929
#[inline]
930930
fn active_thread_stack_mut(
931931
&mut self,
932-
) -> &mut Vec<Frame<'mir, 'tcx, Provenance, FrameData<'tcx>>> {
932+
) -> &mut Vec<Frame<'mir, 'tcx, Provenance, FrameExtra<'tcx>>> {
933933
let this = self.eval_context_mut();
934934
this.machine.threads.active_thread_stack_mut()
935935
}

src/tools/miri/src/concurrency/weak_memory.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ use super::{
9393
vector_clock::{VClock, VTimestamp, VectorIdx},
9494
};
9595

96-
pub type AllocExtra = StoreBufferAlloc;
96+
pub type AllocState = StoreBufferAlloc;
9797

9898
// Each store buffer must be bounded otherwise it will grow indefinitely.
9999
// However, bounding the store buffer means restricting the amount of weak

src/tools/miri/src/helpers.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -988,7 +988,7 @@ impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> {
988988
self.stack()[frame_idx].current_span()
989989
}
990990

991-
fn stack(&self) -> &[Frame<'mir, 'tcx, Provenance, machine::FrameData<'tcx>>] {
991+
fn stack(&self) -> &[Frame<'mir, 'tcx, Provenance, machine::FrameExtra<'tcx>>] {
992992
self.threads.active_thread_stack()
993993
}
994994

src/tools/miri/src/lib.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ pub use crate::eval::{
106106
pub use crate::helpers::EvalContextExt as _;
107107
pub use crate::intptrcast::ProvenanceMode;
108108
pub use crate::machine::{
109-
AllocExtra, FrameData, MiriInterpCx, MiriInterpCxExt, MiriMachine, MiriMemoryKind,
109+
AllocExtra, FrameExtra, MiriInterpCx, MiriInterpCxExt, MiriMachine, MiriMemoryKind,
110110
PrimitiveLayouts, Provenance, ProvenanceExtra, PAGE_SIZE, STACK_ADDR, STACK_SIZE,
111111
};
112112
pub use crate::mono_hash_map::MonoHashMap;

0 commit comments

Comments
 (0)