[Lldb-commits] [PATCH] D155768: [lldb] [NFC] Remove some dead code from Watchpoint class, and a method that makes no sense

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda created this revision.
jasonmolenda added a project: LLDB.
Herald added subscribers: JDevlieghere, atanasyan, kristof.beyls, arichardson, sdardis.
Herald added a project: All.
jasonmolenda requested review of this revision.
Herald added a subscriber: lldb-commits.

Johnny Chen added this code to Watchpoint originally in 2012 via 25c0eb4a38d20a4ac37714312cfcabdd082ff74f / llvm-svn: 161892 including the method `Watchpoint::IncrementFalseAlarmsAndReviseHitCount`.  Most of the code from this change has slowly been removed over the years, but that method remained, and was used for watchpoints hit on a MIPS target where the access was *near* a watchpoint, but not actually within a watched region (the hardware doesn't distinguish the low 3 bits, and lldb has an emulation of the LD/ST instruction to disambiguate).  The MIPS case was trying to use this method to decrement the hit count, which had already been incremented earlier when the watchpoint hit was first broadcast.  The method is doing signed math on unsigned counters and I can't honestly tell what it was meant to be doing in the first place.

We have a method to decrement the hit count.  Use it.

Also remove a couple of ivars in Watchpoint that aren't used anywhere.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D155768

Files:
  lldb/include/lldb/Breakpoint/Watchpoint.h
  lldb/source/Breakpoint/Watchpoint.cpp
  lldb/source/Target/StopInfo.cpp


Index: lldb/source/Target/StopInfo.cpp
===================================================================
--- lldb/source/Target/StopInfo.cpp
+++ lldb/source/Target/StopInfo.cpp
@@ -894,7 +894,7 @@
 
         if (m_silently_skip_wp) {
           m_should_stop = false;
-          wp_sp->IncrementFalseAlarmsAndReviseHitCount();
+          wp_sp->UndoHitCount();
         }
 
         if (wp_sp->GetHitCount() <= wp_sp->GetIgnoreCount()) {
Index: lldb/source/Breakpoint/Watchpoint.cpp
===================================================================
--- lldb/source/Breakpoint/Watchpoint.cpp
+++ lldb/source/Breakpoint/Watchpoint.cpp
@@ -29,8 +29,7 @@
     : 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_being_created(true) {
+      m_watch_write(0), m_ignore_count(0), m_being_created(true) {
 
   if (type && type->IsValid())
     m_type = *type;
@@ -212,19 +211,6 @@
   return (m_new_value_sp && m_new_value_sp->GetError().Success());
 }
 
-void Watchpoint::IncrementFalseAlarmsAndReviseHitCount() {
-  ++m_false_alarms;
-  if (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_counter.GetValue();
-      m_hit_counter.Reset();
-    }
-  }
-}
-
 // RETURNS - true if we should stop at this breakpoint, false if we
 // should continue.
 
Index: lldb/include/lldb/Breakpoint/Watchpoint.h
===================================================================
--- lldb/include/lldb/Breakpoint/Watchpoint.h
+++ lldb/include/lldb/Breakpoint/Watchpoint.h
@@ -63,8 +63,6 @@
 
   ~Watchpoint() override;
 
-  void IncrementFalseAlarmsAndReviseHitCount();
-
   bool IsEnabled() const;
 
   // This doesn't really enable/disable the watchpoint.   It is currently just
@@ -214,12 +212,8 @@
   // again, we check the count, if it is more than 1, it means the user-
   // supplied actions actually want the watchpoint to be disabled!
   uint32_t m_watch_read : 1, // 1 if we stop when the watched data is read from
-      m_watch_write : 1,     // 1 if we stop when the watched data is written to
-      m_watch_was_read : 1, // Set to 1 when watchpoint is hit for a read access
-      m_watch_was_written : 1;  // Set to 1 when watchpoint is hit for a write
-                                // access
+      m_watch_write : 1;     // 1 if we stop when the watched data is written to
   uint32_t m_ignore_count;      // Number of times to ignore this watchpoint
-  uint32_t m_false_alarms;      // Number of false alarms.
   std::string m_decl_str;       // Declaration information, if any.
   std::string m_watch_spec_str; // Spec for the watchpoint.
   lldb::ValueObjectSP m_old_value_sp;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D155768.542265.patch
Type: text/x-patch
Size: 3000 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20230720/4b7abca9/attachment-0001.bin>