Fixes issue #21484 createdump segfaults

Issue #21484: createdump segfaults with ASP.NET app The problem is the ClrDataModule Request faulted on a dynamic module getting the file layout flag. Fixed the Request code not get the file layout and in the crash dump code skip any dynamic modules. Issue #21485: fix EnumProcessModules hPseudoCurrentProcess bug.
dotnet · Jan 10, 2019 · 1cc38a9 · 1cc38a9
1 parent da58208
commit 1cc38a9
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 11 deletions.
diff --git a/src/debug/createdump/crashinfo.cpp b/src/debug/createdump/crashinfo.cpp
@@ -704,6 +704,17 @@ CrashInfo::EnumerateManagedModules(IXCLRDataProcess* pClrDataProcess)
             break;
         }
 
+        // Skip any dynamic modules. The Request call below on some DACs crashes on dynamic modules.
+        ULONG32 flags;
+        if ((hr = pClrDataModule->GetFlags(&flags)) != S_OK) {
+            TRACE("MODULE: GetFlags FAILED %08x\n", hr);
+            continue;
+        }
+        if (flags & CLRDATA_MODULE_IS_DYNAMIC) {
+            TRACE("MODULE: Skipping dynamic module\n");
+            continue;
+        }
+
         DacpGetModuleData moduleData;
         if (SUCCEEDED(hr = moduleData.Request(pClrDataModule.GetPtr())))
         {
@@ -715,17 +726,20 @@ CrashInfo::EnumerateManagedModules(IXCLRDataProcess* pClrDataProcess)
                 ArrayHolder<WCHAR> wszUnicodeName = new WCHAR[MAX_LONGPATH + 1];
                 if (SUCCEEDED(hr = pClrDataModule->GetFileName(MAX_LONGPATH, nullptr, wszUnicodeName)))
                 {
-                    char* pszName = (char*)malloc(MAX_LONGPATH + 1);
-                    if (pszName == nullptr) {
-                        fprintf(stderr, "Allocating module name FAILED\n");
-                        result = false;
-                        break;
+                    // If the module file name isn't empty
+                    if (wszUnicodeName[0] != 0) {
+                        char* pszName = (char*)malloc(MAX_LONGPATH + 1);
+                        if (pszName == nullptr) {
+                            fprintf(stderr, "Allocating module name FAILED\n");
+                            result = false;
+                            break;
+                        }
+                        sprintf_s(pszName, MAX_LONGPATH, "%S", (WCHAR*)wszUnicodeName);
+                        TRACE(" %s\n", pszName);
+
+                        // Change the module mapping name
+                        ReplaceModuleMapping(moduleData.LoadedPEAddress, pszName);
                     }
-                    sprintf_s(pszName, MAX_LONGPATH, "%S", (WCHAR*)wszUnicodeName);
-                    TRACE(" %s\n", pszName);
-
-                    // Change the module mapping name
-                    ReplaceModuleMapping(moduleData.LoadedPEAddress, pszName);
                 }
                 else {
                     TRACE("\nModule.GetFileName FAILED %08x\n", hr);

diff --git a/src/debug/daccess/task.cpp b/src/debug/daccess/task.cpp
@@ -2745,7 +2745,12 @@ ClrDataModule::RequestGetModuleData(
         COUNT_T peSize;
         outGMD->LoadedPEAddress = TO_CDADDR(PTR_TO_TADDR(pPEFile->GetLoadedImageContents(&peSize)));
         outGMD->LoadedPESize = (ULONG64)peSize;
-        outGMD->IsFileLayout = pPEFile->GetLoaded()->IsFlat();
+
+        // Can not get the file layout for a dynamic module
+        if (!outGMD->IsDynamic)
+        {
+            outGMD->IsFileLayout = pPEFile->GetLoaded()->IsFlat();
+        }
     }
 
     // If there is a in memory symbol stream

diff --git a/src/pal/src/thread/process.cpp b/src/pal/src/thread/process.cpp
@@ -2792,6 +2792,7 @@ GetProcessModulesFromHandle(
     if (hPseudoCurrentProcess == hProcess)
     {
         pobjProcess = g_pobjProcess;
+        pobjProcess->AddReference();
     }
     else
     {