jasonmolenda created this revision.
jasonmolenda added reviewers: JDevlieghere, bulbazord.
jasonmolenda added a project: LLDB.
Herald added a project: All.
jasonmolenda requested review of this revision.
Herald added a subscriber: lldb-commits.
This patch addresses two issues I found while looking into a problem yesterday.
First, SBTarget::AddModule calls Target::GetOrCreate which will find the
module in the global module list, the current target, and will call the
DebugSymbols framework on macOS and look in specified directories, and do a
Spotlight search of the local computer. But it doesn't call
`Symbols::DownloadObjectAndSymbolFile()` with a "force expensive lookup" which
we should assume the program is trying to do if they gave us a UUID to look up.
This matches the behavior of `target modules add -u ....` today in the lldb
command line interface.
Also, if the SB API user has created a Target which has no architecture, we
normally will set the Target's arch as soon as we find a hint about the arch --
e.g. when we connect to a gdb remote serial protocol stub, or when we get a
corefile. But in this case, they are creating a Target with no arch, adding a
module to it. We should use the same scheme where a Target inherits the first
likely ArchSpec we see when it has none of its own.
Add a test for these two things, using a fake dsym-for-uuid.sh shell (which
Jonas has pointed out in the past, I really need to create a utility function
that creates these given an array of uuid/binary/dsyms, which I agree with but
haven't done yet. :/)
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D157659
Files:
lldb/source/API/SBTarget.cpp
lldb/test/API/python_api/target-arch-from-module/Makefile
lldb/test/API/python_api/target-arch-from-module/TestTargetArchFromModule.py
lldb/test/API/python_api/target-arch-from-module/main.c
Index: lldb/test/API/python_api/target-arch-from-module/main.c
===================================================================
--- /dev/null
+++ lldb/test/API/python_api/target-arch-from-module/main.c
@@ -0,0 +1,5 @@
+#include <stdio.h>
+int main() {
+ puts("HI i am a program");
+ return 0;
+}
Index: lldb/test/API/python_api/target-arch-from-module/TestTargetArchFromModule.py
===================================================================
--- /dev/null
+++ lldb/test/API/python_api/target-arch-from-module/TestTargetArchFromModule.py
@@ -0,0 +1,107 @@
+"""
+An SBTarget with no arch, call AddModule, SBTarget's arch should be set.
+"""
+
+import os
+import subprocess
+import re
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TargetArchFromModule(TestBase):
+ @skipIf(
+ debug_info=no_match(["dsym"]),
+ bugnumber="This test is looking explicitly for a dSYM",
+ )
+ @skipUnlessDarwin
+ @skipIfRemote
+ def test_target_arch_init(self):
+ self.build()
+ aout_exe = self.getBuildArtifact("a.out")
+ aout_dsym = self.getBuildArtifact("a.out.dSYM")
+ hidden_dir = self.getBuildArtifact("hide.noindex")
+ hidden_aout_exe = self.getBuildArtifact("hide.noindex/a.out")
+ hidden_aout_dsym = self.getBuildArtifact("hide.noindex/a.out.dSYM")
+ dsym_for_uuid = self.getBuildArtifact("dsym-for-uuid.sh")
+
+ # We can hook in our dsym-for-uuid shell script to lldb with
+ # this env var instead of requiring a defaults write.
+ os.environ["LLDB_APPLE_DSYMFORUUID_EXECUTABLE"] = dsym_for_uuid
+ self.addTearDownHook(
+ lambda: os.environ.pop("LLDB_APPLE_DSYMFORUUID_EXECUTABLE", None)
+ )
+
+ dwarfdump_uuid_regex = re.compile("UUID: ([-0-9a-fA-F]+) \(([^\(]+)\) .*")
+ dwarfdump_cmd_output = subprocess.check_output(
+ ('/usr/bin/dwarfdump --uuid "%s"' % 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 self.hidden_aout_exe.
+ shell_cmds = [
+ "#! /bin/sh",
+ "# the last argument is the uuid",
+ "while [ $# -gt 1 ]",
+ "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\\">"',
+ "",
+ 'if [ "$1" = "%s" ]' % aout_uuid,
+ "then",
+ " uuid=%s" % aout_uuid,
+ " bin=%s" % hidden_aout_exe,
+ " dsym=%s.dSYM/Contents/Resources/DWARF/%s"
+ % (hidden_aout_exe, os.path.basename(hidden_aout_exe)),
+ "fi",
+ 'echo " <dict>"',
+ 'echo " <key>$1</key>"',
+ 'echo " <dict>"',
+ 'if [ -z "$uuid" -o -z "$bin" -o ! -f "$bin" ]',
+ "then",
+ ' echo " <key>DBGError</key>"',
+ ' echo " <string>not found by $0</string>"',
+ ' echo " </dict>"',
+ ' echo " </dict>"',
+ ' echo "</plist>"',
+ " exit 0",
+ "fi",
+ "",
+ 'echo "<key>DBGDSYMPath</key><string>$dsym</string>"',
+ 'echo "<key>DBGSymbolRichExecutable</key><string>$bin</string>"',
+ 'echo "</dict></dict></plist>"',
+ "exit $ret",
+ ]
+
+ with open(dsym_for_uuid, "w") as writer:
+ for l in shell_cmds:
+ writer.write(l + "\n")
+
+ os.chmod(dsym_for_uuid, 0o755)
+
+ # Move the main binary and its dSYM into the hide.noindex
+ # directory. Now the only way lldb can find them is with
+ # the LLDB_APPLE_DSYMFORUUID_EXECUTABLE shell script -
+ # so we're testing that this dSYM discovery method works.
+ lldbutil.mkdir_p(hidden_dir)
+ os.rename(aout_exe, hidden_aout_exe)
+ os.rename(aout_dsym, hidden_aout_dsym)
+
+ target = self.dbg.CreateTarget("")
+ self.assertTrue(target.IsValid())
+ self.expect("target list", matching=False, substrs=["arch="])
+
+ m = target.AddModule(None, None, aout_uuid)
+ self.assertTrue(m.IsValid())
+ self.expect("target list", substrs=["arch="])
Index: lldb/test/API/python_api/target-arch-from-module/Makefile
===================================================================
--- /dev/null
+++ lldb/test/API/python_api/target-arch-from-module/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+
+include Makefile.rules
Index: lldb/source/API/SBTarget.cpp
===================================================================
--- lldb/source/API/SBTarget.cpp
+++ lldb/source/API/SBTarget.cpp
@@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//
#include "lldb/API/SBTarget.h"
+#include "lldb/Symbol/LocateSymbolFile.h"
#include "lldb/Utility/Instrumentation.h"
#include "lldb/Utility/LLDBLog.h"
#include "lldb/lldb-public.h"
@@ -1477,28 +1478,29 @@
const char *uuid_cstr, const char *symfile) {
LLDB_INSTRUMENT_VA(this, path, triple, uuid_cstr, symfile);
- lldb::SBModule sb_module;
TargetSP target_sp(GetSP());
- if (target_sp) {
- ModuleSpec module_spec;
- if (path)
- module_spec.GetFileSpec().SetFile(path, FileSpec::Style::native);
+ if (!target_sp)
+ return {};
- if (uuid_cstr)
- module_spec.GetUUID().SetFromStringRef(uuid_cstr);
+ ModuleSpec module_spec;
+ if (path)
+ module_spec.GetFileSpec().SetFile(path, FileSpec::Style::native);
- if (triple)
- module_spec.GetArchitecture() = Platform::GetAugmentedArchSpec(
- target_sp->GetPlatform().get(), triple);
- else
- module_spec.GetArchitecture() = target_sp->GetArchitecture();
+ if (uuid_cstr)
+ module_spec.GetUUID().SetFromStringRef(uuid_cstr);
- if (symfile)
- module_spec.GetSymbolFileSpec().SetFile(symfile, FileSpec::Style::native);
+ if (triple)
+ module_spec.GetArchitecture() =
+ Platform::GetAugmentedArchSpec(target_sp->GetPlatform().get(), triple);
+ else
+ module_spec.GetArchitecture() = target_sp->GetArchitecture();
- sb_module.SetSP(target_sp->GetOrCreateModule(module_spec, true /* notify */));
- }
- return sb_module;
+ if (symfile)
+ module_spec.GetSymbolFileSpec().SetFile(symfile, FileSpec::Style::native);
+
+ SBModuleSpec sb_modulespec(module_spec);
+
+ return AddModule(sb_modulespec);
}
lldb::SBModule SBTarget::AddModule(const SBModuleSpec &module_spec) {
@@ -1506,9 +1508,26 @@
lldb::SBModule sb_module;
TargetSP target_sp(GetSP());
- if (target_sp)
+ if (target_sp) {
sb_module.SetSP(target_sp->GetOrCreateModule(*module_spec.m_opaque_up,
true /* notify */));
+ if (!sb_module.IsValid() && module_spec.m_opaque_up->GetUUID().IsValid()) {
+ Status error;
+ if (Symbols::DownloadObjectAndSymbolFile(*module_spec.m_opaque_up, error,
+ true)) {
+ if (FileSystem::Instance().Exists(
+ module_spec.m_opaque_up->GetFileSpec())) {
+ sb_module.SetSP(target_sp->GetOrCreateModule(*module_spec.m_opaque_up,
+ true /* notify */));
+ }
+ }
+ }
+ }
+ // If the target hasn't initialized any architecture yet, use the
+ // binary's architecture.
+ if (sb_module.IsValid() && !target_sp->GetArchitecture().IsValid() &&
+ sb_module.GetSP() && sb_module.GetSP()->GetArchitecture().IsValid())
+ target_sp->SetArchitecture(sb_module.GetSP()->GetArchitecture());
return sb_module;
}
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
