only set noalias on Box with the global allocator

[RalfJung]

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.

[rustbot]

r? @estebank

rustbot has assigned @estebank.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[rustbot]

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

[RalfJung]

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

[oli-obk]

I don't think we should do this. This makes box more special, when we're trying to make it less special. The FIXME you removed was specifically for making box less special and instead making Unique be the signal for noalias.

[RalfJung]

I don't think we should do this. This makes box more special, when we're trying to make it less special.

It doesn't make Box any more special than it already is. It works around the fact that Box is both a lang primitive and a libs type, causing a big mess. I didn't create that mess, I am trying to clean it up. This PR is part of the experimentation with the nightly feature that is custom allocators.

We could still have hacks to move the semantics to Unique in the future, e.g. making it Unique<T, A> with the semantics of "it's a unique pointer if A = Global but not otherwise". But without such hacks, Unique is just fundamentally incompatible with custom allocators.

If you have another idea for how to fix rust-lang/miri#3341, I'd like to hear it. :)

The FIXME you removed was specifically for making box less special and instead making Unique be the signal for noalias.

That FIXME is many, many years old and nobody even made an attempt at implementing it. I don't think a random FIXME somewhere in the sources is the right way to track such a fundamental aliasing model question. We have issues tracking that question (rust-lang/unsafe-code-guidelines#384, rust-lang/unsafe-code-guidelines#326).

[rustbot]

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo, @GuillaumeGomez

[RalfJung]

Why do cranelift and gcc have their own copies of almost but not quite the same file. :(

[RalfJung]

Maybe we should make the pointer field type an assoc type on allocator, then allocators can choose if they want unique or raw pointer behaviour

That's an interesting alternative, but a lot more work and changes the trait. Might be worth bringing up in rust-lang/wg-allocators#122.

But given that the general vibe is slowly moving towards just not having Box be noalias ever, it's unclear whether we really want custom allocators to have the option of being unique pointers.

[eddyb]

But given that the general vibe is slowly moving towards just not having Box be noalias ever

^{(extremely-out-of-the-loop voice)} Is the alternative specifically having all the relevant annotations on allocator/deallocator functions, so e.g. arbitrary -> Box<T> user functions can only get the same annotations as today when the result is guaranteed to come from a call to one of the explicitly annotated allocator functions? _{(and no other copies of that pointer could have escaped - I suppose that's one place where Capstone's "linear capabilities" can express more in hardware than CHERI, but I digress)}

If that (or at least something in that general direction) is what's being considered, I can't think of anything I could have against it (not that it matters, I'm only here because @oli-obk pointed out something about ancient FIXMEs and I wanted to rule it out quickly).

In other words: if any of my old FIXMEs/plans rely on the type being special, and the move is to specific functions being special instead, feel free to throw the old stuff out. And debuginfo is even less critical, as long as any regressions are tracked, and the most common cases don't completely break (like how you kept a special case for Box<T, Global>).

[RalfJung]

The alternative is to just not annotate Box arguments with noalias. (We already don't annotate return Box values with noalias any more as that was unsound.) I don't see how the functions being special relates to this question. Some functions need to be special as well to justify the return-position noalias on __rust_allocate but that's an entirely different discussion.

[oli-obk]

The alternative is to just not annotate Box arguments with noalias

Does that have performance issues or other issues? That seems like it would be the most straight forward impl

[RalfJung]

That seems like it would be the most straight forward impl

There's a lot of baggage here. Like, years of discussions. ;)
Doing this needs some sort of process involving T-lang I think, to achieve consensus that Box<T> does not have aliasing restrictions.

Meanwhile I just want to fix the problem with custom allocators that Miri found, without affecting stable.

[Noratrieb]

Just removing it doesn't affect stable either as long as we still pretend that it does have that attribute. At worst it may make stable code slightly slower but that seems unlikely to me, and in fact potential performance complaints (or the absence of such) might be useful data for the discussion!

[RalfJung]

It affects the code generated on stable. My aim with this PR is not to do that.

Feel free to file a PR that removes the attribute entirely. :)

[oli-obk]

@bors r+

yea, let's leave the noalias on default box discussion for another time

[bors]

📌 Commit f391c07 has been approved by oli-obk

It is now in the queue for this repository.

[matthewpipie]

I hate to reawaken a dead thread, but I'm using custom allocators and ran into the following comment added in this PR:

// 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> {}

Could somebody help enlighten me as to why dynamic dispatch is impossible from a Box<T, CustomAlloc>? I would have thought that the source of the memory for the Box shouldn't matter.
It is quite tough as I am converting a bunch of Box<T> to Box<T, CustomAlloc> and my trait objects no longer compile.
For instance, I would like to have the code

trait Foo {
    fn foo(self: Box<Self, CustomAlloc>;
}

but then a Box<dyn Foo> only compiles when CustomAlloc = Global.

Edit: DispatchFromDyn is also not implemented for Rc<T, CustomAlloc> or Arc<T, CustomAlloc>. I'm wondering if that has the same limitations as Box or not.

[RalfJung]

Please ask that on Zulip or IRLO; it's not even related to noalias or this PR -- the PR just clarifies what was already an inherent part of the design here.