Skip to content

Commit

Permalink
Various createdump fixes and cleanup (#97253)
Browse files Browse the repository at this point in the history 
* Separate memory and module regions.

Move std::string m_fileName out of MemoryRegion into ModuleRegion that is derived from MemoryRegion.

Reduces createdump memory usage.

* Fix arm32 sign extension issues. Cleanup: make void* address parameters uint64_t.

* Don't add "(deleted)" files to the module list on Linux.

Issue: #90547

* Add deleted modules to other mappings
  • Loading branch information
@mikem8361
mikem8361 authored Jan 26, 2024
1 parent 82bea3f commit e8d76fb
Show file tree
Hide file tree
Showing 8 changed files with 148 additions and 88 deletions.
61 changes: 41 additions & 20 deletions src/coreclr/debug/createdump/crashinfo.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,8 @@ CrashInfo::EnumMemoryRegion(
/* [in] */ CLRDATA_ADDRESS address,
/* [in] */ ULONG32 size)
{
m_enumMemoryPagesAdded += InsertMemoryRegion((ULONG_PTR)address, size);
address = CONVERT_FROM_SIGN_EXTENDED(address);
m_enumMemoryPagesAdded += InsertMemoryRegion(address, size);
return S_OK;
}

Expand Down Expand Up @@ -449,21 +450,23 @@ CrashInfo::EnumerateManagedModules()
DacpGetModuleData moduleData;
if (SUCCEEDED(hr = moduleData.Request(pClrDataModule.GetPtr())))
{
TRACE("MODULE: %" PRIA PRIx64 " dyn %d inmem %d file %d pe %" PRIA PRIx64 " pdb %" PRIA PRIx64, (uint64_t)moduleData.LoadedPEAddress, moduleData.IsDynamic,
uint64_t loadedPEAddress = CONVERT_FROM_SIGN_EXTENDED(moduleData.LoadedPEAddress);

TRACE("MODULE: %" PRIA PRIx64 " dyn %d inmem %d file %d pe %" PRIA PRIx64 " pdb %" PRIA PRIx64, loadedPEAddress, moduleData.IsDynamic,
moduleData.IsInMemory, moduleData.IsFileLayout, (uint64_t)moduleData.PEAssembly, (uint64_t)moduleData.InMemoryPdbAddress);

if (!moduleData.IsDynamic && moduleData.LoadedPEAddress != 0)
if (!moduleData.IsDynamic && loadedPEAddress != 0)
{
ArrayHolder<WCHAR> wszUnicodeName = new WCHAR[MAX_LONGPATH + 1];
if (SUCCEEDED(hr = pClrDataModule->GetFileName(MAX_LONGPATH, nullptr, wszUnicodeName)))
{
std::string moduleName = ConvertString(wszUnicodeName.GetPtr());

// Change the module mapping name
AddOrReplaceModuleMapping(moduleData.LoadedPEAddress, moduleData.LoadedPESize, moduleName);
AddOrReplaceModuleMapping(loadedPEAddress, moduleData.LoadedPESize, moduleName);

// Add managed module info
AddModuleInfo(true, moduleData.LoadedPEAddress, pClrDataModule, moduleName);
AddModuleInfo(true, loadedPEAddress, pClrDataModule, moduleName);
}
else {
TRACE("\nModule.GetFileName FAILED %08x\n", hr);
Expand Down Expand Up @@ -517,21 +520,21 @@ CrashInfo::UnwindAllThreads()
// Replace an existing module mapping with one with a different name.
//
void
CrashInfo::AddOrReplaceModuleMapping(CLRDATA_ADDRESS baseAddress, ULONG64 size, const std::string& name)
CrashInfo::AddOrReplaceModuleMapping(uint64_t baseAddress, uint64_t size, const std::string& name)
{
// Round to page boundary (single-file managed assemblies are not page aligned)
ULONG_PTR start = ((ULONG_PTR)baseAddress) & PAGE_MASK;
uint64_t start = baseAddress & PAGE_MASK;
assert(start > 0);

// Round up to page boundary
ULONG_PTR end = ((baseAddress + size) + (PAGE_SIZE - 1)) & PAGE_MASK;
uint64_t end = ((baseAddress + size) + (PAGE_SIZE - 1)) & PAGE_MASK;
assert(end > 0);

uint32_t flags = GetMemoryRegionFlags((ULONG_PTR)baseAddress);
uint32_t flags = GetMemoryRegionFlags(baseAddress);

// Make sure that the page containing the PE header for the managed asseblies is in the dump
// Make sure that the page containing the PE header for the managed assemblies is in the dump
// especially on MacOS where they are added artificially.
MemoryRegion header(flags, start, start + PAGE_SIZE);
ModuleRegion header(flags, start, start + PAGE_SIZE);
InsertMemoryRegion(header);

// Add or change the module mapping for this PE image. The managed assembly images may already
Expand All @@ -541,7 +544,7 @@ CrashInfo::AddOrReplaceModuleMapping(CLRDATA_ADDRESS baseAddress, ULONG64 size,
if (found == m_moduleMappings.end())
{
// On MacOS the assemblies are always added.
MemoryRegion newRegion(flags, start, end, 0, name);
ModuleRegion newRegion(flags, start, end, 0, name);
m_moduleMappings.insert(newRegion);
m_cbModuleMappings += newRegion.Size();

Expand All @@ -552,7 +555,7 @@ CrashInfo::AddOrReplaceModuleMapping(CLRDATA_ADDRESS baseAddress, ULONG64 size,
else if (found->FileName().compare(name) != 0)
{
// Create the new memory region with the managed assembly name.
MemoryRegion newRegion(*found, name);
ModuleRegion newRegion(*found, name);

// Remove and cleanup the old one
m_moduleMappings.erase(found);
Expand Down Expand Up @@ -648,15 +651,15 @@ CrashInfo::AddModuleInfo(bool isManaged, uint64_t baseAddress, IXCLRDataModule*
if (isManaged)
{
IMAGE_DOS_HEADER dosHeader;
if (ReadMemory((void*)baseAddress, &dosHeader, sizeof(dosHeader)))
if (ReadMemory(baseAddress, &dosHeader, sizeof(dosHeader)))
{
WORD magic;
if (ReadMemory((void*)(baseAddress + dosHeader.e_lfanew + offsetof(IMAGE_NT_HEADERS, OptionalHeader.Magic)), &magic, sizeof(magic)))
if (ReadMemory(baseAddress + dosHeader.e_lfanew + offsetof(IMAGE_NT_HEADERS, OptionalHeader.Magic), &magic, sizeof(magic)))
{
if (magic == IMAGE_NT_OPTIONAL_HDR32_MAGIC)
{
IMAGE_NT_HEADERS32 header;
if (ReadMemory((void*)(baseAddress + dosHeader.e_lfanew), &header, sizeof(header)))
if (ReadMemory(baseAddress + dosHeader.e_lfanew, &header, sizeof(header)))
{
imageSize = header.OptionalHeader.SizeOfImage;
timeStamp = header.FileHeader.TimeDateStamp;
Expand All @@ -665,7 +668,7 @@ CrashInfo::AddModuleInfo(bool isManaged, uint64_t baseAddress, IXCLRDataModule*
else if (magic == IMAGE_NT_OPTIONAL_HDR64_MAGIC)
{
IMAGE_NT_HEADERS64 header;
if (ReadMemory((void*)(baseAddress + dosHeader.e_lfanew), &header, sizeof(header)))
if (ReadMemory(baseAddress + dosHeader.e_lfanew, &header, sizeof(header)))
{
imageSize = header.OptionalHeader.SizeOfImage;
timeStamp = header.FileHeader.TimeDateStamp;
Expand Down Expand Up @@ -694,15 +697,15 @@ CrashInfo::AddModuleInfo(bool isManaged, uint64_t baseAddress, IXCLRDataModule*
// ReadMemory from target and add to memory regions list
//
bool
CrashInfo::ReadMemory(void* address, void* buffer, size_t size)
CrashInfo::ReadMemory(uint64_t address, void* buffer, size_t size)
{
size_t read = 0;
if (!ReadProcessMemory(address, buffer, size, &read))
{
return false;
}
assert(read == size);
InsertMemoryRegion(reinterpret_cast<uint64_t>(address), read);
InsertMemoryRegion(address, read);
return true;
}

Expand All @@ -712,6 +715,7 @@ CrashInfo::ReadMemory(void* address, void* buffer, size_t size)
int
CrashInfo::InsertMemoryRegion(uint64_t address, size_t size)
{
assert(address == CONVERT_FROM_SIGN_EXTENDED(address));
assert(size < UINT_MAX);

// Round to page boundary
Expand Down Expand Up @@ -831,7 +835,7 @@ CrashInfo::PageCanBeRead(uint64_t start)
{
BYTE buffer[1];
size_t read;
return ReadProcessMemory((void*)start, buffer, 1, &read);
return ReadProcessMemory(start, buffer, 1, &read);
}

//
Expand Down Expand Up @@ -889,6 +893,23 @@ CrashInfo::CombineMemoryRegions()
}
}

//
// Searches for a module region for a given address.
//
const ModuleRegion*
CrashInfo::SearchModuleRegions(const ModuleRegion& search)
{
std::set<ModuleRegion>::iterator found = m_moduleMappings.find(search);
for (; found != m_moduleMappings.end(); found++)
{
if (search.StartAddress() >= found->StartAddress() && search.StartAddress() < found->EndAddress())
{
return &*found;
}
}
return nullptr;
}

//
// Searches for a memory region given an address.
//
Expand Down
12 changes: 7 additions & 5 deletions src/coreclr/debug/createdump/crashinfo.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ class CrashInfo : public ICLRDataEnumMemoryRegionsCallback, public ICLRDataLoggi
std::vector<elf_aux_entry> m_auxvEntries; // full auxv entries
#endif
std::vector<ThreadInfo*> m_threads; // threads found and suspended
std::set<MemoryRegion> m_moduleMappings; // module memory mappings
std::set<ModuleRegion> m_moduleMappings; // module memory mappings
std::set<MemoryRegion> m_otherMappings; // other memory mappings
std::set<MemoryRegion> m_memoryRegions; // memory regions from DAC, etc.
std::set<MemoryRegion> m_moduleAddresses; // memory region to module base address
Expand All @@ -97,14 +97,15 @@ class CrashInfo : public ICLRDataEnumMemoryRegionsCallback, public ICLRDataLoggi
bool GatherCrashInfo(DumpType dumpType);
void CombineMemoryRegions();
bool EnumerateMemoryRegionsWithDAC(DumpType dumpType);
bool ReadMemory(void* address, void* buffer, size_t size); // read memory and add to dump
bool ReadProcessMemory(void* address, void* buffer, size_t size, size_t* read); // read raw memory
bool ReadMemory(uint64_t address, void* buffer, size_t size); // read memory and add to dump
bool ReadProcessMemory(uint64_t address, void* buffer, size_t size, size_t* read); // read raw memory
uint64_t GetBaseAddressFromAddress(uint64_t address);
uint64_t GetBaseAddressFromName(const char* moduleName);
ModuleInfo* GetModuleInfoFromBaseAddress(uint64_t baseAddress);
void AddModuleAddressRange(uint64_t startAddress, uint64_t endAddress, uint64_t baseAddress);
void AddModuleInfo(bool isManaged, uint64_t baseAddress, IXCLRDataModule* pClrDataModule, const std::string& moduleName);
int InsertMemoryRegion(uint64_t address, size_t size);
const ModuleRegion* SearchModuleRegions(const ModuleRegion& search);
static const MemoryRegion* SearchMemoryRegions(const std::set<MemoryRegion>& regions, const MemoryRegion& search);

inline pid_t Pid() const { return m_pid; }
Expand All @@ -122,14 +123,15 @@ class CrashInfo : public ICLRDataEnumMemoryRegionsCallback, public ICLRDataLoggi
inline const uint64_t RuntimeBaseAddress() const { return m_runtimeBaseAddress; }

inline const std::vector<ThreadInfo*>& Threads() const { return m_threads; }
inline const std::set<MemoryRegion>& ModuleMappings() const { return m_moduleMappings; }
inline const std::set<ModuleRegion>& ModuleMappings() const { return m_moduleMappings; }
inline const std::set<MemoryRegion>& OtherMappings() const { return m_otherMappings; }
inline const std::set<MemoryRegion>& MemoryRegions() const { return m_memoryRegions; }
inline const siginfo_t* SigInfo() const { return &m_siginfo; }
#ifndef __APPLE__
inline const std::vector<elf_aux_entry>& AuxvEntries() const { return m_auxvEntries; }
inline size_t GetAuxvSize() const { return m_auxvEntries.size() * sizeof(elf_aux_entry); }
#endif
bool ReadMemory(void* address, void* buffer, size_t size) { return ReadMemory((uint64_t)address, buffer, size); }

// IUnknown
STDMETHOD(QueryInterface)(___in REFIID InterfaceId, ___out PVOID* Interface);
Expand Down Expand Up @@ -159,7 +161,7 @@ class CrashInfo : public ICLRDataEnumMemoryRegionsCallback, public ICLRDataLoggi
bool InitializeDAC(DumpType dumpType);
bool EnumerateManagedModules();
bool UnwindAllThreads();
void AddOrReplaceModuleMapping(CLRDATA_ADDRESS baseAddress, ULONG64 size, const std::string& pszName);
void AddOrReplaceModuleMapping(uint64_t baseAddress, uint64_t size, const std::string& pszName);
int InsertMemoryRegion(const MemoryRegion& region);
uint32_t GetMemoryRegionFlags(uint64_t start);
bool PageCanBeRead(uint64_t start);
Expand Down
20 changes: 11 additions & 9 deletions src/coreclr/debug/createdump/crashinfomac.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ CrashInfo::InitializeOtherMappings()
// to include all the native and managed module regions.
for (const MemoryRegion& region : m_allMemoryRegions)
{
std::set<MemoryRegion>::iterator found = m_moduleMappings.find(region);
std::set<ModuleRegion>::iterator found = m_moduleMappings.find(ModuleRegion(region));
if (found == m_moduleMappings.end())
{
m_otherMappings.insert(region);
Expand Down Expand Up @@ -256,7 +256,7 @@ void CrashInfo::VisitModule(MachOModule& module)
m_runtimeBaseAddress = module.BaseAddress();

RuntimeInfo runtimeInfo { };
if (ReadMemory((void*)(module.BaseAddress() + symbolOffset), &runtimeInfo, sizeof(RuntimeInfo)))
if (ReadMemory(module.BaseAddress() + symbolOffset, &runtimeInfo, sizeof(RuntimeInfo)))
{
if (strcmp(runtimeInfo.Signature, RUNTIME_INFO_SIGNATURE) == 0)
{
Expand All @@ -274,7 +274,7 @@ void CrashInfo::VisitModule(MachOModule& module)
m_runtimeBaseAddress = module.BaseAddress();

uint8_t cookie[sizeof(g_debugHeaderCookie)];
if (ReadMemory((void*)(module.BaseAddress() + symbolOffset), cookie, sizeof(cookie)))
if (ReadMemory(module.BaseAddress() + symbolOffset, cookie, sizeof(cookie)))
{
if (memcmp(cookie, g_debugHeaderCookie, sizeof(g_debugHeaderCookie)) == 0)
{
Expand Down Expand Up @@ -315,8 +315,8 @@ void CrashInfo::VisitSegment(MachOModule& module, const segment_command_64& segm
assert(end > 0);

// Add module memory region if not already on the list
MemoryRegion newModule(regionFlags, start, end, offset, module.Name());
std::set<MemoryRegion>::iterator existingModule = m_moduleMappings.find(newModule);
ModuleRegion newModule(regionFlags, start, end, offset, module.Name());
std::set<ModuleRegion>::iterator existingModule = m_moduleMappings.find(newModule);
if (existingModule == m_moduleMappings.end())
{
if (g_diagnosticsVerbose)
Expand All @@ -340,7 +340,7 @@ void CrashInfo::VisitSegment(MachOModule& module, const segment_command_64& segm
uint64_t numberPages = newModule.SizeInPages();
for (size_t p = 0; p < numberPages; p++, start += PAGE_SIZE, offset += PAGE_SIZE)
{
MemoryRegion gap(newModule.Flags(), start, start + PAGE_SIZE, offset, newModule.FileName());
ModuleRegion gap(newModule.Flags(), start, start + PAGE_SIZE, offset, newModule.FileName());

const auto& found = m_moduleMappings.find(gap);
if (found != m_moduleMappings.end())
Expand Down Expand Up @@ -375,20 +375,22 @@ CrashInfo::VisitSection(MachOModule& module, const section_64& section)
uint32_t
CrashInfo::GetMemoryRegionFlags(uint64_t start)
{
assert(start == CONVERT_FROM_SIGN_EXTENDED(start));

MemoryRegion search(0, start, start + PAGE_SIZE, 0);
const MemoryRegion* region = SearchMemoryRegions(m_allMemoryRegions, search);
if (region != nullptr) {
return region->Flags();
}
TRACE("GetMemoryRegionFlags: %016llx FAILED\n", start);
TRACE_VERBOSE("GetMemoryRegionFlags: %016llx FAILED\n", start);
return PF_R | PF_W | PF_X;
}

//
// Read raw memory
//
bool
CrashInfo::ReadProcessMemory(void* address, void* buffer, size_t size, size_t* read)
CrashInfo::ReadProcessMemory(uint64_t address, void* buffer, size_t size, size_t* read)
{
assert(buffer != nullptr);
assert(read != nullptr);
Expand All @@ -411,7 +413,7 @@ CrashInfo::ReadProcessMemory(void* address, void* buffer, size_t size, size_t* r
{
g_readProcessMemoryResult = result;
TRACE_VERBOSE("ReadProcessMemory(%p %d): vm_read_overwrite failed bytesLeft %d bytesRead %d from %p: %s (%x)\n",
address, size, bytesLeft, bytesRead, (void*)addressAligned, mach_error_string(result), result);
(void*)address, size, bytesLeft, bytesRead, (void*)addressAligned, mach_error_string(result), result);
break;
}
ssize_t bytesToCopy = PAGE_SIZE - offset;
Expand Down
Loading

0 comments on commit e8d76fb

Please sign in to comment.