From 9afbaa44db13327fa4958ed17815e54cc07f270a Mon Sep 17 00:00:00 2001 From: Mike McLaughlin Date: Wed, 7 Dec 2022 14:25:35 -0800 Subject: [PATCH 1/2] Fix dump creation on MacOS Ventura Use the task_info(TASK_DYLD_INFO) API to get the dylinker info instead of enumerating all the memory regions. This works on Ventura and simplifies the module enumeration quite a bit. --- src/coreclr/debug/createdump/crashinfo.h | 1 - src/coreclr/debug/createdump/crashinfomac.cpp | 62 ++++------------- src/coreclr/debug/dbgutil/machoreader.cpp | 69 +++++++------------ src/coreclr/debug/dbgutil/machoreader.h | 7 +- 4 files changed, 42 insertions(+), 97 deletions(-) diff --git a/src/coreclr/debug/createdump/crashinfo.h b/src/coreclr/debug/createdump/crashinfo.h index e100b7f216102a..e83b54e1d7373e 100644 --- a/src/coreclr/debug/createdump/crashinfo.h +++ b/src/coreclr/debug/createdump/crashinfo.h @@ -129,7 +129,6 @@ class CrashInfo : public ICLRDataEnumMemoryRegionsCallback, private: #ifdef __APPLE__ bool EnumerateMemoryRegions(); - bool TryFindDyLinker(mach_vm_address_t address, mach_vm_size_t size, bool* found); void VisitModule(MachOModule& module); void VisitSegment(MachOModule& module, const segment_command_64& segment); void VisitSection(MachOModule& module, const section_64& section); diff --git a/src/coreclr/debug/createdump/crashinfomac.cpp b/src/coreclr/debug/createdump/crashinfomac.cpp index 67783ec355d39b..6f0854b0743433 100644 --- a/src/coreclr/debug/createdump/crashinfomac.cpp +++ b/src/coreclr/debug/createdump/crashinfomac.cpp @@ -138,16 +138,20 @@ CrashInfo::EnumerateMemoryRegions() } } - // Now find all the modules and add them to the module list - for (const MemoryRegion& region : m_allMemoryRegions) + // Get the dylinker info and enumerate all the modules + struct task_dyld_info dyld_info; + mach_msg_type_number_t count = TASK_DYLD_INFO_COUNT; + kern_return_t result = ::task_info(Task(), TASK_DYLD_INFO, (task_info_t)&dyld_info, &count); + if (result != KERN_SUCCESS) { - bool found; - if (!TryFindDyLinker(region.StartAddress(), region.Size(), &found)) { - return false; - } - if (found) { - break; - } + TRACE("EnumerateMemoryRegions: task_info(TASK_DYLD_INFO) FAILED %x %s\n", result, mach_error_string(result)); + return false; + } + + // Enumerate all the modules in dyld's image cache. VisitModule is called for every module found. + if (!EnumerateModules(dyld_info.all_image_info_addr)) + { + return false; } // Filter out the module regions from the memory regions gathered @@ -197,46 +201,6 @@ CrashInfo::EnumerateMemoryRegions() return true; } -bool -CrashInfo::TryFindDyLinker(mach_vm_address_t address, mach_vm_size_t size, bool* found) -{ - bool result = true; - *found = false; - - if (size > sizeof(mach_header_64)) - { - mach_header_64 header; - size_t read = 0; - if (ReadProcessMemory((void*)address, &header, sizeof(mach_header_64), &read)) - { - if (header.magic == MH_MAGIC_64) - { - TRACE("TryFindDyLinker: found module header at %016llx %08llx ncmds %d sizeofcmds %08x type %02x\n", - address, - size, - header.ncmds, - header.sizeofcmds, - header.filetype); - - if (header.filetype == MH_DYLINKER) - { - TRACE("TryFindDyLinker: found dylinker\n"); - *found = true; - - // Enumerate all the modules in dyld's image cache. VisitModule is called for every module found. - result = EnumerateModules(address, &header); - } - } - } - else - { - TRACE("TryFindDyLinker: ReadProcessMemory header at %p %d FAILED\n", address, read); - } - } - - return result; -} - void CrashInfo::VisitModule(MachOModule& module) { AddModuleInfo(false, module.BaseAddress(), nullptr, module.Name()); diff --git a/src/coreclr/debug/dbgutil/machoreader.cpp b/src/coreclr/debug/dbgutil/machoreader.cpp index 7fef34e1afb16d..399b186db0f951 100644 --- a/src/coreclr/debug/dbgutil/machoreader.cpp +++ b/src/coreclr/debug/dbgutil/machoreader.cpp @@ -75,7 +75,7 @@ TryGetSymbol(ICorDebugDataTarget* dataTarget, uint64_t baseAddress, const char* // MachO module //-------------------------------------------------------------------- -MachOModule::MachOModule(MachOReader& reader, mach_vm_address_t baseAddress, mach_header_64* header, std::string* name) : +MachOModule::MachOModule(MachOReader& reader, mach_vm_address_t baseAddress, std::string* name) : m_reader(reader), m_baseAddress(baseAddress), m_loadBias(0), @@ -84,9 +84,6 @@ MachOModule::MachOModule(MachOReader& reader, mach_vm_address_t baseAddress, mac m_nlists(nullptr), m_strtabAddress(0) { - if (header != nullptr) { - m_header = *header; - } if (name != nullptr) { m_name = *name; } @@ -363,43 +360,25 @@ MachOReader::MachOReader() } bool -MachOReader::EnumerateModules(mach_vm_address_t address, mach_header_64* header) +MachOReader::EnumerateModules(mach_vm_address_t dyldInfoAddress) { - _ASSERTE(header->magic == MH_MAGIC_64); - _ASSERTE(header->filetype == MH_DYLINKER); - - MachOModule dylinker(*this, address, header); - - // Search for symbol for the dyld image info cache - uint64_t dyldInfoAddress = 0; - if (!dylinker.TryLookupSymbol("dyld_all_image_infos", &dyldInfoAddress)) - { - Trace("ERROR: Can not find the _dyld_all_image_infos symbol\n"); - return false; - } - // Read the all image info from the dylinker image dyld_all_image_infos dyldInfo; - if (!ReadMemory((void*)dyldInfoAddress, &dyldInfo, sizeof(dyld_all_image_infos))) { Trace("ERROR: Failed to read dyld_all_image_infos at %p\n", (void*)dyldInfoAddress); return false; } - std::string dylinkerPath; - if (!ReadString(dyldInfo.dyldPath, dylinkerPath)) + Trace("MOD: infoArray %p infoArrayCount %d\n", dyldInfo.infoArray, dyldInfo.infoArrayCount); + + // Create the dyld module info + if (!CreateModule(dyldInfo.dyldImageLoadAddress, dyldInfo.dyldPath)) { - Trace("ERROR: Failed to read name at %p\n", dyldInfo.dyldPath); + Trace("ERROR: Failed to read dyld header at %p\n", dyldInfo.dyldImageLoadAddress); return false; } - dylinker.SetName(dylinkerPath); - Trace("MOD: %016llx %08x %s\n", dylinker.BaseAddress(), dylinker.Header().flags, dylinker.Name().c_str()); - VisitModule(dylinker); - void* imageInfosAddress = (void*)dyldInfo.infoArray; size_t imageInfosSize = dyldInfo.infoArrayCount * sizeof(dyld_image_info); - Trace("MOD: infoArray %p infoArrayCount %d\n", dyldInfo.infoArray, dyldInfo.infoArrayCount); - ArrayHolder imageInfos = new (std::nothrow) dyld_image_info[dyldInfo.infoArrayCount]; if (imageInfos == nullptr) { @@ -413,23 +392,27 @@ MachOReader::EnumerateModules(mach_vm_address_t address, mach_header_64* header) } for (int i = 0; i < dyldInfo.infoArrayCount; i++) { - mach_vm_address_t imageAddress = (mach_vm_address_t)imageInfos[i].imageLoadAddress; - const char* imageFilePathAddress = imageInfos[i].imageFilePath; + // Ignore any errors and continue to next module + CreateModule(imageInfos[i].imageLoadAddress, imageInfos[i].imageFilePath); + } + return true; +} - std::string imagePath; - if (!ReadString(imageFilePathAddress, imagePath)) - { - Trace("ERROR: Failed to read image name at %p\n", imageFilePathAddress); - continue; - } - MachOModule module(*this, imageAddress, nullptr, &imagePath); - if (!module.ReadHeader()) - { - continue; - } - Trace("MOD: %016llx %08x %s\n", imageAddress, module.Header().flags, imagePath.c_str()); - VisitModule(module); +bool +MachOReader::CreateModule(const struct mach_header* imageAddress, const char* imageFilePathAddress) +{ + std::string imagePath; + if (!ReadString(imageFilePathAddress, imagePath)) + { + return false; + } + MachOModule module(*this, (mach_vm_address_t)imageAddress, &imagePath); + if (!module.ReadHeader()) + { + return false; } + Trace("MOD: %016llx %08x %s\n", imageAddress, module.Header().flags, imagePath.c_str()); + VisitModule(module); return true; } diff --git a/src/coreclr/debug/dbgutil/machoreader.h b/src/coreclr/debug/dbgutil/machoreader.h index a5f458b17ff2d5..8650f2694e5cab 100644 --- a/src/coreclr/debug/dbgutil/machoreader.h +++ b/src/coreclr/debug/dbgutil/machoreader.h @@ -27,7 +27,7 @@ class MachOModule uint64_t m_strtabAddress; public: - MachOModule(MachOReader& reader, mach_vm_address_t baseAddress, mach_header_64* header = nullptr, std::string* name = nullptr); + MachOModule(MachOReader& reader, mach_vm_address_t baseAddress, std::string* name = nullptr); ~MachOModule(); inline mach_vm_address_t BaseAddress() const { return m_baseAddress; } @@ -41,8 +41,6 @@ class MachOModule bool EnumerateSegments(); private: - inline void SetName(std::string& name) { m_name = name; } - bool ReadLoadCommands(); bool ReadSymbolTable(); uint64_t GetAddressFromFileOffset(uint32_t offset); @@ -54,9 +52,10 @@ class MachOReader friend MachOModule; public: MachOReader(); - bool EnumerateModules(mach_vm_address_t address, mach_header_64* header); + bool EnumerateModules(mach_vm_address_t dyldInfoAddress); private: + bool CreateModule(const struct mach_header* imageAddress, const char* imageFilePathAddress); bool ReadString(const char* address, std::string& str); virtual void VisitModule(MachOModule& module) { }; virtual void VisitSegment(MachOModule& module, const segment_command_64& segment) { }; From 44ea33afb106c15b717f8ecdce33545cff8a2ca7 Mon Sep 17 00:00:00 2001 From: Mike McLaughlin Date: Fri, 9 Dec 2022 15:54:21 -0800 Subject: [PATCH 2/2] Need to ensure the symbol table and symbol string table for the dylinker module is part of the core dump for the dump readers. They still need to look up the "dyld_all_image_infos" symbol. --- src/coreclr/debug/dbgutil/machoreader.cpp | 18 +++++++++++++++--- src/coreclr/debug/dbgutil/machoreader.h | 2 +- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/src/coreclr/debug/dbgutil/machoreader.cpp b/src/coreclr/debug/dbgutil/machoreader.cpp index 399b186db0f951..07528e1d176cbf 100644 --- a/src/coreclr/debug/dbgutil/machoreader.cpp +++ b/src/coreclr/debug/dbgutil/machoreader.cpp @@ -372,7 +372,7 @@ MachOReader::EnumerateModules(mach_vm_address_t dyldInfoAddress) Trace("MOD: infoArray %p infoArrayCount %d\n", dyldInfo.infoArray, dyldInfo.infoArrayCount); // Create the dyld module info - if (!CreateModule(dyldInfo.dyldImageLoadAddress, dyldInfo.dyldPath)) + if (!TryRegisterModule(dyldInfo.dyldImageLoadAddress, dyldInfo.dyldPath, true)) { Trace("ERROR: Failed to read dyld header at %p\n", dyldInfo.dyldImageLoadAddress); return false; @@ -393,13 +393,13 @@ MachOReader::EnumerateModules(mach_vm_address_t dyldInfoAddress) for (int i = 0; i < dyldInfo.infoArrayCount; i++) { // Ignore any errors and continue to next module - CreateModule(imageInfos[i].imageLoadAddress, imageInfos[i].imageFilePath); + TryRegisterModule(imageInfos[i].imageLoadAddress, imageInfos[i].imageFilePath, false); } return true; } bool -MachOReader::CreateModule(const struct mach_header* imageAddress, const char* imageFilePathAddress) +MachOReader::TryRegisterModule(const struct mach_header* imageAddress, const char* imageFilePathAddress, bool dylinker) { std::string imagePath; if (!ReadString(imageFilePathAddress, imagePath)) @@ -413,6 +413,18 @@ MachOReader::CreateModule(const struct mach_header* imageAddress, const char* im } Trace("MOD: %016llx %08x %s\n", imageAddress, module.Header().flags, imagePath.c_str()); VisitModule(module); + if (dylinker) + { + // Make sure the memory for the symbol and string tables are in the core dump for our + // dump readers which still use this symbol to enumerate modules. + uint64_t dyldInfoAddress = 0; + if (!module.TryLookupSymbol("dyld_all_image_infos", &dyldInfoAddress)) + { + Trace("ERROR: Can not find the _dyld_all_image_infos symbol\n"); + return false; + } + Trace("MOD: dyldInfoAddress %016llx\n", dyldInfoAddress); + } return true; } diff --git a/src/coreclr/debug/dbgutil/machoreader.h b/src/coreclr/debug/dbgutil/machoreader.h index 8650f2694e5cab..19630e624f61c6 100644 --- a/src/coreclr/debug/dbgutil/machoreader.h +++ b/src/coreclr/debug/dbgutil/machoreader.h @@ -55,7 +55,7 @@ class MachOReader bool EnumerateModules(mach_vm_address_t dyldInfoAddress); private: - bool CreateModule(const struct mach_header* imageAddress, const char* imageFilePathAddress); + bool TryRegisterModule(const struct mach_header* imageAddress, const char* imageFilePathAddress, bool dylinker); bool ReadString(const char* address, std::string& str); virtual void VisitModule(MachOModule& module) { }; virtual void VisitSegment(MachOModule& module, const segment_command_64& segment) { };