JosephTremoulet created this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
Using a BreakpointList corrupts the breakpoints' IDs because
BreakpointList::Add sets the ID, so use a vector instead.
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.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D70907
Files:
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
Index: lldb/source/Target/Target.cpp
===================================================================
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -728,11 +728,11 @@
}
void Target::ApplyNameToBreakpoints(BreakpointName &bp_name) {
- BreakpointList bkpts_with_name(false);
+ std::vector<lldb::BreakpointSP> bkpts_with_name;
m_breakpoint_list.FindBreakpointsByName(bp_name.GetName().AsCString(),
bkpts_with_name);
- for (auto bp_sp : bkpts_with_name.Breakpoints())
+ for (auto bp_sp : bkpts_with_name)
bp_name.ConfigureBreakpoint(bp_sp);
}
Index: lldb/source/Breakpoint/BreakpointList.cpp
===================================================================
--- lldb/source/Breakpoint/BreakpointList.cpp
+++ lldb/source/Breakpoint/BreakpointList.cpp
@@ -129,7 +129,7 @@
}
bool BreakpointList::FindBreakpointsByName(const char *name,
- BreakpointList &matching_bps) {
+ std::vector<lldb::BreakpointSP>
&matching_bps) {
Status error;
if (!name)
return false;
@@ -139,7 +139,7 @@
for (BreakpointSP bkpt_sp : Breakpoints()) {
if (bkpt_sp->MatchesName(name)) {
- matching_bps.Add(bkpt_sp, false);
+ matching_bps.push_back(bkpt_sp);
}
}
Index: lldb/source/API/SBTarget.cpp
===================================================================
--- lldb/source/API/SBTarget.cpp
+++ lldb/source/API/SBTarget.cpp
@@ -1176,12 +1176,12 @@
TargetSP target_sp(GetSP());
if (target_sp) {
std::lock_guard<std::recursive_mutex> guard(target_sp->GetAPIMutex());
- BreakpointList bkpt_list(false);
+ std::vector<lldb::BreakpointSP> bkpt_list;
bool is_valid =
target_sp->GetBreakpointList().FindBreakpointsByName(name, bkpt_list);
if (!is_valid)
return false;
- for (BreakpointSP bkpt_sp : bkpt_list.Breakpoints()) {
+ for (BreakpointSP bkpt_sp : bkpt_list) {
bkpts.AppendByID(bkpt_sp->GetID());
}
}
Index:
lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_names/TestBreakpointNames.py
===================================================================
---
lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_names/TestBreakpointNames.py
+++
lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_names/TestBreakpointNames.py
@@ -155,8 +155,13 @@
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 @@
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)
Index: lldb/include/lldb/Breakpoint/BreakpointList.h
===================================================================
--- lldb/include/lldb/Breakpoint/BreakpointList.h
+++ lldb/include/lldb/Breakpoint/BreakpointList.h
@@ -68,7 +68,7 @@
///
/// \result
/// \bfalse if the input name was not a legal breakpoint name.
- bool FindBreakpointsByName(const char *name, BreakpointList &matching_bps);
+ bool FindBreakpointsByName(const char *name, std::vector<lldb::BreakpointSP>
&matching_bps);
/// Returns the number of elements in this breakpoint list.
///
Index: lldb/source/Target/Target.cpp
===================================================================
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -728,11 +728,11 @@
}
void Target::ApplyNameToBreakpoints(BreakpointName &bp_name) {
- BreakpointList bkpts_with_name(false);
+ std::vector<lldb::BreakpointSP> bkpts_with_name;
m_breakpoint_list.FindBreakpointsByName(bp_name.GetName().AsCString(),
bkpts_with_name);
- for (auto bp_sp : bkpts_with_name.Breakpoints())
+ for (auto bp_sp : bkpts_with_name)
bp_name.ConfigureBreakpoint(bp_sp);
}
Index: lldb/source/Breakpoint/BreakpointList.cpp
===================================================================
--- lldb/source/Breakpoint/BreakpointList.cpp
+++ lldb/source/Breakpoint/BreakpointList.cpp
@@ -129,7 +129,7 @@
}
bool BreakpointList::FindBreakpointsByName(const char *name,
- BreakpointList &matching_bps) {
+ std::vector<lldb::BreakpointSP> &matching_bps) {
Status error;
if (!name)
return false;
@@ -139,7 +139,7 @@
for (BreakpointSP bkpt_sp : Breakpoints()) {
if (bkpt_sp->MatchesName(name)) {
- matching_bps.Add(bkpt_sp, false);
+ matching_bps.push_back(bkpt_sp);
}
}
Index: lldb/source/API/SBTarget.cpp
===================================================================
--- lldb/source/API/SBTarget.cpp
+++ lldb/source/API/SBTarget.cpp
@@ -1176,12 +1176,12 @@
TargetSP target_sp(GetSP());
if (target_sp) {
std::lock_guard<std::recursive_mutex> guard(target_sp->GetAPIMutex());
- BreakpointList bkpt_list(false);
+ std::vector<lldb::BreakpointSP> bkpt_list;
bool is_valid =
target_sp->GetBreakpointList().FindBreakpointsByName(name, bkpt_list);
if (!is_valid)
return false;
- for (BreakpointSP bkpt_sp : bkpt_list.Breakpoints()) {
+ for (BreakpointSP bkpt_sp : bkpt_list) {
bkpts.AppendByID(bkpt_sp->GetID());
}
}
Index: lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_names/TestBreakpointNames.py
===================================================================
--- lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_names/TestBreakpointNames.py
+++ lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_names/TestBreakpointNames.py
@@ -155,8 +155,13 @@
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 @@
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)
Index: lldb/include/lldb/Breakpoint/BreakpointList.h
===================================================================
--- lldb/include/lldb/Breakpoint/BreakpointList.h
+++ lldb/include/lldb/Breakpoint/BreakpointList.h
@@ -68,7 +68,7 @@
///
/// \result
/// \bfalse if the input name was not a legal breakpoint name.
- bool FindBreakpointsByName(const char *name, BreakpointList &matching_bps);
+ bool FindBreakpointsByName(const char *name, std::vector<lldb::BreakpointSP> &matching_bps);
/// Returns the number of elements in this breakpoint list.
///
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
