Remove hardware index from watchpoints and breakpoints

lldb/bindings/interface/SBTargetDocstrings.i

@@ -34,7 +34,7 @@ produces: ::
 
     Watchpoint 1: addr = 0x1034ca048 size = 4 state = enabled type = rw
         declare @ '/Volumes/data/lldb/svn/trunk/test/python_api/watchpoint/main.c:12'
-        hw_index = 0  hit_count = 2     ignore_count = 0"
+        hit_count = 2     ignore_count = 0"
 ) lldb::SBTarget;
 
 %feature("docstring", "

lldb/bindings/interface/SBWatchpointDocstrings.i

@@ -9,7 +9,8 @@ watchpoints of the target."
 ) lldb::SBWatchpoint;
 
 %feature("docstring", "
-    With -1 representing an invalid hardware index."
+    Deprecated.  Previously: Return the hardware index of the 
+    watchpoint register.  Now: -1 is always returned."
 ) lldb::SBWatchpoint::GetHardwareIndex;
 
 %feature("docstring", "

lldb/docs/use/tutorial.rst

@@ -431,7 +431,7 @@ a variable called 'global' for write operation, but only stop if the condition
    Watchpoint 1: addr = 0x100001018 size = 4 state = enabled type = w
       declare @ '/Volumes/data/lldb/svn/ToT/test/functionalities/watchpoint/watchpoint_commands/condition/main.cpp:12'
       condition = '(global==5)'
-      hw_index = 0  hit_count = 5     ignore_count = 0
+      hit_count = 5     ignore_count = 0
    (lldb)
 
 Starting or Attaching to Your Program

lldb/include/lldb/API/SBWatchpoint.h

@@ -45,7 +45,7 @@ class LLDB_API SBWatchpoint {
 
   watch_id_t GetID();
 
-  /// With -1 representing an invalid hardware index.
+  LLDB_DEPRECATED("Hardware index is not available, always returns -1")
   int32_t GetHardwareIndex();
 
   lldb::addr_t GetWatchAddress();

lldb/include/lldb/Breakpoint/StoppointSite.h

@@ -38,10 +38,6 @@ class StoppointSite {
 
   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;
@@ -59,9 +55,6 @@ class StoppointSite {
   /// 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.

lldb/source/API/SBWatchpoint.cpp

@@ -94,16 +94,11 @@ SBError SBWatchpoint::GetError() {
 int32_t SBWatchpoint::GetHardwareIndex() {
   LLDB_INSTRUMENT_VA(this);
 
-  int32_t hw_index = -1;
-
-  lldb::WatchpointSP watchpoint_sp(GetSP());
-  if (watchpoint_sp) {
-    std::lock_guard<std::recursive_mutex> guard(
-        watchpoint_sp->GetTarget().GetAPIMutex());
-    hw_index = watchpoint_sp->GetHardwareIndex();
-  }
-
-  return hw_index;
+  // For processes using gdb remote protocol,
+  // we cannot determine the hardware breakpoint
+  // index reliably; providing possibly correct
+  // guesses is not useful to anyone.
+  return -1;
 }
 
 addr_t SBWatchpoint::GetWatchAddress() {

lldb/source/Breakpoint/BreakpointLocation.cpp

@@ -626,22 +626,19 @@ void BreakpointLocation::Dump(Stream *s) const {
 
   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",
+            "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",
-            is_hardware ? "hardware" : "software", hardware_index,
-            GetHitCount(),
+            is_hardware ? "hardware" : "software", GetHitCount(),
             GetOptionsSpecifyingKind(BreakpointOptions::eIgnoreCount)
                 .GetIgnoreCount());
 }

lldb/source/Breakpoint/BreakpointSite.cpp

@@ -76,9 +76,9 @@ void BreakpointSite::Dump(Stream *s) const {
     return;
 
   s->Printf("BreakpointSite %u: addr = 0x%8.8" PRIx64
-            "  type = %s breakpoint  hw_index = %i  hit_count = %-4u",
+            "  type = %s breakpoint  hit_count = %-4u",
             GetID(), (uint64_t)m_addr, IsHardware() ? "hardware" : "software",
-            GetHardwareIndex(), GetHitCount());
+            GetHitCount());
 }
 
 void BreakpointSite::GetDescription(Stream *s, lldb::DescriptionLevel level) {

lldb/source/Breakpoint/StoppointSite.cpp

@@ -13,11 +13,10 @@ 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() {}
+    : m_id(id), m_addr(addr), m_is_hardware_required(hardware), m_byte_size(0),
+      m_hit_counter() {}
 
-StoppointSite::StoppointSite(break_id_t id, addr_t addr,
-                             uint32_t byte_size, bool hardware)
+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() {}
+      m_byte_size(byte_size), m_hit_counter() {}

lldb/source/Breakpoint/Watchpoint.cpp

@@ -324,8 +324,8 @@ void Watchpoint::DumpWithLevel(Stream *s,
   }
 
   if (description_level >= lldb::eDescriptionLevelVerbose) {
-    s->Printf("\n    hw_index = %i  hit_count = %-4u  ignore_count = %-4u",
-              GetHardwareIndex(), GetHitCount(), GetIgnoreCount());
+    s->Printf("\n    hit_count = %-4u  ignore_count = %-4u", GetHitCount(),
+              GetIgnoreCount());
   }
 }
 
@@ -350,9 +350,7 @@ bool Watchpoint::IsDisabledDuringEphemeralMode() {
 
 void Watchpoint::SetEnabled(bool enabled, bool notify) {
   if (!enabled) {
-    if (!m_is_ephemeral)
-      SetHardwareIndex(LLDB_INVALID_INDEX32);
-    else
+    if (m_is_ephemeral)
       ++m_disabled_count;
 
     // Don't clear the snapshots for now.

lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp

@@ -495,10 +495,6 @@ static StopInfoSP GetStopInfoForHardwareBP(Thread &thread, Target *target,
     lldb::WatchpointSP wp_sp =
         target->GetWatchpointList().FindByAddress((lldb::addr_t)exc_sub_code);
     if (wp_sp && wp_sp->IsEnabled()) {
-      // Debugserver may piggyback the hardware index of the fired watchpoint
-      // in the exception data. Set the hardware index if that's the case.
-      if (exc_data_count >= 3)
-        wp_sp->SetHardwareIndex((uint32_t)exc_sub_sub_code);
       return StopInfo::CreateStopReasonWithWatchpointID(thread, wp_sp->GetID());
     }
   }
@@ -511,10 +507,6 @@ static StopInfoSP GetStopInfoForHardwareBP(Thread &thread, Target *target,
         process_sp->GetBreakpointSiteList().FindByAddress(
             (lldb::addr_t)exc_sub_code);
     if (bp_sp && bp_sp->IsEnabled()) {
-      // Debugserver may piggyback the hardware index of the fired breakpoint
-      // in the exception data. Set the hardware index if that's the case.
-      if (exc_data_count >= 3)
-        bp_sp->SetHardwareIndex((uint32_t)exc_sub_sub_code);
       return StopInfo::CreateStopReasonWithBreakpointSiteID(thread,
                                                             bp_sp->GetID());
     }
@@ -681,11 +673,6 @@ StopInfoSP StopInfoMachException::CreateStopReasonWithMachException(
           wp_sp = target->GetWatchpointList().FindByAddress(
               (lldb::addr_t)exc_sub_code);
         if (wp_sp && wp_sp->IsEnabled()) {
-          // Debugserver may piggyback the hardware index of the fired
-          // watchpoint in the exception data. Set the hardware index if
-          // that's the case.
-          if (exc_data_count >= 3)
-            wp_sp->SetHardwareIndex((uint32_t)exc_sub_sub_code);
           return StopInfo::CreateStopReasonWithWatchpointID(thread,
                                                             wp_sp->GetID());
         } else {
@@ -762,11 +749,6 @@ StopInfoSP StopInfoMachException::CreateStopReasonWithMachException(
           wp_sp = target->GetWatchpointList().FindByAddress(
               (lldb::addr_t)exc_sub_code);
         if (wp_sp && wp_sp->IsEnabled()) {
-          // Debugserver may piggyback the hardware index of the fired
-          // watchpoint in the exception data. Set the hardware index if
-          // that's the case.
-          if (exc_data_count >= 3)
-            wp_sp->SetHardwareIndex((uint32_t)exc_sub_sub_code);
           return StopInfo::CreateStopReasonWithWatchpointID(thread,
                                                             wp_sp->GetID());
         }

lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp

@@ -405,10 +405,6 @@ void ProcessWindows::RefreshStateAfterStop() {
                "{1:x} with watchpoint {2}",
                m_session_data->m_debugger->GetProcess().GetProcessId(), pc, id);
 
-      if (lldb::WatchpointSP wp_sp =
-              GetTarget().GetWatchpointList().FindByID(id))
-        wp_sp->SetHardwareIndex(slot_id);
-
       stop_info = StopInfo::CreateStopReasonWithWatchpointID(
           *stop_thread, id, m_watchpoints[id].address);
       stop_thread->SetStopInfo(stop_info);

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

@@ -1789,8 +1789,12 @@ ThreadSP ProcessGDBRemote::SetThreadStopInfo(
           // disable/step/re-enable it, so one of the valid watchpoint
           // addresses should be provided as \a wp_addr.
           StringExtractor desc_extractor(description.c_str());
+          // FIXME NativeThreadLinux::SetStoppedByWatchpoint sends this
+          // up as
+          //  <address within wp range> <wp hw index> <actual accessed addr>
+          // but this is not reading the <wp hw index>.  Seems like it
+          // wouldn't work on MIPS, where that third field is important.
           addr_t wp_addr = desc_extractor.GetU64(LLDB_INVALID_ADDRESS);
-          uint32_t wp_index = desc_extractor.GetU32(LLDB_INVALID_INDEX32);
           addr_t wp_hit_addr = desc_extractor.GetU64(LLDB_INVALID_ADDRESS);
           watch_id_t watch_id = LLDB_INVALID_WATCH_ID;
           bool silently_continue = false;
@@ -1807,7 +1811,6 @@ ThreadSP ProcessGDBRemote::SetThreadStopInfo(
           if (!wp_sp && wp_addr != LLDB_INVALID_ADDRESS)
             wp_sp = GetTarget().GetWatchpointList().FindByAddress(wp_addr);
           if (wp_sp) {
-            wp_sp->SetHardwareIndex(wp_index);
             watch_id = wp_sp->GetID();
           }
           if (watch_id == LLDB_INVALID_WATCH_ID) {
@@ -2238,17 +2241,13 @@ StateType ProcessGDBRemote::SetThreadStopInfo(StringExtractor &stop_packet) {
 
         WatchpointSP wp_sp =
             GetTarget().GetWatchpointList().FindByAddress(wp_addr);
-        uint32_t wp_index = LLDB_INVALID_INDEX32;
-
-        if (wp_sp)
-          wp_index = wp_sp->GetHardwareIndex();
 
         // Rewrite gdb standard watch/rwatch/awatch to
         // "reason:watchpoint" + "description:ADDR",
         // which is parsed in SetThreadStopInfo.
         reason = "watchpoint";
         StreamString ostr;
-        ostr.Printf("%" PRIu64 " %" PRIu32, wp_addr, wp_index);
+        ostr.Printf("%" PRIu64, wp_addr);
         description = std::string(ostr.GetString());
       } else if (key.compare("library") == 0) {
         auto error = LoadModules();

lldb/source/Target/StopInfo.cpp

@@ -699,7 +699,6 @@ class StopInfoWatchpoint : public StopInfo {
                                     eVoteNoOpinion),
           m_stop_info_sp(stop_info_sp), m_watch_sp(watch_sp) {
       assert(watch_sp);
-      m_watch_index = watch_sp->GetHardwareIndex();
     }
 
     bool DoWillResume(lldb::StateType resume_state,
@@ -753,13 +752,11 @@ class StopInfoWatchpoint : public StopInfo {
         return;
       m_did_disable_wp = true;
       GetThread().GetProcess()->EnableWatchpoint(m_watch_sp.get(), true);
-      m_watch_sp->SetHardwareIndex(m_watch_index);
     }
 
   private:
     StopInfoWatchpointSP m_stop_info_sp;
     WatchpointSP m_watch_sp;
-    uint32_t m_watch_index = LLDB_INVALID_INDEX32;
     bool m_did_disable_wp = false;
   };
 

lldb/test/API/python_api/default-constructor/sb_watchpoint.py

@@ -8,7 +8,6 @@
 def fuzz_obj(obj):
     obj.GetID()
     obj.IsValid()
-    obj.GetHardwareIndex()
     obj.GetWatchAddress()
     obj.GetWatchSize()
     obj.SetEnabled(True)

lldb/test/API/python_api/watchpoint/TestWatchpointIter.py

@@ -83,20 +83,12 @@ def test_watch_iter(self):
         self.assertTrue(thread, "The thread stopped due to watchpoint")
         self.DebugSBValue(value)
 
-        # We currently only support hardware watchpoint.  Verify that we have a
-        # meaningful hardware index at this point.  Exercise the printed repr of
-        # SBWatchpointLocation.
-        print(watchpoint)
-        if not self.affected_by_radar_34564183():
-            self.assertTrue(watchpoint.GetHardwareIndex() != -1)
-
         # SBWatchpoint.GetDescription() takes a description level arg.
         print(lldbutil.get_description(watchpoint, lldb.eDescriptionLevelFull))
 
         # Now disable the 'rw' watchpoint.  The program won't stop when it reads
         # 'global' next.
         watchpoint.SetEnabled(False)
-        self.assertEqual(watchpoint.GetHardwareIndex(), -1)
         self.assertFalse(watchpoint.IsEnabled())
 
         # Continue.  The program does not stop again when the variable is being

llvm/docs/ReleaseNotes.rst

@@ -226,6 +226,8 @@ Changes to the LLVM tools
 Changes to LLDB
 ---------------------------------
 
+* ``SBWatchpoint::GetHardwareIndex`` is deprecated and now returns -1
+  to indicate the index is unavailable.
 * Methods in SBHostOS related to threads have had their implementations
   removed. These methods will return a value indicating failure.
 * ``SBType::FindDirectNestedType`` function is added. It's useful