intel/llvm
1
0
Fork 0
mirror of https://github.com/intel/llvm.git synced 2026-01-14 03:50:17 +08:00
Code Issues Packages Projects Releases Wiki Activity

We were accessing the ModuleList in the target without locking it for tasks like

Browse Source 
setting breakpoints.  That's dangerous, since while we are setting a breakpoint,
the target might hit the dyld load notification, and start removing modules from
the list.  This change adds a GetMutex accessor to the ModuleList class, and
uses it whenever we are accessing the target's ModuleList (as returned by GetImages().)

<rdar://problem/11552372>

llvm-svn: 157668
This commit is contained in:
Jim Ingham
2012-05-30 02:19:25 +00:00
parent 13586ab6d8
commit 3ee12ef26e
12 changed files with 152 additions and 54 deletions
Download Patch File Download Diff File Expand all files Collapse all files

41
lldb/include/lldb/Core/ModuleList.h
View File

@@ -116,6 +116,13 @@ public:
LogUUIDAndPaths (lldb::LogSP &log_sp,
const char *prefix_cstr);
Mutex &
GetMutex ()
{
return m_modules_mutex;
}
uint32_t
GetIndexForModule (const Module *module) const;
@@ -134,6 +141,23 @@ public:
lldb::ModuleSP
GetModuleAtIndex (uint32_t idx);
//------------------------------------------------------------------
/// Get the module shared pointer for the module at index \a idx without
/// acquiring the ModuleList mutex. This MUST already have been
/// acquired with ModuleList::GetMutex and locked for this call to be safe.
///
/// @param[in] idx
/// An index into this module collection.
///
/// @return
/// A shared pointer to a Module which can contain NULL if
/// \a idx is out of range.
///
/// @see ModuleList::GetSize()
//------------------------------------------------------------------
lldb::ModuleSP
GetModuleAtIndexUnlocked (uint32_t idx);
//------------------------------------------------------------------
/// Get the module pointer for the module at index \a idx.
///
@@ -149,6 +173,23 @@ public:
Module*
GetModulePointerAtIndex (uint32_t idx) const;
//------------------------------------------------------------------
/// Get the module pointer for the module at index \a idx without
/// acquiring the ModuleList mutex. This MUST already have been
/// acquired with ModuleList::GetMutex and locked for this call to be safe.
///
/// @param[in] idx
/// An index into this module collection.
///
/// @return
/// A pointer to a Module which can by NULL if \a idx is out
/// of range.
///
/// @see ModuleList::GetSize()
//------------------------------------------------------------------
Module*
GetModulePointerAtIndexUnlocked (uint32_t idx) const;
//------------------------------------------------------------------
/// Find compile units by partial or full path.
///

14
lldb/source/Breakpoint/Breakpoint.cpp
View File

@@ -317,6 +317,7 @@ Breakpoint::ClearAllBreakpointSites ()
void
Breakpoint::ModulesChanged (ModuleList &module_list, bool load, bool delete_locations)
{
Mutex::Locker modules_mutex(module_list.GetMutex());
if (load)
{
// The logic for handling new modules is:
@@ -332,11 +333,11 @@ Breakpoint::ModulesChanged (ModuleList &module_list, bool load, bool delete_loca
// resolving breakpoints will add new locations potentially.
const size_t num_locs = m_locations.GetSize();
for (size_t i = 0; i < module_list.GetSize(); i++)
size_t num_modules = module_list.GetSize();
for (size_t i = 0; i < num_modules; i++)
{
bool seen = false;
ModuleSP module_sp (module_list.GetModuleAtIndex (i));
ModuleSP module_sp (module_list.GetModuleAtIndexUnlocked (i));
if (!m_filter_sp->ModulePasses (module_sp))
continue;
@@ -403,10 +404,11 @@ Breakpoint::ModulesChanged (ModuleList &module_list, bool load, bool delete_loca
shared_from_this());
else
removed_locations_event = NULL;
for (size_t i = 0; i < module_list.GetSize(); i++)
size_t num_modules = module_list.GetSize();
for (size_t i = 0; i < num_modules; i++)
{
ModuleSP module_sp (module_list.GetModuleAtIndex (i));
ModuleSP module_sp (module_list.GetModuleAtIndexUnlocked (i));
if (m_filter_sp->ModulePasses (module_sp))
{
size_t loc_idx = 0;

43
lldb/source/Commands/CommandObjectTarget.cpp
View File

@@ -1882,6 +1882,7 @@ public:
if (command.GetArgumentCount() == 0)
{
// Dump all sections for all modules images
Mutex::Locker modules_locker(target->GetImages().GetMutex());
const uint32_t num_modules = target->GetImages().GetSize();
if (num_modules > 0)
{
@@ -1894,7 +1895,10 @@ public:
result.GetOutputStream().EOL();
}
num_dumped++;
DumpModuleSymtab (m_interpreter, result.GetOutputStream(), target->GetImages().GetModulePointerAtIndex(image_idx), m_options.m_sort_order);
DumpModuleSymtab (m_interpreter,
result.GetOutputStream(),
target->GetImages().GetModulePointerAtIndexUnlocked(image_idx),
m_options.m_sort_order);
}
}
else
@@ -2179,13 +2183,15 @@ public:
if (command.GetArgumentCount() == 0)
{
// Dump all sections for all modules images
const uint32_t num_modules = target->GetImages().GetSize();
ModuleList &target_modules = target->GetImages();
Mutex::Locker modules_locker (target_modules.GetMutex());
const uint32_t num_modules = target_modules.GetSize();
if (num_modules > 0)
{
result.GetOutputStream().Printf("Dumping debug symbols for %u modules.\n", num_modules);
for (uint32_t image_idx = 0; image_idx<num_modules; ++image_idx)
{
if (DumpModuleSymbolVendor (result.GetOutputStream(), target->GetImages().GetModulePointerAtIndex(image_idx)))
if (DumpModuleSymbolVendor (result.GetOutputStream(), target_modules.GetModulePointerAtIndexUnlocked(image_idx)))
num_dumped++;
}
}
@@ -2288,7 +2294,10 @@ public:
for (int arg_idx = 0; (arg_cstr = command.GetArgumentAtIndex(arg_idx)) != NULL; ++arg_idx)
{
FileSpec file_spec(arg_cstr, false);
const uint32_t num_modules = target->GetImages().GetSize();
ModuleList &target_modules = target->GetImages();
Mutex::Locker modules_locker(target_modules.GetMutex());
const uint32_t num_modules = target_modules.GetSize();
if (num_modules > 0)
{
uint32_t num_dumped = 0;
@@ -2296,7 +2305,7 @@ public:
{
if (DumpCompileUnitLineTable (m_interpreter,
result.GetOutputStream(),
target->GetImages().GetModulePointerAtIndex(i),
target_modules.GetModulePointerAtIndexUnlocked(i),
file_spec,
exe_ctx.GetProcessPtr() && exe_ctx.GetProcessRef().IsAlive()))
num_dumped++;
@@ -2807,9 +2816,6 @@ public:
result.GetErrorStream().SetAddressByteSize(addr_byte_size);
}
// Dump all sections for all modules images
uint32_t num_modules = 0;
Mutex::Locker locker;
Stream &strm = result.GetOutputStream();
if (m_options.m_module_addr != LLDB_INVALID_ADDRESS)
@@ -2845,6 +2851,11 @@ public:
return result.Succeeded();
}
uint32_t num_modules = 0;
Mutex::Locker locker; // This locker will be locked on the mutex in module_list_ptr if it is non-NULL.
// Otherwise it will lock the AllocationModuleCollectionMutex when accessing
// the global module list directly.
ModuleList module_list;
ModuleList *module_list_ptr = NULL;
const size_t argc = command.GetArgumentCount();
@@ -2858,7 +2869,6 @@ public:
else
{
module_list_ptr = &target->GetImages();
num_modules = target->GetImages().GetSize();
}
}
else
@@ -2879,9 +2889,14 @@ public:
}
}
num_modules = module_list.GetSize();
module_list_ptr = &module_list;
}
if (module_list_ptr != NULL)
{
locker.Lock(module_list_ptr->GetMutex());
num_modules = module_list_ptr->GetSize();
}
if (num_modules > 0)
{
@@ -2891,7 +2906,7 @@ public:
Module *module;
if (module_list_ptr)
{
module_sp = module_list_ptr->GetModuleAtIndex(image_idx);
module_sp = module_list_ptr->GetModuleAtIndexUnlocked(image_idx);
module = module_sp.get();
}
else
@@ -3414,12 +3429,14 @@ public:
if (command.GetArgumentCount() == 0)
{
// Dump all sections for all modules images
const uint32_t num_modules = target->GetImages().GetSize();
ModuleList &target_modules = target->GetImages();
Mutex::Locker modules_locker(target_modules.GetMutex());
const uint32_t num_modules = target_modules.GetSize();
if (num_modules > 0)
{
for (i = 0; i<num_modules && syntax_error == false; ++i)
{
if (LookupInModule (m_interpreter, target->GetImages().GetModulePointerAtIndex(i), result, syntax_error))
if (LookupInModule (m_interpreter, target_modules.GetModulePointerAtIndexUnlocked(i), result, syntax_error))
{
result.GetOutputStream().EOL();
num_successful_lookups++;

12
lldb/source/Core/ModuleList.cpp
View File

@@ -201,6 +201,12 @@ Module*
ModuleList::GetModulePointerAtIndex (uint32_t idx) const
{
Mutex::Locker locker(m_modules_mutex);
return GetModulePointerAtIndexUnlocked(idx);
}
Module*
ModuleList::GetModulePointerAtIndexUnlocked (uint32_t idx) const
{
if (idx < m_modules.size())
return m_modules[idx].get();
return NULL;
@@ -210,6 +216,12 @@ ModuleSP
ModuleList::GetModuleAtIndex(uint32_t idx)
{
Mutex::Locker locker(m_modules_mutex);
return GetModuleAtIndexUnlocked(idx);
}
ModuleSP
ModuleList::GetModuleAtIndexUnlocked(uint32_t idx)
{
ModuleSP module_sp;
if (idx < m_modules.size())
module_sp = m_modules[idx];

33
lldb/source/Core/SearchFilter.cpp
View File

@@ -160,11 +160,12 @@ SearchFilter::SearchInModuleList (Searcher &searcher, ModuleList &modules)
searcher.SearchCallback (*this, empty_sc, NULL, false);
else
{
Mutex::Locker modules_locker(modules.GetMutex());
const size_t numModules = modules.GetSize();
for (size_t i = 0; i < numModules; i++)
{
ModuleSP module_sp(modules.GetModuleAtIndex(i));
ModuleSP module_sp(modules.GetModuleAtIndexUnlocked(i));
if (ModulePasses(module_sp))
{
if (DoModuleIteration(module_sp, searcher) == Searcher::eCallbackReturnStop)
@@ -191,12 +192,15 @@ SearchFilter::DoModuleIteration (const SymbolContext &context, Searcher &searche
{
if (!context.module_sp)
{
size_t n_modules = m_target_sp->GetImages().GetSize();
ModuleList &target_images = m_target_sp->GetImages();
Mutex::Locker modules_locker(target_images.GetMutex());
size_t n_modules = target_images.GetSize();
for (size_t i = 0; i < n_modules; i++)
{
// If this is the last level supplied, then call the callback directly,
// otherwise descend.
ModuleSP module_sp(m_target_sp->GetImages().GetModuleAtIndex(i));
ModuleSP module_sp(target_images.GetModuleAtIndexUnlocked (i));
if (!ModulePasses (module_sp))
continue;
@@ -427,11 +431,13 @@ SearchFilterByModule::Search (Searcher &searcher)
// filespec that passes. Otherwise, we need to go through all modules and
// find the ones that match the file name.
ModuleList matching_modules;
const size_t num_modules = m_target_sp->GetImages().GetSize ();
ModuleList &target_modules = m_target_sp->GetImages();
Mutex::Locker modules_locker (target_modules.GetMutex());
const size_t num_modules = target_modules.GetSize ();
for (size_t i = 0; i < num_modules; i++)
{
Module* module = m_target_sp->GetImages().GetModulePointerAtIndex(i);
Module* module = target_modules.GetModulePointerAtIndexUnlocked(i);
if (FileSpec::Compare (m_module_spec, module->GetFileSpec(), false) == 0)
{
SymbolContext matchingContext(m_target_sp, module->shared_from_this());
@@ -591,11 +597,13 @@ SearchFilterByModuleList::Search (Searcher &searcher)
// filespec that passes. Otherwise, we need to go through all modules and
// find the ones that match the file name.
ModuleList matching_modules;
const size_t num_modules = m_target_sp->GetImages().GetSize ();
ModuleList &target_modules = m_target_sp->GetImages();
Mutex::Locker modules_locker (target_modules.GetMutex());
const size_t num_modules = target_modules.GetSize ();
for (size_t i = 0; i < num_modules; i++)
{
Module* module = m_target_sp->GetImages().GetModulePointerAtIndex(i);
Module* module = target_modules.GetModulePointerAtIndexUnlocked(i);
if (m_module_spec_list.FindFileIndex(0, module->GetFileSpec(), false) != UINT32_MAX)
{
SymbolContext matchingContext(m_target_sp, module->shared_from_this());
@@ -762,11 +770,14 @@ SearchFilterByModuleListAndCU::Search (Searcher &searcher)
// find the ones that match the file name.
ModuleList matching_modules;
const size_t num_modules = m_target_sp->GetImages().GetSize ();
ModuleList &target_images = m_target_sp->GetImages();
Mutex::Locker modules_locker(target_images.GetMutex());
const size_t num_modules = target_images.GetSize ();
bool no_modules_in_filter = m_module_spec_list.GetSize() == 0;
for (size_t i = 0; i < num_modules; i++)
{
lldb::ModuleSP module_sp = m_target_sp->GetImages().GetModuleAtIndex(i);
lldb::ModuleSP module_sp = target_images.GetModuleAtIndexUnlocked(i);
if (no_modules_in_filter || m_module_spec_list.FindFileIndex(0, module_sp->GetFileSpec(), false) != UINT32_MAX)
{
SymbolContext matchingContext(m_target_sp, module_sp);

15
lldb/source/Expression/ClangASTSource.cpp
View File

@@ -561,13 +561,14 @@ ClangASTSource::FindExternalVisibleDecls (NameSearchContext &context,
}
else
{
ModuleList &images = m_target->GetImages();
ModuleList &target_images = m_target->GetImages();
Mutex::Locker modules_locker (target_images.GetMutex());
for (uint32_t i = 0, e = images.GetSize();
for (uint32_t i = 0, e = target_images.GetSize();
i != e;
++i)
{
lldb::ModuleSP image = images.GetModuleAtIndex(i);
lldb::ModuleSP image = target_images.GetModuleAtIndexUnlocked(i);
if (!image)
continue;
@@ -1339,14 +1340,16 @@ ClangASTSource::CompleteNamespaceMap (ClangASTImporter::NamespaceMapSP &namespac
}
else
{
ModuleList &images = m_target->GetImages();
ModuleList &target_images = m_target->GetImages();
Mutex::Locker modules_locker(target_images.GetMutex());
ClangNamespaceDecl null_namespace_decl;
for (uint32_t i = 0, e = images.GetSize();
for (uint32_t i = 0, e = target_images.GetSize();
i != e;
++i)
{
lldb::ModuleSP image = images.GetModuleAtIndex(i);
lldb::ModuleSP image = target_images.GetModuleAtIndexUnlocked(i);
if (!image)
continue;

8
lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp
View File

@@ -1057,12 +1057,14 @@ DynamicLoaderMacOSXDYLD::InitializeFromAllImageInfos ()
// to an equivalent version. We don't want it to stay in the target's module list or it will confuse
// us, so unload it here.
Target &target = m_process->GetTarget();
ModuleList &modules = target.GetImages();
ModuleList &target_modules = target.GetImages();
ModuleList not_loaded_modules;
size_t num_modules = modules.GetSize();
Mutex::Locker modules_locker(target_modules.GetMutex());
size_t num_modules = target_modules.GetSize();
for (size_t i = 0; i < num_modules; i++)
{
ModuleSP module_sp = modules.GetModuleAtIndex(i);
ModuleSP module_sp = target_modules.GetModuleAtIndexUnlocked (i);
if (!module_sp->IsLoadedInTarget (&target))
{
if (log)

4
lldb/source/Plugins/DynamicLoader/Static/DynamicLoaderStatic.cpp
View File

@@ -97,10 +97,12 @@ DynamicLoaderStatic::LoadAllImagesAtFileAddresses ()
ModuleList loaded_module_list;
Mutex::Locker mutex_locker(module_list.GetMutex());
const size_t num_modules = module_list.GetSize();
for (uint32_t idx = 0; idx < num_modules; ++idx)
{
ModuleSP module_sp (module_list.GetModuleAtIndex (idx));
ModuleSP module_sp (module_list.GetModuleAtIndexUnlocked (idx));
if (module_sp)
{
bool changed = false;

8
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
View File

@@ -263,11 +263,13 @@ AppleObjCRuntime::GetObjCVersion (Process *process, ModuleSP &objc_module_sp)
return eObjC_VersionUnknown;
Target &target = process->GetTarget();
ModuleList &images = target.GetImages();
size_t num_images = images.GetSize();
ModuleList &target_modules = target.GetImages();
Mutex::Locker modules_locker(target_modules.GetMutex());
size_t num_images = target_modules.GetSize();
for (size_t i = 0; i < num_images; i++)
{
ModuleSP module_sp = images.GetModuleAtIndex(i);
ModuleSP module_sp = target_modules.GetModuleAtIndexUnlocked(i);
// One tricky bit here is that we might get called as part of the initial module loading, but
// before all the pre-run libraries get winnowed from the module list. So there might actually
// be an old and incorrect ObjC library sitting around in the list, and we don't want to look at that.

7
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCSymbolVendor.cpp
View File

@@ -43,13 +43,14 @@ AppleObjCSymbolVendor::FindTypes (const SymbolContext& sc,
uint32_t ret = 0;
ModuleList &images = m_process->GetTarget().GetImages();
ModuleList &target_modules = m_process->GetTarget().GetImages();
Mutex::Locker modules_locker(target_modules.GetMutex());
for (size_t image_index = 0, end_index = images.GetSize();
for (size_t image_index = 0, end_index = target_modules.GetSize();
image_index < end_index;
++image_index)
{
Module *image = images.GetModulePointerAtIndex(image_index);
Module *image = target_modules.GetModulePointerAtIndexUnlocked(image_index);
if (!image)
continue;

9
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTrampolineHandler.cpp
View File

@@ -334,15 +334,16 @@ AppleObjCTrampolineHandler::AppleObjCVTables::InitializeVTableSymbols ()
return true;
Target &target = m_process_sp->GetTarget();
ModuleList &modules = target.GetImages();
size_t num_modules = modules.GetSize();
ModuleList &target_modules = target.GetImages();
Mutex::Locker modules_locker(target_modules.GetMutex());
size_t num_modules = target_modules.GetSize();
if (!m_objc_module_sp)
{
for (size_t i = 0; i < num_modules; i++)
{
if (m_process_sp->GetObjCLanguageRuntime()->IsModuleObjCLibrary (modules.GetModuleAtIndex(i)))
if (m_process_sp->GetObjCLanguageRuntime()->IsModuleObjCLibrary (target_modules.GetModuleAtIndexUnlocked(i)))
{
m_objc_module_sp = modules.GetModuleAtIndex(i);
m_objc_module_sp = target_modules.GetModuleAtIndexUnlocked(i);
break;
}
}

12
lldb/source/Target/Process.cpp
View File

@@ -2814,19 +2814,23 @@ Process::CompleteAttach ()
m_os_ap.reset (OperatingSystem::FindPlugin (this, NULL));
// Figure out which one is the executable, and set that in our target:
ModuleList &modules = m_target.GetImages();
ModuleList &target_modules = m_target.GetImages();
Mutex::Locker modules_locker(target_modules.GetMutex());
size_t num_modules = target_modules.GetSize();
ModuleSP new_executable_module_sp;
size_t num_modules = modules.GetSize();
for (int i = 0; i < num_modules; i++)
{
ModuleSP module_sp (modules.GetModuleAtIndex(i));
ModuleSP module_sp (target_modules.GetModuleAtIndexUnlocked (i));
if (module_sp && module_sp->IsExecutable())
{
if (m_target.GetExecutableModulePointer() != module_sp.get())
m_target.SetExecutableModule (module_sp, false);
new_executable_module_sp = module_sp;
break;
}
}
if (new_executable_module_sp)
m_target.SetExecutableModule (new_executable_module_sp, false);
}
Error