Fixed the the breakpoint test case failures.

There were 6 on darwin. All of these were related to the recent changes for exec.

llvm-svn: 194298

lldb/include/lldb/Breakpoint/Breakpoint.h

@@ -311,6 +311,24 @@ public:
     //------------------------------------------------------------------
     lldb::BreakpointLocationSP
     GetLocationAtIndex (size_t index);
+    
+    //------------------------------------------------------------------
+    /// Removes all invalid breakpoint locations.
+    ///
+    /// Removes all breakpoint locations with architectures that aren't
+    /// compatible with \a arch. Also remove any breakpoint locations
+    /// with whose locations have address where the section has been
+    /// deleted (module and object files no longer exist).
+    ///
+    /// This is typically used after the process calls exec, or anytime
+    /// the architecture of the target changes.
+    ///
+    /// @param[in] arch
+    ///     If valid, check the module in each breakpoint to make sure
+    ///     they are compatible, otherwise, ignore architecture.
+    //------------------------------------------------------------------
+    void
+    RemoveInvalidLocations (const ArchSpec &arch);
 
     //------------------------------------------------------------------
     // The next section deals with various breakpoint options.

lldb/include/lldb/Breakpoint/BreakpointList.h

@@ -132,6 +132,25 @@ public:
     bool
     Remove (lldb::break_id_t breakID, bool notify);
 
+    
+    //------------------------------------------------------------------
+    /// Removes all invalid breakpoint locations.
+    ///
+    /// Removes all breakpoint locations in the list with architectures
+    /// that aren't compatible with \a arch. Also remove any breakpoint
+    /// locations with whose locations have address where the section
+    /// has been deleted (module and object files no longer exist).
+    ///
+    /// This is typically used after the process calls exec, or anytime
+    /// the architecture of the target changes.
+    ///
+    /// @param[in] arch
+    ///     If valid, check the module in each breakpoint to make sure
+    ///     they are compatible, otherwise, ignore architecture.
+    //------------------------------------------------------------------
+    void
+    RemoveInvalidLocations (const ArchSpec &arch);
+
     void
     SetEnabledAll (bool enabled);
 

lldb/include/lldb/Breakpoint/BreakpointLocationList.h

@@ -250,6 +250,9 @@ protected:
 
     bool
     RemoveLocation (const lldb::BreakpointLocationSP &bp_loc_sp);
+    
+    void
+    RemoveInvalidLocations (const ArchSpec &arch);
 
     typedef std::vector<lldb::BreakpointLocationSP> collection;
     typedef std::map<lldb_private::Address,

lldb/include/lldb/Core/Address.h

@@ -534,6 +534,16 @@ public:
     bool
     CalculateSymbolContextLineEntry (LineEntry &line_entry) const;
 
+    //------------------------------------------------------------------
+    // Returns true if the section should be valid, but isn't because
+    // the shared pointer to the section can't be reconstructed from
+    // a weak pointer that contains a valid weak reference to a section.
+    // Returns false if the section weak pointer has no reference to
+    // a section, or if the section is still valid
+    //------------------------------------------------------------------
+    bool
+    SectionWasDeleted() const;
+
 protected:
     //------------------------------------------------------------------
     // Member variables.
@@ -550,7 +560,7 @@ protected:
     // have a valid section.
     //------------------------------------------------------------------
     bool
-    SectionWasDeleted() const;
+    SectionWasDeletedPrivate() const;
 
 };
 

lldb/include/lldb/Target/Target.h

@@ -759,8 +759,22 @@ public:
     SymbolsDidLoad (ModuleList &module_list);
     
     void
-    ClearModules();
+    ClearModules(bool delete_locations);
 
+    //------------------------------------------------------------------
+    /// Called as the last function in Process::DidExec().
+    ///
+    /// Process::DidExec() will clear a lot of state in the process,
+    /// then try to reload a dynamic loader plugin to discover what
+    /// binaries are currently available and then this function should
+    /// be called to allow the target to do any cleanup after everything
+    /// has been figured out. It can remove breakpoints that no longer
+    /// make sense as the exec might have changed the target
+    /// architecture, and unloaded some modules that might get deleted.
+    //------------------------------------------------------------------
+    void
+    DidExec ();
+    
     //------------------------------------------------------------------
     /// Gets the module for the main executable.
     ///

lldb/source/Breakpoint/Breakpoint.cpp

@@ -114,6 +114,12 @@ Breakpoint::GetLocationAtIndex (size_t index)
     return m_locations.GetByIndex(index);
 }
 
+void
+Breakpoint::RemoveInvalidLocations (const ArchSpec &arch)
+{
+    m_locations.RemoveInvalidLocations(arch);
+}
+
 // For each of the overall options we need to decide how they propagate to
 // the location options.  This will determine the precedence of options on
 // the breakpoint vs. its locations.

lldb/source/Breakpoint/BreakpointList.cpp

@@ -68,13 +68,21 @@ BreakpointList::Remove (break_id_t break_id, bool notify)
     return false;
 }
 
+void
+BreakpointList::RemoveInvalidLocations (const ArchSpec &arch)
+{
+    Mutex::Locker locker(m_mutex);
+    for (const auto &bp_sp : m_breakpoints)
+        bp_sp->RemoveInvalidLocations(arch);
+}
+
+
 void
 BreakpointList::SetEnabledAll (bool enabled)
 {
     Mutex::Locker locker(m_mutex);
-    bp_collection::iterator pos, end = m_breakpoints.end();
-    for (pos = m_breakpoints.begin(); pos != end; ++pos)
-        (*pos)->SetEnabled (enabled);
+    for (const auto &bp_sp : m_breakpoints)
+        bp_sp->SetEnabled (enabled);
 }
 
 
@@ -163,10 +171,8 @@ BreakpointList::Dump (Stream *s) const
     s->Indent();
     s->Printf("BreakpointList with %u Breakpoints:\n", (uint32_t)m_breakpoints.size());
     s->IndentMore();
-    bp_collection::const_iterator pos;
-    bp_collection::const_iterator end = m_breakpoints.end();
-    for (pos = m_breakpoints.begin(); pos != end; ++pos)
-        (*pos)->Dump(s);
+    for (const auto &bp_sp : m_breakpoints)
+        bp_sp->Dump(s);
     s->IndentLess();
 }
 
@@ -207,10 +213,8 @@ void
 BreakpointList::UpdateBreakpoints (ModuleList& module_list, bool added, bool delete_locations)
 {
     Mutex::Locker locker(m_mutex);
-    bp_collection::iterator end = m_breakpoints.end();
-    bp_collection::iterator pos;
-    for (pos = m_breakpoints.begin(); pos != end; ++pos)
-        (*pos)->ModulesChanged (module_list, added, delete_locations);
+    for (const auto &bp_sp : m_breakpoints)
+        bp_sp->ModulesChanged (module_list, added, delete_locations);
 
 }
 
@@ -218,10 +222,8 @@ void
 BreakpointList::UpdateBreakpointsWhenModuleIsReplaced (ModuleSP old_module_sp, ModuleSP new_module_sp)
 {
     Mutex::Locker locker(m_mutex);
-    bp_collection::iterator end = m_breakpoints.end();
-    bp_collection::iterator pos;
-    for (pos = m_breakpoints.begin(); pos != end; ++pos)
-        (*pos)->ModuleReplaced (old_module_sp, new_module_sp);
+    for (const auto &bp_sp : m_breakpoints)
+        bp_sp->ModuleReplaced (old_module_sp, new_module_sp);
 
 }
 
@@ -229,10 +231,8 @@ void
 BreakpointList::ClearAllBreakpointSites ()
 {
     Mutex::Locker locker(m_mutex);
-    bp_collection::iterator end = m_breakpoints.end();
-    bp_collection::iterator pos;
-    for (pos = m_breakpoints.begin(); pos != end; ++pos)
-        (*pos)->ClearAllBreakpointSites ();
+    for (const auto &bp_sp : m_breakpoints)
+        bp_sp->ClearAllBreakpointSites ();
 
 }
 

lldb/source/Breakpoint/BreakpointLocationList.cpp

@@ -13,8 +13,11 @@
 // Other libraries and framework includes
 // Project includes
 #include "lldb/Breakpoint/BreakpointLocationList.h"
+
 #include "lldb/Breakpoint/BreakpointLocation.h"
 #include "lldb/Breakpoint/Breakpoint.h"
+#include "lldb/Core/ArchSpec.h"
+#include "lldb/Core/Module.h"
 #include "lldb/Core/Section.h"
 #include "lldb/Target/Target.h"
 
@@ -286,7 +289,41 @@ BreakpointLocationList::RemoveLocation (const lldb::BreakpointLocationSP &bp_loc
     return false;
 }
 
-
+void
+BreakpointLocationList::RemoveInvalidLocations (const ArchSpec &arch)
+{
+    Mutex::Locker locker (m_mutex);
+    size_t idx = 0;
+    // Don't cache m_location.size() as it will change since we might
+    // remove locations from our vector...
+    while (idx < m_locations.size())
+    {
+        BreakpointLocation *bp_loc = m_locations[idx].get();
+        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);
+            continue;
+        }
+        if (arch.IsValid())
+        {
+            ModuleSP module_sp (bp_loc->GetAddress().GetModule());
+            if (module_sp)
+            {
+                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);
+                    continue;
+                }
+            }
+        }
+        // Only increment the index if we didn't remove the locations at index "idx"
+        ++idx;
+    }
+}
 
 void
 BreakpointLocationList::StartRecordingNewLocations (BreakpointLocationCollection &new_locations)

lldb/source/Core/Address.cpp

@@ -280,7 +280,7 @@ Address::GetFileAddress () const
         // address by adding the file base address to our offset
         return sect_file_addr + m_offset;
     }
-    else if (SectionWasDeleted())
+    else if (SectionWasDeletedPrivate())
     {
         // Used to have a valid section but it got deleted so the
         // offset doesn't mean anything without the section
@@ -308,7 +308,7 @@ Address::GetLoadAddress (Target *target) const
             }
         }
     }
-    else if (SectionWasDeleted())
+    else if (SectionWasDeletedPrivate())
     {
         // Used to have a valid section but it got deleted so the
         // offset doesn't mean anything without the section
@@ -782,6 +782,14 @@ Address::Dump (Stream *s, ExecutionContextScope *exe_scope, DumpStyle style, Dum
 
 bool
 Address::SectionWasDeleted() const
+{
+    if (GetSection())
+        return false;
+    return SectionWasDeletedPrivate();
+}
+
+bool
+Address::SectionWasDeletedPrivate() const
 {
     lldb::SectionWP empty_section_wp;
 

lldb/source/Target/Process.cpp

@@ -5644,7 +5644,7 @@ Process::DidExec ()
 {
     Target &target = GetTarget();
     target.CleanupProcess ();
-    target.ClearModules();
+    target.ClearModules(false);
     m_dynamic_checkers_ap.reset();
     m_abi_sp.reset();
     m_system_runtime_ap.reset();
@@ -5660,5 +5660,9 @@ Process::DidExec ()
     // Flush the process (threads and all stack frames) after running CompleteAttach()
     // in case the dynamic loader loaded things in new locations.
     Flush();
+    
+    // After we figure out what was loaded/unloaded in CompleteAttach,
+    // we need to let the target know so it can do any cleanup it needs to.
+    target.DidExec();
 }
 

lldb/source/Target/Target.cpp

@@ -192,7 +192,7 @@ Target::Destroy()
     DeleteCurrentProcess ();
     m_platform_sp.reset();
     m_arch.Clear();
-    ClearModules();
+    ClearModules(true);
     m_section_load_list.Clear();
     const bool notify = false;
     m_breakpoint_list.RemoveAll(notify);
@@ -1014,9 +1014,9 @@ LoadScriptingResourceForModule (const ModuleSP &module_sp, Target *target)
 }
 
 void
-Target::ClearModules()
+Target::ClearModules(bool delete_locations)
 {
-    ModulesDidUnload (m_images, true);
+    ModulesDidUnload (m_images, delete_locations);
     GetSectionLoadList().Clear();
     m_images.Clear();
     m_scratch_ast_context_ap.reset();
@@ -1024,11 +1024,19 @@ Target::ClearModules()
     m_ast_importer_ap.reset();
 }
 
+void
+Target::DidExec ()
+{
+    // When a process exec's we need to know about it so we can do some cleanup. 
+    m_breakpoint_list.RemoveInvalidLocations(m_arch);
+    m_internal_breakpoint_list.RemoveInvalidLocations(m_arch);
+}
+
 void
 Target::SetExecutableModule (ModuleSP& executable_sp, bool get_dependent_files)
 {
     Log *log(lldb_private::GetLogIfAllCategoriesSet (LIBLLDB_LOG_TARGET));
-    ClearModules();
+    ClearModules(false);
     
     if (executable_sp.get())
     {
@@ -1098,7 +1106,7 @@ Target::SetArchitecture (const ArchSpec &arch_spec)
         m_arch = arch_spec;
         ModuleSP executable_sp = GetExecutableModule ();
 
-        ClearModules();
+        ClearModules(true);
         // Need to do something about unsetting breakpoints.
         
         if (executable_sp)