jasonmolenda updated this revision to Diff 193609.
jasonmolenda added a comment.
Remove const bool notify's. Rename method to Target::GetOrCreateModule.
Refine the method headerdoc a bit.
Repository:
rLLDB LLDB
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D60172/new/
https://reviews.llvm.org/D60172
Files:
include/lldb/Core/ModuleList.h
include/lldb/Target/Target.h
source/API/SBTarget.cpp
source/Commands/CommandObjectTarget.cpp
source/Core/DynamicLoader.cpp
source/Core/ModuleList.cpp
source/Expression/FunctionCaller.cpp
source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
source/Plugins/DynamicLoader/Hexagon-DYLD/DynamicLoaderHexagonDYLD.cpp
source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp
source/Plugins/Process/Windows/Common/ProcessWindows.cpp
source/Plugins/Process/elf-core/ProcessElfCore.cpp
source/Plugins/Process/minidump/ProcessMinidump.cpp
source/Target/Target.cpp
Index: source/Target/Target.cpp
===================================================================
--- source/Target/Target.cpp
+++ source/Target/Target.cpp
@@ -1442,7 +1442,8 @@
"Target::SetExecutableModule (executable = '%s')",
executable_sp->GetFileSpec().GetPath().c_str());
- m_images.Append(executable_sp); // The first image is our executable file
+ const bool notify = true;
+ m_images.Append(executable_sp, notify); // The first image is our executable file
// If we haven't set an architecture yet, reset our architecture based on
// what we found in the executable module.
@@ -1482,7 +1483,8 @@
platform_dependent_file_spec = dependent_file_spec;
ModuleSpec module_spec(platform_dependent_file_spec, m_arch.GetSpec());
- ModuleSP image_module_sp(GetSharedModule(module_spec));
+ ModuleSP image_module_sp(GetOrCreateModule(module_spec,
+ true /* notify */));
if (image_module_sp) {
ObjectFile *objfile = image_module_sp->GetObjectFile();
if (objfile)
@@ -1984,8 +1986,8 @@
return false;
}
-ModuleSP Target::GetSharedModule(const ModuleSpec &module_spec,
- Status *error_ptr) {
+ModuleSP Target::GetOrCreateModule(const ModuleSpec &module_spec, bool notify,
+ Status *error_ptr) {
ModuleSP module_sp;
Status error;
@@ -2118,8 +2120,9 @@
Module *old_module_ptr = old_module_sp.get();
old_module_sp.reset();
ModuleList::RemoveSharedModuleIfOrphaned(old_module_ptr);
- } else
- m_images.Append(module_sp);
+ } else {
+ m_images.Append(module_sp, notify);
+ }
} else
module_sp.reset();
}
Index: source/Plugins/Process/minidump/ProcessMinidump.cpp
===================================================================
--- source/Plugins/Process/minidump/ProcessMinidump.cpp
+++ source/Plugins/Process/minidump/ProcessMinidump.cpp
@@ -388,7 +388,8 @@
Status error;
// Try and find a module with a full UUID that matches. This function will
// add the module to the target if it finds one.
- lldb::ModuleSP module_sp = GetTarget().GetSharedModule(module_spec, &error);
+ lldb::ModuleSP module_sp = GetTarget().GetOrCreateModule(module_spec,
+ true /* notify */, &error);
if (!module_sp) {
// Try and find a module without specifying the UUID and only looking for
// the file given a basename. We then will look for a partial UUID match
@@ -400,7 +401,8 @@
ModuleSpec basename_module_spec(module_spec);
basename_module_spec.GetUUID().Clear();
basename_module_spec.GetFileSpec().GetDirectory().Clear();
- module_sp = GetTarget().GetSharedModule(basename_module_spec, &error);
+ module_sp = GetTarget().GetOrCreateModule(basename_module_spec,
+ true /* notify */, &error);
if (module_sp) {
// We consider the module to be a match if the minidump UUID is a
// prefix of the actual UUID, or if either of the UUIDs are empty.
@@ -430,7 +432,7 @@
module_sp = Module::CreateModuleFromObjectFile<PlaceholderObjectFile>(
module_spec, module->base_of_image, module->size_of_image);
- GetTarget().GetImages().Append(module_sp);
+ GetTarget().GetImages().Append(module_sp, true /* notify */);
}
bool load_addr_changed = false;
Index: source/Plugins/Process/elf-core/ProcessElfCore.cpp
===================================================================
--- source/Plugins/Process/elf-core/ProcessElfCore.cpp
+++ source/Plugins/Process/elf-core/ProcessElfCore.cpp
@@ -244,7 +244,8 @@
exe_module_spec.GetFileSpec().SetFile(
m_nt_file_entries[0].path.GetCString(), FileSpec::Style::native);
if (exe_module_spec.GetFileSpec()) {
- exe_module_sp = GetTarget().GetSharedModule(exe_module_spec);
+ exe_module_sp = GetTarget().GetOrCreateModule(exe_module_spec,
+ true /* notify */);
if (exe_module_sp)
GetTarget().SetExecutableModule(exe_module_sp, eLoadDependentsNo);
}
Index: source/Plugins/Process/Windows/Common/ProcessWindows.cpp
===================================================================
--- source/Plugins/Process/Windows/Common/ProcessWindows.cpp
+++ source/Plugins/Process/Windows/Common/ProcessWindows.cpp
@@ -915,7 +915,8 @@
FileSystem::Instance().Resolve(executable_file);
ModuleSpec module_spec(executable_file);
Status error;
- module = GetTarget().GetSharedModule(module_spec, &error);
+ module = GetTarget().GetOrCreateModule(module_spec,
+ true /* notify */, &error);
if (!module) {
return;
}
Index: source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp
===================================================================
--- source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp
+++ source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp
@@ -68,11 +68,9 @@
// Resolve the module unless we already have one.
if (!module_sp) {
- // Confusingly, there is no Target::AddSharedModule. Instead, calling
- // GetSharedModule() with a new module will add it to the module list and
- // return a corresponding ModuleSP.
Status error;
- module_sp = m_process->GetTarget().GetSharedModule(module_spec, &error);
+ module_sp = m_process->GetTarget().GetOrCreateModule(module_spec,
+ true /* notify */, &error);
if (error.Fail())
return;
}
Index: source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
===================================================================
--- source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -543,7 +543,8 @@
FileSpec file(info.GetName().GetCString());
ModuleSpec module_spec(file, target.GetArchitecture());
- if (ModuleSP module_sp = target.GetSharedModule(module_spec)) {
+ if (ModuleSP module_sp = target.GetOrCreateModule(module_spec,
+ true /* notify */)) {
UpdateLoadedSections(module_sp, LLDB_INVALID_ADDRESS, m_interpreter_base,
false);
return module_sp;
Index: source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
===================================================================
--- source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
+++ source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
@@ -122,7 +122,9 @@
if (!module_sp) {
if (can_create) {
- module_sp = target.GetSharedModule(module_spec);
+ // We'll call Target::ModulesDidLoad after all the modules have been
+ // added to the target, don't let it be called for every one.
+ module_sp = target.GetOrCreateModule(module_spec, true /* notify */);
if (!module_sp || module_sp->GetObjectFile() == NULL)
module_sp = m_process->ReadModuleFromMemory(image_info.file_spec,
image_info.address);
@@ -637,7 +639,8 @@
module_spec.SetObjectOffset(objfile->GetFileOffset() +
commpage_section->GetFileOffset());
module_spec.SetObjectSize(objfile->GetByteSize());
- commpage_image_module_sp = target.GetSharedModule(module_spec);
+ commpage_image_module_sp = target.GetOrCreateModule(module_spec,
+ true /* notify */);
if (!commpage_image_module_sp ||
commpage_image_module_sp->GetObjectFile() == NULL) {
commpage_image_module_sp = m_process->ReadModuleFromMemory(
Index: source/Plugins/DynamicLoader/Hexagon-DYLD/DynamicLoaderHexagonDYLD.cpp
===================================================================
--- source/Plugins/DynamicLoader/Hexagon-DYLD/DynamicLoaderHexagonDYLD.cpp
+++ source/Plugins/DynamicLoader/Hexagon-DYLD/DynamicLoaderHexagonDYLD.cpp
@@ -199,7 +199,7 @@
return executable;
// TODO: What case is this code used?
- executable = target.GetSharedModule(module_spec);
+ executable = target.GetOrCreateModule(module_spec, true /* notify */);
if (executable.get() != target.GetExecutableModulePointer()) {
// Don't load dependent images since we are in dyld where we will know and
// find out about all images that are loaded
Index: source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
===================================================================
--- source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
+++ source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
@@ -841,7 +841,7 @@
// the DebugSymbols framework with the UUID to find the binary via its
// search methods.
if (!m_module_sp) {
- m_module_sp = target.GetSharedModule(module_spec);
+ m_module_sp = target.GetOrCreateModule(module_spec, true /* notify */);
}
if (IsKernel() && !m_module_sp) {
Index: source/Expression/FunctionCaller.cpp
===================================================================
--- source/Expression/FunctionCaller.cpp
+++ source/Expression/FunctionCaller.cpp
@@ -103,7 +103,8 @@
jit_file.GetFilename() = const_func_name;
jit_module_sp->SetFileSpecAndObjectName(jit_file, ConstString());
m_jit_module_wp = jit_module_sp;
- process->GetTarget().GetImages().Append(jit_module_sp);
+ process->GetTarget().GetImages().Append(jit_module_sp,
+ true /* notify */);
}
}
if (process && m_jit_start_addr)
Index: source/Core/ModuleList.cpp
===================================================================
--- source/Core/ModuleList.cpp
+++ source/Core/ModuleList.cpp
@@ -151,7 +151,9 @@
}
}
-void ModuleList::Append(const ModuleSP &module_sp) { AppendImpl(module_sp); }
+void ModuleList::Append(const ModuleSP &module_sp, bool notify) {
+ AppendImpl(module_sp, notify);
+}
void ModuleList::ReplaceEquivalent(const ModuleSP &module_sp) {
if (module_sp) {
@@ -177,7 +179,7 @@
}
}
-bool ModuleList::AppendIfNeeded(const ModuleSP &module_sp) {
+bool ModuleList::AppendIfNeeded(const ModuleSP &module_sp, bool notify) {
if (module_sp) {
std::lock_guard<std::recursive_mutex> guard(m_modules_mutex);
collection::iterator pos, end = m_modules.end();
@@ -186,7 +188,7 @@
return false; // Already in the list
}
// Only push module_sp on the list if it wasn't already in there.
- Append(module_sp);
+ Append(module_sp, notify);
return true;
}
return false;
Index: source/Core/DynamicLoader.cpp
===================================================================
--- source/Core/DynamicLoader.cpp
+++ source/Core/DynamicLoader.cpp
@@ -96,7 +96,7 @@
}
if (!executable) {
- executable = target.GetSharedModule(module_spec);
+ executable = target.GetOrCreateModule(module_spec, true /* notify */);
if (executable.get() != target.GetExecutableModulePointer()) {
// Don't load dependent images since we are in dyld where we will
// know and find out about all images that are loaded
@@ -166,7 +166,8 @@
return module_sp;
}
- if ((module_sp = target.GetSharedModule(module_spec))) {
+ if ((module_sp = target.GetOrCreateModule(module_spec,
+ true /* notify */))) {
UpdateLoadedSections(module_sp, link_map_addr, base_addr,
base_addr_is_offset);
return module_sp;
@@ -202,7 +203,8 @@
return module_sp;
}
- if ((module_sp = target.GetSharedModule(new_module_spec))) {
+ if ((module_sp = target.GetOrCreateModule(new_module_spec,
+ true /* notify */))) {
UpdateLoadedSections(module_sp, link_map_addr, base_addr, false);
return module_sp;
}
Index: source/Commands/CommandObjectTarget.cpp
===================================================================
--- source/Commands/CommandObjectTarget.cpp
+++ source/Commands/CommandObjectTarget.cpp
@@ -395,7 +395,8 @@
debugger.GetTargetList().SetSelectedTarget(target_sp.get());
if (must_set_platform_path) {
ModuleSpec main_module_spec(file_spec);
- ModuleSP module_sp = target_sp->GetSharedModule(main_module_spec);
+ ModuleSP module_sp = target_sp->GetOrCreateModule(main_module_spec,
+ true /* notify */);
if (module_sp)
module_sp->SetPlatformFileSpec(remote_file);
}
@@ -2598,7 +2599,8 @@
module_spec.GetSymbolFileSpec() =
m_symbol_file.GetOptionValue().GetCurrentValue();
if (Symbols::DownloadObjectAndSymbolFile(module_spec)) {
- ModuleSP module_sp(target->GetSharedModule(module_spec));
+ ModuleSP module_sp(target->GetOrCreateModule(module_spec,
+ true /* notify */));
if (module_sp) {
result.SetStatus(eReturnStatusSuccessFinishResult);
return true;
@@ -2660,7 +2662,8 @@
if (!module_spec.GetArchitecture().IsValid())
module_spec.GetArchitecture() = target->GetArchitecture();
Status error;
- ModuleSP module_sp(target->GetSharedModule(module_spec, &error));
+ ModuleSP module_sp(target->GetOrCreateModule(module_spec,
+ true /* notify */, &error));
if (!module_sp) {
const char *error_cstr = error.AsCString();
if (error_cstr)
Index: source/API/SBTarget.cpp
===================================================================
--- source/API/SBTarget.cpp
+++ source/API/SBTarget.cpp
@@ -1591,7 +1591,7 @@
if (symfile)
module_spec.GetSymbolFileSpec().SetFile(symfile, FileSpec::Style::native);
- sb_module.SetSP(target_sp->GetSharedModule(module_spec));
+ sb_module.SetSP(target_sp->GetOrCreateModule(module_spec, true /* notify */));
}
return LLDB_RECORD_RESULT(sb_module);
}
@@ -1603,7 +1603,8 @@
lldb::SBModule sb_module;
TargetSP target_sp(GetSP());
if (target_sp)
- sb_module.SetSP(target_sp->GetSharedModule(*module_spec.m_opaque_up));
+ sb_module.SetSP(target_sp->GetOrCreateModule(*module_spec.m_opaque_up,
+ true /* notify */));
return LLDB_RECORD_RESULT(sb_module);
}
Index: include/lldb/Target/Target.h
===================================================================
--- include/lldb/Target/Target.h
+++ include/lldb/Target/Target.h
@@ -505,8 +505,42 @@
static void SetDefaultArchitecture(const ArchSpec &arch);
- lldb::ModuleSP GetSharedModule(const ModuleSpec &module_spec,
- Status *error_ptr = nullptr);
+ //------------------------------------------------------------------
+ /// Find a binary on the system and return its Module,
+ /// or return an existing Module that is already in the Target.
+ ///
+ /// Given a ModuleSpec, find a binary satisifying that specification,
+ /// or identify a matching Module already present in the Target,
+ /// and return a shared pointer to it.
+ ///
+ /// \param[in] module_spec
+ /// The criteria that must be matched for the binary being loaded.
+ /// e.g. UUID, architecture, file path.
+ ///
+ /// \param[in] notify
+ /// If notify is true, and the Module is new to this Target,
+ /// Target::ModulesDidLoad will be called.
+ /// If notify is false, it is assumed that the caller is adding
+ /// multiple Modules and will call ModulesDidLoad with the
+ /// full list at the end.
+ /// ModulesDidLoad must be called when a Module/Modules have
+ /// been added to the target, one way or the other.
+ ///
+ /// \param[out] error_ptr
+ /// Optional argument, pointing to a Status object to fill in
+ /// with any results / messages while attempting to find/load
+ /// this binary. Many callers will be internal functions that
+ /// will handle / summarize the failures in a custom way and
+ /// don't use these messages.
+ ///
+ /// \return
+ /// An empty ModuleSP will be returned if no matching file
+ /// was found. If error_ptr was non-nullptr, an error message
+ /// will likely be provided.
+ //------------------------------------------------------------------
+ lldb::ModuleSP GetOrCreateModule(const ModuleSpec &module_spec,
+ bool notify,
+ Status *error_ptr = nullptr);
//----------------------------------------------------------------------
// Settings accessors
Index: include/lldb/Core/ModuleList.h
===================================================================
--- include/lldb/Core/ModuleList.h
+++ include/lldb/Core/ModuleList.h
@@ -146,12 +146,20 @@
//------------------------------------------------------------------
/// Append a module to the module list.
///
- /// Appends the module to the collection.
- ///
/// \param[in] module_sp
/// A shared pointer to a module to add to this collection.
+ ///
+ /// \param[in] notify
+ /// If true, and a notifier function is set, the notifier function
+ /// will be called. Defaults to true.
+ ///
+ /// When this ModuleList is the Target's ModuleList, the notifier
+ /// function is Target::ModulesDidLoad -- the call to
+ /// ModulesDidLoad may be deferred when adding multiple Modules
+ /// to the Target, but it must be called at the end,
+ /// before resuming execution.
//------------------------------------------------------------------
- void Append(const lldb::ModuleSP &module_sp);
+ void Append(const lldb::ModuleSP &module_sp, bool notify = true);
//------------------------------------------------------------------
/// Append a module to the module list and remove any equivalent modules.
@@ -165,7 +173,22 @@
//------------------------------------------------------------------
void ReplaceEquivalent(const lldb::ModuleSP &module_sp);
- bool AppendIfNeeded(const lldb::ModuleSP &module_sp);
+ //------------------------------------------------------------------
+ /// Append a module to the module list, if it is not already there.
+ ///
+ /// \param[in] module_sp
+ ///
+ /// \param[in] notify
+ /// If true, and a notifier function is set, the notifier function
+ /// will be called. Defaults to true.
+ ///
+ /// When this ModuleList is the Target's ModuleList, the notifier
+ /// function is Target::ModulesDidLoad -- the call to
+ /// ModulesDidLoad may be deferred when adding multiple Modules
+ /// to the Target, but it must be called at the end,
+ /// before resuming execution.
+ //------------------------------------------------------------------
+ bool AppendIfNeeded(const lldb::ModuleSP &module_sp, bool notify = true);
void Append(const ModuleList &module_list);
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
