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