Rollup merge of rust-lang#122018 - RalfJung:box-custom-alloc, r=oli-obk

matthiaskrgr · web-flow · commit 8f0b1e496108 · 2024-03-05T19:49:00.000+01:00
only set noalias on Box with the global allocator As discovered in rust-lang/miri#3341, `noalias` and custom allocators don't go well together. rustc can now check whether a Box uses the global allocator. This replaces the previous ad-hoc and rather unprincipled check for a zero-sized allocator. This is the rustc part of fixing that; Miri will also need a patch.
diff --git a/compiler/rustc_abi/src/lib.rs b/compiler/rustc_abi/src/lib.rs
@@ -1612,8 +1612,9 @@ pub enum PointerKind {
     SharedRef { frozen: bool },
     /// Mutable reference. `unpin` indicates the absence of any pinned data.
     MutableRef { unpin: bool },
-    /// Box. `unpin` indicates the absence of any pinned data.
-    Box { unpin: bool },
+    /// Box. `unpin` indicates the absence of any pinned data. `global` indicates whether this box
+    /// uses the global allocator or a custom one.
+    Box { unpin: bool, global: bool },
 }
 
 /// Note that this information is advisory only, and backends are free to ignore it.
@@ -1622,6 +1623,8 @@ pub enum PointerKind {
 pub struct PointeeInfo {
     pub size: Size,
     pub align: Align,
+    /// If this is `None`, then this is a raw pointer, so size and alignment are not guaranteed to
+    /// be reliable.
     pub safe: Option<PointerKind>,
 }
 
diff --git a/compiler/rustc_codegen_cranelift/example/mini_core.rs b/compiler/rustc_codegen_cranelift/example/mini_core.rs
@@ -525,8 +525,11 @@ pub struct Unique<T: ?Sized> {
 impl<T: ?Sized, U: ?Sized> CoerceUnsized<Unique<U>> for Unique<T> where T: Unsize<U> {}
 impl<T: ?Sized, U: ?Sized> DispatchFromDyn<Unique<U>> for Unique<T> where T: Unsize<U> {}
 
+#[lang = "global_alloc_ty"]
+pub struct Global;
+
 #[lang = "owned_box"]
-pub struct Box<T: ?Sized, A = ()>(Unique<T>, A);
+pub struct Box<T: ?Sized, A = Global>(Unique<T>, A);
 
 impl<T: ?Sized + Unsize<U>, U: ?Sized> CoerceUnsized<Box<U>> for Box<T> {}
 
@@ -536,7 +539,7 @@ impl<T> Box<T> {
             let size = intrinsics::size_of::<T>();
             let ptr = libc::malloc(size);
             intrinsics::copy(&val as *const T as *const u8, ptr, size);
-            Box(Unique { pointer: NonNull(ptr as *const T), _marker: PhantomData }, ())
+            Box(Unique { pointer: NonNull(ptr as *const T), _marker: PhantomData }, Global)
         }
     }
 }
diff --git a/compiler/rustc_codegen_cranelift/src/unsize.rs b/compiler/rustc_codegen_cranelift/src/unsize.rs
@@ -74,10 +74,6 @@ fn unsize_ptr<'tcx>(
         | (&ty::RawPtr(ty::TypeAndMut { ty: a, .. }), &ty::RawPtr(ty::TypeAndMut { ty: b, .. })) => {
             (src, unsized_info(fx, *a, *b, old_info))
         }
-        (&ty::Adt(def_a, _), &ty::Adt(def_b, _)) if def_a.is_box() && def_b.is_box() => {
-            let (a, b) = (src_layout.ty.boxed_ty(), dst_layout.ty.boxed_ty());
-            (src, unsized_info(fx, a, b, old_info))
-        }
         (&ty::Adt(def_a, _), &ty::Adt(def_b, _)) => {
             assert_eq!(def_a, def_b);
 
diff --git a/compiler/rustc_codegen_gcc/example/mini_core.rs b/compiler/rustc_codegen_gcc/example/mini_core.rs
@@ -472,6 +472,7 @@ pub trait Allocator {
 
 impl Allocator for () {}
 
+#[lang = "global_alloc_ty"]
 pub struct Global;
 
 impl Allocator for Global {}
diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs b/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs
@@ -454,9 +454,13 @@ pub fn type_di_node<'ll, 'tcx>(cx: &CodegenCx<'ll, 'tcx>, t: Ty<'tcx>) -> &'ll D
         ty::RawPtr(ty::TypeAndMut { ty: pointee_type, .. }) | ty::Ref(_, pointee_type, _) => {
             build_pointer_or_reference_di_node(cx, t, pointee_type, unique_type_id)
         }
-        // Box<T, A> may have a non-1-ZST allocator A. In that case, we
-        // cannot treat Box<T, A> as just an owned alias of `*mut T`.
-        ty::Adt(def, args) if def.is_box() && cx.layout_of(args.type_at(1)).is_1zst() => {
+        // Some `Box` are newtyped pointers, make debuginfo aware of that.
+        // Only works if the allocator argument is a 1-ZST and hence irrelevant for layout
+        // (or if there is no allocator argument).
+        ty::Adt(def, args)
+            if def.is_box()
+                && args.get(1).map_or(true, |arg| cx.layout_of(arg.expect_ty()).is_1zst()) =>
+        {
             build_pointer_or_reference_di_node(cx, t, t.boxed_ty(), unique_type_id)
         }
         ty::FnDef(..) | ty::FnPtr(_) => build_subroutine_type_di_node(cx, unique_type_id),
diff --git a/compiler/rustc_codegen_ssa/src/mir/operand.rs b/compiler/rustc_codegen_ssa/src/mir/operand.rs
@@ -204,6 +204,7 @@ impl<'a, 'tcx, V: CodegenObject> OperandRef<'tcx, V> {
 
     pub fn deref<Cx: LayoutTypeMethods<'tcx>>(self, cx: &Cx) -> PlaceRef<'tcx, V> {
         if self.layout.ty.is_box() {
+            // Derefer should have removed all Box derefs
             bug!("dereferencing {:?} in codegen", self.layout.ty);
         }
 
diff --git a/compiler/rustc_const_eval/src/interpret/place.rs b/compiler/rustc_const_eval/src/interpret/place.rs
@@ -437,6 +437,7 @@ where
         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);
         }
 
diff --git a/compiler/rustc_const_eval/src/interpret/terminator.rs b/compiler/rustc_const_eval/src/interpret/terminator.rs
@@ -359,14 +359,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
             Ok(Some(match ty.kind() {
                 ty::Ref(_, ty, _) => *ty,
                 ty::RawPtr(mt) => mt.ty,
-                // We should only accept `Box` with the default allocator.
-                // It's hard to test for that though so we accept every 1-ZST allocator.
-                ty::Adt(def, args)
-                    if def.is_box()
-                        && self.layout_of(args[1].expect_ty()).is_ok_and(|l| l.is_1zst()) =>
-                {
-                    args[0].expect_ty()
-                }
+                // We only accept `Box` with the default allocator.
+                _ if ty.is_box_global(*self.tcx) => ty.boxed_ty(),
                 _ => return Ok(None),
             }))
         };
diff --git a/compiler/rustc_hir/src/lang_items.rs b/compiler/rustc_hir/src/lang_items.rs
@@ -267,6 +267,8 @@ language_item_table! {
     EhCatchTypeinfo,         sym::eh_catch_typeinfo,   eh_catch_typeinfo,          Target::Static,         GenericRequirement::None;
 
     OwnedBox,                sym::owned_box,           owned_box,                  Target::Struct,         GenericRequirement::Minimum(1);
+    GlobalAlloc,             sym::global_alloc_ty,     global_alloc_ty,            Target::Struct,         GenericRequirement::None;
+
     // Experimental language item for Miri
     PtrUnique,               sym::ptr_unique,          ptr_unique,                 Target::Struct,         GenericRequirement::Exact(1);
 
diff --git a/compiler/rustc_middle/src/ty/layout.rs b/compiler/rustc_middle/src/ty/layout.rs
@@ -969,6 +969,8 @@ where
         }
     }
 
+    /// Compute the information for the pointer stored at the given offset inside this type.
+    /// This will recurse into fields of ADTs to find the inner pointer.
     fn ty_and_layout_pointee_info_at(
         this: TyAndLayout<'tcx>,
         cx: &C,
@@ -1068,15 +1070,17 @@ where
                     }
                 }
 
-                // FIXME(eddyb) This should be for `ptr::Unique<T>`, not `Box<T>`.
+                // Fixup info for the first field of a `Box`. Recursive traversal will have found
+                // the raw pointer, so size and align are set to the boxed type, but `pointee.safe`
+                // will still be `None`.
                 if let Some(ref mut pointee) = result {
-                    if let ty::Adt(def, _) = this.ty.kind() {
-                        if def.is_box() && offset.bytes() == 0 {
-                            let optimize = tcx.sess.opts.optimize != OptLevel::No;
-                            pointee.safe = Some(PointerKind::Box {
-                                unpin: optimize && this.ty.boxed_ty().is_unpin(tcx, cx.param_env()),
-                            });
-                        }
+                    if offset.bytes() == 0 && this.ty.is_box() {
+                        debug_assert!(pointee.safe.is_none());
+                        let optimize = tcx.sess.opts.optimize != OptLevel::No;
+                        pointee.safe = Some(PointerKind::Box {
+                            unpin: optimize && this.ty.boxed_ty().is_unpin(tcx, cx.param_env()),
+                            global: this.ty.is_box_global(tcx),
+                        });
                     }
                 }
 
diff --git a/compiler/rustc_middle/src/ty/sty.rs b/compiler/rustc_middle/src/ty/sty.rs
@@ -1999,6 +1999,27 @@ impl<'tcx> Ty<'tcx> {
         }
     }
 
+    /// Tests whether this is a Box using the global allocator.
+    #[inline]
+    pub fn is_box_global(self, tcx: TyCtxt<'tcx>) -> bool {
+        match self.kind() {
+            Adt(def, args) if def.is_box() => {
+                let Some(alloc) = args.get(1) else {
+                    // Single-argument Box is always global. (for "minicore" tests)
+                    return true;
+                };
+                if let Some(alloc_adt) = alloc.expect_ty().ty_adt_def() {
+                    let global_alloc = tcx.require_lang_item(LangItem::GlobalAlloc, None);
+                    alloc_adt.did() == global_alloc
+                } else {
+                    // Allocator is not an ADT...
+                    false
+                }
+            }
+            _ => false,
+        }
+    }
+
     /// Panics if called on any type other than `Box<T>`.
     pub fn boxed_ty(self) -> Ty<'tcx> {
         match self.kind() {
diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs
@@ -896,6 +896,7 @@ symbols! {
         generic_const_items,
         generic_param_attrs,
         get_context,
+        global_alloc_ty,
         global_allocator,
         global_asm,
         globs,
diff --git a/compiler/rustc_ty_utils/src/abi.rs b/compiler/rustc_ty_utils/src/abi.rs
@@ -452,7 +452,7 @@ fn adjust_for_rust_scalar<'tcx>(
             let no_alias = match kind {
                 PointerKind::SharedRef { frozen } => frozen,
                 PointerKind::MutableRef { unpin } => unpin && noalias_mut_ref,
-                PointerKind::Box { unpin } => unpin && noalias_for_box,
+                PointerKind::Box { unpin, global } => unpin && global && noalias_for_box,
             };
             // We can never add `noalias` in return position; that LLVM attribute has some very surprising semantics
             // (see <https://github.com/rust-lang/unsafe-code-guidelines/issues/385#issuecomment-1368055745>).
diff --git a/library/alloc/src/alloc.rs b/library/alloc/src/alloc.rs
@@ -50,6 +50,8 @@ extern "Rust" {
 #[unstable(feature = "allocator_api", issue = "32838")]
 #[derive(Copy, Clone, Default, Debug)]
 #[cfg(not(test))]
+// the compiler needs to know when a Box uses the global allocator vs a custom one
+#[cfg_attr(not(bootstrap), lang = "global_alloc_ty")]
 pub struct Global;
 
 #[cfg(test)]
diff --git a/library/alloc/src/boxed.rs b/library/alloc/src/boxed.rs
@@ -2062,6 +2062,9 @@ impl<Args: Tuple, F: AsyncFn<Args> + ?Sized, A: Allocator> AsyncFn<Args> for Box
 #[unstable(feature = "coerce_unsized", issue = "18598")]
 impl<T: ?Sized + Unsize<U>, U: ?Sized, A: Allocator> CoerceUnsized<Box<U, A>> for Box<T, A> {}
 
+// It is quite crucial that we only allow the `Global` allocator here.
+// Handling arbitrary custom allocators (which can affect the `Box` layout heavily!)
+// would need a lot of codegen and interpreter adjustments.
 #[unstable(feature = "dispatch_from_dyn", issue = "none")]
 impl<T: ?Sized + Unsize<U>, U: ?Sized> DispatchFromDyn<Box<U>> for Box<T, Global> {}
 
diff --git a/tests/codegen/function-arguments.rs b/tests/codegen/function-arguments.rs
@@ -2,6 +2,7 @@
 #![crate_type = "lib"]
 #![feature(dyn_star)]
 #![feature(generic_nonzero)]
+#![feature(allocator_api)]
 
 use std::mem::MaybeUninit;
 use std::num::NonZero;
@@ -182,6 +183,15 @@ pub fn _box(x: Box<i32>) -> Box<i32> {
   x
 }
 
+// With a custom allocator, it should *not* have `noalias`. (See
+// <https://github.com/rust-lang/miri/issues/3341> for why.) The second argument is the allocator,
+// which is a reference here that still carries `noalias` as usual.
+// CHECK: @_box_custom(ptr noundef nonnull align 4 %x.0, ptr noalias noundef nonnull readonly align 1 %x.1)
+#[no_mangle]
+pub fn _box_custom(x: Box<i32, &std::alloc::Global>) {
+  drop(x)
+}
+
 // CHECK: noundef nonnull align 4 ptr @notunpin_box(ptr noundef nonnull align 4 %x)
 #[no_mangle]
 pub fn notunpin_box(x: Box<NotUnpin>) -> Box<NotUnpin> {
diff --git a/tests/ui/abi/compatibility.rs b/tests/ui/abi/compatibility.rs
@@ -160,6 +160,7 @@ mod prelude {
         pub _marker: PhantomData<T>,
     }
 
+    #[lang = "global_alloc_ty"]
     pub struct Global;
 
     #[lang = "owned_box"]

Original file line number	Diff line number	Diff line change
`@@ -525,8 +525,11 @@ pub struct Unique<T: ?Sized> {`
`525`	`525`	`impl<T: ?Sized, U: ?Sized> CoerceUnsized<Unique<U>> for Unique<T> where T: Unsize<U> {}`
`526`	`526`	`impl<T: ?Sized, U: ?Sized> DispatchFromDyn<Unique<U>> for Unique<T> where T: Unsize<U> {}`
`527`	`527`
	`528`	`+#[lang = "global_alloc_ty"]`
	`529`	`+pub struct Global;`
	`530`	`+`
`528`	`531`	`#[lang = "owned_box"]`
`529`		`-pub struct Box<T: ?Sized, A = ()>(Unique<T>, A);`
	`532`	`+pub struct Box<T: ?Sized, A = Global>(Unique<T>, A);`
`530`	`533`
`531`	`534`	`impl<T: ?Sized + Unsize<U>, U: ?Sized> CoerceUnsized<Box<U>> for Box<T> {}`
`532`	`535`
`@@ -536,7 +539,7 @@ impl<T> Box<T> {`
`536`	`539`	`let size = intrinsics::size_of::<T>();`
`537`	`540`	`let ptr = libc::malloc(size);`
`538`	`541`	`intrinsics::copy(&val as const T as const u8, ptr, size);`
`539`		`- Box(Unique { pointer: NonNull(ptr as *const T), _marker: PhantomData }, ())`
	`542`	`+ Box(Unique { pointer: NonNull(ptr as *const T), _marker: PhantomData }, Global)`
`540`	`543`	`}`
`541`	`544`	`}`
`542`	`545`	`}`
Original file line number	Diff line number	Diff line change
`@@ -204,6 +204,7 @@ impl<'a, 'tcx, V: CodegenObject> OperandRef<'tcx, V> {`
`204`	`204`
`205`	`205`	`pub fn deref<Cx: LayoutTypeMethods<'tcx>>(self, cx: &Cx) -> PlaceRef<'tcx, V> {`
`206`	`206`	`if self.layout.ty.is_box() {`
	`207`	`+ // Derefer should have removed all Box derefs`
`207`	`208`	`bug!("dereferencing {:?} in codegen", self.layout.ty);`
`208`	`209`	`}`
`209`	`210`
Original file line number	Diff line number	Diff line change
`@@ -437,6 +437,7 @@ where`
`437`	`437`	`trace!("deref to {} on {:?}", val.layout.ty, *val);`
`438`	`438`
`439`	`439`	`if val.layout.ty.is_box() {`
	`440`	`+ // Derefer should have removed all Box derefs`
`440`	`441`	`bug!("dereferencing {}", val.layout.ty);`
`441`	`442`	`}`
`442`	`443`
Original file line number	Diff line number	Diff line change
`@@ -969,6 +969,8 @@ where`
`969`	`969`	`}`
`970`	`970`	`}`
`971`	`971`
	`972`	`+ /// Compute the information for the pointer stored at the given offset inside this type.`
	`973`	`+ /// This will recurse into fields of ADTs to find the inner pointer.`
`972`	`974`	`fn ty_and_layout_pointee_info_at(`
`973`	`975`	`this: TyAndLayout<'tcx>,`
`974`	`976`	`cx: &C,`
`@@ -1068,15 +1070,17 @@ where`
`1068`	`1070`	`}`
`1069`	`1071`	`}`
`1070`	`1072`
`1071`		- // FIXME(eddyb) This should be for `ptr::Unique<T>`, not `Box<T>`.
	`1073`	+ // Fixup info for the first field of a `Box`. Recursive traversal will have found
	`1074`	+ // the raw pointer, so size and align are set to the boxed type, but `pointee.safe`
	`1075`	+ // will still be `None`.
`1072`	`1076`	`if let Some(ref mut pointee) = result {`
`1073`		`- if let ty::Adt(def, _) = this.ty.kind() {`
`1074`		`- if def.is_box() && offset.bytes() == 0 {`
`1075`		`- let optimize = tcx.sess.opts.optimize != OptLevel::No;`
`1076`		`- pointee.safe = Some(PointerKind::Box {`
`1077`		`- unpin: optimize && this.ty.boxed_ty().is_unpin(tcx, cx.param_env()),`
`1078`		`- });`
`1079`		`- }`
	`1077`	`+ if offset.bytes() == 0 && this.ty.is_box() {`
	`1078`	`+ debug_assert!(pointee.safe.is_none());`
	`1079`	`+ let optimize = tcx.sess.opts.optimize != OptLevel::No;`
	`1080`	`+ pointee.safe = Some(PointerKind::Box {`
	`1081`	`+ unpin: optimize && this.ty.boxed_ty().is_unpin(tcx, cx.param_env()),`
	`1082`	`+ global: this.ty.is_box_global(tcx),`
	`1083`	`+ });`
`1080`	`1084`	`}`
`1081`	`1085`	`}`
`1082`	`1086`