[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

lldb/include/lldb/Breakpoint/Breakpoint.h

@@ -330,6 +330,9 @@ public:
   ///     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);
 

lldb/include/lldb/Breakpoint/BreakpointList.h

@@ -138,6 +138,9 @@ public:
 
   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

lldb/include/lldb/Breakpoint/BreakpointLocation.h

@@ -87,6 +87,9 @@ public:
   /// 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

lldb/include/lldb/Breakpoint/BreakpointLocationList.h

@@ -126,6 +126,9 @@ public:
   ///     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.
   ///

lldb/include/lldb/Target/Process.h

@@ -884,11 +884,9 @@ public:
 
   /// 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(); }
+  Status WillAttachToProcessWithID(lldb::pid_t pid);
 
   /// Called before attaching to a process.
   ///
@@ -896,8 +894,25 @@ public:
   ///
   /// \return
   ///     Returns an error object.
-  virtual Status WillAttachToProcessWithName(const char *process_name,
-                                             bool wait_for_launch) {
+  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.
+  ///
+  /// Allow Process plug-ins to execute some code before attaching a process.
+  ///
+  /// \return
+  ///     Returns an error object.
+  virtual Status DoWillAttachToProcessWithName(const char *process_name,
+                                               bool wait_for_launch) {
     return Status();
   }
 
@@ -989,13 +1004,18 @@ public:
   /// 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.
   ///

lldb/include/lldb/Target/Target.h

@@ -780,6 +780,9 @@ public:
 
   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.
 

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); }

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);

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;

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");

lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.h

@@ -56,17 +56,17 @@ public:
   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;
 

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();
 }
 

lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h

@@ -77,16 +77,16 @@ public:
   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;
 

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();

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) {

lldb/test/API/functionalities/breakpoint/address_breakpoints/TestAddressBreakpoints.py

@@ -82,5 +82,6 @@ class AddressBreakpointTestCase(TestBase):
             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)

lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py

@@ -313,8 +313,9 @@ class BreakpointCommandTestCase(TestBase):
                     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."""

lldb/test/API/functionalities/breakpoint/breakpoint_reset_upon_run/Makefile

@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules

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)

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;
+}