[Lldb-commits] [lldb] 9749587 - [lldb] Reset breakpoint hit count before new runs
Felipe de Azevedo Piovezan via lldb-commits
lldb-commits at lists.llvm.org
Mon Sep 19 09:56:26 PDT 2022
Author: Felipe de Azevedo Piovezan
Date: 2022-09-19T12:56:12-04:00
New Revision: 974958749827bc4fb88b67feab4bcd7536f70456
URL: https://github.com/llvm/llvm-project/commit/974958749827bc4fb88b67feab4bcd7536f70456
DIFF: https://github.com/llvm/llvm-project/commit/974958749827bc4fb88b67feab4bcd7536f70456.diff
LOG: [lldb] Reset breakpoint hit count before new runs
A common debugging pattern is to set a breakpoint that only stops after
a number of hits is recorded. The current implementation never resets
the hit count of breakpoints; as such, if a user re-`run`s their
program, the debugger will never stop on such a breakpoint again.
This behavior is arguably undesirable, as it renders such breakpoints
ineffective on all but the first run. This commit changes the
implementation of the `Will{Launch, Attach}` methods so that they reset
the _target's_ breakpoint hitcounts.
Differential Revision: https://reviews.llvm.org/D133858
Added:
lldb/test/API/functionalities/breakpoint/breakpoint_reset_upon_run/Makefile
lldb/test/API/functionalities/breakpoint/breakpoint_reset_upon_run/TestBreakpointResetUponRun.py
lldb/test/API/functionalities/breakpoint/breakpoint_reset_upon_run/main.cpp
Modified:
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/Target/Process.h
lldb/include/lldb/Target/Target.h
lldb/source/Breakpoint/Breakpoint.cpp
lldb/source/Breakpoint/BreakpointList.cpp
lldb/source/Breakpoint/BreakpointLocationList.cpp
lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.h
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
lldb/source/Target/Process.cpp
lldb/source/Target/Target.cpp
lldb/test/API/functionalities/breakpoint/address_breakpoints/TestAddressBreakpoints.py
lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
Removed:
################################################################################
diff --git a/lldb/include/lldb/Breakpoint/Breakpoint.h b/lldb/include/lldb/Breakpoint/Breakpoint.h
index 39c6d0b7f3a77..701d0823bd282 100644
--- a/lldb/include/lldb/Breakpoint/Breakpoint.h
+++ b/lldb/include/lldb/Breakpoint/Breakpoint.h
@@ -330,6 +330,9 @@ class Breakpoint : public std::enable_shared_from_this<Breakpoint>,
/// The current hit count for all locations.
uint32_t GetHitCount() const;
+ /// Resets the current hit count for all locations.
+ void ResetHitCount();
+
/// If \a one_shot is \b true, breakpoint will be deleted on first hit.
void SetOneShot(bool one_shot);
diff --git a/lldb/include/lldb/Breakpoint/BreakpointList.h b/lldb/include/lldb/Breakpoint/BreakpointList.h
index 346972ec3a1fc..a7399d385f6f0 100644
--- a/lldb/include/lldb/Breakpoint/BreakpointList.h
+++ b/lldb/include/lldb/Breakpoint/BreakpointList.h
@@ -138,6 +138,9 @@ class BreakpointList {
void ClearAllBreakpointSites();
+ /// Resets the hit count of all breakpoints.
+ void ResetHitCounts();
+
/// Sets the passed in Locker to hold the Breakpoint List mutex.
///
/// \param[in] lock
diff --git a/lldb/include/lldb/Breakpoint/BreakpointLocation.h b/lldb/include/lldb/Breakpoint/BreakpointLocation.h
index a6d1232162fce..2a4f9fc01bf32 100644
--- a/lldb/include/lldb/Breakpoint/BreakpointLocation.h
+++ b/lldb/include/lldb/Breakpoint/BreakpointLocation.h
@@ -87,6 +87,9 @@ class BreakpointLocation
/// Return the current Hit Count.
uint32_t GetHitCount() const { return m_hit_counter.GetValue(); }
+ /// Resets the current Hit Count.
+ void ResetHitCount() { m_hit_counter.Reset(); }
+
/// Return the current Ignore Count.
///
/// \return
diff --git a/lldb/include/lldb/Breakpoint/BreakpointLocationList.h b/lldb/include/lldb/Breakpoint/BreakpointLocationList.h
index 4b36c919ee3c5..f76a8fcfdd7e7 100644
--- a/lldb/include/lldb/Breakpoint/BreakpointLocationList.h
+++ b/lldb/include/lldb/Breakpoint/BreakpointLocationList.h
@@ -126,6 +126,9 @@ class BreakpointLocationList {
/// Hit count of all locations in this list.
uint32_t GetHitCount() const;
+ /// Resets 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.
///
diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h
index 4f5ae45a431ec..6975eb8029de0 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -882,13 +882,28 @@ class Process : public std::enable_shared_from_this<Process>,
// Plug-in Process Control Overrides
//==================================================================
+ /// Called before attaching to a process.
+ ///
+ /// \return
+ /// Returns an error object.
+ Status WillAttachToProcessWithID(lldb::pid_t pid);
+
/// Called before attaching to a process.
///
/// Allow Process plug-ins to execute some code before attaching a process.
///
/// \return
/// Returns an error object.
- virtual Status WillAttachToProcessWithID(lldb::pid_t pid) { return Status(); }
+ virtual Status DoWillAttachToProcessWithID(lldb::pid_t pid) {
+ return Status();
+ }
+
+ /// Called before attaching to a process.
+ ///
+ /// \return
+ /// Returns an error object.
+ Status WillAttachToProcessWithName(const char *process_name,
+ bool wait_for_launch);
/// Called before attaching to a process.
///
@@ -896,8 +911,8 @@ class Process : public std::enable_shared_from_this<Process>,
///
/// \return
/// Returns an error object.
- virtual Status WillAttachToProcessWithName(const char *process_name,
- bool wait_for_launch) {
+ virtual Status DoWillAttachToProcessWithName(const char *process_name,
+ bool wait_for_launch) {
return Status();
}
@@ -989,13 +1004,18 @@ class Process : public std::enable_shared_from_this<Process>,
/// Called after reported vfork completion.
virtual void DidVForkDone() {}
+ /// Called before launching to a process.
+ /// \return
+ /// Returns an error object.
+ Status WillLaunch(Module *module);
+
/// Called before launching to a process.
///
/// Allow Process plug-ins to execute some code before launching a process.
///
/// \return
/// Returns an error object.
- virtual Status WillLaunch(Module *module) { return Status(); }
+ virtual Status DoWillLaunch(Module *module) { return Status(); }
/// Launch a new process.
///
diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h
index 89688ad52a7df..4ba936a81b646 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -780,6 +780,9 @@ class Target : public std::enable_shared_from_this<Target>,
bool RemoveBreakpointByID(lldb::break_id_t break_id);
+ /// Resets the hit count of all breakpoints.
+ void ResetBreakpointHitCounts();
+
// The flag 'end_to_end', default to true, signifies that the operation is
// performed end to end, for both the debugger and the debuggee.
diff --git a/lldb/source/Breakpoint/Breakpoint.cpp b/lldb/source/Breakpoint/Breakpoint.cpp
index 578cf14283d92..aad01a2e6fb67 100644
--- a/lldb/source/Breakpoint/Breakpoint.cpp
+++ b/lldb/source/Breakpoint/Breakpoint.cpp
@@ -328,6 +328,11 @@ uint32_t Breakpoint::GetIgnoreCount() const {
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.IsOneShot(); }
void Breakpoint::SetOneShot(bool one_shot) { m_options.SetOneShot(one_shot); }
diff --git a/lldb/source/Breakpoint/BreakpointList.cpp b/lldb/source/Breakpoint/BreakpointList.cpp
index f4d4b2edbf08b..f7c2cdb938e62 100644
--- a/lldb/source/Breakpoint/BreakpointList.cpp
+++ b/lldb/source/Breakpoint/BreakpointList.cpp
@@ -186,6 +186,12 @@ void BreakpointList::ClearAllBreakpointSites() {
bp_sp->ClearAllBreakpointSites();
}
+void BreakpointList::ResetHitCounts() {
+ std::lock_guard<std::recursive_mutex> guard(m_mutex);
+ for (const auto &bp_sp : m_breakpoints)
+ bp_sp->ResetHitCount();
+}
+
void BreakpointList::GetListMutex(
std::unique_lock<std::recursive_mutex> &lock) {
lock = std::unique_lock<std::recursive_mutex>(m_mutex);
diff --git a/lldb/source/Breakpoint/BreakpointLocationList.cpp b/lldb/source/Breakpoint/BreakpointLocationList.cpp
index 8688f886d25f6..e0f1b9b2c8088 100644
--- a/lldb/source/Breakpoint/BreakpointLocationList.cpp
+++ b/lldb/source/Breakpoint/BreakpointLocationList.cpp
@@ -176,6 +176,12 @@ uint32_t BreakpointLocationList::GetHitCount() const {
return hit_count;
}
+void BreakpointLocationList::ResetHitCount() {
+ std::lock_guard<std::recursive_mutex> guard(m_mutex);
+ for (auto &loc : m_locations)
+ loc->ResetHitCount();
+}
+
size_t BreakpointLocationList::GetNumResolvedLocations() const {
std::lock_guard<std::recursive_mutex> guard(m_mutex);
size_t resolve_count = 0;
diff --git a/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp b/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
index feb878f039ab6..6b9be1e55d4f2 100644
--- a/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
+++ b/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
@@ -168,21 +168,21 @@ ProcessKDP::~ProcessKDP() {
Finalize();
}
-Status ProcessKDP::WillLaunch(Module *module) {
+Status ProcessKDP::DoWillLaunch(Module *module) {
Status error;
error.SetErrorString("launching not supported in kdp-remote plug-in");
return error;
}
-Status ProcessKDP::WillAttachToProcessWithID(lldb::pid_t pid) {
+Status ProcessKDP::DoWillAttachToProcessWithID(lldb::pid_t pid) {
Status error;
error.SetErrorString(
"attaching to a by process ID not supported in kdp-remote plug-in");
return error;
}
-Status ProcessKDP::WillAttachToProcessWithName(const char *process_name,
- bool wait_for_launch) {
+Status ProcessKDP::DoWillAttachToProcessWithName(const char *process_name,
+ bool wait_for_launch) {
Status error;
error.SetErrorString(
"attaching to a by process name not supported in kdp-remote plug-in");
diff --git a/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.h b/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.h
index a8cfb0ec9f870..3c12fd4074499 100644
--- a/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.h
+++ b/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.h
@@ -56,17 +56,17 @@ class ProcessKDP : public lldb_private::Process {
lldb_private::CommandObject *GetPluginCommandObject() override;
// Creating a new process, or attaching to an existing one
- lldb_private::Status WillLaunch(lldb_private::Module *module) override;
+ lldb_private::Status DoWillLaunch(lldb_private::Module *module) override;
lldb_private::Status
DoLaunch(lldb_private::Module *exe_module,
lldb_private::ProcessLaunchInfo &launch_info) override;
- lldb_private::Status WillAttachToProcessWithID(lldb::pid_t pid) override;
+ lldb_private::Status DoWillAttachToProcessWithID(lldb::pid_t pid) override;
lldb_private::Status
- WillAttachToProcessWithName(const char *process_name,
- bool wait_for_launch) override;
+ DoWillAttachToProcessWithName(const char *process_name,
+ bool wait_for_launch) override;
lldb_private::Status DoConnectRemote(llvm::StringRef remote_url) override;
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 0c83f71ad09ab..5484ce5d30ab0 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -510,16 +510,16 @@ void ProcessGDBRemote::BuildDynamicRegisterInfo(bool force) {
AddRemoteRegisters(registers, arch_to_use);
}
-Status ProcessGDBRemote::WillLaunch(lldb_private::Module *module) {
+Status ProcessGDBRemote::DoWillLaunch(lldb_private::Module *module) {
return WillLaunchOrAttach();
}
-Status ProcessGDBRemote::WillAttachToProcessWithID(lldb::pid_t pid) {
+Status ProcessGDBRemote::DoWillAttachToProcessWithID(lldb::pid_t pid) {
return WillLaunchOrAttach();
}
-Status ProcessGDBRemote::WillAttachToProcessWithName(const char *process_name,
- bool wait_for_launch) {
+Status ProcessGDBRemote::DoWillAttachToProcessWithName(const char *process_name,
+ bool wait_for_launch) {
return WillLaunchOrAttach();
}
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
index 8e2a462f61212..0f826ccab56f7 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -77,16 +77,16 @@ class ProcessGDBRemote : public Process,
CommandObject *GetPluginCommandObject() override;
// Creating a new process, or attaching to an existing one
- Status WillLaunch(Module *module) override;
+ Status DoWillLaunch(Module *module) override;
Status DoLaunch(Module *exe_module, ProcessLaunchInfo &launch_info) override;
void DidLaunch() override;
- Status WillAttachToProcessWithID(lldb::pid_t pid) override;
+ Status DoWillAttachToProcessWithID(lldb::pid_t pid) override;
- Status WillAttachToProcessWithName(const char *process_name,
- bool wait_for_launch) override;
+ Status DoWillAttachToProcessWithName(const char *process_name,
+ bool wait_for_launch) override;
Status DoConnectRemote(llvm::StringRef remote_url) override;
diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index 31e15d42a2e3f..1a9befdfcb0ce 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -2760,6 +2760,22 @@ ListenerSP ProcessAttachInfo::GetListenerForProcess(Debugger &debugger) {
return debugger.GetListener();
}
+Status Process::WillLaunch(Module *module) {
+ GetTarget().ResetBreakpointHitCounts();
+ return DoWillLaunch(module);
+}
+
+Status Process::WillAttachToProcessWithID(lldb::pid_t pid) {
+ GetTarget().ResetBreakpointHitCounts();
+ return DoWillAttachToProcessWithID(pid);
+}
+
+Status Process::WillAttachToProcessWithName(const char *process_name,
+ bool wait_for_launch) {
+ GetTarget().ResetBreakpointHitCounts();
+ return DoWillAttachToProcessWithName(process_name, wait_for_launch);
+}
+
Status Process::Attach(ProcessAttachInfo &attach_info) {
m_abi_sp.reset();
m_process_input_reader.reset();
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index f28696ffd95ba..7ff2789880611 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -1003,6 +1003,10 @@ bool Target::EnableBreakpointByID(break_id_t break_id) {
return false;
}
+void Target::ResetBreakpointHitCounts() {
+ GetBreakpointList().ResetHitCounts();
+}
+
Status Target::SerializeBreakpointsToFile(const FileSpec &file,
const BreakpointIDList &bp_ids,
bool append) {
diff --git a/lldb/test/API/functionalities/breakpoint/address_breakpoints/TestAddressBreakpoints.py b/lldb/test/API/functionalities/breakpoint/address_breakpoints/TestAddressBreakpoints.py
index e2369be9ff94b..20934f680d5d8 100644
--- a/lldb/test/API/functionalities/breakpoint/address_breakpoints/TestAddressBreakpoints.py
+++ b/lldb/test/API/functionalities/breakpoint/address_breakpoints/TestAddressBreakpoints.py
@@ -82,5 +82,6 @@ def address_breakpoints(self):
len(threads), 1,
"There should be a thread stopped at our breakpoint")
- # The hit count for the breakpoint should now be 2.
- self.assertEquals(breakpoint.GetHitCount(), 2)
+ # The hit count for the breakpoint should be 1, since we reset counts
+ # for each run.
+ self.assertEquals(breakpoint.GetHitCount(), 1)
diff --git a/lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py b/lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
index 6212ba6c98646..af5441c10f79a 100644
--- a/lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
+++ b/lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
@@ -313,8 +313,9 @@ def breakpoint_command_sequence(self):
substrs=['stopped',
'stop reason = breakpoint'])
- # The breakpoint should have a hit count of 2.
- lldbutil.check_breakpoint(self, bpno = 1, expected_hit_count = 2)
+ # The breakpoint should have a hit count of 1, since we reset counts
+ # for each run.
+ lldbutil.check_breakpoint(self, bpno = 1, expected_hit_count = 1)
def breakpoint_command_script_parameters(self):
"""Test that the frame and breakpoint location are being properly passed to the script breakpoint command function."""
diff --git a/lldb/test/API/functionalities/breakpoint/breakpoint_reset_upon_run/Makefile b/lldb/test/API/functionalities/breakpoint/breakpoint_reset_upon_run/Makefile
new file mode 100644
index 0000000000000..99998b20bcb05
--- /dev/null
+++ b/lldb/test/API/functionalities/breakpoint/breakpoint_reset_upon_run/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
diff --git a/lldb/test/API/functionalities/breakpoint/breakpoint_reset_upon_run/TestBreakpointResetUponRun.py b/lldb/test/API/functionalities/breakpoint/breakpoint_reset_upon_run/TestBreakpointResetUponRun.py
new file mode 100644
index 0000000000000..0811c0773aeab
--- /dev/null
+++ b/lldb/test/API/functionalities/breakpoint/breakpoint_reset_upon_run/TestBreakpointResetUponRun.py
@@ -0,0 +1,58 @@
+"""
+Test breakpoint hit count is reset when target runs.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+
+
+class HitcountResetUponRun(TestBase):
+ BREAKPOINT_TEXT = 'Set a breakpoint here'
+
+ def check_stopped_at_breakpoint_and_hit_once(self, thread, breakpoint):
+ frame0 = thread.GetFrameAtIndex(0)
+ location1 = breakpoint.FindLocationByAddress(frame0.GetPC())
+ self.assertTrue(location1)
+ self.assertEqual(location1.GetHitCount(), 1)
+ self.assertEqual(breakpoint.GetHitCount(), 1)
+
+ def test_hitcount_reset_upon_run(self):
+ self.build()
+
+ exe = self.getBuildArtifact("a.out")
+
+ target = self.dbg.CreateTarget(exe)
+ self.assertTrue(target, VALID_TARGET)
+
+ breakpoint = target.BreakpointCreateBySourceRegex(
+ self.BREAKPOINT_TEXT, lldb.SBFileSpec('main.cpp'))
+ self.assertTrue(
+ breakpoint.IsValid() and breakpoint.GetNumLocations() == 1,
+ VALID_BREAKPOINT)
+
+ process = target.LaunchSimple(
+ None, None, self.get_process_working_directory())
+ self.assertTrue(process, PROCESS_IS_VALID)
+
+ from lldbsuite.test.lldbutil import get_stopped_thread
+
+ # Verify 1st breakpoint location is hit.
+ thread = get_stopped_thread(process, lldb.eStopReasonBreakpoint)
+ self.assertTrue(
+ thread.IsValid(),
+ "There should be a thread stopped due to breakpoint")
+ self.check_stopped_at_breakpoint_and_hit_once(thread, breakpoint)
+
+ # Relaunch
+ process.Kill()
+ process = target.LaunchSimple(
+ None, None, self.get_process_working_directory())
+ self.assertTrue(process, PROCESS_IS_VALID)
+
+ # Verify the hit counts are still one.
+ thread = get_stopped_thread(process, lldb.eStopReasonBreakpoint)
+ self.assertTrue(
+ thread.IsValid(),
+ "There should be a thread stopped due to breakpoint")
+ self.check_stopped_at_breakpoint_and_hit_once(thread, breakpoint)
diff --git a/lldb/test/API/functionalities/breakpoint/breakpoint_reset_upon_run/main.cpp b/lldb/test/API/functionalities/breakpoint/breakpoint_reset_upon_run/main.cpp
new file mode 100644
index 0000000000000..05a741cb05ee5
--- /dev/null
+++ b/lldb/test/API/functionalities/breakpoint/breakpoint_reset_upon_run/main.cpp
@@ -0,0 +1,6 @@
+#include <stdio.h>
+
+int main(int argc, char const *argv[]) {
+ printf("Set a breakpoint here.\n");
+ return 0;
+}
More information about the lldb-commits
mailing list