Skip to content

Commit a83a825

Browse files
committed
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
1 parent 7c706aa commit a83a825

File tree

6 files changed

+80
-0
lines changed

6 files changed

+80
-0
lines changed

lldb/packages/Python/lldbsuite/test/decorators.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -622,6 +622,10 @@ def skipUnlessDarwin(func):
622622
"""Decorate the item to skip tests that should be skipped on any non Darwin platform."""
623623
return skipUnlessPlatform(lldbplatformutil.getDarwinOSTriples())(func)
624624

625+
def skipUnlessLinux(func):
626+
"""Decorate the item to skip tests that should be skipped on any non-Linux platform."""
627+
return skipUnlessPlatform(["linux"])(func)
628+
625629
def skipUnlessTargetAndroid(func):
626630
return unittest2.skipUnless(lldbplatformutil.target_is_android(),
627631
"requires target to be Android")(func)

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -441,6 +441,7 @@ void DynamicLoaderPOSIXDYLD::RefreshModules() {
441441
if (module_sp.get()) {
442442
if (module_sp->GetObjectFile()->GetBaseAddress().GetLoadAddress(
443443
&m_process->GetTarget()) == m_interpreter_base &&
444+
m_interpreter_module.lock() &&
444445
module_sp != m_interpreter_module.lock()) {
445446
// If this is a duplicate instance of ld.so, unload it. We may end up
446447
// with it if we load it via a different path than before (symlink
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
C_SOURCES := main.c
2+
LD_EXTRAS := -Wl,-rpath "-Wl,$(shell pwd)"
3+
USE_LIBDL := 1
4+
5+
feature:
6+
$(MAKE) -f $(MAKEFILE_RULES) \
7+
DYLIB_ONLY=YES DYLIB_NAME=feature DYLIB_C_SOURCES=feature.c
8+
all: feature
9+
10+
include Makefile.rules
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
import lldb
2+
from lldbsuite.test.decorators import *
3+
from lldbsuite.test.lldbtest import *
4+
from lldbsuite.test import lldbutil
5+
6+
class TestCase(TestBase):
7+
8+
mydir = TestBase.compute_mydir(__file__)
9+
10+
def build_launch_and_attach(self):
11+
self.build()
12+
# launch
13+
exe = self.getBuildArtifact("a.out")
14+
popen = self.spawnSubprocess(exe)
15+
# attach
16+
target = self.dbg.CreateTarget(exe)
17+
self.assertTrue(target, VALID_TARGET)
18+
listener = lldb.SBListener("my.attach.listener")
19+
error = lldb.SBError()
20+
process = target.AttachToProcessWithID(listener, popen.pid, error)
21+
self.assertTrue(error.Success() and process, PROCESS_IS_VALID)
22+
return process
23+
24+
def assertModuleIsLoaded(self, module_name):
25+
feature_module = self.dbg.GetSelectedTarget().FindModule(lldb.SBFileSpec(module_name))
26+
self.assertTrue(feature_module.IsValid(), f"Module {module_name} should be loaded")
27+
28+
def assertModuleIsNotLoaded(self, module_name):
29+
feature_module = self.dbg.GetSelectedTarget().FindModule(lldb.SBFileSpec(module_name))
30+
self.assertFalse(feature_module.IsValid(), f"Module {module_name} should not be loaded")
31+
32+
@skipIfRemote
33+
@skipUnlessLinux
34+
@no_debug_info_test
35+
def test(self):
36+
'''
37+
This test makes sure that after attach lldb still gets notifications
38+
about new modules being loaded by the process
39+
'''
40+
process = self.build_launch_and_attach()
41+
thread = process.GetSelectedThread()
42+
self.assertModuleIsNotLoaded("libfeature.so")
43+
thread.GetSelectedFrame().EvaluateExpression("flip_to_1_to_continue = 1")
44+
# Continue so that dlopen is called.
45+
breakpoint = self.target().BreakpointCreateBySourceRegex(
46+
"// break after dlopen", lldb.SBFileSpec("main.c"))
47+
self.assertNotEqual(breakpoint.GetNumResolvedLocations(), 0)
48+
stopped_threads = lldbutil.continue_to_breakpoint(self.process(), breakpoint)
49+
self.assertModuleIsLoaded("libfeature.so")
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
extern void feature() {}
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
#include <dlfcn.h>
2+
#include <assert.h>
3+
#include <unistd.h>
4+
5+
volatile int flip_to_1_to_continue = 0;
6+
7+
int main() {
8+
lldb_enable_attach();
9+
while (! flip_to_1_to_continue) // Wait for debugger to attach
10+
sleep(1);
11+
// dlopen the feature
12+
void *feature = dlopen("libfeature.so", RTLD_NOW);
13+
assert(feature && "dlopen failed?");
14+
return 0; // break after dlopen
15+
}

0 commit comments

Comments
 (0)