aadsm created this revision.
aadsm added reviewers: clayborg, xiaobai, labath.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
aadsm edited the summary of this revision.
aadsm added a parent revision: D62503: Add ReadCStringFromMemory for faster
string reads.
This is the sixth and final patch to improve module loading in a series that
started here (where I explain the motivation and solution): D62499
<https://reviews.llvm.org/D62499>
When `xfer:libraries-svr4` is available the Process class has a LoadModules
function that requests this packet and then loads or unloads modules based on
the current list of loaded modules by the process.
This is function is used by DYLDRendezvous to get the list of loaded modules
before and after the module is loaded. However, this is really not needed since
the LoadModules function already loaded or unloaded the modules accordingly. I
changed this strategy to call LoadModules only once (after the process has
loaded the module).
I also removed the `FromRemote` functions & friends because they were only
maintaining the `m_soentries` structure that is never being used in this
context. (I hope I'm not missing anything obvious here).
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D62504
Files:
lldb/include/lldb/Target/Process.h
lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.h
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -204,6 +204,8 @@
size_t LoadModules() override;
+ bool CanLoadModules() override;
+
Status GetFileLoadAddress(const FileSpec &file, bool &is_loaded,
lldb::addr_t &load_addr) override;
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -4826,6 +4826,10 @@
value_is_offset);
}
+bool ProcessGDBRemote::CanLoadModules() {
+ return XMLDocument::XMLEnabled() && m_gdb_comm.GetQXferLibrariesSVR4ReadSupported();
+}
+
size_t ProcessGDBRemote::LoadModules(LoadedModuleInfoList &module_list) {
using lldb_private::process_gdb_remote::ProcessGDBRemote;
Index: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.h
===================================================================
--- lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.h
+++ lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.h
@@ -222,16 +222,7 @@
/// Updates the current set of SOEntries, the set of added entries, and the
/// set of removed entries.
- bool UpdateSOEntries(bool fromRemote = false);
-
- bool FillSOEntryFromModuleInfo(
- LoadedModuleInfoList::LoadedModuleInfo const &modInfo, SOEntry &entry);
-
- bool SaveSOEntriesFromRemote(LoadedModuleInfoList &module_list);
-
- bool AddSOEntriesFromRemote(LoadedModuleInfoList &module_list);
-
- bool RemoveSOEntriesFromRemote(LoadedModuleInfoList &module_list);
+ bool UpdateSOEntries();
bool AddSOEntries();
Index: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
===================================================================
--- lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
+++ lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
@@ -168,8 +168,17 @@
m_previous = m_current;
m_current = info;
- if (UpdateSOEntries(true))
+ // Delegate the loading or unloading of modules to LoadModules when possible
+ // since it will be much faster than reading each library remotely one by one.
+ if (m_process->CanLoadModules()) {
+ // No need to call LoadModules at any time other than eConsistent since
+ // nothing has changed until then.
+ if (m_current.state == eConsistent) {
+ auto modules_loaded = m_process->LoadModules();
+ return modules_loaded > 0;
+ }
return true;
+ }
return UpdateSOEntries();
}
@@ -178,23 +187,18 @@
return m_rendezvous_addr != LLDB_INVALID_ADDRESS;
}
-bool DYLDRendezvous::UpdateSOEntries(bool fromRemote) {
+bool DYLDRendezvous::UpdateSOEntries() {
SOEntry entry;
LoadedModuleInfoList module_list;
- // If we can't get the SO info from the remote, return failure.
- if (fromRemote && m_process->LoadModules(module_list) == 0)
- return false;
-
- if (!fromRemote && m_current.map_addr == 0)
+ if (m_current.map_addr == 0)
return false;
// When the previous and current states are consistent this is the first time
// we have been asked to update. Just take a snapshot of the currently
// loaded modules.
if (m_previous.state == eConsistent && m_current.state == eConsistent)
- return fromRemote ? SaveSOEntriesFromRemote(module_list)
- : TakeSnapshot(m_soentries);
+ return TakeSnapshot(m_soentries);
// If we are about to add or remove a shared object clear out the current
// state and take a snapshot of the currently loaded images.
@@ -207,9 +211,6 @@
return false;
m_soentries.clear();
- if (fromRemote)
- return SaveSOEntriesFromRemote(module_list);
-
m_added_soentries.clear();
m_removed_soentries.clear();
return TakeSnapshot(m_soentries);
@@ -219,115 +220,13 @@
// Otherwise check the previous state to determine what to expect and update
// accordingly.
if (m_previous.state == eAdd)
- return fromRemote ? AddSOEntriesFromRemote(module_list) : AddSOEntries();
+ return AddSOEntries();
else if (m_previous.state == eDelete)
- return fromRemote ? RemoveSOEntriesFromRemote(module_list)
- : RemoveSOEntries();
+ return RemoveSOEntries();
return false;
}
-bool DYLDRendezvous::FillSOEntryFromModuleInfo(
- LoadedModuleInfoList::LoadedModuleInfo const &modInfo, SOEntry &entry) {
- addr_t link_map_addr;
- addr_t base_addr;
- addr_t dyn_addr;
- std::string name;
-
- if (!modInfo.get_link_map(link_map_addr) || !modInfo.get_base(base_addr) ||
- !modInfo.get_dynamic(dyn_addr) || !modInfo.get_name(name))
- return false;
-
- entry.link_addr = link_map_addr;
- entry.base_addr = base_addr;
- entry.dyn_addr = dyn_addr;
-
- entry.file_spec.SetFile(name, FileSpec::Style::native);
-
- UpdateBaseAddrIfNecessary(entry, name);
-
- // not needed if we're using ModuleInfos
- entry.next = 0;
- entry.prev = 0;
- entry.path_addr = 0;
-
- return true;
-}
-
-bool DYLDRendezvous::SaveSOEntriesFromRemote(
- LoadedModuleInfoList &module_list) {
- for (auto const &modInfo : module_list.m_list) {
- SOEntry entry;
- if (!FillSOEntryFromModuleInfo(modInfo, entry))
- return false;
-
- // Only add shared libraries and not the executable.
- if (!SOEntryIsMainExecutable(entry))
- m_soentries.push_back(entry);
- }
-
- m_loaded_modules = module_list;
- return true;
-}
-
-bool DYLDRendezvous::AddSOEntriesFromRemote(LoadedModuleInfoList &module_list) {
- for (auto const &modInfo : module_list.m_list) {
- bool found = false;
- for (auto const &existing : m_loaded_modules.m_list) {
- if (modInfo == existing) {
- found = true;
- break;
- }
- }
-
- if (found)
- continue;
-
- SOEntry entry;
- if (!FillSOEntryFromModuleInfo(modInfo, entry))
- return false;
-
- // Only add shared libraries and not the executable.
- if (!SOEntryIsMainExecutable(entry))
- m_soentries.push_back(entry);
- }
-
- m_loaded_modules = module_list;
- return true;
-}
-
-bool DYLDRendezvous::RemoveSOEntriesFromRemote(
- LoadedModuleInfoList &module_list) {
- for (auto const &existing : m_loaded_modules.m_list) {
- bool found = false;
- for (auto const &modInfo : module_list.m_list) {
- if (modInfo == existing) {
- found = true;
- break;
- }
- }
-
- if (found)
- continue;
-
- SOEntry entry;
- if (!FillSOEntryFromModuleInfo(existing, entry))
- return false;
-
- // Only add shared libraries and not the executable.
- if (!SOEntryIsMainExecutable(entry)) {
- auto pos = std::find(m_soentries.begin(), m_soentries.end(), entry);
- if (pos == m_soentries.end())
- return false;
-
- m_soentries.erase(pos);
- }
- }
-
- m_loaded_modules = module_list;
- return true;
-}
-
bool DYLDRendezvous::AddSOEntries() {
SOEntry entry;
iterator pos;
Index: lldb/include/lldb/Target/Process.h
===================================================================
--- lldb/include/lldb/Target/Process.h
+++ lldb/include/lldb/Target/Process.h
@@ -685,6 +685,8 @@
virtual size_t LoadModules(LoadedModuleInfoList &) { return 0; }
+ virtual bool CanLoadModules() { return false; }
+
protected:
virtual JITLoaderList &GetJITLoaders();
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
