Bug in tbbmalloc with scalable_aligned_realloc() & scalable_msize()

[James_M_1]

Hi,

I believe I've found a bug in tbbmalloc relating to scalable_aligned_realloc & scalable_msize. In the code below all my asserts will fire.

#include <...>

#define ARRAY_COUNT(x) (sizeof(x)/sizeof(x[0]))

static const uintptr_t slabSize = 16 * 1024;
static uintptr_t findAllocationOffset(const void* address, size_t objectSize) {
	uintptr_t base = (uintptr_t)address & ~(slabSize - 1);
	// calculate offset from the end of the block space
	uint16_t offset = base + slabSize - (uintptr_t)address;
	// find offset difference from a multiple of allocation size
	offset %= objectSize;
	// and move the address down to where the real object starts.
	return offset ? objectSize - offset : 0;
}

void main() {
	static const size_t AllocCount = 4;
	void* Ptrs[AllocCount] = { 0 };
	size_t Sizes[AllocCount] = { 0 };

	for (uint32 i = 0; i < ARRAY_COUNT(Ptrs); ++i) {
		Sizes = 3680;
		Ptrs = scalable_aligned_malloc(Sizes, 128);
	}

	for (uint32 i = 0; i < ARRAY_COUNT(Ptrs); ++i) {
		Sizes = 4016;
		Ptrs = scalable_aligned_realloc(Ptrs, Sizes, 128);
	}

	/** Check that pointer offsets & sizes make sense (the block size should be 4032) */
	for (uint32 i = 0; i < ARRAY_COUNT(Ptrs); ++i) {
		size_t alloc_size = scalable_msize(Ptrs);
		uintptr_t alloc_offset = findAllocationOffset(Ptrs, alloc_size);
		assert(alloc_size + alloc_offset == 4032);
 	}
	/** Check an allocation hasn't gone outside the slab */
	for (uint32 i = 0; i < ARRAY_COUNT(Ptrs); ++i) {
		assert(((uintptr_t)Ptrs & ~(slabSize - 1)) == (((uintptr_t)Ptrs + (Sizes - 1)) & ~(slabSize - 1)));
	}
	/** Check an allocation doesn't overlap with another */
	for (uint32 i = 0; i < ARRAY_COUNT(Ptrs); ++i) {
		for (uint32 j = 0; j < ARRAY_COUNT(Ptrs); ++j) {
			if (i != j) {
				assert((UPTRINT)Ptrs < (UPTRINT)Ptrs || (UPTRINT)Ptrs >= (UPTRINT)Ptrs + Sizes);
			}
		}
	}

	for (uint32 i = 0; i < ARRAY_COUNT(Ptrs); ++i) {
		scalable_free(Ptrs);
	}
}

Basically, I believe the both scalable_aligned_realloc and scalable_msize fail to account for alignment offsets in a small allocation bin. 
With scalable_msize() the function internalMsize appears to be wrong.

    if (ptr) {
        ...
        if (isLargeObject(ptr)) {
            ...
        } else {
            Block* block = (Block *)alignDown(ptr, slabSize);
#if MALLOC_CHECK_RECURSION
            size_t size = block->getSize()? block->getSize() : StartupBlock::msize(ptr);
#else
            size_t size = block->getSize();
#endif
            ...
            return size;
        }
    }

But "return size;" shoud be something like "return size - findAllocationOffset(ptr, block->getSize());"... I think :)

With scalable_aligned_realloc() the function reallocAligned appears to fail here:

        Block* block = (Block *)alignDown(ptr, slabSize);
        copySize = block->getSize();
        if (size <= copySize && (0==alignment || isAligned(ptr, alignment))) {
            return ptr;

Again, I think that "copySize = block->getSize();" should be something along the lines of "copySize = block->getSize() - findAllocationOffset(ptr, block->getSize());" otherwise realloc will return allocations that overlap each other.

Hopefully that made sense :)

Thanks,James

[Alexandr_K_Intel1]

James, Thank you for the report. It seems definitely the issues in our code.

[James_M_1]

Thanks for the reply.

If there is a bug fix put in for this, what kind of time frame am I looking at until it would be released? Would it be just released with the next 4.x version update?

Thanks, James

[Vladimir_P_1234567890]

hello James,

This issue will be addressed in one of next releases. we will put a message to this thread when the fix is available. 

--Vladimir