From d0aedfbb900309bb633b3ba04bf008183cfdd9a7 Mon Sep 17 00:00:00 2001
From: Ralf Jung <post@ralfj.de>
Date: Sat, 31 Aug 2024 11:29:02 +0200
Subject: [PATCH] interpret, codegen: tweak some comments and checks regarding
 Box with custom allocator

---
 .../rustc_codegen_llvm/src/debuginfo/metadata.rs     |  2 ++
 compiler/rustc_const_eval/src/interpret/place.rs     | 12 +++++++-----
 compiler/rustc_middle/src/ty/sty.rs                  |  5 ++++-
 compiler/rustc_mir_transform/src/add_retag.rs        |  2 ++
 4 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs b/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs
index dc59e9349b057..17a9630c6557b 100644
--- a/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs
+++ b/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs
@@ -216,6 +216,8 @@ fn build_pointer_or_reference_di_node<'ll, 'tcx>(
                     //        need to make sure that we don't break existing debuginfo consumers
                     //        by doing that (at least not without a warning period).
                     let layout_type = if ptr_type.is_box() {
+                        // The assertion at the start of this function ensures we have a ZST allocator.
+                        // We'll make debuginfo "skip" all ZST allocators, not just the default allocator.
                         Ty::new_mut_ptr(cx.tcx, pointee_type)
                     } else {
                         ptr_type
diff --git a/compiler/rustc_const_eval/src/interpret/place.rs b/compiler/rustc_const_eval/src/interpret/place.rs
index 2afdd02c88035..ede6a51c712b9 100644
--- a/compiler/rustc_const_eval/src/interpret/place.rs
+++ b/compiler/rustc_const_eval/src/interpret/place.rs
@@ -438,14 +438,16 @@ where
         &self,
         src: &impl Readable<'tcx, M::Provenance>,
     ) -> InterpResult<'tcx, MPlaceTy<'tcx, M::Provenance>> {
+        if src.layout().ty.is_box() {
+            // Derefer should have removed all Box derefs.
+            // Some `Box` are not immediates (if they have a custom allocator)
+            // so the code below would fail.
+            bug!("dereferencing {}", src.layout().ty);
+        }
+
         let val = self.read_immediate(src)?;
         trace!("deref to {} on {:?}", val.layout.ty, *val);
 
-        if val.layout.ty.is_box() {
-            // Derefer should have removed all Box derefs
-            bug!("dereferencing {}", val.layout.ty);
-        }
-
         let mplace = self.ref_to_mplace(&val)?;
         Ok(mplace)
     }
diff --git a/compiler/rustc_middle/src/ty/sty.rs b/compiler/rustc_middle/src/ty/sty.rs
index d60bfb9faa1aa..8db9c93613b2e 100644
--- a/compiler/rustc_middle/src/ty/sty.rs
+++ b/compiler/rustc_middle/src/ty/sty.rs
@@ -1149,7 +1149,10 @@ impl<'tcx> Ty<'tcx> {
         }
     }
 
-    /// Tests whether this is a Box using the global allocator.
+    /// Tests whether this is a Box definitely using the global allocator.
+    ///
+    /// If the allocator is still generic, the answer is `false`, but it may
+    /// later turn out that it does use the global allocator.
     #[inline]
     pub fn is_box_global(self, tcx: TyCtxt<'tcx>) -> bool {
         match self.kind() {
diff --git a/compiler/rustc_mir_transform/src/add_retag.rs b/compiler/rustc_mir_transform/src/add_retag.rs
index 12a68790374e5..79bc21cab147e 100644
--- a/compiler/rustc_mir_transform/src/add_retag.rs
+++ b/compiler/rustc_mir_transform/src/add_retag.rs
@@ -137,6 +137,8 @@ impl<'tcx> MirPass<'tcx> for AddRetag {
                                 // Using `is_box_global` here is a bit sketchy: if this code is
                                 // generic over the allocator, we'll not add a retag! This is a hack
                                 // to make Stacked Borrows compatible with custom allocator code.
+                                // It means the raw pointer inherits the tag of the box, which mostly works
+                                // but can sometimes lead to unexpected aliasing errors.
                                 // Long-term, we'll want to move to an aliasing model where "cast to
                                 // raw pointer" is a complete NOP, and then this will no longer be
                                 // an issue.