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
205 stars 9 forks source link

Question on `alloc_guard` in `raw_vec.rs` #45

Closed TimDiekmann closed 4 years ago

TimDiekmann commented 4 years ago

Every allocation in RawVec is guarded to a maximum allocation size of isize::MAX:

// 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 mem::size_of::<usize>() < 8 && alloc_size > core::isize::MAX as usize {
        Err(CapacityOverflow)
    } else {
        Ok(())
    }
}

Shouldn't this guard be moved into the implementations of AllocRef instead? The guard was added before Alloc was added: raw_vec.rs @ Rust 1.3.0

Amanieu commented 4 years ago

The guard exists for a good reason: LLVM generates buggy code when indexing an array with more than isize::MAX elements, or generally any object with a size larger than isize::MAX bytes.

I don't think that moving this logic into the AllocRef trait is the right thing to do: the guard needs to exist regardless of what the allocator back-end is, it is not something that an allocator would want to customize.

TimDiekmann commented 4 years ago

Ah, wasn't aware of the LLVM bug, thanks!