[Lldb-commits] [lldb] d5629b5 - Fix rendezvous for rebase_exec=true case

Pavel Labath via lldb-commits lldb-commits at lists.llvm.org
Mon Sep 27 04:27:40 PDT 2021


Author: Emre Kultursay
Date: 2021-09-27T13:27:27+02:00
New Revision: d5629b5d4d41ce71703105362f58dfcdbb6cc175

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

LOG: Fix rendezvous for rebase_exec=true case

When rebase_exec=true in DidAttach(), all modules are loaded
before the rendezvous breakpoint is set, which means the
LoadInterpreterModule() method is not called and m_interpreter_module
is not initialized.

This causes the very first rendezvous breakpoint hit with
m_initial_modules_added=false to accidentally unload the
module_sp that corresponds to the dynamic loader.

This bug (introduced in D92187) was causing the rendezvous
mechanism to not work in Android 28. The mechanism works
fine on older/newer versions of Android.

Test: Verified rendezvous on Android 28 and 29
Test: Added dlopen test

Reviewed By: labath

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

Added: 
    lldb/test/API/functionalities/load_after_attach/Makefile
    lldb/test/API/functionalities/load_after_attach/TestLoadAfterAttach.py
    lldb/test/API/functionalities/load_after_attach/b.cpp
    lldb/test/API/functionalities/load_after_attach/main.cpp

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

Removed: 
    


################################################################################
diff  --git a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
index 0bb383ff0c00..7732f27d27ca 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -449,14 +449,18 @@ void DynamicLoaderPOSIXDYLD::RefreshModules() {
         if (module_sp->GetObjectFile()->GetBaseAddress().GetLoadAddress(
                 &m_process->GetTarget()) == m_interpreter_base &&
             module_sp != m_interpreter_module.lock()) {
-          // 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 (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;
+          }
         }
 
         loaded_modules.AppendIfNeeded(module_sp);
@@ -627,6 +631,7 @@ void DynamicLoaderPOSIXDYLD::LoadAllCurrentModules() {
   }
 
   m_process->GetTarget().ModulesDidLoad(module_list);
+  m_initial_modules_added = true;
 }
 
 addr_t DynamicLoaderPOSIXDYLD::ComputeLoadOffset() {

diff  --git a/lldb/test/API/functionalities/load_after_attach/Makefile b/lldb/test/API/functionalities/load_after_attach/Makefile
new file mode 100644
index 000000000000..0f3fb37bdadf
--- /dev/null
+++ b/lldb/test/API/functionalities/load_after_attach/Makefile
@@ -0,0 +1,9 @@
+CXX_SOURCES := main.cpp
+USE_LIBDL := 1
+
+lib_b:
+	$(MAKE) -f $(MAKEFILE_RULES) \
+		DYLIB_ONLY=YES DYLIB_CXX_SOURCES=b.cpp DYLIB_NAME=lib_b
+all: lib_b
+
+include Makefile.rules

diff  --git a/lldb/test/API/functionalities/load_after_attach/TestLoadAfterAttach.py b/lldb/test/API/functionalities/load_after_attach/TestLoadAfterAttach.py
new file mode 100644
index 000000000000..0e9b3c40ff2b
--- /dev/null
+++ b/lldb/test/API/functionalities/load_after_attach/TestLoadAfterAttach.py
@@ -0,0 +1,63 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestCase(TestBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+
+    @skipIfRemote
+    def test_load_after_attach(self):
+        self.build()
+
+        ctx = self.platformContext
+        lib_name = ctx.shlib_prefix + 'lib_b.' + ctx.shlib_extension
+
+        exe = self.getBuildArtifact("a.out")
+        lib = self.getBuildArtifact(lib_name)
+
+        # Spawn a new process.
+        # use realpath to workaround llvm.org/pr48376
+        # Pass path to solib for dlopen to properly locate the library.
+        popen = self.spawnSubprocess(os.path.realpath(exe), args = [os.path.realpath(lib)])
+        pid = popen.pid
+
+        # Attach to the spawned process.
+        self.runCmd("process attach -p " + str(pid))
+
+        target = self.dbg.GetSelectedTarget()
+        process = target.GetProcess()
+        self.assertTrue(process, PROCESS_IS_VALID)
+
+        # Continue until first breakpoint.
+        breakpoint1 = self.target().BreakpointCreateBySourceRegex(
+            "// break here", lldb.SBFileSpec("main.cpp"))
+        self.assertEqual(breakpoint1.GetNumResolvedLocations(), 1)
+        stopped_threads = lldbutil.continue_to_breakpoint(self.process(), breakpoint1)
+        self.assertEqual(len(stopped_threads), 1)
+
+        # Check that image list does not contain liblib_b before dlopen.
+        self.match(
+                "image list",
+                patterns = [lib_name],
+                matching = False,
+                msg = lib_name + " should not have been in image list")
+
+        # Change a variable to escape the loop
+        self.runCmd("expression main_thread_continue = 1")
+
+        # Continue so that dlopen is called.
+        breakpoint2 = self.target().BreakpointCreateBySourceRegex(
+            "// break after dlopen", lldb.SBFileSpec("main.cpp"))
+        self.assertEqual(breakpoint2.GetNumResolvedLocations(), 1)
+        stopped_threads = lldbutil.continue_to_breakpoint(self.process(), breakpoint2)
+        self.assertEqual(len(stopped_threads), 1)
+
+        # Check that image list contains liblib_b after dlopen.
+        self.match(
+                "image list",
+                patterns = [lib_name],
+                matching = True,
+                msg = lib_name + " missing in image list")
+

diff  --git a/lldb/test/API/functionalities/load_after_attach/b.cpp b/lldb/test/API/functionalities/load_after_attach/b.cpp
new file mode 100644
index 000000000000..911447bee489
--- /dev/null
+++ b/lldb/test/API/functionalities/load_after_attach/b.cpp
@@ -0,0 +1 @@
+int LLDB_DYLIB_EXPORT b_function() { return 500; }

diff  --git a/lldb/test/API/functionalities/load_after_attach/main.cpp b/lldb/test/API/functionalities/load_after_attach/main.cpp
new file mode 100644
index 000000000000..d63bd7e2b8cd
--- /dev/null
+++ b/lldb/test/API/functionalities/load_after_attach/main.cpp
@@ -0,0 +1,45 @@
+#ifdef _WIN32
+#include <Windows.h>
+#else
+#include <dlfcn.h>
+#include <unistd.h>
+#endif
+
+#include <assert.h>
+#include <stdio.h>
+#include <thread>
+#include <chrono>
+
+// We do not use the dylib.h implementation, because
+// we need to pass full path to the dylib.
+void* dylib_open(const char* full_path) {
+#ifdef _WIN32
+  return LoadLibraryA(full_path);
+#else
+  return dlopen(full_path, RTLD_LAZY);
+#endif
+}
+
+int main(int argc, char* argv[]) {
+  assert(argc == 2 && "argv[1] must be the full path to lib_b library");
+  const char* dylib_full_path= argv[1];
+  printf("Using dylib at: %s\n", dylib_full_path);
+
+  // Wait until debugger is attached.
+  int main_thread_continue = 0;
+  int i = 0;
+  int timeout = 10;
+  for (i = 0; i < timeout; i++) {
+    std::this_thread::sleep_for(std::chrono::seconds(1));  // break here
+    if (main_thread_continue) {
+      break;
+    }
+  }
+  assert(i != timeout && "timed out waiting for debugger");
+
+  // dlopen the 'liblib_b.so' shared library.
+  void* dylib_handle = dylib_open(dylib_full_path);
+  assert(dylib_handle && "dlopen failed");
+
+  return i; // break after dlopen
+}


        


More information about the lldb-commits mailing list