Fix Breakpoint::RemoveInvalidLocations to fix the exec testcase.

RemoveInvalidLocations was clearing out the m_locations in the
breakpoint by hand, and it wasn't also clearing the locations from
the address->location map, which confused us when we went to update
breakpoint locations.  

I also made Breakpoint::ModulesChanged check the Location's Section
to make sure it hadn't been deleted.  This shouldn't strictly be necessary,
but if the DynamicLoaderPlugin doesn't do it's job right (I'm looking at
you new Darwin DynamicLoader...) then it can end up leaving stale locations
on rerun.  It doesn't hurt to clean them up here as a backstop.

<rdar://problem/36134350>

llvm-svn: 322348

2 files changed, 37 insertions, 8 deletions

diff --git a/lldb/source/Breakpoint/Breakpoint.cpp b/lldb/source/Breakpoint/Breakpoint.cpp
index 3f6c63e1e5b..ace9e832ee8 100644
--- a/lldb/source/Breakpoint/Breakpoint.cpp
+++ b/lldb/source/Breakpoint/Breakpoint.cpp
@@ -23,6 +23,7 @@
 #include "lldb/Core/ModuleList.h"
 #include "lldb/Core/SearchFilter.h"
 #include "lldb/Core/Section.h"
+#include "lldb/Target/SectionLoadList.h"
 #include "lldb/Symbol/CompileUnit.h"
 #include "lldb/Symbol/Function.h"
 #include "lldb/Symbol/Symbol.h"
@@ -535,12 +536,29 @@ void Breakpoint::ModulesChanged(ModuleList &module_list, bool load,
       if (!m_filter_sp->ModulePasses(module_sp))
         continue;
 
+      BreakpointLocationCollection locations_with_no_section;
       for (BreakpointLocationSP break_loc_sp :
            m_locations.BreakpointLocations()) {
+
+        // If the section for this location was deleted, that means
+        // it's Module has gone away but somebody forgot to tell us.
+        // Let's clean it up here.
+        Address section_addr(break_loc_sp->GetAddress());
+        if (section_addr.SectionWasDeleted()) {
+          locations_with_no_section.Add(break_loc_sp);
+          continue;
+        }
+          
         if (!break_loc_sp->IsEnabled())
           continue;
-        SectionSP section_sp(break_loc_sp->GetAddress().GetSection());
-        if (!section_sp || section_sp->GetModule() == module_sp) {
+        
+        SectionSP section_sp(section_addr.GetSection());
+        
+        // If we don't have a Section, that means this location is a raw address
+        // that we haven't resolved to a section yet.  So we'll have to look
+        // in all the new modules to resolve this location.
+        // Otherwise, if it was set in this module, re-resolve it here.
+        if (section_sp && section_sp->GetModule() == module_sp) {
           if (!seen)
             seen = true;
 
@@ -552,6 +570,11 @@ void Breakpoint::ModulesChanged(ModuleList &module_list, bool load,
           }
         }
       }
+      
+      size_t num_to_delete = locations_with_no_section.GetSize();
+      
+      for (size_t i = 0; i < num_to_delete; i++)
+        m_locations.RemoveLocation(locations_with_no_section.GetByIndex(i));
 
       if (!seen)
         new_modules.AppendIfNeeded(module_sp);
diff --git a/lldb/source/Breakpoint/BreakpointLocationList.cpp b/lldb/source/Breakpoint/BreakpointLocationList.cpp
index 1e4b4412a42..97175574e9d 100644
--- a/lldb/source/Breakpoint/BreakpointLocationList.cpp
+++ b/lldb/source/Breakpoint/BreakpointLocationList.cpp
@@ -246,10 +246,10 @@ bool BreakpointLocationList::RemoveLocation(
 
     m_address_to_location.erase(bp_loc_sp->GetAddress());
 
-    collection::iterator pos, end = m_locations.end();
-    for (pos = m_locations.begin(); pos != end; ++pos) {
-      if ((*pos).get() == bp_loc_sp.get()) {
-        m_locations.erase(pos);
+    size_t num_locations = m_locations.size();
+    for (size_t idx = 0; idx < num_locations; idx++) {
+      if (m_locations[idx].get() == bp_loc_sp.get()) {
+        RemoveLocationByIndex(idx);
         return true;
       }
     }
@@ -257,6 +257,12 @@ bool BreakpointLocationList::RemoveLocation(
   return false;
 }
 
+void BreakpointLocationList::RemoveLocationByIndex(size_t idx) {
+  assert (idx < m_locations.size());
+  m_address_to_location.erase(m_locations[idx]->GetAddress());
+  m_locations.erase(m_locations.begin() + idx);
+}
+
 void BreakpointLocationList::RemoveInvalidLocations(const ArchSpec &arch) {
   std::lock_guard<std::recursive_mutex> guard(m_mutex);
   size_t idx = 0;
@@ -267,7 +273,7 @@ void BreakpointLocationList::RemoveInvalidLocations(const ArchSpec &arch) {
     if (bp_loc->GetAddress().SectionWasDeleted()) {
       // Section was deleted which means this breakpoint comes from a module
       // that is no longer valid, so we should remove it.
-      m_locations.erase(m_locations.begin() + idx);
+      RemoveLocationByIndex(idx);
       continue;
     }
     if (arch.IsValid()) {
@@ -276,7 +282,7 @@ void BreakpointLocationList::RemoveInvalidLocations(const ArchSpec &arch) {
         if (!arch.IsCompatibleMatch(module_sp->GetArchitecture())) {
           // The breakpoint was in a module whose architecture is no longer
           // compatible with "arch", so we need to remove it
-          m_locations.erase(m_locations.begin() + idx);
+          RemoveLocationByIndex(idx);
           continue;
         }
       }