Enforce that layout size fits in isize in Layout

[CAD97]

As it turns out, enforcing this in APIs that already enforce usize overflow is fairly trivial. Layout::from_size_align_unchecked continues to "allow" sizes which (when rounded up) would overflow isize, but these are now declared as library UB for Layout, meaning that consumers of Layout no longer have to check this before making an allocation.

(Note that this is "immediate library UB;" IOW it is valid for a future release to make this immediate "language UB," and there is an extant patch to do so, to allow Miri to catch this misuse.)

See also #95252, Zulip discussion.
Fixes #95334

Some relevant quotes:

@eddyb, #95252 (comment)

[B]ecause of the non-trivial presence of both of these among code published on e.g. crates.io:

1. Layout "producers" / GlobalAlloc "users": smart pointers (including alloc::rc copies with small tweaks), collections, etc.
2. Layout "consumers" / GlobalAlloc "providers": perhaps fewer of these, but anything built on top of OS APIs like mmap will expose > isize::MAX allocations (on 32-bit hosts) if they lack extra checks

IMO the only responsible option is to enforce the isize::MAX limit in Layout, which:

• makes Layout sound in terms of only ever allowing allocations where (alloc_base_ptr: *mut u8).offset(size) is never UB
• frees both "producers" and "consumers" of Layout from manually reimplementing the checks
  • manual checks can be risky, e.g. if the final size passed to the allocator isn't the one being checked
  • this applies retroactively, fixing the overall soundness of existing code with zero transition period or any changes required from users (as long as going through Layout is mandatory, making a "choke point")

Feel free to quote this comment onto any relevant issue, I might not be able to keep track of developments.

@Gankra, #95252 (comment)

As someone who spent way too much time optimizing libcollections checks for this stuff and tried to splatter docs about it everywhere on the belief that it was a reasonable thing for people to manually take care of: I concede the point, it is not reasonable. I am wholy spiritually defeated by the fact that liballoc of all places is getting this stuff wrong. This isn't throwing shade at the folks who implemented these Rc features, but rather a statement of how impractical it is to expect anyone out in the wider ecosystem to enforce them if some of the most audited rust code in the library that defines the very notion of allocating memory can't even reliably do it.

We need the nuclear option of Layout enforcing this rule. Code that breaks this rule is deeply broken and any "regressions" from changing Layout's contract is a correctness fix. Anyone who disagrees and is sufficiently motivated can go around our backs but the standard library should 100% refuse to enable them.

cc also @RalfJung @rust-lang/wg-allocators. Even though this technically supersedes #95252, those potential failure points should almost certainly still get nicer panics than just "unwrap failed" (which they would get by this PR).

It might additionally be worth recommending to users of the Layout API that they should ideally use .and_then/? to complete the entire layout calculation, and then panic! from a single location at the end of Layout manipulation, to reduce the overhead of the checks and optimizations preserving the exact location of each panic which are conceptually just one failure: allocation too big.

Probably deserves a T-lang and/or T-libs-api FCP (this technically solidifies the objects must be no larger than isize::MAX rule further, and the UCG document says this hasn't been RFCd) and a crater run. Ideally, no code exists that will start failing with this addition; if it does, it was likely (but not certainly) causing UB.

Changes the raw_vec allocation path, thus deserves a perf run as well.

I suggest hiding whitespace-only changes in the diff view.

[rust-highfive]

r? @joshtriplett

(rust-highfive has picked a reviewer for you, use r? to override)

[CAD97]

Let's see if I remember how to do this...

@rustbot label: +A-allocators +A-layout +needs-fcp +T-lang +T-libs-api

(I think those are all relevant, feel free to fix up)

[eddyb]

Ideally, no code exists that will start failing with this addition;

Sadly, I don't think we can spot this on crater, isn't it x86_64-only?
(I first suspected something like this on 32-bit targets, but I could never come up with an exploit)

[CAD97]

So... we currently don't enforce the isize::MAX limit in raw_vec for 64+ bit platforms, and instead just rely on the platform allocator failing:

rust/library/alloc/tests/string.rs

Lines 696 to 699 in 661e8be

	// On 16/32-bit, we check that allocations don't exceed isize::MAX,
	// on 64-bit, we assume the OS will give an OOM for such a ridiculous size.
	// Any platform that succeeds for these requests is technically broken with
	// ptr::offset because LLVM is the worst.

rust/library/alloc/src/raw_vec.rs

Lines 494 to 510 in 661e8be

	// We need to guarantee the following:
	// * We don't ever allocate `> isize::MAX` byte-size objects.
	// * We don't overflow `usize::MAX` and actually allocate too little.
	//
	// On 64-bit we just need to check for overflow since trying to allocate
	// `> isize::MAX` bytes will surely fail. On 32-bit and 16-bit we need to add
	// an extra guard for this in case we're running on a platform which can use
	// all 4GB in user-space, e.g., PAE or x32.
	#[inline]
	fn alloc_guard(alloc_size: usize) -> Result<(), TryReserveError> {
	if usize::BITS < 64 && alloc_size > isize::MAX as usize {
	Err(CapacityOverflow.into())
	} else {
	Ok(())
	}
	}

This PR would replace alloc_guard by having already caught > isize::MAX sizes (on all platforms) when Layout::array::<T>() was called (in the generic path)... I think. I'm unsure of the exact behavior reading the raw_alloc code right now.

[CAD97]

This changes the allocation path in raw_vec, so is thus perf-sensitive. Notably, alloc_guard skipped the isize::MAX check on 64+ bit platforms (see above), and this is now not only done, but done in the generic path.

@bors rollup=never

[bors]

@CAD97: 🔑 Insufficient privileges: not in try users

[CAD97]

I'm done tweaking this for the time being, but if this does turn out to be a perf impact, this can possibly be improved by changing

let new_layout = Layout::array::<T>(cap);
finish_grow(new_layout, self.current_memory(), &mut self.alloc);

to

let t_layout = Layout::<T>::new();
finish_grow(t_layout, cap, self.current_memory(), &mut self.alloc);

and doing Layout::repeat inside finish_grow, to move all of the dynamic alloc size checks inside the non-generic path.

[CAD97]

(Also noting this here, not just in the Zulip: GlobalAlloc::realloc and alloc::realloc do not put an isize maximum on new_size. All other allocation APIs go through Layout and inherit it's requirements. I also don't see an explicit requirement that sizes are padded to alignment anywhere.)

[eddyb]

Noticed this comment in the snippets above:

// ... On 32-bit and 16-bit we need to add 
// an extra guard for this in case we're running on a platform which can use 
// all 4GB in user-space, e.g., PAE or x32.

AFAIK this is a bit off, these are the distinctions I'm aware of:

• x86_32 kernel
  • x86_32 userspace
    • Linux: has a 3GiB/1GiB userspace/kernel split, IIUC even without PAE
    • Windows: has either 2GiB/2GiB or 3GiB/1GiB
  • PAE: increases the amount of physical memory the whole system can address
    • without it, there is less than 4GiB of total RAM (as some physical addresses are MMIO)
    • maybe some kernels do not give userspace more than 2GiB unless PAE is available? not sure
• x86_64 kernel
  • x86_32 userspace
    • Linux: userspace gets nearly 4GiB, AFAICT (easier to see with musl than glibc)
      • experimentally, with musl, you only lose ~256MiB:
        • lowest ~128MiB: static executable mapped at 0x08000000 (can probably lower that?)
        • highest ~128MiB: vDSO and the main thread's stack are mapped there
      • with fragmentation you can use more of the gaps, but a contiguous allocation is going to cap out at around ~3.75GiB (and closer to ~2.5GiB for glibc but that's mostly the ASLR worsening fragmentation AFAICT, so it's more of a gamble)
    • Windows: according to the 4GT ("4GiB Tuning") docs:

      On 64-bit editions of Windows, 32-bit applications marked with the IMAGE_FILE_LARGE_ADDRESS_AWARE flag have 4 GB of address space available.

  • x86_64 userspace
    • by default, you get 64-bit address space
      • however, for a long time the hardware was limited to 48-bit, and some (most?) kernels split that into two halves, leaving 47-bit for userspace
    • "x32" is a Linux x86_64 ABI that uses the bottom 4GiB of the 64-bit address space, relying on x86 addressing modes to enjoy the smaller pointers
      ^{(I wonder if you could go as low as 16-bit addresses while still having 64-bit registers, heh)}
      • compilers support this as an ABI with 32-bit pointers "by default" (think "near" pointers)
      • Linux x86_64 syscalls have a bit (in the syscall number) indicating whether the kernel should account for x32 variations, like smaller struct sizes/fields offsets, not returning pointers above 4GiB (e.g. from mmap), etc.
      • x86_64 syscalls and 64-bit pointers can be used simultaneously with x32 if enough care is taken (either through FFI, or a compiler that allows specifying "far" pointers in x32 mode)

So AFAICT, PAE isn't really relevant and x32 is more of a "pointer compression trick" than a "fundamentally" 32-bit userspace.
^{(efficiency and lack of OS support are the only things stopping someone from making up a pseudo-Harvard ABI, on any platform, with smaller _{(even as small as 8-bit)} data pointers, that only need to address statics, maybe some heap, and the stack)}

Taking into account that "can use all 4GB" should be "can use more than 2GiB", I think we can just say that:

• 32-bit (userspace) programs would mainly be kept away from 2GiB by available memory in the system
• 64-bit (userspace) programs, on the other hand, will likely never have more than 63 bits for each of the kernel and userspace, even if 64-bit virtual addresses finally get implemented in hardware (because that's the most convenient way to split the two, AFAIK)

That leaves bare-metal/kernel 64-bit code on some future CPU (that could still be targeted just fine by an existing Rust version!), unaccounted for. And probably only bare-metal without an userspace, because you wouldn't have an allocation crossing the kernel/userspace split even if you could see the entire 64-bit address space at once.

I guess that could justify skipping the checks in those cases, but I would feel more comfortable with something like cfg!(any(unix, windows)) to rule out long-term-plausible weird configurations.

[Gankra]

fwiw all the fiddly shit in RawVec is largely an aesthetic vanity project of my own making because the entire premise of saving an add/comparison is ridiculous when entering RawVec's code implies you are about to do a dynamic call to the allocator and do O(n) work.

(Hot paths are largely guarded by if self.len == self.cap { in Vec itself.)

[RalfJung]

@bors rollup=never

[RalfJung]

Relatedly, we also have wording like this in our offset methods:

Most platforms fundamentally can’t even construct such an allocation [meaning one that exceeds isize::MAX]. For instance, no known 64-bit platform can ever serve a request for 2^63 bytes due to page-table limitations or splitting the address space. However, some 32-bit and 16-bit platforms may successfully serve a request for more than isize::MAX bytes with things like Physical Address Extension. As such, memory acquired directly from allocators or memory mapped files may be too large to handle with this function.

Not sure if that will be affected by this change though. Does LLVM consider it okay to (somehow) have a larger-than-isize::MAX allocation and use wrapping_offset to move more than isize::MAX inside it?

[Gankra]

C has a rule that you can't stride such an allocation in one step, and instead have to do it in two steps. Presumably llvm supports this semantic. As I understand it, there was also vaguely a period where C tried to insist that there should actually be a 1-bit difference between some of the integer types to accommodate the issue with signed offsets, which was completely wild and no implementation actually did.

Long ago before even like 1.0 we all agreed "that's all a fucking nightmare" (and I seriously doubt almost any C/C++ code actually supports detecting and handling any of that) and concluded that the only sound solution (in the sense that all code wouldn't have subtle UB) was to mandate everything be restricted to isize::MAX. I believe modern versions of glibc and jemalloc also actually just silently enforce this because again, anything else is completely broken.

(I am told that gcc also hardcodes this limitation, and it's not just an llvm thing)

[RalfJung]

(I am told that gcc also hardcodes this limitation, and it's not just an llvm thing)

That is very surprising. To me the entire thing looks wholly unnecessary and motivated entirely by the fact that LLVM's getelementptr is a really odd API that interprets the total effective offset as signed. If LLVM had proper unsigned offsetting, and if we used that for field and array accesses and so on, there would be no issue. (Negative offsets are exceedingly rare and could be delegated to special methods.)

Why does GCC have a similar limitation?

[Gankra]

Because it's a C limitation and they're both riddled with C semantics.

[tavianator]

Here's some pointers for GCC:

• https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63303#c5
• https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67999#c5

[Gankra]

So yeah basically this is a defacto restriction that all programming languages must conform to (if only for C interop), and which doesn't actually matter on 64-bit because it's unobservable. Anyone who wants to Be Clever on 32/16-bit gets to reach for mmap and go around the language's back and be very careful / hope they don't get miscompiled.

[RalfJung]

Wow computers truly are terrible. :'-(

[RalfJung]

GlobalAlloc::realloc and alloc_realloc do not put an isize maximum on new_size

So what's the plan for those two? Impose a requirement on GlobalAlloc implementations that reallocation MUST fail if new_size > isize::MAX? It is an unsafe trait, so implementation requirements are possible, but this is technically a breaking change.

[Lokathor]

It's technically breaking, but it's also for soundness.