Miri: properly check return-position noalias

compiler/rustc_middle/src/mir/mod.rs

@@ -1444,6 +1444,7 @@ impl Debug for Statement<'_> {
                 "Retag({}{:?})",
                 match kind {
                     RetagKind::FnEntry => "[fn entry] ",
+                    RetagKind::FnReturn => "[fn return] ",
                     RetagKind::TwoPhase => "[2phase] ",
                     RetagKind::Raw => "[raw] ",
                     RetagKind::Default => "",

compiler/rustc_middle/src/mir/syntax.rs

@@ -323,7 +323,7 @@ pub enum StatementKind<'tcx> {
     /// For code that is not specific to stacked borrows, you should consider retags to read and
     /// modify the place in an opaque way.
     ///
-    /// Only `RetagKind::Default` and `RetagKind::FnEntry` are permitted.
+    /// Only `RetagKind::Default`, `RetagKind::FnEntry`, `RetagKind::FnReturn` are permitted.
     Retag(RetagKind, Box<Place<'tcx>>),
 
     /// Encodes a user's type ascription. These need to be preserved
@@ -412,6 +412,8 @@ impl std::fmt::Display for NonDivergingIntrinsic<'_> {
 pub enum RetagKind {
     /// The initial retag of arguments when entering a function.
     FnEntry,
+    /// The retag of a value that was just returned from another function.
+    FnReturn,
     /// Retag preparing for a two-phase borrow.
     TwoPhase,
     /// Retagging raw pointers.

compiler/rustc_mir_transform/src/add_retag.rs

@@ -112,7 +112,7 @@ impl<'tcx> MirPass<'tcx> for AddRetag {
                 0,
                 Statement {
                     source_info,
-                    kind: StatementKind::Retag(RetagKind::Default, Box::new(dest_place)),
+                    kind: StatementKind::Retag(RetagKind::FnReturn, Box::new(dest_place)),
                 },
             );
         }

src/test/mir-opt/retag.main.SimplifyCfg-elaborate-drops.after.mir

@@ -76,7 +76,7 @@ fn main() -> () {
     }
 
     bb1: {
-        Retag(_3);                       // scope 1 at $DIR/retag.rs:+3:17: +3:36
+        Retag([fn return] _3);           // scope 1 at $DIR/retag.rs:+3:17: +3:36
         StorageDead(_6);                 // scope 1 at $DIR/retag.rs:+3:35: +3:36
         StorageDead(_4);                 // scope 1 at $DIR/retag.rs:+3:35: +3:36
         StorageDead(_7);                 // scope 1 at $DIR/retag.rs:+3:36: +3:37
@@ -122,7 +122,7 @@ fn main() -> () {
     }
 
     bb3: {
-        Retag(_15);                      // scope 6 at $DIR/retag.rs:+15:14: +15:19
+        Retag([fn return] _15);          // scope 6 at $DIR/retag.rs:+15:14: +15:19
         StorageDead(_17);                // scope 6 at $DIR/retag.rs:+15:18: +15:19
         StorageDead(_16);                // scope 6 at $DIR/retag.rs:+15:18: +15:19
         StorageDead(_18);                // scope 6 at $DIR/retag.rs:+15:19: +15:20
@@ -148,7 +148,7 @@ fn main() -> () {
     }
 
     bb4: {
-        Retag(_19);                      // scope 7 at $DIR/retag.rs:+18:5: +18:24
+        Retag([fn return] _19);          // scope 7 at $DIR/retag.rs:+18:5: +18:24
         StorageDead(_22);                // scope 7 at $DIR/retag.rs:+18:23: +18:24
         StorageDead(_20);                // scope 7 at $DIR/retag.rs:+18:23: +18:24
         StorageDead(_23);                // scope 7 at $DIR/retag.rs:+18:24: +18:25

src/tools/miri/src/borrow_tracker/mod.rs

@@ -87,15 +87,15 @@ pub struct GlobalStateInner {
     /// Table storing the "base" tag for each allocation.
     /// The base tag is the one used for the initial pointer.
     /// We need this in a separate table to handle cyclic statics.
-    pub base_ptr_tags: FxHashMap<AllocId, BorTag>,
+    base_ptr_tags: FxHashMap<AllocId, BorTag>,
     /// Next unused call ID (for protectors).
     pub next_call_id: CallId,
     /// All currently protected tags.
     /// An item is protected if its tag is in this set, *and* it has the "protected" bit set.
     /// We add tags to this when they are created with a protector in `reborrow`, and
     /// we remove tags from this when the call which is protecting them returns, in
     /// `GlobalStateInner::end_call`. See `Stack::item_popped` for more details.
-    pub protected_tags: FxHashMap<BorTag, ProtectorKind>,
+    pub protected_tags: FxHashSet<BorTag>,
     /// The pointer ids to trace
     pub tracked_pointer_tags: FxHashSet<BorTag>,
     /// The call ids to trace
@@ -142,26 +142,6 @@ pub enum RetagFields {
     OnlyScalar,
 }
 
-/// The flavor of the protector.
-#[derive(Copy, Clone, Debug, PartialEq, Eq)]
-pub enum ProtectorKind {
-    /// Protected against aliasing violations from other pointers.
-    ///
-    /// Items protected like this cause UB when they are invalidated, *but* the pointer itself may
-    /// still be used to issue a deallocation.
-    ///
-    /// This is required for LLVM IR pointers that are `noalias` but *not* `dereferenceable`.
-    WeakProtector,
-
-    /// Protected against any kind of invalidation.
-    ///
-    /// Items protected like this cause UB when they are invalidated or the memory is deallocated.
-    /// This is strictly stronger protection than `WeakProtector`.
-    ///
-    /// This is required for LLVM IR pointers that are `dereferenceable` (and also allows `noalias`).
-    StrongProtector,
-}
-
 /// Utilities for initialization and ID generation
 impl GlobalStateInner {
     pub fn new(
@@ -175,7 +155,7 @@ impl GlobalStateInner {
             next_ptr_tag: BorTag::one(),
             base_ptr_tags: FxHashMap::default(),
             next_call_id: NonZeroU64::new(1).unwrap(),
-            protected_tags: FxHashMap::default(),
+            protected_tags: FxHashSet::default(),
             tracked_pointer_tags,
             tracked_call_ids,
             retag_fields,

src/tools/miri/src/borrow_tracker/stacked_borrows/diagnostics.rs

@@ -6,8 +6,7 @@ use rustc_span::{Span, SpanData};
 use rustc_target::abi::Size;
 
 use crate::borrow_tracker::{
-    stacked_borrows::{err_sb_ub, Permission},
-    AccessKind, GlobalStateInner, ProtectorKind,
+    stacked_borrows::{err_sb_ub, Permission}, AccessKind, GlobalStateInner,
 };
 use crate::*;
 
@@ -87,12 +86,7 @@ impl fmt::Display for InvalidationCause {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self {
             InvalidationCause::Access(kind) => write!(f, "{kind}"),
-            InvalidationCause::Retag(perm, kind) =>
-                if *kind == RetagCause::FnEntry {
-                    write!(f, "{perm:?} FnEntry retag")
-                } else {
-                    write!(f, "{perm:?} retag")
-                },
+            InvalidationCause::Retag(perm, kind) => write!(f, "{perm:?} {retag}", retag = kind.summary()),
         }
     }
 }
@@ -193,7 +187,8 @@ struct RetagOp {
 #[derive(Debug, Clone, Copy, PartialEq)]
 pub enum RetagCause {
     Normal,
-    FnReturn,
+    Box,
+    FnReturnPlace,
     FnEntry,
     TwoPhase,
 }
@@ -398,11 +393,7 @@ impl<'history, 'ecx, 'mir, 'tcx> DiagnosticCx<'history, 'ecx, 'mir, 'tcx> {
     }
 
     #[inline(never)] // This is only called on fatal code paths
-    pub(super) fn protector_error(&self, item: &Item, kind: ProtectorKind) -> InterpError<'tcx> {
-        let protected = match kind {
-            ProtectorKind::WeakProtector => "weakly protected",
-            ProtectorKind::StrongProtector => "strongly protected",
-        };
+    pub(super) fn protector_error(&self, item: &Item) -> InterpError<'tcx> {
         let call_id = self
             .machine
             .threads
@@ -417,15 +408,15 @@ impl<'history, 'ecx, 'mir, 'tcx> DiagnosticCx<'history, 'ecx, 'mir, 'tcx> {
         match self.operation {
             Operation::Dealloc(_) =>
                 err_sb_ub(
-                    format!("deallocating while item {item:?} is {protected} by call {call_id:?}",),
+                    format!("deallocating while item {item:?} is protected by call {call_id:?}",),
                     None,
                     None,
                 ),
             Operation::Retag(RetagOp { orig_tag: tag, .. })
             | Operation::Access(AccessOp { tag, .. }) =>
                 err_sb_ub(
                     format!(
-                        "not granting access to tag {tag:?} because that would remove {item:?} which is {protected} because it is an argument of call {call_id:?}",
+                        "not granting access to tag {tag:?} because that would remove {item:?} which is protected because it is an argument of call {call_id:?}",
                     ),
                     None,
                     tag.and_then(|tag| self.get_logs_relevant_to(tag, Some(item.tag()))),
@@ -495,8 +486,9 @@ impl RetagCause {
     fn summary(&self) -> String {
         match self {
             RetagCause::Normal => "retag",
+            RetagCause::Box => "Box retag",
             RetagCause::FnEntry => "FnEntry retag",
-            RetagCause::FnReturn => "FnReturn retag",
+            RetagCause::FnReturnPlace => "return-place retag",
             RetagCause::TwoPhase => "two-phase retag",
         }
         .to_string()