Skip to content

Commit 7f37fa6

Browse files
saethlinsaethlin
committed
Try to add some diagnostics for protectors too
1 parent 4d3f3da commit 7f37fa6

File tree

2 files changed

+75
-21
lines changed

2 files changed

+75
-21
lines changed

‎src/diagnostics.rs

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -164,15 +164,19 @@ pub fn report_error<'tcx, 'mir>(
164164
(None, format!("see {} for further information", url)),
165165
];
166166
match history {
167-
Some(TagHistory::Tagged {tag, created: (created_range, created_span), invalidated}) => {
167+
Some(TagHistory::Tagged {tag, created: (created_range, created_span), invalidated, protected }) => {
168168
let msg = format!("{:?} was created due to a retag at offsets {}", tag, HexRange(*created_range));
169169
helps.push((Some(created_span.clone()), msg));
170170
if let Some((invalidated_range, invalidated_span)) = invalidated {
171171
let msg = format!("{:?} was later invalidated due to a retag at offsets {}", tag, HexRange(*invalidated_range));
172172
helps.push((Some(invalidated_span.clone()), msg));
173173
}
174+
if let Some((protecting_tag, protecting_tag_span, protection_span)) = protected {
175+
helps.push((Some(protecting_tag_span.clone()), format!("{:?} was protected due to {:?} which was created here", tag, protecting_tag)));
176+
helps.push((Some(protection_span.clone()), "this protector is live for this call".to_string()));
177+
}
174178
}
175-
Some(TagHistory::Untagged{ recently_created, recently_invalidated, matching_created }) => {
179+
Some(TagHistory::Untagged{ recently_created, recently_invalidated, matching_created, protected }) => {
176180
if let Some((range, span)) = recently_created {
177181
let msg = format!("tag was most recently created at offsets {}", HexRange(*range));
178182
helps.push((Some(span.clone()), msg));
@@ -185,6 +189,10 @@ pub fn report_error<'tcx, 'mir>(
185189
let msg = format!("this tag was also created here at offsets {}", HexRange(*range));
186190
helps.push((Some(span.clone()), msg));
187191
}
192+
if let Some((protecting_tag, protecting_tag_span, protection_span)) = protected {
193+
helps.push((Some(protecting_tag_span.clone()), format!("{:?} was protected due to a tag which was created here", protecting_tag)));
194+
helps.push((Some(protection_span.clone()), "this protector is live for this call".to_string()));
195+
}
188196
}
189197
None => {}
190198
}

‎src/stacked_borrows.rs

Lines changed: 65 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -126,11 +126,20 @@ struct AllocHistory {
126126
current_time: usize,
127127
creations: Vec<Event>,
128128
invalidations: Vec<Event>,
129+
protectors: Vec<Protection>,
130+
}
131+
132+
#[derive(Debug)]
133+
struct Protection {
134+
orig_tag: SbTag,
135+
tag: SbTag,
136+
span: Span,
129137
}
130138

131139
#[derive(Debug)]
132140
struct Event {
133141
time: usize,
142+
parent: Option<SbTag>,
134143
tag: SbTag,
135144
range: AllocRange,
136145
span: Span,
@@ -141,11 +150,13 @@ pub enum TagHistory {
141150
tag: SbTag,
142151
created: (AllocRange, SpanData),
143152
invalidated: Option<(AllocRange, SpanData)>,
153+
protected: Option<(SbTag, SpanData, SpanData)>,
144154
},
145155
Untagged {
146156
recently_created: Option<(AllocRange, SpanData)>,
147157
recently_invalidated: Option<(AllocRange, SpanData)>,
148158
matching_created: Option<(AllocRange, SpanData)>,
159+
protected: Option<(SbTag, SpanData, SpanData)>,
149160
},
150161
}
151162

@@ -258,9 +269,10 @@ impl GlobalState {
258269
tag
259270
}
260271

261-
fn add_creation(&mut self, tag: SbTag, alloc: AllocId, range: AllocRange) {
272+
fn add_creation(&mut self, parent: Option<SbTag>, tag: SbTag, alloc: AllocId, range: AllocRange) {
262273
let mut extras = self.extras.entry(alloc).or_default();
263274
extras.creations.push(Event {
275+
parent,
264276
tag,
265277
range,
266278
span: self.current_span,
@@ -272,6 +284,7 @@ impl GlobalState {
272284
fn add_invalidation(&mut self, tag: SbTag, alloc: AllocId, range: AllocRange) {
273285
let mut extras = self.extras.entry(alloc).or_default();
274286
extras.invalidations.push(Event {
287+
parent: None,
275288
tag,
276289
range,
277290
span: self.current_span,
@@ -280,14 +293,32 @@ impl GlobalState {
280293
extras.current_time += 1;
281294
}
282295

296+
fn add_protector(&mut self, orig_tag: SbTag, tag: SbTag, alloc: AllocId) {
297+
let mut extras = self.extras.entry(alloc).or_default();
298+
extras.protectors.push(Protection {orig_tag, tag, span: self.current_span});
299+
extras.current_time += 1;
300+
}
301+
283302
fn get_stack_history(
284303
&self,
285304
tag: SbTag,
286305
alloc: AllocId,
287306
alloc_range: AllocRange,
288307
offset: Size,
308+
protector_tag: Option<SbTag>,
289309
) -> Option<TagHistory> {
290310
let extras = self.extras.get(&alloc)?;
311+
let protected = protector_tag
312+
.and_then(|protector| {
313+
extras.protectors.iter().find_map(|protection| {
314+
if protection.tag == protector { Some((protection.orig_tag, protection.span.data())) } else { None }
315+
})
316+
})
317+
.and_then(|(tag, call_span)| {
318+
extras.creations.iter().rev().find_map(|event| {
319+
if event.tag == tag { Some((event.parent.unwrap(), event.span.data(), call_span)) } else { None }
320+
})
321+
});
291322
if let SbTag::Tagged(_) = tag {
292323
let get_matching = |events: &[Event]| {
293324
events.iter().rev().find_map(|event| {
@@ -298,6 +329,7 @@ impl GlobalState {
298329
tag,
299330
created: get_matching(&extras.creations).unwrap(),
300331
invalidated: get_matching(&extras.invalidations),
332+
protected,
301333
})
302334
} else {
303335
let mut created_time = 0;
@@ -348,7 +380,12 @@ impl GlobalState {
348380
} else {
349381
None
350382
};
351-
Some(TagHistory::Untagged { recently_created, matching_created, recently_invalidated })
383+
Some(TagHistory::Untagged {
384+
recently_created,
385+
matching_created,
386+
recently_invalidated,
387+
protected,
388+
})
352389
}
353390
}
354391
}
@@ -446,27 +483,33 @@ impl<'tcx> Stack {
446483
/// `None` during a deallocation.
447484
fn check_protector(
448485
item: &Item,
449-
provoking_access: Option<(SbTag, AccessKind)>,
486+
provoking_access: Option<(SbTag, AllocId, AllocRange, Size)>, // just for debug printing amd error messages
450487
global: &GlobalState,
451488
) -> InterpResult<'tcx> {
452489
if let SbTag::Tagged(id) = item.tag {
453490
if Some(id) == global.tracked_pointer_tag {
454491
register_diagnostic(NonHaltingDiagnostic::PoppedPointerTag(
455492
item.clone(),
456-
provoking_access,
493+
None, // FIXME
457494
));
458495
}
459496
}
460497
if let Some(call) = item.protector {
461498
if global.is_active(call) {
462-
if let Some((tag, _)) = provoking_access {
499+
if let Some((tag, alloc_id, alloc_range, offset)) = provoking_access {
463500
Err(err_sb_ub(
464501
format!(
465502
"not granting access to tag {:?} because incompatible item is protected: {:?}",
466503
tag, item
467504
),
468505
None,
469-
None,
506+
global.get_stack_history(
507+
tag,
508+
alloc_id,
509+
alloc_range,
510+
offset,
511+
Some(item.tag),
512+
),
470513
))?
471514
} else {
472515
Err(err_sb_ub(
@@ -506,7 +549,7 @@ impl<'tcx> Stack {
506549
let first_incompatible_idx = self.find_first_write_incompatible(granting_idx);
507550
for item in self.borrows.drain(first_incompatible_idx..).rev() {
508551
trace!("access: popping item {:?}", item);
509-
Stack::check_protector(&item, Some((tag, access)), global)?;
552+
Stack::check_protector(&item, Some((tag, alloc_id, alloc_range, offset)), global)?;
510553
global.add_invalidation(item.tag, alloc_id, alloc_range);
511554
}
512555
} else {
@@ -522,7 +565,11 @@ impl<'tcx> Stack {
522565
let item = &mut self.borrows[idx];
523566
if item.perm == Permission::Unique {
524567
trace!("access: disabling item {:?}", item);
525-
Stack::check_protector(item, Some((tag, access)), global)?;
568+
Stack::check_protector(
569+
item,
570+
Some((tag, alloc_id, alloc_range, offset)),
571+
global,
572+
)?;
526573
item.perm = Permission::Disabled;
527574
global.add_invalidation(item.tag, alloc_id, alloc_range);
528575
}
@@ -548,7 +595,7 @@ impl<'tcx> Stack {
548595
tag, alloc_id,
549596
),
550597
None,
551-
global.get_stack_history(tag, alloc_id, alloc_range, offset),
598+
global.get_stack_history(tag, alloc_id, alloc_range, offset, None),
552599
)
553600
})?;
554601

@@ -640,7 +687,7 @@ impl<'tcx> Stack {
640687
err_sb_ub(
641688
format!("{}{}", action, self.error_cause(derived_from)),
642689
Some(Self::operation_summary("a reborrow", alloc_id, alloc_range)),
643-
global.get_stack_history(derived_from, alloc_id, alloc_range, error_offset),
690+
global.get_stack_history(derived_from, alloc_id, alloc_range, error_offset, None),
644691
)
645692
}
646693

@@ -664,7 +711,7 @@ impl<'tcx> Stack {
664711
err_sb_ub(
665712
format!("{}{}", action, self.error_cause(tag)),
666713
Some(Self::operation_summary("an access", alloc_id, alloc_range)),
667-
global.get_stack_history(tag, alloc_id, alloc_range, error_offset),
714+
global.get_stack_history(tag, alloc_id, alloc_range, error_offset, None),
668715
)
669716
}
670717

@@ -768,7 +815,7 @@ impl Stacks {
768815
(tag, Permission::SharedReadWrite)
769816
}
770817
};
771-
extra.add_creation(base_tag, id, alloc_range(Size::ZERO, size));
818+
extra.add_creation(None, base_tag, id, alloc_range(Size::ZERO, size));
772819
Stacks::new(size, perm, base_tag)
773820
}
774821

@@ -857,6 +904,12 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
857904
let (alloc_id, base_offset, ptr) = this.memory.ptr_get_alloc(place.ptr)?;
858905
let orig_tag = ptr.provenance.sb;
859906

907+
let mem_extra = this.memory.extra.stacked_borrows.as_mut().unwrap().get_mut();
908+
mem_extra.add_creation(Some(orig_tag), new_tag, alloc_id, alloc_range(base_offset, base_offset + size));
909+
if protect {
910+
mem_extra.add_protector(orig_tag, new_tag, alloc_id);
911+
}
912+
860913
// Ensure we bail out if the pointer goes out-of-bounds (see miri#1050).
861914
let (alloc_size, _) =
862915
this.memory.get_size_and_align(alloc_id, AllocCheck::Dereferenceable)?;
@@ -959,9 +1012,6 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
9591012
None => return Ok(*val),
9601013
};
9611014

962-
// May return Err for retags of size 0
963-
let alloc = this.memory.ptr_get_alloc(place.ptr);
964-
9651015
// Compute new borrow.
9661016
let mem_extra = this.memory.extra.stacked_borrows.as_mut().unwrap().get_mut();
9671017
let new_tag = match kind {
@@ -971,10 +1021,6 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
9711021
_ => SbTag::Tagged(mem_extra.new_ptr()),
9721022
};
9731023

974-
if let Ok((alloc_id, base_offset, _ptr)) = alloc {
975-
mem_extra.add_creation(new_tag, alloc_id, alloc_range(base_offset, base_offset + size));
976-
}
977-
9781024
// Reborrow.
9791025
this.reborrow(&place, size, kind, new_tag, protect)?;
9801026

0 commit comments

Comments
 (0)