search

rust-lang / miri

An interpreter for Rust's mid-level intermediate representation
Apache License 2.0
4.16k stars 318 forks source link

Implement aligned_alloc #3577

Closed RalfJung closed 1 month ago

RalfJung commented 1 month ago

C11 has a new allocation function: aligned_alloc. We should implement it in Miri. Docs can be found here.

RalfJung commented 1 month ago

Ah, but the function spec is completely terrible of course...

alignment - specifies the alignment. Must be a valid alignment supported by the implementation.

There is no guarantee at all for what is a "valid alignment supported by the implementation". So this can basically always return null, great.

Given the comments in the docs, it seems like a reasonable choice of "valid alignment supported by the implementation" is to require the alignment to be at least the size of a pointer. I have no clue why an implementation would reject smaller alignment, but it is what posix_memalign does so it seems to be common... somehow...

All "fundamental" alignments have to be supported -- which I hope doesn't mean we have to support non-power-of-two alignments smaller than max_align_t. I think for now the best course of action is to support all powers of two and return NULL otherwise. A size of 0 should be handled like malloc.

workingjubilee commented 1 month ago

indeed, void* minimum in practice, it seems: https://www.gnu.org/software/gnulib/manual/html_node/aligned_005falloc.html

workingjubilee commented 1 month ago

though asking for too-large alignments is also bad: https://github.com/rust-lang/rust/pull/124788/commits/53bd38b7c5d5dc43feced7cf76904fa5b788de0c

RalfJung commented 1 month ago

indeed, void* minimum in practice, it seems: https://www.gnu.org/software/gnulib/manual/html_node/aligned_005falloc.html

That's for that one implementation. But who knows about others...

though asking for too-large alignments is also bad: rust-lang/rust@53bd38b

That's "obviously" a platform bug though, I don't think Miri should model that. (Returning NULL for very big alignments would be allowed by the spec, but returning an unaligned value is clearly violating the spec.)

RalfJung commented 1 month ago

indeed, void* minimum in practice, it seems: https://www.gnu.org/software/gnulib/manual/html_node/aligned_005falloc.html

Looking at the spec again, it says fundamental alignments must be supported. So I think a void* minimum is actually non-compliant. macOS (and Illumos) implementations of this function are buggy.

RalfJung commented 1 month ago

:joy: turns out libc doesn't have this function yet... https://github.com/rust-lang/libc/issues/3689. That certainly makes testing harder.

workingjubilee commented 1 month ago

Looking at the spec again, it says fundamental alignments must be supported. So I think a void* minimum is actually non-compliant. macOS (and Illumos) implementations of this function are buggy.

And some platforms just don't implement it at all, of course, for legacy reasons.

RalfJung commented 1 month ago

According to my research, at least all the Unixes Miri currently has any support for do all have it. That's good enough for me to say we just provide it on all Unixes.

workingjubilee commented 1 month ago

Oh yes, the notable exception is Windows.

RalfJung commented 1 month ago

As usual ;)