From: Mike McLaughlin Date: Tue, 31 Dec 2019 02:14:52 +0000 (-0800) Subject: Misc cleanup and fixes found during runtime change testing. X-Git-Tag: submit/tizen_5.5/20200504.045052~11^2^2~186 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=8c8e7d139e9c0bcce3cfc99afbd84cc8eabb52d1;p=platform%2Fcore%2Fdotnet%2Fdiagnostics.git Misc cleanup and fixes found during runtime change testing. Change ctrl-c message issue: https://github.com/dotnet/diagnostics/issues/532 Revert 5.0.x runtime version until issue https://github.com/dotnet/runtime/issues/1229 is fixed. --- diff --git a/eng/Versions.props b/eng/Versions.props index 6fc508c6c..3bb721add 100644 --- a/eng/Versions.props +++ b/eng/Versions.props @@ -15,8 +15,8 @@ $(MicrosoftNETCoreApp30Version) 3.1.0 $(MicrosoftNETCoreApp31Version) - 5.0.0-alpha.1.19627.5 - 5.0.0-alpha.1.19614.22 + 5.0.0-alpha.1.19562.8 + 5.0.0-alpha.1.19626.3 diff --git a/src/SOS/SOS.NETCore/SymbolReader.cs b/src/SOS/SOS.NETCore/SymbolReader.cs index a345efbce..50231e29b 100644 --- a/src/SOS/SOS.NETCore/SymbolReader.cs +++ b/src/SOS/SOS.NETCore/SymbolReader.cs @@ -83,6 +83,8 @@ namespace SOS public TargetStream(ulong address, int size, ReadMemoryDelegate readMemory) : base() { + Debug.Assert(address != 0); + Debug.Assert(size != 0); _address = address; _readMemory = readMemory; Length = size; @@ -294,7 +296,9 @@ namespace SOS s_tracer.Verbose("{0} {1}", key.FullPathName, key.Index); // Don't download the sos binaries that come with the runtime - if (moduleFileName != "SOS.NETCore.dll" && !moduleFileName.StartsWith("libsos.")) + if (!IsPathEqual(moduleFileName, "SOS.NETCore.dll") && + !IsPathEqual(moduleFileName, "sos.dll") && + !moduleFileName.StartsWith("libsos.")) { string downloadFilePath = GetSymbolFile(key); if (downloadFilePath != null) diff --git a/src/SOS/SOS.UnitTests/SOSRunner.cs b/src/SOS/SOS.UnitTests/SOSRunner.cs index a86eb1c95..de87100f8 100644 --- a/src/SOS/SOS.UnitTests/SOSRunner.cs +++ b/src/SOS/SOS.UnitTests/SOSRunner.cs @@ -704,11 +704,11 @@ public class SOSRunner : IDisposable } try { - await RunSosCommand("sosstatus"); + await RunSosCommand("SOSStatus"); } catch (Exception ex) { - WriteLine("Exception executing sosstatus {0}", ex.ToString()); + WriteLine("Exception executing SOSStatus {0}", ex.ToString()); } throw; } @@ -1245,6 +1245,10 @@ public class SOSRunner : IDisposable Task currentTask = null; lock (this) { + if (_taskQueue.Count == 0) + { + return false; + } currentTask = _taskQueue[0]; _taskQueue.RemoveAt(0); } @@ -1256,6 +1260,7 @@ public class SOSRunner : IDisposable Task currentTask = null; lock (this) { + Debug.Assert(_taskQueue.Count > 0); currentTask = _taskQueue[0]; } return currentTask; diff --git a/src/SOS/Strike/exts.cpp b/src/SOS/Strike/exts.cpp index 32083f858..63f27dc91 100644 --- a/src/SOS/Strike/exts.cpp +++ b/src/SOS/Strike/exts.cpp @@ -14,15 +14,11 @@ #define VER_PRODUCTVERSION_W (0x0100) -extern void SOSShutdown(); - // // globals // WINDBG_EXTENSION_APIS ExtensionApis; -OnUnloadTask *OnUnloadTask::s_pUnloadTaskList = NULL; - // // Valid for the lifetime of the debug session. // @@ -41,6 +37,8 @@ bool g_palInitialized = false; #endif // FEATURE_PAL +OnUnloadTask *OnUnloadTask::s_pUnloadTaskList = NULL; + IMachine* g_targetMachine = NULL; BOOL g_bDacBroken = FALSE; @@ -64,6 +62,7 @@ PDEBUG_SYSTEM_OBJECTS g_ExtSystem; extern "C" HRESULT ExtQuery(PDEBUG_CLIENT client) { + HRESULT Status; g_ExtClient = client; #else extern "C" HRESULT @@ -79,12 +78,16 @@ ExtQuery(ILLDBServices* services) g_palInitialized = true; } g_ExtServices = services; - services->QueryInterface(__uuidof(ILLDBServices2), (void**)&g_ExtServices2); + HRESULT Status = services->QueryInterface(__uuidof(ILLDBServices2), (void**)&g_ExtServices2); + if (FAILED(Status)) + { + g_ExtServices = NULL; + return Status; + } DebugClient* client = new DebugClient(services, g_ExtServices2); g_DebugClient = client; #endif - HRESULT Status; SOS_ExtQueryFailGo(g_ExtControl, IDebugControl2); SOS_ExtQueryFailGo(g_ExtData, IDebugDataSpaces); SOS_ExtQueryFailGo(g_ExtRegisters, IDebugRegisters); @@ -265,7 +268,6 @@ DebugExtensionInitialize(PULONG Version, PULONG Flags) ExtRelease(); OnUnloadTask::Register(CleanupEventCallbacks); - OnUnloadTask::Register(SOSShutdown); g_pCallbacksClient = DebugClient; EventCallbacks* pCallbacksObj = new EventCallbacks(DebugClient); IDebugEventCallbacks* pCallbacks = NULL; @@ -295,7 +297,6 @@ void CALLBACK DebugExtensionNotify(ULONG Notify, ULONG64 /*Argument*/) { - return; } extern "C" @@ -303,12 +304,12 @@ void CALLBACK DebugExtensionUninitialize(void) { - // execute all registered cleanup tasks + // Execute all registered cleanup tasks OnUnloadTask::Run(); - return; } -BOOL WINAPI DllMain(HANDLE hInstance, DWORD dwReason, LPVOID lpReserved) +BOOL WINAPI +DllMain(HANDLE hInstance, DWORD dwReason, LPVOID lpReserved) { if (dwReason == DLL_PROCESS_ATTACH) { @@ -319,6 +320,14 @@ BOOL WINAPI DllMain(HANDLE hInstance, DWORD dwReason, LPVOID lpReserved) #else // FEATURE_PAL +__attribute__((destructor)) +void +Uninitialize(void) +{ + // Execute all registered cleanup tasks + OnUnloadTask::Run(); +} + HRESULT DebugClient::QueryInterface( REFIID InterfaceId, diff --git a/src/SOS/Strike/exts.h b/src/SOS/Strike/exts.h index c6c9f5a61..e15e88776 100644 --- a/src/SOS/Strike/exts.h +++ b/src/SOS/Strike/exts.h @@ -172,7 +172,7 @@ inline BOOL IsInterrupt() { if (!ControlC && g_ExtControl->GetInterrupt() == S_OK) { - ExtOut("Command cancelled at the user's request.\n"); + ExtOut("Command canceled at the user's request.\n"); ControlC = TRUE; } diff --git a/src/SOS/Strike/hostcoreclr.cpp b/src/SOS/Strike/hostcoreclr.cpp index f56c7706e..e362a1b84 100644 --- a/src/SOS/Strike/hostcoreclr.cpp +++ b/src/SOS/Strike/hostcoreclr.cpp @@ -434,10 +434,10 @@ static HRESULT GetHostRuntime(std::string& coreClrPath, std::string& hostRuntime return S_OK; } -// -// Returns the unique temporary directory for this instnace of SOS -// -static LPCSTR GetTempDirectory() +/**********************************************************************\ + * Returns the unique temporary directory for this instance of SOS +\**********************************************************************/ +LPCSTR GetTempDirectory() { if (g_tmpPath == nullptr) { @@ -454,6 +454,7 @@ static LPCSTR GetTempDirectory() CreateDirectoryA(tmpPath, NULL); g_tmpPath = _strdup(tmpPath); + OnUnloadTask::Register(CleanupTempDirectory); } return g_tmpPath; } @@ -461,10 +462,7 @@ static LPCSTR GetTempDirectory() /**********************************************************************\ * Clean up the temporary directory files and DAC symlink. \**********************************************************************/ -#ifdef FEATURE_PAL -__attribute__((destructor)) -#endif -void SOSShutdown() +void CleanupTempDirectory() { LPCSTR tmpPath = (LPCSTR)InterlockedExchangePointer((PVOID *)&g_tmpPath, nullptr); if (tmpPath != nullptr) @@ -785,7 +783,16 @@ HRESULT InitializeSymbolStore( IfFailRet(InitializeHosting()); _ASSERTE(g_SOSNetCoreCallbacks.InitializeSymbolStoreDelegate != nullptr); - if (!g_SOSNetCoreCallbacks.InitializeSymbolStoreDelegate(logging, msdl, symweb, GetTempDirectory(), symbolServer, timeoutInMinutes, cacheDirectory, searchDirectory, windowsSymbolPath)) + if (!g_SOSNetCoreCallbacks.InitializeSymbolStoreDelegate( + logging, + msdl, + symweb, + GetTempDirectory(), + symbolServer, + timeoutInMinutes, + cacheDirectory, + searchDirectory, + windowsSymbolPath)) { ExtErr("Error initializing symbol server support\n"); return E_FAIL; @@ -828,7 +835,16 @@ void InitializeSymbolStoreFromSymPath() { if (strlen(symbolPath) > 0) { - if (!g_SOSNetCoreCallbacks.InitializeSymbolStoreDelegate(false, false, false, GetTempDirectory(), nullptr, 0, nullptr, nullptr, symbolPath)) + if (!g_SOSNetCoreCallbacks.InitializeSymbolStoreDelegate( + false, // logging + false, // msdl + false, // symweb + GetTempDirectory(), // tempDirectory + nullptr, // symbolServerPath + 0, // timeoutInMinutes + nullptr, // symbolCachePath + nullptr, // symbolDirectoryPath + symbolPath)) // windowsSymbolPath { ExtErr("Windows symbol path parsing FAILED\n"); return; @@ -1087,19 +1103,21 @@ HRESULT SymbolReader::LoadSymbols(___in IMetaDataImport* pMD, ___in IXCLRDataMod ExtOut("LoadSymbols moduleData.Request FAILED 0x%08x\n", hr); return hr; #else - ULONG64 modBase; - hr = GetClrModuleImages(pModule, CLRDATA_MODULE_PE_FILE, &modBase); + ULONG64 moduleBase; + ULONG64 moduleSize; + hr = GetClrModuleImages(pModule, CLRDATA_MODULE_PE_FILE, &moduleBase, &moduleSize); if (FAILED(hr)) { ExtOut("LoadSymbols GetClrModuleImages FAILED 0x%08x\n", hr); return hr; } - hr = LoadSymbolsForWindowsPDB(pMD, modBase, pModuleName, FALSE); + hr = LoadSymbolsForWindowsPDB(pMD, moduleBase, pModuleName, FALSE); if (SUCCEEDED(hr)) { return hr; } - moduleData.LoadedPEAddress = modBase; + moduleData.LoadedPEAddress = moduleBase; + moduleData.LoadedPESize = moduleSize; moduleData.IsFileLayout = TRUE; #endif } diff --git a/src/SOS/Strike/hostcoreclr.h b/src/SOS/Strike/hostcoreclr.h index 390dccd25..f4848c6d2 100644 --- a/src/SOS/Strike/hostcoreclr.h +++ b/src/SOS/Strike/hostcoreclr.h @@ -16,7 +16,17 @@ typedef void (*WriteLineDelegate)(const char*); typedef int (*ReadMemoryDelegate)(ULONG64, uint8_t*, int); typedef void (*SymbolFileCallbackDelegate)(void*, const char* moduleFileName, const char* symbolFilePath); -typedef BOOL (*InitializeSymbolStoreDelegate)(BOOL, BOOL, BOOL, const char*, const char*, int, const char*, const char*, const char*); +typedef BOOL (*InitializeSymbolStoreDelegate)( + BOOL logging, + BOOL msdl, + BOOL symweb, + const char* tempDirectory, + const char* symbolServerPath, + int timeoutInMinutes, + const char* symbolCacehPath, + const char* symbolDirectoryPath, + const char* windowsSymbolPath); + typedef void (*DisplaySymbolStoreDelegate)(WriteLineDelegate); typedef void (*DisableSymbolStoreDelegate)(); typedef void (*LoadNativeSymbolsDelegate)(SymbolFileCallbackDelegate, void*, const char*, ULONG64, int, ReadMemoryDelegate); @@ -69,6 +79,8 @@ extern HRESULT GetRuntimeDirectory(std::string& runtimeDirectory); extern HRESULT GetRuntimeDirectory(LPWSTR modulePath, int modulePathSize); extern LPCSTR GetDacFilePath(); extern LPCSTR GetDbiFilePath(); +extern LPCSTR GetTempDirectory(); +extern void CleanupTempDirectory(); extern BOOL IsHostingInitialized(); extern HRESULT InitializeHosting(); diff --git a/src/SOS/Strike/sos.h b/src/SOS/Strike/sos.h index 4d154ef76..b8a85660b 100644 --- a/src/SOS/Strike/sos.h +++ b/src/SOS/Strike/sos.h @@ -107,7 +107,7 @@ namespace sos inline void CheckInterrupt() { if (g_ExtControl->GetInterrupt() == S_OK) - Throw("User interrupt."); + Throw("Command canceled at the user's request."); } /* ThinLock struct. Use Object::GetThinLock to fill the struct. diff --git a/src/SOS/Strike/util.cpp b/src/SOS/Strike/util.cpp index 939ba9d85..9b9332ab4 100644 --- a/src/SOS/Strike/util.cpp +++ b/src/SOS/Strike/util.cpp @@ -2371,13 +2371,15 @@ Failure: * instance passed in, and the extent type requested. * * * \**********************************************************************/ -HRESULT GetClrModuleImages(__in IXCLRDataModule* module, __in CLRDataModuleExtentType desiredType, __out PULONG64 firstAdd) +HRESULT GetClrModuleImages(__in IXCLRDataModule* module, __in CLRDataModuleExtentType desiredType, __out PULONG64 pBase, __out PULONG64 pSize) { CLRDATA_ENUM enumExtents; HRESULT hr; - _ASSERTE(firstAdd != nullptr); - *firstAdd = 0; + _ASSERTE(pBase != nullptr); + _ASSERTE(pSize != nullptr); + *pBase = 0; + *pSize = 0; if (FAILED(hr = module->StartEnumExtents(&enumExtents))) { @@ -2388,12 +2390,24 @@ HRESULT GetClrModuleImages(__in IXCLRDataModule* module, __in CLRDataModuleExten { if ((desiredType == CLRDATA_MODULE_OTHER) || (desiredType == extent.type)) { - ULONG64 modBase; - if (FAILED(hr = g_ExtSymbols->GetModuleByOffset(extent.base, 0, nullptr, &modBase))) + ULONG64 moduleBase; + if (FAILED(hr = g_ExtSymbols->GetModuleByOffset(extent.base, 0, nullptr, &moduleBase))) + { + if (desiredType == CLRDATA_MODULE_PE_FILE) + { + *pBase = extent.base; + *pSize = extent.length; + hr = S_OK; + } + break; + } + DEBUG_MODULE_PARAMETERS params; + if (FAILED(hr = g_ExtSymbols->GetModuleParameters(1, &moduleBase, 0, ¶ms))) { break; } - *firstAdd = modBase; + *pBase = moduleBase; + *pSize = params.Size; hr = S_OK; break; } @@ -2438,7 +2452,7 @@ HRESULT GetModuleFromAddress(___in CLRDATA_ADDRESS peAddress, ___out IXCLRDataMo #ifdef FEATURE_PAL return hr; #else - hr = GetClrModuleImages(module, CLRDATA_MODULE_PE_FILE, &moduleData.LoadedPEAddress); + hr = GetClrModuleImages(module, CLRDATA_MODULE_PE_FILE, &moduleData.LoadedPEAddress, &moduleData.LoadedPESize); if (FAILED(hr)) { return hr; diff --git a/src/SOS/Strike/util.h b/src/SOS/Strike/util.h index 2f5406640..febfdda08 100644 --- a/src/SOS/Strike/util.h +++ b/src/SOS/Strike/util.h @@ -1391,7 +1391,7 @@ private: }; #ifndef FEATURE_PAL -HRESULT GetClrModuleImages(__in IXCLRDataModule* module, __in CLRDataModuleExtentType desiredType, __out PULONG64 firstAdd); +HRESULT GetClrModuleImages(__in IXCLRDataModule* module, __in CLRDataModuleExtentType desiredType, __out PULONG64 pBase, __out PULONG64 pSize); #endif HRESULT GetMethodDefinitionsFromName(DWORD_PTR ModulePtr, IXCLRDataModule* mod, const char* name, IXCLRDataMethodDefinition **ppMethodDefinitions, int numMethods, int *numMethodsNeeded); HRESULT GetMethodDescsFromName(DWORD_PTR ModulePtr, IXCLRDataModule* mod, const char* name, DWORD_PTR **pOut, int *numMethodDescs); diff --git a/src/SOS/Strike/xplat/dbgeng.h b/src/SOS/Strike/xplat/dbgeng.h index 42a506042..d278fe500 100644 --- a/src/SOS/Strike/xplat/dbgeng.h +++ b/src/SOS/Strike/xplat/dbgeng.h @@ -34,9 +34,7 @@ public: m_lldbservices2(lldbservices2) { m_lldbservices->AddRef(); - if (m_lldbservices2 != nullptr) { - m_lldbservices2->AddRef(); - } + m_lldbservices2->AddRef(); } //---------------------------------------------------------------------------- @@ -281,9 +279,6 @@ public: ULONG bufferSize, PULONG versionInfoSize) { - if (m_lldbservices2 == nullptr) { - return E_NOINTERFACE; - } return m_lldbservices2->GetModuleVersionInformation(index, base, item, buffer, bufferSize, versionInfoSize); } diff --git a/src/SOS/lldbplugin/services.cpp b/src/SOS/lldbplugin/services.cpp index a2340bdd3..478a8e456 100644 --- a/src/SOS/lldbplugin/services.cpp +++ b/src/SOS/lldbplugin/services.cpp @@ -1348,8 +1348,10 @@ LLDBServices::GetModuleSize( size += section.GetByteSize(); } } - - return size; + // For core dumps lldb doesn't return the section sizes when it + // doesn't have access to the actual module file, but SOS (like + // the SymbolReader code) still needs a non-zero module size. + return size != 0 ? size : LONG_MAX; } //---------------------------------------------------------------------------- diff --git a/src/SOS/lldbplugin/soscommand.cpp b/src/SOS/lldbplugin/soscommand.cpp index 487c62d56..8031e72c9 100644 --- a/src/SOS/lldbplugin/soscommand.cpp +++ b/src/SOS/lldbplugin/soscommand.cpp @@ -172,5 +172,6 @@ sosCommandInitialize(lldb::SBDebugger debugger) interpreter.AddCommand("sympath", new sosCommand("SetSymbolServer", "-sympath"), "Add server, cache and directory paths in the Windows symbol path format."); interpreter.AddCommand("soshelp", new sosCommand("Help"), "Displays all available commands when no parameter is specified, or displays detailed help information about the specified command. soshelp "); interpreter.AddCommand("sosstatus", new sosCommand("SOSStatus"), "Displays the global SOS status."); + interpreter.AddCommand("sosflush", new sosCommand("SOSFlush"), "Flushes the DAC caches."); return true; }