search

matklad / once_cell

Rust library for single assignment cells and lazy statics without macros
Apache License 2.0
1.84k stars 110 forks source link

Fix alignment of `imp_std::Waiter` #114

Closed eduardosm closed 3 years ago

eduardosm commented 3 years ago

Using #[repr(align(4))] directly on Waiter will downgrade alignment form 8 to 4 bytes on 64-bit targets.

To fix this, an empty struct is defined (Align4), which is tagged with #[repr(align(4))], and an instance of this empty struct is added to Waiter. Waiter will now have an alignment of at least 4 bytes, but it will be greater on targets that require it.

matklad commented 3 years ago

Hm, I don't think this how align works?

For align, if the specified alignment is less than the alignment of the type without the align modifier, then the alignment is unaffected.

from https://doc.rust-lang.org/reference/type-layout.html#the-alignment-modifiers

eduardosm commented 3 years ago

Ouch, I mixed the align and packed concepts, sorry. Closing this PR then.

matklad commented 3 years ago

It's all good!

JFYI, here's how I figured that align might not work that way:

If align were to force the alignment, than it would be unsafe to do field access with such a struct. Given that there's no unsafe in the attribute, and the compiler doesn't require unsafe when accessing fields (which would be the case for packed), then it seems likely that it only sets the min alighment.