From b060e19c0c2ee8b653d9d878181f1426f6ddb916 Mon Sep 17 00:00:00 2001 From: Jim Ingham Date: Tue, 14 Oct 2025 15:57:23 -0700 Subject: [PATCH 1/5] Fix a potential use-after-free in StopInfoBreakpoint. StopInfoBreakpoint keeps a BreakpointLocationCollection for all the breakpoint locations at the BreakpointSite that was hit. It is also lives through the time a given thread is stopped, so there are plenty of opportunities for one of the owning breakpoints to get deleted. But BreakpointLocations don't keep their owner Breakpoints alive, so if the BreakpointLocationCollection can live past when some code gets a chance to delete an owner breakpoint, and then you ask that location for some breakpoint information, it will access freed memory. This wasn't a problem before PR #158128 because the StopInfoBreakpoint just kept the BreakpointSite that was hit, and when you asked it questions, it relooked up that list. That was not great, however, because if you hit breakpoints 5 & 6, deleted 5 and then asked which breakpoints got hit, you would just get 6. For that and other reasons that PR changed to storing a BreakpointLocationCollection of the breakpoints that were hit. That's better from a UI perspective but caused this potential problem. I fix it by adding a variant of the BreakpointLocationCollection that also holds onto a shared pointer to the Breakpoints that own the locations that were hit, thus keeping them alive till the StopInfoBreakpoint goes away. This fixed the ASAN assertion. I also added a test that works harder to cause trouble by deleting breakpoints during a stop. --- .../Breakpoint/BreakpointLocationCollection.h | 28 ++++++-- .../BreakpointLocationCollection.cpp | 22 ++++++- lldb/source/Target/StopInfo.cpp | 13 +++- .../callback_deletes_breakpoints/Makefile | 4 ++ .../TestCallbackDeletesBreakpoints.py | 65 +++++++++++++++++++ .../callback_deletes_breakpoints/main.c | 15 +++++ 6 files changed, 137 insertions(+), 10 deletions(-) create mode 100644 lldb/test/API/functionalities/breakpoint/callback_deletes_breakpoints/Makefile create mode 100644 lldb/test/API/functionalities/breakpoint/callback_deletes_breakpoints/TestCallbackDeletesBreakpoints.py create mode 100644 lldb/test/API/functionalities/breakpoint/callback_deletes_breakpoints/main.c diff --git a/lldb/include/lldb/Breakpoint/BreakpointLocationCollection.h b/lldb/include/lldb/Breakpoint/BreakpointLocationCollection.h index 1df4e074680f5..10a22476aba9b 100644 --- a/lldb/include/lldb/Breakpoint/BreakpointLocationCollection.h +++ b/lldb/include/lldb/Breakpoint/BreakpointLocationCollection.h @@ -9,6 +9,7 @@ #ifndef LLDB_BREAKPOINT_BREAKPOINTLOCATIONCOLLECTION_H #define LLDB_BREAKPOINT_BREAKPOINTLOCATIONCOLLECTION_H +#include #include #include @@ -19,9 +20,17 @@ namespace lldb_private { class BreakpointLocationCollection { public: - BreakpointLocationCollection(); - - ~BreakpointLocationCollection(); + /// Breakpoint locations don't keep their breakpoint owners alive, so neither + /// will a collection of breakpoint locations. However, if you need to + /// use this collection in a context where some of the breakpoints whose + /// locations are in the collection might get deleted during its lifespan, + /// then you need to make sure the breakpoints don't get deleted out from + /// under you. To do that, pass true for preserving, and so long as there is + /// a location for a given breakpoint in the collection, the breakpoint will + /// not get destroyed. + BreakpointLocationCollection(bool preserving = false); + + virtual ~BreakpointLocationCollection(); BreakpointLocationCollection &operator=(const BreakpointLocationCollection &rhs); @@ -30,7 +39,7 @@ class BreakpointLocationCollection { /// \param[in] bp_loc_sp /// Shared pointer to the breakpoint location that will get added /// to the list. - void Add(const lldb::BreakpointLocationSP &bp_loc_sp); + virtual void Add(const lldb::BreakpointLocationSP &bp_loc_sp); /// Removes the breakpoint location given by \b breakID from this /// list. @@ -43,7 +52,7 @@ class BreakpointLocationCollection { /// /// \result /// \b true if the breakpoint was in the list. - bool Remove(lldb::break_id_t break_id, lldb::break_id_t break_loc_id); + virtual bool Remove(lldb::break_id_t break_id, lldb::break_id_t break_loc_id); /// Returns a shared pointer to the breakpoint location with id \a /// breakID. @@ -164,6 +173,14 @@ class BreakpointLocationCollection { collection m_break_loc_collection; mutable std::mutex m_collection_mutex; + /// These are used if we're preserving breakpoints in this list: + const bool m_preserving_bkpts = false; + struct RefCountedBPSP { + RefCountedBPSP(lldb::BreakpointSP in_bp_sp) : ref_cnt(1), bp_sp(in_bp_sp) {} + uint64_t ref_cnt; + lldb::BreakpointSP bp_sp; + }; + std::map m_preserved_bps; public: typedef llvm::iterator_range @@ -172,7 +189,6 @@ class BreakpointLocationCollection { return BreakpointLocationCollectionIterable(m_break_loc_collection); } }; - } // namespace lldb_private #endif // LLDB_BREAKPOINT_BREAKPOINTLOCATIONCOLLECTION_H diff --git a/lldb/source/Breakpoint/BreakpointLocationCollection.cpp b/lldb/source/Breakpoint/BreakpointLocationCollection.cpp index 1d052c5fc9bb6..f250091e46f71 100644 --- a/lldb/source/Breakpoint/BreakpointLocationCollection.cpp +++ b/lldb/source/Breakpoint/BreakpointLocationCollection.cpp @@ -17,7 +17,8 @@ using namespace lldb; using namespace lldb_private; // BreakpointLocationCollection constructor -BreakpointLocationCollection::BreakpointLocationCollection() = default; +BreakpointLocationCollection::BreakpointLocationCollection(bool preserving) : + m_preserving_bkpts(preserving) {} // Destructor BreakpointLocationCollection::~BreakpointLocationCollection() = default; @@ -26,8 +27,18 @@ void BreakpointLocationCollection::Add(const BreakpointLocationSP &bp_loc) { std::lock_guard guard(m_collection_mutex); BreakpointLocationSP old_bp_loc = FindByIDPair(bp_loc->GetBreakpoint().GetID(), bp_loc->GetID()); - if (!old_bp_loc.get()) + if (!old_bp_loc.get()) { m_break_loc_collection.push_back(bp_loc); + if (m_preserving_bkpts) { + Breakpoint &bkpt = bp_loc->GetBreakpoint(); + lldb::break_id_t bp_id = bkpt.GetID(); + auto entry = m_preserved_bps.find(bp_id); + if (entry == m_preserved_bps.end()) + m_preserved_bps.emplace(bp_id, RefCountedBPSP(bkpt.shared_from_this())); + else + entry->second.ref_cnt++; + } + } } bool BreakpointLocationCollection::Remove(lldb::break_id_t bp_id, @@ -35,6 +46,13 @@ bool BreakpointLocationCollection::Remove(lldb::break_id_t bp_id, std::lock_guard guard(m_collection_mutex); collection::iterator pos = GetIDPairIterator(bp_id, bp_loc_id); // Predicate if (pos != m_break_loc_collection.end()) { + if (m_preserving_bkpts) { + auto entry = m_preserved_bps.find(bp_id); + assert(entry != m_preserved_bps.end() + && "Breakpoint added to base but not preserving map."); + if (--entry->second.ref_cnt == 0) + m_preserved_bps.erase(entry); + } m_break_loc_collection.erase(pos); return true; } diff --git a/lldb/source/Target/StopInfo.cpp b/lldb/source/Target/StopInfo.cpp index 7fa1fc5d71f13..6d5572c62294c 100644 --- a/lldb/source/Target/StopInfo.cpp +++ b/lldb/source/Target/StopInfo.cpp @@ -87,11 +87,15 @@ bool StopInfo::HasTargetRunSinceMe() { namespace lldb_private { class StopInfoBreakpoint : public StopInfo { public: + // We use a "breakpoint preserving BreakpointLocationCollection because we + // may need to hand out the "breakpoint hit" list as any point, potentially + // after the breakpoint has been deleted. But we still need to refer to them. StopInfoBreakpoint(Thread &thread, break_id_t break_id) : StopInfo(thread, break_id), m_should_stop(false), m_should_stop_is_valid(false), m_should_perform_action(true), m_address(LLDB_INVALID_ADDRESS), m_break_id(LLDB_INVALID_BREAK_ID), - m_was_all_internal(false), m_was_one_shot(false) { + m_was_all_internal(false), m_was_one_shot(false), + m_async_stopped_locs(true) { StoreBPInfo(); } @@ -99,7 +103,8 @@ class StopInfoBreakpoint : public StopInfo { : StopInfo(thread, break_id), m_should_stop(should_stop), m_should_stop_is_valid(true), m_should_perform_action(true), m_address(LLDB_INVALID_ADDRESS), m_break_id(LLDB_INVALID_BREAK_ID), - m_was_all_internal(false), m_was_one_shot(false) { + m_was_all_internal(false), m_was_one_shot(false), + m_async_stopped_locs(true) { StoreBPInfo(); } @@ -699,6 +704,10 @@ class StopInfoBreakpoint : public StopInfo { lldb::break_id_t m_break_id; bool m_was_all_internal; bool m_was_one_shot; + /// The StopInfoBreakpoint lives after the stop, and could get queried + /// at any time so we need to make sure that it keeps the breakpoints for + /// each of the locations it records alive while it is around. That's what + /// The BreakpointPreservingLocationCollection does. BreakpointLocationCollection m_async_stopped_locs; }; diff --git a/lldb/test/API/functionalities/breakpoint/callback_deletes_breakpoints/Makefile b/lldb/test/API/functionalities/breakpoint/callback_deletes_breakpoints/Makefile new file mode 100644 index 0000000000000..695335e068c0c --- /dev/null +++ b/lldb/test/API/functionalities/breakpoint/callback_deletes_breakpoints/Makefile @@ -0,0 +1,4 @@ +C_SOURCES := main.c +CFLAGS_EXTRAS := -std=c99 + +include Makefile.rules diff --git a/lldb/test/API/functionalities/breakpoint/callback_deletes_breakpoints/TestCallbackDeletesBreakpoints.py b/lldb/test/API/functionalities/breakpoint/callback_deletes_breakpoints/TestCallbackDeletesBreakpoints.py new file mode 100644 index 0000000000000..e8173dc000386 --- /dev/null +++ b/lldb/test/API/functionalities/breakpoint/callback_deletes_breakpoints/TestCallbackDeletesBreakpoints.py @@ -0,0 +1,65 @@ +""" +Make sure that deleting breakpoints in another breakpoint +callback doesn't cause problems. +""" + + +import lldb +import lldbsuite.test.lldbutil as lldbutil +from lldbsuite.test.lldbtest import * + + +class TestBreakpointDeletionInCallback(TestBase): + NO_DEBUG_INFO_TESTCASE = True + + def test_breakpoint_deletion_in_callback(self): + self.build() + self.main_source_file = lldb.SBFileSpec("main.c") + self.delete_others_test() + + def delete_others_test(self): + """You might use the test implementation in several ways, say so here.""" + + # This function starts a process, "a.out" by default, sets a source + # breakpoint, runs to it, and returns the thread, process & target. + # It optionally takes an SBLaunchOption argument if you want to pass + # arguments or environment variables. + (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint( + self, "Set a breakpoint here", self.main_source_file + ) + + # Now set a breakpoint on "I did something" several times + # + bkpt_numbers = [] + for idx in range(0,5): + bkpt_numbers.append(lldbutil.run_break_set_by_source_regexp(self, "// Deletable location")) + + # And add commands to the third one to delete two others: + deleter = target.FindBreakpointByID(bkpt_numbers[2]) + self.assertTrue(deleter.IsValid(), "Deleter is a good breakpoint") + commands = lldb.SBStringList() + deleted_ids = [bkpt_numbers[0], bkpt_numbers[3]] + for idx in deleted_ids: + commands.AppendString(f"break delete {idx}") + + deleter.SetCommandLineCommands(commands) + + thread_list = lldbutil.continue_to_breakpoint(process, deleter) + self.assertEqual(len(thread_list), 1) + stop_data = thread.stop_reason_data + # There are 5 breakpoints so 10 break_id, break_loc_id. + self.assertEqual(len(stop_data), 10) + # We should have been able to get break ID's and locations for all the + # breakpoints that we originally hit, but some won't be around anymore: + for idx in range(0,5): + bkpt_id = stop_data[idx*2] + print(f"{idx}: {bkpt_id}") + self.assertIn(bkpt_id, bkpt_numbers, "Found breakpoints are right") + loc_id = stop_data[idx*2 + 1] + self.assertEqual(loc_id, 1, "All breakpoints have one location") + bkpt = target.FindBreakpointByID(bkpt_id) + if bkpt_id in deleted_ids: + # Looking these up should be an error: + self.assertFalse(bkpt.IsValid(), "Deleted breakpoints are deleted") + else: + self.assertTrue(bkpt.IsValid(), "The rest are still valid") diff --git a/lldb/test/API/functionalities/breakpoint/callback_deletes_breakpoints/main.c b/lldb/test/API/functionalities/breakpoint/callback_deletes_breakpoints/main.c new file mode 100644 index 0000000000000..279b359523cab --- /dev/null +++ b/lldb/test/API/functionalities/breakpoint/callback_deletes_breakpoints/main.c @@ -0,0 +1,15 @@ +#include + +int +do_something(int input) { + return input % 5; // Deletable location +} + +int +main() +{ + printf ("Set a breakpoint here.\n"); + do_something(100); + do_something(200); + return 0; +} From 5e144dfe254b5d71cf561ba7c60263c94d848690 Mon Sep 17 00:00:00 2001 From: Jim Ingham Date: Tue, 14 Oct 2025 16:15:19 -0700 Subject: [PATCH 2/5] I had done this as a subclass, but decided not to. Remove the `virtual` marker I no longer need. --- lldb/include/lldb/Breakpoint/BreakpointLocationCollection.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lldb/include/lldb/Breakpoint/BreakpointLocationCollection.h b/lldb/include/lldb/Breakpoint/BreakpointLocationCollection.h index 10a22476aba9b..6f15c6078c425 100644 --- a/lldb/include/lldb/Breakpoint/BreakpointLocationCollection.h +++ b/lldb/include/lldb/Breakpoint/BreakpointLocationCollection.h @@ -30,7 +30,7 @@ class BreakpointLocationCollection { /// not get destroyed. BreakpointLocationCollection(bool preserving = false); - virtual ~BreakpointLocationCollection(); + ~BreakpointLocationCollection(); BreakpointLocationCollection &operator=(const BreakpointLocationCollection &rhs); @@ -39,7 +39,7 @@ class BreakpointLocationCollection { /// \param[in] bp_loc_sp /// Shared pointer to the breakpoint location that will get added /// to the list. - virtual void Add(const lldb::BreakpointLocationSP &bp_loc_sp); + void Add(const lldb::BreakpointLocationSP &bp_loc_sp); /// Removes the breakpoint location given by \b breakID from this /// list. @@ -52,7 +52,7 @@ class BreakpointLocationCollection { /// /// \result /// \b true if the breakpoint was in the list. - virtual bool Remove(lldb::break_id_t break_id, lldb::break_id_t break_loc_id); + bool Remove(lldb::break_id_t break_id, lldb::break_id_t break_loc_id); /// Returns a shared pointer to the breakpoint location with id \a /// breakID. From 292592414d3bf6b8b78924c4f1ecdd0deb91123a Mon Sep 17 00:00:00 2001 From: Jim Ingham Date: Tue, 14 Oct 2025 16:20:44 -0700 Subject: [PATCH 3/5] formatting --- .../lldb/Breakpoint/BreakpointLocationCollection.h | 6 +++--- .../Breakpoint/BreakpointLocationCollection.cpp | 12 ++++++------ lldb/source/Target/StopInfo.cpp | 4 ++-- .../TestCallbackDeletesBreakpoints.py | 12 +++++++----- .../breakpoint/callback_deletes_breakpoints/main.c | 9 +++------ 5 files changed, 21 insertions(+), 22 deletions(-) diff --git a/lldb/include/lldb/Breakpoint/BreakpointLocationCollection.h b/lldb/include/lldb/Breakpoint/BreakpointLocationCollection.h index 6f15c6078c425..30ec340258e33 100644 --- a/lldb/include/lldb/Breakpoint/BreakpointLocationCollection.h +++ b/lldb/include/lldb/Breakpoint/BreakpointLocationCollection.h @@ -27,7 +27,7 @@ class BreakpointLocationCollection { /// then you need to make sure the breakpoints don't get deleted out from /// under you. To do that, pass true for preserving, and so long as there is /// a location for a given breakpoint in the collection, the breakpoint will - /// not get destroyed. + /// not get destroyed. BreakpointLocationCollection(bool preserving = false); ~BreakpointLocationCollection(); @@ -178,9 +178,9 @@ class BreakpointLocationCollection { struct RefCountedBPSP { RefCountedBPSP(lldb::BreakpointSP in_bp_sp) : ref_cnt(1), bp_sp(in_bp_sp) {} uint64_t ref_cnt; - lldb::BreakpointSP bp_sp; + lldb::BreakpointSP bp_sp; }; - std::map m_preserved_bps; + std::map m_preserved_bps; public: typedef llvm::iterator_range diff --git a/lldb/source/Breakpoint/BreakpointLocationCollection.cpp b/lldb/source/Breakpoint/BreakpointLocationCollection.cpp index f250091e46f71..ef8921e2d645d 100644 --- a/lldb/source/Breakpoint/BreakpointLocationCollection.cpp +++ b/lldb/source/Breakpoint/BreakpointLocationCollection.cpp @@ -17,8 +17,8 @@ using namespace lldb; using namespace lldb_private; // BreakpointLocationCollection constructor -BreakpointLocationCollection::BreakpointLocationCollection(bool preserving) : - m_preserving_bkpts(preserving) {} +BreakpointLocationCollection::BreakpointLocationCollection(bool preserving) + : m_preserving_bkpts(preserving) {} // Destructor BreakpointLocationCollection::~BreakpointLocationCollection() = default; @@ -35,8 +35,8 @@ void BreakpointLocationCollection::Add(const BreakpointLocationSP &bp_loc) { auto entry = m_preserved_bps.find(bp_id); if (entry == m_preserved_bps.end()) m_preserved_bps.emplace(bp_id, RefCountedBPSP(bkpt.shared_from_this())); - else - entry->second.ref_cnt++; + else + entry->second.ref_cnt++; } } } @@ -48,8 +48,8 @@ bool BreakpointLocationCollection::Remove(lldb::break_id_t bp_id, if (pos != m_break_loc_collection.end()) { if (m_preserving_bkpts) { auto entry = m_preserved_bps.find(bp_id); - assert(entry != m_preserved_bps.end() - && "Breakpoint added to base but not preserving map."); + assert(entry != m_preserved_bps.end() && + "Breakpoint added to base but not preserving map."); if (--entry->second.ref_cnt == 0) m_preserved_bps.erase(entry); } diff --git a/lldb/source/Target/StopInfo.cpp b/lldb/source/Target/StopInfo.cpp index 6d5572c62294c..e9e534a57973e 100644 --- a/lldb/source/Target/StopInfo.cpp +++ b/lldb/source/Target/StopInfo.cpp @@ -94,7 +94,7 @@ class StopInfoBreakpoint : public StopInfo { : StopInfo(thread, break_id), m_should_stop(false), m_should_stop_is_valid(false), m_should_perform_action(true), m_address(LLDB_INVALID_ADDRESS), m_break_id(LLDB_INVALID_BREAK_ID), - m_was_all_internal(false), m_was_one_shot(false), + m_was_all_internal(false), m_was_one_shot(false), m_async_stopped_locs(true) { StoreBPInfo(); } @@ -103,7 +103,7 @@ class StopInfoBreakpoint : public StopInfo { : StopInfo(thread, break_id), m_should_stop(should_stop), m_should_stop_is_valid(true), m_should_perform_action(true), m_address(LLDB_INVALID_ADDRESS), m_break_id(LLDB_INVALID_BREAK_ID), - m_was_all_internal(false), m_was_one_shot(false), + m_was_all_internal(false), m_was_one_shot(false), m_async_stopped_locs(true) { StoreBPInfo(); } diff --git a/lldb/test/API/functionalities/breakpoint/callback_deletes_breakpoints/TestCallbackDeletesBreakpoints.py b/lldb/test/API/functionalities/breakpoint/callback_deletes_breakpoints/TestCallbackDeletesBreakpoints.py index e8173dc000386..2b8fc662ad42e 100644 --- a/lldb/test/API/functionalities/breakpoint/callback_deletes_breakpoints/TestCallbackDeletesBreakpoints.py +++ b/lldb/test/API/functionalities/breakpoint/callback_deletes_breakpoints/TestCallbackDeletesBreakpoints.py @@ -31,8 +31,10 @@ def delete_others_test(self): # Now set a breakpoint on "I did something" several times # bkpt_numbers = [] - for idx in range(0,5): - bkpt_numbers.append(lldbutil.run_break_set_by_source_regexp(self, "// Deletable location")) + for idx in range(0, 5): + bkpt_numbers.append( + lldbutil.run_break_set_by_source_regexp(self, "// Deletable location") + ) # And add commands to the third one to delete two others: deleter = target.FindBreakpointByID(bkpt_numbers[2]) @@ -51,11 +53,11 @@ def delete_others_test(self): self.assertEqual(len(stop_data), 10) # We should have been able to get break ID's and locations for all the # breakpoints that we originally hit, but some won't be around anymore: - for idx in range(0,5): - bkpt_id = stop_data[idx*2] + for idx in range(0, 5): + bkpt_id = stop_data[idx * 2] print(f"{idx}: {bkpt_id}") self.assertIn(bkpt_id, bkpt_numbers, "Found breakpoints are right") - loc_id = stop_data[idx*2 + 1] + loc_id = stop_data[idx * 2 + 1] self.assertEqual(loc_id, 1, "All breakpoints have one location") bkpt = target.FindBreakpointByID(bkpt_id) if bkpt_id in deleted_ids: diff --git a/lldb/test/API/functionalities/breakpoint/callback_deletes_breakpoints/main.c b/lldb/test/API/functionalities/breakpoint/callback_deletes_breakpoints/main.c index 279b359523cab..2ffb897b2f92d 100644 --- a/lldb/test/API/functionalities/breakpoint/callback_deletes_breakpoints/main.c +++ b/lldb/test/API/functionalities/breakpoint/callback_deletes_breakpoints/main.c @@ -1,14 +1,11 @@ #include -int -do_something(int input) { +int do_something(int input) { return input % 5; // Deletable location } -int -main() -{ - printf ("Set a breakpoint here.\n"); +int main() { + printf("Set a breakpoint here.\n"); do_something(100); do_something(200); return 0; From 7de870c164a6327589218d88073f5b840c977e0f Mon Sep 17 00:00:00 2001 From: Jim Ingham Date: Mon, 20 Oct 2025 13:54:27 -0700 Subject: [PATCH 4/5] Store a BreakpointSP per BreakpointLocation, rather than manually refcounting. --- .../Breakpoint/BreakpointLocationCollection.h | 7 +------ .../BreakpointLocationCollection.cpp | 19 +++++++++++-------- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/lldb/include/lldb/Breakpoint/BreakpointLocationCollection.h b/lldb/include/lldb/Breakpoint/BreakpointLocationCollection.h index 30ec340258e33..29432019fde3c 100644 --- a/lldb/include/lldb/Breakpoint/BreakpointLocationCollection.h +++ b/lldb/include/lldb/Breakpoint/BreakpointLocationCollection.h @@ -175,12 +175,7 @@ class BreakpointLocationCollection { mutable std::mutex m_collection_mutex; /// These are used if we're preserving breakpoints in this list: const bool m_preserving_bkpts = false; - struct RefCountedBPSP { - RefCountedBPSP(lldb::BreakpointSP in_bp_sp) : ref_cnt(1), bp_sp(in_bp_sp) {} - uint64_t ref_cnt; - lldb::BreakpointSP bp_sp; - }; - std::map m_preserved_bps; + std::map, lldb::BreakpointSP> m_preserved_bps; public: typedef llvm::iterator_range diff --git a/lldb/source/Breakpoint/BreakpointLocationCollection.cpp b/lldb/source/Breakpoint/BreakpointLocationCollection.cpp index ef8921e2d645d..b04156feb6224 100644 --- a/lldb/source/Breakpoint/BreakpointLocationCollection.cpp +++ b/lldb/source/Breakpoint/BreakpointLocationCollection.cpp @@ -30,13 +30,14 @@ void BreakpointLocationCollection::Add(const BreakpointLocationSP &bp_loc) { if (!old_bp_loc.get()) { m_break_loc_collection.push_back(bp_loc); if (m_preserving_bkpts) { + lldb::break_id_t bp_loc_id = bp_loc->GetID(); Breakpoint &bkpt = bp_loc->GetBreakpoint(); lldb::break_id_t bp_id = bkpt.GetID(); - auto entry = m_preserved_bps.find(bp_id); + std::pair key + = std::make_pair(bp_id, bp_loc_id); + auto entry = m_preserved_bps.find(key); if (entry == m_preserved_bps.end()) - m_preserved_bps.emplace(bp_id, RefCountedBPSP(bkpt.shared_from_this())); - else - entry->second.ref_cnt++; + m_preserved_bps.emplace(key, bkpt.shared_from_this()); } } } @@ -47,10 +48,12 @@ bool BreakpointLocationCollection::Remove(lldb::break_id_t bp_id, collection::iterator pos = GetIDPairIterator(bp_id, bp_loc_id); // Predicate if (pos != m_break_loc_collection.end()) { if (m_preserving_bkpts) { - auto entry = m_preserved_bps.find(bp_id); - assert(entry != m_preserved_bps.end() && - "Breakpoint added to base but not preserving map."); - if (--entry->second.ref_cnt == 0) + std::pair key + = std::make_pair(bp_id, bp_loc_id); + auto entry = m_preserved_bps.find(key); + if (entry == m_preserved_bps.end()) + assert(0 && "Breakpoint added to collection but not preserving map."); + else m_preserved_bps.erase(entry); } m_break_loc_collection.erase(pos); From 8f765e4bb6671dad210b5ef1f897973711b6a441 Mon Sep 17 00:00:00 2001 From: Jim Ingham Date: Mon, 20 Oct 2025 13:57:59 -0700 Subject: [PATCH 5/5] formatting --- .../lldb/Breakpoint/BreakpointLocationCollection.h | 3 ++- lldb/source/Breakpoint/BreakpointLocationCollection.cpp | 8 ++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/lldb/include/lldb/Breakpoint/BreakpointLocationCollection.h b/lldb/include/lldb/Breakpoint/BreakpointLocationCollection.h index 29432019fde3c..124cb55eaf723 100644 --- a/lldb/include/lldb/Breakpoint/BreakpointLocationCollection.h +++ b/lldb/include/lldb/Breakpoint/BreakpointLocationCollection.h @@ -175,7 +175,8 @@ class BreakpointLocationCollection { mutable std::mutex m_collection_mutex; /// These are used if we're preserving breakpoints in this list: const bool m_preserving_bkpts = false; - std::map, lldb::BreakpointSP> m_preserved_bps; + std::map, lldb::BreakpointSP> + m_preserved_bps; public: typedef llvm::iterator_range diff --git a/lldb/source/Breakpoint/BreakpointLocationCollection.cpp b/lldb/source/Breakpoint/BreakpointLocationCollection.cpp index b04156feb6224..97715836ec104 100644 --- a/lldb/source/Breakpoint/BreakpointLocationCollection.cpp +++ b/lldb/source/Breakpoint/BreakpointLocationCollection.cpp @@ -33,8 +33,8 @@ void BreakpointLocationCollection::Add(const BreakpointLocationSP &bp_loc) { lldb::break_id_t bp_loc_id = bp_loc->GetID(); Breakpoint &bkpt = bp_loc->GetBreakpoint(); lldb::break_id_t bp_id = bkpt.GetID(); - std::pair key - = std::make_pair(bp_id, bp_loc_id); + std::pair key = + std::make_pair(bp_id, bp_loc_id); auto entry = m_preserved_bps.find(key); if (entry == m_preserved_bps.end()) m_preserved_bps.emplace(key, bkpt.shared_from_this()); @@ -48,8 +48,8 @@ bool BreakpointLocationCollection::Remove(lldb::break_id_t bp_id, collection::iterator pos = GetIDPairIterator(bp_id, bp_loc_id); // Predicate if (pos != m_break_loc_collection.end()) { if (m_preserving_bkpts) { - std::pair key - = std::make_pair(bp_id, bp_loc_id); + std::pair key = + std::make_pair(bp_id, bp_loc_id); auto entry = m_preserved_bps.find(key); if (entry == m_preserved_bps.end()) assert(0 && "Breakpoint added to collection but not preserving map.");