Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
Jules-Bertholet opened this issue Oct 20, 2022 · 2 comments
Open

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

Jules-Bertholet opened this issue Oct 20, 2022 · 2 comments

Comments

@Jules-Bertholet
Copy link

Jules-Bertholet commented Oct 20, 2022

(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
Copy link

CAD97 commented Oct 20, 2022

(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
Copy link

zakarumych commented Feb 6, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants