Make sure that when a breakpoint is hit but its condition is not met,

the hit count is not updated.
Also, keep the hit count for the breakpoint in the breakpoint.  We were
using just the sum of the location's hit counts, but that was wrong since if a shared library is
unloaded, and the location goes away, the breakpoint hit count should not suddenly drop
by the number of hits there were on that location.

llvm-svn: 226074

8 files changed, 60 insertions, 8 deletions

diff --git a/lldb/include/lldb/Breakpoint/Breakpoint.h b/lldb/include/lldb/Breakpoint/Breakpoint.h
index 61acc061aeb..883571a3ce9 100644
--- a/lldb/include/lldb/Breakpoint/Breakpoint.h
+++ b/lldb/include/lldb/Breakpoint/Breakpoint.h
@@ -714,6 +714,19 @@ protected:
     bool
     IgnoreCountShouldStop ();
 
+    void
+    IncrementHitCount()
+    {
+        m_hit_count++;
+    }
+
+    void
+    DecrementHitCount()
+    {
+        assert (m_hit_count > 0);
+        m_hit_count--;
+    }
+
 private:
     // This one should only be used by Target to copy breakpoints from target to target - primarily from the dummy
     // target to prime new targets.
@@ -733,7 +746,10 @@ private:
     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.
+
     void
     SendBreakpointChangedEvent (lldb::BreakpointEventType eventKind);
     
diff --git a/lldb/include/lldb/Breakpoint/BreakpointLocation.h b/lldb/include/lldb/Breakpoint/BreakpointLocation.h
index 8d5ebce411d..64225638678 100644
--- a/lldb/include/lldb/Breakpoint/BreakpointLocation.h
+++ b/lldb/include/lldb/Breakpoint/BreakpointLocation.h
@@ -390,6 +390,7 @@ 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.
@@ -417,6 +418,9 @@ private:
     void
     BumpHitCount();
 
+    void
+    UndoBumpHitCount();
+
 
     //------------------------------------------------------------------
     // Constructors and Destructors
diff --git a/lldb/include/lldb/Breakpoint/StoppointLocation.h b/lldb/include/lldb/Breakpoint/StoppointLocation.h
index 452c6388c82..32a1dbd4386 100644
--- a/lldb/include/lldb/Breakpoint/StoppointLocation.h
+++ b/lldb/include/lldb/Breakpoint/StoppointLocation.h
@@ -134,6 +134,9 @@ protected:
         ++m_hit_count;
     }
 
+    void
+    DecrementHitCount ();
+
 private:
     //------------------------------------------------------------------
     // For StoppointLocation only
diff --git a/lldb/source/Breakpoint/Breakpoint.cpp b/lldb/source/Breakpoint/Breakpoint.cpp
index bc269cdb95a..beb0f6bc5a6 100644
--- a/lldb/source/Breakpoint/Breakpoint.cpp
+++ b/lldb/source/Breakpoint/Breakpoint.cpp
@@ -60,7 +60,8 @@ Breakpoint::Breakpoint(Target &target,
     m_resolver_sp (resolver_sp),
     m_options (),
     m_locations (*this),
-    m_resolve_indirect_symbols(resolve_indirect_symbols)
+    m_resolve_indirect_symbols(resolve_indirect_symbols),
+    m_hit_count(0)
 {
     m_being_created = false;
 }
@@ -72,7 +73,8 @@ Breakpoint::Breakpoint (Target &new_target, Breakpoint &source_bp) :
     m_name_list (source_bp.m_name_list),
     m_options (source_bp.m_options),
     m_locations(*this),
-    m_resolve_indirect_symbols(source_bp.m_resolve_indirect_symbols)
+    m_resolve_indirect_symbols(source_bp.m_resolve_indirect_symbols),
+    m_hit_count(0)
 {
     // Now go through and copy the filter & resolver:
     m_resolver_sp = source_bp.m_resolver_sp->CopyForBreakpoint(*this);
@@ -207,7 +209,7 @@ Breakpoint::IgnoreCountShouldStop ()
 uint32_t
 Breakpoint::GetHitCount () const
 {
-    return m_locations.GetHitCount();
+    return m_hit_count;
 }
 
 bool
diff --git a/lldb/source/Breakpoint/BreakpointLocation.cpp b/lldb/source/Breakpoint/BreakpointLocation.cpp
index 11ecfecc5bc..85233c9374c 100644
--- a/lldb/source/Breakpoint/BreakpointLocation.cpp
+++ b/lldb/source/Breakpoint/BreakpointLocation.cpp
@@ -477,7 +477,22 @@ void
 BreakpointLocation::BumpHitCount()
 {
     if (IsEnabled())
+    {
+        // Step our hit count, and also step the hit count of the owner.
         IncrementHitCount();
+        m_owner.IncrementHitCount();
+    }
+}
+
+void
+BreakpointLocation::UndoBumpHitCount()
+{
+    if (IsEnabled())
+    {
+        // Step our hit count, and also step the hit count of the owner.
+        DecrementHitCount();
+        m_owner.DecrementHitCount();
+    }
 }
 
 bool
diff --git a/lldb/source/Breakpoint/StoppointLocation.cpp b/lldb/source/Breakpoint/StoppointLocation.cpp
index 9d8d9241253..35e5979bd9e 100644
--- a/lldb/source/Breakpoint/StoppointLocation.cpp
+++ b/lldb/source/Breakpoint/StoppointLocation.cpp
@@ -46,3 +46,10 @@ StoppointLocation::StoppointLocation (break_id_t bid, addr_t addr, uint32_t byte
 StoppointLocation::~StoppointLocation()
 {
 }
+
+void
+StoppointLocation::DecrementHitCount ()
+{
+    assert (m_hit_count > 0);
+    --m_hit_count;
+}
diff --git a/lldb/source/Target/StopInfo.cpp b/lldb/source/Target/StopInfo.cpp
index 4b57ca65a2d..457b94c1dc2 100644
--- a/lldb/source/Target/StopInfo.cpp
+++ b/lldb/source/Target/StopInfo.cpp
@@ -479,7 +479,12 @@ protected:
                                                  condition_says_stop);
                                 }
                                 if (!condition_says_stop)
+                                {
+                                    // We don't want to increment the hit count of breakpoints if the condition fails.
+                                    // We've already bumped it by the time we get here, so undo the bump:
+                                    bp_loc_sp->UndoBumpHitCount();
                                     continue;
+                                }
                             }
                         }
 
diff --git a/lldb/test/functionalities/breakpoint/breakpoint_conditions/TestBreakpointConditions.py b/lldb/test/functionalities/breakpoint/breakpoint_conditions/TestBreakpointConditions.py
index ea8606f9b10..ad8c295be3b 100644
--- a/lldb/test/functionalities/breakpoint/breakpoint_conditions/TestBreakpointConditions.py
+++ b/lldb/test/functionalities/breakpoint/breakpoint_conditions/TestBreakpointConditions.py
@@ -90,10 +90,10 @@ class BreakpointConditionsTestCase(TestBase):
             startstr = '(int) val = 3')
 
         # Also check the hit count, which should be 3, by design.
-        self.expect("breakpoint list -f", BREAKPOINT_HIT_THRICE,
+        self.expect("breakpoint list -f", BREAKPOINT_HIT_ONCE,
             substrs = ["resolved = 1",
                        "Condition: val == 3",
-                       "hit count = 3"])
+                       "hit count = 1"])
 
         # The frame #0 should correspond to main.c:36, the executable statement
         # in function name 'c'.  And the parent frame should point to main.c:24.
@@ -188,8 +188,8 @@ class BreakpointConditionsTestCase(TestBase):
         self.assertTrue(frame0.GetLineEntry().GetLine() == self.line1 and
                         var.GetValue() == '3')
 
-        # The hit count for the breakpoint should be 3.
-        self.assertTrue(breakpoint.GetHitCount() == 3)
+        # The hit count for the breakpoint should be 1.
+        self.assertTrue(breakpoint.GetHitCount() == 1)
 
         process.Continue()