diff --git a/src/diagnostics.rs b/src/diagnostics.rs index 54c7971df9..199c3e7ed3 100644 --- a/src/diagnostics.rs +++ b/src/diagnostics.rs @@ -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)); @@ -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 => {} } diff --git a/src/stacked_borrows.rs b/src/stacked_borrows.rs index f7d64cccf0..effb4bca20 100644 --- a/src/stacked_borrows.rs +++ b/src/stacked_borrows.rs @@ -126,11 +126,20 @@ struct AllocHistory { current_time: usize, creations: Vec, invalidations: Vec, + protectors: Vec, +} + +#[derive(Debug)] +struct Protection { + orig_tag: SbTag, + tag: SbTag, + span: Span, } #[derive(Debug)] struct Event { time: usize, + parent: Option, tag: SbTag, range: AllocRange, span: Span, @@ -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)>, }, } @@ -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, + 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, @@ -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, @@ -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, ) -> Option { 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| { @@ -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; @@ -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, + }) } } } @@ -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( @@ -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 { @@ -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); } @@ -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), ) })?; @@ -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), ) } @@ -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), ) } @@ -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) } @@ -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)?; @@ -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 { @@ -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)?;