The breakpoint location hit counts were getting incremented in

BreakpointLocation::ShouldStop.  That worked but wasn't really right,
since there's nothing to guarantee that won't get called more than
once.  So this change moves that responsibility to the StopInfoBreakpoint
directly, and then it uses the BreakpointSite to actually do the bumping.

Also fix a test case that was assuming if you had many threads running some 
code with a breakpoint in it, the hit count when you stopped would always be
1.  Many of the threads could have hit it at the same time...

<rdar://problem/18577603>

llvm-svn: 220358

4 files changed, 31 insertions, 3 deletions

diff --git a/lldb/source/Breakpoint/BreakpointLocation.cpp b/lldb/source/Breakpoint/BreakpointLocation.cpp
index 02d9609e248..11ecfecc5bc 100644
--- a/lldb/source/Breakpoint/BreakpointLocation.cpp
+++ b/lldb/source/Breakpoint/BreakpointLocation.cpp
@@ -449,8 +449,7 @@ BreakpointLocation::ShouldStop (StoppointCallbackContext *context)
     bool should_stop = true;
     Log *log = lldb_private::GetLogIfAllCategoriesSet (LIBLLDB_LOG_BREAKPOINTS);
 
-    IncrementHitCount();
-
+    // Do this first, if a location is disabled, it shouldn't increment its hit count.
     if (!IsEnabled())
         return false;
 
@@ -474,6 +473,13 @@ BreakpointLocation::ShouldStop (StoppointCallbackContext *context)
     return should_stop;
 }
 
+void
+BreakpointLocation::BumpHitCount()
+{
+    if (IsEnabled())
+        IncrementHitCount();
+}
+
 bool
 BreakpointLocation::IsResolved () const
 {
diff --git a/lldb/source/Breakpoint/BreakpointLocationCollection.cpp b/lldb/source/Breakpoint/BreakpointLocationCollection.cpp
index ee3f56f928d..5756ccedfaa 100644
--- a/lldb/source/Breakpoint/BreakpointLocationCollection.cpp
+++ b/lldb/source/Breakpoint/BreakpointLocationCollection.cpp
@@ -196,3 +196,4 @@ BreakpointLocationCollection::GetDescription (Stream *s, lldb::DescriptionLevel
         (*pos)->GetDescription(s, level);
     }
 }
+
diff --git a/lldb/source/Breakpoint/BreakpointSite.cpp b/lldb/source/Breakpoint/BreakpointSite.cpp
index 3cf6d37af37..469514b03f8 100644
--- a/lldb/source/Breakpoint/BreakpointSite.cpp
+++ b/lldb/source/Breakpoint/BreakpointSite.cpp
@@ -199,6 +199,15 @@ BreakpointSite::ValidForThisThread (Thread *thread)
     return m_owners.ValidForThisThread(thread);
 }
 
+void
+BreakpointSite::BumpHitCounts()
+{
+    for (BreakpointLocationSP loc_sp : m_owners.BreakpointLocations())
+    {
+        loc_sp->BumpHitCount();
+    }
+}
+
 bool
 BreakpointSite::IntersectsRange(lldb::addr_t addr, size_t size, lldb::addr_t *intersect_addr, size_t *intersect_size, size_t *opcode_offset) const
 {
diff --git a/lldb/source/Target/StopInfo.cpp b/lldb/source/Target/StopInfo.cpp
index 99b1ac1f3e3..4b57ca65a2d 100644
--- a/lldb/source/Target/StopInfo.cpp
+++ b/lldb/source/Target/StopInfo.cpp
@@ -182,6 +182,7 @@ public:
                 {
                     ExecutionContext exe_ctx (thread_sp->GetStackFrameAtIndex(0));
                     StoppointCallbackContext context (event_ptr, exe_ctx, true);
+                    bp_site_sp->BumpHitCounts();
                     m_should_stop = bp_site_sp->ShouldStop (&context);
                 }
                 else
@@ -403,10 +404,21 @@ protected:
 
                     // Let's copy the breakpoint locations out of the site and store them in a local list.  That way if
                     // one of the breakpoint actions changes the site, then we won't be operating on a bad list.
+                    // For safety's sake let's also grab an extra reference to the breakpoint owners of the locations we're
+                    // going to examine, since the locations are going to have to get back to their breakpoints, and the
+                    // locations don't keep their owners alive.  I'm just sticking the BreakpointSP's in a vector since
+                    // I'm only really using it to locally increment their retain counts.
 
                     BreakpointLocationCollection site_locations;
+                    std::vector<lldb::BreakpointSP> location_owners;
+
                     for (size_t j = 0; j < num_owners; j++)
-                        site_locations.Add(bp_site_sp->GetOwnerAtIndex(j));
+                    {
+                        BreakpointLocationSP loc(bp_site_sp->GetOwnerAtIndex(j));
+                        site_locations.Add(loc);
+                        location_owners.push_back(loc->GetBreakpoint().shared_from_this());
+
+                    }
 
                     for (size_t j = 0; j < num_owners; j++)
                     {