dvidelabs / flatcc

FlatBuffers Compiler and Library in C for C
Apache License 2.0
632 stars 180 forks source link

alignment must be an integer power of 2. #207

Closed zhiyaluo closed 2 years ago

zhiyaluo commented 2 years ago

Hi all, In my opinion, the alignment value, which must be an integer power of 2. Could you please give me some advice? Thank you!

mikkelfj commented 2 years ago

Hi @zhiyaluo thanks for bringing this up. However, as far as I can tell, a power of two is not a requirement in general, but an accepted limitation on implemementations based on posix_memalign:

https://en.cppreference.com/w/c/memory/aligned_alloc

See Notes section.

As an example of the "supported by the implementation" requirement, POSIX function posix_memalign accepts any alignment that is a power of two and a multiple of sizeof(void *), and POSIX-based implementations of aligned_alloc inherit this requirements.

However, C11 had undefined behaviour if the function was not a multiple of alignment argument, but was later changed to fail, and also if the implemetnation has other limitations such as power of two:

Passing a size which is not an integral multiple of alignment or a alignment which is not valid or not supported by the implementation causes the function to fail and return a null pointer (C11, as published, specified undefined behavior in this case, this was corrected by DR 460).

Thus it seems the implemtentation was originally correct but outdated by DR 460. However, your suggested change will not correctly handle this because it doesn't check multiples of alignment - this incidentally requires a multiplication to check. Also, there would need to be asserts in all branch implementations.

I'm not sure if this is really worthwhile given that aligned_alloc will be used when available.

Furthermore, the note also states the removal of size restrictions has been proposed in the future:

Removal of size restrictions to make it possible to allocate small objects at restrictive alignment boundaries (similar to alignas) has been proposed by n2072.

zhiyaluo commented 2 years ago

@mikkelfj Thanks for your professional advise and sorry about my delayed reply.

I have only studied that how Visual Studio implemented align_malloc:

https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/aligned-malloc?view=msvc-160

VS only required that alignment must be an integer power of 2. It seems that different compilers implemented different standards.

Thanks again!

mikkelfj commented 2 years ago

You are welcome and thanks for contributing even if it didn't make it.