From: Mike McLaughlin Date: Sat, 13 Nov 2021 17:09:29 +0000 (-0800) Subject: Misc SOS fixes (#2746) X-Git-Tag: submit/tizen/20220302.040122~18^2^2~11 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=0d4202074ac789d621dc5d6479274f2e33a98d7c;p=platform%2Fcore%2Fdotnet%2Fdiagnostics.git Misc SOS fixes (#2746) * Misc SOS fixes Cleanup g_symbolStoreInitialized flag. DisplayStoreStore() wasn't displaying the symbol path set from the dbgeng .sympath change notification. Simplified the symbol service initialization logic. * Make DebugServices tests more reliable * Always use diasymreader if no managed symbol service * Actually flush the clrprocess/cordebug instances --- diff --git a/src/SOS/SOS.Hosting/RuntimeWrapper.cs b/src/SOS/SOS.Hosting/RuntimeWrapper.cs index 71cf779bf..62928a90d 100644 --- a/src/SOS/SOS.Hosting/RuntimeWrapper.cs +++ b/src/SOS/SOS.Hosting/RuntimeWrapper.cs @@ -135,13 +135,13 @@ namespace SOS.Hosting private void Flush() { // TODO: there is a better way to flush _corDebugProcess with ICorDebugProcess4::ProcessStateChanged(FLUSH_ALL) - if (_corDebugProcess == IntPtr.Zero) + if (_corDebugProcess != IntPtr.Zero) { COMHelper.Release(_corDebugProcess); _corDebugProcess = IntPtr.Zero; } // TODO: there is a better way to flush _clrDataProcess with ICLRDataProcess::Flush() - if (_clrDataProcess == IntPtr.Zero) + if (_clrDataProcess != IntPtr.Zero) { COMHelper.Release(_clrDataProcess); _clrDataProcess = IntPtr.Zero; diff --git a/src/SOS/SOS.Hosting/SymbolServiceWrapper.cs b/src/SOS/SOS.Hosting/SymbolServiceWrapper.cs index 9b5f62dc0..25dd90b3a 100644 --- a/src/SOS/SOS.Hosting/SymbolServiceWrapper.cs +++ b/src/SOS/SOS.Hosting/SymbolServiceWrapper.cs @@ -273,52 +273,55 @@ namespace SOS.Hosting int moduleIndexSize, IntPtr moduleIndex) { - try + if (_symbolService.IsSymbolStoreEnabled) { - KeyTypeFlags flags = specialKeys ? KeyTypeFlags.DacDbiKeys : KeyTypeFlags.IdentityKey; - byte[] id = new byte[moduleIndexSize]; - Marshal.Copy(moduleIndex, id, 0, moduleIndexSize); - - IEnumerable keys = null; - switch (config) + try { - case RuntimeConfiguration.UnixCore: - keys = ELFFileKeyGenerator.GetKeys(flags, moduleFilePath, id, symbolFile: false, symbolFileName: null); - break; + KeyTypeFlags flags = specialKeys ? KeyTypeFlags.DacDbiKeys : KeyTypeFlags.IdentityKey; + byte[] id = new byte[moduleIndexSize]; + Marshal.Copy(moduleIndex, id, 0, moduleIndexSize); - case RuntimeConfiguration.OSXCore: - keys = MachOFileKeyGenerator.GetKeys(flags, moduleFilePath, id, symbolFile: false, symbolFileName: null); - break; - - case RuntimeConfiguration.WindowsCore: - case RuntimeConfiguration.WindowsDesktop: - uint timeStamp = BitConverter.ToUInt32(id, 0); - uint fileSize = BitConverter.ToUInt32(id, 4); - SymbolStoreKey key = PEFileKeyGenerator.GetKey(moduleFilePath, timeStamp, fileSize); - keys = new SymbolStoreKey[] { key }; - break; + IEnumerable keys = null; + switch (config) + { + case RuntimeConfiguration.UnixCore: + keys = ELFFileKeyGenerator.GetKeys(flags, moduleFilePath, id, symbolFile: false, symbolFileName: null); + break; + + case RuntimeConfiguration.OSXCore: + keys = MachOFileKeyGenerator.GetKeys(flags, moduleFilePath, id, symbolFile: false, symbolFileName: null); + break; + + case RuntimeConfiguration.WindowsCore: + case RuntimeConfiguration.WindowsDesktop: + uint timeStamp = BitConverter.ToUInt32(id, 0); + uint fileSize = BitConverter.ToUInt32(id, 4); + SymbolStoreKey key = PEFileKeyGenerator.GetKey(moduleFilePath, timeStamp, fileSize); + keys = new SymbolStoreKey[] { key }; + break; + + default: + Trace.TraceError("LoadNativeSymbolsFromIndex: unsupported platform {0}", config); + return; + } + foreach (SymbolStoreKey key in keys) + { + string moduleFileName = Path.GetFileName(key.FullPathName); + Trace.TraceInformation("{0} {1}", key.FullPathName, key.Index); - default: - Trace.TraceError("LoadNativeSymbolsFromIndex: unsupported platform {0}", config); - return; + string downloadFilePath = _symbolService.DownloadFile(key); + if (downloadFilePath != null) + { + Trace.TraceInformation("{0}: {1}", moduleFileName, downloadFilePath); + callback(parameter, moduleFileName, downloadFilePath); + } + } } - foreach (SymbolStoreKey key in keys) + catch (Exception ex) when (ex is BadInputFormatException || ex is InvalidVirtualAddressException || ex is TaskCanceledException) { - string moduleFileName = Path.GetFileName(key.FullPathName); - Trace.TraceInformation("{0} {1}", key.FullPathName, key.Index); - - string downloadFilePath = _symbolService.DownloadFile(key); - if (downloadFilePath != null) - { - Trace.TraceInformation("{0}: {1}", moduleFileName, downloadFilePath); - callback(parameter, moduleFileName, downloadFilePath); - } + Trace.TraceError("{0} - {1}", ex.Message, moduleFilePath); } } - catch (Exception ex) when (ex is BadInputFormatException || ex is InvalidVirtualAddressException || ex is TaskCanceledException) - { - Trace.TraceError("{0} - {1}", ex.Message, moduleFilePath); - } } /// diff --git a/src/SOS/Strike/platform/runtimeimpl.cpp b/src/SOS/Strike/platform/runtimeimpl.cpp index 463b4ef75..c0ec1143c 100644 --- a/src/SOS/Strike/platform/runtimeimpl.cpp +++ b/src/SOS/Strike/platform/runtimeimpl.cpp @@ -700,15 +700,13 @@ void Runtime::DisplayStatus() } } -extern bool g_symbolStoreInitialized; - /**********************************************************************\ * Attempt to download the runtime modules (runtime, DAC and DBI) \**********************************************************************/ void Runtime::LoadRuntimeModules() { HRESULT hr = InitializeSymbolService(); - if (SUCCEEDED(hr) && g_symbolStoreInitialized) + if (SUCCEEDED(hr)) { if (m_runtimeInfo != nullptr) { diff --git a/src/SOS/Strike/symbols.cpp b/src/SOS/Strike/symbols.cpp index 20e3bd508..649f369b1 100644 --- a/src/SOS/Strike/symbols.cpp +++ b/src/SOS/Strike/symbols.cpp @@ -34,8 +34,6 @@ #define IfFailRet(EXPR) do { Status = (EXPR); if(FAILED(Status)) { return (Status); } } while (0) #endif -bool g_symbolStoreInitialized = false; - #ifndef FEATURE_PAL HMODULE g_hmoduleSymBinder = nullptr; ISymUnmanagedBinder3 *g_pSymBinder = nullptr; @@ -78,15 +76,16 @@ extern "C" void STDMETHODCALLTYPE SOSUninitializeByHost() \**********************************************************************/ HRESULT InitializeSymbolService() { - if (!g_symbolStoreInitialized) + static bool initialized = false; + if (!initialized) { ISymbolService* symbolService = GetSymbolService(); if (symbolService == nullptr) { return E_NOINTERFACE; } - g_symbolStoreInitialized = symbolService->IsSymbolStoreEnabled(); + initialized = true; #ifndef FEATURE_PAL - // When SOS is hosted on dotnet-dump, the ExtensionApis are not set so + // When SOS is hosted on dotnet-dump on Windows, the ExtensionApis are not set so // the expression evaluation function needs to be supplied. if (GetExpression == nullptr) { @@ -96,6 +95,7 @@ HRESULT InitializeSymbolService() } #endif OnUnloadTask::Register([]() { + initialized = false; DisableSymbolStore(); }); } @@ -137,7 +137,6 @@ HRESULT InitializeSymbolStore( return E_FAIL; } } - g_symbolStoreInitialized = true; return S_OK; } @@ -175,9 +174,13 @@ static void LoadNativeSymbolsCallback(void* param, const char* moduleFilePath, U \**********************************************************************/ HRESULT LoadNativeSymbols(bool runtimeOnly) { - if (g_symbolStoreInitialized) + ISymbolService* symbolService = GetSymbolService(); + if (symbolService != nullptr) { - return g_ExtServices2->LoadNativeSymbols(runtimeOnly, LoadNativeSymbolsCallback); + if (symbolService->IsSymbolStoreEnabled()) + { + return g_ExtServices2->LoadNativeSymbols(runtimeOnly, LoadNativeSymbolsCallback); + } } return E_FAIL; } @@ -189,9 +192,10 @@ HRESULT LoadNativeSymbols(bool runtimeOnly) \**********************************************************************/ void DisplaySymbolStore() { - if (g_symbolStoreInitialized) + ISymbolService* symbolService = GetSymbolService(); + if (symbolService != nullptr) { - GetSymbolService()->DisplaySymbolStore([] (const char* message) { + symbolService->DisplaySymbolStore([] (const char* message) { ExtOut(message); ExtOut("\n"); }); @@ -203,10 +207,10 @@ void DisplaySymbolStore() \**********************************************************************/ void DisableSymbolStore() { - if (g_symbolStoreInitialized) + ISymbolService* symbolService = GetSymbolService(); + if (symbolService != nullptr) { - g_symbolStoreInitialized = false; - GetSymbolService()->DisableSymbolStore(); + symbolService->DisableSymbolStore(); } } @@ -226,7 +230,6 @@ HRESULT GetMetadataLocator( { HRESULT Status = S_OK; IfFailRet(InitializeSymbolService()); - return GetSymbolService()->GetMetadataLocator(imagePath, imageTimestamp, imageSize, mvid, mdRva, flags, bufferSize, buffer, dataSize); } @@ -438,7 +441,7 @@ HRESULT SymbolReader::LoadSymbols(___in IMetaDataImport* pMD, ___in IXCLRDataMod ExtOut("LoadSymbols GetClrModuleImages FAILED 0x%08x\n", hr); return hr; } - if (!HasPortablePDB(moduleBase)) + if (GetSymbolService() == nullptr || !HasPortablePDB(moduleBase)) { hr = LoadSymbolsForWindowsPDB(pMD, moduleBase, pModuleName, FALSE); if (SUCCEEDED(hr)) @@ -453,9 +456,9 @@ HRESULT SymbolReader::LoadSymbols(___in IMetaDataImport* pMD, ___in IXCLRDataMod } #ifndef FEATURE_PAL - // TODO: in-memory windows PDB not supported - if (!HasPortablePDB(moduleData.LoadedPEAddress)) + if (GetSymbolService() == nullptr || !HasPortablePDB(moduleData.LoadedPEAddress)) { + // TODO: in-memory windows PDB not supported hr = LoadSymbolsForWindowsPDB(pMD, moduleData.LoadedPEAddress, pModuleName, moduleData.IsFileLayout); if (SUCCEEDED(hr)) { diff --git a/src/tests/Microsoft.Diagnostics.DebugServices.UnitTests/TestDataReader.cs b/src/tests/Microsoft.Diagnostics.DebugServices.UnitTests/TestDataReader.cs index 99f49d6dd..f7af9dd2d 100644 --- a/src/tests/Microsoft.Diagnostics.DebugServices.UnitTests/TestDataReader.cs +++ b/src/tests/Microsoft.Diagnostics.DebugServices.UnitTests/TestDataReader.cs @@ -71,6 +71,7 @@ namespace Microsoft.Diagnostics.DebugServices.UnitTests /// resulting object public static void GetValue(Type type, string valueString, ref object result) { + valueString = valueString.Trim(); if (type == typeof(string)) { result = valueString ?? ""; @@ -287,24 +288,24 @@ namespace Microsoft.Diagnostics.DebugServices.UnitTests if (nullableType != null && memberValue == null) { - memberValue = ""; + memberValue = string.Empty; } else if (memberType == typeof(string)) { - memberValue ??= ""; + memberValue ??= string.Empty; } else if (memberValue is ImmutableArray buildId) { memberType = typeof(string); - memberValue = !buildId.IsDefaultOrEmpty ? string.Concat(buildId.Select((b) => b.ToString("x2"))) : ""; + memberValue = !buildId.IsDefaultOrEmpty ? string.Concat(buildId.Select((b) => b.ToString("x2"))) : string.Empty; } else if (!memberType.IsPrimitive && !memberType.IsEnum) { memberType = typeof(string); - memberValue = memberValue?.ToString() ?? ""; + memberValue = memberValue?.ToString() ?? string.Empty; } object testDataValue = testData.Value.GetValue(memberType); - Trace.TraceInformation($"CompareMembers {testData.Key}: {memberValue} == {testDataValue}"); + Trace.TraceInformation($"CompareMembers {testData.Key}: '{memberValue}' == '{testDataValue}'"); Assert.Equal(memberValue, testDataValue); } }