JDevlieghere created this revision.
JDevlieghere added reviewers: jasonmolenda, jingham, LLDB.
JDevlieghere added a dependency: D51859: [NFC] Turn "load dependent files" 
boolean into an enum .
JDevlieghere edited the summary of this revision.

When creating a target, lldb loads all dependent files (i.e. libs in 
`LC_LOAD_DYLIB` for Mach-O). This can be confusing, especially when two 
versions of the same library end up in the shared cache. It's possible to 
change this behavior,  by specifying  `target create -d <target>` these 
dependents are not loaded.

This patch changes the default behavior to only load dependent files only when 
the target is an executable. When creating a target for a library, it is now no 
longer necessary to pass `-d`. The user can still override this behavior by 
specifying the `-d` option. However, rather than a boolean you can now specify 
one of three values: `default`, `yes`, or `no`.

rdar://problem/43721382


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D51934

Files:
  packages/Python/lldbsuite/test/functionalities/target_create_deps/Makefile
  
packages/Python/lldbsuite/test/functionalities/target_create_deps/TestTargetCreateDeps.py
  packages/Python/lldbsuite/test/functionalities/target_create_deps/a.cpp
  packages/Python/lldbsuite/test/functionalities/target_create_deps/a.mk
  packages/Python/lldbsuite/test/functionalities/target_create_deps/main.cpp
  packages/Python/lldbsuite/test/tools/lldb-vscode/attach/TestVSCode_attach.py
  source/Commands/CommandObjectTarget.cpp

Index: source/Commands/CommandObjectTarget.cpp
===================================================================
--- source/Commands/CommandObjectTarget.cpp
+++ source/Commands/CommandObjectTarget.cpp
@@ -138,6 +138,65 @@
   return num_targets;
 }
 
+static OptionEnumValueElement g_dependents_enumaration[4] = {
+    {eLoadDependentsDefault, "default",
+     "Only load dependents when the target is an executables."},
+    {eLoadDependentsYes, "yes",
+     "Load dependents, even if the target is not an executable."},
+    {eLoadDependentsNo, "no",
+     "Don't load dependents, even if the target is an executable."},
+    {0, nullptr, nullptr}};
+
+static OptionDefinition g_dependents_options[1] = {
+    {LLDB_OPT_SET_1, false, "dependents", 'd', OptionParser::eRequiredArgument,
+     nullptr, g_dependents_enumaration, 0, eArgTypeValue,
+     "Whether or not to load dependents when creating a target."}};
+
+class OptionGroupDependents : public OptionGroup {
+public:
+  OptionGroupDependents() {}
+
+  ~OptionGroupDependents() override {}
+
+  llvm::ArrayRef<OptionDefinition> GetDefinitions() override {
+    return llvm::makeArrayRef(g_dependents_options);
+  }
+
+  Status SetOptionValue(uint32_t option_idx, llvm::StringRef option_value,
+                        ExecutionContext *execution_context) override {
+    Status error;
+    const int short_option = g_dependents_options[option_idx].short_option;
+    switch (short_option) {
+    case 'd': {
+      LoadDependentFiles tmp_load_dependents;
+      tmp_load_dependents = (LoadDependentFiles)OptionArgParser::ToOptionEnum(
+          option_value, g_dependents_options[option_idx].enum_values, 0, error);
+      if (error.Success()) {
+        m_load_dependent_files = tmp_load_dependents;
+      }
+      break;
+    }
+    default:
+      error.SetErrorStringWithFormat("unrecognized short option '%c'",
+                                     short_option);
+      break;
+    }
+
+    return error;
+  }
+
+  Status SetOptionValue(uint32_t, const char *, ExecutionContext *) = delete;
+
+  void OptionParsingStarting(ExecutionContext *execution_context) override {
+    m_load_dependent_files = eLoadDependentsDefault;
+  }
+
+  LoadDependentFiles m_load_dependent_files;
+
+private:
+  DISALLOW_COPY_AND_ASSIGN(OptionGroupDependents);
+};
+
 #pragma mark CommandObjectTargetCreate
 
 //-------------------------------------------------------------------------
@@ -158,16 +217,14 @@
                         eArgTypePath,
                         "Path to the remote file to use for this target."),
         m_symbol_file(LLDB_OPT_SET_1, false, "symfile", 's', 0,
-                      eArgTypeFilename, "Fullpath to a stand alone debug "
-                                        "symbols file for when debug symbols "
-                                        "are not in the executable."),
+                      eArgTypeFilename,
+                      "Fullpath to a stand alone debug "
+                      "symbols file for when debug symbols "
+                      "are not in the executable."),
         m_remote_file(
             LLDB_OPT_SET_1, false, "remote-file", 'r', 0, eArgTypeFilename,
             "Fullpath to the file on the remote host if debugging remotely."),
-        m_add_dependents(LLDB_OPT_SET_1, false, "no-dependents", 'd',
-                         "Don't load dependent files when creating the target, "
-                         "just add the specified executable.",
-                         true, true) {
+        m_add_dependents() {
     CommandArgumentEntry arg;
     CommandArgumentData file_arg;
 
@@ -259,12 +316,9 @@
 
       TargetSP target_sp;
       llvm::StringRef arch_cstr = m_arch_option.GetArchitectureName();
-      const bool get_dependent_files =
-          m_add_dependents.GetOptionValue().GetCurrentValue();
       Status error(debugger.GetTargetList().CreateTarget(
           debugger, file_path, arch_cstr,
-          get_dependent_files ? eLoadDependentsYes : eLoadDependentsNo, nullptr,
-          target_sp));
+          m_add_dependents.m_load_dependent_files, nullptr, target_sp));
 
       if (target_sp) {
         // Only get the platform after we create the target because we might
@@ -412,7 +466,7 @@
   OptionGroupFile m_platform_path;
   OptionGroupFile m_symbol_file;
   OptionGroupFile m_remote_file;
-  OptionGroupBoolean m_add_dependents;
+  OptionGroupDependents m_add_dependents;
 };
 
 #pragma mark CommandObjectTargetList
Index: packages/Python/lldbsuite/test/tools/lldb-vscode/attach/TestVSCode_attach.py
===================================================================
--- packages/Python/lldbsuite/test/tools/lldb-vscode/attach/TestVSCode_attach.py
+++ packages/Python/lldbsuite/test/tools/lldb-vscode/attach/TestVSCode_attach.py
@@ -144,7 +144,7 @@
         # if we are able to use attach commands to create any kind of a target
         # and use it for debugging
         attachCommands = [
-            'target create -d "%s"' % (program),
+            'target create -d no "%s"' % (program),
             'process launch -- arg1'
         ]
         initCommands = ['target list', 'platform list']
Index: packages/Python/lldbsuite/test/functionalities/target_create_deps/main.cpp
===================================================================
--- /dev/null
+++ packages/Python/lldbsuite/test/functionalities/target_create_deps/main.cpp
@@ -0,0 +1,17 @@
+//===-- main.c --------------------------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+extern int a_function ();
+extern int b_function ();
+
+int
+main (int argc, char const *argv[])
+{
+    return a_function();
+}
Index: packages/Python/lldbsuite/test/functionalities/target_create_deps/a.mk
===================================================================
--- /dev/null
+++ packages/Python/lldbsuite/test/functionalities/target_create_deps/a.mk
@@ -0,0 +1,9 @@
+LEVEL := ../../make
+
+LIB_PREFIX := load_
+
+DYLIB_NAME := $(LIB_PREFIX)a
+DYLIB_CXX_SOURCES := a.cpp
+DYLIB_ONLY := YES
+
+include $(LEVEL)/Makefile.rules
Index: packages/Python/lldbsuite/test/functionalities/target_create_deps/a.cpp
===================================================================
--- /dev/null
+++ packages/Python/lldbsuite/test/functionalities/target_create_deps/a.cpp
@@ -0,0 +1,13 @@
+//===-- b.c -----------------------------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+int a_function ()
+{
+    return 500;
+}
Index: packages/Python/lldbsuite/test/functionalities/target_create_deps/TestTargetCreateDeps.py
===================================================================
--- /dev/null
+++ packages/Python/lldbsuite/test/functionalities/target_create_deps/TestTargetCreateDeps.py
@@ -0,0 +1,82 @@
+"""
+Test that breakpoint by symbol name works correctly with dynamic libs.
+"""
+
+from __future__ import print_function
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TargetDependentsTestCase(TestBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+
+    NO_DEBUG_INFO_TESTCASE = True
+
+    def setUp(self):
+        TestBase.setUp(self)
+        self.build()
+
+    def has_exactly_one_image(self, matching, msg=""):
+        self.expect(
+            "image list",
+            "image list should contain at least one image",
+            substrs=['[  0]'])
+        should_match = not matching
+        self.expect(
+            "image list", msg, matching=should_match, substrs=['[  1]'])
+
+    def test_dependents_implicit_default_exe(self):
+        """Test default behavior"""
+        exe = self.getBuildArtifact("a.out")
+        self.runCmd("target create  " + exe, CURRENT_EXECUTABLE_SET)
+        self.has_exactly_one_image(False)
+
+    def test_dependents_explicit_default_exe(self):
+        """Test default behavior"""
+        exe = self.getBuildArtifact("a.out")
+        self.runCmd("target create -d default " + exe, CURRENT_EXECUTABLE_SET)
+        self.has_exactly_one_image(False)
+
+    def test_dependents_explicit_yes_exe(self):
+        """Test default behavior"""
+        exe = self.getBuildArtifact("a.out")
+        self.runCmd("target create -d yes " + exe, CURRENT_EXECUTABLE_SET)
+        self.has_exactly_one_image(False)
+
+    def test_dependents_explicit_no_exe(self):
+        """Test default behavior"""
+        exe = self.getBuildArtifact("a.out")
+        self.runCmd("target create -d no " + exe, CURRENT_EXECUTABLE_SET)
+        self.has_exactly_one_image(True)
+
+    def test_dependents_implicit_default_lib(self):
+        ctx = self.platformContext
+        dylibName = ctx.shlib_prefix + 'load_a.' + ctx.shlib_extension
+        lib = self.getBuildArtifact(dylibName)
+        self.runCmd("target create " + lib, CURRENT_EXECUTABLE_SET)
+        self.has_exactly_one_image(True)
+
+    def test_dependents_explicit_default_lib(self):
+        ctx = self.platformContext
+        dylibName = ctx.shlib_prefix + 'load_a.' + ctx.shlib_extension
+        lib = self.getBuildArtifact(dylibName)
+        self.runCmd("target create -d default " + lib, CURRENT_EXECUTABLE_SET)
+        self.has_exactly_one_image(True)
+
+    def test_dependents_explicit_yes_lib(self):
+        ctx = self.platformContext
+        dylibName = ctx.shlib_prefix + 'load_a.' + ctx.shlib_extension
+        lib = self.getBuildArtifact(dylibName)
+        self.runCmd("target create -d yes " + lib, CURRENT_EXECUTABLE_SET)
+        self.has_exactly_one_image(False)
+
+    def test_dependents_explicit_no_lib(self):
+        ctx = self.platformContext
+        dylibName = ctx.shlib_prefix + 'load_a.' + ctx.shlib_extension
+        lib = self.getBuildArtifact(dylibName)
+        self.runCmd("target create -d no " + lib, CURRENT_EXECUTABLE_SET)
+        self.has_exactly_one_image(True)
Index: packages/Python/lldbsuite/test/functionalities/target_create_deps/Makefile
===================================================================
--- /dev/null
+++ packages/Python/lldbsuite/test/functionalities/target_create_deps/Makefile
@@ -0,0 +1,16 @@
+LEVEL := ../../make
+
+LIB_PREFIX := load_
+
+LD_EXTRAS := -L. -l$(LIB_PREFIX)a
+CXX_SOURCES := main.cpp
+
+include $(LEVEL)/Makefile.rules
+
+a.out: lib_a
+
+lib_%:
+	$(MAKE) VPATH=$(SRCDIR) -I $(SRCDIR) -f $(SRCDIR)/$*.mk
+
+clean::
+	$(MAKE) -f $(SRCDIR)/a.mk clean
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to