Fix memstack free not clearing metadata

This commit is contained in:
Doyle Thai 2018-02-03 15:51:47 +11:00
parent 6a02e38706
commit 7228a8b344
2 changed files with 66 additions and 34 deletions

92
dqn.h
View File

@ -345,14 +345,21 @@ class DqnSmartArray : public DqnArray<T>
// #DqnAllocatorMetadata // #DqnAllocatorMetadata
// ================================================================================================= // =================================================================================================
// Allocation Layout // Allocation Layout
// +----------------------------------------------------------------------------------------------------------------------------+ // +------------------------------------------------------------+ +-----------------+
// | | Allocation Head | | Allocation Tail | // | Allocation Head | | Allocation Tail |
// +----------------------------------------------------------------------------------------------------------------------------+ // +--------------------+------------------------------------------------------------+------------------------+-----------------+
// | Ptr From Allocator | Offset To Src | Alignment | Alloc Amount | B. Guard (Opt.) | Aligned Ptr For Client | B. Guard (Opt.) | // | Ptr From Allocator | Offset To Src | Alignment | Alloc Amount | B. Guard (Opt.) | Aligned Ptr For Client | B. Guard (Opt.) |
// +----------------------------------------------------------------------------------------------------------------------------+ // +----------------------------------------------------------------------------------------------------------------------------+
// Ptr From Allocator: The pointer returned by the allocator, not aligned
// Alignment: The pointer given to the client is aligned to this power of two boundary
// Alloc Amount: The requested allocation amount by the client (so does not include metadata)
// B. Guard: Bounds Guard value.
// Aligned Ptr For Client: The allocated memory for the client.
// Offset To Src: Number of bytes to subtract from the "Aligned Ptr For Client" to return to the "Ptr From ALlocator"
struct DqnAllocatorMetadata struct DqnAllocatorMetadata
{ {
static u32 const GUARD_VALUE = 0xBEBAFECA; static u32 const HEAD_GUARD_VALUE = 0xCAFEBABE;
static u32 const TAIL_GUARD_VALUE = 0xDEADBEEF;
static u32 const OFFSET_TO_SRC_SIZE = sizeof(u8); static u32 const OFFSET_TO_SRC_SIZE = sizeof(u8);
static u32 const ALIGNMENT_SIZE = sizeof(u8); static u32 const ALIGNMENT_SIZE = sizeof(u8);
static u32 const ALLOC_AMOUNT_SIZE = sizeof(usize); static u32 const ALLOC_AMOUNT_SIZE = sizeof(usize);
@ -369,12 +376,13 @@ struct DqnAllocatorMetadata
auto GetAllocHeadSize () const { return allocHeadSize; } auto GetAllocHeadSize () const { return allocHeadSize; }
auto GetAllocTailSize () const { return allocTailSize; } auto GetAllocTailSize () const { return allocTailSize; }
usize GetAllocationSize (usize size, u8 alignment) const { return GetAllocHeadSize() + size + GetAllocTailSize() + (alignment - 1); }
u32 *PtrToHeadBoundsGuard(u8 const *ptr) const; // ptr: The ptr given to the client when allocating. u32 *PtrToHeadBoundsGuard(u8 const *ptr) const; // ptr: The ptr given to the client when allocating.
u32 *PtrToTailBoundsGuard(u8 const *ptr) const; // IMPORTANT: Uses "Alloc Amount" metadata to find the tail! u32 *PtrToTailBoundsGuard(u8 const *ptr) const; // IMPORTANT: Uses "Alloc Amount" metadata to find the tail!
u8 *PtrToAlignment (u8 const *ptr) const; u8 *PtrToAlignment (u8 const *ptr) const;
u8 *PtrToOffsetToSrc (u8 const *ptr) const; u8 *PtrToOffsetToSrc (u8 const *ptr) const;
usize *PtrToAllocAmount (u8 const *ptr) const; usize *PtrToAllocAmount (u8 const *ptr) const;
usize GetAllocationSize (usize size, u8 alignment) const { return GetAllocHeadSize() + size + GetAllocTailSize() + (alignment - 1); }
private: private:
u32 boundsGuardSize; // sizeof(GUARD_VALUE) OR 0 if BoundsGuard is disabled. u32 boundsGuardSize; // sizeof(GUARD_VALUE) OR 0 if BoundsGuard is disabled.
@ -403,8 +411,10 @@ struct DqnMemStack
enum Flag enum Flag
{ {
NonExpandable = (1 << 0), // Disallow additional memory blocks when full. NonExpandable = (1 << 0), // Disallow additional memory blocks when full.
BoundsGuard = (1 << 1), // Track, check and add guards on all allocations NonExpandableAssert = (1 << 1), // Assert when NonExpandable is set with allocation on a full stack.
BoundsGuard = (1 << 2), // Track, check and add guards on all allocations
All = (NonExpandable | NonExpandableAssert | BoundsGuard),
}; };
struct Info // Statistics of the memory stack. struct Info // Statistics of the memory stack.
@ -437,7 +447,7 @@ struct DqnMemStack
// Uses fixed buffer, allocations will be sourced from the buffer and fail after buffer is full. // Uses fixed buffer, allocations will be sourced from the buffer and fail after buffer is full.
// mem: Memory to use for the memory stack // mem: Memory to use for the memory stack
// return: FALSE if args are invalid, or insufficient memSize. // return: FALSE if args are invalid, or insufficient memSize.
bool Init(u8 *const mem, usize size, u32 flags_ = 0); bool Init(void *const mem, usize size, bool zeroClear, u32 flags_ = 0);
// Memory Stack capable of expanding when full, but only if NonExpandable flag is not set. // Memory Stack capable of expanding when full, but only if NonExpandable flag is not set.
bool Init(usize size, bool zeroClear, u32 flags_ = 0, DqnMemAPI *const api = &DQN_DEFAULT_HEAP_ALLOCATOR); bool Init(usize size, bool zeroClear, u32 flags_ = 0, DqnMemAPI *const api = &DQN_DEFAULT_HEAP_ALLOCATOR);
@ -3255,10 +3265,12 @@ DqnMemAPI DqnMemAPI::StackAllocator(struct DqnMemStack *const stack)
// ================================================================================================= // =================================================================================================
void DqnAllocatorMetadata::Init(bool boundsGuard) void DqnAllocatorMetadata::Init(bool boundsGuard)
{ {
// TODO(doyle): How to handle memory here.
if (boundsGuard) if (boundsGuard)
{ {
this->boundsGuardSize = sizeof(GUARD_VALUE); this->boundsGuardSize = sizeof(HEAD_GUARD_VALUE);
DQN_ASSERT(this->allocations.InitSize(128)); LOCAL_PERSIST DqnMemAPI heap = DqnMemAPI::HeapAllocator();
DQN_ASSERT(this->allocations.InitSize(128, &heap));
} }
else else
{ {
@ -3292,20 +3304,22 @@ void DqnAllocatorMetadata::RemoveAllocation(u8 *ptr)
void DqnAllocatorMetadata::CheckAllocations() const void DqnAllocatorMetadata::CheckAllocations() const
{ {
for (u8 const *ptr : this->allocations) for (auto index = 0; index < this->allocations.count; index++)
{ {
u8 const *ptr = this->allocations.data[index];
u32 const *headGuard = this->PtrToHeadBoundsGuard(ptr); u32 const *headGuard = this->PtrToHeadBoundsGuard(ptr);
u32 const *tailGuard = this->PtrToTailBoundsGuard(ptr); u32 const *tailGuard = this->PtrToTailBoundsGuard(ptr);
DQN_ASSERTM(*headGuard == GUARD_VALUE, DQN_ASSERTM(*headGuard == HEAD_GUARD_VALUE,
"Bounds guard has been destroyed at the head end of the allocation! Expected: " "Bounds guard has been destroyed at the head end of the allocation! Expected: "
"%x, received: %x", "%x, received: %x",
GUARD_VALUE, *headGuard); HEAD_GUARD_VALUE, *headGuard);
DQN_ASSERTM(*tailGuard == GUARD_VALUE, DQN_ASSERTM(*tailGuard == TAIL_GUARD_VALUE,
"Bounds guard has been destroyed at the tail end of the allocation! Expected: " "Bounds guard has been destroyed at the tail end of the allocation! Expected: "
"%x, received: %x", "%x, received: %x",
GUARD_VALUE, *tailGuard); TAIL_GUARD_VALUE, *tailGuard);
} }
} }
@ -3388,7 +3402,7 @@ DQN_FILE_SCOPE DqnMemStack::Block *DqnMemStackInternal_AllocateBlock(usize size,
return result; return result;
} }
bool DqnMemStack::Init(u8 *const mem, usize size, u32 flags_) bool DqnMemStack::Init(void *const mem, usize size, bool zeroClear, u32 flags_)
{ {
if (!mem) if (!mem)
{ {
@ -3405,8 +3419,11 @@ bool DqnMemStack::Init(u8 *const mem, usize size, u32 flags_)
return false; return false;
} }
if (zeroClear)
DqnMem_Set(mem, 0, size);
this->block = (DqnMemStack::Block *)mem; this->block = (DqnMemStack::Block *)mem;
this->block->memory = mem + sizeof(DqnMemStack::Block); this->block->memory = (u8 *)mem + sizeof(DqnMemStack::Block);
this->block->used = 0; this->block->used = 0;
this->block->size = size - sizeof(DqnMemStack::Block); this->block->size = size - sizeof(DqnMemStack::Block);
this->block->prevBlock = nullptr; this->block->prevBlock = nullptr;
@ -3488,6 +3505,9 @@ void *DqnMemStack::Push(usize size, u8 alignment)
if (Dqn_BitIsSet(this->flags, Flag::NonExpandable)) if (Dqn_BitIsSet(this->flags, Flag::NonExpandable))
{ {
DQN_LOGE("Allocator is non-expandable and has run out of memory."); DQN_LOGE("Allocator is non-expandable and has run out of memory.");
if (Dqn_BitIsSet(this->flags, Flag::NonExpandableAssert))
DQN_ASSERT(DQN_INVALID_CODE_PATH);
return nullptr; return nullptr;
} }
@ -3532,8 +3552,8 @@ void *DqnMemStack::Push(usize size, u8 alignment)
{ {
auto *headGuard = myMetadata->PtrToHeadBoundsGuard(result); auto *headGuard = myMetadata->PtrToHeadBoundsGuard(result);
auto *tailGuard = myMetadata->PtrToTailBoundsGuard(result); auto *tailGuard = myMetadata->PtrToTailBoundsGuard(result);
*headGuard = DqnAllocatorMetadata::GUARD_VALUE; *headGuard = DqnAllocatorMetadata::HEAD_GUARD_VALUE;
*tailGuard = DqnAllocatorMetadata::GUARD_VALUE; *tailGuard = DqnAllocatorMetadata::TAIL_GUARD_VALUE;
} }
} }
@ -3555,6 +3575,22 @@ void *DqnMemStack::Push(usize size, u8 alignment)
return result; return result;
} }
FILE_SCOPE void DqnMemStackInternal_KillMetadataPtrsExistingInBlock(DqnAllocatorMetadata *metadata,
DqnMemStack::Block const *block)
{
u8 const *blockStart = block->memory;
u8 const *blockEnd = block->memory + block->size;
for (auto index = 0; index < metadata->allocations.count; index++)
{
u8 *ptr = metadata->allocations.data[index];
if (ptr >= blockStart && ptr <= blockEnd)
{
metadata->allocations.RemoveStable(index);
index--;
}
}
}
void DqnMemStack::Pop(void *const ptr, bool zeroClear) void DqnMemStack::Pop(void *const ptr, bool zeroClear)
{ {
if (!ptr) return; if (!ptr) return;
@ -3622,17 +3658,7 @@ bool DqnMemStack::FreeMemBlock(DqnMemStack::Block *memBlock)
if (Dqn_BitIsSet(this->flags, Flag::BoundsGuard)) if (Dqn_BitIsSet(this->flags, Flag::BoundsGuard))
{ {
u8 const *blockStart = blockToFree->memory; DqnMemStackInternal_KillMetadataPtrsExistingInBlock(&this->metadata, blockToFree);
u8 const *blockEnd = blockToFree->memory + blockToFree->size;
for (auto index = 0; index < this->metadata.allocations.count; index++)
{
u8 *ptr = this->metadata.allocations.data[index];
if (ptr >= blockStart && ptr <= blockEnd)
{
this->metadata.allocations.RemoveStable(index);
index--;
}
}
} }
usize realSize = blockToFree->size + sizeof(DqnMemStack::Block); usize realSize = blockToFree->size + sizeof(DqnMemStack::Block);
@ -3656,11 +3682,17 @@ void DqnMemStack::ClearCurrBlock(bool zeroClear)
{ {
if (this->block) if (this->block)
{ {
if (Dqn_BitIsSet(this->flags, Flag::BoundsGuard))
{
DqnMemStackInternal_KillMetadataPtrsExistingInBlock(&this->metadata, this->block);
}
this->block->used = 0; this->block->used = 0;
if (zeroClear) if (zeroClear)
{ {
DqnMem_Clear(this->block->memory, 0, this->block->size); DqnMem_Clear(this->block->memory, 0, this->block->size);
} }
} }
} }

View File

@ -2493,7 +2493,7 @@ FILE_SCOPE void DqnMemStack_Test()
{ {
u8 memBuf[sizeof(DqnMemStack::Block) - 1] = {}; u8 memBuf[sizeof(DqnMemStack::Block) - 1] = {};
DqnMemStack stack = {}; DqnMemStack stack = {};
auto result = stack.Init(&(memBuf[0]), DQN_ARRAY_COUNT(memBuf)); auto result = stack.Init(&(memBuf[0]), DQN_ARRAY_COUNT(memBuf), false);
DQN_ASSERT(result == false); DQN_ASSERT(result == false);
DQN_ASSERT(stack.block == nullptr); DQN_ASSERT(stack.block == nullptr);
@ -2506,7 +2506,7 @@ FILE_SCOPE void DqnMemStack_Test()
i32 const bufSize = sizeof(DqnMemStack::Block) * 5; i32 const bufSize = sizeof(DqnMemStack::Block) * 5;
u8 memBuf[bufSize] = {}; u8 memBuf[bufSize] = {};
DqnMemStack stack = {}; DqnMemStack stack = {};
auto result = stack.Init(memBuf, bufSize); auto result = stack.Init(&(memBuf[0]), bufSize, false);
DQN_ASSERT(result == true); DQN_ASSERT(result == true);
DQN_ASSERT(stack.block); DQN_ASSERT(stack.block);
@ -2594,8 +2594,8 @@ FILE_SCOPE void DqnMemStack_Test()
// TODO(doyle): check head and tail are adjacent to the bounds of the allocation // TODO(doyle): check head and tail are adjacent to the bounds of the allocation
u32 *head = stack.metadata.PtrToHeadBoundsGuard((u8 *)result); u32 *head = stack.metadata.PtrToHeadBoundsGuard((u8 *)result);
u32 *tail = stack.metadata.PtrToTailBoundsGuard((u8 *)result); u32 *tail = stack.metadata.PtrToTailBoundsGuard((u8 *)result);
DQN_ASSERT(*head == DqnAllocatorMetadata::GUARD_VALUE); DQN_ASSERT(*head == DqnAllocatorMetadata::HEAD_GUARD_VALUE);
DQN_ASSERT(*tail == DqnAllocatorMetadata::GUARD_VALUE); DQN_ASSERT(*tail == DqnAllocatorMetadata::TAIL_GUARD_VALUE);
Log(Status::Ok, "Bounds guards are placed adjacent and have magic values."); Log(Status::Ok, "Bounds guards are placed adjacent and have magic values.");
} }