about summary refs log tree commit diff
diff options
context:
space:
mode:
authorSimon Sapin <simon.sapin@exyr.org>2017-11-20 15:30:04 +0100
committerSimon Sapin <simon.sapin@exyr.org>2017-11-20 15:56:53 +0100
commit21d899272a7fb39a497424e3260ddab773af7983 (patch)
treeca082d56ca602fafd323932925df299a5dfe5803
parent41e03c3c469d1c89735fa518a9af4eb3df8b0728 (diff)
downloadrust-21d899272a7fb39a497424e3260ddab773af7983.tar.gz
rust-21d899272a7fb39a497424e3260ddab773af7983.zip
alloc_system: don’t assume MIN_ALIGN for small sizes, fix #45955
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.
-rw-r--r--src/liballoc/tests/heap.rs40
-rw-r--r--src/liballoc/tests/lib.rs4
-rw-r--r--src/liballoc_system/lib.rs6
3 files changed, 47 insertions, 3 deletions
diff --git a/src/liballoc/tests/heap.rs b/src/liballoc/tests/heap.rs
new file mode 100644
index 00000000000..9423aabc82b
--- /dev/null
+++ b/src/liballoc/tests/heap.rs
@@ -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())
+        }
+    }
+}
diff --git a/src/liballoc/tests/lib.rs b/src/liballoc/tests/lib.rs
index 00ebd88d464..f1e95883b38 100644
--- a/src/liballoc/tests/lib.rs
+++ b/src/liballoc/tests/lib.rs
@@ -10,6 +10,8 @@
 
 #![deny(warnings)]
 
+#![feature(allocator_api)]
+#![feature(alloc_system)]
 #![feature(attr_literals)]
 #![feature(box_syntax)]
 #![feature(inclusive_range_syntax)]
@@ -29,6 +31,7 @@
 #![feature(unboxed_closures)]
 #![feature(unicode)]
 
+extern crate alloc_system;
 extern crate std_unicode;
 extern crate rand;
 
@@ -39,6 +42,7 @@ mod binary_heap;
 mod btree;
 mod cow_str;
 mod fmt;
+mod heap;
 mod linked_list;
 mod slice;
 mod str;
diff --git a/src/liballoc_system/lib.rs b/src/liballoc_system/lib.rs
index 05cacf6e881..8077ab2063d 100644
--- a/src/liballoc_system/lib.rs
+++ b/src/liballoc_system/lib.rs
@@ -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)
@@ -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)
@@ -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)