jasonmolenda created this revision.
jasonmolenda added reviewers: JDevlieghere, jingham.
jasonmolenda added a project: LLDB.
Herald added a reviewer: ributzka.
Herald added a project: All.
jasonmolenda requested review of this revision.
Herald added a subscriber: lldb-commits.

This builds on the work in https://reviews.llvm.org/D133534 where support for a 
platform binary in live process debugging was added, and adds it to the Mach-O 
corefile loading as well.  It has five changes:

1. Add a name field to `DynamicLoader::LoadBinaryWithUUIDAndAddress` when we 
have a name, but may end up reading the binary out of memory, so we can use the 
name instead of a generic placeholder.

2. In `ObjectFileMachO::LoadCoreFileImages`, check if the image address is a 
platform binary, load it and set the Platform/DynamicLoader as appropriate.

3. Clean up the different cases in `LoadCoreFileImages` that handle corefile 
binary loading so it is clearer to read.

4. Separate `ProcessMachCore::DoLoadCore` into half a dozen smaller helper 
methods.  This method had grown quite large and it was difficult to trace the 
code flow through the different parts, and the complexity was unnecessary.

5. In the new `ProcessMachCore::LoadBinariesViaMetadata` method which handles 
LC_NOTE type hints about binaries, when we have a `main bin spec` LC_NOTE that 
is explicitly a kernel, don't load the binary here -- just save the address and 
note that this will be a `DynamicLoaderDarwinKernel` dynamic loader, and leave 
the loading up to DynamicLoaderDarwinKernel to do correctly.

The minor cleanups in `ObjectFileMachO::LoadCoreFileImages` are easy to read in 
the phabracator if you only read the new version of the code.  The (seriously 
needed) separation of `ObjectFileMachO::LoadCoreFileImages` into separate 
methods has made the diff very difficult to read.  In my own reviewing of the 
patch, I've settled on simply reading through the new version of the file.  
Most of it is simple code movement, but I did make some cleanups to how 
sections of code were working -- for instance in the exhaustive search for a 
dyld or kernel binary, when multiple images that appear to be a kernel are 
found, I handle this better now by allowing DynamicLoaderDarwinKernel to pick 
the kernel image.  It's common that we get fragments in a corefile that look 
similar to a kernel and can confuse the exhaustive search algorithm.

Similar to the problem of testing this with a live process, to test this I'd 
need to construct a corefile with a Mach-O fileset with a kernel image in it.  
It would not be easy to construct a test case without a corefile writer that 
synthesized all of this together, a not small bit of code, and it would also 
need to modify a test binary's mach header to look enough like a kernel that 
lldb would be tricked into using it.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133680

Files:
  lldb/include/lldb/Target/DynamicLoader.h
  lldb/source/Core/DynamicLoader.cpp
  lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
  lldb/source/Plugins/Process/mach-core/ProcessMachCore.h

Index: lldb/source/Plugins/Process/mach-core/ProcessMachCore.h
===================================================================
--- lldb/source/Plugins/Process/mach-core/ProcessMachCore.h
+++ lldb/source/Plugins/Process/mach-core/ProcessMachCore.h
@@ -85,7 +85,14 @@
                         lldb_private::MemoryRegionInfo &region_info) override;
 
 private:
-  bool GetDynamicLoaderAddress(lldb::addr_t addr);
+  void CreateMemoryRegions();
+  void LoadBinariesViaMetadata();
+  void LoadBinariesViaExhaustiveSearch();
+  void LoadBinariesAndSetDYLD();
+  void CleanupMemoryRegionPermissions();
+
+  bool CheckAddressForDyldOrKernel(lldb::addr_t addr, lldb::addr_t &dyld,
+                                   lldb::addr_t &kernel);
 
   enum CorefilePreference { eUserProcessCorefile, eKernelCorefile };
 
Index: lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
===================================================================
--- lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
+++ lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
@@ -40,6 +40,7 @@
 #include "Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.h"
 #include "Plugins/DynamicLoader/Static/DynamicLoaderStatic.h"
 #include "Plugins/ObjectFile/Mach-O/ObjectFileMachO.h"
+#include "Plugins/Platform/MacOSX/PlatformDarwinKernel.h"
 
 #include <memory>
 #include <mutex>
@@ -124,10 +125,13 @@
   Finalize();
 }
 
-bool ProcessMachCore::GetDynamicLoaderAddress(lldb::addr_t addr) {
+bool ProcessMachCore::CheckAddressForDyldOrKernel(lldb::addr_t addr,
+                                                  addr_t &dyld,
+                                                  addr_t &kernel) {
   Log *log(GetLog(LLDBLog::DynamicLoader | LLDBLog::Process));
   llvm::MachO::mach_header header;
   Status error;
+  dyld = kernel = LLDB_INVALID_ADDRESS;
   if (DoReadMemory(addr, &header, sizeof(header), error) != sizeof(header))
     return false;
   if (header.magic == llvm::MachO::MH_CIGAM ||
@@ -141,9 +145,6 @@
     header.flags = llvm::ByteSwap_32(header.flags);
   }
 
-  // TODO: swap header if needed...
-  // printf("0x%16.16" PRIx64 ": magic = 0x%8.8x, file_type= %u\n", vaddr,
-  // header.magic, header.filetype);
   if (header.magic == llvm::MachO::MH_MAGIC ||
       header.magic == llvm::MachO::MH_MAGIC_64) {
     // Check MH_EXECUTABLE to see if we can find the mach image that contains
@@ -152,26 +153,23 @@
     // has the list of kexts to load
     switch (header.filetype) {
     case llvm::MachO::MH_DYLINKER:
-      // printf("0x%16.16" PRIx64 ": file_type = MH_DYLINKER\n", vaddr);
-      // Address of dyld "struct mach_header" in the core file
       LLDB_LOGF(log,
-                "ProcessMachCore::GetDynamicLoaderAddress found a user "
+                "ProcessMachCore::CheckAddressForDyldOrKernel found a user "
                 "process dyld binary image at 0x%" PRIx64,
                 addr);
-      m_dyld_addr = addr;
+      dyld = addr;
       return true;
 
     case llvm::MachO::MH_EXECUTE:
-      // printf("0x%16.16" PRIx64 ": file_type = MH_EXECUTE\n", vaddr);
       // Check MH_EXECUTABLE file types to see if the dynamic link object flag
       // is NOT set. If it isn't, then we have a mach_kernel.
       if ((header.flags & llvm::MachO::MH_DYLDLINK) == 0) {
         LLDB_LOGF(log,
-                  "ProcessMachCore::GetDynamicLoaderAddress found a mach "
+                  "ProcessMachCore::CheckAddressForDyldOrKernel found a mach "
                   "kernel binary image at 0x%" PRIx64,
                   addr);
         // Address of the mach kernel "struct mach_header" in the core file.
-        m_mach_kernel_addr = addr;
+        kernel = addr;
         return true;
       }
       break;
@@ -180,46 +178,10 @@
   return false;
 }
 
-// Process Control
-Status ProcessMachCore::DoLoadCore() {
-  Log *log(GetLog(LLDBLog::DynamicLoader | LLDBLog::Process));
-  Status error;
-  if (!m_core_module_sp) {
-    error.SetErrorString("invalid core module");
-    return error;
-  }
-
+void ProcessMachCore::CreateMemoryRegions() {
   ObjectFile *core_objfile = m_core_module_sp->GetObjectFile();
-  if (core_objfile == nullptr) {
-    error.SetErrorString("invalid core object file");
-    return error;
-  }
-
-  if (core_objfile->GetNumThreadContexts() == 0) {
-    error.SetErrorString("core file doesn't contain any LC_THREAD load "
-                         "commands, or the LC_THREAD architecture is not "
-                         "supported in this lldb");
-    return error;
-  }
-
   SectionList *section_list = core_objfile->GetSectionList();
-  if (section_list == nullptr) {
-    error.SetErrorString("core file has no sections");
-    return error;
-  }
-
   const uint32_t num_sections = section_list->GetNumSections(0);
-  if (num_sections == 0) {
-    error.SetErrorString("core file has no sections");
-    return error;
-  }
-
-  SetCanJIT(false);
-
-  llvm::MachO::mach_header header;
-  DataExtractor data(&header, sizeof(header),
-                     m_core_module_sp->GetArchitecture().GetByteOrder(),
-                     m_core_module_sp->GetArchitecture().GetAddressByteSize());
 
   bool ranges_are_sorted = true;
   addr_t vm_addr = 0;
@@ -259,39 +221,51 @@
     m_core_aranges.Sort();
     m_core_range_infos.Sort();
   }
+}
 
+void ProcessMachCore::LoadBinariesViaMetadata() {
+  Log *log(GetLog(LLDBLog::DynamicLoader | LLDBLog::Process));
+  ObjectFile *core_objfile = m_core_module_sp->GetObjectFile();
   bool found_main_binary_definitively = false;
 
   addr_t objfile_binary_value;
   bool objfile_binary_value_is_offset;
   UUID objfile_binary_uuid;
   ObjectFile::BinaryType type;
+
   if (core_objfile->GetCorefileMainBinaryInfo(objfile_binary_value,
                                               objfile_binary_value_is_offset,
                                               objfile_binary_uuid, type)) {
     if (log) {
-      log->Printf(
-          "ProcessMachCore::DoLoadCore: using binary hint from 'main bin spec' "
-          "LC_NOTE with UUID %s value 0x%" PRIx64
-          " value is offset %d and type %d",
-          objfile_binary_uuid.GetAsString().c_str(), objfile_binary_value,
-          objfile_binary_value_is_offset, type);
-    }
-    const bool force_symbol_search = true;
-    const bool notify = true;
-    if (DynamicLoader::LoadBinaryWithUUIDAndAddress(
-            this, objfile_binary_uuid, objfile_binary_value,
-            objfile_binary_value_is_offset, force_symbol_search, notify)) {
-      found_main_binary_definitively = true;
-      m_dyld_plugin_name = DynamicLoaderStatic::GetPluginNameStatic();
-    }
-    if (type == ObjectFile::eBinaryTypeUser) {
-      m_dyld_addr = objfile_binary_value;
-      m_dyld_plugin_name = DynamicLoaderMacOSXDYLD::GetPluginNameStatic();
+      log->Printf("ProcessMachCore::LoadBinariesViaMetadata: using binary hint "
+                  "from 'main bin spec' "
+                  "LC_NOTE with UUID %s value 0x%" PRIx64
+                  " value is offset %d and type %d",
+                  objfile_binary_uuid.GetAsString().c_str(),
+                  objfile_binary_value, objfile_binary_value_is_offset, type);
     }
+
+    // If this is the xnu kernel, don't load it now.  Note the correct
+    // DynamicLoader plugin to use, and the address of the kernel, and
+    // let the DynamicLoader handle the finding & loading of the binary.
     if (type == ObjectFile::eBinaryTypeKernel) {
       m_mach_kernel_addr = objfile_binary_value;
       m_dyld_plugin_name = DynamicLoaderDarwinKernel::GetPluginNameStatic();
+      found_main_binary_definitively = true;
+    } else {
+      const bool force_symbol_search = true;
+      const bool notify = true;
+      if (DynamicLoader::LoadBinaryWithUUIDAndAddress(
+              this, llvm::StringRef(), objfile_binary_uuid,
+              objfile_binary_value, objfile_binary_value_is_offset,
+              force_symbol_search, notify)) {
+        found_main_binary_definitively = true;
+        m_dyld_plugin_name = DynamicLoaderStatic::GetPluginNameStatic();
+      }
+      if (type == ObjectFile::eBinaryTypeUser) {
+        m_dyld_addr = objfile_binary_value;
+        m_dyld_plugin_name = DynamicLoaderMacOSXDYLD::GetPluginNameStatic();
+      }
     }
   }
 
@@ -329,8 +303,9 @@
     if (corefile_identifier.find("Darwin Kernel") != std::string::npos &&
         ident_uuid.IsValid() && ident_binary_addr != LLDB_INVALID_ADDRESS) {
       if (log)
-        log->Printf("ProcessMachCore::DoLoadCore: Found kernel binary via "
-                    "LC_IDENT/kern ver str LC_NOTE");
+        log->Printf(
+            "ProcessMachCore::LoadBinariesViaMetadata: Found kernel binary via "
+            "LC_IDENT/kern ver str LC_NOTE");
       m_mach_kernel_addr = ident_binary_addr;
       found_main_binary_definitively = true;
     } else if (ident_uuid.IsValid()) {
@@ -340,81 +315,99 @@
       const bool force_symbol_search = true;
       const bool notify = true;
       if (DynamicLoader::LoadBinaryWithUUIDAndAddress(
-              this, ident_uuid, ident_binary_addr, value_is_offset,
-              force_symbol_search, notify)) {
+              this, llvm::StringRef(), ident_uuid, ident_binary_addr,
+              value_is_offset, force_symbol_search, notify)) {
         found_main_binary_definitively = true;
         m_dyld_plugin_name = DynamicLoaderStatic::GetPluginNameStatic();
       }
     }
   }
 
-  bool did_load_extra_binaries = core_objfile->LoadCoreFileImages(*this);
-  // If we have a "all image infos" LC_NOTE, try to load all of the
-  // binaries listed, and set their Section load addresses in the Target.
-  if (found_main_binary_definitively == false && did_load_extra_binaries) {
-    m_dyld_plugin_name = DynamicLoaderDarwinKernel::GetPluginNameStatic();
-    found_main_binary_definitively = true;
-  }
+  // Finally, load any binaries noted by "load binary" LC_NOTEs in the
+  // corefile
+  core_objfile->LoadCoreFileImages(*this);
 
-  if (!found_main_binary_definitively &&
-      (m_dyld_addr == LLDB_INVALID_ADDRESS ||
-       m_mach_kernel_addr == LLDB_INVALID_ADDRESS)) {
-    // We need to locate the main executable in the memory ranges we have in
-    // the core file.  We need to search for both a user-process dyld binary
-    // and a kernel binary in memory; we must look at all the pages in the
-    // binary so we don't miss one or the other.  Step through all memory
-    // segments searching for a kernel binary and for a user process dyld --
-    // we'll decide which to prefer later if both are present.
-
-    const size_t num_core_aranges = m_core_aranges.GetSize();
-    for (size_t i = 0; i < num_core_aranges; ++i) {
-      const VMRangeToFileOffset::Entry *entry =
-          m_core_aranges.GetEntryAtIndex(i);
-      lldb::addr_t section_vm_addr_start = entry->GetRangeBase();
-      lldb::addr_t section_vm_addr_end = entry->GetRangeEnd();
-      for (lldb::addr_t section_vm_addr = section_vm_addr_start;
-           section_vm_addr < section_vm_addr_end; section_vm_addr += 0x1000) {
-        GetDynamicLoaderAddress(section_vm_addr);
-      }
-    }
-  }
+  // LoadCoreFileImges may have set the dynamic loader; if we now have
+  // a dynamic loader, save its name so we don't un-set it later.
+  if (GetDynamicLoader())
+    m_dyld_plugin_name = GetDynamicLoader()->GetPluginName();
+}
 
-  if (!found_main_binary_definitively &&
-      m_mach_kernel_addr != LLDB_INVALID_ADDRESS) {
-    // In the case of multiple kernel images found in the core file via
-    // exhaustive search, we may not pick the correct one.  See if the
-    // DynamicLoaderDarwinKernel's search heuristics might identify the correct
-    // one. Most of the time, I expect the address from SearchForDarwinKernel()
-    // will be the same as the address we found via exhaustive search.
+void ProcessMachCore::LoadBinariesViaExhaustiveSearch() {
+  Log *log(GetLog(LLDBLog::DynamicLoader | LLDBLog::Process));
 
-    if (!GetTarget().GetArchitecture().IsValid() && m_core_module_sp.get()) {
-      GetTarget().SetArchitecture(m_core_module_sp->GetArchitecture());
+  // Search the pages of the corefile for dyld or mach kernel
+  // binaries.  There may be multiple things that look like a kernel
+  // in the corefile; disambiguating to the correct one can be difficult.
+
+  std::vector<addr_t> dylds_found;
+  std::vector<addr_t> kernels_found;
+
+  const size_t num_core_aranges = m_core_aranges.GetSize();
+  for (size_t i = 0; i < num_core_aranges; ++i) {
+    const VMRangeToFileOffset::Entry *entry = m_core_aranges.GetEntryAtIndex(i);
+    lldb::addr_t section_vm_addr_start = entry->GetRangeBase();
+    lldb::addr_t section_vm_addr_end = entry->GetRangeEnd();
+    for (lldb::addr_t section_vm_addr = section_vm_addr_start;
+         section_vm_addr < section_vm_addr_end; section_vm_addr += 0x1000) {
+      addr_t dyld, kernel;
+      if (CheckAddressForDyldOrKernel(section_vm_addr, dyld, kernel)) {
+        if (dyld != LLDB_INVALID_ADDRESS)
+          dylds_found.push_back(dyld);
+        if (kernel != LLDB_INVALID_ADDRESS)
+          kernels_found.push_back(dyld);
+      }
     }
+  }
 
-    // SearchForDarwinKernel will end up calling back into this this class in
-    // the GetImageInfoAddress method which will give it the
-    // m_mach_kernel_addr/m_dyld_addr it already has.  Save that aside and set
-    // m_mach_kernel_addr/m_dyld_addr to an invalid address temporarily so
-    // DynamicLoaderDarwinKernel does a real search for the kernel using its
-    // own heuristics.
-
-    addr_t saved_mach_kernel_addr = m_mach_kernel_addr;
-    addr_t saved_user_dyld_addr = m_dyld_addr;
-    m_mach_kernel_addr = LLDB_INVALID_ADDRESS;
-    m_dyld_addr = LLDB_INVALID_ADDRESS;
-
-    addr_t better_kernel_address =
-        DynamicLoaderDarwinKernel::SearchForDarwinKernel(this);
+  // If we found more than one dyld mach-o header in the corefile,
+  // pick the first one.
+  if (dylds_found.size() > 0)
+    m_dyld_addr = dylds_found[0];
+  if (kernels_found.size() > 0)
+    m_mach_kernel_addr = kernels_found[0];
+
+  // Zero or one kernels found, we're done.
+  if (kernels_found.size() < 2)
+    return;
+
+  // In the case of multiple kernel images found in the core file via
+  // exhaustive search, we may not pick the correct one.  See if the
+  // DynamicLoaderDarwinKernel's search heuristics might identify the correct
+  // one.
+
+  // SearchForDarwinKernel will call this class' GetImageInfoAddress method
+  // which will give it the addresses we already have.
+  // Save those aside and set
+  // m_mach_kernel_addr/m_dyld_addr to an invalid address temporarily so
+  // DynamicLoaderDarwinKernel does a real search for the kernel using its
+  // own heuristics.
+
+  addr_t saved_mach_kernel_addr = m_mach_kernel_addr;
+  addr_t saved_user_dyld_addr = m_dyld_addr;
+  m_mach_kernel_addr = LLDB_INVALID_ADDRESS;
+  m_dyld_addr = LLDB_INVALID_ADDRESS;
+
+  addr_t better_kernel_address =
+      DynamicLoaderDarwinKernel::SearchForDarwinKernel(this);
+
+  m_mach_kernel_addr = saved_mach_kernel_addr;
+  m_dyld_addr = saved_user_dyld_addr;
+
+  if (better_kernel_address != LLDB_INVALID_ADDRESS) {
+    LLDB_LOGF(log, "ProcessMachCore::LoadBinariesViaExhaustiveSearch: Using "
+                   "the kernel address "
+                   "from DynamicLoaderDarwinKernel");
+    m_mach_kernel_addr = better_kernel_address;
+  }
+}
 
-    m_mach_kernel_addr = saved_mach_kernel_addr;
-    m_dyld_addr = saved_user_dyld_addr;
+void ProcessMachCore::LoadBinariesAndSetDYLD() {
+  Log *log(GetLog(LLDBLog::DynamicLoader | LLDBLog::Process));
 
-    if (better_kernel_address != LLDB_INVALID_ADDRESS) {
-      LLDB_LOGF(log, "ProcessMachCore::DoLoadCore: Using the kernel address "
-                     "from DynamicLoaderDarwinKernel");
-      m_mach_kernel_addr = better_kernel_address;
-    }
-  }
+  LoadBinariesViaMetadata();
+  if (m_dyld_plugin_name.empty())
+    LoadBinariesViaExhaustiveSearch();
 
   if (m_dyld_plugin_name.empty()) {
     // If we found both a user-process dyld and a kernel binary, we need to
@@ -422,34 +415,40 @@
     if (GetCorefilePreference() == eKernelCorefile) {
       if (m_mach_kernel_addr != LLDB_INVALID_ADDRESS) {
         LLDB_LOGF(log,
-                  "ProcessMachCore::DoLoadCore: Using kernel corefile image "
+                  "ProcessMachCore::LoadBinariesAndSetDYLD: Using kernel "
+                  "corefile image "
                   "at 0x%" PRIx64,
                   m_mach_kernel_addr);
         m_dyld_plugin_name = DynamicLoaderDarwinKernel::GetPluginNameStatic();
       } else if (m_dyld_addr != LLDB_INVALID_ADDRESS) {
-        LLDB_LOGF(log,
-                  "ProcessMachCore::DoLoadCore: Using user process dyld "
-                  "image at 0x%" PRIx64,
-                  m_dyld_addr);
+        LLDB_LOGF(
+            log,
+            "ProcessMachCore::LoadBinariesAndSetDYLD: Using user process dyld "
+            "image at 0x%" PRIx64,
+            m_dyld_addr);
         m_dyld_plugin_name = DynamicLoaderMacOSXDYLD::GetPluginNameStatic();
       }
     } else {
       if (m_dyld_addr != LLDB_INVALID_ADDRESS) {
-        LLDB_LOGF(log,
-                  "ProcessMachCore::DoLoadCore: Using user process dyld "
-                  "image at 0x%" PRIx64,
-                  m_dyld_addr);
+        LLDB_LOGF(
+            log,
+            "ProcessMachCore::LoadBinariesAndSetDYLD: Using user process dyld "
+            "image at 0x%" PRIx64,
+            m_dyld_addr);
         m_dyld_plugin_name = DynamicLoaderMacOSXDYLD::GetPluginNameStatic();
       } else if (m_mach_kernel_addr != LLDB_INVALID_ADDRESS) {
         LLDB_LOGF(log,
-                  "ProcessMachCore::DoLoadCore: Using kernel corefile image "
+                  "ProcessMachCore::LoadBinariesAndSetDYLD: Using kernel "
+                  "corefile image "
                   "at 0x%" PRIx64,
                   m_mach_kernel_addr);
         m_dyld_plugin_name = DynamicLoaderDarwinKernel::GetPluginNameStatic();
       }
     }
   }
+}
 
+void ProcessMachCore::CleanupMemoryRegionPermissions() {
   if (m_dyld_plugin_name != DynamicLoaderMacOSXDYLD::GetPluginNameStatic()) {
     // For non-user process core files, the permissions on the core file
     // segments are usually meaningless, they may be just "read", because we're
@@ -466,16 +465,42 @@
       ent->data = lldb::ePermissionsReadable | lldb::ePermissionsExecutable;
     }
   }
+}
 
-  // Even if the architecture is set in the target, we need to override it to
-  // match the core file which is always single arch.
-  ArchSpec arch(m_core_module_sp->GetArchitecture());
-  if (arch.GetCore() == ArchSpec::eCore_x86_32_i486) {
-    arch = Platform::GetAugmentedArchSpec(GetTarget().GetPlatform().get(), "i386");
+// Process Control
+Status ProcessMachCore::DoLoadCore() {
+  Status error;
+  if (!m_core_module_sp) {
+    error.SetErrorString("invalid core module");
+    return error;
+  }
+
+  ObjectFile *core_objfile = m_core_module_sp->GetObjectFile();
+  if (core_objfile == nullptr) {
+    error.SetErrorString("invalid core object file");
+    return error;
+  }
+
+  if (core_objfile->GetNumThreadContexts() == 0) {
+    error.SetErrorString("core file doesn't contain any LC_THREAD load "
+                         "commands, or the LC_THREAD architecture is not "
+                         "supported in this lldb");
+    return error;
   }
+
+  SetCanJIT(false);
+
+  // The corefile's architecture is our best starting point.
+  ArchSpec arch(m_core_module_sp->GetArchitecture());
   if (arch.IsValid())
     GetTarget().SetArchitecture(arch);
 
+  CreateMemoryRegions();
+
+  LoadBinariesAndSetDYLD();
+
+  CleanupMemoryRegionPermissions();
+
   addr_t address_mask = core_objfile->GetAddressMask();
   if (address_mask != 0) {
     SetCodeAddressMask(address_mask);
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -575,7 +575,7 @@
           const bool force_symbol_search = true;
           const bool notify = true;
           DynamicLoader::LoadBinaryWithUUIDAndAddress(
-              this, standalone_uuid, standalone_value,
+              this, llvm::StringRef(), standalone_uuid, standalone_value,
               standalone_value_is_offset, force_symbol_search, notify);
         }
       }
@@ -607,7 +607,8 @@
           const bool force_symbol_search = true;
           // Second manually load this binary into the Target.
           DynamicLoader::LoadBinaryWithUUIDAndAddress(
-              this, uuid, addr, value_is_slide, force_symbol_search, notify);
+              this, llvm::StringRef(), uuid, addr, value_is_slide,
+              force_symbol_search, notify);
         }
       }
 
Index: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
===================================================================
--- lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -6975,44 +6975,81 @@
 bool ObjectFileMachO::LoadCoreFileImages(lldb_private::Process &process) {
   MachOCorefileAllImageInfos image_infos = GetCorefileAllImageInfos();
   Log *log = GetLog(LLDBLog::DynamicLoader);
+  Status error;
 
+  bool found_platform_binary = false;
   ModuleList added_modules;
-  for (const MachOCorefileImageEntry &image : image_infos.all_image_infos) {
-    ModuleSP module_sp;
+  for (MachOCorefileImageEntry &image : image_infos.all_image_infos) {
+    ModuleSP module_sp, local_filesystem_module_sp;
+
+    // If this is a platform binary, it has been loaded (or registered with
+    // the DynamicLoader to be loaded), we don't need to do any further
+    // processing.  We're not going to call ModulesDidLoad on this in this
+    // method, so notify==true.
+    if (process.GetTarget()
+            .GetDebugger()
+            .GetPlatformList()
+            .LoadPlatformBinaryAndSetup(&process, image.load_address,
+                                        true /* notify */)) {
+      LLDB_LOGF(log,
+                "ObjectFileMachO::LoadCoreFileImages binary at 0x%" PRIx64
+                " is a platform binary, has been handled by a Platform plugin.",
+                image.load_address);
+      continue;
+    }
 
-    if (!image.filename.empty()) {
-      Status error;
+    // If this binary is currently executing, we want to force a
+    // possibly expensive search for the binary and its dSYM.
+    if (image.currently_executing && image.uuid.IsValid()) {
       ModuleSpec module_spec;
       module_spec.GetUUID() = image.uuid;
-      module_spec.GetFileSpec() = FileSpec(image.filename.c_str());
-      if (image.currently_executing) {
-        Symbols::DownloadObjectAndSymbolFile(module_spec, error, true);
-        if (FileSystem::Instance().Exists(module_spec.GetFileSpec())) {
-          process.GetTarget().GetOrCreateModule(module_spec, false);
-        }
+      Symbols::DownloadObjectAndSymbolFile(module_spec, error, true);
+      if (FileSystem::Instance().Exists(module_spec.GetFileSpec())) {
+        module_sp = process.GetTarget().GetOrCreateModule(module_spec, false);
+        process.GetTarget().GetImages().AppendIfNeeded(module_sp,
+                                                       false /* notify */);
       }
+    }
+
+    // We have an address, that's the best way to discover the binary.
+    if (!module_sp && image.load_address != LLDB_INVALID_ADDRESS) {
+      module_sp = DynamicLoader::LoadBinaryWithUUIDAndAddress(
+          &process, image.filename, image.uuid, image.load_address,
+          false /* value_is_offset */, image.currently_executing,
+          false /* notify */);
+    }
+
+    // If we have a slide, we need to find the original binary
+    // by UUID, then we can apply the slide value.
+    if (!module_sp && image.uuid.IsValid() &&
+        image.slide != LLDB_INVALID_ADDRESS) {
+      module_sp = DynamicLoader::LoadBinaryWithUUIDAndAddress(
+          &process, image.filename, image.uuid, image.slide,
+          true /* value_is_offset */, image.currently_executing,
+          false /* notify */);
+    }
+
+    // Try to find the binary by UUID or filename on the local
+    // filesystem or in lldb's global module cache.
+    if (!module_sp) {
+      Status error;
+      ModuleSpec module_spec;
+      if (image.uuid.IsValid())
+        module_spec.GetUUID() = image.uuid;
+      if (!image.filename.empty())
+        module_spec.GetFileSpec() = FileSpec(image.filename.c_str());
       module_sp =
           process.GetTarget().GetOrCreateModule(module_spec, false, &error);
       process.GetTarget().GetImages().AppendIfNeeded(module_sp,
                                                      false /* notify */);
-    } else {
-      if (image.load_address != LLDB_INVALID_ADDRESS) {
-        module_sp = DynamicLoader::LoadBinaryWithUUIDAndAddress(
-            &process, image.uuid, image.load_address,
-            false /* value_is_offset */, image.currently_executing,
-            false /* notify */);
-      } else if (image.slide != LLDB_INVALID_ADDRESS) {
-        module_sp = DynamicLoader::LoadBinaryWithUUIDAndAddress(
-            &process, image.uuid, image.slide, true /* value_is_offset */,
-            image.currently_executing, false /* notify */);
-      }
     }
 
-    if (module_sp.get()) {
-      // Will call ModulesDidLoad with all modules once they've all
-      // been added to the Target with load addresses.  Don't notify
-      // here, before the load address is set.
+    // We have a ModuleSP to load in the Target.  Load it at the
+    // correct address/slide and notify/load scripting resources.
+    if (module_sp) {
       added_modules.Append(module_sp, false /* notify */);
+
+      // We have a list of segment load address
       if (image.segment_load_addresses.size() > 0) {
         if (log) {
           std::string uuidstr = image.uuid.GetAsString();
@@ -7073,5 +7110,11 @@
     process.Flush();
     return true;
   }
+  // Return true if the only binary we found was the platform binary,
+  // and it was loaded outside the scope of this method.
+  if (found_platform_binary)
+    return true;
+
+  // No binaries.
   return false;
 }
Index: lldb/source/Core/DynamicLoader.cpp
===================================================================
--- lldb/source/Core/DynamicLoader.cpp
+++ lldb/source/Core/DynamicLoader.cpp
@@ -175,17 +175,19 @@
   return nullptr;
 }
 
-static ModuleSP ReadUnnamedMemoryModule(Process *process, addr_t addr) {
+static ModuleSP ReadUnnamedMemoryModule(Process *process, addr_t addr,
+                                        llvm::StringRef name) {
   char namebuf[80];
-  snprintf(namebuf, sizeof(namebuf), "memory-image-0x%" PRIx64, addr);
-  return process->ReadModuleFromMemory(FileSpec(namebuf), addr);
+  if (name.empty()) {
+    snprintf(namebuf, sizeof(namebuf), "memory-image-0x%" PRIx64, addr);
+    name = namebuf;
+  }
+  return process->ReadModuleFromMemory(FileSpec(name), addr);
 }
 
-ModuleSP DynamicLoader::LoadBinaryWithUUIDAndAddress(Process *process,
-                                                     UUID uuid, addr_t value,
-                                                     bool value_is_offset,
-                                                     bool force_symbol_search,
-                                                     bool notify) {
+ModuleSP DynamicLoader::LoadBinaryWithUUIDAndAddress(
+    Process *process, llvm::StringRef name, UUID uuid, addr_t value,
+    bool value_is_offset, bool force_symbol_search, bool notify) {
   ModuleSP memory_module_sp;
   ModuleSP module_sp;
   PlatformSP platform_sp = process->GetTarget().GetPlatform();
@@ -195,7 +197,7 @@
   module_spec.GetUUID() = uuid;
 
   if (!uuid.IsValid() && !value_is_offset) {
-    memory_module_sp = ReadUnnamedMemoryModule(process, value);
+    memory_module_sp = ReadUnnamedMemoryModule(process, value, name);
 
     if (memory_module_sp)
       uuid = memory_module_sp->GetUUID();
@@ -223,7 +225,7 @@
   // read it out of memory.
   if (!module_sp.get() && value != LLDB_INVALID_ADDRESS && !value_is_offset) {
     if (!memory_module_sp)
-      memory_module_sp = ReadUnnamedMemoryModule(process, value);
+      memory_module_sp = ReadUnnamedMemoryModule(process, value, name);
     if (memory_module_sp)
       module_sp = memory_module_sp;
   }
Index: lldb/include/lldb/Target/DynamicLoader.h
===================================================================
--- lldb/include/lldb/Target/DynamicLoader.h
+++ lldb/include/lldb/Target/DynamicLoader.h
@@ -226,6 +226,10 @@
   /// \param[in] process
   ///     The process to add this binary to.
   ///
+  /// \param[in] name
+  ///     Name of the binary, if available.  If an empty StringRef is
+  ///     passed here, a name will be constructed for the module.
+  ///
   /// \param[in] uuid
   ///     UUID of the binary to be loaded.  UUID may be empty, and if a
   ///     load address is supplied, will read the binary from memory, get
@@ -251,10 +255,9 @@
   ///
   /// \return
   ///     Returns a shared pointer for the Module that has been added.
-  static lldb::ModuleSP
-  LoadBinaryWithUUIDAndAddress(Process *process, UUID uuid, lldb::addr_t value,
-                               bool value_is_offset, bool force_symbol_search,
-                               bool notify);
+  static lldb::ModuleSP LoadBinaryWithUUIDAndAddress(
+      Process *process, llvm::StringRef name, UUID uuid, lldb::addr_t value,
+      bool value_is_offset, bool force_symbol_search, bool notify);
 
   /// Get information about the shared cache for a process, if possible.
   ///
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to