-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
We call posix_memalign with a too small alignment #62251
Comments
The only reason I can imagine for |
A lack of comment is disturbing here. If git history does not reveal any such issue, then we should just remove that check entirely. |
I have opened jemalloc/jemalloc#1533 in jemalloc to get clarification from them about their alignment guarantees. It would be nice if someone with more knowledge could chime in, in particular knowledge about where exactly that value of 16 for |
Possibly from here: https://www.gnu.org/software/libc/manual/html_node/Aligned-Memory-Blocks.html#Aligned-Memory-Blocks
|
@RalfJung C has The largest scalar type of C implementations needs to be supported in all contexts. On most platforms, this type is often On these platforms, all memory allocation functions need to return memory aligned to a 16 byte boundary when called with a In practice, the fastest thing you can do, is calling malloc if The else branch is calling a "generic" rust/src/libstd/sys/unix/alloc.rs Line 83 in 9ebf478
appears to have the bug you mention, and should probably call I suspect that the implementation of Line 19 in 6d59933
doesn't have this issue, because it probably just forwards the call to |
So basically on the C side, one assumes that With that assumption, I agree it is enough for The bug in the way we call Still, it would be nice not to be able to reason about "in practice", but get some commitment from jemalloc -- see jemalloc/jemalloc#1533. And finally, this only concerns |
The docs are quite clear IMO:
Anything more concrete than that will be target specific, and those details are outside jemalloc's control - if jemalloc wants to replace the system allocator in a target, it needs to comply with the ABI there. If it were to guarantee more, e.g., all allocations are 16byte aligned, it wouldn't work on platforms using the SysV i386 ABI. |
Well, with And for users to manually do the alignment, getting something more concrete would be useful. And also for Rust! Currently you are deducing a whole lot of things from a few words in the spec. I have zero confidence that this is the only way to read the spec. So something like: "for would IMO be prudent to ask from the jemalloc devs. Or else we'll find ourselves with more bugs when someone interprets all this slightly differently. |
I don't know how useful would that be: malloc only needs to comply with the C standard. Parts of it are implementation-defined, and what those are defined to is often specified by the system ABI. That's the only thing the global allocator can rely on. On most platforms, the system allocator - which is the default global allocator for Rust programs - is not jemalloc - it is the one from glibc, musl, microsoft, apple, google, mozilla, etc. In some platforms, like FreeBSD, the system allocator is jemalloc, but even there we can't exploit that knowledge because users can just So even if jemalloc were to offer extra guarantees here, Rust's system allocator cannot make use of them. The only places were we can can exploit jemalloc's specific guarantees, is when using crates like jemallocator, to explicitly hardcode the allocator to a particular one. As in, if that allocator is not linked, you get a linker error or similar. But even there, it makes sense to stick to what C and the platform ABI guarantees, because jemalloc is free to break anything else in the next version. |
Well it would save us this entire discussion. If that doesn't demonstrate its usefulness I don't know what does.^^ I don't know what the "system allocator" is for Linux, but the one in glibc actually guarantees a 16-byte alignment on 64bit systems for all allocations. So by that standard jemalloc would just be incorrect. |
The platform ABI guarantees that. What the malloc implementations guarantee is irrelevant. If glibc were to guarantee that all addresses are 32-byte aligned, there is no way in which we could exploit that information in the implementation of the system allocator, because the platform only requires 16-byte alignment, and it is valid to pick any allocator at run-time that satisfies that. |
So there a document somewhere saying "for x86-64 bit, 16-byte alignment is guaranteed"? But that document contains an exception for small sizes, making |
Yes, there are documents containing the rules of what's legal, at least for Linux x86_64. Those document allow an exception for small sizes. One API that uses those is
I still have no idea what jemalloc has to do with any of this - your original question is about |
@RalfJung A document that could be relevant is the C standard. Unfortunately, as mentioned in the commit message for 21d8992, it is rather tautological on this topic:
The standard does not give any numeric value. Like many other aspects of C, it looks like the actual value of |
When using jemalloc,
Do you have a reference for something that concretely calls out an exception for small types? The C standard only does this very indirectly, as you showed. |
That was a bug in our implementation of system that applies to pretty much all allocators, not only jemalloc.
C18 (C17 draft) says (emphasis mine):
Since the alignment of an object is a power of two that is always smaller than or equal to the object size, rounding down the alignment of an allocation to the previous power of two always produces an allocation properly aligned for every type that can fit it. That holds for all allocations, so there is no exception for that. That's the rule. Since that would mean that large allocations must be unreasonably aligned, the standard does provide an exception for allocations of size larger than than If you believe this is incorrect, please show a counter-example for which this doesn't work. |
AFAIK it is a normal way of using
I don't say it is incorrect, I say it is awfully indirect. It's like reading tea leaves. I am looking for a clear definite statement. That doesn't seem to exist though. :( There clearly is some kind of "non-continuity" where all allocations are aligned MIN_ALIGN except for small ones. Or vice versa, all allocations are aligned "size rounded down to power of 2" except for big ones. You are convincingly deriving that non-continuity from standard wording. I don't say you are wrong, but this feels like interpreting ancient scripture. A clear spec looks different. Also, an object of size 2 with alignment requirement 4 is "fundamental" according to your definition, and yet rounding down the size to the next power of 2 does not give the right alignment. So your rule does have exceptions, namely types marked |
Can you show such an object in C and/or Rust ?
It's probably the most common way of switching allocators.
What |
The goal is not to rely on assumptions that only hold for Because that's what we used to do, before #45955 got fixed. We were assuming something that was true for system To make sure we don't do that again, I want to be sure that all assumptions we are making hold for |
Ah I forgot about the stride vs. size thing. So such an object does not exist. But such a And we do have objects with align > size, namely all ZST. |
Yeah, I agree. The only cases for which the "round to the previous power of two approach" doesn't work is for ZSTs, and types (or allocations) with align > size. Since those just cannot happen in C, the C standard doesn't mention them as exceptions. That is, we can only call |
request at least ptr-size alignment from posix_memalign Fixes rust-lang#62251
request at least ptr-size alignment from posix_memalign Fixes rust-lang#62251
request at least ptr-size alignment from posix_memalign Fixes rust-lang#62251
request at least ptr-size alignment from posix_memalign Fixes rust-lang#62251
The man page for
posix_memalign
saysAnd yet Miri found libstd calling this function with an alignment of 4 on a 64bit-platform. This happens when size=2, align=4. The fact that size<align makes it enter the code path for
posix_memalign
.rust/src/libstd/sys/unix/alloc.rs
Line 9 in 0af8e87
The text was updated successfully, but these errors were encountered: