[Lldb-commits] [lldb] a83a825 - Make sure the interpreter module was loaded before making checks against it
António Afonso via lldb-commits
lldb-commits at lists.llvm.org
Sun Feb 21 09:28:34 PST 2021
Author: António Afonso
Date: 2021-02-21T09:28:04-08:00
New Revision: a83a825e9902b54b315870e9ed85723525208f09
URL: https://github.com/llvm/llvm-project/commit/a83a825e9902b54b315870e9ed85723525208f09
DIFF: https://github.com/llvm/llvm-project/commit/a83a825e9902b54b315870e9ed85723525208f09.diff
LOG: Make sure the interpreter module was loaded before making checks against it
This issue was introduced in https://reviews.llvm.org/D92187.
The guard I'm changing were is supposed to act when linux is loading the linker for the second time (due to differences in paths like symlinks).
This is done by checking `module_sp != m_interpreter_module.lock()` however this will be true when `m_interpreter_module` wasn't initialized, making linux unload the linker module (the most visible result here is that lldb will stop getting notified about new modules loaded by the process, because it can't set the rendezvous breakpoint again after the stepping over it once).
The `m_interpreter_module` is not getting initialize when it goes through this path: https://github.com/llvm/llvm-project/blob/dbfdb139f75470a9abc78e7c9faf743fdd963c2d/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp#L332, which happens when lldb was able to read the address from the dynamic section of the executable.
What I'm not sure about though, is if when we go through this path if we still load the linker twice on linux. If that's the case then it means we need to somehow set the m_interpreter_module instead of the fix I provide here. I've only tested this on Android.
Differential Revision: https://reviews.llvm.org/D96637
Added:
lldb/test/API/functionalities/module_load_attach/Makefile
lldb/test/API/functionalities/module_load_attach/TestModuleLoadAttach.py
lldb/test/API/functionalities/module_load_attach/feature.c
lldb/test/API/functionalities/module_load_attach/main.c
Modified:
lldb/packages/Python/lldbsuite/test/decorators.py
lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
Removed:
################################################################################
diff --git a/lldb/packages/Python/lldbsuite/test/decorators.py b/lldb/packages/Python/lldbsuite/test/decorators.py
index 6d781defdea46..628b0303916c2 100644
--- a/lldb/packages/Python/lldbsuite/test/decorators.py
+++ b/lldb/packages/Python/lldbsuite/test/decorators.py
@@ -622,6 +622,10 @@ def skipUnlessDarwin(func):
"""Decorate the item to skip tests that should be skipped on any non Darwin platform."""
return skipUnlessPlatform(lldbplatformutil.getDarwinOSTriples())(func)
+def skipUnlessLinux(func):
+ """Decorate the item to skip tests that should be skipped on any non-Linux platform."""
+ return skipUnlessPlatform(["linux"])(func)
+
def skipUnlessTargetAndroid(func):
return unittest2.skipUnless(lldbplatformutil.target_is_android(),
"requires target to be Android")(func)
diff --git a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
index 160faa74af239..720fc16148f2a 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -441,6 +441,7 @@ void DynamicLoaderPOSIXDYLD::RefreshModules() {
if (module_sp.get()) {
if (module_sp->GetObjectFile()->GetBaseAddress().GetLoadAddress(
&m_process->GetTarget()) == m_interpreter_base &&
+ m_interpreter_module.lock() &&
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
diff --git a/lldb/test/API/functionalities/module_load_attach/Makefile b/lldb/test/API/functionalities/module_load_attach/Makefile
new file mode 100644
index 0000000000000..7b7435fdb24e9
--- /dev/null
+++ b/lldb/test/API/functionalities/module_load_attach/Makefile
@@ -0,0 +1,10 @@
+C_SOURCES := main.c
+LD_EXTRAS := -Wl,-rpath "-Wl,$(shell pwd)"
+USE_LIBDL := 1
+
+feature:
+ $(MAKE) -f $(MAKEFILE_RULES) \
+ DYLIB_ONLY=YES DYLIB_NAME=feature DYLIB_C_SOURCES=feature.c
+all: feature
+
+include Makefile.rules
diff --git a/lldb/test/API/functionalities/module_load_attach/TestModuleLoadAttach.py b/lldb/test/API/functionalities/module_load_attach/TestModuleLoadAttach.py
new file mode 100644
index 0000000000000..71759dcb88e67
--- /dev/null
+++ b/lldb/test/API/functionalities/module_load_attach/TestModuleLoadAttach.py
@@ -0,0 +1,49 @@
+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__)
+
+ def build_launch_and_attach(self):
+ self.build()
+ # launch
+ exe = self.getBuildArtifact("a.out")
+ popen = self.spawnSubprocess(exe)
+ # attach
+ target = self.dbg.CreateTarget(exe)
+ self.assertTrue(target, VALID_TARGET)
+ listener = lldb.SBListener("my.attach.listener")
+ error = lldb.SBError()
+ process = target.AttachToProcessWithID(listener, popen.pid, error)
+ self.assertTrue(error.Success() and process, PROCESS_IS_VALID)
+ return process
+
+ def assertModuleIsLoaded(self, module_name):
+ feature_module = self.dbg.GetSelectedTarget().FindModule(lldb.SBFileSpec(module_name))
+ self.assertTrue(feature_module.IsValid(), f"Module {module_name} should be loaded")
+
+ def assertModuleIsNotLoaded(self, module_name):
+ feature_module = self.dbg.GetSelectedTarget().FindModule(lldb.SBFileSpec(module_name))
+ self.assertFalse(feature_module.IsValid(), f"Module {module_name} should not be loaded")
+
+ @skipIfRemote
+ @skipUnlessLinux
+ @no_debug_info_test
+ def test(self):
+ '''
+ This test makes sure that after attach lldb still gets notifications
+ about new modules being loaded by the process
+ '''
+ process = self.build_launch_and_attach()
+ thread = process.GetSelectedThread()
+ self.assertModuleIsNotLoaded("libfeature.so")
+ thread.GetSelectedFrame().EvaluateExpression("flip_to_1_to_continue = 1")
+ # Continue so that dlopen is called.
+ breakpoint = self.target().BreakpointCreateBySourceRegex(
+ "// break after dlopen", lldb.SBFileSpec("main.c"))
+ self.assertNotEqual(breakpoint.GetNumResolvedLocations(), 0)
+ stopped_threads = lldbutil.continue_to_breakpoint(self.process(), breakpoint)
+ self.assertModuleIsLoaded("libfeature.so")
diff --git a/lldb/test/API/functionalities/module_load_attach/feature.c b/lldb/test/API/functionalities/module_load_attach/feature.c
new file mode 100644
index 0000000000000..122312048e8df
--- /dev/null
+++ b/lldb/test/API/functionalities/module_load_attach/feature.c
@@ -0,0 +1 @@
+extern void feature() {}
diff --git a/lldb/test/API/functionalities/module_load_attach/main.c b/lldb/test/API/functionalities/module_load_attach/main.c
new file mode 100644
index 0000000000000..6953b6488f999
--- /dev/null
+++ b/lldb/test/API/functionalities/module_load_attach/main.c
@@ -0,0 +1,15 @@
+#include <dlfcn.h>
+#include <assert.h>
+#include <unistd.h>
+
+volatile int flip_to_1_to_continue = 0;
+
+int main() {
+ lldb_enable_attach();
+ while (! flip_to_1_to_continue) // Wait for debugger to attach
+ sleep(1);
+ // dlopen the feature
+ void *feature = dlopen("libfeature.so", RTLD_NOW);
+ assert(feature && "dlopen failed?");
+ return 0; // break after dlopen
+}
More information about the lldb-commits
mailing list