Feature crashdump #1

slurmlord · 2025-12-01T15:47:09Z

Test

Copilot

Pull request overview

This PR adds comprehensive crash dump functionality to both Command & Conquer: Generals and Zero Hour. The feature creates minidumps on unhandled exceptions and release crashes, capturing both minimal and extended memory information to aid in debugging crashes in production builds.

Implements MiniDumper class with support for three dump types (minimal, game memory, and full)
Integrates crash dump triggering into existing exception handlers and crash paths
Adds iterator infrastructure to dump GameMemory allocations for detailed crash analysis

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 18 comments.

Show a summary per file

File	Description
cmake/config-memory.cmake	Adds build configuration option for crash dump feature
Core/Libraries/Source/WWVegas/WWLib/DbgHelpLoader_minidump.h	Backports Windows minidump API definitions for VC6 compatibility
Core/Libraries/Source/WWVegas/WWLib/DbgHelpLoader.h	Adds MiniDumpWriteDump function wrapper to debug help loader
Core/Libraries/Source/WWVegas/WWLib/DbgHelpLoader.cpp	Implements loading of MiniDumpWriteDump from dbghelp.dll
Core/Libraries/Source/WWVegas/WWLib/CMakeLists.txt	Adds minidump header to build
Core/GameEngine/Include/Common/MiniDumper.h	Defines MiniDumper class interface and dump types
Core/GameEngine/Include/Common/GameMemory.h	Adds AllocationRangeIterator for iterating memory allocations
Core/GameEngine/Source/Common/System/MiniDumper.cpp	Implements crash dump creation with memory pool introspection
Core/GameEngine/Source/Common/System/GameMemory.cpp	Implements iterator and exposes memory pool internals for dumping
Core/GameEngine/Source/Common/System/Debug.cpp	Integrates crash dumping into ReleaseCrash functions
Core/GameEngine/CMakeLists.txt	Adds MiniDumper files to build
GeneralsMD/Code/Main/WinMain.cpp	Initializes MiniDumper and triggers dumps on exceptions
Generals/Code/Main/WinMain.cpp	Initializes MiniDumper and triggers dumps on exceptions

Copilot · 2025-12-01T15:54:30Z

Core/GameEngine/Source/Common/System/MiniDumper.cpp

+void MiniDumper::initMiniDumper(const AsciiString& userDirPath)
+{
+	DEBUG_ASSERTCRASH(TheMiniDumper == NULL, ("MiniDumper::initMiniDumper called on already created instance"));
+
+	// Use placement new on the process heap so TheMiniDumper is placed outside the MemoryPoolFactory managed area.
+	// If the crash is due to corrupted MemoryPoolFactory structures, try to mitigate the chances of MiniDumper memory also being corrupted
+	TheMiniDumper = new (::HeapAlloc(::GetProcessHeap(), HEAP_GENERATE_EXCEPTIONS, sizeof(MiniDumper))) MiniDumper;
+	TheMiniDumper->Initialize(userDirPath);
+}


According to the project guidelines, every user-facing change requires a TheSuperHackers comment with the format:

// TheSuperHackers @feature author DD/MM/YYYY Description

This new crash dump functionality is a significant user-facing feature and should be documented with this comment format.

Copilot · 2025-12-01T15:54:30Z

Core/GameEngine/Include/Common/GameMemory.h

+		m_currentBlobInPool = NULL;
+		m_factory = NULL;
+		m_range = MemoryPoolAllocatedRange();
+	};


The default constructor at line 529 also has an unnecessary semicolon after the closing brace on line 535. Remove it for consistency with C++ style.

Suggested change

};

}

Copilot · 2025-12-01T15:54:30Z

Core/GameEngine/Include/Common/MiniDumper.h

+{
+	// Smallest dump type with call stacks and some supporting variables
+	DUMP_TYPE_MINIMAL = 'M',
+	// Large dump including all memory regions allocated by the GameMemory implementaion


Spelling error: "implementaion" should be "implementation".

Suggested change

// Large dump including all memory regions allocated by the GameMemory implementaion

// Large dump including all memory regions allocated by the GameMemory implementation

Copilot · 2025-12-01T15:54:31Z

Core/GameEngine/Source/Common/System/GameMemory.cpp

+void AllocationRangeIterator::MoveToNextBlob()
+{
+	// Advances to the next blob, advancing to the next MemoryPool if needed.
+	m_currentBlobInPool = m_currentBlobInPool->getNextInList();
+	if (m_currentBlobInPool != NULL)
+	{
+		return;
+	}
+	do
+	{
+		m_currentPool = m_currentPool->getNextPoolInList();
+	} while (m_currentPool != NULL && m_currentPool->m_firstBlob == NULL);
+
+	if (m_currentPool != NULL)
+	{
+		m_currentBlobInPool = m_currentPool->m_firstBlob;
+	}
+	else
+	{
+		m_currentBlobInPool = NULL;
+	}
+}


The MoveToNextBlob method doesn't check if m_currentBlobInPool is NULL before calling getNextInList() on line 3292. If the iterator is already at the end or in an invalid state, this will cause a crash. Add a NULL check.

Copilot · 2025-12-01T15:54:31Z

cmake/config-memory.cmake

 add_feature_info(MemoryPoolDebugIntenseDmaBookkeeping RTS_MEMORYPOOL_DEBUG_INTENSE_DMA_BOOKKEEPING "Build with Memory Pool intense DMA bookkeeping")

+# Memory dump features
+add_feature_info(CrashDumpEnable "RTS_CRASHDUMP_ENABLE" "Build with Crash Dumps")


The first parameter to add_feature_info should be the feature name without quotes. The second parameter should be the variable name. According to CMake documentation, the correct syntax is:

add_feature_info(CrashDumpEnable RTS_CRASHDUMP_ENABLE "Build with Crash Dumps")

This is consistent with all other add_feature_info calls in this file (lines 27-41).

Suggested change

add_feature_info(CrashDumpEnable "RTS_CRASHDUMP_ENABLE" "Build with Crash Dumps")

add_feature_info(CrashDumpEnable RTS_CRASHDUMP_ENABLE "Build with Crash Dumps")

Copilot · 2025-12-01T15:54:34Z

GeneralsMD/Code/Main/WinMain.cpp

+#ifdef RTS_ENABLE_CRASHDUMP
+	MiniDumper::shutdownMiniDumper();
+#endif


The MiniDumper shutdown is placed after the catch block but before the critical section cleanup. If an exception occurs during the main game loop (caught by the catch-all at line 931), the minidump facilities may try to access already-destroyed memory pools during shutdown. Consider moving the shutdownMiniDumper() call to before the catch block or ensure it handles this scenario gracefully.

Copilot · 2025-12-01T15:54:34Z

Generals/Code/Main/WinMain.cpp

+#ifdef RTS_ENABLE_CRASHDUMP
+	MiniDumper::shutdownMiniDumper();
+#endif


The MiniDumper shutdown is placed after the catch block but before the critical section cleanup. If an exception occurs during the main game loop (caught by the catch-all at line 881), the minidump facilities may try to access already-destroyed memory pools during shutdown. Consider moving the shutdownMiniDumper() call to before the catch block or ensure it handles this scenario gracefully.

Copilot · 2025-12-01T15:54:34Z

Core/GameEngine/Source/Common/System/GameMemory.cpp

+{
+	m_factory = factory;
+	m_currentPool = factory->m_firstPoolInFactory;
+	m_currentBlobInPool = m_currentPool->m_firstBlob;


The AllocationRangeIterator constructor doesn't check if m_currentPool is NULL before dereferencing it on line 3280. If factory->m_firstPoolInFactory is NULL (which can happen if the factory has no pools), this will cause a crash. Add appropriate NULL checks.

Suggested change

m_currentBlobInPool = m_currentPool->m_firstBlob;

if (m_currentPool != NULL)

{

m_currentBlobInPool = m_currentPool->m_firstBlob;

}

else

{

m_currentBlobInPool = NULL;

}

Copilot · 2025-12-01T15:54:35Z

Core/GameEngine/Source/Common/System/GameMemory.cpp

+void AllocationRangeIterator::UpdateRange()
+{
+	m_currentBlobInPool->fillAllocatedRange(m_range);


The UpdateRange method doesn't check if m_currentBlobInPool is NULL before dereferencing it. This could cause a crash if the iterator has moved past the end or if it's in an invalid state. Add a NULL check or assert.

Suggested change

void AllocationRangeIterator::UpdateRange()

{

m_currentBlobInPool->fillAllocatedRange(m_range);

// TheSuperHackers @bugfix agent 07/06/2024 Add NULL check and assertion to prevent crash if m_currentBlobInPool is NULL

void AllocationRangeIterator::UpdateRange()

{

ASSERT(m_currentBlobInPool != NULL); // Defensive: catch in debug builds

if (m_currentBlobInPool != NULL)

{

m_currentBlobInPool->fillAllocatedRange(m_range);

}

else

{

m_range = MemoryPoolAllocatedRange(); // Set to default/empty range

}

Copilot · 2025-12-01T15:54:35Z

Core/GameEngine/Source/Common/System/MiniDumper.cpp

+	// Use placement new on the process heap so TheMiniDumper is placed outside the MemoryPoolFactory managed area.
+	// If the crash is due to corrupted MemoryPoolFactory structures, try to mitigate the chances of MiniDumper memory also being corrupted
+	TheMiniDumper = new (::HeapAlloc(::GetProcessHeap(), HEAP_GENERATE_EXCEPTIONS, sizeof(MiniDumper))) MiniDumper;
+	TheMiniDumper->Initialize(userDirPath);


If HeapAlloc fails with HEAP_GENERATE_EXCEPTIONS flag, it will raise an exception rather than returning NULL. However, if the Initialize call on line 48 fails (by returning early), TheMiniDumper will be left in a partially initialized state without any cleanup. Consider wrapping the initialization in a try-catch block or checking if initialization succeeded and cleaning up on failure.

Suggested change

TheMiniDumper->Initialize(userDirPath);

if (!TheMiniDumper->Initialize(userDirPath))

{

// TheSuperHackers @bugfix agent 08/06/2024 Clean up MiniDumper if initialization fails

TheMiniDumper->~MiniDumper();

::HeapFree(::GetProcessHeap(), NULL, TheMiniDumper);

TheMiniDumper = NULL;

}

…entations to cpp

slurmlord added 4 commits November 26, 2025 19:31

feat(crashdump): Add crash dump functionality

a241b80

Refactor CallbackInternal and DumpMemoryObjects

5e543ba

Improve iteration in DumpMemoryObjects

7399cce

Make state enums private, init all meory objs in DumpObjectsState_Begin

7cc9b3c

slurmlord requested a review from Copilot December 1, 2025 15:47

Copilot started reviewing on behalf of slurmlord December 1, 2025 15:47 View session

Copilot finished reviewing on behalf of slurmlord December 1, 2025 15:51

Copilot AI reviewed Dec 1, 2025

View reviewed changes

Include enum name in enum values, move AllocationRangeIterator implem…

bcb86fe

…entations to cpp

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Feature crashdump #1

Feature crashdump #1

Uh oh!

slurmlord commented Dec 1, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Dec 1, 2025

Uh oh!

Copilot AI Dec 1, 2025

Uh oh!

Copilot AI Dec 1, 2025

Uh oh!

Copilot AI Dec 1, 2025

Uh oh!

Copilot AI Dec 1, 2025

Uh oh!

Copilot AI Dec 1, 2025

Uh oh!

Copilot AI Dec 1, 2025

Uh oh!

Copilot AI Dec 1, 2025

Uh oh!

Copilot AI Dec 1, 2025

Uh oh!

Copilot AI Dec 1, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	// Large dump including all memory regions allocated by the GameMemory implementaion
	// Large dump including all memory regions allocated by the GameMemory implementation

	add_feature_info(CrashDumpEnable "RTS_CRASHDUMP_ENABLE" "Build with Crash Dumps")
	add_feature_info(CrashDumpEnable RTS_CRASHDUMP_ENABLE "Build with Crash Dumps")

-	TheMiniDumper->Initialize(userDirPath);
+	if (!TheMiniDumper->Initialize(userDirPath))
+	{
+		// TheSuperHackers @bugfix agent 08/06/2024 Clean up MiniDumper if initialization fails
+		TheMiniDumper->~MiniDumper();
+		::HeapFree(::GetProcessHeap(), NULL, TheMiniDumper);
+		TheMiniDumper = NULL;
+	}

Feature crashdump #1

Are you sure you want to change the base?

Feature crashdump #1

Uh oh!

Conversation

slurmlord commented Dec 1, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants