From 3b5f4015e77187ac36a101d4548bbd77a82cd20e Mon Sep 17 00:00:00 2001 From: noahfalk Date: Tue, 5 Feb 2019 00:23:34 -0800 Subject: [PATCH] Fix various problems in the "clrstack -i" command that trash memory and fail the tests (work originally by mikem, I just split his mega-commit so history would a little more understandable) --- src/SOS/Strike/hostcoreclr.cpp | 32 +++++++++++++------------- src/SOS/Strike/metadata.cpp | 42 ++++------------------------------ src/SOS/Strike/strike.cpp | 18 +++++++++------ src/SOS/Strike/util.cpp | 38 ++++++++++++++++++++++++++++++ src/SOS/Strike/util.h | 1 + 5 files changed, 71 insertions(+), 60 deletions(-) diff --git a/src/SOS/Strike/hostcoreclr.cpp b/src/SOS/Strike/hostcoreclr.cpp index 7c8e06e03..7529414d0 100644 --- a/src/SOS/Strike/hostcoreclr.cpp +++ b/src/SOS/Strike/hostcoreclr.cpp @@ -643,14 +643,15 @@ void DisableSymbolStore() } } +/**********************************************************************\ + * Load symbols for an ICorDebugModule. Used by "clrstack -i". +\**********************************************************************/ HRESULT SymbolReader::LoadSymbols(___in IMetaDataImport* pMD, ___in ICorDebugModule* pModule) { HRESULT Status = S_OK; + BOOL isDynamic = FALSE; - BOOL isInMemory = FALSE; IfFailRet(pModule->IsDynamic(&isDynamic)); - IfFailRet(pModule->IsInMemory(&isInMemory)); - if (isDynamic) { // Dynamic and in memory assemblies are a special case which we will ignore for now @@ -659,23 +660,17 @@ HRESULT SymbolReader::LoadSymbols(___in IMetaDataImport* pMD, ___in ICorDebugMod } ULONG64 peAddress = 0; - ULONG32 peSize = 0; IfFailRet(pModule->GetBaseAddress(&peAddress)); - IfFailRet(pModule->GetSize(&peSize)); - ULONG32 len = 0; - WCHAR moduleName[MAX_LONGPATH]; - IfFailRet(pModule->GetName(_countof(moduleName), &len, moduleName)); + IXCLRDataModule* pClrModule; + IfFailRet(GetModuleFromAddress(peAddress, &pClrModule)); -#ifndef FEATURE_PAL - if (SUCCEEDED(LoadSymbolsForWindowsPDB(pMD, peAddress, moduleName, isInMemory))) - { - return S_OK; - } -#endif // FEATURE_PAL - return LoadSymbolsForPortablePDB(moduleName, isInMemory, isInMemory, peAddress, peSize, 0, 0); + return LoadSymbols(pMD, pClrModule); } +/**********************************************************************\ + * Load symbols for a module. +\**********************************************************************/ HRESULT SymbolReader::LoadSymbols(___in IMetaDataImport* pMD, ___in IXCLRDataModule* pModule) { ULONG32 flags; @@ -730,6 +725,9 @@ HRESULT SymbolReader::LoadSymbols(___in IMetaDataImport* pMD, ___in IXCLRDataMod #ifndef FEATURE_PAL +/**********************************************************************\ + * Attempts to load Windows PDBs on Windows. +\**********************************************************************/ HRESULT SymbolReader::LoadSymbolsForWindowsPDB(___in IMetaDataImport* pMD, ___in ULONG64 peAddress, __in_z WCHAR* pModuleName, ___in BOOL isFileLayout) { HRESULT Status = S_OK; @@ -740,7 +738,6 @@ HRESULT SymbolReader::LoadSymbolsForWindowsPDB(___in IMetaDataImport* pMD, ___in IfFailRet(CoInitialize(NULL)); // We now need a binder object that will take the module and return a - // reader object ToRelease pSymBinder; if (FAILED(Status = CreateInstanceCustom(CLSID_CorSymBinder_SxS, IID_ISymUnmanagedBinder3, @@ -801,6 +798,9 @@ HRESULT SymbolReader::LoadSymbolsForWindowsPDB(___in IMetaDataImport* pMD, ___in #endif // FEATURE_PAL +/**********************************************************************\ + * Attempts to load a portable or embeded PDB. Both Windows and xplat. +\**********************************************************************/ HRESULT SymbolReader::LoadSymbolsForPortablePDB(__in_z WCHAR* pModuleName, ___in BOOL isInMemory, ___in BOOL isFileLayout, ___in ULONG64 peAddress, ___in ULONG64 peSize, ___in ULONG64 inMemoryPdbAddress, ___in ULONG64 inMemoryPdbSize) { diff --git a/src/SOS/Strike/metadata.cpp b/src/SOS/Strike/metadata.cpp index 8502aaa45..77bccd52e 100644 --- a/src/SOS/Strike/metadata.cpp +++ b/src/SOS/Strike/metadata.cpp @@ -18,7 +18,6 @@ * * \**********************************************************************/ // Caller should guard against exception -// !!! mdName should have at least mdNameLen WCHAR static HRESULT NameForTypeDef_s(mdTypeDef tkTypeDef, IMetaDataImport *pImport, __out_ecount (capacity_mdName) WCHAR *mdName, size_t capacity_mdName) { @@ -26,7 +25,7 @@ static HRESULT NameForTypeDef_s(mdTypeDef tkTypeDef, IMetaDataImport *pImport, ULONG nameLen; HRESULT hr = pImport->GetTypeDefProps(tkTypeDef, mdName, - mdNameLen, &nameLen, + (ULONG)capacity_mdName, &nameLen, &flags, NULL); if (hr != S_OK) { return hr; @@ -42,54 +41,23 @@ static HRESULT NameForTypeDef_s(mdTypeDef tkTypeDef, IMetaDataImport *pImport, } WCHAR *name = (WCHAR*)_alloca((nameLen+1)*sizeof(WCHAR)); wcscpy_s (name, nameLen+1, mdName); - hr = NameForTypeDef_s(tkEnclosingClass,pImport,mdName, capacity_mdName); + hr = NameForTypeDef_s(tkEnclosingClass, pImport, mdName, capacity_mdName); if (hr != S_OK) { return hr; } size_t Len = _wcslen (mdName); - if (Len < mdNameLen-2) { + if (Len < capacity_mdName - 2) { mdName[Len++] = L'+'; mdName[Len] = L'\0'; } - Len = mdNameLen-1 - Len; + Len = capacity_mdName - 1 - Len; if (Len > nameLen) { Len = nameLen; } - wcsncat_s (mdName,capacity_mdName,name,Len); + wcsncat_s (mdName, capacity_mdName, name, Len); return hr; } -/**********************************************************************\ -* Routine Description: * -* * -* This function is called to find the name of a TypeDef using * -* metadata API. * -* * -\**********************************************************************/ -// Caller should guard against exception -// !!! mdName should have at least mdNameLen WCHAR -/* -static HRESULT NameForTypeDefNew(mdTypeDef tkTypeDef, IMDInternalImport *pImport, - WCHAR *mdName) -{ - DWORD flags; - ULONG nameLen; - char *name = (char *)_alloca((mdNameLen+1)*sizeof(char)); - char *namesp = (char *)_alloca((mdNameLen+1)*sizeof(char)); - - HRESULT hr = pImport->GetNameOfTypeDef(tkTypeDef, name, namesp); - if (FAILED(hr)) - { - return hr; - } - - strcpy (namesp, "."); - strcpy (namesp, name); - MultiByteToWideChar (CP_ACP,0,namesp,-1,mdName,mdNameLen); - return hr; -} -*/ - /**********************************************************************\ * Routine Description: * * * diff --git a/src/SOS/Strike/strike.cpp b/src/SOS/Strike/strike.cpp index bd2584ddd..2234586cd 100644 --- a/src/SOS/Strike/strike.cpp +++ b/src/SOS/Strike/strike.cpp @@ -12682,21 +12682,25 @@ public: break; } - CROSS_PLATFORM_CONTEXT context; + // This is a workaround for a problem in the MacOS DAC/DBI PAL. The PAL exception + // handling is unnecessarily enabled for DLLs and is not properly passing what I + // think is recoverable stack fault on to the OS. Instead it is causing a fault + // GP fault. Putting this struct in the heap works around this fault. + ArrayHolder context = new CROSS_PLATFORM_CONTEXT[1]; ULONG32 cbContextActual; - if ((Status=pStackWalk->GetContext( + if ((Status = pStackWalk->GetContext( DT_CONTEXT_FULL, - sizeof(context), + sizeof(CROSS_PLATFORM_CONTEXT), &cbContextActual, - (BYTE *)&context))!=S_OK) + (BYTE *)context.GetPtr())) != S_OK) { ExtOut("GetFrameContext failed: %lx\n",Status); break; } // First find the info for the Frame object, if the current frame has an associated clr!Frame. - CLRDATA_ADDRESS sp = GetSP(context); - CLRDATA_ADDRESS ip = GetIP(context); + CLRDATA_ADDRESS sp = GetSP(*context.GetPtr()); + CLRDATA_ADDRESS ip = GetIP(*context.GetPtr()); ToRelease pFrame; IfFailRet(pStackWalk->GetFrame(&pFrame)); @@ -12765,7 +12769,7 @@ public: IfFailRet(pFunction->GetModule(&pModule)); IfFailRet(pFunction->GetToken(&methodDef)); - WCHAR wszModuleName[100]; + WCHAR wszModuleName[MAX_LONGPATH]; ULONG32 cchModuleNameActual; IfFailRet(pModule->GetName(_countof(wszModuleName), &cchModuleNameActual, wszModuleName)); diff --git a/src/SOS/Strike/util.cpp b/src/SOS/Strike/util.cpp index cfe1159c8..74feb7903 100644 --- a/src/SOS/Strike/util.cpp +++ b/src/SOS/Strike/util.cpp @@ -2560,6 +2560,44 @@ Failure: return NULL; } +/**********************************************************************\ +* Routine Description: * +* * +* Find the IXCLRDataModule instance for the base address. * +* * +\**********************************************************************/ +HRESULT GetModuleFromAddress(___in CLRDATA_ADDRESS peAddress, ___out IXCLRDataModule** ppModule) +{ + HRESULT hr = E_FAIL; + *ppModule = nullptr; + + int numModule; + ArrayHolder moduleList = ModuleFromName(NULL, &numModule); + if (moduleList != nullptr) + { + for (int i = 0; i < numModule; i++) + { + ToRelease module; + hr = g_sos->GetModule(moduleList[i], &module); + if (FAILED(hr)) { + break; + } + DacpGetModuleData moduleData; + HRESULT hr = moduleData.Request(module); + if (FAILED(hr)) { + break; + } + if (peAddress == moduleData.LoadedPEAddress) + { + *ppModule = module.Detach(); + break; + } + } + } + + return hr; +} + /**********************************************************************\ * Routine Description: * * * diff --git a/src/SOS/Strike/util.h b/src/SOS/Strike/util.h index 883cd722c..9660e009a 100644 --- a/src/SOS/Strike/util.h +++ b/src/SOS/Strike/util.h @@ -1865,6 +1865,7 @@ BOOL TryGetMethodDescriptorForDelegate(CLRDATA_ADDRESS delegateAddr, CLRDATA_ADD * ArrayHolder class. */ DWORD_PTR *ModuleFromName(__in_opt LPSTR name, int *numModules); +HRESULT GetModuleFromAddress(___in CLRDATA_ADDRESS peAddress, ___out IXCLRDataModule** ppModule); void GetInfoFromName(DWORD_PTR ModuleAddr, const char* name); void GetInfoFromModule (DWORD_PTR ModuleAddr, ULONG token, DWORD_PTR *ret=NULL); -- 2.34.1