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

Reply via email to