Rename StoppointLocation to StoppointSite and drop its relationship with BreakpointLocation

[tatyana-krasnukha]

Summary

Working on the patch D84257 I noticed that both BreakpointLocation and BreakpointSite were inherited from StoppointLocation. Also, I noticed that they have not so much in common, except the id and hit counting logic.
There is no polymorphic code that uses a pointer/reference to StoppointLocation to handle one of BreakpointLocation and BreakpointSite either.

The patch renames StoppointLocation to StoppointSite and stops BreakpointLocation's inheriting from it.
Hit counting is encapsulated into StoppointHitCounter which is re-used in StoppointSite, BreakpointLocation, and Breakpoint.

[JDevlieghere]

Some nits but overall this looks good.

lldb/include/lldb/Breakpoint/Breakpoint.h

656–657

Please make this a Doxygen comment /// above the variable.

lldb/include/lldb/Breakpoint/StoppointHitCounter.h

38

With the 80 col limit the trailing comments are really hard to read and maintain. Please put them above the variable.

lldb/include/lldb/Breakpoint/StoppointSite.h

21

I'm slowly getting rid of these (rather useless) comments, so let's not add any new ones :-)

53

Same comment as earlier.

67

Private already conveys that.

[jingham]

This overall change makes sense to me.

It seems a little awkward that Target has to know that Watchpoints have a m_hit_counter. It's also a little weird that we're clearing watchpoint hit counts when a process dies, but not breakpoint hit counts. If we do one we should do the other... That isn't your doing, but if you need to have Target resetting both the WP & BP hit counts, it might be nicer to do this with an API.

[tatyana-krasnukha]

Addressed comments.

It's also a little weird that we're clearing watchpoint hit counts when a process dies, but not breakpoint hit counts. If we do one we should do the other...

BreakpointSites die with the Process, so there is no need to reset their hit count. I'm not sure what logic Breakpoint should follow (as it lives without a process). If we are going to change it, I would do it in a different revision.

[JDevlieghere]

LGTM

[jingham]

Remember to do the action thingie...

lldb/include/lldb/Breakpoint/Breakpoint.h

 #include <vector>
 
 #include "lldb/Breakpoint/BreakpointID.h"
 #include "lldb/Breakpoint/BreakpointLocationCollection.h"
 #include "lldb/Breakpoint/BreakpointLocationList.h"
 #include "lldb/Breakpoint/BreakpointName.h"
 #include "lldb/Breakpoint/BreakpointOptions.h"
 #include "lldb/Breakpoint/Stoppoint.h"
+#include "lldb/Breakpoint/StoppointHitCounter.h"
 #include "lldb/Core/SearchFilter.h"
 #include "lldb/Utility/Event.h"
 #include "lldb/Utility/StringList.h"
 #include "lldb/Utility/StructuredData.h"
 
 namespace lldb_private {
 
 /// \class Breakpoint Breakpoint.h "lldb/Breakpoint/Breakpoint.h" Class that
protected:
   // BreakpointLocation::IgnoreCountShouldStop &
   // Breakpoint::IgnoreCountShouldStop can only be called once per stop, and
   // BreakpointLocation::IgnoreCountShouldStop should be tested first, and if
   // it returns false we should continue, otherwise we should test
   // Breakpoint::IgnoreCountShouldStop.
 
   bool IgnoreCountShouldStop();
 
-  void IncrementHitCount() { m_hit_count++; }
-
-  void DecrementHitCount() {
-    assert(m_hit_count > 0);
-    m_hit_count--;
-  }
-
 private:
   // To call from CopyFromBreakpoint.
   Breakpoint(Target &new_target, const Breakpoint &bp_to_copy_from);
 
   // For Breakpoint only
   bool m_being_created;
   bool
       m_hardware; // If this breakpoint is required to use a hardware breakpoint
private:
   // to skip certain breakpoint hits.  For instance, exception breakpoints use
   // this to limit the stop to certain exception classes, while leaving the
   // condition & callback free for user specification.
   std::unique_ptr<BreakpointOptions>
       m_options_up; // Settable breakpoint options
   BreakpointLocationList
       m_locations; // The list of locations currently found for this breakpoint.
   std::string m_kind_description;
   bool m_resolve_indirect_symbols;
-  uint32_t m_hit_count; // Number of times this breakpoint/watchpoint has been
-                        // hit.  This is kept
-  // separately from the locations hit counts, since locations can go away when
-  // their backing library gets unloaded, and we would lose hit counts.
+
+  /// Number of times this breakpoint has been hit. This is kept separately
+  /// from the locations hit counts, since locations can go away when their
+  /// backing library gets unloaded, and we would lose hit counts.
+  StoppointHitCounter m_hit_counter;
+
   BreakpointName::Permissions m_permissions;
 
   void SendBreakpointChangedEvent(lldb::BreakpointEventType eventKind);
 
   void SendBreakpointChangedEvent(BreakpointEventData *data);
 
   Breakpoint(const Breakpoint &) = delete;
   const Breakpoint &operator=(const Breakpoint &) = delete;
 };
 
 } // namespace lldb_private
 
 #endif // LLDB_BREAKPOINT_BREAKPOINT_H

lldb/include/lldb/Breakpoint/BreakpointLocation.h

 //===-- BreakpointLocation.h ------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLDB_BREAKPOINT_BREAKPOINTLOCATION_H
 #define LLDB_BREAKPOINT_BREAKPOINTLOCATION_H
 
 #include <memory>
 #include <mutex>
 
 #include "lldb/Breakpoint/BreakpointOptions.h"
-#include "lldb/Breakpoint/StoppointLocation.h"
+#include "lldb/Breakpoint/StoppointHitCounter.h"
 #include "lldb/Core/Address.h"
 #include "lldb/Utility/UserID.h"
 #include "lldb/lldb-private.h"
 
 namespace lldb_private {
 
 /// \class BreakpointLocation BreakpointLocation.h
 /// "lldb/Breakpoint/BreakpointLocation.h" Class that manages one unique (by
 /// address) instance of a logical breakpoint.
 
 /// General Outline:
 /// A breakpoint location is defined by the breakpoint that produces it,
 /// and the address that resulted in this particular instantiation. Each
 /// breakpoint location also may have a breakpoint site if its address has
 /// been loaded into the program. Finally it has a settable options object.
 ///
 /// FIXME: Should we also store some fingerprint for the location, so
 /// we can map one location to the "equivalent location" on rerun?  This would
 /// be useful if you've set options on the locations.
 
 class BreakpointLocation
-    : public std::enable_shared_from_this<BreakpointLocation>,
-      public StoppointLocation {
+    : public std::enable_shared_from_this<BreakpointLocation> {
 public:
-  ~BreakpointLocation() override;
+  ~BreakpointLocation();
 
   /// Gets the load address for this breakpoint location \return
   ///     Returns breakpoint location load address, \b
   ///     LLDB_INVALID_ADDRESS if not yet set.
-  lldb::addr_t GetLoadAddress() const override;
+  lldb::addr_t GetLoadAddress() const;
 
   /// Gets the Address for this breakpoint location \return
   ///     Returns breakpoint location Address.
   Address &GetAddress();
   /// Gets the Breakpoint that created this breakpoint location \return
   ///     Returns the owning breakpoint.
   Breakpoint &GetBreakpoint();
 
   Target &GetTarget();
 
   /// Determines whether we should stop due to a hit at this breakpoint
   /// location.
   ///
   /// Side Effects: This may evaluate the breakpoint condition, and run the
   /// callback.  So this command may do a considerable amount of work.
   ///
   /// \return
   ///     \b true if this breakpoint location thinks we should stop,
   ///     \b false otherwise.
-  bool ShouldStop(StoppointCallbackContext *context) override;
+  bool ShouldStop(StoppointCallbackContext *context);
 
   // The next section deals with various breakpoint options.
 
   /// If \a enabled is \b true, enable the breakpoint, if \b false disable it.
   void SetEnabled(bool enabled);
 
   /// Check the Enable/Disable state.
   ///
   /// \return
   ///     \b true if the breakpoint is enabled, \b false if disabled.
   bool IsEnabled() const;
 
-  /// Check if it is a hardware breakpoint.
-  ///
-  /// \return
-  ///     \b true if the breakpoint is a harware breakpoint.
-  bool IsHardware() const override;
-
   /// If \a auto_continue is \b true, set the breakpoint to continue when hit.
   void SetAutoContinue(bool auto_continue);
 
   /// Check the AutoContinue state.
   ///
   /// \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(); }
+
   /// Return the current Ignore Count.
   ///
   /// \return
   ///     The number of breakpoint hits to be ignored.
-  uint32_t GetIgnoreCount();
+  uint32_t GetIgnoreCount() const;
 
   /// Set the breakpoint to ignore the next \a count breakpoint hits.
   ///
   /// \param[in] n
   ///    The number of breakpoint hits to ignore.
   void SetIgnoreCount(uint32_t n);
 
   /// Set the callback action invoked when the breakpoint is hit.
public:
   /// \param[in] level
   ///     The description level that indicates the detail level to
   ///     provide.
   ///
   /// \see lldb::DescriptionLevel
   void GetDescription(Stream *s, lldb::DescriptionLevel level);
 
   /// Standard "Dump" method.  At present it does nothing.
-  void Dump(Stream *s) const override;
+  void Dump(Stream *s) const;
 
   /// Use this to set location specific breakpoint options.
   ///
   /// It will create a copy of the containing breakpoint's options if that
   /// hasn't been done already
   ///
   /// \return
   ///    A pointer to the breakpoint options.
public:
   ///
   /// \param[in] location
   ///    The location to compare against.
   ///
   /// \return
   ///     \b true or \b false as given in the description above.
   bool EquivalentToLocation(BreakpointLocation &location);
 
+  /// Returns the breakpoint location ID.
+  lldb::break_id_t GetID() const { return m_loc_id; }
+
 protected:
   friend class BreakpointSite;
   friend class BreakpointLocationList;
   friend class Process;
   friend class StopInfoBreakpoint;
 
   /// Set the breakpoint site for this location to \a bp_site_sp.
   ///
lldb::BreakpointSiteSP m_bp_site_sp; ///< Our breakpoint site (it may be
                                        ///shared by more than one location.)
   lldb::UserExpressionSP m_user_expression_sp; ///< The compiled expression to
                                                ///use in testing our condition.
   std::mutex m_condition_mutex; ///< Guards parsing and evaluation of the
                                 ///condition, which could be evaluated by
                                 /// multiple processes.
   size_t m_condition_hash; ///< For testing whether the condition source code
                            ///changed.
+  lldb::break_id_t m_loc_id; ///< Breakpoint location ID.
+  StoppointHitCounter m_hit_counter; ///< Number of times this breakpoint
+                                     /// location has been hit.
 
   void SetShouldResolveIndirectFunctions(bool do_resolve) {
     m_should_resolve_indirect_functions = do_resolve;
   }
 
   void SendBreakpointLocationChangedEvent(lldb::BreakpointEventType eventKind);
 
   BreakpointLocation(const BreakpointLocation &) = delete;
   const BreakpointLocation &operator=(const BreakpointLocation &) = delete;
 };
 
 } // namespace lldb_private
 
 #endif // LLDB_BREAKPOINT_BREAKPOINTLOCATION_H

lldb/include/lldb/Breakpoint/BreakpointSite.h

 //===-- BreakpointSite.h ----------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLDB_BREAKPOINT_BREAKPOINTSITE_H
 #define LLDB_BREAKPOINT_BREAKPOINTSITE_H
 
 #include <list>
 #include <mutex>
 
 
 #include "lldb/Breakpoint/BreakpointLocationCollection.h"
-#include "lldb/Breakpoint/StoppointLocation.h"
+#include "lldb/Breakpoint/StoppointSite.h"
 #include "lldb/Utility/LLDBAssert.h"
 #include "lldb/Utility/UserID.h"
 #include "lldb/lldb-forward.h"
 
 namespace lldb_private {
 
 /// \class BreakpointSite BreakpointSite.h "lldb/Breakpoint/BreakpointSite.h"
 /// Class that manages the actual breakpoint that will be inserted into the
 /// running program.
 ///
 /// The BreakpointSite class handles the physical breakpoint that is actually
 /// inserted in the target program.  As such, it is also the one that  gets
 /// hit, when the program stops. It keeps a list of all BreakpointLocations
 /// that share this physical site. When the breakpoint is hit, all the
 /// locations are informed by the breakpoint site. Breakpoint sites are owned
 /// by the process.
 
 class BreakpointSite : public std::enable_shared_from_this<BreakpointSite>,
-                       public StoppointLocation {
+                       public StoppointSite {
 public:
   enum Type {
     eSoftware, // Breakpoint opcode has been written to memory and
                // m_saved_opcode
                // and m_trap_opcode contain the saved and written opcode.
     eHardware, // Breakpoint site is set as a hardware breakpoint
     eExternal  // Breakpoint site is managed by an external debug nub or
                // debug interface where memory reads transparently will not
public:
   const uint8_t *GetTrapOpcodeBytes() const;
 
   /// Get the size of the trap opcode for this address
   size_t GetTrapOpcodeMaxByteSize() const;
 
   /// Sets the trap opcode
   bool SetTrapOpcode(const uint8_t *trap_opcode, uint32_t trap_opcode_size);
 
-  void SetHardwareIndex(uint32_t index) override;
-
   /// Gets the original instruction bytes that were overwritten by the trap
   uint8_t *GetSavedOpcodeBytes();
 
   /// Gets the original instruction bytes that were overwritten by the trap
   /// const version
   const uint8_t *GetSavedOpcodeBytes() const;
 
   /// Says whether \a addr and size \a size intersects with the address \a

lldb/include/lldb/Breakpoint/StoppointHitCounter.h

+//===-- StoppointHitCounter.h -----------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_BREAKPOINT_STOPPOINT_HIT_COUNTER_H
+#define LLDB_BREAKPOINT_STOPPOINT_HIT_COUNTER_H
+
+#include <assert.h>
+#include <cstdint>
+#include <limits>
+
+#include "lldb/Utility/LLDBAssert.h"
+
+namespace lldb_private {
+
+class StoppointHitCounter {
+public:
+  uint32_t GetValue() const { return m_hit_count; }
+
+  void Increment(uint32_t difference = 1) {
+    lldbassert(std::numeric_limits<uint32_t>::max() - m_hit_count >= difference);
+    m_hit_count += difference;
+  }
+
+  void Decrement(uint32_t difference = 1) {
+    lldbassert(m_hit_count >= difference);
+    m_hit_count -= difference;
+  }
+
+  void Reset() { m_hit_count = 0; }
+
+private:
+  /// Number of times this breakpoint/watchpoint has been hit.
+  uint32_t m_hit_count = 0;
+};
+
+} // namespace lldb_private
+
+#endif // LLDB_BREAKPOINT_STOPPOINT_HIT_COUNTER_H

lldb/include/lldb/Breakpoint/StoppointLocation.h

-//===-- StoppointLocation.h -------------------------------------*- C++ -*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-#ifndef LLDB_BREAKPOINT_STOPPOINTLOCATION_H
-#define LLDB_BREAKPOINT_STOPPOINTLOCATION_H
-
-#include "lldb/Utility/UserID.h"
-#include "lldb/lldb-private.h"
-// #include "lldb/Breakpoint/BreakpointOptions.h"
-
-namespace lldb_private {
-
-class StoppointLocation {
-public:
-  // Constructors and Destructors
-  StoppointLocation(lldb::break_id_t bid, lldb::addr_t m_addr, bool hardware);
-
-  StoppointLocation(lldb::break_id_t bid, lldb::addr_t m_addr,
-                    uint32_t byte_size, bool hardware);
-
-  virtual ~StoppointLocation();
-
-  // Operators
-
-  // Methods
-  virtual lldb::addr_t GetLoadAddress() const { return m_addr; }
-
-  virtual void SetLoadAddress(lldb::addr_t addr) { m_addr = addr; }
-
-  uint32_t GetByteSize() const { return m_byte_size; }
-
-  uint32_t GetHitCount() const { return m_hit_count; }
-
-  uint32_t GetHardwareIndex() const { return m_hardware_index; }
-
-  bool HardwareRequired() const { return m_hardware; }
-
-  virtual bool IsHardware() const = 0;
-
-  virtual bool ShouldStop(StoppointCallbackContext *context) { return true; }
-
-  virtual void Dump(Stream *stream) const {}
-
-  virtual void SetHardwareIndex(uint32_t index) { m_hardware_index = index; }
-
-  lldb::break_id_t GetID() const { return m_loc_id; }
-
-protected:
-  // Classes that inherit from StoppointLocation can see and modify these
-  lldb::break_id_t m_loc_id; // Stoppoint location ID
-  lldb::addr_t
-      m_addr; // The load address of this stop point. The base Stoppoint doesn't
-  // store a full Address since that's not needed for the breakpoint sites.
-  bool m_hardware; // True if this point has been is required to use hardware
-                   // (which may fail due to lack of resources)
-  uint32_t m_hardware_index; // The hardware resource index for this
-                             // breakpoint/watchpoint
-  uint32_t m_byte_size; // The size in bytes of stop location.  e.g. the length
-                        // of the trap opcode for
-  // software breakpoints, or the optional length in bytes for hardware
-  // breakpoints, or the length of the watchpoint.
-  uint32_t
-      m_hit_count; // Number of times this breakpoint/watchpoint has been hit
-
-  // If you override this, be sure to call the base class to increment the
-  // internal counter.
-  void IncrementHitCount() { ++m_hit_count; }
-
-  void DecrementHitCount();
-
-private:
-  // For StoppointLocation only
-  StoppointLocation(const StoppointLocation &) = delete;
-  const StoppointLocation &operator=(const StoppointLocation &) = delete;
-  StoppointLocation() = delete;
-};
-
-} // namespace lldb_private
-
-#endif // LLDB_BREAKPOINT_STOPPOINTLOCATION_H

lldb/include/lldb/Breakpoint/StoppointSite.h

+//===-- StoppointSite.h -----------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_BREAKPOINT_STOPPOINTSITE_H
+#define LLDB_BREAKPOINT_STOPPOINTSITE_H
+
+#include "lldb/Breakpoint/StoppointHitCounter.h"
+#include "lldb/Utility/UserID.h"
+#include "lldb/lldb-private.h"
+
+namespace lldb_private {
+
+class StoppointSite {
+public:
+  StoppointSite(lldb::break_id_t bid, lldb::addr_t m_addr, bool hardware);
+
+  StoppointSite(lldb::break_id_t bid, lldb::addr_t m_addr,
+                uint32_t byte_size, bool hardware);
+
+  virtual ~StoppointSite() = default;
+
+  virtual lldb::addr_t GetLoadAddress() const { return m_addr; }
+
+  virtual void SetLoadAddress(lldb::addr_t addr) { m_addr = addr; }
+
+  uint32_t GetByteSize() const { return m_byte_size; }
+
+  uint32_t GetHitCount() const { return m_hit_counter.GetValue(); }
+
+  void ResetHitCount() { m_hit_counter.Reset(); }
+
+  bool HardwareRequired() const { return m_is_hardware_required; }
+
+  virtual bool IsHardware() const = 0;
+
+  uint32_t GetHardwareIndex() const { return m_hardware_index; }
+
+  void SetHardwareIndex(uint32_t index) { m_hardware_index = index; }
+
+  virtual bool ShouldStop(StoppointCallbackContext* context) = 0;
+
+  virtual void Dump(Stream* stream) const = 0;
+
+  lldb::break_id_t GetID() const { return m_id; }
+
+protected:
+  /// Stoppoint site ID.
+  lldb::break_id_t m_id;
+
+  /// The load address of this stop point.
+  lldb::addr_t m_addr;
+
+  /// True if this point is required to use hardware (which may fail due to
+  /// the lack of resources).
+  bool m_is_hardware_required;
+
+  /// The hardware resource index for this breakpoint/watchpoint.
+  uint32_t m_hardware_index;
+
+  /// The size in bytes of stoppoint, e.g. the length of the trap opcode for
+  /// software breakpoints, or the optional length in bytes for hardware
+  /// breakpoints, or the length of the watchpoint.
+  uint32_t m_byte_size;
+
+  /// Number of times this breakpoint/watchpoint has been hit.
+  StoppointHitCounter m_hit_counter;
+
+private:
+  StoppointSite(const StoppointSite &) = delete;
+  const StoppointSite &operator=(const StoppointSite &) = delete;
+  StoppointSite() = delete;
+};
+
+} // namespace lldb_private
+
+#endif // LLDB_BREAKPOINT_STOPPOINTSITE_H

lldb/include/lldb/Breakpoint/Watchpoint.h

 //===-- Watchpoint.h --------------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLDB_BREAKPOINT_WATCHPOINT_H
 #define LLDB_BREAKPOINT_WATCHPOINT_H
 
 #include <memory>
 #include <string>
 
-#include "lldb/Breakpoint/StoppointLocation.h"
+#include "lldb/Breakpoint/StoppointSite.h"
 #include "lldb/Breakpoint/WatchpointOptions.h"
 #include "lldb/Symbol/CompilerType.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Utility/UserID.h"
 #include "lldb/lldb-private.h"
 
 namespace lldb_private {
 
 class Watchpoint : public std::enable_shared_from_this<Watchpoint>,
-                   public StoppointLocation {
+                   public StoppointSite {
 public:
   class WatchpointEventData : public EventData {
   public:
     WatchpointEventData(lldb::WatchpointEventType sub_type,
                         const lldb::WatchpointSP &new_watchpoint_sp);
 
     ~WatchpointEventData() override;
 
public:
   bool IsDisabledDuringEphemeralMode();
 
   const CompilerType &GetCompilerType() { return m_type; }
 
 private:
   friend class Target;
   friend class WatchpointList;
 
-  void ResetHitCount() { m_hit_count = 0; }
-
   void ResetHistoricValues() {
     m_old_value_sp.reset();
     m_new_value_sp.reset();
   }
 
   Target &m_target;
   bool m_enabled;           // Is this watchpoint enabled
   bool m_is_hardware;       // Is this a hardware watchpoint
Status m_error; // An error object describing errors associated with this
                   // watchpoint.
   WatchpointOptions
       m_options; // Settable watchpoint options, which is a delegate to handle
                  // the callback machinery.
   bool m_being_created;
 
   std::unique_ptr<UserExpression> m_condition_up; // The condition to test.
 
-  void SetID(lldb::watch_id_t id) { m_loc_id = id; }
+  void SetID(lldb::watch_id_t id) { m_id = id; }
 
   void SendWatchpointChangedEvent(lldb::WatchpointEventType eventKind);
 
   void SendWatchpointChangedEvent(WatchpointEventData *data);
 
   Watchpoint(const Watchpoint &) = delete;
   const Watchpoint &operator=(const Watchpoint &) = delete;
 };
 
 } // namespace lldb_private
 
 #endif // LLDB_BREAKPOINT_WATCHPOINT_H

lldb/include/lldb/lldb-forward.h

 class StackFrameList;
 class StackFrameRecognizer;
 class StackFrameRecognizerManager;
 class StackID;
 class Status;
 class StopInfo;
 class Stoppoint;
 class StoppointCallbackContext;
-class StoppointLocation;
 class Stream;
 class StreamFile;
 class StreamString;
 class StringList;
 class StructuredDataImpl;
 class StructuredDataPlugin;
 class Symbol;
 class SymbolContext;

 typedef std::unique_ptr<lldb_private::StackFrame> StackFrameUP;
 typedef std::weak_ptr<lldb_private::StackFrame> StackFrameWP;
 typedef std::shared_ptr<lldb_private::StackFrameList> StackFrameListSP;
 typedef std::shared_ptr<lldb_private::StackFrameRecognizer>
     StackFrameRecognizerSP;
 typedef std::unique_ptr<lldb_private::StackFrameRecognizerManager>
     StackFrameRecognizerManagerUP;
 typedef std::shared_ptr<lldb_private::StopInfo> StopInfoSP;
-typedef std::shared_ptr<lldb_private::StoppointLocation> StoppointLocationSP;
 typedef std::shared_ptr<lldb_private::Stream> StreamSP;
 typedef std::weak_ptr<lldb_private::Stream> StreamWP;
 typedef std::shared_ptr<lldb_private::StreamFile> StreamFileSP;
 typedef std::shared_ptr<lldb_private::StringSummaryFormat>
     StringTypeSummaryImplSP;
 typedef std::unique_ptr<lldb_private::StructuredDataImpl> StructuredDataImplUP;
 typedef std::shared_ptr<lldb_private::StructuredDataPlugin>
     StructuredDataPluginSP;

lldb/source/Breakpoint/Breakpoint.cpp

 
 // Breakpoint constructor
 Breakpoint::Breakpoint(Target &target, SearchFilterSP &filter_sp,
                        BreakpointResolverSP &resolver_sp, bool hardware,
                        bool resolve_indirect_symbols)
     : m_being_created(true), m_hardware(hardware), m_target(target),
       m_filter_sp(filter_sp), m_resolver_sp(resolver_sp),
       m_options_up(new BreakpointOptions(true)), m_locations(*this),
-      m_resolve_indirect_symbols(resolve_indirect_symbols), m_hit_count(0) {
+      m_resolve_indirect_symbols(resolve_indirect_symbols), m_hit_counter() {
   m_being_created = false;
 }
 
 Breakpoint::Breakpoint(Target &new_target, const Breakpoint &source_bp)
     : m_being_created(true), m_hardware(source_bp.m_hardware),
       m_target(new_target), m_name_list(source_bp.m_name_list),
       m_options_up(new BreakpointOptions(*source_bp.m_options_up)),
       m_locations(*this),
       m_resolve_indirect_symbols(source_bp.m_resolve_indirect_symbols),
-      m_hit_count(0) {}
+      m_hit_counter() {}
 
 // Destructor
 Breakpoint::~Breakpoint() = default;
 
 BreakpointSP Breakpoint::CopyFromBreakpoint(TargetSP new_target,
     const Breakpoint& bp_to_copy_from) {
   if (!new_target)
     return BreakpointSP();
if (ignore != 0) {
     // an ignore count, since by contract we call it first...  So we don't have
     // to find & decrement it, we only have to decrement our own ignore count.
     DecrementIgnoreCount();
     return false;
   } else
     return true;
 }
 
-uint32_t Breakpoint::GetHitCount() const { return m_hit_count; }
+uint32_t Breakpoint::GetHitCount() const { return m_hit_counter.GetValue(); }
 
 bool Breakpoint::IsOneShot() const { return m_options_up->IsOneShot(); }
 
 void Breakpoint::SetOneShot(bool one_shot) {
   m_options_up->SetOneShot(one_shot);
 }
 
 bool Breakpoint::IsAutoContinue() const { 

lldb/source/Breakpoint/BreakpointLocation.cpp

 #include "lldb/Utility/StreamString.h"
 
 using namespace lldb;
 using namespace lldb_private;
 
 BreakpointLocation::BreakpointLocation(break_id_t loc_id, Breakpoint &owner,
                                        const Address &addr, lldb::tid_t tid,
                                        bool hardware, bool check_for_resolver)
-    : StoppointLocation(loc_id, addr.GetOpcodeLoadAddress(&owner.GetTarget()),
-                        hardware),
-      m_being_created(true), m_should_resolve_indirect_functions(false),
+    : m_being_created(true), m_should_resolve_indirect_functions(false),
       m_is_reexported(false), m_is_indirect(false), m_address(addr),
-      m_owner(owner), m_options_up(), m_bp_site_sp(), m_condition_mutex() {
+      m_owner(owner), m_options_up(), m_bp_site_sp(), m_condition_mutex(),
+      m_condition_hash(0), m_loc_id(loc_id), m_hit_counter() {
   if (check_for_resolver) {
     Symbol *symbol = m_address.CalculateSymbolContextSymbol();
     if (symbol && symbol->IsIndirect()) {
       SetShouldResolveIndirectFunctions(true);
     }
   }
 
   SetThreadID(tid);

 }
 
 Address &BreakpointLocation::GetAddress() { return m_address; }
 
 Breakpoint &BreakpointLocation::GetBreakpoint() { return m_owner; }
 
 Target &BreakpointLocation::GetTarget() { return m_owner.GetTarget(); }
 
-bool BreakpointLocation::IsHardware() const {
-  if (m_bp_site_sp)
-    return m_bp_site_sp->IsHardware();
-
-  // If breakpoint location is not resolved yet, it cannot be hardware.
-  return false;
-}
-
 bool BreakpointLocation::IsEnabled() const {
   if (!m_owner.IsEnabled())
     return false;
   else if (m_options_up != nullptr)
     return m_options_up->IsEnabled();
   else
     return true;
 }
if (result_code == eExpressionCompleted) {
     ret = false;
     error.SetErrorStringWithFormat("Couldn't execute expression:\n%s",
                                    diagnostics.GetString().c_str());
   }
 
   return ret;
 }
 
-uint32_t BreakpointLocation::GetIgnoreCount() {
+uint32_t BreakpointLocation::GetIgnoreCount() const {
   return GetOptionsSpecifyingKind(BreakpointOptions::eIgnoreCount)
       ->GetIgnoreCount();
 }
 
 void BreakpointLocation::SetIgnoreCount(uint32_t n) {
   GetLocationOptions()->SetIgnoreCount(n);
   SendBreakpointLocationChangedEvent(eBreakpointEventTypeIgnoreChanged);
 }
bool BreakpointLocation::ShouldStop(StoppointCallbackContext *context) {
   }
 
   return should_stop;
 }
 
 void BreakpointLocation::BumpHitCount() {
   if (IsEnabled()) {
     // Step our hit count, and also step the hit count of the owner.
-    IncrementHitCount();
-    m_owner.IncrementHitCount();
+    m_hit_counter.Increment();
+    m_owner.m_hit_counter.Increment();
   }
 }
 
 void BreakpointLocation::UndoBumpHitCount() {
   if (IsEnabled()) {
     // Step our hit count, and also step the hit count of the owner.
-    DecrementHitCount();
-    m_owner.DecrementHitCount();
+    m_hit_counter.Decrement();
+    m_owner.m_hit_counter.Decrement();
   }
 }
 
 bool BreakpointLocation::IsResolved() const {
   return m_bp_site_sp.get() != nullptr;
 }
 
 lldb::BreakpointSiteSP BreakpointLocation::GetBreakpointSite() const {
if (resolved_symbol) {
         s->EOL();
         s->Indent();
       }
       s->Printf("indirect target = %s",
                 resolved_symbol->GetName().GetCString());
     }
   }
 
+  bool is_resolved = IsResolved();
+  bool is_hardware = is_resolved && m_bp_site_sp->IsHardware();
+
   if (level == lldb::eDescriptionLevelVerbose) {
     s->EOL();
     s->Indent();
-    s->Printf("resolved = %s\n", IsResolved() ? "true" : "false");
+    s->Printf("resolved = %s\n", is_resolved ? "true" : "false");
     s->Indent();
-    s->Printf("hardware = %s\n", IsHardware() ? "true" : "false");
+    s->Printf("hardware = %s\n", is_hardware ? "true" : "false");
     s->Indent();
     s->Printf("hit count = %-4u\n", GetHitCount());
 
     if (m_options_up) {
       s->Indent();
       m_options_up->GetDescription(s, level);
       s->EOL();
     }
     s->IndentLess();
   } else if (level != eDescriptionLevelInitial) {
-    s->Printf(", %sresolved, %shit count = %u ", (IsResolved() ? "" : "un"),
-              (IsHardware() ? "hardware, " : ""), GetHitCount());
+    s->Printf(", %sresolved, %shit count = %u ", (is_resolved ? "" : "un"),
+              (is_hardware ? "hardware, " : ""), GetHitCount());
     if (m_options_up) {
       m_options_up->GetDescription(s, level);
     }
   }
 }
 
 void BreakpointLocation::Dump(Stream *s) const {
   if (s == nullptr)
     return;
 
+  bool is_resolved = IsResolved();
+  bool is_hardware = is_resolved && m_bp_site_sp->IsHardware();
+  auto hardware_index = is_resolved ?
+      m_bp_site_sp->GetHardwareIndex() : LLDB_INVALID_INDEX32;
+
   lldb::tid_t tid = GetOptionsSpecifyingKind(BreakpointOptions::eThreadSpec)
       ->GetThreadSpecNoCreate()->GetTID();
   s->Printf("BreakpointLocation %u: tid = %4.4" PRIx64
             "  load addr = 0x%8.8" PRIx64 "  state = %s  type = %s breakpoint  "
             "hw_index = %i  hit_count = %-4u  ignore_count = %-4u",
             GetID(), tid,
             (uint64_t)m_address.GetOpcodeLoadAddress(&m_owner.GetTarget()),
             (m_options_up ? m_options_up->IsEnabled() : m_owner.IsEnabled())
                 ? "enabled "
                 : "disabled",
-            IsHardware() ? "hardware" : "software", GetHardwareIndex(),
-            GetHitCount(),
+            is_hardware ? "hardware" : "software", hardware_index, GetHitCount(),
             GetOptionsSpecifyingKind(BreakpointOptions::eIgnoreCount)
                 ->GetIgnoreCount());
 }
 
 void BreakpointLocation::SendBreakpointLocationChangedEvent(
     lldb::BreakpointEventType eventKind) {
   if (!m_being_created && !m_owner.IsInternal() &&
       m_owner.GetTarget().EventTypeHasListeners(

lldb/source/Breakpoint/BreakpointSite.cpp

 #include "lldb/Utility/Stream.h"
 
 using namespace lldb;
 using namespace lldb_private;
 
 BreakpointSite::BreakpointSite(BreakpointSiteList *list,
                                const BreakpointLocationSP &owner,
                                lldb::addr_t addr, bool use_hardware)
-    : StoppointLocation(GetNextID(), addr, 0, use_hardware),
+    : StoppointSite(GetNextID(), addr, 0, use_hardware),
       m_type(eSoftware), // Process subclasses need to set this correctly using
                          // SetType()
       m_saved_opcode(), m_trap_opcode(),
       m_enabled(false), // Need to create it disabled, so the first enable turns
                         // it on.
       m_owners(), m_owners_mutex() {
   m_owners.Add(owner);
 }
break_id_t BreakpointSite::GetNextID() {
   static break_id_t g_next_id = 0;
   return ++g_next_id;
 }
 
 // RETURNS - true if we should stop at this breakpoint, false if we
 // should continue.
 
 bool BreakpointSite::ShouldStop(StoppointCallbackContext *context) {
-  IncrementHitCount();
+  m_hit_counter.Increment();
   // ShouldStop can do a lot of work, and might even come come back and hit
   // this breakpoint site again.  So don't hold the m_owners_mutex the whole
   // while.  Instead make a local copy of the collection and call ShouldStop on
   // the copy.
   BreakpointLocationCollection owners_copy;
   {
     std::lock_guard<std::recursive_mutex> guard(m_owners_mutex);
     owners_copy = m_owners;

 
 void BreakpointSite::BumpHitCounts() {
   std::lock_guard<std::recursive_mutex> guard(m_owners_mutex);
   for (BreakpointLocationSP loc_sp : m_owners.BreakpointLocations()) {
     loc_sp->BumpHitCount();
   }
 }
 
-void BreakpointSite::SetHardwareIndex(uint32_t index) {
-  std::lock_guard<std::recursive_mutex> guard(m_owners_mutex);
-  for (BreakpointLocationSP loc_sp : m_owners.BreakpointLocations()) {
-    loc_sp->SetHardwareIndex(index);
-  }
-}
-
 bool BreakpointSite::IntersectsRange(lldb::addr_t addr, size_t size,
                                      lldb::addr_t *intersect_addr,
                                      size_t *intersect_size,
                                      size_t *opcode_offset) const {
   // The function should be called only for software breakpoints.
   lldbassert(GetType() == Type::eSoftware);
 
   if (m_byte_size == 0)

lldb/source/Breakpoint/CMakeLists.txt

add_lldb_library(lldbBreakpoint
   BreakpointResolverFileLine.cpp
   BreakpointResolverFileRegex.cpp
   BreakpointResolverName.cpp
   BreakpointResolverScripted.cpp
   BreakpointSite.cpp
   BreakpointSiteList.cpp
   Stoppoint.cpp
   StoppointCallbackContext.cpp
-  StoppointLocation.cpp
+  StoppointSite.cpp
   Watchpoint.cpp
   WatchpointList.cpp
   WatchpointOptions.cpp
 
   LINK_LIBS
     lldbCore
     lldbExpression
     lldbInterpreter
     lldbSymbol
     lldbTarget
     lldbUtility
 
   LINK_COMPONENTS
     Support
   )

lldb/source/Breakpoint/StoppointLocation.cpp

-//===-- StoppointLocation.cpp ---------------------------------------------===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-#include "lldb/Breakpoint/StoppointLocation.h"
-
-
-using namespace lldb;
-using namespace lldb_private;
-
-// StoppointLocation constructor
-StoppointLocation::StoppointLocation(break_id_t bid, addr_t addr, bool hardware)
-    : m_loc_id(bid), m_addr(addr), m_hardware(hardware),
-      m_hardware_index(LLDB_INVALID_INDEX32), m_byte_size(0), m_hit_count(0) {}
-
-StoppointLocation::StoppointLocation(break_id_t bid, addr_t addr,
-                                     uint32_t byte_size, bool hardware)
-    : m_loc_id(bid), m_addr(addr), m_hardware(hardware),
-      m_hardware_index(LLDB_INVALID_INDEX32), m_byte_size(byte_size),
-      m_hit_count(0) {}
-
-// Destructor
-StoppointLocation::~StoppointLocation() {}
-
-void StoppointLocation::DecrementHitCount() {
-  assert(m_hit_count > 0);
-  --m_hit_count;
-}

lldb/source/Breakpoint/StoppointSite.cpp

+//===-- StoppointSite.cpp ---------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "lldb/Breakpoint/StoppointSite.h"
+
+
+using namespace lldb;
+using namespace lldb_private;
+
+StoppointSite::StoppointSite(break_id_t id, addr_t addr, bool hardware)
+    : m_id(id), m_addr(addr), m_is_hardware_required(hardware),
+      m_hardware_index(LLDB_INVALID_INDEX32), m_byte_size(0), m_hit_counter() {}
+
+StoppointSite::StoppointSite(break_id_t id, addr_t addr,
+                             uint32_t byte_size, bool hardware)
+    : m_id(id), m_addr(addr), m_is_hardware_required(hardware),
+      m_hardware_index(LLDB_INVALID_INDEX32), m_byte_size(byte_size),
+      m_hit_counter() {}

lldb/source/Breakpoint/Watchpoint.cpp

 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/Stream.h"
 
 using namespace lldb;
 using namespace lldb_private;
 
 Watchpoint::Watchpoint(Target &target, lldb::addr_t addr, uint32_t size,
                        const CompilerType *type, bool hardware)
-    : StoppointLocation(0, addr, size, hardware), m_target(target),
+    : StoppointSite(0, addr, size, hardware), m_target(target),
       m_enabled(false), m_is_hardware(hardware), m_is_watch_variable(false),
       m_is_ephemeral(false), m_disabled_count(0), m_watch_read(0),
       m_watch_write(0), m_watch_was_read(0), m_watch_was_written(0),
       m_ignore_count(0), m_false_alarms(0), m_decl_str(), m_watch_spec_str(),
       m_type(), m_error(), m_options(), m_being_created(true) {
 
   if (type && type->IsValid())
     m_type = *type;

 void Watchpoint::SetDeclInfo(const std::string &str) { m_decl_str = str; }
 
 std::string Watchpoint::GetWatchSpec() { return m_watch_spec_str; }
 
 void Watchpoint::SetWatchSpec(const std::string &str) {
   m_watch_spec_str = str;
 }
 
-// Override default impl of StoppointLocation::IsHardware() since m_is_hardware
-// member field is more accurate.
 bool Watchpoint::IsHardware() const {
   lldbassert(m_is_hardware || !HardwareRequired());
   return m_is_hardware;
 }
 
 bool Watchpoint::IsWatchVariable() const { return m_is_watch_variable; }
 
 void Watchpoint::SetWatchVariable(bool val) { m_is_watch_variable = val; }
m_new_value_sp = ValueObjectMemory::Create(
       watch_address, m_type);
   m_new_value_sp = m_new_value_sp->CreateConstantValue(watch_name);
   return (m_new_value_sp && m_new_value_sp->GetError().Success());
 }
 
 void Watchpoint::IncrementFalseAlarmsAndReviseHitCount() {
   ++m_false_alarms;
   if (m_false_alarms) {
-    if (m_hit_count >= m_false_alarms) {
-      m_hit_count -= m_false_alarms;
+    if (m_hit_counter.GetValue() >= m_false_alarms) {
+      m_hit_counter.Decrement(m_false_alarms);
       m_false_alarms = 0;
     } else {
-      m_false_alarms -= m_hit_count;
-      m_hit_count = 0;
+      m_false_alarms -= m_hit_counter.GetValue();
+      m_hit_counter.Reset();
     }
   }
 }
 
 // RETURNS - true if we should stop at this breakpoint, false if we
 // should continue.
 
 bool Watchpoint::ShouldStop(StoppointCallbackContext *context) {
-  IncrementHitCount();
+  m_hit_counter.Increment();
 
   return IsEnabled();
 }
 
 void Watchpoint::GetDescription(Stream *s, lldb::DescriptionLevel level) {
   DumpWithLevel(s, level);
 }