Document that heap allocations are not guaranteed to happen, even if explicitly performed in the code

[oli-obk]

cc @RalfJung

[rust-highfive]

r? @shepmaster

(rust_highfive has picked a reviewer for you, use r? to override)

[RalfJung]

Any idea how to ping the "allocators" WG? https://github.com/rust-lang/wg-allocators/

[oli-obk]

cc @TimDiekmann you have no pingable team ^^

[RalfJung]

If the allocator has an API to query the number of allocs, I think the compiler is not allowed to bypass this, as calling alloc has a side effect (e.g. incrementing a counter in the allocator).

Note that this makes it impossible to ever support the "normal" allocator during CTFE. After all, allocations that occur during CTFE will never hit the real allocator. Now imagine something like this:

// Type invariant: constructing an element of this type will cause a heap allocation
pub struct Evil(());

impl Evil {
  pub const fn new() {
    drop(Box::new(0));
    Evil
  }

  pub check(&self) {
    let number_of_heap_allocs = /* call private allocator API */;
    if number_of_heap_allocs == 0 { unsafe { unreachable_unchecked() }}
  }
}

It would not be hard to permit CTFE to perform "transient" allocations as long as the final computed value does not depend on them -- something like Evil::new. But this is only possible if the compiler is allowed to drop calls to the allocation function.

So, if the compiler has to preserve the exact number of calls to Global::allocator, then heap allocation during CTFE will always have to use a sepcial compile-time allocator -- it is impossible to use the same code for compile-time and run-time allocation.

[TimDiekmann]

As the allocator-wg isn't pingable, I'll mention the same people I link when doing a FCP in the WG:

cc @Amanieu @Lokathor @Wodann @CAD97 @scottjmaddox @vertexclique

[CAD97]

This mostly matches my prior understanding, so I have no objections.

A potential alternative is to only allow allocation optimization when using the global allocator (so e.g. Global.dealloc(Global.alloc(...)) would optimize out, but Geiger.dealloc(Geiger.alloc(...)) wouldn't), but I don't think that's necessary, and would unnecessarily pessimize use of local allocators.

[TimDiekmann]

A potential alternative is to only allow allocation optimization when using the global allocator. [...] I don't think that's necessary, and would unnecessarily pessimize use of local allocators.

This would also lead to confusion and unexpected performance regressions I guess. This would also introduce corner cases. Does Box<T, Global> behave like Box<T>? If Global == System, does Box<T> behave like Box<T, System>?

[RalfJung]

A potential alternative is to only allow allocation optimization when using the global allocator

This would mean there could be no blanket impl of GlobalAlloc in terms of AllocRef.

(However, what you describe actually matches the current implementation. But I'd call that an implementation artifact.)

[shepmaster]

r? @RalfJung

[RalfJung]

Since I am the one who proposed this remark to @oli-obk, I think it'd be better if someone from WG-allocators could do the reviewing.

r? @TimDiekmann

Also there are some open discussions, so I do not think this is ready for merging quite yet.

[TimDiekmann]

As this behavior already occurs in release builds, this should definitely be added to #[global_allocator]. I guess, it's also possible to occur with AllocRef but the example for AllocRef::alloc may needs to be adjusted, as it does not use AllocRef.

[TimDiekmann]

@rustbot modify labels: -S-waiting-on-review +S-waiting-on-author

[oli-obk]

@bors r=TimDiekmann rollup

[bors]

📌 Commit 58d62b8 has been approved by TimDiekmann

[RalfJung]

I think this should be tweaked some more -- see my feedback above.
@bors r-

[iximeow]

in the case of a program like

use std::alloc::Layout;
use std::alloc::{alloc, dealloc};

fn main() {
    unsafe {
        let layout = Layout::from_size_align(
            0xffff_ffff_ffff_ffff, // big data
            1
        ).unwrap();
        let ptr = alloc(layout);
        if ptr.is_null() {
            panic!("allocation failed");
        }
        dealloc(ptr, layout);
    };
}

is removing this heap allocation (and ptr.is_null() becoming a constant false) considered changing program behavior? the truthiness of ptr.is_null() is not a consequence of the allocation happening, but the allocator implementation.

i'm not sure if this paragraph should be widened to say that in some circumstances program behavior can change, or the above main becoming a no-op is a rustc bug.

[CAD97]

Given that 99.9% of allocation is done via ptr::NonNull::new(alloc(layout)).unwrap_or_else(|| handle_alloc_error(layout)) (Box does nearly exactly this), if the allocation removal optimization is ever allowed to apply, it needs to be able to optimize the allocation out even when doing an arbitrary -> ! on allocation failure (as handle_alloc_error calls the dynamic alloc error hook).

[CAD97]

I think the wording to apply here would be to say (somehow) that allocation removal optimization happens assuming that allocation is infallible, or iow that an optimized out allocation always succeeds, or iow that "without changing program behavior when allocation succeeds" or similar.

[RalfJung]

is removing this heap allocation (and ptr.is_null() becoming a constant false) considered changing program behavior?

I wish I knew the answer.^^ In practice, LLVM will optimize away even such huge allocations that can never succeed. So the LLVM devs consider this to not be changing program behavior.

However, I know of no formal model that would actually explain this, and it might even be inconsistent. Somehow the formal model needs to say that the allocation can succeed even if it is too big to fit into the finite memory, and then proceed in a consistent way even if the program sanity-checks the integer addresses that it got (since those checks can be optimized away if their result is unused) and so on.

I consider this to be one of the hardest unanswered questions in formal semantics for low-level languages: to either show that this makes sense, or to show that the optimization is broken and leads to miscompilations.

[Amanieu]

I think that this should only apply to the global allocator since AllocRef may be used to allocate "special" memory (e.g. GPU memory) which cannot be replaced with just a stack allocation.

Specifically these semantics should be on the #[global_allocator] attribute rather than the AllocRef trait, but we don't really have a better place for it in the docs.

[RalfJung]

I think that this should only apply to the global allocator since AllocRef may be used to allocate "special" memory (e.g. GPU memory) which cannot be replaced with just a stack allocation.

Wouldn't that be unsound, since I could use that AllocRef with Box which will assume it to live in the normal address space?

[Amanieu]

Wouldn't that be unsound, since I could use that AllocRef with Box which will assume it to live in the normal address space?

In this context GPU memory would be mapped into the address space of the process and would be accessible just like normal memory.

[RalfJung]

Oh I see.

That would mean rustc must not assign any special semantics to AllocRef::alloc calls, and treat them like any other function call (except if that in turn happens to call the global allocator which is special). This seems reasonable, if we can ensure that it is indeed the case.

[oli-obk]

@bors r=TimDiekmann rollup

[bors]

📌 Commit efcd8a9 has been approved by TimDiekmann

[bors]

⌛ Testing commit efcd8a9 with merge 0b644e4...

[bors]

☀️ Test successful - checks-actions
Approved by: TimDiekmann
Pushing 0b644e4 to master...