Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

System.alloc returns unaligned pointer if align > size #45955

Closed
RalfJung opened this issue Nov 13, 2017 · 4 comments
Closed

System.alloc returns unaligned pointer if align > size #45955

RalfJung opened this issue Nov 13, 2017 · 4 comments
Labels
A-allocators Area: Custom and system allocators C-bug Category: This is a bug.

Comments

@RalfJung
Copy link
Member

RalfJung commented Nov 13, 2017

The following code:

#![feature(allocator_api, alloc_system)]

extern crate alloc_system;

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

pub fn main() {
    unsafe {
        let x = System.alloc(Layout::from_size_align(8, 16).unwrap()).unwrap();
        let y = System.alloc(Layout::from_size_align(8, 16).unwrap()).unwrap();

        println!("{:?} {:?}", (x as usize) % 16, (y as usize) % 16);
    }
}

prints 8 0 on my machine and on the playground. That's a bug; since I requested alignment 16 for both pointers, it should only ever print 0 0.

This is probably caused by the MIN_ALIGN optimization in alloc_system. This optimization is currently mostly useless, but it also seems to be wrong. I do not know what the actual alignment guarantees are; maybe the values are wrong or maybe the guarantee only holds for size >= align.

@RalfJung
Copy link
Member Author

I think some explanation for what's going on is that it actually seems to use jemalloc, even though I explicitly called the system allocator. That can't be right, can it?

If I add

#[global_allocator]
static GLOBAL: System = System;

the buggy behavior disappears, but shouldn't I be able to use the system allocator locally without affecting the global default?

@SimonSapin
Copy link
Contributor

SimonSapin commented Nov 13, 2017

I did some digging. The MIN_ALIGN constant and its “guaranteed by the architecture” comment were first added in #17095. Based on comments at #17095 (comment) and #13094 (comment), I believe this is based this section of the x86_64 ABI specification:

https://software.intel.com/sites/default/files/article/402129/mpx-linux64-abi.pdf#subsection.3.1.2

An array uses the same alignment as its elements, except that a local or global array variable of length at least 16 bytes or a C99 variable-length array variable always has alignment of at least 16 bytes.

“Global array variable” presumably applies to malloc since it needs to allocate memory suitable for arrays. So maybe Rust has a bug in not checking for the “of length at least 16 bytes” part.

I haven’t looked if other architecture’s ABIs have a similar requirement that we can rely on.

I did look in the C standard. It says that malloc must return a pointer aligned enough for max_align_t, which in turn is defined by an unhelpful tautology. Perhaps the libc crate could have a max_align_t type, based on research of what it is on various platforms/architectures/ABIs ?

However alignof(max_align_t) is indeed 16 with GCC 7.2.0 and clang 5.0.0 on my x86_64 laptop. So this is arguably a jemalloc bug[1]: gperftools/gperftools#724. Or possibly jemalloc deliberately ignores this aspect of the standard: https://gcc.gnu.org/ml/gcc-patches/2016-08/msg01902.html.

[1] It looks like when alloc_system::System is not not selected with #[global_allocator] the malloc symbol called by alloc_system::System is jemalloc, not glibc: #45831 (comment). Edit: I’ve filed #45966.

C11 references

https://port70.net/~nsz/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 and then used to access such an object or an array of such objects in the space allocated

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).

https://port70.net/~nsz/c/c11/n1570.html#7.19p2

max_align_t
which is an object type whose alignment is as great as is supported by the implementation in all contexts

@SimonSapin
Copy link
Contributor

Firefox ran into crashes when mozjemalloc did not guarantee 8 / 16: http://www.erahm.org/2016/03/24/minimum-alignment-of-allocation-across-platforms/

GNU libc’s allocator does guarantee 8 / 16 (https://www.gnu.org/software/libc/manual/html_node/Aligned-Memory-Blocks.html), but we apparently can’t rely on it being behind the malloc symbol even when the rest of libc is GNU.

@SimonSapin
Copy link
Contributor

#46117 should fix this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-allocators Area: Custom and system allocators C-bug Category: This is a bug.
Projects
None yet
Development

No branches or pull requests

3 participants