From 9be5194b1727842cf2ff92ebee7c2b2f6c090527 Mon Sep 17 00:00:00 2001 From: Doyle Thai Date: Sat, 3 Feb 2018 21:39:15 +1100 Subject: [PATCH] Write more tests for push pop to tail --- dqn.h | 64 +++++++++++-------- dqn_unit_test.cpp | 153 +++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 183 insertions(+), 34 deletions(-) diff --git a/dqn.h b/dqn.h index d319807..dba7e00 100644 --- a/dqn.h +++ b/dqn.h @@ -457,15 +457,16 @@ struct DqnMemStack // Allocation API // ============================================================================================= - void *PushOnTail (isize size, u8 alignment = 4); // Allocate memory from the MemStack. // alignment: Ptr returned from allocator is aligned to this value and MUST be power of 2. // return: nullptr if out of space OR stack is using fixed memory/size OR stack full and platform malloc fails. void *Push (isize size, u8 alignment = 4); + void *PushOnTail (isize size, u8 alignment = 4); // Frees the given ptr. It MUST be the last allocated item in the stack, asserts otherwise. void Pop (void *const ptr, bool zeroClear = false); + void PopOnTail (void *const ptr, bool zeroClear = false); // Frees all blocks belonging to this stack. void Free (); @@ -3194,7 +3195,6 @@ FILE_SCOPE u8 *DqnMemAPIInternal_StackAllocatorCallback(DqnMemAPI *api, DqnMemAP if (enoughSpace) { stack->Pop(ptr, false); - result = (u8 *)stack->Push(request->newSize, alignment); DQN_ASSERT(stack->block == block && result == request->oldMemPtr); success = true; @@ -3208,7 +3208,7 @@ FILE_SCOPE u8 *DqnMemAPIInternal_StackAllocatorCallback(DqnMemAPI *api, DqnMemAP enoughSpace = (block->tail - extraBytesReq) > block->head; if (enoughSpace) { - stack->Pop(ptr, false); + stack->PopOnTail(ptr, false); result = (u8 *)stack->Push(request->newSize, alignment); DqnMem_Copy(result, ptr, oldMemSize); result[oldMemSize] = 0; @@ -3243,7 +3243,9 @@ FILE_SCOPE u8 *DqnMemAPIInternal_StackAllocatorCallback(DqnMemAPI *api, DqnMemAP // Switch to old block, pop the ptr and return the new block on top. auto *newBlock = stack->block; stack->block = oldBlock; - stack->Pop(ptr, false); + + if (type == PtrType::Head) stack->Pop(ptr, false); + else stack->PopOnTail(ptr, false); stack->block = newBlock; success = true; } @@ -3292,7 +3294,9 @@ FILE_SCOPE u8 *DqnMemAPIInternal_StackAllocatorCallback(DqnMemAPI *api, DqnMemAP if (PtrIsLastAllocationInBlock(&stack->metadata, block, ptr)) { - stack->Pop(ptr, false); + PtrType type = ClassifyPtr(block, ptr); + if (type == PtrType::Head) stack->Pop(ptr, false); + else stack->PopOnTail(ptr, false); } else { @@ -3709,16 +3713,17 @@ FILE_SCOPE void DqnMemStackInternal_KillMetadataPtrsExistingInBlock(DqnAllocator } } -void DqnMemStack::Pop(void *const ptr, bool zeroClear) + +FILE_SCOPE void DqnMemStackInternal_Pop(DqnMemStack *stack, void *const ptr, bool zeroClear, bool popHead) { if (!ptr) return; - DQN_ASSERT(this->block); + DQN_ASSERT(stack->block); u8 *const bytePtr = (u8 *)ptr; - DqnAllocatorMetadata *myMetadata = &this->metadata; + DqnAllocatorMetadata *myMetadata = &stack->metadata; // Check instrumented data - if (Dqn_BitIsSet(this->flags, Flag::BoundsGuard)) + if (Dqn_BitIsSet(stack->flags, DqnMemStack::Flag::BoundsGuard)) { myMetadata->CheckAllocations(); myMetadata->RemoveAllocation(bytePtr); @@ -3731,30 +3736,41 @@ void DqnMemStack::Pop(void *const ptr, bool zeroClear) isize actualSize = myMetadata->GetAllocationSize(size, alignment); u8 *const start = bytePtr - offsetToSrc; u8 *const end = start + actualSize; - u8 const *const blockEnd = this->block->memory + this->block->size; + u8 const *const blockEnd = stack->block->memory + stack->block->size; - if (bytePtr >= this->block->memory && bytePtr < this->block->head) + if (popHead) { - DQN_ASSERTM(end == this->block->head, "Pointer to pop was not the last allocation! %p != %p", end, this->block->head); - - this->block->head -= actualSize; - DQN_ASSERT(this->block->head >= this->block->memory); - } - else if (bytePtr >= this->block->tail && bytePtr < blockEnd) - { - DQN_ASSERTM(start == this->block->tail, "Pointer to pop was not the last allocation! %p != %p", start, - this->block->tail); - - this->block->tail += actualSize; - DQN_ASSERT(this->block->tail <= blockEnd); + DQN_ASSERTM(end == stack->block->head, "Pointer to pop was not the last allocation! %p != %p", end, stack->block->head); + stack->block->head -= actualSize; + DQN_ASSERT(stack->block->head >= stack->block->memory); } else { - DQN_ASSERTM(DQN_INVALID_CODE_PATH, "Pointer to free does not belong to current block!"); + DQN_ASSERTM(start == stack->block->tail, "Pointer to pop was not the last allocation! %p != %p", start, stack->block->tail); + stack->block->tail += actualSize; + DQN_ASSERT(stack->block->tail <= blockEnd); } if (zeroClear) DqnMem_Set(start, 0, end - start); + + if (stack->block->tail == blockEnd && stack->block->head == stack->block->memory) + { + if (stack->block->prevBlock) + { + stack->FreeLastBlock(); + } + } +} + +void DqnMemStack::Pop(void *const ptr, bool zeroClear) +{ + DqnMemStackInternal_Pop(this, ptr, zeroClear, true); +} + +void DqnMemStack::PopOnTail(void *const ptr, bool zeroClear) +{ + DqnMemStackInternal_Pop(this, ptr, zeroClear, false); } void DqnMemStack::Free() diff --git a/dqn_unit_test.cpp b/dqn_unit_test.cpp index 8c45d9f..4eb9960 100644 --- a/dqn_unit_test.cpp +++ b/dqn_unit_test.cpp @@ -2426,12 +2426,12 @@ FILE_SCOPE void DqnMemStack_Test() // Temporary Regions if (1) { - DqnMemStack stack = {}; - DQN_ASSERT(stack.Init(DQN_MEGABYTE(1), true)); - // Check temporary regions if (1) { + DqnMemStack stack = {}; + DQN_ASSERT(stack.Init(DQN_MEGABYTE(1), true, DqnMemStack::Flag::BoundsGuard)); + DqnMemStack::Block *blockToReturnTo = stack.block; auto headBefore = blockToReturnTo->head; auto tailBefore = blockToReturnTo->tail; @@ -2456,11 +2456,16 @@ FILE_SCOPE void DqnMemStack_Test() DQN_ASSERT(stack.block == blockToReturnTo); DQN_ASSERT(stack.block->head == headBefore); DQN_ASSERT(stack.block->tail == tailBefore); + + stack.Free(); } // Check temporary regions keep state if (1) { + DqnMemStack stack = {}; + DQN_ASSERT(stack.Init(DQN_MEGABYTE(1), true, DqnMemStack::Flag::BoundsGuard)); + DqnMemStack::Block *blockToReturnTo = stack.block; auto headBefore = blockToReturnTo->head; auto tailBefore = blockToReturnTo->tail; @@ -2486,9 +2491,55 @@ FILE_SCOPE void DqnMemStack_Test() DQN_ASSERT(stack.block != blockToReturnTo); DQN_ASSERT(stack.block->prevBlock == blockToReturnTo); DQN_ASSERT(stack.tempRegionCount == 0); + + stack.Free(); + } + + // Check temporary regions with tail and head pushes + if (1) + { + DqnMemStack stack = {}; + DQN_ASSERT(stack.Init(DQN_MEGABYTE(1), true, DqnMemStack::Flag::BoundsGuard)); + + auto *pop1 = stack.Push(222); + auto *pop2 = stack.PushOnTail(333); + + DqnMemStack::Block *blockToReturnTo = stack.block; + auto headBefore = blockToReturnTo->head; + auto tailBefore = blockToReturnTo->tail; + if (1) + { + auto memGuard1 = stack.TempRegionGuard(); + auto *result2 = stack.Push(100); + auto *result3 = stack.PushOnTail(100); + auto *result4 = stack.Push(100); + auto *result5 = stack.PushOnTail(100); + DQN_ASSERT(result2 && result3 && result4 && result5); + DQN_ASSERT(result3 > result5); + DQN_ASSERT(result2 < result4); + DQN_ASSERT(stack.block->head > headBefore && stack.block->head < stack.block->tail); + DQN_ASSERT(stack.block->tail >= stack.block->head && stack.block->tail < (stack.block->memory + stack.block->size)); + DQN_ASSERT(stack.block->memory == blockToReturnTo->memory); + + // Force allocation of new block + auto *result6 = stack.Push(DQN_MEGABYTE(5)); + DQN_ASSERT(result6); + DQN_ASSERT(stack.block != blockToReturnTo); + DQN_ASSERT(stack.tempRegionCount == 1); + } + + DQN_ASSERT(stack.block == blockToReturnTo); + DQN_ASSERT(stack.block->head == headBefore); + DQN_ASSERT(stack.block->tail == tailBefore); + + stack.Pop(pop1); + stack.PopOnTail(pop2); + DQN_ASSERT(stack.block->head == stack.block->memory); + DQN_ASSERT(stack.block->tail == stack.block->memory + stack.block->size); + + stack.Free(); } Log(Status::Ok, "Temporary regions return state and/or keep changes if requested."); - stack.Free(); } // Check Fixed Mem Init @@ -2607,15 +2658,97 @@ FILE_SCOPE void DqnMemStack_Test() Log(Status::Ok, "Bounds guards are placed adjacent and have magic values."); } - // Push to tail and head if (1) { - DqnMemStack stack = {}; - DQN_ASSERT(stack.Init(DQN_MEGABYTE(1), true, DqnMemStack::Flag::BoundsGuard)); + // Push to tail and head + if (1) + { + DqnMemStack stack = {}; + DQN_ASSERT(stack.Init(DQN_MEGABYTE(1), true, DqnMemStack::Flag::BoundsGuard)); - auto *result1 = stack.Push(100); - auto *result2 = stack.PushOnTail(100); - stack.Free(); + auto *result1 = stack.Push(100); + auto *result2 = stack.PushOnTail(100); + auto *headBefore = stack.block->head; + auto *tailBefore = stack.block->tail; + DQN_ASSERT(result2 && result1); + DQN_ASSERT(result2 != result1 && result1 < result2); + + stack.PopOnTail(result2); + DQN_ASSERT(headBefore == stack.block->head) + DQN_ASSERT(tailBefore != stack.block->tail) + + stack.Pop(result1); + DQN_ASSERT(stack.block->prevBlock == false); + DQN_ASSERT(stack.block->head == stack.block->memory); + DQN_ASSERT(stack.block->tail == stack.block->memory + stack.block->size); + stack.Free(); + Log(Status::Ok, "Push, pop to tail and head."); + } + + // Expansion with tail + if (1) + { + // Push too much to tail causes expansion + if (1) + { + DqnMemStack stack = {}; + DQN_ASSERT(stack.Init(DQN_MEGABYTE(1), true, DqnMemStack::Flag::BoundsGuard)); + + auto *result1 = stack.Push(100); + DQN_ASSERT(stack.block->prevBlock == nullptr); + DQN_ASSERT(stack.block->head > stack.block->memory && stack.block->head < stack.block->tail); + DQN_ASSERT(stack.block->tail == stack.block->memory + stack.block->size); + auto *blockBefore = stack.block; + + auto *result2 = stack.PushOnTail(DQN_MEGABYTE(1)); + DQN_ASSERT(result2 && result1); + DQN_ASSERT(result2 != result1); + DQN_ASSERT(stack.block->prevBlock == blockBefore); + DQN_ASSERT(stack.block != blockBefore); + + DQN_ASSERT(stack.block->head == stack.block->memory); + DQN_ASSERT(stack.block->tail < stack.block->memory + stack.block->size && + stack.block->tail >= stack.block->head); + + stack.PopOnTail(result2); + DQN_ASSERT(blockBefore == stack.block); + + stack.Pop(result1); + DQN_ASSERT(blockBefore == stack.block); + + stack.Free(); + } + + // Push too much to tail fails to expand when non expandable + if (1) + { + DqnMemStack stack = {}; + DQN_ASSERT(stack.Init(DQN_MEGABYTE(1), true, DqnMemStack::Flag::NonExpandable)); + + auto *result1 = stack.Push(100); + DQN_ASSERT(stack.block->prevBlock == nullptr); + DQN_ASSERT(stack.block->head != stack.block->memory); + DQN_ASSERT(stack.block->tail == stack.block->memory + stack.block->size); + auto *blockBefore = stack.block; + + auto *result2 = stack.PushOnTail(DQN_MEGABYTE(1)); + DQN_ASSERT(result2 == nullptr); + DQN_ASSERT(stack.block->prevBlock == nullptr); + DQN_ASSERT(stack.block == blockBefore); + DQN_ASSERT(stack.block->head > stack.block->memory && stack.block->head < stack.block->tail); + DQN_ASSERT(stack.block->tail == stack.block->memory + stack.block->size); + + stack.PopOnTail(result2); + DQN_ASSERT(blockBefore == stack.block); + + stack.Pop(result1); + DQN_ASSERT(blockBefore == stack.block); + + stack.Free(); + } + + Log(Status::Ok, "Non-Expanding and expanding stack with tail push."); + } } }