ETLCPP / etl

Embedded Template Library
https://www.etlcpp.com
MIT License
2.05k stars 373 forks source link

etl::pool silently produces unaligned allocations for types with stricter alignment requirements than built in types #767

Closed clnhlzmn closed 9 months ago

clnhlzmn commented 9 months ago

A coworker of mine is trying to use etl::pool to allocate objects (byte arrays) that must be aligned to 32 bytes. She has tried using etl::generic_pool<..., 32, ...> but it turns out that is failing to produce 32 byte aligned allocations because etl::type_with_alignment<32>::type silently produces char as type when the requested alignment doesn't match the alignment of one of int_least8_t, int_least16_t, int32_t, int64_t, float, double, or void* (https://github.com/ETLCPP/etl/blob/master/include/etl/alignment.h#L210).

Example:

image

If I make this change to type_with_alignment:

image

my example produces the correct alignment:

image

The etl::pool docs imply that the alignment will be correct for T.

image

It would be nice at least to get a static assert if the requested alignment cannot be achieved, but ideally the pool buffer should "just" meet the alignment requirements of the type being allocated.

jwellbelove commented 9 months ago

The static_assert is a good idea. I can also modify the code so that it uses alignas for C++11 and above.

clnhlzmn commented 9 months ago

That would be awesome. I'm happy to help if I can. But I don't know the consequences of the change I suggested throughout the rest of etl.

jwellbelove commented 9 months ago

I implemented a static_assert to check that the requested alignment is achieved. Also the code uses alignas if C++11 or above, dropping back to the old method for C++03 and below. I've run the CI locally and on Github with no issues. If user code breaks then it will highlight an issue with their code.

jwellbelove commented 9 months ago

Fixed 20.38.3