jasonmolenda created this revision.
jasonmolenda added reviewers: bulbazord, JDevlieghere.
jasonmolenda added a project: LLDB.
Herald added a project: All.
jasonmolenda requested review of this revision.
Herald added subscribers: lldb-commits, wangpc.
DynamicLoader::LoadBinaryWithUUIDAndAddress() tries a few different ways of
finding a binary, and if it can't find an actual binary (and symbol file) on
the local computer, it will try to read a binary out of memory, with the
thought that there may be a symbol table in memory so we'll get some symbol
names. It will create a placeholder name of `memory-image-0x...`. It seemed
like a good idea when I wrote this method, but in practice for binaries outside
of userland binaries in a corefile, this hasn't been helpful.
This patch adds a flag to LoadBinaryWithUUIDAndAddress to control if we should
create a memory image module, or not. The only case where we still enable this
behavior is in ObjectFileMachO::LoadCoreFileImages() when a binary has segment
load addresses. This is the case with the "all image infos" LC_NOTE that lldb
creates for userland corefiles with binaries that typically have some symbols
in their in-memory symbol tables.
The other type of userland corefiles (gcore-created) will not use this LC_NOTE
but an entirely different codepath -- through the Darwin userland DynamicLoader
plugin -- is taken in those cases, so we'll still read them out of memory.
This patch makes firmware/standalone binary loading not create these
`memory-image-0x...` placeholder binaries with no symbols in them, the scenario
where it is more annoying than useful.
The test case TestMultipleBinaryCorefile.py tests this standalone-binary
corefile scenario, so it needed adjusting to the new setup where the binary
image is not added to the target. I also cleaned up the fake dsym-for-uuid.sh
it creates to return a properly formatted DBGError message for
https://reviews.llvm.org/D157160
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D157167
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/test/API/macosx/lc-note/multiple-binary-corefile/TestMultipleBinaryCorefile.py
lldb/test/API/macosx/lc-note/multiple-binary-corefile/create-multibin-corefile.cpp
Index: lldb/test/API/macosx/lc-note/multiple-binary-corefile/create-multibin-corefile.cpp
===================================================================
--- lldb/test/API/macosx/lc-note/multiple-binary-corefile/create-multibin-corefile.cpp
+++ 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
Index: lldb/test/API/macosx/lc-note/multiple-binary-corefile/TestMultipleBinaryCorefile.py
===================================================================
--- lldb/test/API/macosx/lc-note/multiple-binary-corefile/TestMultipleBinaryCorefile.py
+++ 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,6 +35,8 @@
self.libtwo_exe,
self.libtwo_slide,
)
+ if self.TraceOn():
+ self.runCmd("script print('Creating corefile with command %s')" % cmd)
call(cmd, shell=True)
def load_corefile_and_test(self):
@@ -48,28 +50,13 @@
self.runCmd("script 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 @@
)
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 @@
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 @@
"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:
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
@@ -259,10 +259,12 @@
const bool force_symbol_search = true;
const bool notify = true;
const bool set_address_in_target = true;
+ const bool allow_use_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_use_memory_image_last_resort)) {
found_main_binary_definitively = true;
m_dyld_plugin_name = DynamicLoaderStatic::GetPluginNameStatic();
}
@@ -315,10 +317,11 @@
const bool force_symbol_search = true;
const bool notify = true;
const bool set_address_in_target = true;
+ const bool allow_use_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_use_memory_image_last_resort)) {
found_main_binary_definitively = true;
m_dyld_plugin_name = DynamicLoaderStatic::GetPluginNameStatic();
}
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
@@ -1001,10 +1001,11 @@
const bool force_symbol_search = true;
const bool notify = true;
const bool set_address_in_target = true;
+ const bool allow_use_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_use_memory_image_last_resort);
}
}
@@ -1033,10 +1034,12 @@
const bool force_symbol_search = true;
const bool set_address_in_target = true;
+ const bool allow_use_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_use_memory_image_last_resort);
}
}
}
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
@@ -6938,9 +6938,15 @@
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_use_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_use_memory_image_last_resort);
}
// We have a ModuleSP to load in the Target. Load it at the
Index: lldb/source/Core/DynamicLoader.cpp
===================================================================
--- lldb/source/Core/DynamicLoader.cpp
+++ lldb/source/Core/DynamicLoader.cpp
@@ -188,7 +188,7 @@
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_use_memory_image_last_resort) {
ModuleSP memory_module_sp;
ModuleSP module_sp;
PlatformSP platform_sp = process->GetTarget().GetPlatform();
@@ -245,7 +245,8 @@
// 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_use_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)
Index: lldb/include/lldb/Target/DynamicLoader.h
===================================================================
--- lldb/include/lldb/Target/DynamicLoader.h
+++ lldb/include/lldb/Target/DynamicLoader.h
@@ -264,13 +264,18 @@
/// load address for the binary or its segments in the Target if it passes
/// true.
///
+ /// \param[in] allow_use_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_use_memory_image_last_resort);
/// 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
