From 2bce9ef257d3ef35f64c37bcda5135bdd1a67eaa Mon Sep 17 00:00:00 2001 From: IbrahimAlzaidi Date: Sun, 24 May 2026 17:12:50 +0200 Subject: [PATCH] bugfix(memory): Harden memory manager edge cases Prevent MemoryPool::releaseEmpties() from freeing the final blob so fixed-size pools remain allocatable after cleanup. Parse MemoryPools.ini override lines with bounded token handling so malformed or overlong pool names cannot overflow the local buffer or alter defaults accidentally. Add sized scalar and array delete overloads for both real and null memory manager builds so VS2022/C++20 sized deallocation paths free through the same allocator routes as existing unsized deletes. Validation: openspec validate memory-manager-hardening --type change --strict; Release z_gameengine builds for real and null memory-manager configurations. --- Core/GameEngine/Include/Common/GameMemory.h | 4 ++ .../Source/Common/System/GameMemory.cpp | 32 +++++++++- .../Source/Common/System/GameMemoryInit.cpp | 59 +++++++++++++++++-- .../Source/Common/System/GameMemoryNull.cpp | 12 ++++ 4 files changed, 102 insertions(+), 5 deletions(-) diff --git a/Core/GameEngine/Include/Common/GameMemory.h b/Core/GameEngine/Include/Common/GameMemory.h index 53c0e59619d..52e5d3e894f 100644 --- a/Core/GameEngine/Include/Common/GameMemory.h +++ b/Core/GameEngine/Include/Common/GameMemory.h @@ -837,6 +837,10 @@ extern void userMemoryManagerGetDmaParms(Int *numSubPools, const PoolInitRec **p */ extern void userMemoryManagerInitPools(); +#ifdef MEMORY_MANAGER_HARDENING_TESTS +extern Bool userMemoryManagerParsePoolOverrideLineForTest(const char* line, char* poolName, size_t poolNameSize, Int* initial, Int* overflow); +#endif + /** This function is declared in this header, but is not defined anywhere -- you must provide it in your code. It is called by createMemoryPool to adjust the allocation size(s) for a diff --git a/Core/GameEngine/Source/Common/System/GameMemory.cpp b/Core/GameEngine/Source/Common/System/GameMemory.cpp index 5c70904eb95..32b63b5586f 100644 --- a/Core/GameEngine/Source/Common/System/GameMemory.cpp +++ b/Core/GameEngine/Source/Common/System/GameMemory.cpp @@ -1775,12 +1775,16 @@ Int MemoryPool::releaseEmpties() ScopedCriticalSection scopedCriticalSection(TheMemoryPoolCriticalSection); Int released = 0; + Int blobCount = countBlobsInPool(); for (MemoryPoolBlob* blob = m_firstBlob; blob;) { MemoryPoolBlob* pNext = blob->getNextInList(); - if (blob->getUsedBlockCount() == 0) + if (blob->getUsedBlockCount() == 0 && blobCount > 1) + { released += freeBlob(blob); + --blobCount; + } blob = pNext; } return released; @@ -3292,6 +3296,19 @@ void operator delete(void *p) TheDynamicMemoryAllocator->freeBytes(p); } +//----------------------------------------------------------------------------- +/** + overload for sized global operator delete; send requests to TheDynamicMemoryAllocator. +*/ +void operator delete(void *p, size_t size) +{ + (void)size; + ++theLinkTester; + preMainInitMemoryManager(); + DEBUG_ASSERTCRASH(TheDynamicMemoryAllocator != nullptr, ("must init memory manager before calling global operator delete")); + TheDynamicMemoryAllocator->freeBytes(p); +} + //----------------------------------------------------------------------------- /** overload for global operator delete[]; send requests to TheDynamicMemoryAllocator. @@ -3304,6 +3321,19 @@ void operator delete[](void *p) TheDynamicMemoryAllocator->freeBytes(p); } +//----------------------------------------------------------------------------- +/** + overload for sized global operator delete[]; send requests to TheDynamicMemoryAllocator. +*/ +void operator delete[](void *p, size_t size) +{ + (void)size; + ++theLinkTester; + preMainInitMemoryManager(); + DEBUG_ASSERTCRASH(TheDynamicMemoryAllocator != nullptr, ("must init memory manager before calling global operator delete")); + TheDynamicMemoryAllocator->freeBytes(p); +} + //----------------------------------------------------------------------------- /** overload for global operator new (MFC debug version); send requests to TheDynamicMemoryAllocator. diff --git a/Core/GameEngine/Source/Common/System/GameMemoryInit.cpp b/Core/GameEngine/Source/Common/System/GameMemoryInit.cpp index 6bdd7e65706..85e50d51665 100644 --- a/Core/GameEngine/Source/Common/System/GameMemoryInit.cpp +++ b/Core/GameEngine/Source/Common/System/GameMemoryInit.cpp @@ -44,6 +44,9 @@ #include "PreRTS.h" // This must go first in EVERY cpp file in the GameEngine // SYSTEM INCLUDES +#include +#include +#include // USER INCLUDES #include "Lib/BaseType.h" @@ -101,6 +104,56 @@ static Int roundUpMemBound(Int i) return (i + (MEM_BOUND_ALIGNMENT-1)) & ~(MEM_BOUND_ALIGNMENT-1); } +//----------------------------------------------------------------------------- +static Bool parseMemoryPoolOverrideLine(const char* line, char* poolName, size_t poolNameSize, Int* initial, Int* overflow) +{ + if (line == nullptr || poolName == nullptr || poolNameSize == 0 || initial == nullptr || overflow == nullptr) + return false; + + if (line[0] == ';') + return false; + + const char* cursor = line; + while (isspace(static_cast(*cursor))) + ++cursor; + + if (*cursor == '\0') + return false; + + const char* poolNameBegin = cursor; + while (*cursor != '\0' && !isspace(static_cast(*cursor))) + ++cursor; + + const size_t poolNameLength = cursor - poolNameBegin; + if (poolNameLength == 0 || poolNameLength >= poolNameSize) + return false; + + char* end = nullptr; + errno = 0; + const long parsedInitial = strtol(cursor, &end, 10); + if (end == cursor || errno == ERANGE) + return false; + + cursor = end; + errno = 0; + const long parsedOverflow = strtol(cursor, &end, 10); + if (end == cursor || errno == ERANGE) + return false; + + memcpy(poolName, poolNameBegin, poolNameLength); + poolName[poolNameLength] = '\0'; + *initial = static_cast(parsedInitial); + *overflow = static_cast(parsedOverflow); + return true; +} + +#ifdef MEMORY_MANAGER_HARDENING_TESTS +Bool userMemoryManagerParsePoolOverrideLineForTest(const char* line, char* poolName, size_t poolNameSize, Int* initial, Int* overflow) +{ + return parseMemoryPoolOverrideLine(line, poolName, poolNameSize, initial, overflow); +} +#endif + //----------------------------------------------------------------------------- void userMemoryManagerInitPools() { @@ -123,12 +176,10 @@ void userMemoryManagerInitPools() if (fp) { char poolName[256]; - int initial, overflow; + Int initial, overflow; while (fgets(buf, _MAX_PATH, fp)) { - if (buf[0] == ';') - continue; - if (sscanf(buf, "%s %d %d", poolName, &initial, &overflow ) == 3) + if (parseMemoryPoolOverrideLine(buf, poolName, ARRAY_SIZE(poolName), &initial, &overflow)) { for (PoolSizeRec* p = PoolSizes; p->name != nullptr; ++p) { diff --git a/Core/GameEngine/Source/Common/System/GameMemoryNull.cpp b/Core/GameEngine/Source/Common/System/GameMemoryNull.cpp index 54f8fd770fe..1a34365467d 100644 --- a/Core/GameEngine/Source/Common/System/GameMemoryNull.cpp +++ b/Core/GameEngine/Source/Common/System/GameMemoryNull.cpp @@ -172,6 +172,12 @@ extern void __cdecl operator delete(void *p) free(p); } +extern void __cdecl operator delete(void *p, size_t size) +{ + (void)size; + free(p); +} + extern void * __cdecl operator new[](size_t size) { void *p = malloc(size); @@ -186,6 +192,12 @@ extern void __cdecl operator delete[](void *p) free(p); } +extern void __cdecl operator delete[](void *p, size_t size) +{ + (void)size; + free(p); +} + // additional overloads to account for VC/MFC funky versions extern void* __cdecl operator new(size_t size, const char *, int) {