[lldb] Reset breakpoint hit count before new runs

[fdeazeve]

Summary

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-runs 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.

[fdeazeve]

lldb/source/Breakpoint/Breakpoint.cpp

332

By the way, I don' think this counter is ever incremented anywhere. I'll removing it in a separate patch and see what happens

[fdeazeve]

lldb/source/Breakpoint/Breakpoint.cpp

332

Ah, nvm, Breakpoint is a friend of BreakpointLocation, and the latter increments this counter

[JDevlieghere]

lldb/source/Target/Process.cpp

2763–2766

Can this be a private member so we don't have to pass in *this?

[fdeazeve]

lldb/source/Target/Process.cpp

2763–2766

No strong preferences on my part, but I had made it a free function because it can be implemented in terms of the public behaviour, i.e. it doesn't rely on implementation details.

[jingham]

Resetting the hit counts on rerun is a more useful behavior, so this is all fine. But the Target is the one that does all the breakpoint management, so I think the resetting should go through the Target, not the Process. And we generally delegate "do on all breakpoints" operations to the BreakpointList, so it would be more consistent with the current structure to go Process::WillLaunch -> Target::ResetHitCounts -> BreakpointList::ResetHitCounts.

lldb/source/Target/Process.cpp

2763–2766

Resetting all the hit counts in a BreakpointList seems to me a job of the BreakpointList. Also, while DoWillLaunch/Attach is where this action belongs, which is how the Process gets involved, the Target is the one that manages the breakpoints in general. So I think it would be better to have the Target do ResetHitCounts, and these Process calls should just dial up it's Target.

[fdeazeve]

Addressed review comments

[fdeazeve]

In D133858#3790013, @jingham wrote:

Resetting the hit counts on rerun is a more useful behavior, so this is all fine. But the Target is the one that does all the breakpoint management, so I think the resetting should go through the Target, not the Process. And we generally delegate "do on all breakpoints" operations to the BreakpointList, so it would be more consistent with the current structure to go Process::WillLaunch -> Target::ResetHitCounts -> BreakpointList::ResetHitCounts.

This makes sense! I've fixed this in the latest revision

[fdeazeve]

lldb/source/Breakpoint/Breakpoint.cpp

334

@jingham actually, should I apply the same idea to BreakpointLocationList?

[fdeazeve]

Also applied Jim's comment to the LocationList class, as this involves a single
lock, instead of one lock per location.
If this is wrong, please let me know and I'll revert.

[mib]

In D133858#3790013, @jingham wrote:

Resetting the hit counts on rerun is a more useful behavior, so this is all fine. But the Target is the one that does all the breakpoint management, so I think the resetting should go through the Target, not the Process. And we generally delegate "do on all breakpoints" operations to the BreakpointList, so it would be more consistent with the current structure to go Process::WillLaunch -> Target::ResetHitCounts -> BreakpointList::ResetHitCounts.

@fdeazeve Pretty cool! But I agree with Jim, breakpoints are handled by the target not the process. As a side-note, in the current implementation, I don't think it's necessary to add the DoWillAttachToProcessWithID method and override it in each process plugin. I think you could have just reset the hit_counter in the top-level Process::{Attach,Launch} class.

[fdeazeve]

In D133858#3791290, @mib wrote:

I don't think it's necessary to add the DoWillAttachToProcessWithID method and override it in each process plugin. I think you could have just reset the hit_counter in the top-level Process::{Attach,Launch} class.

@jingham Thoughts on this?

[jingham]

The more we gather up the "things you have to do before an operation starts" or after it ends, etc. into WillDoOperation methods, the less ad hoc code you have in the callers that you have to remember to copy over if you are introducing a different way of doing the same operation. It also expresses when the operation needs to be done, which if you just had a call out in Process::Attach or whatever you would not know. Either way will work, but I think packaging these "do before" and "do after" tasks into methods makes everything clearer and easier to maintain.

[jingham]

This version looks good to me.

[labath]

I am afraid that this patch misses one method of initiating a debug session -- connecting to a running debug server (process connect, SBTarget::ConnectRemote). The breakpoint hit counts don't get reset in case of a reconnect. This isn't a particularly common use case (and the only reason I've noticed it is that for PlatformQemuUser, all "launches" are actually "connects" under the hood), but I've verified that this problem can be reproduced by issuing connect commands manually (on the regular host platform). I'm pretty sure that was not intentional.

Fixing this by adding another callout to ResetBreakpointHitCounts would be easy enough, but I'm also thinking if there isn't a better place from which to call this function, one that would capture all three scenarios in a single statement. I think that one such place could be Target::CreateProcess. This function is called by all three code paths, and it's a very good indicator that we will be starting a new debug session.

What do you think?

[fdeazeve]

In D133858#3805630, @labath wrote:

I am afraid that this patch misses one method of initiating a debug session -- connecting to a running debug server (process connect, SBTarget::ConnectRemote). The breakpoint hit counts don't get reset in case of a reconnect. This isn't a particularly common use case (and the only reason I've noticed it is that for PlatformQemuUser, all "launches" are actually "connects" under the hood), but I've verified that this problem can be reproduced by issuing connect commands manually (on the regular host platform). I'm pretty sure that was not intentional.

Fixing this by adding another callout to ResetBreakpointHitCounts would be easy enough, but I'm also thinking if there isn't a better place from which to call this function, one that would capture all three scenarios in a single statement. I think that one such place could be Target::CreateProcess. This function is called by all three code paths, and it's a very good indicator that we will be starting a new debug session.

What do you think?

My understanding is that there is an obligation of calling the WillXX methods before calling XX, so by placing the reset code in the WillXX functions we can rely on that guarantee. Right now, the same is implicit for "one must call Target::CreateProcess before launching or attaching". We could instead define a WillConnect and have the usual contract for that.

The code is fairly new to me, so I'm not confident enough to make a judgement call here. Thoughts?

[labath]

In D133858#3816105, @fdeazeve wrote:

In D133858#3805630, @labath wrote:

I am afraid that this patch misses one method of initiating a debug session -- connecting to a running debug server (process connect, SBTarget::ConnectRemote). The breakpoint hit counts don't get reset in case of a reconnect. This isn't a particularly common use case (and the only reason I've noticed it is that for PlatformQemuUser, all "launches" are actually "connects" under the hood), but I've verified that this problem can be reproduced by issuing connect commands manually (on the regular host platform). I'm pretty sure that was not intentional.

Fixing this by adding another callout to ResetBreakpointHitCounts would be easy enough, but I'm also thinking if there isn't a better place from which to call this function, one that would capture all three scenarios in a single statement. I think that one such place could be Target::CreateProcess. This function is called by all three code paths, and it's a very good indicator that we will be starting a new debug session.

What do you think?

My understanding is that there is an obligation of calling the WillXX methods before calling XX, so by placing the reset code in the WillXX functions we can rely on that guarantee. Right now, the same is implicit for "one must call Target::CreateProcess before launching or attaching". We could instead define a WillConnect and have the usual contract for that.

I wouldn't really call it a "usual" contract, but yes, I'm sure this could be fixed by adding a WillConnect method. It might be also sufficient to just call WillAttach from at some appropriate place, since a "connect" operation looks very similar to an "attach", and a lot of the initialization operations are the same. I think we're already something like this somewhere (maybe for DidAttach?). However, that still leaves us with three (or two) places that need to be kept in sync.

The code is fairly new to me, so I'm not confident enough to make a judgement call here. Thoughts?

I don't see any advantage in doing this particular action "just before" a launch, as opposed to doing in on process creation, so I would definitely do it that way.

I also find it weird to be going through the Process class just to call another Target method, when all of the launch/attach/connect operations already go through the Target class, and so it should be perfectly capable of calling that method on its own.

[jingham]

If we use Target::CreateProcess to handle this task, how do we ensure that that will always get called any time we make a new process? That function doesn't do all that much special, it's only a couple lines long so it could easily get inlined somewhere.

[labath]

In D133858#3818755, @jingham wrote:

If we use Target::CreateProcess to handle this task, how do we ensure that that will always get called any time we make a new process?

I'm not really sure how to answer that.

Clearly something like that can happen, but I think the overall risk is low. The most likely scenario for that happening would be if someone adds a new, fourth, method of initiating a process (in addition to launching, attaching and connecting), but I just don't know what would that be. But if one is doing that, I think it's at least as likely that he will forget to create the DoWillInitiate method and call the breakpoint reset function from there.

I think the only way to ensure that can't happen is to store the actual hit counts within the Process class itself, but I don't think anyone has an apetite to design something like that.

Anyone trying to create a function by bypassing a function called CreateProcess should think really hard before proceeding. And the more stuff we put into that function, the harder it will be for someone to bypass it.

lldb/include/lldb/Breakpoint/Breakpoint.h

public:
   /// \return
   ///     The number of breakpoint hits to be ignored.
   uint32_t GetIgnoreCount() const;
 
   /// Return the current hit count for all locations. \return
   ///     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);
 
   /// Check the OneShot state.
   /// \return
   ///     \b true if the breakpoint is one shot, \b false otherwise.
   bool IsOneShot() const;
 

lldb/include/lldb/Breakpoint/BreakpointList.h

public:
   void UpdateBreakpoints(ModuleList &module_list, bool load,
                          bool delete_locations);
 
   void UpdateBreakpointsWhenModuleIsReplaced(lldb::ModuleSP old_module_sp,
                                              lldb::ModuleSP new_module_sp);
 
   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
   ///   The locker object that is set.
   void GetListMutex(std::unique_lock<std::recursive_mutex> &lock);
 
 protected:
   typedef std::vector<lldb::BreakpointSP> bp_collection;

lldb/include/lldb/Breakpoint/BreakpointLocation.h

public:
   ///
   /// \return
   ///     \b true if the breakpoint is set to auto-continue, \b false if not.
   bool IsAutoContinue() const;
 
   /// 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
   ///     The number of breakpoint hits to be ignored.
   uint32_t GetIgnoreCount() const;
 
   /// Set the breakpoint to ignore the next \a count breakpoint hits.
   ///

lldb/include/lldb/Breakpoint/BreakpointLocationList.h

public:
   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;
 
+  /// 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.
   ///
   /// \param[in] context
   ///     This contains the information about this stop.
   ///
   /// \param[in] breakID
   ///     This break ID that we hit.

lldb/include/lldb/Target/Process.h

/// \see Process::Notifications
   const lldb::UnixSignalsSP &GetUnixSignals();
 
   //==================================================================
   // 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.
   ///
   /// Allow Process plug-ins to execute some code before attaching a process.
   ///
   /// \return
   ///     Returns an error object.
-  virtual Status WillAttachToProcessWithName(const char *process_name,
+  virtual Status DoWillAttachToProcessWithName(const char *process_name,
                                                bool wait_for_launch) {
     return Status();
   }
 
   /// Attach to a remote system via a URL
   ///
   /// \param[in] remote_url
   ///     The URL format that we are connecting to.
   ///
/// \see Process::Notifications
 
   /// Called after a reported vfork.
   virtual void DidVFork(lldb::pid_t child_pid, lldb::tid_t child_tid) {}
 
   /// 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.
   ///
   /// Launch a new process by spawning a new process using \a exe_module's
   /// file as the file to launch. Launch details are provided in \a
   /// launch_info.
   ///
   /// \param[in] exe_module

lldb/include/lldb/Target/Target.h

public:
   void EnableAllowedBreakpoints();
 
   bool DisableBreakpointByID(lldb::break_id_t break_id);
 
   bool EnableBreakpointByID(lldb::break_id_t break_id);
 
   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.
 
   bool RemoveAllWatchpoints(bool end_to_end = true);
 
   bool DisableAllWatchpoints(bool end_to_end = true);
 
   bool EnableAllWatchpoints(bool end_to_end = true);

lldb/source/Breakpoint/Breakpoint.cpp

 }
 
 uint32_t Breakpoint::GetIgnoreCount() const {
   return m_options.GetIgnoreCount();
 }
 
 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); }
 
 bool Breakpoint::IsAutoContinue() const { return m_options.IsAutoContinue(); }
 
 void Breakpoint::SetAutoContinue(bool auto_continue) {
   m_options.SetAutoContinue(auto_continue);

lldb/source/Breakpoint/BreakpointList.cpp

 }
 
 void BreakpointList::ClearAllBreakpointSites() {
   std::lock_guard<std::recursive_mutex> guard(m_mutex);
   for (const auto &bp_sp : m_breakpoints)
     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

uint32_t BreakpointLocationList::GetHitCount() const {
   uint32_t hit_count = 0;
   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;
 }
 
+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;
   collection::const_iterator pos, end = m_locations.end();
   for (pos = m_locations.begin(); pos != end; ++pos) {
     if ((*pos)->IsResolved())
       ++resolve_count;
   }

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

public:
   ~ProcessKDP() override;
 
   // Check if a given Process
   bool CanDebug(lldb::TargetSP target_sp,
                 bool plugin_specified_by_name) override;
   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,
+  DoWillAttachToProcessWithName(const char *process_name,
                                 bool wait_for_launch) override;
 
   lldb_private::Status DoConnectRemote(llvm::StringRef remote_url) override;
 
   lldb_private::Status DoAttachToProcessWithID(
       lldb::pid_t pid,
       const lldb_private::ProcessAttachInfo &attach_info) override;
 
   lldb_private::Status DoAttachToProcessWithName(

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

ProcessKDP::~ProcessKDP() {
   Clear();
   // We need to call finalize on the process before destroying ourselves to
   // make sure all of the broadcaster cleanup goes as planned. If we destruct
   // this class, then Process::~Process() might have problems trying to fully
   // destroy the broadcaster.
   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,
+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");
   return error;
 }
 
 bool ProcessKDP::GetHostArchitecture(ArchSpec &arch) {
   uint32_t cpu = m_comm.GetCPUType();

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

public:
 
   // Check if a given Process
   bool CanDebug(lldb::TargetSP target_sp,
                 bool plugin_specified_by_name) override;
 
   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,
+  Status DoWillAttachToProcessWithName(const char *process_name,
                                        bool wait_for_launch) override;
 
   Status DoConnectRemote(llvm::StringRef remote_url) override;
 
   Status WillLaunchOrAttach();
 
   Status DoAttachToProcessWithID(lldb::pid_t pid,
                                  const ProcessAttachInfo &attach_info) override;
 

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

void ProcessGDBRemote::BuildDynamicRegisterInfo(bool force) {
   }
 
   if (registers.empty())
     registers = GetFallbackRegisters(arch_to_use);
 
   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,
+Status ProcessGDBRemote::DoWillAttachToProcessWithName(const char *process_name,
                                                        bool wait_for_launch) {
   return WillLaunchOrAttach();
 }
 
 Status ProcessGDBRemote::DoConnectRemote(llvm::StringRef remote_url) {
   Log *log = GetLog(GDBRLog::Process);
 
   Status error(WillLaunchOrAttach());
   if (error.Fail())

lldb/source/Target/Process.cpp

 
 ListenerSP ProcessAttachInfo::GetListenerForProcess(Debugger &debugger) {
   if (m_listener_sp)
     return m_listener_sp;
   else
     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();
   m_dyld_up.reset();
   m_jit_loaders_up.reset();
   m_system_runtime_up.reset();
   m_os_up.reset();
 

lldb/source/Target/Target.cpp

bool Target::EnableBreakpointByID(break_id_t break_id) {
 
   if (bp_sp) {
     bp_sp->SetEnabled(true);
     return true;
   }
   return false;
 }
 
+void Target::ResetBreakpointHitCounts() {
+  GetBreakpointList().ResetHitCounts();
+}
+
 Status Target::SerializeBreakpointsToFile(const FileSpec &file,
                                           const BreakpointIDList &bp_ids,
                                           bool append) {
   Status error;
 
   if (!file) {
     error.SetErrorString("Invalid FileSpec.");
     return error;

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

def address_breakpoints(self):
         process = target.Launch(launch_info, error)
         self.assertTrue(process, PROCESS_IS_VALID)
 
         thread = get_threads_stopped_at_breakpoint(process, breakpoint)
         self.assertEqual(
             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

def breakpoint_command_sequence(self):
 
         # We should be stopped again due to breakpoint 1.
 
         # The stop reason of the thread should be breakpoint.
         self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
                     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."""
         exe = self.getBuildArtifact("a.out")
         self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
 
         # Add a breakpoint.
         lldbutil.run_break_set_by_file_and_line(

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

+CXX_SOURCES := main.cpp
+
+include Makefile.rules

lldb/test/API/functionalities/breakpoint/breakpoint_reset_upon_run/TestBreakpointResetUponRun.py

+"""
+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

+#include <stdio.h>
+
+int main(int argc, char const *argv[]) {
+  printf("Set a breakpoint here.\n");
+  return 0;
+}