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

only set noalias on Box with the global allocator

As discovered in https://github.com/rust-lang/miri/issues/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.
This commit is contained in:
Matthias Krüger 2024-03-05 22:10:02 +01:00 committed by GitHub
commit b08837f180
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
17 changed files with 75 additions and 28 deletions

View File

@ -1612,8 +1612,9 @@ pub enum PointerKind {
SharedRef { frozen: bool }, SharedRef { frozen: bool },
/// Mutable reference. `unpin` indicates the absence of any pinned data. /// Mutable reference. `unpin` indicates the absence of any pinned data.
MutableRef { unpin: bool }, MutableRef { unpin: bool },
/// Box. `unpin` indicates the absence of any pinned data. /// Box. `unpin` indicates the absence of any pinned data. `global` indicates whether this box
Box { unpin: bool }, /// 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. /// 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 struct PointeeInfo {
pub size: Size, pub size: Size,
pub align: Align, 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>, pub safe: Option<PointerKind>,
} }

View File

@ -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> 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> {} 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"] #[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> {} impl<T: ?Sized + Unsize<U>, U: ?Sized> CoerceUnsized<Box<U>> for Box<T> {}
@ -536,7 +539,7 @@ pub fn new(val: T) -> Box<T> {
let size = intrinsics::size_of::<T>(); let size = intrinsics::size_of::<T>();
let ptr = libc::malloc(size); let ptr = libc::malloc(size);
intrinsics::copy(&val as *const T as *const u8, ptr, 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)
} }
} }
} }

View File

@ -74,10 +74,6 @@ fn unsize_ptr<'tcx>(
| (&ty::RawPtr(ty::TypeAndMut { ty: a, .. }), &ty::RawPtr(ty::TypeAndMut { ty: b, .. })) => { | (&ty::RawPtr(ty::TypeAndMut { ty: a, .. }), &ty::RawPtr(ty::TypeAndMut { ty: b, .. })) => {
(src, unsized_info(fx, *a, *b, old_info)) (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, _)) => { (&ty::Adt(def_a, _), &ty::Adt(def_b, _)) => {
assert_eq!(def_a, def_b); assert_eq!(def_a, def_b);

View File

@ -472,6 +472,7 @@ pub trait Allocator {
impl Allocator for () {} impl Allocator for () {}
#[lang = "global_alloc_ty"]
pub struct Global; pub struct Global;
impl Allocator for Global {} impl Allocator for Global {}

View File

@ -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, _) => { ty::RawPtr(ty::TypeAndMut { ty: pointee_type, .. }) | ty::Ref(_, pointee_type, _) => {
build_pointer_or_reference_di_node(cx, t, pointee_type, unique_type_id) 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 // Some `Box` are newtyped pointers, make debuginfo aware of that.
// cannot treat Box<T, A> as just an owned alias of `*mut T`. // Only works if the allocator argument is a 1-ZST and hence irrelevant for layout
ty::Adt(def, args) if def.is_box() && cx.layout_of(args.type_at(1)).is_1zst() => { // (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) 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), ty::FnDef(..) | ty::FnPtr(_) => build_subroutine_type_di_node(cx, unique_type_id),

View File

@ -204,6 +204,7 @@ pub fn immediate(self) -> V {
pub fn deref<Cx: LayoutTypeMethods<'tcx>>(self, cx: &Cx) -> PlaceRef<'tcx, V> { pub fn deref<Cx: LayoutTypeMethods<'tcx>>(self, cx: &Cx) -> PlaceRef<'tcx, V> {
if self.layout.ty.is_box() { if self.layout.ty.is_box() {
// Derefer should have removed all Box derefs
bug!("dereferencing {:?} in codegen", self.layout.ty); bug!("dereferencing {:?} in codegen", self.layout.ty);
} }

View File

@ -437,6 +437,7 @@ pub fn deref_pointer(
trace!("deref to {} on {:?}", val.layout.ty, *val); trace!("deref to {} on {:?}", val.layout.ty, *val);
if val.layout.ty.is_box() { if val.layout.ty.is_box() {
// Derefer should have removed all Box derefs
bug!("dereferencing {}", val.layout.ty); bug!("dereferencing {}", val.layout.ty);
} }

View File

@ -359,14 +359,8 @@ fn layout_compat(
Ok(Some(match ty.kind() { Ok(Some(match ty.kind() {
ty::Ref(_, ty, _) => *ty, ty::Ref(_, ty, _) => *ty,
ty::RawPtr(mt) => mt.ty, ty::RawPtr(mt) => mt.ty,
// We should only accept `Box` with the default allocator. // We only accept `Box` with the default allocator.
// It's hard to test for that though so we accept every 1-ZST allocator. _ if ty.is_box_global(*self.tcx) => ty.boxed_ty(),
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()
}
_ => return Ok(None), _ => return Ok(None),
})) }))
}; };

View File

@ -267,6 +267,8 @@ pub fn extract(attrs: &[ast::Attribute]) -> Option<(Symbol, Span)> {
EhCatchTypeinfo, sym::eh_catch_typeinfo, eh_catch_typeinfo, Target::Static, GenericRequirement::None; EhCatchTypeinfo, sym::eh_catch_typeinfo, eh_catch_typeinfo, Target::Static, GenericRequirement::None;
OwnedBox, sym::owned_box, owned_box, Target::Struct, GenericRequirement::Minimum(1); 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 // Experimental language item for Miri
PtrUnique, sym::ptr_unique, ptr_unique, Target::Struct, GenericRequirement::Exact(1); PtrUnique, sym::ptr_unique, ptr_unique, Target::Struct, GenericRequirement::Exact(1);

View File

@ -969,6 +969,8 @@ fn field_ty_or_layout<'tcx>(
} }
} }
/// 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( fn ty_and_layout_pointee_info_at(
this: TyAndLayout<'tcx>, this: TyAndLayout<'tcx>,
cx: &C, cx: &C,
@ -1068,15 +1070,17 @@ fn ty_and_layout_pointee_info_at(
} }
} }
// 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 Some(ref mut pointee) = result {
if let ty::Adt(def, _) = this.ty.kind() { if offset.bytes() == 0 && this.ty.is_box() {
if def.is_box() && offset.bytes() == 0 { debug_assert!(pointee.safe.is_none());
let optimize = tcx.sess.opts.optimize != OptLevel::No; let optimize = tcx.sess.opts.optimize != OptLevel::No;
pointee.safe = Some(PointerKind::Box { pointee.safe = Some(PointerKind::Box {
unpin: optimize && this.ty.boxed_ty().is_unpin(tcx, cx.param_env()), unpin: optimize && this.ty.boxed_ty().is_unpin(tcx, cx.param_env()),
}); global: this.ty.is_box_global(tcx),
} });
} }
} }

View File

@ -1999,6 +1999,27 @@ pub fn is_box(self) -> bool {
} }
} }
/// 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>`. /// Panics if called on any type other than `Box<T>`.
pub fn boxed_ty(self) -> Ty<'tcx> { pub fn boxed_ty(self) -> Ty<'tcx> {
match self.kind() { match self.kind() {

View File

@ -896,6 +896,7 @@
generic_const_items, generic_const_items,
generic_param_attrs, generic_param_attrs,
get_context, get_context,
global_alloc_ty,
global_allocator, global_allocator,
global_asm, global_asm,
globs, globs,

View File

@ -452,7 +452,7 @@ fn adjust_for_rust_scalar<'tcx>(
let no_alias = match kind { let no_alias = match kind {
PointerKind::SharedRef { frozen } => frozen, PointerKind::SharedRef { frozen } => frozen,
PointerKind::MutableRef { unpin } => unpin && noalias_mut_ref, 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 // 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>). // (see <https://github.com/rust-lang/unsafe-code-guidelines/issues/385#issuecomment-1368055745>).

View File

@ -50,6 +50,8 @@
#[unstable(feature = "allocator_api", issue = "32838")] #[unstable(feature = "allocator_api", issue = "32838")]
#[derive(Copy, Clone, Default, Debug)] #[derive(Copy, Clone, Default, Debug)]
#[cfg(not(test))] #[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; pub struct Global;
#[cfg(test)] #[cfg(test)]

View File

@ -2062,6 +2062,9 @@ extern "rust-call" fn async_call(&self, args: Args) -> Self::CallFuture<'_> {
#[unstable(feature = "coerce_unsized", issue = "18598")] #[unstable(feature = "coerce_unsized", issue = "18598")]
impl<T: ?Sized + Unsize<U>, U: ?Sized, A: Allocator> CoerceUnsized<Box<U, A>> for Box<T, A> {} 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")] #[unstable(feature = "dispatch_from_dyn", issue = "none")]
impl<T: ?Sized + Unsize<U>, U: ?Sized> DispatchFromDyn<Box<U>> for Box<T, Global> {} impl<T: ?Sized + Unsize<U>, U: ?Sized> DispatchFromDyn<Box<U>> for Box<T, Global> {}

View File

@ -2,6 +2,7 @@
#![crate_type = "lib"] #![crate_type = "lib"]
#![feature(dyn_star)] #![feature(dyn_star)]
#![feature(generic_nonzero)] #![feature(generic_nonzero)]
#![feature(allocator_api)]
use std::mem::MaybeUninit; use std::mem::MaybeUninit;
use std::num::NonZero; use std::num::NonZero;
@ -182,6 +183,15 @@ pub fn _box(x: Box<i32>) -> Box<i32> {
x 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) // CHECK: noundef nonnull align 4 ptr @notunpin_box(ptr noundef nonnull align 4 %x)
#[no_mangle] #[no_mangle]
pub fn notunpin_box(x: Box<NotUnpin>) -> Box<NotUnpin> { pub fn notunpin_box(x: Box<NotUnpin>) -> Box<NotUnpin> {

View File

@ -160,6 +160,7 @@ pub struct Unique<T: ?Sized> {
pub _marker: PhantomData<T>, pub _marker: PhantomData<T>,
} }
#[lang = "global_alloc_ty"]
pub struct Global; pub struct Global;
#[lang = "owned_box"] #[lang = "owned_box"]