diff --git a/src/CrashHandler.cpp b/src/CrashHandler.cpp index f7322f901e9..eba3b177205 100644 --- a/src/CrashHandler.cpp +++ b/src/CrashHandler.cpp @@ -273,37 +273,31 @@ static bool DownloadAndUnzipSymbols(const char* symDir) { return ok; } -bool CrashHandlerDownloadSymbols() { - log("CrashHandlerDownloadSymbols()\n"); - if (!dir::CreateAll(gSymbolsDir)) { - log("CrashHandlerDownloadSymbols: couldn't create symbols dir\n"); - return false; - } - +bool InitializeDbgHelp() { TempWStr ws = ToWStrTemp(gSymbolPath); if (!dbghelp::Initialize(ws, false)) { - log("CrashHandlerDownloadSymbols: dbghelp::Initialize() failed\n"); + logf("InitializeDbgHelp: dbghelp::Initialize('%s') failed\n", gSymbolPath); return false; } - if (dbghelp::HasSymbols()) { - log("CrashHandlerDownloadSymbols(): skipping because dbghelp::HasSymbols()\n"); - return true; - } - - if (!DownloadAndUnzipSymbols(gSymbolsDir)) { - log("CrashHandlerDownloadSymbols: failed to download symbols\n"); + if (!dbghelp::HasSymbols()) { + log("InitializeDbgHelp(): dbghelp::HasSymbols() failed\n"); return false; } + log("InitializeDbgHelp(): did initialize ok\n"); + return true; +} - if (!dbghelp::Initialize(ws, true)) { - log("CrashHandlerDownloadSymbols: second dbghelp::Initialize() failed\n"); +bool CrashHandlerDownloadSymbols() { + log("CrashHandlerDownloadSymbols()\n"); + + if (!dir::CreateAll(gSymbolsDir)) { + logf("CrashHandlerDownloadSymbols: couldn't create symbols dir '%s'\n", gSymbolsDir); return false; } - if (!dbghelp::HasSymbols()) { - logf("CrashHandlerDownloadSymbols: HasSymbols() false after downloading symbols, gSymbolPath:'%s'\n", - gSymbolPath); + if (!DownloadAndUnzipSymbols(gSymbolsDir)) { + log("CrashHandlerDownloadSymbols: failed to download symbols\n"); return false; } return true; @@ -332,6 +326,7 @@ void _uploadDebugReport(const char* condStr, bool noCallstack) { // reports from official release return; } + if (gIsDebugBuild) { // exclude debug builds. Those are most likely other people modyfing Sumatra return; @@ -342,13 +337,17 @@ void _uploadDebugReport(const char* condStr, bool noCallstack) { return; } - logf("_uploadDebugReport: noCallstack: %d, gSymbolPathW: '%s'\n", (int)noCallstack, gSymbolPath); + logf("_uploadDebugReport: noCallstack: %d, gSymbolPath: '%s'\n", (int)noCallstack, gSymbolPath); - if (!noCallstack) { - bool ok = CrashHandlerDownloadSymbols(); + bool needsSymbols = !noCallstack; + if (needsSymbols) { + // we proceed even if we fail to download symbols + bool ok = InitializeDbgHelp(); if (!ok) { - log("_uploadDebugReport(): CrashHandlerDownloadSymbols() failed\n"); - return; + ok = CrashHandlerDownloadSymbols(); + if (ok) { + InitializeDbgHelp(); + } } } @@ -377,9 +376,15 @@ void TryUploadCrashReport() { logf("TryUploadCrashReport: gSymbolPathW: '%s'\n", gSymbolPath); - bool ok = CrashHandlerDownloadSymbols(); - if (!ok) { - log("TryUploadCrashReport(): CrashHandlerDownloadSymbols() failed\n"); + bool ok = InitializeDbgHelp(); + if (ok) { + // we preceed even if we can't download symbols or initialize dbghelp + ok = CrashHandlerDownloadSymbols(); + if (!ok) { + log("TryUploadCrashReport(): CrashHandlerDownloadSymbols() failed\n"); + } else { + InitializeDbgHelp(); + } } char* sv = BuildCrashInfoText(nullptr, true); @@ -616,6 +621,8 @@ static void BuildSystemInfo() { http://p-nand-q.com/python/procmon.html https://learn.microsoft.com/en-us/windows-hardware/drivers/debugger/debugger-download-tools + + Setting symbol path: add GetEnvironmentVariableA("_NT_SYMBOL_PATH", ..., ...) add GetEnvironmentVariableA("_NT_ALT_SYMBOL_PATH ", ..., ...) @@ -627,10 +634,25 @@ for symbols server symsrv.dll is needed, installed with debug tools for windows static bool gAddNtSymbolPath = true; static bool gAddSymbolServer = true; +static bool gAddExeDir = true; -static void BuildSymbolPath() { +static void BuildSymbolPath(const char* symDir) { str::Str path(1024); + bool symDirExists = dir::Exists(symDir); + + if (symDirExists) { + path.Append(symDir); + path.Append(";"); + } + + // in debug builds the symbols are in the same directory as .exe + if (gIsDebugBuild || gAddExeDir) { + TempStr dir = GetExeDirTemp(); + path.Append(dir); + path.Append(";"); + } + if (gAddNtSymbolPath) { TempStr ntSymPath = GetEnvVariableTemp("_NT_SYMBOL_PATH"); // internet talks about both _NT_ALT_SYMBOL_PATH and _NT_ALTERNATE_SYMBOL_PATH @@ -645,20 +667,17 @@ static void BuildSymbolPath() { path.Append(";"); } } - path.Append(gSymbolsDir); - if (gAddSymbolServer) { - // this probably typicall doesn't work as it needs symsrv.dll - path.AppendFmt("cache*%s;srv*http://msdl.microsoft.com/download/symbols;", gSymbolsDir); + if (gAddSymbolServer && symDirExists) { + // this probably won't work as it needs symsrv.dll and that's not included with Windows + // TODO: maybe try to scan system directories for symsrv.dll and somehow add it? + path.AppendFmt("cache*%s;srv*https://msdl.microsoft.com/download/symbols;", symDir); } -#if 0 - // when running local builds, *.pdb is in the same dir as *.exe - WCHAR* exePath = GetExePathTemp().Get(); - path.Append(L";"); - path.Append(exePath); -#endif + // remove ";" from the end + path.RemoveLast(); - str::ReplaceWithCopy(&gSymbolPath, path.Get()); + char* p = path.CStr(); + str::ReplaceWithCopy(&gSymbolPath, p); } bool SetSymbolsDir(const char* symDir) { @@ -666,7 +685,7 @@ bool SetSymbolsDir(const char* symDir) { return false; } str::ReplaceWithCopy(&gSymbolsDir, symDir); - BuildSymbolPath(); + BuildSymbolPath(gSymbolsDir); return true; } diff --git a/src/CrashHandler.h b/src/CrashHandler.h index e50b0ee51d7..9f8e9ee32d5 100644 --- a/src/CrashHandler.h +++ b/src/CrashHandler.h @@ -7,4 +7,5 @@ extern char* gSymbolsDir; void InstallCrashHandler(const char* crashDumpPath, const char* crashFilePath, const char* symDir); void UninstallCrashHandler(); bool CrashHandlerDownloadSymbols(); +bool InitializeDbgHelp(); bool SetSymbolsDir(const char* symDir); diff --git a/src/SumatraPDF.cpp b/src/SumatraPDF.cpp index 84f45cd1934..326e8ae30ac 100644 --- a/src/SumatraPDF.cpp +++ b/src/SumatraPDF.cpp @@ -140,7 +140,6 @@ static bool gDontSavePrefs = false; static void CloseDocumentInCurrentTab(MainWindow*, bool keepUIEnabled, bool deleteModel); static void OnSidebarSplitterMove(SplitterMoveEvent*); static void OnFavSplitterMove(SplitterMoveEvent*); -static void DownloadDebugSymbols(); LoadArgs::LoadArgs(const char* origPath, MainWindow* win) { this->fileArgs = ParseFileArgs(origPath); @@ -4866,6 +4865,27 @@ void ClearHistory(MainWindow* win) { // TODO: deletion takes time so run it async with RunAsync() } +static void DownloadDebugSymbols() { + TempStr msg = (TempStr) "Symbols were already downloaded"; + bool ok = InitializeDbgHelp(); + if (ok) { + goto ShowMessage; + } + ok = CrashHandlerDownloadSymbols(); + if (!ok) { + msg = (TempStr) "Failed to download symbols"; + goto ShowMessage; + } + msg = str::FormatTemp("Downloaded symbols to %s", gSymbolsDir); + if (ok) { + bool didInitializeDbgHelp = InitializeDbgHelp(); + ReportIfQuick(!didInitializeDbgHelp); + } +ShowMessage: + uint flags = MB_ICONINFORMATION | MB_OK | MbRtlReadingMaybe(); + MessageBoxA(nullptr, msg, "Downloading symbols", flags); +} + // try to trigger a crash due to corrupting allocator // this is a different kind of a crash than just referencing invalid memory // as corrupted memory migh prevent crash handler from working @@ -6149,16 +6169,6 @@ void ShowCrashHandlerMessage() { LaunchFileShell(url, nullptr, "open"); } -static void DownloadDebugSymbols() { - bool ok = CrashHandlerDownloadSymbols(); - TempStr msg = (TempStr) "Failed to download symbols."; - if (ok) { - msg = str::FormatTemp("Downloaded symbols to %s", gSymbolsDir); - } - uint flags = MB_ICONINFORMATION | MB_OK | MbRtlReadingMaybe(); - MessageBoxA(nullptr, msg, "Downloading symbols", flags); -} - void ShutdownCleanup() { gAllowedFileTypes.Reset(); gAllowedLinkProtocols.Reset(); diff --git a/src/utils/DbgHelpDyn.cpp b/src/utils/DbgHelpDyn.cpp index 1d88df62c5e..e082fefdcb4 100644 --- a/src/utils/DbgHelpDyn.cpp +++ b/src/utils/DbgHelpDyn.cpp @@ -149,9 +149,15 @@ NO_INLINE bool CanSymbolizeAddress(DWORD64 addr) { DWORD64 symDisp = 0; BOOL ok = DynSymFromAddr(GetCurrentProcess(), addr, &symDisp, symInfo); + if (!ok) { + return false; + } int symLen = symInfo->NameLen; + if (symLen < 4) { + return false; + } char* name = symInfo->Name; - return ok && symLen > 4 && (name[0] != 0); + return *name != 0; } // a heuristic to test if we have symbols for our own binaries by testing if