Flag for LoadBinaryWithUUIDAndAddress, to create memory image or not

DynamicLoader::LoadBinaryWithUUIDAndAddress can create a Module based
on the binary image in memory, which in some cases contains symbol
names and can be genuinely useful.  If we don't have a filename, it
creates a name in the form `memory-image-0x...` with the header address.

In practice, this is most useful with Darwin userland corefiles
where the binary was stored in the corefile in whole, and we can't
find a binary with the matching UUID.  Using the binary out of
the corefile memory in this case works well.

But in other cases, akin to firmware debugging, we merely end up
with an oddly named binary image and no symbols.

Add a flag to control whether we will create these memory images
and add them to the Target or not; only set it to true when working
with a userland Mach-O image with the "all image infos" LC_NOTE for
a userland corefile.

Differential Revision: https://reviews.llvm.org/D157167

Author: jasonmolenda

lldb/include/lldb/Target/DynamicLoader.h

@@ -264,13 +264,18 @@ class DynamicLoader : public PluginInterface {
   ///     load address for the binary or its segments in the Target if it passes
   ///     true.
   ///
+  /// \param[in] allow_memory_image_last_resort
+  ///     If no better binary image can be found, allow reading the binary
+  ///     out of memory, if possible, and create the Module based on that.
+  ///     May be slow to read a binary out of memory, and for unusual
+  ///     environments, may be no symbols mapped in memory at all.
+  ///
   /// \return
   ///     Returns a shared pointer for the Module that has been added.
-  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, bool set_address_in_target);
+  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,
+      bool set_address_in_target, bool allow_memory_image_last_resort);
 
   /// Get information about the shared cache for a process, if possible.
   ///

lldb/source/Core/DynamicLoader.cpp

@@ -188,7 +188,7 @@ static ModuleSP ReadUnnamedMemoryModule(Process *process, addr_t addr,
 ModuleSP DynamicLoader::LoadBinaryWithUUIDAndAddress(
     Process *process, llvm::StringRef name, UUID uuid, addr_t value,
     bool value_is_offset, bool force_symbol_search, bool notify,
-    bool set_address_in_target) {
+    bool set_address_in_target, bool allow_memory_image_last_resort) {
   ModuleSP memory_module_sp;
   ModuleSP module_sp;
   PlatformSP platform_sp = process->GetTarget().GetPlatform();
@@ -245,7 +245,8 @@ ModuleSP DynamicLoader::LoadBinaryWithUUIDAndAddress(
 
   // If we couldn't find the binary anywhere else, as a last resort,
   // read it out of memory.
-  if (!module_sp.get() && value != LLDB_INVALID_ADDRESS && !value_is_offset) {
+  if (allow_memory_image_last_resort && !module_sp.get() &&
+      value != LLDB_INVALID_ADDRESS && !value_is_offset) {
     if (!memory_module_sp)
       memory_module_sp = ReadUnnamedMemoryModule(process, value, name);
     if (memory_module_sp)

lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp

@@ -6938,9 +6938,15 @@ bool ObjectFileMachO::LoadCoreFileImages(lldb_private::Process &process) {
     if (image.uuid.IsValid() ||
         (!value_is_offset && value != LLDB_INVALID_ADDRESS)) {
       const bool set_load_address = image.segment_load_addresses.size() == 0;
+      const bool notify = false;
+      // Userland Darwin binaries will have segment load addresses via
+      // the `all image infos` LC_NOTE.
+      const bool allow_memory_image_last_resort =
+          image.segment_load_addresses.size();
       module_sp = DynamicLoader::LoadBinaryWithUUIDAndAddress(
           &process, image.filename, image.uuid, value, value_is_offset,
-          image.currently_executing, false /* notify */, set_load_address);
+          image.currently_executing, notify, set_load_address,
+          allow_memory_image_last_resort);
     }
 
     // We have a ModuleSP to load in the Target.  Load it at the

lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

@@ -1001,10 +1001,11 @@ void ProcessGDBRemote::LoadStubBinaries() {
       const bool force_symbol_search = true;
       const bool notify = true;
       const bool set_address_in_target = true;
+      const bool allow_memory_image_last_resort = false;
       DynamicLoader::LoadBinaryWithUUIDAndAddress(
           this, "", standalone_uuid, standalone_value,
           standalone_value_is_offset, force_symbol_search, notify,
-          set_address_in_target);
+          set_address_in_target, allow_memory_image_last_resort);
     }
   }
 
@@ -1033,10 +1034,12 @@ void ProcessGDBRemote::LoadStubBinaries() {
 
       const bool force_symbol_search = true;
       const bool set_address_in_target = true;
+      const bool allow_memory_image_last_resort = false;
       // Second manually load this binary into the Target.
       DynamicLoader::LoadBinaryWithUUIDAndAddress(
           this, llvm::StringRef(), uuid, addr, value_is_slide,
-          force_symbol_search, notify, set_address_in_target);
+          force_symbol_search, notify, set_address_in_target,
+          allow_memory_image_last_resort);
     }
   }
 }

lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp

@@ -259,10 +259,12 @@ void ProcessMachCore::LoadBinariesViaMetadata() {
       const bool force_symbol_search = true;
       const bool notify = true;
       const bool set_address_in_target = true;
+      const bool allow_memory_image_last_resort = false;
       if (DynamicLoader::LoadBinaryWithUUIDAndAddress(
               this, llvm::StringRef(), objfile_binary_uuid,
               objfile_binary_value, objfile_binary_value_is_offset,
-              force_symbol_search, notify, set_address_in_target)) {
+              force_symbol_search, notify, set_address_in_target,
+              allow_memory_image_last_resort)) {
         found_main_binary_definitively = true;
         m_dyld_plugin_name = DynamicLoaderStatic::GetPluginNameStatic();
       }
@@ -315,10 +317,11 @@ void ProcessMachCore::LoadBinariesViaMetadata() {
       const bool force_symbol_search = true;
       const bool notify = true;
       const bool set_address_in_target = true;
+      const bool allow_memory_image_last_resort = false;
       if (DynamicLoader::LoadBinaryWithUUIDAndAddress(
               this, llvm::StringRef(), ident_uuid, ident_binary_addr,
               value_is_offset, force_symbol_search, notify,
-              set_address_in_target)) {
+              set_address_in_target, allow_memory_image_last_resort)) {
         found_main_binary_definitively = true;
         m_dyld_plugin_name = DynamicLoaderStatic::GetPluginNameStatic();
       }

lldb/test/API/macosx/lc-note/multiple-binary-corefile/TestMultipleBinaryCorefile.py

@@ -1,4 +1,4 @@
-"""Test corefiles with "main bin spec"/"load binary" with only addrs work."""
+"""Test corefiles with "main bin spec"/"load binary" with only vmaddrs works."""
 
 
 import os
@@ -35,41 +35,28 @@ def initial_setup(self):
             self.libtwo_exe,
             self.libtwo_slide,
         )
+        if self.TraceOn():
+            print("Creating corefile with command %s")
         call(cmd, shell=True)
 
     def load_corefile_and_test(self):
         target = self.dbg.CreateTarget("")
         err = lldb.SBError()
         if self.TraceOn():
-            self.runCmd("script print('loading corefile %s')" % self.corefile)
+            print("loading corefile %s" % self.corefile)
         process = target.LoadCore(self.corefile)
         self.assertEqual(process.IsValid(), True)
         if self.TraceOn():
-            self.runCmd("script print('image list after loading corefile:')")
+            print("image list after loading corefile:")
             self.runCmd("image list")
 
-        self.assertEqual(target.GetNumModules(), 3)
+        ## We don't have libone.dylib in the global module cache or from
+        ## dsymForUUID, and lldb will not read the binary out of memory.
+        self.assertEqual(target.GetNumModules(), 2)
         fspec = target.GetModuleAtIndex(0).GetFileSpec()
         self.assertEqual(fspec.GetFilename(), self.aout_exe_basename)
 
-        # libone.dylib was never loaded into lldb, see that we added a memory module.
         fspec = target.GetModuleAtIndex(1).GetFileSpec()
-        self.assertIn("memory-image", fspec.GetFilename())
-
-        dwarfdump_uuid_regex = re.compile("UUID: ([-0-9a-fA-F]+) \(([^\(]+)\) .*")
-        dwarfdump_cmd_output = subprocess.check_output(
-            ('/usr/bin/dwarfdump --uuid "%s"' % self.libone_exe), shell=True
-        ).decode("utf-8")
-        libone_uuid = None
-        for line in dwarfdump_cmd_output.splitlines():
-            match = dwarfdump_uuid_regex.search(line)
-            if match:
-                libone_uuid = match.group(1)
-
-        memory_image_uuid = target.GetModuleAtIndex(1).GetUUIDString()
-        self.assertEqual(libone_uuid, memory_image_uuid)
-
-        fspec = target.GetModuleAtIndex(2).GetFileSpec()
         self.assertEqual(fspec.GetFilename(), self.libtwo_exe_basename)
 
         # Executables "always" have this base address
@@ -80,17 +67,9 @@ def load_corefile_and_test(self):
         )
         self.assertEqual(aout_load, 0x100000000 + self.aout_slide)
 
-        # Value from Makefile
-        libone_load = (
-            target.GetModuleAtIndex(1)
-            .GetObjectFileHeaderAddress()
-            .GetLoadAddress(target)
-        )
-        self.assertEqual(libone_load, self.libone_slide)
-
         # Value from Makefile
         libtwo_load = (
-            target.GetModuleAtIndex(2)
+            target.GetModuleAtIndex(1)
             .GetObjectFileHeaderAddress()
             .GetLoadAddress(target)
         )
@@ -140,6 +119,15 @@ def test_corefile_binaries_dsymforuuid(self):
         self.assertNotEqual(
             libtwo_uuid, None, "Could not get uuid of built libtwo.dylib"
         )
+        dwarfdump_cmd_output = subprocess.check_output(
+            ('/usr/bin/dwarfdump --uuid "%s"' % self.aout_exe), shell=True
+        ).decode("utf-8")
+        aout_uuid = None
+        for line in dwarfdump_cmd_output.splitlines():
+            match = dwarfdump_uuid_regex.search(line)
+            if match:
+                aout_uuid = match.group(1)
+        self.assertNotEqual(aout_uuid, None, "Could not get uuid of built a.out")
 
         ###  Create our dsym-for-uuid shell script which returns aout_exe
         shell_cmds = [
@@ -149,27 +137,47 @@ def test_corefile_binaries_dsymforuuid(self):
             "do",
             "  shift",
             "done",
-            "ret=0",
             'echo "<?xml version=\\"1.0\\" encoding=\\"UTF-8\\"?>"',
             'echo "<!DOCTYPE plist PUBLIC \\"-//Apple//DTD PLIST 1.0//EN\\" \\"http://www.apple.com/DTDs/PropertyList-1.0.dtd\\">"',
             'echo "<plist version=\\"1.0\\">"',
+            'echo "  <dict>"',
+            'echo "    <key>$1</key>"',
+            'echo "    <dict>"',
             "",
-            'if [ "$1" != "%s" ]' % (libtwo_uuid),
+            'if [ "$1" != "%s" -a "$1" != "%s" ]' % (libtwo_uuid, aout_uuid),
             "then",
-            '  echo "<key>DBGError</key><string>not found</string>"',
+            '  echo "      <key>DBGError</key>"',
+            '  echo "      <string>not found by $0</string>"',
+            '  echo "    </dict>"',
+            '  echo "  </dict>"',
             '  echo "</plist>"',
-            "  exit 1",
+            "  exit 0",
             "fi",
+            #  UUID matches libtwo.dylib
+            'if [ "$1" = "%s" ]' % (libtwo_uuid),
+            "then",
             "  uuid=%s" % libtwo_uuid,
             "  bin=%s" % self.libtwo_exe,
             "  dsym=%s.dSYM/Contents/Resources/DWARF/%s"
             % (self.libtwo_exe, os.path.basename(self.libtwo_exe)),
-            'echo "<dict><key>$uuid</key><dict>"',
+            "fi",
+            #  UUID matches a.out
+            'if [ "$1" = "%s" ]' % (aout_uuid),
+            "then",
+            "  uuid=%s" % aout_uuid,
+            "  bin=%s" % self.aout_exe,
+            "  dsym=%s.dSYM/Contents/Resources/DWARF/%s"
+            % (self.aout_exe, os.path.basename(self.aout_exe)),
+            "fi",
             "",
-            'echo "<key>DBGDSYMPath</key><string>$dsym</string>"',
-            'echo "<key>DBGSymbolRichExecutable</key><string>$bin</string>"',
-            'echo "</dict></dict></plist>"',
-            "exit $ret",
+            'echo "      <key>DBGDSYMPath</key>"',
+            'echo "      <string>$dsym</string>"',
+            'echo "      <key>DBGSymbolRichExecutable</key>"',
+            'echo "      <string>$bin</string>"',
+            'echo "    </dict>"',
+            'echo "  </dict>"',
+            'echo "</plist>"',
+            "exit 0",
         ]
 
         with open(dsym_for_uuid, "w") as writer:
@@ -183,7 +191,7 @@ def test_corefile_binaries_dsymforuuid(self):
         self.dbg.DeleteTarget(target)
 
         if self.TraceOn():
-            self.runCmd("script print('Global image list, before loading corefile:')")
+            print("Global image list, before loading corefile:")
             self.runCmd("image list -g")
 
         self.load_corefile_and_test()
@@ -206,7 +214,7 @@ def test_corefile_binaries_preloaded(self):
         self.dbg.DeleteTarget(target)
 
         if self.TraceOn():
-            self.runCmd("script print('Global image list, before loading corefile:')")
+            print("Global image list, before loading corefile:")
             self.runCmd("image list -g")
 
         self.load_corefile_and_test()

lldb/test/API/macosx/lc-note/multiple-binary-corefile/create-multibin-corefile.cpp

@@ -16,7 +16,7 @@
 #include <vector>
 
 // Given a list of binaries, and optional slides to be applied,
-// create a corefile whose memory is those binaries laid at at
+// create a corefile whose memory is those binaries laid down at
 // their slid addresses.
 //
 // Add a 'main bin spec' LC_NOTE for the first binary, and