[Lldb-commits] [lldb] 5535582 - [lldb/dyld-posix] Avoid reading the module list in inconsistent states

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2022-07-01T08:08:22+02:00
New Revision: 553558292eaef8867940856bd5272478cc51f4e0

URL: https://github.com/llvm/llvm-project/commit/553558292eaef8867940856bd5272478cc51f4e0
DIFF: https://github.com/llvm/llvm-project/commit/553558292eaef8867940856bd5272478cc51f4e0.diff

LOG: [lldb/dyld-posix] Avoid reading the module list in inconsistent states

New glibc versions (since 2.34 or including this
<https://github.com/bminor/glibc/commit/ed3ce71f5c64c5f07cbde0ef03554ea8950d8f2c>
patch) trigger the rendezvous breakpoint after they have already added
some modules to the list. This did not play well with our dynamic
loader plugin which was doing a diff of the the reported modules in the
before (RT_ADD) and after (RT_CONSISTENT) states. Specifically, it
caused us to miss some of the modules.

While I think the old behavior makes more sense, I don't think that lldb
is doing the right thing either, as the documentation states that we
should not be expecting a consistent view in the RT_ADD (and RT_DELETE)
states.

Therefore, this patch changes the lldb algorithm to compare the module
list against the previous consistent snapshot. This fixes the previous
issue, and I believe it is more correct in general. It also reduces the
number of times we are fetching the module info, which should speed up
the debugging of processes with many shared libraries.

The change in RefreshModules ensures we don't broadcast the loaded
notification for the dynamic loader (ld.so) module more than once.

Differential Revision: https://reviews.llvm.org/D128264

Added: 
    

Modified: 
    lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
    lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
index ec7e91df2c59..4869cf0fd9c8 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
@@ -210,14 +210,7 @@ DYLDRendezvous::RendezvousAction DYLDRendezvous::GetAction() const {
 
   case eAdd:
   case eDelete:
-    // Some versions of the android dynamic linker might send two
-    // notifications with state == eAdd back to back. Ignore them until we
-    // get an eConsistent notification.
-    if (!(m_previous.state == eConsistent ||
-          (m_previous.state == eAdd && m_current.state == eDelete)))
-      return eNoAction;
-
-    return eTakeSnapshot;
+    return eNoAction;
   }
 
   return eNoAction;
@@ -229,9 +222,9 @@ bool DYLDRendezvous::UpdateSOEntriesFromRemote() {
   if (action == eNoAction)
     return false;
 
+  m_added_soentries.clear();
+  m_removed_soentries.clear();
   if (action == eTakeSnapshot) {
-    m_added_soentries.clear();
-    m_removed_soentries.clear();
     // We already have the loaded list from the previous update so no need to
     // find all the modules again.
     if (!m_loaded_modules.m_list.empty())
@@ -260,11 +253,11 @@ bool DYLDRendezvous::UpdateSOEntriesFromRemote() {
 }
 
 bool DYLDRendezvous::UpdateSOEntries() {
+  m_added_soentries.clear();
+  m_removed_soentries.clear();
   switch (GetAction()) {
   case eTakeSnapshot:
     m_soentries.clear();
-    m_added_soentries.clear();
-    m_removed_soentries.clear();
     return TakeSnapshot(m_soentries);
   case eAddModules:
     return AddSOEntries();

diff  --git a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
index 64bc5323fc4e..8a708c1f9898 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -433,27 +433,31 @@ void DynamicLoaderPOSIXDYLD::RefreshModules() {
     for (; I != E; ++I) {
       ModuleSP module_sp =
           LoadModuleAtAddress(I->file_spec, I->link_addr, I->base_addr, true);
-      if (module_sp.get()) {
-        if (module_sp->GetObjectFile()->GetBaseAddress().GetLoadAddress(
-                &m_process->GetTarget()) == m_interpreter_base &&
-            module_sp != m_interpreter_module.lock()) {
-          if (m_interpreter_module.lock() == nullptr) {
-            m_interpreter_module = module_sp;
-          } else {
-            // If this is a duplicate instance of ld.so, unload it.  We may end
-            // up with it if we load it via a 
diff erent path than before
-            // (symlink vs real path).
-            // TODO: remove this once we either fix library matching or avoid
-            // loading the interpreter when setting the rendezvous breakpoint.
-            UnloadSections(module_sp);
-            loaded_modules.Remove(module_sp);
-            continue;
-          }
+      if (!module_sp.get())
+        continue;
+
+      if (module_sp->GetObjectFile()->GetBaseAddress().GetLoadAddress(
+              &m_process->GetTarget()) == m_interpreter_base) {
+        ModuleSP interpreter_sp = m_interpreter_module.lock();
+        if (m_interpreter_module.lock() == nullptr) {
+          m_interpreter_module = module_sp;
+        } else if (module_sp == interpreter_sp) {
+          // Module already loaded.
+          continue;
+        } else {
+          // If this is a duplicate instance of ld.so, unload it.  We may end
+          // up with it if we load it via a 
diff erent path than before
+          // (symlink vs real path).
+          // TODO: remove this once we either fix library matching or avoid
+          // loading the interpreter when setting the rendezvous breakpoint.
+          UnloadSections(module_sp);
+          loaded_modules.Remove(module_sp);
+          continue;
         }
-
-        loaded_modules.AppendIfNeeded(module_sp);
-        new_modules.Append(module_sp);
       }
+
+      loaded_modules.AppendIfNeeded(module_sp);
+      new_modules.Append(module_sp);
     }
     m_process->GetTarget().ModulesDidLoad(new_modules);
   }