Skip to content

Commit

Permalink
Fix rendezvous for rebase_exec=true case
Browse files Browse the repository at this point in the history
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
  • Loading branch information
emrekultursay authored and labath committed Sep 27, 2021
1 parent 7424deb commit d5629b5
Show file tree
Hide file tree
Showing 5 changed files with 131 additions and 8 deletions.
Expand Up @@ -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 different 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 different 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);
Expand Down Expand Up @@ -627,6 +631,7 @@ void DynamicLoaderPOSIXDYLD::LoadAllCurrentModules() {
}

m_process->GetTarget().ModulesDidLoad(module_list);
m_initial_modules_added = true;
}

addr_t DynamicLoaderPOSIXDYLD::ComputeLoadOffset() {
Expand Down
9 changes: 9 additions & 0 deletions 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
@@ -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")

1 change: 1 addition & 0 deletions lldb/test/API/functionalities/load_after_attach/b.cpp
@@ -0,0 +1 @@
int LLDB_DYLIB_EXPORT b_function() { return 500; }
45 changes: 45 additions & 0 deletions 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
}

0 comments on commit d5629b5

Please sign in to comment.