[lldb] Fix crash in BreakpointSite::BumpHitCounts (#166876)

Fix crash in `BreakpointSite::BumpHitCounts` due to missing
synchronization. When bumping the hit count, we were correctly acquiring
the constituents mutex, but didn't protect the breakpoint location
collection. This PR fixes the issue by making the iterator returned by
the collection a `LockingAdaptedIterable`.

rdar://163760832
This commit is contained in:
Jonas Devlieghere 2025-11-10 15:35:11 -08:00 committed by GitHub
parent 46dc4b841d
commit 97d50b5c8a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 17 additions and 13 deletions

View File

@ -32,7 +32,8 @@ public:
~BreakpointLocationCollection();
BreakpointLocationCollection &operator=(const BreakpointLocationCollection &rhs);
BreakpointLocationCollection &
operator=(const BreakpointLocationCollection &rhs);
/// Add the breakpoint \a bp_loc_sp to the list.
///
@ -172,17 +173,18 @@ private:
lldb::break_id_t break_loc_id) const;
collection m_break_loc_collection;
mutable std::mutex m_collection_mutex;
mutable std::recursive_mutex m_collection_mutex;
/// These are used if we're preserving breakpoints in this list:
const bool m_preserving_bkpts = false;
std::map<std::pair<lldb::break_id_t, lldb::break_id_t>, lldb::BreakpointSP>
m_preserved_bps;
public:
typedef llvm::iterator_range<collection::const_iterator>
typedef LockingAdaptedIterable<std::recursive_mutex, collection>
BreakpointLocationCollectionIterable;
BreakpointLocationCollectionIterable BreakpointLocations() {
return BreakpointLocationCollectionIterable(m_break_loc_collection);
return BreakpointLocationCollectionIterable(m_break_loc_collection,
m_collection_mutex);
}
};
} // namespace lldb_private

View File

@ -24,7 +24,7 @@ BreakpointLocationCollection::BreakpointLocationCollection(bool preserving)
BreakpointLocationCollection::~BreakpointLocationCollection() = default;
void BreakpointLocationCollection::Add(const BreakpointLocationSP &bp_loc) {
std::lock_guard<std::mutex> guard(m_collection_mutex);
std::lock_guard<std::recursive_mutex> guard(m_collection_mutex);
BreakpointLocationSP old_bp_loc =
FindByIDPair(bp_loc->GetBreakpoint().GetID(), bp_loc->GetID());
if (!old_bp_loc.get()) {
@ -44,7 +44,7 @@ void BreakpointLocationCollection::Add(const BreakpointLocationSP &bp_loc) {
bool BreakpointLocationCollection::Remove(lldb::break_id_t bp_id,
lldb::break_id_t bp_loc_id) {
std::lock_guard<std::mutex> guard(m_collection_mutex);
std::lock_guard<std::recursive_mutex> 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) {
@ -117,7 +117,7 @@ const BreakpointLocationSP BreakpointLocationCollection::FindByIDPair(
}
BreakpointLocationSP BreakpointLocationCollection::GetByIndex(size_t i) {
std::lock_guard<std::mutex> guard(m_collection_mutex);
std::lock_guard<std::recursive_mutex> guard(m_collection_mutex);
BreakpointLocationSP stop_sp;
if (i < m_break_loc_collection.size())
stop_sp = m_break_loc_collection[i];
@ -127,7 +127,7 @@ BreakpointLocationSP BreakpointLocationCollection::GetByIndex(size_t i) {
const BreakpointLocationSP
BreakpointLocationCollection::GetByIndex(size_t i) const {
std::lock_guard<std::mutex> guard(m_collection_mutex);
std::lock_guard<std::recursive_mutex> guard(m_collection_mutex);
BreakpointLocationSP stop_sp;
if (i < m_break_loc_collection.size())
stop_sp = m_break_loc_collection[i];
@ -168,7 +168,7 @@ bool BreakpointLocationCollection::ShouldStop(
}
bool BreakpointLocationCollection::ValidForThisThread(Thread &thread) {
std::lock_guard<std::mutex> guard(m_collection_mutex);
std::lock_guard<std::recursive_mutex> guard(m_collection_mutex);
collection::iterator pos, begin = m_break_loc_collection.begin(),
end = m_break_loc_collection.end();
@ -180,7 +180,7 @@ bool BreakpointLocationCollection::ValidForThisThread(Thread &thread) {
}
bool BreakpointLocationCollection::IsInternal() const {
std::lock_guard<std::mutex> guard(m_collection_mutex);
std::lock_guard<std::recursive_mutex> guard(m_collection_mutex);
collection::const_iterator pos, begin = m_break_loc_collection.begin(),
end = m_break_loc_collection.end();
@ -197,7 +197,7 @@ bool BreakpointLocationCollection::IsInternal() const {
void BreakpointLocationCollection::GetDescription(
Stream *s, lldb::DescriptionLevel level) {
std::lock_guard<std::mutex> guard(m_collection_mutex);
std::lock_guard<std::recursive_mutex> guard(m_collection_mutex);
collection::iterator pos, begin = m_break_loc_collection.begin(),
end = m_break_loc_collection.end();
@ -212,8 +212,10 @@ BreakpointLocationCollection &BreakpointLocationCollection::operator=(
const BreakpointLocationCollection &rhs) {
if (this != &rhs) {
std::lock(m_collection_mutex, rhs.m_collection_mutex);
std::lock_guard<std::mutex> lhs_guard(m_collection_mutex, std::adopt_lock);
std::lock_guard<std::mutex> rhs_guard(rhs.m_collection_mutex, std::adopt_lock);
std::lock_guard<std::recursive_mutex> lhs_guard(m_collection_mutex,
std::adopt_lock);
std::lock_guard<std::recursive_mutex> rhs_guard(rhs.m_collection_mutex,
std::adopt_lock);
m_break_loc_collection = rhs.m_break_loc_collection;
}
return *this;