Skip to content

Commit 5c9f4b8

Browse files
Dutchman101Dutchman101
committed
Crash handler improvements (Includes: don't try to symbolize stack traces on non-dev builds)
1 parent 6173c2f commit 5c9f4b8

File tree

6 files changed

+736
-163
lines changed

6 files changed

+736
-163
lines changed

‎Client/core/CCrashDumpWriter.cpp‎

Lines changed: 54 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
*****************************************************************************/
1111

1212
#include "StdInc.h"
13+
#include "StackTraceHelpers.h"
1314
#include <SharedUtil.Misc.h>
1415
#include <game/CGame.h>
1516
#include <game/CPools.h>
@@ -31,6 +32,7 @@
3132
#include <ctime>
3233
#include <optional>
3334
#include <utility>
35+
#include <mutex>
3436

3537
static constexpr DWORD CRASH_EXIT_CODE = 3;
3638
static constexpr std::size_t LOG_EVENT_SIZE = 200;
@@ -118,21 +120,35 @@ namespace
118120
static std::atomic_flag configured = ATOMIC_FLAG_INIT;
119121
if (!configured.test_and_set(std::memory_order_acq_rel))
120122
{
121-
SymSetOptions(SYMOPT_LOAD_LINES | SYMOPT_UNDNAME | SYMOPT_FAIL_CRITICAL_ERRORS);
123+
SymSetOptions(SYMOPT_LOAD_LINES | SYMOPT_UNDNAME | SYMOPT_FAIL_CRITICAL_ERRORS | SYMOPT_DEFERRED_LOADS);
122124
}
123125
}
126+
127+
std::mutex& GetSymInitMutex() noexcept
128+
{
129+
static std::mutex symMutex;
130+
return symMutex;
131+
}
124132
} // namespace
125133

126134
class SymbolHandlerGuard
127135
{
128136
public:
129-
explicit SymbolHandlerGuard(HANDLE process) noexcept : m_process(process), m_initialized(false)
137+
explicit SymbolHandlerGuard(HANDLE process, bool enableSymbols) noexcept : m_process(process), m_initialized(false)
130138
{
139+
if (!enableSymbols)
140+
return;
141+
131142
if (m_process != nullptr)
132143
{
144+
std::lock_guard<std::mutex> lock{GetSymInitMutex()};
145+
133146
ConfigureDbgHelpOptions();
147+
148+
const SString& processDir = SharedUtil::GetProcessBaseDir();
149+
const char* searchPath = processDir.empty() ? nullptr : processDir.c_str();
134150

135-
if (SymInitialize(m_process, nullptr, TRUE) != FALSE)
151+
if (SymInitialize(m_process, searchPath, TRUE) != FALSE)
136152
m_initialized = true;
137153
}
138154
}
@@ -516,6 +532,15 @@ static void AppendCrashDiagnostics(const SString& text)
516532
if (pException == nullptr || pException->ContextRecord == nullptr)
517533
return false;
518534

535+
const bool hasSymbols = CrashHandler::ProcessHasLocalDebugSymbols();
536+
if (!hasSymbols)
537+
{
538+
static std::once_flag logOnce;
539+
std::call_once(logOnce, [] {
540+
SAFE_DEBUG_OUTPUT("CaptureStackTraceText: capturing without symbols (raw addresses only)\n");
541+
});
542+
}
543+
519544
// For callback exceptions (0xC000041D), context and stack may be unreliable
520545
const bool isCallbackException = (pException->ExceptionRecord != nullptr &&
521546
pException->ExceptionRecord->ExceptionCode == 0xC000041D);
@@ -548,9 +573,12 @@ static void AppendCrashDiagnostics(const SString& text)
548573
frame.AddrFrame.Mode = AddrModeFlat;
549574
frame.AddrStack.Mode = AddrModeFlat;
550575

551-
SymbolHandlerGuard symbolGuard(hProcess);
552-
if (!symbolGuard.IsInitialized())
553-
return false;
576+
SymbolHandlerGuard symbolGuard(hProcess, hasSymbols);
577+
578+
const bool useDbgHelp = symbolGuard.IsInitialized();
579+
const auto routines = useDbgHelp
580+
? StackTraceHelpers::MakeStackWalkRoutines(true)
581+
: StackTraceHelpers::MakeStackWalkRoutines(false);
554582

555583
static_assert(MAX_SYM_NAME > 1, "MAX_SYM_NAME must include room for a terminator");
556584
constexpr DWORD kSymbolNameCapacity = MAX_SYM_NAME - 1;
@@ -564,8 +592,15 @@ static void AppendCrashDiagnostics(const SString& text)
564592

565593
for (std::size_t frameIndex = 0; frameIndex < MAX_FALLBACK_STACK_FRAMES; ++frameIndex)
566594
{
567-
BOOL bWalked =
568-
StackWalk64(IMAGE_FILE_MACHINE_I386, hProcess, hThread, &frame, &context, nullptr, SymFunctionTableAccess64, SymGetModuleBase64, nullptr);
595+
BOOL bWalked = StackWalk64(IMAGE_FILE_MACHINE_I386,
596+
hProcess,
597+
hThread,
598+
&frame,
599+
&context,
600+
routines.readMemory,
601+
routines.functionTableAccess,
602+
routines.moduleBase,
603+
nullptr);
569604
if (bWalked == FALSE)
570605
break;
571606

@@ -583,23 +618,29 @@ static void AppendCrashDiagnostics(const SString& text)
583618
visitedAddresses[visitedCount++] = address;
584619

585620
SString symbolName = SString("0x%llX", static_cast<unsigned long long>(address));
586-
if (SymFromAddr(hProcess, address, nullptr, pSymbol) != FALSE)
621+
if (useDbgHelp && SymFromAddr(hProcess, address, nullptr, pSymbol) != FALSE)
587622
{
588623
const auto terminatorIndex = static_cast<std::size_t>(pSymbol->MaxNameLen);
589624
if (terminatorIndex < MAX_SYM_NAME)
590625
pSymbol->Name[terminatorIndex] = '\0';
591626
symbolName = pSymbol->Name;
592627
}
593628

594-
IMAGEHLP_LINE64 lineInfo{};
595-
lineInfo.SizeOfStruct = sizeof(IMAGEHLP_LINE64);
629+
IMAGEHLP_LINE64 lineInfo{};
630+
lineInfo.SizeOfStruct = sizeof(IMAGEHLP_LINE64);
596631
DWORD lineDisplacement = 0;
597-
SString lineDetail = "unknown";
598-
if (SymGetLineFromAddr64(hProcess, address, &lineDisplacement, &lineInfo) != FALSE)
632+
SString lineDetail;
633+
634+
if (useDbgHelp && SymGetLineFromAddr64(hProcess, address, &lineDisplacement, &lineInfo) != FALSE)
599635
{
600636
const char* fileName = lineInfo.FileName != nullptr ? lineInfo.FileName : "unknown";
601637
lineDetail = SString("%s:%lu", fileName, static_cast<unsigned long>(lineInfo.LineNumber));
602638
}
639+
else
640+
{
641+
const std::string formatted = StackTraceHelpers::FormatAddressWithModule(address);
642+
lineDetail = formatted.c_str();
643+
}
603644

604645
outText += SString("#%02u %s [0x%llX] (%s)\n", static_cast<unsigned int>(frameIndex), symbolName.c_str(), static_cast<unsigned long long>(address),
605646
lineDetail.c_str());

‎Client/core/CExceptionInformation_Impl.cpp‎

Lines changed: 55 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
*****************************************************************************/
1111

1212
#include "StdInc.h"
13+
#include "StackTraceHelpers.h"
1314
#include <SharedUtil.Misc.h>
1415
#include <SharedUtil.Detours.h>
1516
#include <SharedUtil.Memory.h>
@@ -129,10 +130,18 @@ constexpr std::size_t MAX_FILE_NAME = 260;
129130
class SymbolHandlerGuard
130131
{
131132
public:
132-
explicit SymbolHandlerGuard(HANDLE process) noexcept : m_process(process), m_initialized(false), m_uncaughtExceptions(std::uncaught_exceptions())
133+
explicit SymbolHandlerGuard(HANDLE process, bool enableSymbols) noexcept
134+
: m_process(process), m_initialized(false), m_uncaughtExceptions(std::uncaught_exceptions())
133135
{
136+
if (!enableSymbols)
137+
{
138+
return;
139+
}
140+
134141
if (m_process != nullptr)
135142
{
143+
SymSetOptions(SYMOPT_LOAD_LINES | SYMOPT_UNDNAME | SYMOPT_FAIL_CRITICAL_ERRORS);
144+
136145
if (SymInitialize(m_process, nullptr, TRUE) != FALSE)
137146
{
138147
m_initialized = true;
@@ -178,6 +187,15 @@ class SymbolHandlerGuard
178187
if (pException == nullptr || pException->ContextRecord == nullptr)
179188
return std::nullopt;
180189

190+
const bool hasSymbols = CrashHandler::ProcessHasLocalDebugSymbols();
191+
if (!hasSymbols)
192+
{
193+
static std::once_flag logOnce;
194+
std::call_once(logOnce, [] {
195+
SafeDebugPrintPrefixed(DEBUG_PREFIX_EXCEPTION_INFO, "CaptureEnhancedStackTrace - capturing without symbols (raw addresses only)\n");
196+
});
197+
}
198+
181199
std::vector<StackFrameInfo> frames;
182200
frames.reserve(MAX_STACK_FRAMES);
183201

@@ -196,11 +214,13 @@ class SymbolHandlerGuard
196214
frame.AddrFrame.Mode = AddrModeFlat;
197215
frame.AddrStack.Mode = AddrModeFlat;
198216

199-
SymbolHandlerGuard symbolGuard(hProcess);
200-
if (!symbolGuard.IsInitialized())
201-
return std::nullopt;
217+
SymbolHandlerGuard symbolGuard(hProcess, hasSymbols);
218+
219+
const bool useDbgHelp = symbolGuard.IsInitialized();
202220

203-
SymSetOptions(SYMOPT_LOAD_LINES | SYMOPT_UNDNAME | SYMOPT_FAIL_CRITICAL_ERRORS);
221+
const auto routines = useDbgHelp
222+
? StackTraceHelpers::MakeStackWalkRoutines(true)
223+
: StackTraceHelpers::MakeStackWalkRoutines(false);
204224
alignas(SYMBOL_INFO) std::uint8_t symbolBuffer[sizeof(SYMBOL_INFO) + MAX_SYMBOL_NAME];
205225
memset(symbolBuffer, 0, sizeof(symbolBuffer));
206226
PSYMBOL_INFO pSymbol = reinterpret_cast<PSYMBOL_INFO>(symbolBuffer);
@@ -209,8 +229,15 @@ class SymbolHandlerGuard
209229

210230
for (std::size_t frameIndex = 0; frameIndex < MAX_STACK_FRAMES; ++frameIndex)
211231
{
212-
BOOL bWalked =
213-
StackWalk64(IMAGE_FILE_MACHINE_I386, hProcess, hThread, &frame, &context, nullptr, SymFunctionTableAccess64, SymGetModuleBase64, nullptr);
232+
BOOL bWalked = StackWalk64(IMAGE_FILE_MACHINE_I386,
233+
hProcess,
234+
hThread,
235+
&frame,
236+
&context,
237+
routines.readMemory,
238+
routines.functionTableAccess,
239+
routines.moduleBase,
240+
nullptr);
214241
if (bWalked == FALSE)
215242
break;
216243

@@ -222,28 +249,31 @@ class SymbolHandlerGuard
222249
frameInfo.isValid = true;
223250

224251
DWORD64 address = frame.AddrPC.Offset;
225-
frameInfo.symbolName = std::to_string(address);
252+
frameInfo.symbolName = StackTraceHelpers::FormatAddressWithModuleAndAbsolute(address);
226253

227-
DWORD64 displacement = 0;
228-
if (SymFromAddr(hProcess, address, &displacement, pSymbol) != FALSE)
254+
if (useDbgHelp)
229255
{
230-
frameInfo.symbolName = pSymbol->Name[0] != '\0' ? pSymbol->Name : "unknown";
231-
}
256+
DWORD64 displacement = 0;
257+
if (SymFromAddr(hProcess, address, &displacement, pSymbol) != FALSE)
258+
{
259+
frameInfo.symbolName = pSymbol->Name[0] != '\0' ? pSymbol->Name : "unknown";
260+
}
232261

233-
IMAGEHLP_LINE64 lineInfo;
234-
memset(&lineInfo, 0, sizeof(lineInfo));
235-
lineInfo.SizeOfStruct = sizeof(lineInfo);
236-
DWORD lineDisplacement = 0;
262+
IMAGEHLP_LINE64 lineInfo;
263+
memset(&lineInfo, 0, sizeof(lineInfo));
264+
lineInfo.SizeOfStruct = sizeof(lineInfo);
265+
DWORD lineDisplacement = 0;
237266

238-
if (SymGetLineFromAddr64(hProcess, address, &lineDisplacement, &lineInfo) != FALSE)
239-
{
240-
frameInfo.fileName = lineInfo.FileName ? lineInfo.FileName : "unknown";
241-
frameInfo.lineNumber = lineInfo.LineNumber;
242-
}
243-
else
244-
{
245-
frameInfo.fileName = "unknown";
246-
frameInfo.lineNumber = 0;
267+
if (SymGetLineFromAddr64(hProcess, address, &lineDisplacement, &lineInfo) != FALSE)
268+
{
269+
frameInfo.fileName = lineInfo.FileName ? lineInfo.FileName : "unknown";
270+
frameInfo.lineNumber = lineInfo.LineNumber;
271+
}
272+
else
273+
{
274+
frameInfo.fileName.clear();
275+
frameInfo.lineNumber = 0;
276+
}
247277
}
248278

249279
frames.push_back(std::move(frameInfo));

0 commit comments

Comments
 (0)