Author: Joseph Tremoulet Date: 2019-12-04T09:57:15-05:00 New Revision: 95b2e516bd3e4587953e44bf062054ff84f2b057
URL: https://github.com/llvm/llvm-project/commit/95b2e516bd3e4587953e44bf062054ff84f2b057 DIFF: https://github.com/llvm/llvm-project/commit/95b2e516bd3e4587953e44bf062054ff84f2b057.diff LOG: Change Target::FindBreakpointsByName to return Expected<vector> Summary: Using a BreakpointList corrupts the breakpoints' IDs because BreakpointList::Add sets the ID, so use a vector instead, and update the signature to return the vector wrapped in an llvm::Expected which can propagate any error from the inner call to StringIsBreakpointName. Note that, despite the similar name, SBTarget::FindBreakpointsByName doesn't suffer the same problem, because it uses a SBBreakpointList, which is more like a BreakpointIDList than a BreakpointList under the covers. Add a check to TestBreakpointNames that, without this fix, notices the ID getting mutated and fails. Reviewers: jingham, JDevlieghere Reviewed By: JDevlieghere Subscribers: lldb-commits Tags: #lldb Differential Revision: https://reviews.llvm.org/D70907 Added: Modified: lldb/include/lldb/Breakpoint/BreakpointList.h lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_names/TestBreakpointNames.py lldb/source/API/SBTarget.cpp lldb/source/Breakpoint/BreakpointList.cpp lldb/source/Target/Target.cpp Removed: ################################################################################ diff --git a/lldb/include/lldb/Breakpoint/BreakpointList.h b/lldb/include/lldb/Breakpoint/BreakpointList.h index 110e8d41f36b..ad68151fefc7 100644 --- a/lldb/include/lldb/Breakpoint/BreakpointList.h +++ b/lldb/include/lldb/Breakpoint/BreakpointList.h @@ -67,8 +67,10 @@ class BreakpointList { /// The breakpoint name for which to search. /// /// \result - /// \bfalse if the input name was not a legal breakpoint name. - bool FindBreakpointsByName(const char *name, BreakpointList &matching_bps); + /// error if the input name was not a legal breakpoint name, vector + /// of breakpoints otherwise. + llvm::Expected<std::vector<lldb::BreakpointSP>> + FindBreakpointsByName(const char *name); /// Returns the number of elements in this breakpoint list. /// diff --git a/lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_names/TestBreakpointNames.py b/lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_names/TestBreakpointNames.py index 4a5ed87e330f..9513278ba084 100644 --- a/lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_names/TestBreakpointNames.py +++ b/lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_names/TestBreakpointNames.py @@ -155,8 +155,13 @@ def do_check_illegal_names(self): def do_check_using_names(self): """Use Python APIs to check names work in place of breakpoint ID's.""" + # Create a dummy breakpoint to use up ID 1 + _ = self.target.BreakpointCreateByLocation(self.main_file_spec, 30) + + # Create a breakpiont to test with bkpt = self.target.BreakpointCreateByLocation(self.main_file_spec, 10) bkpt_name = "ABreakpoint" + bkpt_id = bkpt.GetID() other_bkpt_name= "_AnotherBreakpoint" # Add a name and make sure we match it: @@ -169,6 +174,7 @@ def do_check_using_names(self): self.assertTrue(bkpts.GetSize() == 1, "One breakpoint matched.") found_bkpt = bkpts.GetBreakpointAtIndex(0) self.assertTrue(bkpt.GetID() == found_bkpt.GetID(),"The right breakpoint.") + self.assertTrue(bkpt.GetID() == bkpt_id,"With the same ID as before.") retval = lldb.SBCommandReturnObject() self.dbg.GetCommandInterpreter().HandleCommand("break disable %s"%(bkpt_name), retval) diff --git a/lldb/source/API/SBTarget.cpp b/lldb/source/API/SBTarget.cpp index 7013e2b45e5f..312e4df75863 100644 --- a/lldb/source/API/SBTarget.cpp +++ b/lldb/source/API/SBTarget.cpp @@ -1176,12 +1176,15 @@ bool SBTarget::FindBreakpointsByName(const char *name, TargetSP target_sp(GetSP()); if (target_sp) { std::lock_guard<std::recursive_mutex> guard(target_sp->GetAPIMutex()); - BreakpointList bkpt_list(false); - bool is_valid = - target_sp->GetBreakpointList().FindBreakpointsByName(name, bkpt_list); - if (!is_valid) + llvm::Expected<std::vector<BreakpointSP>> expected_vector = + target_sp->GetBreakpointList().FindBreakpointsByName(name); + if (!expected_vector) { + LLDB_LOG(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_BREAKPOINTS), + "invalid breakpoint name: {}", + llvm::toString(expected_vector.takeError())); return false; - for (BreakpointSP bkpt_sp : bkpt_list.Breakpoints()) { + } + for (BreakpointSP bkpt_sp : *expected_vector) { bkpts.AppendByID(bkpt_sp->GetID()); } } diff --git a/lldb/source/Breakpoint/BreakpointList.cpp b/lldb/source/Breakpoint/BreakpointList.cpp index c80fb917b490..5b23c633d14c 100644 --- a/lldb/source/Breakpoint/BreakpointList.cpp +++ b/lldb/source/Breakpoint/BreakpointList.cpp @@ -10,6 +10,8 @@ #include "lldb/Target/Target.h" +#include "llvm/Support/Errc.h" + using namespace lldb; using namespace lldb_private; @@ -128,22 +130,24 @@ BreakpointSP BreakpointList::FindBreakpointByID(break_id_t break_id) const { return {}; } -bool BreakpointList::FindBreakpointsByName(const char *name, - BreakpointList &matching_bps) { - Status error; +llvm::Expected<std::vector<lldb::BreakpointSP>> +BreakpointList::FindBreakpointsByName(const char *name) { if (!name) - return false; + return llvm::createStringError(llvm::errc::invalid_argument, + "FindBreakpointsByName requires a name"); + Status error; if (!BreakpointID::StringIsBreakpointName(llvm::StringRef(name), error)) - return false; + return error.ToError(); + std::vector<lldb::BreakpointSP> matching_bps; for (BreakpointSP bkpt_sp : Breakpoints()) { if (bkpt_sp->MatchesName(name)) { - matching_bps.Add(bkpt_sp, false); + matching_bps.push_back(bkpt_sp); } } - return true; + return matching_bps; } void BreakpointList::Dump(Stream *s) const { diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp index 01b9a92cafcf..59f72141ee5f 100644 --- a/lldb/source/Target/Target.cpp +++ b/lldb/source/Target/Target.cpp @@ -728,11 +728,17 @@ void Target::ConfigureBreakpointName( } void Target::ApplyNameToBreakpoints(BreakpointName &bp_name) { - BreakpointList bkpts_with_name(false); - m_breakpoint_list.FindBreakpointsByName(bp_name.GetName().AsCString(), - bkpts_with_name); + llvm::Expected<std::vector<BreakpointSP>> expected_vector = + m_breakpoint_list.FindBreakpointsByName(bp_name.GetName().AsCString()); - for (auto bp_sp : bkpts_with_name.Breakpoints()) + if (!expected_vector) { + LLDB_LOG(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_BREAKPOINTS), + "invalid breakpoint name: {}", + llvm::toString(expected_vector.takeError())); + return; + } + + for (auto bp_sp : *expected_vector) bp_name.ConfigureBreakpoint(bp_sp); } _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
