MacOS single-file diagnostic support changes (dotnet#51425)

* MacOS single-file support changes

Use dysymtab_command to just load the extern/export symbols. Don't read
the whole symbol string table; read the symbol name a char at a time.

dotnet#38901

* Don't add null to symbol name

* Code review feedback

* Code review feedback

* Code review feedback fix

src/coreclr/debug/createdump/crashinfomac.cpp

@@ -252,9 +252,15 @@ void CrashInfo::VisitModule(MachOModule& module)
     // Save the runtime module path
     if (m_coreclrPath.empty())
     {
-        size_t last = module.Name().rfind(MAKEDLLNAME_A("coreclr"));
+        size_t last = module.Name().rfind(DIRECTORY_SEPARATOR_STR_A MAKEDLLNAME_A("coreclr"));
         if (last != std::string::npos) {
-            m_coreclrPath = module.Name().substr(0, last);
+            m_coreclrPath = module.Name().substr(0, last + 1);
+
+            uint64_t symbolOffset;
+            if (!module.TryLookupSymbol("g_dacTable", &symbolOffset))
+            {
+                TRACE("TryLookupSymbol(g_dacTable) FAILED\n");
+            }
         }
     }
     // VisitSegment is called for each segment of the module

src/coreclr/debug/createdump/crashinfounix.cpp

@@ -265,16 +265,19 @@ CrashInfo::VisitModule(uint64_t baseAddress, std::string& moduleName)
     }
     if (m_coreclrPath.empty())
     {
-        size_t last = moduleName.rfind(MAKEDLLNAME_A("coreclr"));
+        size_t last = moduleName.rfind(DIRECTORY_SEPARATOR_STR_A MAKEDLLNAME_A("coreclr"));
         if (last != std::string::npos) {
-            m_coreclrPath = moduleName.substr(0, last);
+            m_coreclrPath = moduleName.substr(0, last + 1);
 
             // Now populate the elfreader with the runtime module info and
             // lookup the DAC table symbol to ensure that all the memory
             // necessary is in the core dump.
             if (PopulateForSymbolLookup(baseAddress)) {
                 uint64_t symbolOffset;
-                TryLookupSymbol("g_dacTable", &symbolOffset);
+                if (!TryLookupSymbol("g_dacTable", &symbolOffset))
+                {
+                    TRACE("TryLookupSymbol(g_dacTable) FAILED\n");
+                }
             }
         }
     }

src/coreclr/debug/createdump/specialthreadinfo.h

@@ -10,7 +10,7 @@
 // This defines a workaround to the MacOS dump format not having the OS process
 // and thread ids that SOS needs to map thread "indexes" to thread "ids". The MacOS
 // createdump adds this special memory region at this specific address that is not
-// in the user or kernel address spaces. lldb is find with it.
+// in the user or kernel address spaces. lldb is fine with it.
 
 #define SPECIAL_THREADINFO_SIGNATURE "THREADINFO"
 
@@ -20,7 +20,7 @@ struct SpecialThreadInfoHeader
 {
     char signature[16];
     uint32_t pid;
-    uint32_t numThreads;
+    uint32_t numThreads;        // The number of SpecialThreadInfoEntry's after this header
 };
 
 struct SpecialThreadInfoEntry

src/coreclr/debug/daccess/CMakeLists.txt

@@ -42,7 +42,7 @@ target_precompile_headers(daccess PRIVATE [["stdafx.h"]])
 
 add_dependencies(daccess eventing_headers)
 
-if(CLR_CMAKE_HOST_OSX OR CLR_CMAKE_HOST_FREEBSD OR CLR_CMAKE_HOST_NETBSD OR CLR_CMAKE_HOST_SUNOS)
+if(CLR_CMAKE_HOST_FREEBSD OR CLR_CMAKE_HOST_NETBSD OR CLR_CMAKE_HOST_SUNOS)
   add_definitions(-DUSE_DAC_TABLE_RVA)
 
   set(args $<$<NOT:$<BOOL:${CLR_CMAKE_HOST_OSX}>>:--dynamic> $<TARGET_FILE:coreclr> ${GENERATED_INCLUDE_DIR}/dactablerva.h)
@@ -67,4 +67,4 @@ if(CLR_CMAKE_HOST_OSX OR CLR_CMAKE_HOST_FREEBSD OR CLR_CMAKE_HOST_NETBSD OR CLR_
   )
 
   add_dependencies(daccess dactablerva_header)
-endif(CLR_CMAKE_HOST_OSX OR CLR_CMAKE_HOST_FREEBSD OR CLR_CMAKE_HOST_NETBSD OR CLR_CMAKE_HOST_SUNOS)
+endif(CLR_CMAKE_HOST_FREEBSD OR CLR_CMAKE_HOST_NETBSD OR CLR_CMAKE_HOST_SUNOS)

src/coreclr/debug/daccess/daccess.cpp

@@ -28,7 +28,7 @@
 #ifdef USE_DAC_TABLE_RVA
 #include <dactablerva.h>
 #else
-extern bool TryGetSymbol(ICorDebugDataTarget* dataTarget, uint64_t baseAddress, const char* symbolName, uint64_t* symbolAddress);
+extern "C" bool TryGetSymbol(ICorDebugDataTarget* dataTarget, uint64_t baseAddress, const char* symbolName, uint64_t* symbolAddress);
 #endif
 #endif
 
@@ -7238,7 +7238,7 @@ GetDacTableAddress(ICorDebugDataTarget* dataTarget, ULONG64 baseAddress, PULONG6
     // On MacOS, FreeBSD or NetBSD use the RVA include file
     *dacTableAddress = baseAddress + DAC_TABLE_RVA;
 #else
-    // On Linux try to get the dac table address via the export symbol
+    // On Linux/MacOS try to get the dac table address via the export symbol
     if (!TryGetSymbol(dataTarget, baseAddress, "g_dacTable", dacTableAddress))
     {
         return CORDBG_E_MISSING_DEBUGGER_EXPORTS;

src/coreclr/debug/dbgutil/CMakeLists.txt

@@ -25,10 +25,10 @@ if(CLR_CMAKE_TARGET_LINUX)
     )
 endif(CLR_CMAKE_TARGET_LINUX)
 
-if(CLR_CMAKE_HOST_OSX)
+if(CLR_CMAKE_TARGET_OSX)
     list(APPEND DBGUTIL_SOURCES
         machoreader.cpp
     )
-endif(CLR_CMAKE_HOST_OSX)
+endif(CLR_CMAKE_TARGET_OSX)
 
 add_library_clr(dbgutil STATIC ${DBGUTIL_SOURCES})

src/coreclr/debug/dbgutil/elfreader.cpp

@@ -62,7 +62,7 @@ class ElfReaderExport : public ElfReader
 //
 // Main entry point to get an export symbol
 //
-bool
+extern "C" bool
 TryGetSymbol(ICorDebugDataTarget* dataTarget, uint64_t baseAddress, const char* symbolName, uint64_t* symbolAddress)
 {
     ElfReaderExport elfreader(dataTarget);

src/coreclr/debug/dbgutil/machoreader.cpp

@@ -52,7 +52,7 @@ class MachOReaderExport : public MachOReader
 //
 // Main entry point to get an export symbol
 //
-bool
+extern "C" bool
 TryGetSymbol(ICorDebugDataTarget* dataTarget, uint64_t baseAddress, const char* symbolName, uint64_t* symbolAddress)
 {
     MachOReaderExport reader(dataTarget);
@@ -64,7 +64,7 @@ TryGetSymbol(ICorDebugDataTarget* dataTarget, uint64_t baseAddress, const char*
     uint64_t symbolOffset;
     if (module.TryLookupSymbol(symbolName, &symbolOffset))
     {
-        *symbolAddress = baseAddress + symbolOffset;
+        *symbolAddress = symbolOffset;
         return true;
     }
     *symbolAddress = 0;
@@ -82,7 +82,7 @@ MachOModule::MachOModule(MachOReader& reader, mach_vm_address_t baseAddress, mac
     m_commands(nullptr),
     m_symtabCommand(nullptr),
     m_nlists(nullptr),
-    m_strtab(nullptr)
+    m_strtabAddress(0)
 {
     if (header != nullptr) {
         m_header = *header;
@@ -102,10 +102,6 @@ MachOModule::~MachOModule()
         free(m_nlists);
         m_nlists = nullptr;
     }
-    if (m_strtab != nullptr) {
-        free(m_strtab);
-        m_strtab = nullptr;
-    }
 }
 
 bool
@@ -128,14 +124,19 @@ MachOModule::TryLookupSymbol(const char* symbolName, uint64_t* symbolValue)
     if (ReadSymbolTable())
     {
         _ASSERTE(m_nlists != nullptr);
-        _ASSERTE(m_strtab != nullptr);
+        _ASSERTE(m_strtabAddress != 0);
 
-        for (int i = 0; i < m_symtabCommand->nsyms; i++)
+        for (int i = 0; i < m_dysymtabCommand->nextdefsym; i++)
         {
-            char* name = m_strtab + m_nlists[i].n_un.n_strx;
-            if (strcmp(name, symbolName) == 0)
+            std::string name = GetSymbolName(i);
+            // Skip the leading underscores to match Linux externs
+            if (name[0] == '_')
+            {
+                name.erase(0, 1);
+            }
+            if (strcmp(name.c_str(), symbolName) == 0)
             {
-                *symbolValue = m_nlists[i].n_value;
+                *symbolValue = m_loadBias + m_nlists[i].n_value;
                 return true;
             }
         }
@@ -197,6 +198,10 @@ MachOModule::ReadLoadCommands()
                 m_symtabCommand = (symtab_command*)command;
                 break;
 
+            case LC_DYSYMTAB:
+                m_dysymtabCommand = (dysymtab_command*)command;
+                break;
+
             case LC_SEGMENT_64:
                 segment_command_64* segment = (segment_command_64*)command;
                 m_segments.push_back(segment);
@@ -255,62 +260,72 @@ MachOModule::ReadSymbolTable()
             return false;
         }
         _ASSERTE(m_symtabCommand != nullptr);
-        _ASSERTE(m_strtab == nullptr);
+        _ASSERTE(m_strtabAddress == 0);
 
-        m_reader.TraceVerbose("SYM: symoff %08x nsyms %d stroff %08x strsize %d\n",
+        m_reader.TraceVerbose("SYM: symoff %08x nsyms %d stroff %08x strsize %d iext %d next %d\n",
             m_symtabCommand->symoff,
             m_symtabCommand->nsyms,
             m_symtabCommand->stroff,
-            m_symtabCommand->strsize);
-
-        // Read symbol table. An array of "nlist" structs.
-        void* symtabAddress = GetAddressFromFileOffset(m_symtabCommand->symoff);
-        size_t symtabSize = sizeof(nlist_64) * m_symtabCommand->nsyms;
+            m_symtabCommand->strsize,
+            m_dysymtabCommand->iextdefsym,
+            m_dysymtabCommand->nextdefsym);
 
+        // Read the external symbol part of symbol table. An array of "nlist" structs.
+        void* extSymbolTableAddress = (void*)(GetAddressFromFileOffset(m_symtabCommand->symoff) + (m_dysymtabCommand->iextdefsym * sizeof(nlist_64)));
+        size_t symtabSize = sizeof(nlist_64) * m_dysymtabCommand->nextdefsym;
         m_nlists = (nlist_64*)malloc(symtabSize);
         if (m_nlists == nullptr)
         {
-            m_reader.Trace("ERROR: Failed to allocate %zu byte symbol table\n", symtabSize);
+            m_reader.Trace("ERROR: Failed to allocate %zu byte external symbol table\n", symtabSize);
             return false;
         }
-        if (!m_reader.ReadMemory(symtabAddress, m_nlists, symtabSize))
+        if (!m_reader.ReadMemory(extSymbolTableAddress, m_nlists, symtabSize))
         {
-            m_reader.Trace("ERROR: Failed to read symtab at %p of %zu\n", symtabAddress, symtabSize);
+            m_reader.Trace("ERROR: Failed to read external symtab at %p of %zu\n", extSymbolTableAddress, symtabSize);
             return false;
         }
 
-        // Read the symbol string table.
-        void* strtabAddress = GetAddressFromFileOffset(m_symtabCommand->stroff);
-        size_t strtabSize = m_symtabCommand->strsize;
-
-        m_strtab = (char*)malloc(strtabSize);
-        if (m_strtab == nullptr)
-        {
-            m_reader.Trace("ERROR: Failed to allocate %zu byte symbol string table\n", strtabSize);
-            return false;
-        }
-        if (!m_reader.ReadMemory(strtabAddress, m_strtab, strtabSize))
-        {
-            m_reader.Trace("ERROR: Failed to read string table at %p of %zu\n", strtabAddress, strtabSize);
-            return false;
-        }
+        // Save the symbol string table address.
+        m_strtabAddress = GetAddressFromFileOffset(m_symtabCommand->stroff);
     }
     return true;
 }
 
-void*
+uint64_t
 MachOModule::GetAddressFromFileOffset(uint32_t offset)
 {
     _ASSERTE(!m_segments.empty());
-
     for (const segment_command_64* segment : m_segments)
     {
         if (offset >= segment->fileoff && offset < (segment->fileoff + segment->filesize))
         {
-            return (void*)(m_baseAddress + offset + segment->vmaddr - segment->fileoff);
+            return m_loadBias + offset + segment->vmaddr - segment->fileoff;
+        }
+    }
+    return m_loadBias + offset;
+}
+
+std::string
+MachOModule::GetSymbolName(int index)
+{
+    uint64_t symbolNameAddress = m_strtabAddress + m_nlists[index].n_un.n_strx;
+    std::string result;
+    while (true)
+    {
+        char c = 0;
+        if (!m_reader.ReadMemory((void*)symbolNameAddress, &c, sizeof(char)))
+        {
+            m_reader.Trace("ERROR: Failed to read string table at %p\n", (void*)symbolNameAddress);
+            break;
+        }
+        if (c == '\0')
+        {
+            break;
         }
+        result.append(1, c);
+        symbolNameAddress++;
     }
-    return (void*)(m_baseAddress + offset);
+    return result;
 }
 
 //--------------------------------------------------------------------
@@ -330,20 +345,19 @@ MachOReader::EnumerateModules(mach_vm_address_t address, mach_header_64* header)
     MachOModule dylinker(*this, address, header);
 
     // Search for symbol for the dyld image info cache
-    uint64_t dyldInfoOffset = 0;
-    if (!dylinker.TryLookupSymbol("_dyld_all_image_infos", &dyldInfoOffset))
+    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
-    void* dyldInfoAddress = (void*)(address + dyldInfoOffset);
     dyld_all_image_infos dyldInfo;
 
-    if (!ReadMemory(dyldInfoAddress, &dyldInfo, sizeof(dyld_all_image_infos)))
+    if (!ReadMemory((void*)dyldInfoAddress, &dyldInfo, sizeof(dyld_all_image_infos)))
     {
-        Trace("ERROR: Failed to read dyld_all_image_infos at %p\n", dyldInfoAddress);
+        Trace("ERROR: Failed to read dyld_all_image_infos at %p\n", (void*)dyldInfoAddress);
         return false;
     }
     std::string dylinkerPath;

src/coreclr/debug/dbgutil/machoreader.h

@@ -22,8 +22,9 @@ class MachOModule
     load_command* m_commands;
     std::vector<segment_command_64*> m_segments;
     symtab_command* m_symtabCommand;
+    dysymtab_command* m_dysymtabCommand;
     nlist_64* m_nlists;
-    char* m_strtab;
+    uint64_t m_strtabAddress;
 
 public:
     MachOModule(MachOReader& reader, mach_vm_address_t baseAddress, mach_header_64* header = nullptr, std::string* name = nullptr);
@@ -43,7 +44,8 @@ class MachOModule
 
     bool ReadLoadCommands();
     bool ReadSymbolTable();
-    void* GetAddressFromFileOffset(uint32_t offset);
+    uint64_t GetAddressFromFileOffset(uint32_t offset);
+    std::string GetSymbolName(int index);
 };
 
 class MachOReader