[Lldb-commits] [lldb] 95b2e51 - Change Target::FindBreakpointsByName to return Expected<vector>
Joseph Tremoulet via lldb-commits
lldb-commits at lists.llvm.org
Wed Dec 4 06:57:52 PST 2019
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);
}
More information about the lldb-commits
mailing list