- 
                Notifications
    You must be signed in to change notification settings 
- Fork 13.9k
Open
Labels
A-alignArea: alignment control (`repr(align(N))` and so on)Area: alignment control (`repr(align(N))` and so on)C-feature-requestCategory: A feature request, i.e: not implemented / a PR.Category: A feature request, i.e: not implemented / a PR.T-libsRelevant to the library team, which will review and decide on the PR/issue.Relevant to the library team, which will review and decide on the PR/issue.
Description
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:
rust/library/alloc/src/raw_vec.rs
Lines 442 to 478 in 8885239
| 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
Labels
A-alignArea: alignment control (`repr(align(N))` and so on)Area: alignment control (`repr(align(N))` and so on)C-feature-requestCategory: A feature request, i.e: not implemented / a PR.Category: A feature request, i.e: not implemented / a PR.T-libsRelevant to the library team, which will review and decide on the PR/issue.Relevant to the library team, which will review and decide on the PR/issue.