Skip to content

Commit e8d76fb

Browse files
author
Mike McLaughlin
authored
Various createdump fixes and cleanup (#97253)
* 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
1 parent 82bea3f commit e8d76fb

File tree

8 files changed

+148
-88
lines changed

8 files changed

+148
-88
lines changed

‎src/coreclr/debug/createdump/crashinfo.cpp‎

Lines changed: 41 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,8 @@ CrashInfo::EnumMemoryRegion(
138138
/* [in] */ CLRDATA_ADDRESS address,
139139
/* [in] */ ULONG32 size)
140140
{
141-
m_enumMemoryPagesAdded += InsertMemoryRegion((ULONG_PTR)address, size);
141+
address = CONVERT_FROM_SIGN_EXTENDED(address);
142+
m_enumMemoryPagesAdded += InsertMemoryRegion(address, size);
142143
return S_OK;
143144
}
144145

@@ -449,21 +450,23 @@ CrashInfo::EnumerateManagedModules()
449450
DacpGetModuleData moduleData;
450451
if (SUCCEEDED(hr = moduleData.Request(pClrDataModule.GetPtr())))
451452
{
452-
TRACE("MODULE: %" PRIA PRIx64 " dyn %d inmem %d file %d pe %" PRIA PRIx64 " pdb %" PRIA PRIx64, (uint64_t)moduleData.LoadedPEAddress, moduleData.IsDynamic,
453+
uint64_t loadedPEAddress = CONVERT_FROM_SIGN_EXTENDED(moduleData.LoadedPEAddress);
454+
455+
TRACE("MODULE: %" PRIA PRIx64 " dyn %d inmem %d file %d pe %" PRIA PRIx64 " pdb %" PRIA PRIx64, loadedPEAddress, moduleData.IsDynamic,
453456
moduleData.IsInMemory, moduleData.IsFileLayout, (uint64_t)moduleData.PEAssembly, (uint64_t)moduleData.InMemoryPdbAddress);
454457

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

462465
// Change the module mapping name
463-
AddOrReplaceModuleMapping(moduleData.LoadedPEAddress, moduleData.LoadedPESize, moduleName);
466+
AddOrReplaceModuleMapping(loadedPEAddress, moduleData.LoadedPESize, moduleName);
464467

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

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

530-
uint32_t flags = GetMemoryRegionFlags((ULONG_PTR)baseAddress);
533+
uint32_t flags = GetMemoryRegionFlags(baseAddress);
531534

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

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

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

557560
// Remove and cleanup the old one
558561
m_moduleMappings.erase(found);
@@ -648,15 +651,15 @@ CrashInfo::AddModuleInfo(bool isManaged, uint64_t baseAddress, IXCLRDataModule*
648651
if (isManaged)
649652
{
650653
IMAGE_DOS_HEADER dosHeader;
651-
if (ReadMemory((void*)baseAddress, &dosHeader, sizeof(dosHeader)))
654+
if (ReadMemory(baseAddress, &dosHeader, sizeof(dosHeader)))
652655
{
653656
WORD magic;
654-
if (ReadMemory((void*)(baseAddress + dosHeader.e_lfanew + offsetof(IMAGE_NT_HEADERS, OptionalHeader.Magic)), &magic, sizeof(magic)))
657+
if (ReadMemory(baseAddress + dosHeader.e_lfanew + offsetof(IMAGE_NT_HEADERS, OptionalHeader.Magic), &magic, sizeof(magic)))
655658
{
656659
if (magic == IMAGE_NT_OPTIONAL_HDR32_MAGIC)
657660
{
658661
IMAGE_NT_HEADERS32 header;
659-
if (ReadMemory((void*)(baseAddress + dosHeader.e_lfanew), &header, sizeof(header)))
662+
if (ReadMemory(baseAddress + dosHeader.e_lfanew, &header, sizeof(header)))
660663
{
661664
imageSize = header.OptionalHeader.SizeOfImage;
662665
timeStamp = header.FileHeader.TimeDateStamp;
@@ -665,7 +668,7 @@ CrashInfo::AddModuleInfo(bool isManaged, uint64_t baseAddress, IXCLRDataModule*
665668
else if (magic == IMAGE_NT_OPTIONAL_HDR64_MAGIC)
666669
{
667670
IMAGE_NT_HEADERS64 header;
668-
if (ReadMemory((void*)(baseAddress + dosHeader.e_lfanew), &header, sizeof(header)))
671+
if (ReadMemory(baseAddress + dosHeader.e_lfanew, &header, sizeof(header)))
669672
{
670673
imageSize = header.OptionalHeader.SizeOfImage;
671674
timeStamp = header.FileHeader.TimeDateStamp;
@@ -694,15 +697,15 @@ CrashInfo::AddModuleInfo(bool isManaged, uint64_t baseAddress, IXCLRDataModule*
694697
// ReadMemory from target and add to memory regions list
695698
//
696699
bool
697-
CrashInfo::ReadMemory(void* address, void* buffer, size_t size)
700+
CrashInfo::ReadMemory(uint64_t address, void* buffer, size_t size)
698701
{
699702
size_t read = 0;
700703
if (!ReadProcessMemory(address, buffer, size, &read))
701704
{
702705
return false;
703706
}
704707
assert(read == size);
705-
InsertMemoryRegion(reinterpret_cast<uint64_t>(address), read);
708+
InsertMemoryRegion(address, read);
706709
return true;
707710
}
708711

@@ -712,6 +715,7 @@ CrashInfo::ReadMemory(void* address, void* buffer, size_t size)
712715
int
713716
CrashInfo::InsertMemoryRegion(uint64_t address, size_t size)
714717
{
718+
assert(address == CONVERT_FROM_SIGN_EXTENDED(address));
715719
assert(size < UINT_MAX);
716720

717721
// Round to page boundary
@@ -831,7 +835,7 @@ CrashInfo::PageCanBeRead(uint64_t start)
831835
{
832836
BYTE buffer[1];
833837
size_t read;
834-
return ReadProcessMemory((void*)start, buffer, 1, &read);
838+
return ReadProcessMemory(start, buffer, 1, &read);
835839
}
836840

837841
//
@@ -889,6 +893,23 @@ CrashInfo::CombineMemoryRegions()
889893
}
890894
}
891895

896+
//
897+
// Searches for a module region for a given address.
898+
//
899+
const ModuleRegion*
900+
CrashInfo::SearchModuleRegions(const ModuleRegion& search)
901+
{
902+
std::set<ModuleRegion>::iterator found = m_moduleMappings.find(search);
903+
for (; found != m_moduleMappings.end(); found++)
904+
{
905+
if (search.StartAddress() >= found->StartAddress() && search.StartAddress() < found->EndAddress())
906+
{
907+
return &*found;
908+
}
909+
}
910+
return nullptr;
911+
}
912+
892913
//
893914
// Searches for a memory region given an address.
894915
//

‎src/coreclr/debug/createdump/crashinfo.h‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ class CrashInfo : public ICLRDataEnumMemoryRegionsCallback, public ICLRDataLoggi
7171
std::vector<elf_aux_entry> m_auxvEntries; // full auxv entries
7272
#endif
7373
std::vector<ThreadInfo*> m_threads; // threads found and suspended
74-
std::set<MemoryRegion> m_moduleMappings; // module memory mappings
74+
std::set<ModuleRegion> m_moduleMappings; // module memory mappings
7575
std::set<MemoryRegion> m_otherMappings; // other memory mappings
7676
std::set<MemoryRegion> m_memoryRegions; // memory regions from DAC, etc.
7777
std::set<MemoryRegion> m_moduleAddresses; // memory region to module base address
@@ -97,14 +97,15 @@ class CrashInfo : public ICLRDataEnumMemoryRegionsCallback, public ICLRDataLoggi
9797
bool GatherCrashInfo(DumpType dumpType);
9898
void CombineMemoryRegions();
9999
bool EnumerateMemoryRegionsWithDAC(DumpType dumpType);
100-
bool ReadMemory(void* address, void* buffer, size_t size); // read memory and add to dump
101-
bool ReadProcessMemory(void* address, void* buffer, size_t size, size_t* read); // read raw memory
100+
bool ReadMemory(uint64_t address, void* buffer, size_t size); // read memory and add to dump
101+
bool ReadProcessMemory(uint64_t address, void* buffer, size_t size, size_t* read); // read raw memory
102102
uint64_t GetBaseAddressFromAddress(uint64_t address);
103103
uint64_t GetBaseAddressFromName(const char* moduleName);
104104
ModuleInfo* GetModuleInfoFromBaseAddress(uint64_t baseAddress);
105105
void AddModuleAddressRange(uint64_t startAddress, uint64_t endAddress, uint64_t baseAddress);
106106
void AddModuleInfo(bool isManaged, uint64_t baseAddress, IXCLRDataModule* pClrDataModule, const std::string& moduleName);
107107
int InsertMemoryRegion(uint64_t address, size_t size);
108+
const ModuleRegion* SearchModuleRegions(const ModuleRegion& search);
108109
static const MemoryRegion* SearchMemoryRegions(const std::set<MemoryRegion>& regions, const MemoryRegion& search);
109110

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

124125
inline const std::vector<ThreadInfo*>& Threads() const { return m_threads; }
125-
inline const std::set<MemoryRegion>& ModuleMappings() const { return m_moduleMappings; }
126+
inline const std::set<ModuleRegion>& ModuleMappings() const { return m_moduleMappings; }
126127
inline const std::set<MemoryRegion>& OtherMappings() const { return m_otherMappings; }
127128
inline const std::set<MemoryRegion>& MemoryRegions() const { return m_memoryRegions; }
128129
inline const siginfo_t* SigInfo() const { return &m_siginfo; }
129130
#ifndef __APPLE__
130131
inline const std::vector<elf_aux_entry>& AuxvEntries() const { return m_auxvEntries; }
131132
inline size_t GetAuxvSize() const { return m_auxvEntries.size() * sizeof(elf_aux_entry); }
132133
#endif
134+
bool ReadMemory(void* address, void* buffer, size_t size) { return ReadMemory((uint64_t)address, buffer, size); }
133135

134136
// IUnknown
135137
STDMETHOD(QueryInterface)(___in REFIID InterfaceId, ___out PVOID* Interface);
@@ -159,7 +161,7 @@ class CrashInfo : public ICLRDataEnumMemoryRegionsCallback, public ICLRDataLoggi
159161
bool InitializeDAC(DumpType dumpType);
160162
bool EnumerateManagedModules();
161163
bool UnwindAllThreads();
162-
void AddOrReplaceModuleMapping(CLRDATA_ADDRESS baseAddress, ULONG64 size, const std::string& pszName);
164+
void AddOrReplaceModuleMapping(uint64_t baseAddress, uint64_t size, const std::string& pszName);
163165
int InsertMemoryRegion(const MemoryRegion& region);
164166
uint32_t GetMemoryRegionFlags(uint64_t start);
165167
bool PageCanBeRead(uint64_t start);

‎src/coreclr/debug/createdump/crashinfomac.cpp‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ CrashInfo::InitializeOtherMappings()
179179
// to include all the native and managed module regions.
180180
for (const MemoryRegion& region : m_allMemoryRegions)
181181
{
182-
std::set<MemoryRegion>::iterator found = m_moduleMappings.find(region);
182+
std::set<ModuleRegion>::iterator found = m_moduleMappings.find(ModuleRegion(region));
183183
if (found == m_moduleMappings.end())
184184
{
185185
m_otherMappings.insert(region);
@@ -256,7 +256,7 @@ void CrashInfo::VisitModule(MachOModule& module)
256256
m_runtimeBaseAddress = module.BaseAddress();
257257

258258
RuntimeInfo runtimeInfo { };
259-
if (ReadMemory((void*)(module.BaseAddress() + symbolOffset), &runtimeInfo, sizeof(RuntimeInfo)))
259+
if (ReadMemory(module.BaseAddress() + symbolOffset, &runtimeInfo, sizeof(RuntimeInfo)))
260260
{
261261
if (strcmp(runtimeInfo.Signature, RUNTIME_INFO_SIGNATURE) == 0)
262262
{
@@ -274,7 +274,7 @@ void CrashInfo::VisitModule(MachOModule& module)
274274
m_runtimeBaseAddress = module.BaseAddress();
275275

276276
uint8_t cookie[sizeof(g_debugHeaderCookie)];
277-
if (ReadMemory((void*)(module.BaseAddress() + symbolOffset), cookie, sizeof(cookie)))
277+
if (ReadMemory(module.BaseAddress() + symbolOffset, cookie, sizeof(cookie)))
278278
{
279279
if (memcmp(cookie, g_debugHeaderCookie, sizeof(g_debugHeaderCookie)) == 0)
280280
{
@@ -315,8 +315,8 @@ void CrashInfo::VisitSegment(MachOModule& module, const segment_command_64& segm
315315
assert(end > 0);
316316

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

345345
const auto& found = m_moduleMappings.find(gap);
346346
if (found != m_moduleMappings.end())
@@ -375,20 +375,22 @@ CrashInfo::VisitSection(MachOModule& module, const section_64& section)
375375
uint32_t
376376
CrashInfo::GetMemoryRegionFlags(uint64_t start)
377377
{
378+
assert(start == CONVERT_FROM_SIGN_EXTENDED(start));
379+
378380
MemoryRegion search(0, start, start + PAGE_SIZE, 0);
379381
const MemoryRegion* region = SearchMemoryRegions(m_allMemoryRegions, search);
380382
if (region != nullptr) {
381383
return region->Flags();
382384
}
383-
TRACE("GetMemoryRegionFlags: %016llx FAILED\n", start);
385+
TRACE_VERBOSE("GetMemoryRegionFlags: %016llx FAILED\n", start);
384386
return PF_R | PF_W | PF_X;
385387
}
386388

387389
//
388390
// Read raw memory
389391
//
390392
bool
391-
CrashInfo::ReadProcessMemory(void* address, void* buffer, size_t size, size_t* read)
393+
CrashInfo::ReadProcessMemory(uint64_t address, void* buffer, size_t size, size_t* read)
392394
{
393395
assert(buffer != nullptr);
394396
assert(read != nullptr);
@@ -411,7 +413,7 @@ CrashInfo::ReadProcessMemory(void* address, void* buffer, size_t size, size_t* r
411413
{
412414
g_readProcessMemoryResult = result;
413415
TRACE_VERBOSE("ReadProcessMemory(%p %d): vm_read_overwrite failed bytesLeft %d bytesRead %d from %p: %s (%x)\n",
414-
address, size, bytesLeft, bytesRead, (void*)addressAligned, mach_error_string(result), result);
416+
(void*)address, size, bytesLeft, bytesRead, (void*)addressAligned, mach_error_string(result), result);
415417
break;
416418
}
417419
ssize_t bytesToCopy = PAGE_SIZE - offset;

0 commit comments

Comments
 (0)