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;
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;
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
