Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

only set noalias on Box with the global allocator #122018

Merged
merged 1 commit into from
Mar 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions compiler/rustc_abi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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>,
}

Expand Down
7 changes: 5 additions & 2 deletions compiler/rustc_codegen_cranelift/example/mini_core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {}

Expand All @@ -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)
}
}
}
Expand Down
4 changes: 0 additions & 4 deletions compiler/rustc_codegen_cranelift/src/unsize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
Copy link
Member Author

@RalfJung RalfJung Mar 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bjorn3 the equivalent function in codegen_ssa does not have this Box special case, so I assume it is not needed here either.

(&ty::Adt(def_a, _), &ty::Adt(def_b, _)) => {
assert_eq!(def_a, def_b);

Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_codegen_gcc/example/mini_core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -472,6 +472,7 @@ pub trait Allocator {

impl Allocator for () {}

#[lang = "global_alloc_ty"]
pub struct Global;

impl Allocator for Global {}
Expand Down
10 changes: 7 additions & 3 deletions compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_codegen_ssa/src/mir/operand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_const_eval/src/interpret/place.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
10 changes: 2 additions & 8 deletions compiler/rustc_const_eval/src/interpret/terminator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}))
};
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_hir/src/lang_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
RalfJung marked this conversation as resolved.
Show resolved Hide resolved
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);

Expand Down
20 changes: 12 additions & 8 deletions compiler/rustc_middle/src/ty/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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),
});
}
}

Expand Down
21 changes: 21 additions & 0 deletions compiler/rustc_middle/src/ty/sty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -896,6 +896,7 @@ symbols! {
generic_const_items,
generic_param_attrs,
get_context,
global_alloc_ty,
global_allocator,
global_asm,
globs,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_ty_utils/src/abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>).
Expand Down
2 changes: 2 additions & 0 deletions library/alloc/src/alloc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
3 changes: 3 additions & 0 deletions library/alloc/src/boxed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {}

Expand Down
10 changes: 10 additions & 0 deletions tests/codegen/function-arguments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
#![crate_type = "lib"]
#![feature(dyn_star)]
#![feature(generic_nonzero)]
#![feature(allocator_api)]

use std::mem::MaybeUninit;
use std::num::NonZero;
Expand Down Expand Up @@ -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> {
Expand Down
1 change: 1 addition & 0 deletions tests/ui/abi/compatibility.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ mod prelude {
pub _marker: PhantomData<T>,
}

#[lang = "global_alloc_ty"]
pub struct Global;

#[lang = "owned_box"]
Expand Down
Loading