Fix a few of createdump bugs. (#15079)

1) Fix createdump p_vaddr bias/base problem.
2) createdump was loading the DAC from the directory where createdump was executed from and not from the libcoreclr.so path. Fixed this.
3) Don't use the dac if the libcoreclr.so module is not found.
4) Remove the QI of ICoreDebugDataTarget4 in datatarget.cpp. Forgot to do this when the VirtualUnwind function was removed. Caused crash.

Author: Mike McLaughlin

src/debug/createdump/crashinfo.cpp

@@ -146,7 +146,7 @@ CrashInfo::EnumerateAndSuspendThreads()
 // Gather all the necessary crash dump info.
 //
 bool
-CrashInfo::GatherCrashInfo(const char* programPath, MINIDUMP_TYPE minidumpType)
+CrashInfo::GatherCrashInfo(MINIDUMP_TYPE minidumpType)
 {
     // Get the process info
     if (!GetStatus(m_pid, &m_ppid, &m_tgid, &m_name))
@@ -206,7 +206,7 @@ CrashInfo::GatherCrashInfo(const char* programPath, MINIDUMP_TYPE minidumpType)
         }
     }
     // Gather all the useful memory regions from the DAC
-    if (!EnumerateMemoryRegionsWithDAC(programPath, minidumpType))
+    if (!EnumerateMemoryRegionsWithDAC(minidumpType))
     {
         return false;
     }
@@ -363,10 +363,21 @@ CrashInfo::EnumerateModuleMappings()
             }
             MemoryRegion memoryRegion(regionFlags, start, end, offset, moduleName);
 
-            if (moduleName != nullptr && *moduleName == '/') {
+            if (moduleName != nullptr && *moduleName == '/')
+            {
+                if (m_coreclrPath.empty())
+                {
+                    std::string coreclrPath;
+                    coreclrPath.append(moduleName);
+                    size_t last = coreclrPath.rfind(MAKEDLLNAME_A("coreclr"));
+                    if (last != -1) {
+                        m_coreclrPath = coreclrPath.substr(0, last);
+                    }
+                }
                 m_moduleMappings.insert(memoryRegion);
             }
-            else {
+            else 
+            {
                 m_otherMappings.insert(memoryRegion);
             }
             if (linuxGateAddress != nullptr && reinterpret_cast<void*>(start) == linuxGateAddress)
@@ -412,40 +423,15 @@ CrashInfo::GetDSOInfo()
         return false;
     }
     uint64_t baseAddress = (uint64_t)phdrAddr - sizeof(Ehdr);
-    TRACE("DSO: base %" PRIA PRIx64 " phdr %p phnum %d\n", baseAddress, phdrAddr, phnum);
-
-    // Search for the program PT_DYNAMIC header 
     ElfW(Dyn)* dynamicAddr = nullptr;
-    for (int i = 0; i < phnum; i++, phdrAddr++)
-    {
-        Phdr ph;
-        if (!ReadMemory(phdrAddr, &ph, sizeof(ph))) {
-            fprintf(stderr, "ReadMemory(%p, %" PRIx ") phdr FAILED\n", phdrAddr, sizeof(ph));
-            return false;
-        }
-        TRACE("DSO: phdr %p type %d (%x) vaddr %" PRIxA " memsz %" PRIxA " offset %" PRIxA "\n",
-            phdrAddr, ph.p_type, ph.p_type, ph.p_vaddr, ph.p_memsz, ph.p_offset);
 
-        switch (ph.p_type)
-        {
-        case PT_DYNAMIC:
-            dynamicAddr = reinterpret_cast<ElfW(Dyn)*>(ph.p_vaddr);
-            break;
-
-        case PT_NOTE:
-        case PT_GNU_EH_FRAME:
-            if (ph.p_vaddr != 0 && ph.p_memsz != 0) {
-                InsertMemoryRegion(ph.p_vaddr, ph.p_memsz);
-            }
-            break;
+    TRACE("DSO: base %" PRIA PRIx64 " phdr %p phnum %d\n", baseAddress, phdrAddr, phnum);
 
-        case PT_LOAD:
-            MemoryRegion region(0, ph.p_vaddr, ph.p_vaddr + ph.p_memsz, baseAddress);
-            m_moduleAddresses.insert(region);
-            break;
-        }
+    // Enumerate program headers searching for the PT_DYNAMIC header, etc.
+    if (!EnumerateProgramHeaders(phdrAddr, phnum, baseAddress, &dynamicAddr))
+    {
+        return false;
     }
-
     if (dynamicAddr == nullptr) {
         return false;
     }
@@ -542,32 +528,70 @@ CrashInfo::GetELFInfo(uint64_t baseAddress)
     {
         Phdr* phdrAddr = reinterpret_cast<Phdr*>(baseAddress + ehdr.e_phoff);
 
-        // Add the program headers and search for the module's note and unwind info segments
-        for (int i = 0; i < phnum; i++, phdrAddr++)
+        if (!EnumerateProgramHeaders(phdrAddr, phnum, baseAddress, nullptr))
         {
-            Phdr ph;
-            if (!ReadMemory(phdrAddr, &ph, sizeof(ph))) {
-                fprintf(stderr, "ReadMemory(%p, %" PRIx ") phdr FAILED\n", phdrAddr, sizeof(ph));
-                return false;
-            }
-            TRACE("ELF: phdr %p type %d (%x) vaddr %" PRIxA " memsz %" PRIxA " paddr %" PRIxA " filesz %" PRIxA " offset %" PRIxA " align %" PRIxA "\n",
-                phdrAddr, ph.p_type, ph.p_type, ph.p_vaddr, ph.p_memsz, ph.p_paddr, ph.p_filesz, ph.p_offset, ph.p_align);
+            return false;
+        }
+    }
+
+    return true;
+}
+
+//
+// Enumerate the program headers adding the build id note, unwind frame 
+// region and module addresses to the crash info.
+//
+bool
+CrashInfo::EnumerateProgramHeaders(Phdr* phdrAddr, int phnum, uint64_t baseAddress, ElfW(Dyn)** pdynamicAddr)
+{
+    uint64_t loadbias = baseAddress;
 
-            switch (ph.p_type)
+    for (int i = 0; i < phnum; i++)
+    {
+        Phdr ph;
+        if (!ReadMemory(phdrAddr + i, &ph, sizeof(ph))) {
+            fprintf(stderr, "ReadMemory(%p, %" PRIx ") phdr FAILED\n", phdrAddr + i, sizeof(ph));
+            return false;
+        }
+        if (ph.p_type == PT_LOAD && ph.p_offset == 0)
+        {
+            loadbias -= ph.p_vaddr;
+            TRACE("PHDR: loadbias %" PRIA PRIx64 "\n", loadbias);
+            break;
+        }
+    }
+
+    for (int i = 0; i < phnum; i++)
+    {
+        Phdr ph;
+        if (!ReadMemory(phdrAddr + i, &ph, sizeof(ph))) {
+            fprintf(stderr, "ReadMemory(%p, %" PRIx ") phdr FAILED\n", phdrAddr + i, sizeof(ph));
+            return false;
+        }
+        TRACE("PHDR: %p type %d (%x) vaddr %" PRIxA " memsz %" PRIxA " paddr %" PRIxA " filesz %" PRIxA " offset %" PRIxA " align %" PRIxA "\n",
+            phdrAddr + i, ph.p_type, ph.p_type, ph.p_vaddr, ph.p_memsz, ph.p_paddr, ph.p_filesz, ph.p_offset, ph.p_align);
+
+        switch (ph.p_type)
+        {
+        case PT_DYNAMIC:
+            if (pdynamicAddr != nullptr)
             {
-            case PT_DYNAMIC:
-            case PT_NOTE:
-            case PT_GNU_EH_FRAME:
-                if (ph.p_vaddr != 0 && ph.p_memsz != 0) {
-                    InsertMemoryRegion(baseAddress + ph.p_vaddr, ph.p_memsz);
-                }
+                *pdynamicAddr = reinterpret_cast<ElfW(Dyn)*>(loadbias + ph.p_vaddr);
                 break;
+            }
+            // fall into InsertMemoryRegion
 
-            case PT_LOAD:
-                MemoryRegion region(0, baseAddress + ph.p_vaddr, baseAddress + ph.p_vaddr + ph.p_memsz, baseAddress);
-                m_moduleAddresses.insert(region);
-                break;
+        case PT_NOTE:
+        case PT_GNU_EH_FRAME:
+            if (ph.p_vaddr != 0 && ph.p_memsz != 0) {
+                InsertMemoryRegion(loadbias + ph.p_vaddr, ph.p_memsz);
             }
+            break;
+
+        case PT_LOAD:
+            MemoryRegion region(0, loadbias + ph.p_vaddr, loadbias + ph.p_vaddr + ph.p_memsz, baseAddress);
+            m_moduleAddresses.insert(region);
+            break;
         }
     }
 
@@ -578,7 +602,7 @@ CrashInfo::GetELFInfo(uint64_t baseAddress)
 // Enumerate all the memory regions using the DAC memory region support given a minidump type
 //
 bool
-CrashInfo::EnumerateMemoryRegionsWithDAC(const char* programPath, MINIDUMP_TYPE minidumpType)
+CrashInfo::EnumerateMemoryRegionsWithDAC(MINIDUMP_TYPE minidumpType)
 {
     PFN_CLRDataCreateInstance pfnCLRDataCreateInstance = nullptr;
     ICLRDataEnumMemoryRegions* pClrDataEnumRegions = nullptr;
@@ -587,50 +611,55 @@ CrashInfo::EnumerateMemoryRegionsWithDAC(const char* programPath, MINIDUMP_TYPE
     HRESULT hr = S_OK;
     bool result = false;
 
-    // We assume that the DAC is in the same location as this createdump exe
-    std::string dacPath;
-    dacPath.append(programPath);
-    dacPath.append("/");
-    dacPath.append(MAKEDLLNAME_A("mscordaccore"));
-
-    // Load and initialize the DAC
-    hdac = LoadLibraryA(dacPath.c_str());
-    if (hdac == nullptr)
-    {
-        fprintf(stderr, "LoadLibraryA(%s) FAILED %d\n", dacPath.c_str(), GetLastError());
-        goto exit;
-    }
-    pfnCLRDataCreateInstance = (PFN_CLRDataCreateInstance)GetProcAddress(hdac, "CLRDataCreateInstance");
-    if (pfnCLRDataCreateInstance == nullptr)
-    {
-        fprintf(stderr, "GetProcAddress(CLRDataCreateInstance) FAILED %d\n", GetLastError());
-        goto exit;
-    }
-    if ((minidumpType & MiniDumpWithFullMemory) == 0)
+    if (!m_coreclrPath.empty())
     {
-        hr = pfnCLRDataCreateInstance(__uuidof(ICLRDataEnumMemoryRegions), m_dataTarget, (void**)&pClrDataEnumRegions);
-        if (FAILED(hr))
+        // We assume that the DAC is in the same location as the libcoreclr.so module
+        std::string dacPath;
+        dacPath.append(m_coreclrPath);
+        dacPath.append(MAKEDLLNAME_A("mscordaccore"));
+
+        // Load and initialize the DAC
+        hdac = LoadLibraryA(dacPath.c_str());
+        if (hdac == nullptr)
+        {
+            fprintf(stderr, "LoadLibraryA(%s) FAILED %d\n", dacPath.c_str(), GetLastError());
+            goto exit;
+        }
+        pfnCLRDataCreateInstance = (PFN_CLRDataCreateInstance)GetProcAddress(hdac, "CLRDataCreateInstance");
+        if (pfnCLRDataCreateInstance == nullptr)
         {
-            fprintf(stderr, "CLRDataCreateInstance(ICLRDataEnumMemoryRegions) FAILED %08x\n", hr);
+            fprintf(stderr, "GetProcAddress(CLRDataCreateInstance) FAILED %d\n", GetLastError());
             goto exit;
         }
-        // Calls CrashInfo::EnumMemoryRegion for each memory region found by the DAC
-        hr = pClrDataEnumRegions->EnumMemoryRegions(this, minidumpType, CLRDATA_ENUM_MEM_DEFAULT);
+        if ((minidumpType & MiniDumpWithFullMemory) == 0)
+        {
+            hr = pfnCLRDataCreateInstance(__uuidof(ICLRDataEnumMemoryRegions), m_dataTarget, (void**)&pClrDataEnumRegions);
+            if (FAILED(hr))
+            {
+                fprintf(stderr, "CLRDataCreateInstance(ICLRDataEnumMemoryRegions) FAILED %08x\n", hr);
+                goto exit;
+            }
+            // Calls CrashInfo::EnumMemoryRegion for each memory region found by the DAC
+            hr = pClrDataEnumRegions->EnumMemoryRegions(this, minidumpType, CLRDATA_ENUM_MEM_DEFAULT);
+            if (FAILED(hr))
+            {
+                fprintf(stderr, "EnumMemoryRegions FAILED %08x\n", hr);
+                goto exit;
+            }
+        }
+        hr = pfnCLRDataCreateInstance(__uuidof(IXCLRDataProcess), m_dataTarget, (void**)&pClrDataProcess);
         if (FAILED(hr))
         {
-            fprintf(stderr, "EnumMemoryRegions FAILED %08x\n", hr);
+            fprintf(stderr, "CLRDataCreateInstance(IXCLRDataProcess) FAILED %08x\n", hr);
+            goto exit;
+        }
+        if (!EnumerateManagedModules(pClrDataProcess))
+        {
             goto exit;
         }
     }
-    hr = pfnCLRDataCreateInstance(__uuidof(IXCLRDataProcess), m_dataTarget, (void**)&pClrDataProcess);
-    if (FAILED(hr))
-    {
-        fprintf(stderr, "CLRDataCreateInstance(IXCLRDataProcess) FAILED %08x\n", hr);
-        goto exit;
-    }
-    if (!EnumerateManagedModules(pClrDataProcess))
-    {
-        goto exit;
+    else {
+        TRACE("EnumerateMemoryRegionsWithDAC: coreclr not found; not using DAC\n");
     }
     if (!UnwindAllThreads(pClrDataProcess))
     {

src/debug/createdump/crashinfo.h

@@ -32,7 +32,8 @@ class CrashInfo : public ICLRDataEnumMemoryRegionsCallback
     pid_t m_ppid;                                   // parent pid
     pid_t m_tgid;                                   // process group
     char* m_name;                                   // exe name
-    bool m_sos;                                     // true if running under sos
+    bool m_sos;                                     // if true, running under sos
+    std::string m_coreclrPath;                      // the path of the coreclr module or empty if none
     ICLRDataTarget* m_dataTarget;                   // read process memory, etc.
     std::array<elf_aux_val_t, AT_MAX> m_auxvValues; // auxv values
     std::vector<elf_aux_entry> m_auxvEntries;       // full auxv entries
@@ -46,7 +47,7 @@ class CrashInfo : public ICLRDataEnumMemoryRegionsCallback
     CrashInfo(pid_t pid, ICLRDataTarget* dataTarget, bool sos);
     virtual ~CrashInfo();
     bool EnumerateAndSuspendThreads();
-    bool GatherCrashInfo(const char* programPath, MINIDUMP_TYPE minidumpType);
+    bool GatherCrashInfo(MINIDUMP_TYPE minidumpType);
     void ResumeThreads();
     bool ReadMemory(void* address, void* buffer, size_t size);
     uint64_t GetBaseAddress(uint64_t ip);
@@ -80,7 +81,8 @@ class CrashInfo : public ICLRDataEnumMemoryRegionsCallback
     bool EnumerateModuleMappings();
     bool GetDSOInfo();
     bool GetELFInfo(uint64_t baseAddress);
-    bool EnumerateMemoryRegionsWithDAC(const char* programPath, MINIDUMP_TYPE minidumpType);
+    bool EnumerateProgramHeaders(ElfW(Phdr)* phdrAddr, int phnum, uint64_t baseAddress, ElfW(Dyn)** pdynamicAddr);
+    bool EnumerateMemoryRegionsWithDAC(MINIDUMP_TYPE minidumpType);
     bool EnumerateManagedModules(IXCLRDataProcess* pClrDataProcess);
     bool UnwindAllThreads(IXCLRDataProcess* pClrDataProcess);
     void ReplaceModuleMapping(CLRDATA_ADDRESS baseAddress, const char* pszName);

src/debug/createdump/createdump.cpp

@@ -10,7 +10,7 @@ bool g_diagnostics = false;
 // The common create dump code
 //
 bool 
-CreateDumpCommon(const char* programPath, const char* dumpPathTemplate, MINIDUMP_TYPE minidumpType, CrashInfo* crashInfo)
+CreateDumpCommon(const char* dumpPathTemplate, MINIDUMP_TYPE minidumpType, CrashInfo* crashInfo)
 {
     ReleaseHolder<DumpWriter> dumpWriter = new DumpWriter(*crashInfo);
     bool result = false;
@@ -44,7 +44,7 @@ CreateDumpCommon(const char* programPath, const char* dumpPathTemplate, MINIDUMP
         goto exit;
     }
     // Gather all the info about the process, threads (registers, etc.) and memory regions
-    if (!crashInfo->GatherCrashInfo(programPath, minidumpType))
+    if (!crashInfo->GatherCrashInfo(minidumpType))
     {
         goto exit;
     }
@@ -69,5 +69,5 @@ bool
 CreateDumpForSOS(const char* programPath, const char* dumpPathTemplate, pid_t pid, MINIDUMP_TYPE minidumpType, ICLRDataTarget* dataTarget)
 {
     ReleaseHolder<CrashInfo> crashInfo = new CrashInfo(pid, dataTarget, true);
-    return CreateDumpCommon(programPath, dumpPathTemplate, minidumpType, crashInfo);
-}
+    return CreateDumpCommon(dumpPathTemplate, minidumpType, crashInfo);
+}

src/debug/createdump/datatarget.cpp

@@ -52,12 +52,6 @@ DumpDataTarget::QueryInterface(
         AddRef();
         return S_OK;
     }
-    else if (InterfaceId == IID_ICorDebugDataTarget4)
-    {
-        *Interface = (ICorDebugDataTarget4*)this;
-        AddRef();
-        return S_OK;
-    }
     else
     {
         *Interface = NULL;

src/debug/createdump/main.cpp

@@ -12,7 +12,7 @@ const char* g_help = "createdump [options] pid\n"
 "-u, --full - create full core dump.\n" 
 "-d, --diag - enable diagnostic messages.\n";
 
-bool CreateDumpCommon(const char* programPath, const char* dumpPathTemplate, MINIDUMP_TYPE minidumpType, CrashInfo* crashInfo);
+bool CreateDumpCommon(const char* dumpPathTemplate, MINIDUMP_TYPE minidumpType, CrashInfo* crashInfo);
 
 //
 // Main entry point
@@ -34,13 +34,8 @@ int __cdecl main(const int argc, const char* argv[])
         return exitCode;
     }
 
-    // Parse off the program name leaving just the path. Used to locate/load the DAC module.
-    std::string programPath;
-    programPath.append(*argv++);
-    size_t last = programPath.find_last_of('/');
-    programPath = programPath.substr(0, last);
-
     // Parse the command line options and target pid
+    argv++;
     for (int i = 1; i < argc; i++)
     {
         if (*argv != nullptr)
@@ -83,7 +78,7 @@ int __cdecl main(const int argc, const char* argv[])
         // The initialize the data target's ReadVirtual support (opens /proc/$pid/mem)
         if (dataTarget->Initialize(crashInfo))
         {
-            if (!CreateDumpCommon(programPath.c_str(), dumpPathTemplate, minidumpType, crashInfo))
+            if (!CreateDumpCommon(dumpPathTemplate, minidumpType, crashInfo))
             {
                 exitCode = -1;
             }