From f391c0793b443d30ef8c4d4228550439d4dbfead Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 5 Mar 2024 11:32:03 +0100 Subject: [PATCH] only set noalias on Box with the global allocator --- compiler/rustc_abi/src/lib.rs | 7 +++++-- .../example/mini_core.rs | 7 +++++-- .../rustc_codegen_cranelift/src/unsize.rs | 4 ---- .../rustc_codegen_gcc/example/mini_core.rs | 1 + .../src/debuginfo/metadata.rs | 10 ++++++--- compiler/rustc_codegen_ssa/src/mir/operand.rs | 1 + .../rustc_const_eval/src/interpret/place.rs | 1 + .../src/interpret/terminator.rs | 10 ++------- compiler/rustc_hir/src/lang_items.rs | 2 ++ compiler/rustc_middle/src/ty/layout.rs | 20 +++++++++++------- compiler/rustc_middle/src/ty/sty.rs | 21 +++++++++++++++++++ compiler/rustc_span/src/symbol.rs | 1 + compiler/rustc_ty_utils/src/abi.rs | 2 +- library/alloc/src/alloc.rs | 2 ++ library/alloc/src/boxed.rs | 3 +++ tests/codegen/function-arguments.rs | 10 +++++++++ tests/ui/abi/compatibility.rs | 1 + 17 files changed, 75 insertions(+), 28 deletions(-) diff --git a/compiler/rustc_abi/src/lib.rs b/compiler/rustc_abi/src/lib.rs index 34529e0a086..4f2b9d0ef50 100644 --- 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, } diff --git a/compiler/rustc_codegen_cranelift/example/mini_core.rs b/compiler/rustc_codegen_cranelift/example/mini_core.rs index a79909ce0c8..67a0d0dabea 100644 --- 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 { impl CoerceUnsized> for Unique where T: Unsize {} impl DispatchFromDyn> for Unique where T: Unsize {} +#[lang = "global_alloc_ty"] +pub struct Global; + #[lang = "owned_box"] -pub struct Box(Unique, A); +pub struct Box(Unique, A); impl, U: ?Sized> CoerceUnsized> for Box {} @@ -536,7 +539,7 @@ impl Box { let size = intrinsics::size_of::(); 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 index acfa461a6f3..7b61dc64cb1 100644 --- 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 index 230009741dc..cc3d647c8c8 100644 --- 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 index 1a5f9b42947..660f1647367 100644 --- 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 may have a non-1-ZST allocator A. In that case, we - // cannot treat Box 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 index 94eb37e78e0..932926976b5 100644 --- 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>(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 index 6e987784ff9..672008edfd3 100644 --- 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 index e72ace8be35..d29e69d753e 100644 --- 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 index 8a89a3b5faa..5118bf5c3b7 100644 --- 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 index 5a0e2aa691d..dda41f70f06 100644 --- 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`, not `Box`. + // 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 index 17659ac2330..1f68571cd37 100644 --- 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`. 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 index 3784a08b1b7..9e628e4ef91 100644 --- 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 index 43042dbd366..a5328baadb5 100644 --- 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 ). diff --git a/library/alloc/src/alloc.rs b/library/alloc/src/alloc.rs index 0b142939755..ed7b0618e9b 100644 --- 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 index b202d9250d5..2736e5ee6c5 100644 --- a/library/alloc/src/boxed.rs +++ b/library/alloc/src/boxed.rs @@ -2062,6 +2062,9 @@ impl + ?Sized, A: Allocator> AsyncFn for Box #[unstable(feature = "coerce_unsized", issue = "18598")] impl, U: ?Sized, A: Allocator> CoerceUnsized> for Box {} +// 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, U: ?Sized> DispatchFromDyn> for Box {} diff --git a/tests/codegen/function-arguments.rs b/tests/codegen/function-arguments.rs index b75c188f51a..e6e2f645714 100644 --- 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) -> Box { x } +// With a custom allocator, it should *not* have `noalias`. (See +// 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) { + drop(x) +} + // CHECK: noundef nonnull align 4 ptr @notunpin_box(ptr noundef nonnull align 4 %x) #[no_mangle] pub fn notunpin_box(x: Box) -> Box { diff --git a/tests/ui/abi/compatibility.rs b/tests/ui/abi/compatibility.rs index 2449e515f5f..a4f60ea2684 100644 --- a/tests/ui/abi/compatibility.rs +++ b/tests/ui/abi/compatibility.rs @@ -160,6 +160,7 @@ mod prelude { pub _marker: PhantomData, } + #[lang = "global_alloc_ty"] pub struct Global; #[lang = "owned_box"]