search

mtrebi / memory-allocators

Custom memory allocators in C++ to improve the performance of dynamic memory allocation
MIT License
1.74k stars 159 forks source link

Bug in Free List Allocator on 32bit systems #18

Open kochol opened 4 years ago

kochol commented 4 years ago

Hi There is a bug in your Free List Allocator when alignmentPadding is not zero like on 32 bit systems.

https://github.com/mtrebi/memory-allocators/blob/0d6ede36636115c7162ec11ab340065820a179fc/src/FreeListAllocator.cpp#L46

One bug is here obviously and newFreeNode address is inside of the current block when alignmentPadding is not zero.

https://github.com/mtrebi/memory-allocators/blob/0d6ede36636115c7162ec11ab340065820a179fc/src/FreeListAllocator.cpp#L53

degski commented 4 years ago

That depends (on Windows) where you get the memory from, as far as I know (VirtualMemory is zeroed), the padding bytes are undefined (so it depends on what the allocator did). On the other hand, you can just set them to zero by making the padding part of your type:

struct alignas ( 8 ) type  { int val, _ = 0; };
kochol commented 4 years ago

It is still a bug.

degski commented 4 years ago

I'm just pointing out to how to keep the padding of an object under control as it might be that comparison might depend on it (like with std::memcmp ) as the size of the object includes the padding (so one could be more careful from the get-go), it will not compare equal when the padding differs, while they all have equivalent (e.i. not unequal) value (equal is not necessarily equal to not unequal, its' defined as equivalence). Somewhere someone needs to put a zero, and/or make sure the padding is not considered in any comparison, initing the padding is an option (and makes clear there is some unused space in the struct available for future use):

struct alignas ( 16 ) type2  { long long value, reserved_ = 0; };

The trick is to have no padding, which can be statically asserted and/or sfinaed, specilized.