Try to add some diagnostics for protectors too

Author: saethlin

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 => {}
                     }

src/stacked_borrows.rs

@@ -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,
@@ -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<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,
@@ -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<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| {
@@ -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)?;