Skip to content

Commit

Permalink
Try to add some diagnostics for protectors too
Browse files Browse the repository at this point in the history
  • Loading branch information
@saethlin
saethlin committed Apr 2, 2022
1 parent 4d3f3da commit 50f19e3
Show file tree
Hide file tree
Showing 2 changed files with 95 additions and 22 deletions.
12 changes: 10 additions & 2 deletions src/diagnostics.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -164,15 +164,19 @@ pub fn report_error<'tcx, 'mir>(
(None, format!("see {} for further information", url)),
];
match history {
Some(TagHistory::Tagged {tag, created: (created_range, created_span), invalidated}) => {
Some(TagHistory::Tagged {tag, created: (created_range, created_span), invalidated, protected }) => {
let msg = format!("{:?} was created due to a retag at offsets {}", tag, HexRange(*created_range));
helps.push((Some(created_span.clone()), msg));
if let Some((invalidated_range, invalidated_span)) = invalidated {
let msg = format!("{:?} was later invalidated due to a retag at offsets {}", tag, HexRange(*invalidated_range));
helps.push((Some(invalidated_span.clone()), msg));
}
if let Some((protecting_tag, protecting_tag_span, protection_span)) = protected {
helps.push((Some(protecting_tag_span.clone()), format!("{:?} was protected due to {:?} which was created here", tag, protecting_tag)));
helps.push((Some(protection_span.clone()), "this protector is live for this call".to_string()));
}
}
Some(TagHistory::Untagged{ recently_created, recently_invalidated, matching_created }) => {
Some(TagHistory::Untagged{ recently_created, recently_invalidated, matching_created, protected }) => {
if let Some((range, span)) = recently_created {
let msg = format!("tag was most recently created at offsets {}", HexRange(*range));
helps.push((Some(span.clone()), msg));
Expand All @@ -185,6 +189,10 @@ pub fn report_error<'tcx, 'mir>(
let msg = format!("this tag was also created here at offsets {}", HexRange(*range));
helps.push((Some(span.clone()), msg));
}
if let Some((protecting_tag, protecting_tag_span, protection_span)) = protected {
helps.push((Some(protecting_tag_span.clone()), format!("{:?} was protected due to a tag which was created here", protecting_tag)));
helps.push((Some(protection_span.clone()), "this protector is live for this call".to_string()));
}
}
None => {}
}
Expand Down
105 changes: 85 additions & 20 deletions src/stacked_borrows.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,11 +126,20 @@ struct AllocHistory {
current_time: usize,
creations: Vec<Event>,
invalidations: Vec<Event>,
protectors: Vec<Protection>,
}

#[derive(Debug)]
struct Protection {
orig_tag: SbTag,
tag: SbTag,
span: Span,
}

#[derive(Debug)]
struct Event {
time: usize,
parent: Option<SbTag>,
tag: SbTag,
range: AllocRange,
span: Span,
Expand All @@ -141,11 +150,13 @@ pub enum TagHistory {
tag: SbTag,
created: (AllocRange, SpanData),
invalidated: Option<(AllocRange, SpanData)>,
protected: Option<(SbTag, SpanData, SpanData)>,
},
Untagged {
recently_created: Option<(AllocRange, SpanData)>,
recently_invalidated: Option<(AllocRange, SpanData)>,
matching_created: Option<(AllocRange, SpanData)>,
protected: Option<(SbTag, SpanData, SpanData)>,
},
}

Expand Down Expand Up @@ -258,9 +269,16 @@ impl GlobalState {
tag
}

fn add_creation(&mut self, tag: SbTag, alloc: AllocId, range: AllocRange) {
fn add_creation(
&mut self,
parent: Option<SbTag>,
tag: SbTag,
alloc: AllocId,
range: AllocRange,
) {
let mut extras = self.extras.entry(alloc).or_default();
extras.creations.push(Event {
parent,
tag,
range,
span: self.current_span,
Expand All @@ -272,6 +290,7 @@ impl GlobalState {
fn add_invalidation(&mut self, tag: SbTag, alloc: AllocId, range: AllocRange) {
let mut extras = self.extras.entry(alloc).or_default();
extras.invalidations.push(Event {
parent: None,
tag,
range,
span: self.current_span,
Expand All @@ -280,14 +299,40 @@ impl GlobalState {
extras.current_time += 1;
}

fn add_protector(&mut self, orig_tag: SbTag, tag: SbTag, alloc: AllocId) {
let mut extras = self.extras.entry(alloc).or_default();
extras.protectors.push(Protection { orig_tag, tag, span: self.current_span });
extras.current_time += 1;
}

fn get_stack_history(
&self,
tag: SbTag,
alloc: AllocId,
alloc_range: AllocRange,
offset: Size,
protector_tag: Option<SbTag>,
) -> Option<TagHistory> {
let extras = self.extras.get(&alloc)?;
let protected = protector_tag
.and_then(|protector| {
extras.protectors.iter().find_map(|protection| {
if protection.tag == protector {
Some((protection.orig_tag, protection.span.data()))
} else {
None
}
})
})
.and_then(|(tag, call_span)| {
extras.creations.iter().rev().find_map(|event| {
if event.tag == tag {
Some((event.parent?, event.span.data(), call_span))
} else {
None
}
})
});
if let SbTag::Tagged(_) = tag {
let get_matching = |events: &[Event]| {
events.iter().rev().find_map(|event| {
Expand All @@ -296,8 +341,9 @@ impl GlobalState {
};
Some(TagHistory::Tagged {
tag,
created: get_matching(&extras.creations).unwrap(),
created: get_matching(&extras.creations)?,
invalidated: get_matching(&extras.invalidations),
protected,
})
} else {
let mut created_time = 0;
Expand Down Expand Up @@ -348,7 +394,12 @@ impl GlobalState {
} else {
None
};
Some(TagHistory::Untagged { recently_created, matching_created, recently_invalidated })
Some(TagHistory::Untagged {
recently_created,
matching_created,
recently_invalidated,
protected,
})
}
}
}
Expand Down Expand Up @@ -446,27 +497,33 @@ impl<'tcx> Stack {
/// `None` during a deallocation.
fn check_protector(
item: &Item,
provoking_access: Option<(SbTag, AccessKind)>,
provoking_access: Option<(SbTag, AllocId, AllocRange, Size)>, // just for debug printing amd error messages
global: &GlobalState,
) -> InterpResult<'tcx> {
if let SbTag::Tagged(id) = item.tag {
if Some(id) == global.tracked_pointer_tag {
register_diagnostic(NonHaltingDiagnostic::PoppedPointerTag(
item.clone(),
provoking_access,
None, // FIXME
));
}
}
if let Some(call) = item.protector {
if global.is_active(call) {
if let Some((tag, _)) = provoking_access {
if let Some((tag, alloc_id, alloc_range, offset)) = provoking_access {
Err(err_sb_ub(
format!(
"not granting access to tag {:?} because incompatible item is protected: {:?}",
tag, item
),
None,
None,
global.get_stack_history(
tag,
alloc_id,
alloc_range,
offset,
Some(item.tag),
),
))?
} else {
Err(err_sb_ub(
Expand Down Expand Up @@ -506,7 +563,7 @@ impl<'tcx> Stack {
let first_incompatible_idx = self.find_first_write_incompatible(granting_idx);
for item in self.borrows.drain(first_incompatible_idx..).rev() {
trace!("access: popping item {:?}", item);
Stack::check_protector(&item, Some((tag, access)), global)?;
Stack::check_protector(&item, Some((tag, alloc_id, alloc_range, offset)), global)?;
global.add_invalidation(item.tag, alloc_id, alloc_range);
}
} else {
Expand All @@ -522,7 +579,11 @@ impl<'tcx> Stack {
let item = &mut self.borrows[idx];
if item.perm == Permission::Unique {
trace!("access: disabling item {:?}", item);
Stack::check_protector(item, Some((tag, access)), global)?;
Stack::check_protector(
item,
Some((tag, alloc_id, alloc_range, offset)),
global,
)?;
item.perm = Permission::Disabled;
global.add_invalidation(item.tag, alloc_id, alloc_range);
}
Expand All @@ -548,7 +609,7 @@ impl<'tcx> Stack {
tag, alloc_id,
),
None,
global.get_stack_history(tag, alloc_id, alloc_range, offset),
global.get_stack_history(tag, alloc_id, alloc_range, offset, None),
)
})?;

Expand Down Expand Up @@ -640,7 +701,7 @@ impl<'tcx> Stack {
err_sb_ub(
format!("{}{}", action, self.error_cause(derived_from)),
Some(Self::operation_summary("a reborrow", alloc_id, alloc_range)),
global.get_stack_history(derived_from, alloc_id, alloc_range, error_offset),
global.get_stack_history(derived_from, alloc_id, alloc_range, error_offset, None),
)
}

Expand All @@ -664,7 +725,7 @@ impl<'tcx> Stack {
err_sb_ub(
format!("{}{}", action, self.error_cause(tag)),
Some(Self::operation_summary("an access", alloc_id, alloc_range)),
global.get_stack_history(tag, alloc_id, alloc_range, error_offset),
global.get_stack_history(tag, alloc_id, alloc_range, error_offset, None),
)
}

Expand Down Expand Up @@ -768,7 +829,7 @@ impl Stacks {
(tag, Permission::SharedReadWrite)
}
};
extra.add_creation(base_tag, id, alloc_range(Size::ZERO, size));
extra.add_creation(None, base_tag, id, alloc_range(Size::ZERO, size));
Stacks::new(size, perm, base_tag)
}

Expand Down Expand Up @@ -857,6 +918,17 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
let (alloc_id, base_offset, ptr) = this.memory.ptr_get_alloc(place.ptr)?;
let orig_tag = ptr.provenance.sb;

let mem_extra = this.memory.extra.stacked_borrows.as_mut().unwrap().get_mut();
mem_extra.add_creation(
Some(orig_tag),
new_tag,
alloc_id,
alloc_range(base_offset, base_offset + size),
);
if protect {
mem_extra.add_protector(orig_tag, new_tag, alloc_id);
}

// Ensure we bail out if the pointer goes out-of-bounds (see miri#1050).
let (alloc_size, _) =
this.memory.get_size_and_align(alloc_id, AllocCheck::Dereferenceable)?;
Expand Down Expand Up @@ -959,9 +1031,6 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
None => return Ok(*val),
};

// May return Err for retags of size 0
let alloc = this.memory.ptr_get_alloc(place.ptr);

// Compute new borrow.
let mem_extra = this.memory.extra.stacked_borrows.as_mut().unwrap().get_mut();
let new_tag = match kind {
Expand All @@ -971,10 +1040,6 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
_ => SbTag::Tagged(mem_extra.new_ptr()),
};

if let Ok((alloc_id, base_offset, _ptr)) = alloc {
mem_extra.add_creation(new_tag, alloc_id, alloc_range(base_offset, base_offset + size));
}

// Reborrow.
this.reborrow(&place, size, kind, new_tag, protect)?;

Expand Down

0 comments on commit 50f19e3

Please sign in to comment.