Skip to content

Suggestion: debug-assert that the allocator returns aligned memory #131189

Open
@emilk

Description

@emilk

If the global allocator returns unaligned memory, the standard library catches it very late. Take this code:

#[global_allocator]
static GLOBAL: AccountingAllocator<mimalloc::MiMalloc> =
    AccountingAllocator::new(mimalloc::MiMalloc);

#[repr(C, align(256))]
pub struct BigStruct {}

fn collect() {
    assert_eq!(align_of::<BigStruct>(), 256);
    assert_eq!(size_of::<BigStruct>(), 256);
    let vec: Vec<T> = std::iter::once(BigStruct{}).collect();
    dbg!(vec.as_ptr() as usize % std::mem::align_of::<T>()); // Sometimes prints 64
    let slice = vec.as_slice(); // panics in debug builds with:
    // 'unsafe precondition(s) violated: slice::from_raw_parts requires the pointer to be aligned and non-null, and the total size of the slice not to exceed `isize::MAX`'
}

Because of a bug in MiMalloc, the Vec gets non-aligned memory.
This is not discovered until vec.as_slice calls slice::from_raw_parts which has a debug-assert.

I spent a lot of time looking for this bug before I realized it was MiMalloc that was to blame.

I suggest we add a debug-assert the standard library to check that the allocator returns aligned memory, i.e. here:

fn try_allocate_in(
capacity: usize,
init: AllocInit,
alloc: A,
elem_layout: Layout,
) -> Result<Self, TryReserveError> {
// We avoid `unwrap_or_else` here because it bloats the amount of
// LLVM IR generated.
let layout = match layout_array(capacity, elem_layout) {
Ok(layout) => layout,
Err(_) => return Err(CapacityOverflow.into()),
};
// Don't allocate here because `Drop` will not deallocate when `capacity` is 0.
if layout.size() == 0 {
return Ok(Self::new_in(alloc, elem_layout.align()));
}
if let Err(err) = alloc_guard(layout.size()) {
return Err(err);
}
let result = match init {
AllocInit::Uninitialized => alloc.allocate(layout),
#[cfg(not(no_global_oom_handling))]
AllocInit::Zeroed => alloc.allocate_zeroed(layout),
};
let ptr = match result {
Ok(ptr) => ptr,
Err(_) => return Err(AllocError { layout, non_exhaustive: () }.into()),
};
// Allocators currently return a `NonNull<[u8]>` whose length
// matches the size requested. If that ever changes, the capacity
// here should change to `ptr.len() / mem::size_of::<T>()`.
Ok(Self { ptr: Unique::from(ptr.cast()), cap: unsafe { Cap(capacity) }, alloc })
}

That would then report "The global allocator returned non-aligned memory", directly pointing the user at the problem, saving a lol of time for the next person.

Metadata

Metadata

Assignees

No one assigned

    Labels

    A-alignArea: alignment control (`repr(align(N))` and so on)C-feature-requestCategory: A feature request, i.e: not implemented / a PR.T-libsRelevant to the library team, which will review and decide on the PR/issue.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions