Fix various problems in the "clrstack -i" command that trash memory and fail the...

(work originally by mikem, I just split his mega-commit so history would a little more understandable)

diff --git a/src/SOS/Strike/hostcoreclr.cpp b/src/SOS/Strike/hostcoreclr.cpp
index 7c8e06e03dba8943e64eba09fd65387b206d0b90..7529414d07e9e8a615cd72a745c93cfcf3333dbe 100644 (file)
--- 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<ISymUnmanagedBinder3> 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 8502aaa45b0cfa45b4a451b2b5665b858375f5c8..77bccd52ec9092541f7b47bdaab22d04e31797df 100644 (file)
--- 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 bd2584dddb5ef54dd5b2733f04c4c228f3db489d..2234586cd00766a704d2ae0491b88769224f3338 100644 (file)
--- 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<CROSS_PLATFORM_CONTEXT> 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<ICorDebugFrame> 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 cfe1159c8397bbf580bd13fdc54bdb413bb0e2b3..74feb7903dd1b47f50579e8b42818977378a591e 100644 (file)
--- 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<DWORD_PTR> moduleList = ModuleFromName(NULL, &numModule);
+    if (moduleList != nullptr)
+    {
+        for (int i = 0; i < numModule; i++)
+        {
+            ToRelease<IXCLRDataModule> 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 883cd722ce8ec6d7784258679a0f22f8adfc6ad9..9660e009a79bed4be824df3086ef6fd413a38371 100644 (file)
--- 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);