`Allocator::grow` does not verify all safety conditions of `GlobalAlloc::realloc`

[djkoloski]

Allocator::grow does not verify that the new allocation size, when rounded up to a multiple of align, does not overflow isize::MAX. This can cause it to call realloc with invalid arguments:

#![deny(unsafe_op_in_unsafe_fn, clippy::missing_safety_doc, clippy::undocumented_unsafe_blocks)]
#![feature(allocator_api)]

use std::alloc::{Allocator, Global, GlobalAlloc, Layout, System};

struct MyGlobalAlloc;

unsafe impl GlobalAlloc for MyGlobalAlloc {
    unsafe fn alloc(&self, layout: Layout) -> *mut u8 {
        // SAFETY: The safety requirements for `alloc` match the safety docs for `System.alloc`.
        unsafe { System.alloc(layout) }
    }

    unsafe fn dealloc(&self, ptr: *mut u8, layout: Layout) {
        // SAFETY: The safety requirements for `dealloc` match the safety docs for `System.dealloc`.
        unsafe {
            System.dealloc(ptr, layout);
        }
    }

    unsafe fn realloc(&self, ptr: *mut u8, layout: Layout, new_size: usize) -> *mut u8 {
        let new_layout = Layout::from_size_align(new_size, layout.align()).unwrap();
        // Uh oh! The safety guarantees for `realloc` have been violated!
        assert!(new_layout.pad_to_align().size() <= isize::MAX as usize);

        // SAFETY: The safety requirements for `realloc` match the safety docs for `System.realloc`.
        unsafe { System.realloc(ptr, layout, new_size) }
    }
}

#[global_allocator]
static GLOBAL: MyGlobalAlloc = MyGlobalAlloc;

fn main() {
    let initial_layout = Layout::from_size_align(2, 2).unwrap();
    let final_layout = Layout::from_size_align(isize::MAX as usize, 2).unwrap();
    let ptr = Global.allocate(initial_layout).unwrap();

    // SAFETY:
    // - `ptr` is allocated from `Global` with layout `initial_layout`.
    // - `final_layout.size()` is `isize::MAX` which is greater than `initial_layout.size()`, which
    //   is 2.
    let ptr = unsafe { Global.grow(ptr.cast(), initial_layout, final_layout).unwrap() };
    // SAFETY: `ptr` is allocated from `Global` with `final_layout`.
    unsafe {
        Global.deallocate(ptr.cast(), final_layout);
    }
}

This sample panics at the assertion in <MyGlobalAlloc as GlobalAlloc>::realloc. It's not clear whether:

• The safety requirements of Allocator::grow need to include the missing clause from realloc.
• realloc should be relaxed to allow for allocations larger than isize::MAX.
• The implementation of Allocator::grow should check for this situation and return AllocError.

Meta

rustc --version --verbose:

rustc 1.61.0-nightly (0677edc86 2022-03-31)
binary: rustc
commit-hash: 0677edc86e342f333d4828b0ee1ef395a4e70fe5
commit-date: 2022-03-31
host: x86_64-pc-windows-msvc
release: 1.61.0-nightly
LLVM version: 14.0.0

[djkoloski]

@rustbot label +A-allocators +T-libs-api

[scottmcm]

Given work like #95295, I'm pretty sure it's not "realloc is supposed to work for sizes above isize::MAX".

[CAD97]

There is, however, the option that GlobalAlloc::realloc is required to not be UB and return an allocation failure for size > isize::MAX.

Personally, I don't really like that option, as the goal of #95295 is to make it so that the isize::MAX rule is automatically enforced by the type system, and developers don't have to think about the rule, it's just automatically followed.

There's two ways to accomplish that goal w.r.t. realloc:

• GlobalAlloc::realloc requires providers to handle the isize::MAX limit. Consumers never have to worry about the limit, as it just does the right thing.
• GlobalAlloc::realloc requires consumers to handle the isize::MAX limit. Providers never have to worry about the limit. Consumers should use Allocator::grow and Allocator::shrink instead, which do handle the isize::MAX limit.

Personally, I'm weakly in favor of the latter, because that leads to a world where ideally only consumers of the deprecated API have to worry about the limit. Providers of the API will be around ~forever, as it's how to provide a global allocator. Also, this doesn't change the requirements of GlobalAlloc.