Auto merge of #2519 - saethlin:rustup, r=RalfJung

Use the better FnEntry spans in protector errors

Example error, from `tests/fail/stacked_borrows/invalidate_against_protector1.rs`:
```
error: Undefined Behavior: not granting access to tag <3095> because that would remove [Unique for <3099>] which is protected because it is an argument of call 943
  --> tests/fail/stacked_borrows/invalidate_against_protector1.rs:5:25
   |
5  |     let _val = unsafe { *x }; //~ ERROR: protect
   |                         ^^ not granting access to tag <3095> because that would remove [Unique for <3099>] which is protected because it is an argument of call 943
   |
   = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental
   = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
help: <3095> was created by a SharedReadWrite retag at offsets [0x0..0x4]
  --> tests/fail/stacked_borrows/invalidate_against_protector1.rs:10:16
   |
10 |     let xraw = &mut x as *mut _;
   |                ^^^^^^
help: <3099> is this argument
  --> tests/fail/stacked_borrows/invalidate_against_protector1.rs:1:23
   |
1  | fn inner(x: *mut i32, _y: &mut i32) {
   |                       ^^
   = note: backtrace:
   = note: inside `inner` at tests/fail/stacked_borrows/invalidate_against_protector1.rs:5:25
note: inside `main` at tests/fail/stacked_borrows/invalidate_against_protector1.rs:12:5
  --> tests/fail/stacked_borrows/invalidate_against_protector1.rs:12:5
   |
12 |     inner(xraw, xref);
   |     ^^^^^^^^^^^^^^^^^
```

Benchmarks report no change, within noise.

rust-version

@@ -1 +1 @@
-4065b89b1e7287047d7d6c65e7abd7b8ee70bcf0
+94b2b15e63c5d2b2a6a0910e3dae554ce9415bf9

src/diagnostics.rs

@@ -183,9 +183,8 @@ pub fn report_error<'tcx, 'mir>(
                         if let Some((msg, span)) = invalidated {
                             helps.push((Some(span), msg));
                         }
-                        if let Some([(protector_msg, protector_span), (protection_msg, protection_span)]) = protected {
+                        if let Some((protector_msg, protector_span)) = protected {
                             helps.push((Some(protector_span), protector_msg));
-                            helps.push((Some(protection_span), protection_msg));
                         }
                     }
                     helps

src/machine.rs

@@ -743,10 +743,15 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> {
         }
 
         let alloc = alloc.into_owned();
-        let stacks =
-            ecx.machine.stacked_borrows.as_ref().map(|stacked_borrows| {
-                Stacks::new_allocation(id, alloc.size(), stacked_borrows, kind)
-            });
+        let stacks = ecx.machine.stacked_borrows.as_ref().map(|stacked_borrows| {
+            Stacks::new_allocation(
+                id,
+                alloc.size(),
+                stacked_borrows,
+                kind,
+                ecx.machine.current_span(*ecx.tcx),
+            )
+        });
         let race_alloc = ecx.machine.data_race.as_ref().map(|data_race| {
             data_race::AllocExtra::new_allocation(
                 data_race,

src/stacked_borrows/diagnostics.rs

@@ -2,7 +2,7 @@ use smallvec::SmallVec;
 use std::fmt;
 
 use rustc_middle::mir::interpret::{alloc_range, AllocId, AllocRange};
-use rustc_span::{Span, SpanData, DUMMY_SP};
+use rustc_span::{Span, SpanData};
 use rustc_target::abi::Size;
 
 use crate::helpers::CurrentSpan;
@@ -14,6 +14,7 @@ use rustc_middle::mir::interpret::InterpError;
 #[derive(Clone, Debug)]
 pub struct AllocHistory {
     id: AllocId,
+    base: (Item, Span),
     creations: smallvec::SmallVec<[Creation; 1]>,
     invalidations: smallvec::SmallVec<[Invalidation; 1]>,
     protectors: smallvec::SmallVec<[Protection; 1]>,
@@ -91,8 +92,6 @@ impl fmt::Display for InvalidationCause {
 
 #[derive(Clone, Debug)]
 struct Protection {
-    /// The parent tag from which this protected tag was derived.
-    orig_tag: ProvenanceExtra,
     tag: SbTag,
     span: Span,
 }
@@ -101,7 +100,7 @@ struct Protection {
 pub struct TagHistory {
     pub created: (String, SpanData),
     pub invalidated: Option<(String, SpanData)>,
-    pub protected: Option<([(String, SpanData); 2])>,
+    pub protected: Option<(String, SpanData)>,
 }
 
 pub struct DiagnosticCxBuilder<'span, 'ecx, 'mir, 'tcx> {
@@ -228,9 +227,10 @@ struct DeallocOp {
 }
 
 impl AllocHistory {
-    pub fn new(id: AllocId) -> Self {
+    pub fn new(id: AllocId, item: Item, current_span: &mut CurrentSpan<'_, '_, '_>) -> Self {
         Self {
             id,
+            base: (item, current_span.get()),
             creations: SmallVec::new(),
             invalidations: SmallVec::new(),
             protectors: SmallVec::new(),
@@ -290,11 +290,7 @@ impl<'span, 'history, 'ecx, 'mir, 'tcx> DiagnosticCx<'span, 'history, 'ecx, 'mir
         let Operation::Retag(op) = &self.operation else {
             unreachable!("Protectors can only be created during a retag")
         };
-        self.history.protectors.push(Protection {
-            orig_tag: op.orig_tag,
-            tag: op.new_tag,
-            span: self.current_span.get(),
-        });
+        self.history.protectors.push(Protection { tag: op.new_tag, span: self.current_span.get() });
     }
 
     pub fn get_logs_relevant_to(
@@ -331,6 +327,17 @@ impl<'span, 'history, 'ecx, 'mir, 'tcx> DiagnosticCx<'span, 'history, 'ecx, 'mir
                         None
                     }
                 })
+            }).or_else(|| {
+                // If we didn't find a retag that created this tag, it might be the base tag of
+                // this allocation.
+                if self.history.base.0.tag() == tag {
+                    Some((
+                        format!("{:?} was created here, as a base tag for {:?}", tag, self.history.id),
+                        self.history.base.1.data()
+                    ))
+                } else {
+                    None
+                }
             }) else {
                 // But if we don't have a creation event, this is related to a wildcard, and there
                 // is really nothing we can do to help.
@@ -343,40 +350,11 @@ impl<'span, 'history, 'ecx, 'mir, 'tcx> DiagnosticCx<'span, 'history, 'ecx, 'mir
 
         let protected = protector_tag
             .and_then(|protector| {
-                self.history.protectors.iter().find(|protection| {
-                    protection.tag == protector
-                })
+                self.history.protectors.iter().find(|protection| protection.tag == protector)
             })
-            .and_then(|protection| {
-                self.history.creations.iter().rev().find_map(|event| {
-                    if ProvenanceExtra::Concrete(event.retag.new_tag) == protection.orig_tag {
-                        Some((protection, event))
-                    } else {
-                        None
-                    }
-                })
-            })
-            .map(|(protection, protection_parent)| {
+            .map(|protection| {
                 let protected_tag = protection.tag;
-                [
-                    (
-                        format!(
-                            "{tag:?} cannot be used for memory access because that would remove protected tag {protected_tag:?}, protected by this function call",
-                        ),
-                        protection.span.data(),
-                    ),
-                    if protection_parent.retag.new_tag == tag {
-                        (format!("{protected_tag:?} was derived from {tag:?}, the tag used for this memory access"), DUMMY_SP.data())
-                    } else {
-                        (
-                            format!(
-                                "{protected_tag:?} was derived from {protected_parent_tag:?}, which in turn was created here",
-                                protected_parent_tag = protection_parent.retag.new_tag,
-                            ),
-                            protection_parent.span.data()
-                        )
-                    }
-                ]
+                (format!("{protected_tag:?} is this argument"), protection.span.data())
             });
 
         Some(TagHistory { created, invalidated, protected })
@@ -448,7 +426,7 @@ impl<'span, 'history, 'ecx, 'mir, 'tcx> DiagnosticCx<'span, 'history, 'ecx, 'mir
             | Operation::Access(AccessOp { tag, .. }) =>
                 err_sb_ub(
                     format!(
-                        "not granting access to tag {:?} because incompatible item {:?} is protected by call {:?}",
+                        "not granting access to tag {:?} because that would remove {:?} which is protected because it is an argument of call {:?}",
                         tag, item, call_id
                     ),
                     None,

src/stacked_borrows/mod.rs

@@ -500,13 +500,19 @@ impl<'tcx> Stack {
 impl<'tcx> Stacks {
     /// Creates a new stack with an initial tag. For diagnostic purposes, we also need to know
     /// the [`AllocId`] of the allocation this is associated with.
-    fn new(size: Size, perm: Permission, tag: SbTag, id: AllocId) -> Self {
+    fn new(
+        size: Size,
+        perm: Permission,
+        tag: SbTag,
+        id: AllocId,
+        current_span: &mut CurrentSpan<'_, '_, '_>,
+    ) -> Self {
         let item = Item::new(tag, perm, false);
         let stack = Stack::new(item);
 
         Stacks {
             stacks: RangeMap::new(size, stack),
-            history: AllocHistory::new(id),
+            history: AllocHistory::new(id, item, current_span),
             exposed_tags: FxHashSet::default(),
         }
     }
@@ -538,6 +544,7 @@ impl Stacks {
         size: Size,
         state: &GlobalState,
         kind: MemoryKind<MiriMemoryKind>,
+        mut current_span: CurrentSpan<'_, '_, '_>,
     ) -> Self {
         let mut extra = state.borrow_mut();
         let (base_tag, perm) = match kind {
@@ -550,7 +557,7 @@ impl Stacks {
             // Everything else is shared by default.
             _ => (extra.base_ptr_tag(id), Permission::SharedReadWrite),
         };
-        Stacks::new(size, perm, base_tag, id)
+        Stacks::new(size, perm, base_tag, id, &mut current_span)
     }
 
     #[inline(always)]

tests/fail/stacked_borrows/aliasing_mut1.stderr

@@ -1,8 +1,8 @@
-error: Undefined Behavior: not granting access to tag <TAG> because incompatible item [Unique for <TAG>] is protected by call ID
+error: Undefined Behavior: not granting access to tag <TAG> because that would remove [Unique for <TAG>] which is protected because it is an argument of call ID
   --> $DIR/aliasing_mut1.rs:LL:CC
    |
 LL | pub fn safe(_x: &mut i32, _y: &mut i32) {}
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not granting access to tag <TAG> because incompatible item [Unique for <TAG>] is protected by call ID
+   |                           ^^ not granting access to tag <TAG> because that would remove [Unique for <TAG>] which is protected because it is an argument of call ID
    |
    = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental
    = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
@@ -11,12 +11,11 @@ help: <TAG> was created by a Unique retag at offsets [0x0..0x4]
    |
 LL |     let xraw: *mut i32 = unsafe { mem::transmute(&mut x) };
    |                                                  ^^^^^^
-help: <TAG> cannot be used for memory access because that would remove protected tag <TAG>, protected by this function call
+help: <TAG> is this argument
   --> $DIR/aliasing_mut1.rs:LL:CC
    |
 LL | pub fn safe(_x: &mut i32, _y: &mut i32) {}
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-   = help: <TAG> was derived from <TAG>, the tag used for this memory access
+   |             ^^
    = note: backtrace:
    = note: inside `safe` at $DIR/aliasing_mut1.rs:LL:CC
 note: inside `main` at $DIR/aliasing_mut1.rs:LL:CC

tests/fail/stacked_borrows/aliasing_mut2.stderr

@@ -1,8 +1,8 @@
-error: Undefined Behavior: not granting access to tag <TAG> because incompatible item [SharedReadOnly for <TAG>] is protected by call ID
+error: Undefined Behavior: not granting access to tag <TAG> because that would remove [SharedReadOnly for <TAG>] which is protected because it is an argument of call ID
   --> $DIR/aliasing_mut2.rs:LL:CC
    |
 LL | pub fn safe(_x: &i32, _y: &mut i32) {}
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not granting access to tag <TAG> because incompatible item [SharedReadOnly for <TAG>] is protected by call ID
+   |                       ^^ not granting access to tag <TAG> because that would remove [SharedReadOnly for <TAG>] which is protected because it is an argument of call ID
    |
    = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental
    = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
@@ -11,16 +11,11 @@ help: <TAG> was created by a Unique retag at offsets [0x0..0x4]
    |
 LL |     let xref = &mut x;
    |                ^^^^^^
-help: <TAG> cannot be used for memory access because that would remove protected tag <TAG>, protected by this function call
+help: <TAG> is this argument
   --> $DIR/aliasing_mut2.rs:LL:CC
    |
 LL | pub fn safe(_x: &i32, _y: &mut i32) {}
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-help: <TAG> was derived from <TAG>, which in turn was created here
-  --> $DIR/aliasing_mut2.rs:LL:CC
-   |
-LL |     safe_raw(xshr, xraw);
-   |              ^^^^
+   |             ^^
    = note: backtrace:
    = note: inside `safe` at $DIR/aliasing_mut2.rs:LL:CC
 note: inside `main` at $DIR/aliasing_mut2.rs:LL:CC

tests/fail/stacked_borrows/aliasing_mut3.stderr

@@ -2,10 +2,10 @@ error: Undefined Behavior: trying to retag from <TAG> for SharedReadOnly permiss
   --> $DIR/aliasing_mut3.rs:LL:CC
    |
 LL | pub fn safe(_x: &mut i32, _y: &i32) {}
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-   | |
-   | trying to retag from <TAG> for SharedReadOnly permission at ALLOC[0x0], but that tag does not exist in the borrow stack for this location
-   | this error occurs as part of FnEntry retag at ALLOC[0x0..0x4]
+   |                           ^^
+   |                           |
+   |                           trying to retag from <TAG> for SharedReadOnly permission at ALLOC[0x0], but that tag does not exist in the borrow stack for this location
+   |                           this error occurs as part of FnEntry retag at ALLOC[0x0..0x4]
    |
    = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental
    = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information

tests/fail/stacked_borrows/aliasing_mut4.stderr

@@ -1,8 +1,8 @@
-error: Undefined Behavior: not granting access to tag <TAG> because incompatible item [SharedReadOnly for <TAG>] is protected by call ID
+error: Undefined Behavior: not granting access to tag <TAG> because that would remove [SharedReadOnly for <TAG>] which is protected because it is an argument of call ID
   --> $DIR/aliasing_mut4.rs:LL:CC
    |
 LL | pub fn safe(_x: &i32, _y: &mut Cell<i32>) {}
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not granting access to tag <TAG> because incompatible item [SharedReadOnly for <TAG>] is protected by call ID
+   |                       ^^ not granting access to tag <TAG> because that would remove [SharedReadOnly for <TAG>] which is protected because it is an argument of call ID
    |
    = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental
    = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
@@ -11,16 +11,11 @@ help: <TAG> was created by a Unique retag at offsets [0x0..0x4]
    |
 LL |     let xref = &mut x;
    |                ^^^^^^
-help: <TAG> cannot be used for memory access because that would remove protected tag <TAG>, protected by this function call
+help: <TAG> is this argument
   --> $DIR/aliasing_mut4.rs:LL:CC
    |
 LL | pub fn safe(_x: &i32, _y: &mut Cell<i32>) {}
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-help: <TAG> was derived from <TAG>, which in turn was created here
-  --> $DIR/aliasing_mut4.rs:LL:CC
-   |
-LL |     safe_raw(xshr, xraw as *mut _);
-   |              ^^^^
+   |             ^^
    = note: backtrace:
    = note: inside `safe` at $DIR/aliasing_mut4.rs:LL:CC
 note: inside `main` at $DIR/aliasing_mut4.rs:LL:CC

tests/fail/stacked_borrows/illegal_write6.rs

@@ -7,6 +7,6 @@ fn main() {
 fn foo(a: &mut u32, y: *mut u32) -> u32 {
     *a = 1;
     let _b = &*a;
-    unsafe { *y = 2 }; //~ ERROR: /not granting access .* because incompatible item .* is protected/
+    unsafe { *y = 2 }; //~ ERROR: /not granting access .* because that would remove .* which is protected/
     return *a;
 }

tests/fail/stacked_borrows/illegal_write6.stderr

@@ -1,8 +1,8 @@
-error: Undefined Behavior: not granting access to tag <TAG> because incompatible item [Unique for <TAG>] is protected by call ID
+error: Undefined Behavior: not granting access to tag <TAG> because that would remove [Unique for <TAG>] which is protected because it is an argument of call ID
   --> $DIR/illegal_write6.rs:LL:CC
    |
 LL |     unsafe { *y = 2 };
-   |              ^^^^^^ not granting access to tag <TAG> because incompatible item [Unique for <TAG>] is protected by call ID
+   |              ^^^^^^ not granting access to tag <TAG> because that would remove [Unique for <TAG>] which is protected because it is an argument of call ID
    |
    = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental
    = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
@@ -11,21 +11,11 @@ help: <TAG> was created by a SharedReadWrite retag at offsets [0x0..0x4]
    |
 LL |     let p = x as *mut u32;
    |             ^
-help: <TAG> cannot be used for memory access because that would remove protected tag <TAG>, protected by this function call
+help: <TAG> is this argument
   --> $DIR/illegal_write6.rs:LL:CC
    |
-LL | / fn foo(a: &mut u32, y: *mut u32) -> u32 {
-LL | |     *a = 1;
-LL | |     let _b = &*a;
-LL | |     unsafe { *y = 2 };
-LL | |     return *a;
-LL | | }
-   | |_^
-help: <TAG> was derived from <TAG>, which in turn was created here
-  --> $DIR/illegal_write6.rs:LL:CC
-   |
-LL |     foo(x, p);
-   |         ^
+LL | fn foo(a: &mut u32, y: *mut u32) -> u32 {
+   |        ^
    = note: backtrace:
    = note: inside `foo` at $DIR/illegal_write6.rs:LL:CC
 note: inside `main` at $DIR/illegal_write6.rs:LL:CC

tests/fail/stacked_borrows/invalidate_against_protector1.stderr

@@ -1,8 +1,8 @@
-error: Undefined Behavior: not granting access to tag <TAG> because incompatible item [Unique for <TAG>] is protected by call ID
+error: Undefined Behavior: not granting access to tag <TAG> because that would remove [Unique for <TAG>] which is protected because it is an argument of call ID
   --> $DIR/invalidate_against_protector1.rs:LL:CC
    |
 LL |     let _val = unsafe { *x };
-   |                         ^^ not granting access to tag <TAG> because incompatible item [Unique for <TAG>] is protected by call ID
+   |                         ^^ not granting access to tag <TAG> because that would remove [Unique for <TAG>] which is protected because it is an argument of call ID
    |
    = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental
    = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
@@ -11,21 +11,11 @@ help: <TAG> was created by a SharedReadWrite retag at offsets [0x0..0x4]
    |
 LL |     let xraw = &mut x as *mut _;
    |                ^^^^^^
-help: <TAG> cannot be used for memory access because that would remove protected tag <TAG>, protected by this function call
+help: <TAG> is this argument
   --> $DIR/invalidate_against_protector1.rs:LL:CC
    |
-LL | / fn inner(x: *mut i32, _y: &mut i32) {
-LL | |     // If `x` and `y` alias, retagging is fine with this... but we really
-LL | |     // shouldn't be allowed to use `x` at all because `y` was assumed to be
-LL | |     // unique for the duration of this call.
-LL | |     let _val = unsafe { *x };
-LL | | }
-   | |_^
-help: <TAG> was derived from <TAG>, which in turn was created here
-  --> $DIR/invalidate_against_protector1.rs:LL:CC
-   |
-LL |     inner(xraw, xref);
-   |                 ^^^^
+LL | fn inner(x: *mut i32, _y: &mut i32) {
+   |                       ^^
    = note: backtrace:
    = note: inside `inner` at $DIR/invalidate_against_protector1.rs:LL:CC
 note: inside `main` at $DIR/invalidate_against_protector1.rs:LL:CC