Skip to content

Commit f391c07

Browse files
RalfJungRalfJung
committed
only set noalias on Box with the global allocator
1 parent 5a1e544 commit f391c07

File tree

17 files changed

+75
-28
lines changed

17 files changed

+75
-28
lines changed

‎compiler/rustc_abi/src/lib.rs

+5-2
Original file line numberDiff line numberDiff line change
@@ -1612,8 +1612,9 @@ pub enum PointerKind {
16121612
SharedRef { frozen: bool },
16131613
/// Mutable reference. `unpin` indicates the absence of any pinned data.
16141614
MutableRef { unpin: bool },
1615-
/// Box. `unpin` indicates the absence of any pinned data.
1616-
Box { unpin: bool },
1615+
/// Box. `unpin` indicates the absence of any pinned data. `global` indicates whether this box
1616+
/// uses the global allocator or a custom one.
1617+
Box { unpin: bool, global: bool },
16171618
}
16181619

16191620
/// Note that this information is advisory only, and backends are free to ignore it.
@@ -1622,6 +1623,8 @@ pub enum PointerKind {
16221623
pub struct PointeeInfo {
16231624
pub size: Size,
16241625
pub align: Align,
1626+
/// If this is `None`, then this is a raw pointer, so size and alignment are not guaranteed to
1627+
/// be reliable.
16251628
pub safe: Option<PointerKind>,
16261629
}
16271630

‎compiler/rustc_codegen_cranelift/example/mini_core.rs

+5-2
Original file line numberDiff line numberDiff line change
@@ -525,8 +525,11 @@ pub struct Unique<T: ?Sized> {
525525
impl<T: ?Sized, U: ?Sized> CoerceUnsized<Unique<U>> for Unique<T> where T: Unsize<U> {}
526526
impl<T: ?Sized, U: ?Sized> DispatchFromDyn<Unique<U>> for Unique<T> where T: Unsize<U> {}
527527

528+
#[lang = "global_alloc_ty"]
529+
pub struct Global;
530+
528531
#[lang = "owned_box"]
529-
pub struct Box<T: ?Sized, A = ()>(Unique<T>, A);
532+
pub struct Box<T: ?Sized, A = Global>(Unique<T>, A);
530533

531534
impl<T: ?Sized + Unsize<U>, U: ?Sized> CoerceUnsized<Box<U>> for Box<T> {}
532535

@@ -536,7 +539,7 @@ impl<T> Box<T> {
536539
let size = intrinsics::size_of::<T>();
537540
let ptr = libc::malloc(size);
538541
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)
540543
}
541544
}
542545
}

‎compiler/rustc_codegen_cranelift/src/unsize.rs

-4
Original file line numberDiff line numberDiff line change
@@ -74,10 +74,6 @@ fn unsize_ptr<'tcx>(
7474
| (&ty::RawPtr(ty::TypeAndMut { ty: a, .. }), &ty::RawPtr(ty::TypeAndMut { ty: b, .. })) => {
7575
(src, unsized_info(fx, *a, *b, old_info))
7676
}
77-
(&ty::Adt(def_a, _), &ty::Adt(def_b, _)) if def_a.is_box() && def_b.is_box() => {
78-
let (a, b) = (src_layout.ty.boxed_ty(), dst_layout.ty.boxed_ty());
79-
(src, unsized_info(fx, a, b, old_info))
80-
}
8177
(&ty::Adt(def_a, _), &ty::Adt(def_b, _)) => {
8278
assert_eq!(def_a, def_b);
8379

‎compiler/rustc_codegen_gcc/example/mini_core.rs

+1
Original file line numberDiff line numberDiff line change
@@ -472,6 +472,7 @@ pub trait Allocator {
472472

473473
impl Allocator for () {}
474474

475+
#[lang = "global_alloc_ty"]
475476
pub struct Global;
476477

477478
impl Allocator for Global {}

‎compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs

+7-3
Original file line numberDiff line numberDiff line change
@@ -454,9 +454,13 @@ pub fn type_di_node<'ll, 'tcx>(cx: &CodegenCx<'ll, 'tcx>, t: Ty<'tcx>) -> &'ll D
454454
ty::RawPtr(ty::TypeAndMut { ty: pointee_type, .. }) | ty::Ref(_, pointee_type, _) => {
455455
build_pointer_or_reference_di_node(cx, t, pointee_type, unique_type_id)
456456
}
457-
// Box<T, A> may have a non-1-ZST allocator A. In that case, we
458-
// cannot treat Box<T, A> as just an owned alias of `*mut T`.
459-
ty::Adt(def, args) if def.is_box() && cx.layout_of(args.type_at(1)).is_1zst() => {
457+
// Some `Box` are newtyped pointers, make debuginfo aware of that.
458+
// Only works if the allocator argument is a 1-ZST and hence irrelevant for layout
459+
// (or if there is no allocator argument).
460+
ty::Adt(def, args)
461+
if def.is_box()
462+
&& args.get(1).map_or(true, |arg| cx.layout_of(arg.expect_ty()).is_1zst()) =>
463+
{
460464
build_pointer_or_reference_di_node(cx, t, t.boxed_ty(), unique_type_id)
461465
}
462466
ty::FnDef(..) | ty::FnPtr(_) => build_subroutine_type_di_node(cx, unique_type_id),

‎compiler/rustc_codegen_ssa/src/mir/operand.rs

+1
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,7 @@ impl<'a, 'tcx, V: CodegenObject> OperandRef<'tcx, V> {
204204

205205
pub fn deref<Cx: LayoutTypeMethods<'tcx>>(self, cx: &Cx) -> PlaceRef<'tcx, V> {
206206
if self.layout.ty.is_box() {
207+
// Derefer should have removed all Box derefs
207208
bug!("dereferencing {:?} in codegen", self.layout.ty);
208209
}
209210

‎compiler/rustc_const_eval/src/interpret/place.rs

+1
Original file line numberDiff line numberDiff line change
@@ -437,6 +437,7 @@ where
437437
trace!("deref to {} on {:?}", val.layout.ty, *val);
438438

439439
if val.layout.ty.is_box() {
440+
// Derefer should have removed all Box derefs
440441
bug!("dereferencing {}", val.layout.ty);
441442
}
442443

‎compiler/rustc_const_eval/src/interpret/terminator.rs

+2-8
Original file line numberDiff line numberDiff line change
@@ -359,14 +359,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
359359
Ok(Some(match ty.kind() {
360360
ty::Ref(_, ty, _) => *ty,
361361
ty::RawPtr(mt) => mt.ty,
362-
// We should only accept `Box` with the default allocator.
363-
// It's hard to test for that though so we accept every 1-ZST allocator.
364-
ty::Adt(def, args)
365-
if def.is_box()
366-
&& self.layout_of(args[1].expect_ty()).is_ok_and(|l| l.is_1zst()) =>
367-
{
368-
args[0].expect_ty()
369-
}
362+
// We only accept `Box` with the default allocator.
363+
_ if ty.is_box_global(*self.tcx) => ty.boxed_ty(),
370364
_ => return Ok(None),
371365
}))
372366
};

‎compiler/rustc_hir/src/lang_items.rs

+2
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,8 @@ language_item_table! {
267267
EhCatchTypeinfo, sym::eh_catch_typeinfo, eh_catch_typeinfo, Target::Static, GenericRequirement::None;
268268

269269
OwnedBox, sym::owned_box, owned_box, Target::Struct, GenericRequirement::Minimum(1);
270+
GlobalAlloc, sym::global_alloc_ty, global_alloc_ty, Target::Struct, GenericRequirement::None;
271+
270272
// Experimental language item for Miri
271273
PtrUnique, sym::ptr_unique, ptr_unique, Target::Struct, GenericRequirement::Exact(1);
272274

‎compiler/rustc_middle/src/ty/layout.rs

+12-8
Original file line numberDiff line numberDiff line change
@@ -969,6 +969,8 @@ where
969969
}
970970
}
971971

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.
972974
fn ty_and_layout_pointee_info_at(
973975
this: TyAndLayout<'tcx>,
974976
cx: &C,
@@ -1068,15 +1070,17 @@ where
10681070
}
10691071
}
10701072

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`.
10721076
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+
});
10801084
}
10811085
}
10821086

‎compiler/rustc_middle/src/ty/sty.rs

+21
Original file line numberDiff line numberDiff line change
@@ -1999,6 +1999,27 @@ impl<'tcx> Ty<'tcx> {
19991999
}
20002000
}
20012001

2002+
/// Tests whether this is a Box using the global allocator.
2003+
#[inline]
2004+
pub fn is_box_global(self, tcx: TyCtxt<'tcx>) -> bool {
2005+
match self.kind() {
2006+
Adt(def, args) if def.is_box() => {
2007+
let Some(alloc) = args.get(1) else {
2008+
// Single-argument Box is always global. (for "minicore" tests)
2009+
return true;
2010+
};
2011+
if let Some(alloc_adt) = alloc.expect_ty().ty_adt_def() {
2012+
let global_alloc = tcx.require_lang_item(LangItem::GlobalAlloc, None);
2013+
alloc_adt.did() == global_alloc
2014+
} else {
2015+
// Allocator is not an ADT...
2016+
false
2017+
}
2018+
}
2019+
_ => false,
2020+
}
2021+
}
2022+
20022023
/// Panics if called on any type other than `Box<T>`.
20032024
pub fn boxed_ty(self) -> Ty<'tcx> {
20042025
match self.kind() {

‎compiler/rustc_span/src/symbol.rs

+1
Original file line numberDiff line numberDiff line change
@@ -896,6 +896,7 @@ symbols! {
896896
generic_const_items,
897897
generic_param_attrs,
898898
get_context,
899+
global_alloc_ty,
899900
global_allocator,
900901
global_asm,
901902
globs,

‎compiler/rustc_ty_utils/src/abi.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -452,7 +452,7 @@ fn adjust_for_rust_scalar<'tcx>(
452452
let no_alias = match kind {
453453
PointerKind::SharedRef { frozen } => frozen,
454454
PointerKind::MutableRef { unpin } => unpin && noalias_mut_ref,
455-
PointerKind::Box { unpin } => unpin && noalias_for_box,
455+
PointerKind::Box { unpin, global } => unpin && global && noalias_for_box,
456456
};
457457
// We can never add `noalias` in return position; that LLVM attribute has some very surprising semantics
458458
// (see <https://github.com/rust-lang/unsafe-code-guidelines/issues/385#issuecomment-1368055745>).

‎library/alloc/src/alloc.rs

+2
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,8 @@ extern "Rust" {
5050
#[unstable(feature = "allocator_api", issue = "32838")]
5151
#[derive(Copy, Clone, Default, Debug)]
5252
#[cfg(not(test))]
53+
// the compiler needs to know when a Box uses the global allocator vs a custom one
54+
#[cfg_attr(not(bootstrap), lang = "global_alloc_ty")]
5355
pub struct Global;
5456

5557
#[cfg(test)]

‎library/alloc/src/boxed.rs

+3
Original file line numberDiff line numberDiff line change
@@ -2062,6 +2062,9 @@ impl<Args: Tuple, F: AsyncFn<Args> + ?Sized, A: Allocator> AsyncFn<Args> for Box
20622062
#[unstable(feature = "coerce_unsized", issue = "18598")]
20632063
impl<T: ?Sized + Unsize<U>, U: ?Sized, A: Allocator> CoerceUnsized<Box<U, A>> for Box<T, A> {}
20642064

2065+
// It is quite crucial that we only allow the `Global` allocator here.
2066+
// Handling arbitrary custom allocators (which can affect the `Box` layout heavily!)
2067+
// would need a lot of codegen and interpreter adjustments.
20652068
#[unstable(feature = "dispatch_from_dyn", issue = "none")]
20662069
impl<T: ?Sized + Unsize<U>, U: ?Sized> DispatchFromDyn<Box<U>> for Box<T, Global> {}
20672070

‎tests/codegen/function-arguments.rs

+10
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
#![crate_type = "lib"]
33
#![feature(dyn_star)]
44
#![feature(generic_nonzero)]
5+
#![feature(allocator_api)]
56

67
use std::mem::MaybeUninit;
78
use std::num::NonZero;
@@ -182,6 +183,15 @@ pub fn _box(x: Box<i32>) -> Box<i32> {
182183
x
183184
}
184185

186+
// With a custom allocator, it should *not* have `noalias`. (See
187+
// <https://github.com/rust-lang/miri/issues/3341> for why.) The second argument is the allocator,
188+
// which is a reference here that still carries `noalias` as usual.
189+
// CHECK: @_box_custom(ptr noundef nonnull align 4 %x.0, ptr noalias noundef nonnull readonly align 1 %x.1)
190+
#[no_mangle]
191+
pub fn _box_custom(x: Box<i32, &std::alloc::Global>) {
192+
drop(x)
193+
}
194+
185195
// CHECK: noundef nonnull align 4 ptr @notunpin_box(ptr noundef nonnull align 4 %x)
186196
#[no_mangle]
187197
pub fn notunpin_box(x: Box<NotUnpin>) -> Box<NotUnpin> {

‎tests/ui/abi/compatibility.rs

+1
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,7 @@ mod prelude {
160160
pub _marker: PhantomData<T>,
161161
}
162162

163+
#[lang = "global_alloc_ty"]
163164
pub struct Global;
164165

165166
#[lang = "owned_box"]

0 commit comments

Comments
 (0)