If we notice that a module with a given file path is replaced by another with the same file

path on rerunning, evict the old module from the target module list, inform the breakpoints
about this so they can do something intelligent as well.

rdar://problem/11273043

llvm-svn: 157008

4 files changed, 159 insertions, 65 deletions

diff --git a/lldb/source/Breakpoint/Breakpoint.cpp b/lldb/source/Breakpoint/Breakpoint.cpp
index 62e1b248280..5d0cbd47e20 100644
--- a/lldb/source/Breakpoint/Breakpoint.cpp
+++ b/lldb/source/Breakpoint/Breakpoint.cpp
@@ -315,7 +315,7 @@ Breakpoint::ClearAllBreakpointSites ()
 //----------------------------------------------------------------------
 
 void
-Breakpoint::ModulesChanged (ModuleList &module_list, bool load)
+Breakpoint::ModulesChanged (ModuleList &module_list, bool load, bool delete_locations)
 {
     if (load)
     {
@@ -395,11 +395,7 @@ Breakpoint::ModulesChanged (ModuleList &module_list, bool load)
     else
     {
         // Go through the currently set locations and if any have breakpoints in
-        // the module list, then remove their breakpoint sites.
-        // FIXME: Think about this...  Maybe it's better to delete the locations?
-        // Are we sure that on load-unload-reload the module pointer will remain
-        // the same?  Or do we need to do an equality on modules that is an
-        // "equivalence"???
+        // the module list, then remove their breakpoint sites, and their locations if asked to.
 
         BreakpointEventData *removed_locations_event;
         if (!IsInternal())
@@ -414,7 +410,9 @@ Breakpoint::ModulesChanged (ModuleList &module_list, bool load)
             if (m_filter_sp->ModulePasses (module_sp))
             {
                 size_t loc_idx = 0;
-                while (loc_idx < m_locations.GetSize())
+                size_t num_locations = m_locations.GetSize();
+                BreakpointLocationCollection locations_to_remove;
+                for (loc_idx = 0; loc_idx < num_locations; loc_idx++)
                 {
                     BreakpointLocationSP break_loc_sp (m_locations.GetByIndex(loc_idx));
                     SectionSP section_sp (break_loc_sp->GetAddress().GetSection());
@@ -429,9 +427,17 @@ Breakpoint::ModulesChanged (ModuleList &module_list, bool load)
                         {
                             removed_locations_event->GetBreakpointLocationCollection().Add(break_loc_sp);
                         }
-                        //m_locations.RemoveLocation  (break_loc_sp);
+                        if (delete_locations)
+                            locations_to_remove.Add (break_loc_sp);
+                            
                     }
-                    ++loc_idx;
+                }
+                
+                if (delete_locations)
+                {
+                    size_t num_locations_to_remove = locations_to_remove.GetSize();
+                    for (loc_idx = 0; loc_idx < num_locations_to_remove; loc_idx++)
+                        m_locations.RemoveLocation  (locations_to_remove.GetByIndex(loc_idx));
                 }
             }
         }
@@ -440,6 +446,23 @@ Breakpoint::ModulesChanged (ModuleList &module_list, bool load)
 }
 
 void
+Breakpoint::ModuleReplaced (ModuleSP old_module_sp, ModuleSP new_module_sp)
+{
+    ModuleList temp_list;
+    temp_list.Append (new_module_sp);
+    ModulesChanged (temp_list, true);
+
+    // TO DO: For now I'm just adding locations for the new module and removing the
+    // breakpoint locations that were in the old module.
+    // We should really go find the ones that are in the new module & if we can determine that they are "equivalent"
+    // carry over the options from the old location to the new.
+
+    temp_list.Clear();
+    temp_list.Append (old_module_sp);
+    ModulesChanged (temp_list, false, true);
+}
+
+void
 Breakpoint::Dump (Stream *)
 {
 }
diff --git a/lldb/source/Breakpoint/BreakpointList.cpp b/lldb/source/Breakpoint/BreakpointList.cpp
index e1aa2dbcb1e..6a91bd6731a 100644
--- a/lldb/source/Breakpoint/BreakpointList.cpp
+++ b/lldb/source/Breakpoint/BreakpointList.cpp
@@ -210,6 +210,17 @@ BreakpointList::UpdateBreakpoints (ModuleList& module_list, bool added)
 }
 
 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);
+
+}
+
+void
 BreakpointList::ClearAllBreakpointSites ()
 {
     Mutex::Locker locker(m_mutex);
diff --git a/lldb/source/Core/ModuleList.cpp b/lldb/source/Core/ModuleList.cpp
index c5a89554c62..8f852d4d8d1 100644
--- a/lldb/source/Core/ModuleList.cpp
+++ b/lldb/source/Core/ModuleList.cpp
@@ -110,6 +110,29 @@ ModuleList::Remove (const ModuleSP &module_sp)
     return false;
 }
 
+bool
+ModuleList::RemoveIfOrphaned (const Module *module_ptr)
+{
+    if (module_ptr)
+    {
+        Mutex::Locker locker(m_modules_mutex);
+        collection::iterator pos, end = m_modules.end();
+        for (pos = m_modules.begin(); pos != end; ++pos)
+        {
+            if (pos->get() == module_ptr)
+            {
+                if (pos->unique())
+                {
+                    pos = m_modules.erase (pos);
+                    return true;
+                }
+                else
+                    return false;
+            }
+        }
+    }
+    return false;
+}
 
 size_t
 ModuleList::RemoveOrphans (bool mandatory)
@@ -817,4 +840,10 @@ ModuleList::RemoveSharedModule (lldb::ModuleSP &module_sp)
     return GetSharedModuleList ().Remove (module_sp);
 }
 
+bool
+ModuleList::RemoveSharedModuleIfOrphaned (const Module *module_ptr)
+{
+    return GetSharedModuleList ().RemoveIfOrphaned (module_ptr);
+}
+
 
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index 7255b04038b..a1caed4b9c5 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -978,12 +978,7 @@ void
 Target::ModuleUpdated (ModuleSP &old_module_sp, ModuleSP &new_module_sp)
 {
     // A module is replacing an already added module
-    ModuleList module_list;
-    module_list.Append (old_module_sp);
-    ModulesDidUnload (module_list);
-    module_list.Clear ();
-    module_list.Append (new_module_sp);
-    ModulesDidLoad (module_list);
+    m_breakpoint_list.UpdateBreakpointsWhenModuleIsReplaced(old_module_sp, new_module_sp);
 }
 
 void
@@ -1287,76 +1282,112 @@ Target::ReadPointerFromMemory (const Address& addr,
 ModuleSP
 Target::GetSharedModule (const ModuleSpec &module_spec, Error *error_ptr)
 {
-    // Don't pass in the UUID so we can tell if we have a stale value in our list
-    ModuleSP old_module_sp; // This will get filled in if we have a new version of the library
-    bool did_create_module = false;
     ModuleSP module_sp;
 
     Error error;
 
-    // If there are image search path entries, try to use them first to acquire a suitable image.
-    if (m_image_search_paths.GetSize())
-    {
-        ModuleSpec transformed_spec (module_spec);
-        if (m_image_search_paths.RemapPath (module_spec.GetFileSpec().GetDirectory(), transformed_spec.GetFileSpec().GetDirectory()))
-        {
-            transformed_spec.GetFileSpec().GetFilename() = module_spec.GetFileSpec().GetFilename();
-            error = ModuleList::GetSharedModule (transformed_spec, 
-                                                 module_sp, 
-                                                 &GetExecutableSearchPaths(),
-                                                 &old_module_sp, 
-                                                 &did_create_module);
-        }
-    }
+    // First see if we already have this module in our module list.  If we do, then we're done, we don't need
+    // to consult the shared modules list.  But only do this if we are passed a UUID.
     
+    if (module_spec.GetUUID().IsValid())
+        module_sp = m_images.FindFirstModule(module_spec);
+        
     if (!module_sp)
     {
-        // If we have a UUID, we can check our global shared module list in case
-        // we already have it. If we don't have a valid UUID, then we can't since
-        // the path in "module_spec" will be a platform path, and we will need to
-        // let the platform find that file. For example, we could be asking for
-        // "/usr/lib/dyld" and if we do not have a UUID, we don't want to pick
-        // the local copy of "/usr/lib/dyld" since our platform could be a remote
-        // platform that has its own "/usr/lib/dyld" in an SDK or in a local file
-        // cache.
-        if (module_spec.GetUUID().IsValid())
+        ModuleSP old_module_sp; // This will get filled in if we have a new version of the library
+        bool did_create_module = false;
+    
+        // If there are image search path entries, try to use them first to acquire a suitable image.
+        if (m_image_search_paths.GetSize())
         {
-            // We have a UUID, it is OK to check the global module list...
-            error = ModuleList::GetSharedModule (module_spec,
-                                                 module_sp, 
-                                                 &GetExecutableSearchPaths(),
-                                                 &old_module_sp, 
-                                                 &did_create_module);
+            ModuleSpec transformed_spec (module_spec);
+            if (m_image_search_paths.RemapPath (module_spec.GetFileSpec().GetDirectory(), transformed_spec.GetFileSpec().GetDirectory()))
+            {
+                transformed_spec.GetFileSpec().GetFilename() = module_spec.GetFileSpec().GetFilename();
+                error = ModuleList::GetSharedModule (transformed_spec, 
+                                                     module_sp, 
+                                                     &GetExecutableSearchPaths(),
+                                                     &old_module_sp, 
+                                                     &did_create_module);
+            }
         }
-
+        
         if (!module_sp)
         {
-            // The platform is responsible for finding and caching an appropriate
-            // module in the shared module cache.
-            if (m_platform_sp)
+            // If we have a UUID, we can check our global shared module list in case
+            // we already have it. If we don't have a valid UUID, then we can't since
+            // the path in "module_spec" will be a platform path, and we will need to
+            // let the platform find that file. For example, we could be asking for
+            // "/usr/lib/dyld" and if we do not have a UUID, we don't want to pick
+            // the local copy of "/usr/lib/dyld" since our platform could be a remote
+            // platform that has its own "/usr/lib/dyld" in an SDK or in a local file
+            // cache.
+            if (module_spec.GetUUID().IsValid())
             {
-                FileSpec platform_file_spec;        
-                error = m_platform_sp->GetSharedModule (module_spec, 
-                                                        module_sp, 
-                                                        &GetExecutableSearchPaths(),
-                                                        &old_module_sp, 
-                                                        &did_create_module);
+                // We have a UUID, it is OK to check the global module list...
+                error = ModuleList::GetSharedModule (module_spec,
+                                                     module_sp, 
+                                                     &GetExecutableSearchPaths(),
+                                                     &old_module_sp, 
+                                                     &did_create_module);
             }
-            else
+
+            if (!module_sp)
             {
-                error.SetErrorString("no platform is currently set");
+                // The platform is responsible for finding and caching an appropriate
+                // module in the shared module cache.
+                if (m_platform_sp)
+                {
+                    FileSpec platform_file_spec;        
+                    error = m_platform_sp->GetSharedModule (module_spec, 
+                                                            module_sp, 
+                                                            &GetExecutableSearchPaths(),
+                                                            &old_module_sp, 
+                                                            &did_create_module);
+                }
+                else
+                {
+                    error.SetErrorString("no platform is currently set");
+                }
             }
         }
-    }
 
-    // If a module hasn't been found yet, use the unmodified path.
-    if (module_sp)
-    {
-        m_images.Append (module_sp);
-        if (did_create_module)
+        // We found a module that wasn't in our target list.  Let's make sure that there wasn't an equivalent
+        // module in the list already, and if there was, let's remove it.
+        if (module_sp)
         {
+            // GetSharedModule is not guaranteed to find the old shared module, for instance
+            // in the common case where you pass in the UUID, it is only going to find the one
+            // module matching the UUID.  In fact, it has no good way to know what the "old module"
+            // relevant to this target is, since there might be many copies of a module with this file spec
+            // in various running debug sessions, but only one of them will belong to this target.
+            // So let's remove the UUID from the module list, and look in the target's module list.
+            // Only do this if there is SOMETHING else in the module spec...
+            if (!old_module_sp)
+            {
+                if (module_spec.GetUUID().IsValid() && !module_spec.GetFileSpec().GetFilename().IsEmpty() && !module_spec.GetFileSpec().GetDirectory().IsEmpty())
+                {
+                    ModuleSpec module_spec_copy(module_spec.GetFileSpec());
+                    module_spec_copy.GetUUID().Clear();
+                    
+                    ModuleList found_modules;
+                    size_t num_found = m_images.FindModules (module_spec_copy, found_modules);
+                    if (num_found == 1)
+                    {
+                        old_module_sp = found_modules.GetModuleAtIndex(0);
+                    }
+                }
+            }
+            
+            m_images.Append (module_sp);
             if (old_module_sp && m_images.GetIndexForModule (old_module_sp.get()) != LLDB_INVALID_INDEX32)
+            {
                 ModuleUpdated(old_module_sp, module_sp);
+                m_images.Remove (old_module_sp);
+                Module *old_module_ptr = old_module_sp.get();
+                old_module_sp.reset();
+                ModuleList::RemoveSharedModuleIfOrphaned (old_module_ptr);
+            }
             else
                 ModuleAdded(module_sp);
         }