[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