[Lldb-commits] [lldb] r157008 - in /lldb/trunk: include/lldb/Breakpoint/Breakpoint.h include/lldb/Breakpoint/BreakpointList.h include/lldb/Core/ModuleList.h source/Breakpoint/Breakpoint.cpp source/Breakpoint/BreakpointList.cpp source/Core/ModuleList.cpp source/Target/Target.cpp
Jim Ingham
jingham at apple.com
Thu May 17 11:38:42 PDT 2012
Author: jingham
Date: Thu May 17 13:38:42 2012
New Revision: 157008
URL: http://llvm.org/viewvc/llvm-project?rev=157008&view=rev
Log:
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
Modified:
lldb/trunk/include/lldb/Breakpoint/Breakpoint.h
lldb/trunk/include/lldb/Breakpoint/BreakpointList.h
lldb/trunk/include/lldb/Core/ModuleList.h
lldb/trunk/source/Breakpoint/Breakpoint.cpp
lldb/trunk/source/Breakpoint/BreakpointList.cpp
lldb/trunk/source/Core/ModuleList.cpp
lldb/trunk/source/Target/Target.cpp
Modified: lldb/trunk/include/lldb/Breakpoint/Breakpoint.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Breakpoint/Breakpoint.h?rev=157008&r1=157007&r2=157008&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Breakpoint/Breakpoint.h (original)
+++ lldb/trunk/include/lldb/Breakpoint/Breakpoint.h Thu May 17 13:38:42 2012
@@ -203,11 +203,11 @@
/// Tell this breakpoint to scan a given module list and resolve any
/// new locations that match the breakpoint's specifications.
///
- /// @param[in] changedModules
+ /// @param[in] changed_modules
/// The list of modules to look in for new locations.
//------------------------------------------------------------------
void
- ResolveBreakpointInModules (ModuleList &changedModules);
+ ResolveBreakpointInModules (ModuleList &changed_modules);
//------------------------------------------------------------------
@@ -219,13 +219,29 @@
/// The list of modules to look in for new locations.
/// @param[in] load_event
/// If \b true then the modules were loaded, if \b false, unloaded.
+ /// @param[in] delete_locations
+ /// If \b true then the modules were unloaded delete any locations in the changed modules.
//------------------------------------------------------------------
void
- ModulesChanged (ModuleList &changedModules,
- bool load_event);
+ ModulesChanged (ModuleList &changed_modules,
+ bool load_event,
+ bool delete_locations = false);
//------------------------------------------------------------------
+ /// Tells the breakpoint the old module \a old_module_sp has been
+ /// replaced by new_module_sp (usually because the underlying file has been
+ /// rebuilt, and the old version is gone.)
+ ///
+ /// @param[in] old_module_sp
+ /// The old module that is going away.
+ /// @param[in] new_module_sp
+ /// The new module that is replacing it.
+ //------------------------------------------------------------------
+ void
+ ModuleReplaced (lldb::ModuleSP old_module_sp, lldb::ModuleSP new_module_sp);
+
+ //------------------------------------------------------------------
// The next set of methods provide access to the breakpoint locations
// for this breakpoint.
//------------------------------------------------------------------
Modified: lldb/trunk/include/lldb/Breakpoint/BreakpointList.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Breakpoint/BreakpointList.h?rev=157008&r1=157007&r2=157008&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Breakpoint/BreakpointList.h (original)
+++ lldb/trunk/include/lldb/Breakpoint/BreakpointList.h Thu May 17 13:38:42 2012
@@ -154,6 +154,9 @@
//------------------------------------------------------------------
void
UpdateBreakpoints (ModuleList &module_list, bool added);
+
+ void
+ UpdateBreakpointsWhenModuleIsReplaced (lldb::ModuleSP old_module_sp, lldb::ModuleSP new_module_sp);
void
ClearAllBreakpointSites ();
Modified: lldb/trunk/include/lldb/Core/ModuleList.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/ModuleList.h?rev=157008&r1=157007&r2=157008&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Core/ModuleList.h (original)
+++ lldb/trunk/include/lldb/Core/ModuleList.h Thu May 17 13:38:42 2012
@@ -355,6 +355,9 @@
size_t
Remove (ModuleList &module_list);
+ bool
+ RemoveIfOrphaned (const Module *module_ptr);
+
size_t
RemoveOrphans (bool mandatory);
@@ -419,6 +422,9 @@
static uint32_t
RemoveOrphanSharedModules (bool mandatory);
+
+ static bool
+ RemoveSharedModuleIfOrphaned (const Module *module_ptr);
protected:
//------------------------------------------------------------------
Modified: lldb/trunk/source/Breakpoint/Breakpoint.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Breakpoint/Breakpoint.cpp?rev=157008&r1=157007&r2=157008&view=diff
==============================================================================
--- lldb/trunk/source/Breakpoint/Breakpoint.cpp (original)
+++ lldb/trunk/source/Breakpoint/Breakpoint.cpp Thu May 17 13:38:42 2012
@@ -315,7 +315,7 @@
//----------------------------------------------------------------------
void
-Breakpoint::ModulesChanged (ModuleList &module_list, bool load)
+Breakpoint::ModulesChanged (ModuleList &module_list, bool load, bool delete_locations)
{
if (load)
{
@@ -395,11 +395,7 @@
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 @@
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 @@
{
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 @@
}
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 *)
{
}
Modified: lldb/trunk/source/Breakpoint/BreakpointList.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Breakpoint/BreakpointList.cpp?rev=157008&r1=157007&r2=157008&view=diff
==============================================================================
--- lldb/trunk/source/Breakpoint/BreakpointList.cpp (original)
+++ lldb/trunk/source/Breakpoint/BreakpointList.cpp Thu May 17 13:38:42 2012
@@ -210,6 +210,17 @@
}
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);
Modified: lldb/trunk/source/Core/ModuleList.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/ModuleList.cpp?rev=157008&r1=157007&r2=157008&view=diff
==============================================================================
--- lldb/trunk/source/Core/ModuleList.cpp (original)
+++ lldb/trunk/source/Core/ModuleList.cpp Thu May 17 13:38:42 2012
@@ -110,6 +110,29 @@
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 @@
return GetSharedModuleList ().Remove (module_sp);
}
+bool
+ModuleList::RemoveSharedModuleIfOrphaned (const Module *module_ptr)
+{
+ return GetSharedModuleList ().RemoveIfOrphaned (module_ptr);
+}
+
Modified: lldb/trunk/source/Target/Target.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/Target.cpp?rev=157008&r1=157007&r2=157008&view=diff
==============================================================================
--- lldb/trunk/source/Target/Target.cpp (original)
+++ lldb/trunk/source/Target/Target.cpp Thu May 17 13:38:42 2012
@@ -978,12 +978,7 @@
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 @@
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())
- {
- // 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);
+ 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())
+ {
+ 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);
}
More information about the lldb-commits
mailing list