rust-lang / wg-allocators

Home of the Allocators working group: Paving a path for a standard set of allocator traits to be used in collections!
http://bit.ly/hello-wg-allocators
203 stars 9 forks source link

Must the returned sizes of `Allocator` methods fit in `Layout`? #107

Open Jules-Bertholet opened 1 year ago

Jules-Bertholet commented 1 year ago

(Discussion on Zulip)

The documentation of Allocator::allocate makes only the following guarantees in case of successful allocation:

On success, returns a NonNull<[u8]> meeting the size and alignment guarantees of layout.

The returned block may have a larger size than specified by layout.size(), and may or may not have its contents initialized.

grow and shrink have similar guarantees.

The preconditions of Layout::from_size_align contain the following:

size, when rounded up to the nearest multiple of align, must not overflow (i.e., the rounded value must be less than or equal to isize::MAX.

Does this imply that the allocation returned by Allocator::allocate may have a size that is less than isize::MAX but very close, such that rounding it up to layout.align() will lead to an overflow?

In other words, assuming allocate doesn't panic, can the following function ever panic?

#![feature(allocator_api)]

use core::alloc::{Allocator, Layout};

pub fn foo(a: &impl Allocator) {
    const ALIGN: usize = 64;
    let alloc_layout: Layout = Layout::from_size_align(300, ALIGN).unwrap();

    let allocation_result = a.allocate(alloc_layout); // assume this never panics

    if let Ok(ptr) = allocation_result {
        assert!(Layout::from_size_align(ptr.len(), ALIGN).is_ok()); // can this panic?
    }
}
CAD97 commented 1 year ago

(Layout currently documents size as overflowing at usize::MAX, not isize::MAX. If the allocator respects the currently implicit limit of allocations being no larger than isize::MAX, then rounding up to alignment cannot exceed usize::MAX. Of course, I'm spearheading the effort to make the limit a lot less implicit by putting the Layout size overflow at isize::MAX instead.)

Is it reasonable to require the reported allocation size to be a multiple of the requested alignment? I don't see any potential reason to overallocate which isn't to a multiple of alignment other than to cause problems. This also has the benefit of the caller being able to use from_size_align_unchecked.

I suppose the use case is when making a request for a layout which is not padded to alignment itself, e.g. a small type aligned to a page.

Interestingly enough, though, the behavior that a caller would want (where returned size is not a multiple of align) is actually to round down to alignment, as the rounded up value isn't really usable for nearly anything, as it can't be used to check access for being inbounds nor even to deallocate the pointer.

I think this is a reasonable requirement. In practice, allocations which would fail this test are pretty degenerate anyway, as they take up a quarter of the address space at absolute minimum.

zakarumych commented 1 year ago

From latest Layout documentation

size, when rounded up to the nearest multiple of align, must not overflow isize (i.e., the rounded value must be less than or equal to isize::MAX).

So statement above is no longer valid (I guess they were successful in their effort).

There may be reason to overallocate with size not a multiple of alignment. Reason for overallocating in general applies - to not keep too small free blocks. It is possible to ask Allocator impl to round down reported size to the nearest multiple of align. However I don't see why caller can't use additional memory if they would so desire.

For convenience we may add a method to construct Layout without panic for a common use-case. Layout::from_oversize_align that rounds size down instead of up.