tatyana-krasnukha created this revision.
tatyana-krasnukha added a reviewer: jingham.
tatyana-krasnukha added a project: LLDB.
Herald added subscribers: lldb-commits, JDevlieghere.
tatyana-krasnukha requested review of this revision.
Target clears watchpoints hit count but doesn't do the same for breakpoints. As
was discussed in D84527 <https://reviews.llvm.org/D84527>, this behavior should
be unified.
Now responsibility for cleanup lies on *List symmetrically for breakpoints and
watchpoints.
The test case verifies that hit counts are reset to 0 after killing the process.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D84954
Files:
lldb/include/lldb/Breakpoint/Breakpoint.h
lldb/include/lldb/Breakpoint/BreakpointList.h
lldb/include/lldb/Breakpoint/BreakpointLocation.h
lldb/include/lldb/Breakpoint/BreakpointLocationList.h
lldb/include/lldb/Breakpoint/WatchpointList.h
lldb/include/lldb/Target/Target.h
lldb/source/Breakpoint/Breakpoint.cpp
lldb/source/Breakpoint/BreakpointList.cpp
lldb/source/Breakpoint/BreakpointLocationList.cpp
lldb/source/Breakpoint/WatchpointList.cpp
lldb/source/Target/Target.cpp
lldb/test/API/functionalities/breakpoint/breakpoint_hit_count/TestBreakpointHitCount.py
Index: lldb/test/API/functionalities/breakpoint/breakpoint_hit_count/TestBreakpointHitCount.py
===================================================================
--- lldb/test/API/functionalities/breakpoint/breakpoint_hit_count/TestBreakpointHitCount.py
+++ lldb/test/API/functionalities/breakpoint/breakpoint_hit_count/TestBreakpointHitCount.py
@@ -19,15 +19,32 @@
def test_breakpoint_location_hit_count(self):
"""Use Python APIs to check breakpoint hit count."""
self.build()
- self.do_test_breakpoint_location_hit_count()
+ target = self.create_target()
+ breakpoint = self.create_breakpoint(target)
+ process = self.launch_process(target)
+ self.do_test_breakpoint_location_hit_count(process, breakpoint)
+
+ def test_breakpoint_hit_count_cleanup(self):
+ """Use Python APIs to check that breakpoint hit count is cleared
+ as well as locations hit counts when process dies."""
+ self.build()
+ target = self.create_target()
+ breakpoint = self.create_breakpoint(target)
+ process = self.launch_process(target)
+ self.do_test_breakpoint_location_hit_count(process, breakpoint)
+
+ # Kill the process and launch a new one.
+ status = process.Kill()
+ self.assertTrue(status, "The process was killed.")
+ new_process = self.launch_process(target)
+
+ # Test hit location of the existing breakpoint. It should be like we didn't have hits before.
+ self.do_test_breakpoint_location_hit_count(new_process, breakpoint)
def test_breakpoint_one_shot(self):
"""Check that one-shot breakpoints trigger only once."""
self.build()
-
- exe = self.getBuildArtifact("a.out")
- target = self.dbg.CreateTarget(exe)
- self.assertTrue(target, VALID_TARGET)
+ target = self.create_target()
self.runCmd("tb a")
process = target.LaunchSimple(
@@ -54,13 +71,14 @@
self.a_float_body_line_no = line_number(
'main.cpp', '// Breakpoint Location 2')
- def do_test_breakpoint_location_hit_count(self):
- """Use Python APIs to check breakpoint hit count."""
+ def create_target(self):
exe = self.getBuildArtifact("a.out")
target = self.dbg.CreateTarget(exe)
self.assertTrue(target, VALID_TARGET)
+ return target
+ def create_breakpoint(self, target):
# Create a breakpoint in main.cpp by name 'a',
# there should be two locations.
breakpoint = target.BreakpointCreateByName('a', 'a.out')
@@ -79,11 +97,19 @@
location2.IsEnabled(),
VALID_BREAKPOINT_LOCATION)
+ return breakpoint
+
+ def launch_process(self, target):
# Launch the process, and do not stop at entry point.
process = target.LaunchSimple(
None, None, self.get_process_working_directory())
self.assertTrue(process, PROCESS_IS_VALID)
+ return process
+
+ def do_test_breakpoint_location_hit_count(self, process, breakpoint):
+ """Use Python APIs to check breakpoint hit count."""
+
# Verify 1st breakpoint location is hit.
from lldbsuite.test.lldbutil import get_stopped_thread
thread = get_stopped_thread(process, lldb.eStopReasonBreakpoint)
@@ -129,5 +155,3 @@
self.assertEqual(location2.GetHitCount(), 2)
self.assertEqual(location1.GetHitCount(), 1)
self.assertEqual(breakpoint.GetHitCount(), 3)
-
- process.Continue()
Index: lldb/source/Target/Target.cpp
===================================================================
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -174,14 +174,11 @@
// Do any cleanup of the target we need to do between process instances.
// NB It is better to do this before destroying the process in case the
// clean up needs some help from the process.
- m_breakpoint_list.ClearAllBreakpointSites();
- m_internal_breakpoint_list.ClearAllBreakpointSites();
+ m_breakpoint_list.CleanupBreakpoints();
+ m_internal_breakpoint_list.CleanupBreakpoints();
// Disable watchpoints just on the debugger side.
- std::unique_lock<std::recursive_mutex> lock;
- this->GetWatchpointList().GetListMutex(lock);
- DisableAllWatchpoints(false);
- ClearAllWatchpointHitCounts();
- ClearAllWatchpointHistoricValues();
+ m_watchpoint_list.SetEnabledAll(false);
+ m_watchpoint_list.CleanupWatchpoints();
}
void Target::DeleteCurrentProcess() {
@@ -1222,38 +1219,6 @@
return true; // Success!
}
-// Assumption: Caller holds the list mutex lock for m_watchpoint_list.
-bool Target::ClearAllWatchpointHitCounts() {
- Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_WATCHPOINTS));
- LLDB_LOGF(log, "Target::%s\n", __FUNCTION__);
-
- size_t num_watchpoints = m_watchpoint_list.GetSize();
- for (size_t i = 0; i < num_watchpoints; ++i) {
- WatchpointSP wp_sp = m_watchpoint_list.GetByIndex(i);
- if (!wp_sp)
- return false;
-
- wp_sp->ResetHitCount();
- }
- return true; // Success!
-}
-
-// Assumption: Caller holds the list mutex lock for m_watchpoint_list.
-bool Target::ClearAllWatchpointHistoricValues() {
- Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_WATCHPOINTS));
- LLDB_LOGF(log, "Target::%s\n", __FUNCTION__);
-
- size_t num_watchpoints = m_watchpoint_list.GetSize();
- for (size_t i = 0; i < num_watchpoints; ++i) {
- WatchpointSP wp_sp = m_watchpoint_list.GetByIndex(i);
- if (!wp_sp)
- return false;
-
- wp_sp->ResetHistoricValues();
- }
- return true; // Success!
-}
-
// Assumption: Caller holds the list mutex lock for m_watchpoint_list during
// these operations.
bool Target::IgnoreAllWatchpoints(uint32_t ignore_count) {
Index: lldb/source/Breakpoint/WatchpointList.cpp
===================================================================
--- lldb/source/Breakpoint/WatchpointList.cpp
+++ lldb/source/Breakpoint/WatchpointList.cpp
@@ -246,6 +246,18 @@
m_watchpoints.clear();
}
+void WatchpointList::CleanupWatchpoints() {
+ std::lock_guard<std::recursive_mutex> guard(m_mutex);
+
+ for (const auto &wp_sp : m_watchpoints) {
+ if (!wp_sp)
+ return;
+
+ wp_sp->ResetHitCount();
+ wp_sp->ResetHistoricValues();
+ }
+}
+
void WatchpointList::GetListMutex(
std::unique_lock<std::recursive_mutex> &lock) {
lock = std::unique_lock<std::recursive_mutex>(m_mutex);
Index: lldb/source/Breakpoint/BreakpointLocationList.cpp
===================================================================
--- lldb/source/Breakpoint/BreakpointLocationList.cpp
+++ lldb/source/Breakpoint/BreakpointLocationList.cpp
@@ -168,13 +168,11 @@
}
}
-uint32_t BreakpointLocationList::GetHitCount() const {
- uint32_t hit_count = 0;
+void BreakpointLocationList::ResetHitCount() {
std::lock_guard<std::recursive_mutex> guard(m_mutex);
collection::const_iterator pos, end = m_locations.end();
- for (pos = m_locations.begin(); pos != end; ++pos)
- hit_count += (*pos)->GetHitCount();
- return hit_count;
+ for (auto &location : m_locations)
+ location->ResetHitCount();
}
size_t BreakpointLocationList::GetNumResolvedLocations() const {
Index: lldb/source/Breakpoint/BreakpointList.cpp
===================================================================
--- lldb/source/Breakpoint/BreakpointList.cpp
+++ lldb/source/Breakpoint/BreakpointList.cpp
@@ -81,10 +81,10 @@
void BreakpointList::RemoveAll(bool notify) {
std::lock_guard<std::recursive_mutex> guard(m_mutex);
- ClearAllBreakpointSites();
- if (notify) {
- for (const auto &bp_sp : m_breakpoints)
+ for (const auto &bp_sp : m_breakpoints) {
+ bp_sp->ClearAllBreakpointSites();
+ if (notify)
NotifyChange(bp_sp, eBreakpointEventTypeRemoved);
}
@@ -183,10 +183,12 @@
bp_sp->ModuleReplaced(old_module_sp, new_module_sp);
}
-void BreakpointList::ClearAllBreakpointSites() {
+void BreakpointList::CleanupBreakpoints() {
std::lock_guard<std::recursive_mutex> guard(m_mutex);
- for (const auto &bp_sp : m_breakpoints)
+ for (const auto &bp_sp : m_breakpoints) {
bp_sp->ClearAllBreakpointSites();
+ bp_sp->ResetHitCount();
+ }
}
void BreakpointList::GetListMutex(
Index: lldb/source/Breakpoint/Breakpoint.cpp
===================================================================
--- lldb/source/Breakpoint/Breakpoint.cpp
+++ lldb/source/Breakpoint/Breakpoint.cpp
@@ -343,6 +343,11 @@
uint32_t Breakpoint::GetHitCount() const { return m_hit_counter.GetValue(); }
+void Breakpoint::ResetHitCount() {
+ m_hit_counter.Reset();
+ m_locations.ResetHitCount();
+}
+
bool Breakpoint::IsOneShot() const { return m_options_up->IsOneShot(); }
void Breakpoint::SetOneShot(bool one_shot) {
Index: lldb/include/lldb/Target/Target.h
===================================================================
--- lldb/include/lldb/Target/Target.h
+++ lldb/include/lldb/Target/Target.h
@@ -747,10 +747,6 @@
bool EnableAllWatchpoints(bool end_to_end = true);
- bool ClearAllWatchpointHitCounts();
-
- bool ClearAllWatchpointHistoricValues();
-
bool IgnoreAllWatchpoints(uint32_t ignore_count);
bool DisableWatchpointByID(lldb::watch_id_t watch_id);
Index: lldb/include/lldb/Breakpoint/WatchpointList.h
===================================================================
--- lldb/include/lldb/Breakpoint/WatchpointList.h
+++ lldb/include/lldb/Breakpoint/WatchpointList.h
@@ -178,6 +178,9 @@
void RemoveAll(bool notify);
+ /// Clears all watchpoints hit counts and historic values.
+ void WatchpointList::CleanupWatchpoints();
+
/// Sets the passed in Locker to hold the Watchpoint List mutex.
///
/// \param[in] lock
Index: lldb/include/lldb/Breakpoint/BreakpointLocationList.h
===================================================================
--- lldb/include/lldb/Breakpoint/BreakpointLocationList.h
+++ lldb/include/lldb/Breakpoint/BreakpointLocationList.h
@@ -120,11 +120,8 @@
/// Number of qualifying breakpoint locations.
size_t GetNumResolvedLocations() const;
- /// Returns the number hit count of all locations in this list.
- ///
- /// \result
- /// Hit count of all locations in this list.
- uint32_t GetHitCount() const;
+ /// Clears the hit count of all locations in this list.
+ void ResetHitCount();
/// Enquires of the breakpoint location in this list with ID \a breakID
/// whether we should stop.
Index: lldb/include/lldb/Breakpoint/BreakpointLocation.h
===================================================================
--- lldb/include/lldb/Breakpoint/BreakpointLocation.h
+++ lldb/include/lldb/Breakpoint/BreakpointLocation.h
@@ -87,6 +87,9 @@
/// Return the current Hit Count.
uint32_t GetHitCount() const { return m_hit_counter.GetValue(); }
+ /// Reset the current Hit Count.
+ void ResetHitCount() { return m_hit_counter.Reset(); }
+
/// Return the current Ignore Count.
///
/// \return
Index: lldb/include/lldb/Breakpoint/BreakpointList.h
===================================================================
--- lldb/include/lldb/Breakpoint/BreakpointList.h
+++ lldb/include/lldb/Breakpoint/BreakpointList.h
@@ -136,7 +136,8 @@
void UpdateBreakpointsWhenModuleIsReplaced(lldb::ModuleSP old_module_sp,
lldb::ModuleSP new_module_sp);
- void ClearAllBreakpointSites();
+ /// Clears BreakpointSites and resets breakpoint hit counts.
+ void CleanupBreakpoints();
/// Sets the passed in Locker to hold the Breakpoint List mutex.
///
Index: lldb/include/lldb/Breakpoint/Breakpoint.h
===================================================================
--- lldb/include/lldb/Breakpoint/Breakpoint.h
+++ lldb/include/lldb/Breakpoint/Breakpoint.h
@@ -321,10 +321,14 @@
/// The number of breakpoint hits to be ignored.
uint32_t GetIgnoreCount() const;
- /// Return the current hit count for all locations. \return
+ /// Return the current hit count.
+ /// \return
/// The current hit count for all locations.
uint32_t GetHitCount() const;
+ /// Reset the current hit count for itself and for all locations.
+ void ResetHitCount();
+
/// If \a one_shot is \b true, breakpoint will be deleted on first hit.
void SetOneShot(bool one_shot);
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
