[lldb] Don't use hardware index to determine whether a breakpoint site is hardware

Most process plugins (if not all) don't set hardware index for breakpoints. They even
are not able to determine this index.

This patch makes StoppointLocation::IsHardware pure virtual and lets BreakpointSite
override it using more accurate BreakpointSite::Type.

It also adds assertions to be sure that a breakpoint site is hardware when this is required.

Differential Revision: https://reviews.llvm.org/D84257

Author: tkrasnukha

lldb/include/lldb/Breakpoint/BreakpointLocation.h

@@ -76,6 +76,12 @@ class BreakpointLocation
   ///     \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);
 

lldb/include/lldb/Breakpoint/BreakpointSite.h

@@ -15,6 +15,7 @@
 
 #include "lldb/Breakpoint/BreakpointLocationCollection.h"
 #include "lldb/Breakpoint/StoppointLocation.h"
+#include "lldb/Utility/LLDBAssert.h"
 #include "lldb/Utility/UserID.h"
 #include "lldb/lldb-forward.h"
 
@@ -184,6 +185,12 @@ class BreakpointSite : public std::enable_shared_from_this<BreakpointSite>,
   ///     \b false otherwise.
   bool IsInternal() const;
 
+  bool IsHardware() const override {
+    lldbassert(BreakpointSite::Type::eHardware == GetType() ||
+               !HardwareRequired());
+    return BreakpointSite::Type::eHardware == GetType();
+  }
+
   BreakpointSite::Type GetType() const { return m_type; }
 
   void SetType(BreakpointSite::Type type) { m_type = type; }

lldb/include/lldb/Breakpoint/StoppointLocation.h

@@ -40,9 +40,7 @@ class StoppointLocation {
 
   bool HardwareRequired() const { return m_hardware; }
 
-  virtual bool IsHardware() const {
-    return m_hardware_index != LLDB_INVALID_INDEX32;
-  }
+  virtual bool IsHardware() const = 0;
 
   virtual bool ShouldStop(StoppointCallbackContext *context) { return true; }
 

lldb/source/Breakpoint/BreakpointLocation.cpp

@@ -68,6 +68,14 @@ 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;

lldb/source/Breakpoint/Watchpoint.cpp

@@ -95,7 +95,10 @@ void Watchpoint::SetWatchSpec(const std::string &str) {
 
 // Override default impl of StoppointLocation::IsHardware() since m_is_hardware
 // member field is more accurate.
-bool Watchpoint::IsHardware() const { return m_is_hardware; }
+bool Watchpoint::IsHardware() const {
+  lldbassert(m_is_hardware || !HardwareRequired());
+  return m_is_hardware;
+}
 
 bool Watchpoint::IsWatchVariable() const { return m_is_watch_variable; }
 

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

@@ -3204,14 +3204,8 @@ Status ProcessGDBRemote::DisableBreakpointSite(BreakpointSite *bp_site) {
       break;
 
     case BreakpointSite::eExternal: {
-      GDBStoppointType stoppoint_type;
-      if (bp_site->IsHardware())
-        stoppoint_type = eBreakpointHardware;
-      else
-        stoppoint_type = eBreakpointSoftware;
-
-      if (m_gdb_comm.SendGDBStoppointTypePacket(stoppoint_type, false, addr,
-                                                bp_op_size))
+      if (m_gdb_comm.SendGDBStoppointTypePacket(eBreakpointSoftware, false,
+                                                addr, bp_op_size))
         error.SetErrorToGenericError();
     } break;
     }

lldb/test/API/functionalities/breakpoint/hardware_breakpoints/hardware_breakpoint_on_multiple_threads/TestHWBreakMultiThread.py

@@ -17,45 +17,20 @@ class HardwareBreakpointMultiThreadTestCase(HardwareBreakpointTestBase):
     def does_not_support_hw_breakpoints(self):
         return not super().supports_hw_breakpoints()
 
-    # LLDB on linux supports hardware breakpoints for arm and aarch64
-    # architectures.
-    @skipUnlessPlatform(oslist=['linux'])
-    @skipTestIfFn(does_not_support_hw_breakpoints)
-    def test_hw_break_set_delete_multi_thread_linux(self):
-        self.build()
-        self.setTearDownCleanup()
-        self.break_multi_thread('delete', False) # llvm.org/PR44659
-
-    # LLDB on linux supports hardware breakpoints for arm and aarch64
-    # architectures.
-    @skipUnlessPlatform(oslist=['linux'])
-    @skipTestIfFn(does_not_support_hw_breakpoints)
-    def test_hw_break_set_disable_multi_thread_linux(self):
-        self.build()
-        self.setTearDownCleanup()
-        self.break_multi_thread('disable', False) # llvm.org/PR44659
-
-    # LLDB on darwin supports hardware breakpoints for x86_64 and i386
-    # architectures.
-    @skipUnlessDarwin
     @skipIfOutOfTreeDebugserver
     @skipTestIfFn(does_not_support_hw_breakpoints)
     def test_hw_break_set_delete_multi_thread_macos(self):
         self.build()
         self.setTearDownCleanup()
         self.break_multi_thread('delete')
 
-    # LLDB on darwin supports hardware breakpoints for x86_64 and i386
-    # architectures.
-    @skipUnlessDarwin
     @skipIfOutOfTreeDebugserver
     @skipTestIfFn(does_not_support_hw_breakpoints)
     def test_hw_break_set_disable_multi_thread_macos(self):
         self.build()
         self.setTearDownCleanup()
         self.break_multi_thread('disable')
 
-
     def setUp(self):
         # Call super's setUp().
         TestBase.setUp(self)
@@ -65,7 +40,7 @@ def setUp(self):
         self.first_stop = line_number(
             self.source, 'Starting thread creation with hardware breakpoint set')
 
-    def break_multi_thread(self, removal_type, check_hw_bp=True):
+    def break_multi_thread(self, removal_type):
         """Test that lldb hardware breakpoints work for multiple threads."""
         self.runCmd("file " + self.getBuildArtifact("a.out"),
                     CURRENT_EXECUTABLE_SET)
@@ -109,10 +84,9 @@ def break_multi_thread(self, removal_type, check_hw_bp=True):
             # Continue the loop and test that we are stopped 4 times.
             count += 1
 
-        if check_hw_bp:
-            # Check the breakpoint list.
-            self.expect("breakpoint list", substrs=['hw_break_function', 'hardware'])
-            self.expect("breakpoint list -v", substrs=['function = hw_break_function', 'hardware = true'])
+        # Check the breakpoint list.
+        self.expect("breakpoint list", substrs=['hw_break_function', 'hardware'])
+        self.expect("breakpoint list -v", substrs=['function = hw_break_function', 'hardware = true'])
 
         if removal_type == 'delete':
             self.runCmd("settings set auto-confirm true")