From 26ba928214273cba2d1fb951bd395e6fa74896bb Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere Date: Thu, 20 Sep 2018 09:09:13 +0000 Subject: [PATCH] [target] Change target create's behavior wrt loading dependent files. 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 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 to change this behavior. rdar://problem/43721382 Differential revision: https://reviews.llvm.org/D51934 llvm-svn: 342634 --- .../functionalities/target_create_deps/Makefile | 16 ++++ .../target_create_deps/TestTargetCreateDeps.py | 96 ++++++++++++++++++++++ .../test/functionalities/target_create_deps/a.cpp | 13 +++ .../test/functionalities/target_create_deps/a.mk | 9 ++ .../functionalities/target_create_deps/main.cpp | 17 ++++ lldb/source/Commands/CommandObjectTarget.cpp | 85 ++++++++++++++++--- 6 files changed, 224 insertions(+), 12 deletions(-) create mode 100644 lldb/packages/Python/lldbsuite/test/functionalities/target_create_deps/Makefile create mode 100644 lldb/packages/Python/lldbsuite/test/functionalities/target_create_deps/TestTargetCreateDeps.py create mode 100644 lldb/packages/Python/lldbsuite/test/functionalities/target_create_deps/a.cpp create mode 100644 lldb/packages/Python/lldbsuite/test/functionalities/target_create_deps/a.mk create mode 100644 lldb/packages/Python/lldbsuite/test/functionalities/target_create_deps/main.cpp diff --git a/lldb/packages/Python/lldbsuite/test/functionalities/target_create_deps/Makefile b/lldb/packages/Python/lldbsuite/test/functionalities/target_create_deps/Makefile new file mode 100644 index 0000000..15cb0b6 --- /dev/null +++ b/lldb/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 diff --git a/lldb/packages/Python/lldbsuite/test/functionalities/target_create_deps/TestTargetCreateDeps.py b/lldb/packages/Python/lldbsuite/test/functionalities/target_create_deps/TestTargetCreateDeps.py new file mode 100644 index 0000000..a6886da --- /dev/null +++ b/lldb/packages/Python/lldbsuite/test/functionalities/target_create_deps/TestTargetCreateDeps.py @@ -0,0 +1,96 @@ +""" +Test that loading of dependents works correctly for all the potential +combinations. +""" + +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 -ddefault " + exe, CURRENT_EXECUTABLE_SET) + self.has_exactly_one_image(False) + + def test_dependents_explicit_true_exe(self): + """Test default behavior""" + exe = self.getBuildArtifact("a.out") + self.runCmd("target create -dtrue " + exe, CURRENT_EXECUTABLE_SET) + self.has_exactly_one_image(True) + + def test_dependents_explicit_false_exe(self): + """Test default behavior""" + exe = self.getBuildArtifact("a.out") + self.runCmd("target create -dfalse " + exe, CURRENT_EXECUTABLE_SET) + self.has_exactly_one_image(False) + + def test_dependents_implicit_false_exe(self): + """Test default behavior""" + exe = self.getBuildArtifact("a.out") + self.runCmd("target create -d " + 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 -ddefault " + lib, CURRENT_EXECUTABLE_SET) + self.has_exactly_one_image(True) + + def test_dependents_explicit_true_lib(self): + ctx = self.platformContext + dylibName = ctx.shlib_prefix + 'load_a.' + ctx.shlib_extension + lib = self.getBuildArtifact(dylibName) + self.runCmd("target create -dtrue " + lib, CURRENT_EXECUTABLE_SET) + self.has_exactly_one_image(True) + + def test_dependents_explicit_false_lib(self): + ctx = self.platformContext + dylibName = ctx.shlib_prefix + 'load_a.' + ctx.shlib_extension + lib = self.getBuildArtifact(dylibName) + self.runCmd("target create -dfalse " + lib, CURRENT_EXECUTABLE_SET) + self.has_exactly_one_image(False) + + def test_dependents_implicit_false_lib(self): + ctx = self.platformContext + dylibName = ctx.shlib_prefix + 'load_a.' + ctx.shlib_extension + lib = self.getBuildArtifact(dylibName) + self.runCmd("target create -d " + lib, CURRENT_EXECUTABLE_SET) + self.has_exactly_one_image(True) diff --git a/lldb/packages/Python/lldbsuite/test/functionalities/target_create_deps/a.cpp b/lldb/packages/Python/lldbsuite/test/functionalities/target_create_deps/a.cpp new file mode 100644 index 0000000..c0dac40 --- /dev/null +++ b/lldb/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; +} diff --git a/lldb/packages/Python/lldbsuite/test/functionalities/target_create_deps/a.mk b/lldb/packages/Python/lldbsuite/test/functionalities/target_create_deps/a.mk new file mode 100644 index 0000000..f199bfe --- /dev/null +++ b/lldb/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 diff --git a/lldb/packages/Python/lldbsuite/test/functionalities/target_create_deps/main.cpp b/lldb/packages/Python/lldbsuite/test/functionalities/target_create_deps/main.cpp new file mode 100644 index 0000000..08fbb59 --- /dev/null +++ b/lldb/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(); +} diff --git a/lldb/source/Commands/CommandObjectTarget.cpp b/lldb/source/Commands/CommandObjectTarget.cpp index 7ba0d6c..09e47b4 100644 --- a/lldb/source/Commands/CommandObjectTarget.cpp +++ b/lldb/source/Commands/CommandObjectTarget.cpp @@ -138,6 +138,72 @@ static uint32_t DumpTargetList(TargetList &target_list, return num_targets; } +// Note that the negation in the argument name causes a slightly confusing +// mapping of the enum values, +static OptionEnumValueElement g_dependents_enumaration[4] = { + {eLoadDependentsDefault, "default", + "Only load dependents when the target is an executable."}, + {eLoadDependentsNo, "true", + "Don't load dependents, even if the target is an executable."}, + {eLoadDependentsYes, "false", + "Load dependents, even if the target is not an executable."}, + {0, nullptr, nullptr}}; + +static OptionDefinition g_dependents_options[1] = { + {LLDB_OPT_SET_1, false, "no-dependents", 'd', + OptionParser::eOptionalArgument, nullptr, g_dependents_enumaration, 0, + eArgTypeValue, + "Whether or not to load dependents when creating a target. If the option " + "is not specified, the value is implicitly 'default'. If the option is " + "specified but without a value, the value is implicitly 'true'."}}; + +class OptionGroupDependents : public OptionGroup { +public: + OptionGroupDependents() {} + + ~OptionGroupDependents() override {} + + llvm::ArrayRef GetDefinitions() override { + return llvm::makeArrayRef(g_dependents_options); + } + + Status SetOptionValue(uint32_t option_idx, llvm::StringRef option_value, + ExecutionContext *execution_context) override { + Status error; + + // For compatibility no value means don't load dependents. + if (option_value.empty()) { + m_load_dependent_files = eLoadDependentsNo; + return error; + } + + const char short_option = g_dependents_options[option_idx].short_option; + if (short_option == '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; + } else { + error.SetErrorStringWithFormat("unrecognized short option '%c'", + short_option); + } + + 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 +224,14 @@ public: 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 +323,9 @@ protected: 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 +473,7 @@ private: OptionGroupFile m_platform_path; OptionGroupFile m_symbol_file; OptionGroupFile m_remote_file; - OptionGroupBoolean m_add_dependents; + OptionGroupDependents m_add_dependents; }; #pragma mark CommandObjectTargetList -- 2.7.4