[Lldb-commits] [lldb] 57cbd26 - Flag for LoadBinaryWithUUIDAndAddress, to create memory image or not

Jason Molenda via lldb-commits lldb-commits at lists.llvm.org
Mon Aug 7 15:19:51 PDT 2023


Author: Jason Molenda
Date: 2023-08-07T15:19:45-07:00
New Revision: 57cbd26a68ab61631f5f4272d3c90df2eb0ce4f6

URL: https://github.com/llvm/llvm-project/commit/57cbd26a68ab61631f5f4272d3c90df2eb0ce4f6
DIFF: https://github.com/llvm/llvm-project/commit/57cbd26a68ab61631f5f4272d3c90df2eb0ce4f6.diff

LOG: 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

Added: 
    

Modified: 
    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/test/API/macosx/lc-note/multiple-binary-corefile/TestMultipleBinaryCorefile.py
    lldb/test/API/macosx/lc-note/multiple-binary-corefile/create-multibin-corefile.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/include/lldb/Target/DynamicLoader.h b/lldb/include/lldb/Target/DynamicLoader.h
index 3aa92398d0130d..e508d192d27598 100644
--- a/lldb/include/lldb/Target/DynamicLoader.h
+++ b/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.
   ///

diff  --git a/lldb/source/Core/DynamicLoader.cpp b/lldb/source/Core/DynamicLoader.cpp
index 2e5378f654a51a..0d138d7bb34584 100644
--- a/lldb/source/Core/DynamicLoader.cpp
+++ b/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)

diff  --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
index 56e1726aa5710c..1f200923f37d5f 100644
--- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ b/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

diff  --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index b6f146fd872e80..e466ea21e23d31 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/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);
     }
   }
 }

diff  --git a/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp b/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
index 40918dba489059..e989c1b0bb7c90 100644
--- a/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
+++ b/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();
       }

diff  --git a/lldb/test/API/macosx/lc-note/multiple-binary-corefile/TestMultipleBinaryCorefile.py b/lldb/test/API/macosx/lc-note/multiple-binary-corefile/TestMultipleBinaryCorefile.py
index 62b77203ed1025..0a0bc68646e626 100644
--- a/lldb/test/API/macosx/lc-note/multiple-binary-corefile/TestMultipleBinaryCorefile.py
+++ b/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()

diff  --git a/lldb/test/API/macosx/lc-note/multiple-binary-corefile/create-multibin-corefile.cpp b/lldb/test/API/macosx/lc-note/multiple-binary-corefile/create-multibin-corefile.cpp
index ebe71606806dd6..28baed7a5991ea 100644
--- a/lldb/test/API/macosx/lc-note/multiple-binary-corefile/create-multibin-corefile.cpp
+++ b/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


        


More information about the lldb-commits mailing list