aadsm updated this revision to Diff 203761.
aadsm added a comment.

Do not try to use LoadModules again if they failed in the past


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62504/new/

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
@@ -198,6 +198,10 @@
   /// Resolve().
   SOEntryList m_removed_soentries;
 
+  /// By default LoadModules is used (when available) to load or unload module
+  /// changes. If LoadModules fails for some reason then we stop using it.
+  bool m_should_load_modules = true;
+
   /// Threading metadata read from the inferior.
   ThreadInfo m_thread_info;
 
@@ -222,16 +226,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,19 @@
   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_should_load_modules && 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();
+      // If LoadModules fails do not try to use it again.
+      m_should_load_modules = modules_loaded > 0;
+      return m_should_load_modules;
+    }
     return true;
+  }
 
   return UpdateSOEntries();
 }
@@ -178,23 +189,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 +213,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 +222,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

Reply via email to