100% Crash in MallocBinned

Hi there.

I found an inappropriate behavior in MallocBinned source code. And before I made a pool request(in case I decide to make it), I’d like to clarify some items.
The problem looks pretty simple to explain:
The crash is occurred in FMallocBinned when system tries to resolve collisions in hash table.

UPTRINT Key=Ptr>>HashKeyShift;
UPTRINT Hash=Key&(MaxHashBuckets-1);
UPTRINT PoolIndex=((UPTRINT)Ptr >> PoolBitShift) & PoolMask;

PoolHashBucket* collision=&HashBuckets[Hash];
do
{
	if (collision->Key==Key || !collision->FirstPool)
	{
		if (!collision->FirstPool)
		{
			collision->Key=Key;
			InitializeHashBucket(collision);
			CA_ASSUME(collision->FirstPool);
		}
		return &collision->FirstPool[PoolIndex];
	}
	collision=collision->Next;
} while (collision!=&HashBuckets[Hash]);

Take a look at GetPoolInfo function:
Once the statement “if (collision->Key==Key || !collision->FirstPool)” isn’t passed
We go to the statement “collision=collision->Next;” where collision->Next is a NULL pointer
Then we to the next cycle pass and try to execute “if (collision->Key==Key || !collision->FirstPool)” again.
The program crashes.

The problem is that cyclic two dimension linked list for collision handling is never linked.
It starts to deal with linkage right after the loop described before:

//Create a new hash bucket entry
PoolHashBucket* NewBucket=CreateHashBucket();
NewBucket->Key=Key;
HashBuckets[Hash].Link(NewBucket);
return &NewBucket->FirstPool[PoolIndex];

However, it won’t get linked anyway. Due to potential crash in Link function. Take a look at PoolHashBucket struct:

static void Link( PoolHashBucket* Node, PoolHashBucket* Before, PoolHashBucket* After )
{
     Node->Prev=Before;
     Node->Next=After;
     Before->Next=Node;
     After->Prev=Node;
}

“Before” is a NULL pointer for the first node we’re going to link

In order to reproduce that problems quickly, you just need to force collision resolving. So, just set a low value to MaxHashBuckets (must be POT) in constructor of FMallocBinned.

So, is it possible to get some comments form developers? Because it is weird that no other users have faced that problems before.

Hi,

As I look at the code, Link is called with the Prev data member, and that is set in the constructor to ‘this’, which cannot be null.

Do you have an exact repro for what you’re seeing? I’ve tried setting MaxHashBuckets to 2 and it worked fine.

Steve

Hi,

I’m sorry. After you said that Prev is set to “this”, I checked this place it and noticed that Prev is set to “nullptr” on my end. I accidentally replaced “this” by “nullptr” when performed “Find and Replace” action.

Sorry again for the false call.

Tim