From d68589484b37a78901e0204aaaae0252af0d0900 Mon Sep 17 00:00:00 2001 From: Mike McLaughlin Date: Mon, 20 Nov 2017 15:02:22 -0800 Subject: [PATCH] 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. --- src/debug/createdump/crashinfo.cpp | 209 ++++++++++++++++++++---------------- src/debug/createdump/crashinfo.h | 8 +- src/debug/createdump/createdump.cpp | 8 +- src/debug/createdump/datatarget.cpp | 6 -- src/debug/createdump/main.cpp | 11 +- src/debug/createdump/threadinfo.cpp | 55 +++++----- 6 files changed, 160 insertions(+), 137 deletions(-) diff --git a/src/debug/createdump/crashinfo.cpp b/src/debug/createdump/crashinfo.cpp index ae3e70f..8064691 100644 --- a/src/debug/createdump/crashinfo.cpp +++ b/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(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(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(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(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)) { diff --git a/src/debug/createdump/crashinfo.h b/src/debug/createdump/crashinfo.h index d7c3f03..46bbcc3 100644 --- a/src/debug/createdump/crashinfo.h +++ b/src/debug/createdump/crashinfo.h @@ -32,7 +32,8 @@ private: 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 m_auxvValues; // auxv values std::vector m_auxvEntries; // full auxv entries @@ -46,7 +47,7 @@ public: 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 @@ private: 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); diff --git a/src/debug/createdump/createdump.cpp b/src/debug/createdump/createdump.cpp index f6ea1db..d95e008 100644 --- a/src/debug/createdump/createdump.cpp +++ b/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 = 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 = new CrashInfo(pid, dataTarget, true); - return CreateDumpCommon(programPath, dumpPathTemplate, minidumpType, crashInfo); -} \ No newline at end of file + return CreateDumpCommon(dumpPathTemplate, minidumpType, crashInfo); +} diff --git a/src/debug/createdump/datatarget.cpp b/src/debug/createdump/datatarget.cpp index 9bbfcc7..b0fb6eb 100644 --- a/src/debug/createdump/datatarget.cpp +++ b/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; diff --git a/src/debug/createdump/main.cpp b/src/debug/createdump/main.cpp index cb4d3c6..2caf3a8 100644 --- a/src/debug/createdump/main.cpp +++ b/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; } diff --git a/src/debug/createdump/threadinfo.cpp b/src/debug/createdump/threadinfo.cpp index 3e5a5cc..07d8193 100644 --- a/src/debug/createdump/threadinfo.cpp +++ b/src/debug/createdump/threadinfo.cpp @@ -126,9 +126,6 @@ ThreadInfo::UnwindNativeFrames(CrashInfo& crashInfo, CONTEXT* pContext) bool ThreadInfo::UnwindThread(CrashInfo& crashInfo, IXCLRDataProcess* pClrDataProcess) { - ReleaseHolder pTask; - ReleaseHolder pStackwalk; - TRACE("Unwind: thread %04x\n", Tid()); // Get starting native context for the thread @@ -138,33 +135,39 @@ ThreadInfo::UnwindThread(CrashInfo& crashInfo, IXCLRDataProcess* pClrDataProcess // Unwind the native frames at the top of the stack UnwindNativeFrames(crashInfo, &context); - // Get the managed stack walker for this thread - if (SUCCEEDED(pClrDataProcess->GetTaskByOSThreadID(Tid(), &pTask))) + if (pClrDataProcess != nullptr) { - pTask->CreateStackWalk( - CLRDATA_SIMPFRAME_UNRECOGNIZED | - CLRDATA_SIMPFRAME_MANAGED_METHOD | - CLRDATA_SIMPFRAME_RUNTIME_MANAGED_CODE | - CLRDATA_SIMPFRAME_RUNTIME_UNMANAGED_CODE, - &pStackwalk); - } + ReleaseHolder pTask; + ReleaseHolder pStackwalk; - // For each managed frame (if any) - if (pStackwalk != nullptr) - { - TRACE("Unwind: managed frames\n"); - do + // Get the managed stack walker for this thread + if (SUCCEEDED(pClrDataProcess->GetTaskByOSThreadID(Tid(), &pTask))) { - // Get the managed stack frame context - if (pStackwalk->GetContext(CONTEXT_ALL, sizeof(context), nullptr, (BYTE *)&context) != S_OK) { - TRACE("Unwind: stack walker GetContext FAILED\n"); - break; - } - - // Unwind all the native frames after the managed frame - UnwindNativeFrames(crashInfo, &context); + pTask->CreateStackWalk( + CLRDATA_SIMPFRAME_UNRECOGNIZED | + CLRDATA_SIMPFRAME_MANAGED_METHOD | + CLRDATA_SIMPFRAME_RUNTIME_MANAGED_CODE | + CLRDATA_SIMPFRAME_RUNTIME_UNMANAGED_CODE, + &pStackwalk); + } - } while (pStackwalk->Next() == S_OK); + // For each managed frame (if any) + if (pStackwalk != nullptr) + { + TRACE("Unwind: managed frames\n"); + do + { + // Get the managed stack frame context + if (pStackwalk->GetContext(CONTEXT_ALL, sizeof(context), nullptr, (BYTE *)&context) != S_OK) { + TRACE("Unwind: stack walker GetContext FAILED\n"); + break; + } + + // Unwind all the native frames after the managed frame + UnwindNativeFrames(crashInfo, &context); + + } while (pStackwalk->Next() == S_OK); + } } return true; -- 2.7.4