Skip to content

Commit

Permalink
alloc_system: don’t assume MIN_ALIGN for small sizes, fix #45955
Browse files Browse the repository at this point in the history
The GNU C library (glibc) is documented to always allocate with an alignment
of at least 8 or 16 bytes, on 32-bit or 64-bit platforms:
https://www.gnu.org/software/libc/manual/html_node/Aligned-Memory-Blocks.html

This matches our use of `MIN_ALIGN` before this commit.
However, even when libc is glibc, the program might be linked
with another allocator that redefines the `malloc` symbol and friends.
(The `alloc_jemalloc` crate does, in some cases.)

So `alloc_system` doesn’t know which allocator it calls,
and needs to be conservative in assumptions it makes.

The C standard says:

https://port70.net/%7Ensz/c/c11/n1570.html#7.22.3
> The pointer returned if the allocation succeeds is suitably aligned
> so that it may be assigned to a pointer to any type of object
> with a fundamental alignment requirement

https://port70.net/~nsz/c/c11/n1570.html#6.2.8p2
> A fundamental alignment is represented by an alignment less than
> or equal to the greatest alignment supported by the implementation
> in all contexts, which is equal to `_Alignof (max_align_t)`.

`_Alignof (max_align_t)` depends on the ABI and doesn’t seem to have
a clear definition, but it seems to match our `MIN_ALIGN` in practice.

However, the size of objects is rounded up to the next multiple
of their alignment (since that size is also the stride used in arrays).
Conversely, the alignment of a non-zero-size object is at most its size.
So for example it seems ot be legal for `malloc(8)` to return a pointer
that’s only 8-bytes-aligned, even if `_Alignof (max_align_t)` is 16.
  • Loading branch information
SimonSapin committed Nov 20, 2017
1 parent 41e03c3 commit 21d8992
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 3 deletions.
40 changes: 40 additions & 0 deletions src/liballoc/tests/heap.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

use alloc_system::System;
use std::heap::{Heap, Alloc, Layout};

/// https://github.com/rust-lang/rust/issues/45955
///
/// Note that `#[global_allocator]` is not used,
/// so `liballoc_jemalloc` is linked (on some platforms).
#[test]
fn alloc_system_overaligned_request() {
check_overalign_requests(System)
}

fn check_overalign_requests<T: Alloc>(mut allocator: T) {
let size = 8;
let align = 16; // greater than size
let iterations = 100;
unsafe {
let pointers: Vec<_> = (0..iterations).map(|_| {
allocator.alloc(Layout::from_size_align(size, align).unwrap()).unwrap()
}).collect();
for &ptr in &pointers {
assert_eq!((ptr as usize) % align, 0, "Got a pointer less aligned than requested")
}

// Clean up
for &ptr in &pointers {
allocator.dealloc(ptr, Layout::from_size_align(size, align).unwrap())
}
}
}
4 changes: 4 additions & 0 deletions src/liballoc/tests/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@

#![deny(warnings)]

#![feature(allocator_api)]
#![feature(alloc_system)]
#![feature(attr_literals)]
#![feature(box_syntax)]
#![feature(inclusive_range_syntax)]
Expand All @@ -29,6 +31,7 @@
#![feature(unboxed_closures)]
#![feature(unicode)]

extern crate alloc_system;
extern crate std_unicode;
extern crate rand;

Expand All @@ -39,6 +42,7 @@ mod binary_heap;
mod btree;
mod cow_str;
mod fmt;
mod heap;
mod linked_list;
mod slice;
mod str;
Expand Down
6 changes: 3 additions & 3 deletions src/liballoc_system/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ mod platform {
unsafe impl<'a> Alloc for &'a System {
#[inline]
unsafe fn alloc(&mut self, layout: Layout) -> Result<*mut u8, AllocErr> {
let ptr = if layout.align() <= MIN_ALIGN {
let ptr = if layout.align() <= MIN_ALIGN && layout.align() <= layout.size() {
libc::malloc(layout.size()) as *mut u8
} else {
aligned_malloc(&layout)
Expand All @@ -148,7 +148,7 @@ mod platform {
unsafe fn alloc_zeroed(&mut self, layout: Layout)
-> Result<*mut u8, AllocErr>
{
if layout.align() <= MIN_ALIGN {
if layout.align() <= MIN_ALIGN && layout.align() <= layout.size() {
let ptr = libc::calloc(layout.size(), 1) as *mut u8;
if !ptr.is_null() {
Ok(ptr)
Expand Down Expand Up @@ -180,7 +180,7 @@ mod platform {
})
}

if new_layout.align() <= MIN_ALIGN {
if new_layout.align() <= MIN_ALIGN && new_layout.align() <= new_layout.size(){
let ptr = libc::realloc(ptr as *mut libc::c_void, new_layout.size());
if !ptr.is_null() {
Ok(ptr as *mut u8)
Expand Down

0 comments on commit 21d8992

Please sign in to comment.