From 185ef697ef5c60d7a5c801925e6abdad52226c2b Mon Sep 17 00:00:00 2001 From: Tatyana Krasnukha Date: Thu, 13 Feb 2020 16:48:38 +0300 Subject: [PATCH] [lldb] Don't call CopyForBreakpoint from a Breakpoint's constructor Some implementations (BreakpointResolverScripted) try calling the breakpoint's shared_from_this(), that makes LLDB crash. --- lldb/include/lldb/Breakpoint/Breakpoint.h | 10 ++++-- lldb/source/Breakpoint/Breakpoint.cpp | 17 ++++++---- lldb/source/Target/Target.cpp | 6 ++-- .../scripted_bkpt/TestScriptedResolver.py | 36 ++++++++++++++++++++-- 4 files changed, 55 insertions(+), 14 deletions(-) diff --git a/lldb/include/lldb/Breakpoint/Breakpoint.h b/lldb/include/lldb/Breakpoint/Breakpoint.h index 553b911..c48a0ad 100644 --- a/lldb/include/lldb/Breakpoint/Breakpoint.h +++ b/lldb/include/lldb/Breakpoint/Breakpoint.h @@ -568,6 +568,11 @@ public: return GetPermissions().GetAllowDelete(); } + // This one should only be used by Target to copy breakpoints from target to + // target - primarily from the dummy target to prime new targets. + static lldb::BreakpointSP CopyFromBreakpoint(Target& new_target, + const Breakpoint &bp_to_copy_from); + protected: friend class Target; // Protected Methods @@ -625,9 +630,8 @@ protected: } private: - // This one should only be used by Target to copy breakpoints from target to - // target - primarily from the dummy target to prime new targets. - Breakpoint(Target &new_target, Breakpoint &bp_to_copy_from); + // To call from CopyFromBreakpoint. + Breakpoint(Target &new_target, const Breakpoint &bp_to_copy_from); // For Breakpoint only bool m_being_created; diff --git a/lldb/source/Breakpoint/Breakpoint.cpp b/lldb/source/Breakpoint/Breakpoint.cpp index 78aef01..39c6828 100644 --- a/lldb/source/Breakpoint/Breakpoint.cpp +++ b/lldb/source/Breakpoint/Breakpoint.cpp @@ -55,21 +55,26 @@ Breakpoint::Breakpoint(Target &target, SearchFilterSP &filter_sp, m_being_created = false; } -Breakpoint::Breakpoint(Target &new_target, Breakpoint &source_bp) +Breakpoint::Breakpoint(Target &new_target, const Breakpoint &source_bp) : m_being_created(true), m_hardware(source_bp.m_hardware), m_target(new_target), m_name_list(source_bp.m_name_list), m_options_up(new BreakpointOptions(*source_bp.m_options_up)), m_locations(*this), m_resolve_indirect_symbols(source_bp.m_resolve_indirect_symbols), - m_hit_count(0) { - // Now go through and copy the filter & resolver: - m_resolver_sp = source_bp.m_resolver_sp->CopyForBreakpoint(*this); - m_filter_sp = source_bp.m_filter_sp->CopyForBreakpoint(*this); -} + m_hit_count(0) {} // Destructor Breakpoint::~Breakpoint() = default; +BreakpointSP Breakpoint::CopyFromBreakpoint(Target& new_target, + const Breakpoint& bp_to_copy_from) { + BreakpointSP bp(new Breakpoint(new_target, bp_to_copy_from)); + // Now go through and copy the filter & resolver: + bp->m_resolver_sp = bp_to_copy_from.m_resolver_sp->CopyForBreakpoint(*bp); + bp->m_filter_sp = bp_to_copy_from.m_filter_sp->CopyForBreakpoint(*bp); + return bp; +} + // Serialization StructuredData::ObjectSP Breakpoint::SerializeToStructuredData() { // Serialize the resolver: diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp index 9e41e90..49cb52b 100644 --- a/lldb/source/Target/Target.cpp +++ b/lldb/source/Target/Target.cpp @@ -127,12 +127,12 @@ void Target::PrimeFromDummyTarget(Target *target) { m_stop_hooks = target->m_stop_hooks; - for (BreakpointSP breakpoint_sp : target->m_breakpoint_list.Breakpoints()) { + for (const auto &breakpoint_sp : target->m_breakpoint_list.Breakpoints()) { if (breakpoint_sp->IsInternal()) continue; - BreakpointSP new_bp(new Breakpoint(*this, *breakpoint_sp.get())); - AddBreakpoint(new_bp, false); + BreakpointSP new_bp(Breakpoint::CopyFromBreakpoint(*this, *breakpoint_sp)); + AddBreakpoint(std::move(new_bp), false); } for (auto bp_name_entry : target->m_breakpoint_names) { diff --git a/lldb/test/API/functionalities/breakpoint/scripted_bkpt/TestScriptedResolver.py b/lldb/test/API/functionalities/breakpoint/scripted_bkpt/TestScriptedResolver.py index 9f0ba11..b0b0411 100644 --- a/lldb/test/API/functionalities/breakpoint/scripted_bkpt/TestScriptedResolver.py +++ b/lldb/test/API/functionalities/breakpoint/scripted_bkpt/TestScriptedResolver.py @@ -40,8 +40,21 @@ class TestScriptedResolver(TestBase): self.build() self.do_test_bad_options() + @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24528") + def test_copy_from_dummy_target(self): + """Make sure we don't crash during scripted breakpoint copy from dummy target""" + self.build() + self.do_test_copy_from_dummy_target() + def make_target_and_import(self): - target = lldbutil.run_to_breakpoint_make_target(self) + target = self.make_target() + self.import_resolver_script() + return target + + def make_target(self): + return lldbutil.run_to_breakpoint_make_target(self) + + def import_resolver_script(self): interp = self.dbg.GetCommandInterpreter() error = lldb.SBError() @@ -52,7 +65,6 @@ class TestScriptedResolver(TestBase): result = lldb.SBCommandReturnObject() interp.HandleCommand(command, result) self.assertTrue(result.Succeeded(), "com scr imp failed: %s"%(result.GetError())) - return target def make_extra_args(self): json_string = '{"symbol":"break_on_me", "test1": "value1"}' @@ -222,3 +234,23 @@ class TestScriptedResolver(TestBase): substrs=['Value: "a_value" missing matching key']) self.expect("break set -P resolver.Resolver -k a_key -k a_key -v another_value", error = True, msg="Missing value among args", substrs=['Key: "a_key" missing value']) + + def do_test_copy_from_dummy_target(self): + # Import breakpoint scripted resolver. + self.import_resolver_script() + + # Create a scripted breakpoint. + self.runCmd("breakpoint set -P resolver.Resolver -k symbol -v break_on_me", + BREAKPOINT_CREATED) + + # This is the function to remove breakpoints from the dummy target + # to get a clean state for the next test case. + def cleanup(): + self.runCmd('breakpoint delete -D -f', check=False) + self.runCmd('breakpoint list', check=False) + + # Execute the cleanup function during test case tear down. + self.addTearDownHook(cleanup) + + # Check that target creating doesn't crash. + target = self.make_target() -- 2.7.4