[lldb/test] Don't use preexec_fn for launching inferiors

As the documentation states, using this is not safe in multithreaded
programs, and I have traced it to a rare deadlock in some of the tests.

The reason this was introduced was to be able to attach to a program
from the very first instruction, where our usual mechanism of
synchronization -- waiting for a file to appear -- does not work.

However, this is only needed for a single test
(TestGdbRemoteAttachWait) so instead of doing this everywhere, I create
a bespoke solution for that single test. The solution basically
consists of outsourcing the preexec_fn code to a separate (and
single-threaded) shim process, which enables attaching and then executes
the real program.

This pattern could be generalized in case we needed to use it for other
tests, but I suspect that we will not be having many tests like this.

This effectively reverts commit
a997a1d7fb.
This commit is contained in:
Pavel Labath
2022-07-01 14:32:50 +02:00
parent fabe915705
commit b15b1421bc
6 changed files with 32 additions and 17 deletions

View File

@@ -4,12 +4,11 @@ architecture and/or the platform dependent nature of the tests. """
from __future__ import absolute_import
# System modules
import ctypes
import itertools
import os
import re
import subprocess
import sys
import os
# Third-party modules
import six
@@ -192,14 +191,3 @@ def hasChattyStderr(test_case):
if match_android_device(test_case.getArchitecture(), ['aarch64'], range(22, 25+1)):
return True # The dynamic linker on the device will complain about unknown DT entries
return False
if getHostPlatform() == "linux":
def enable_attach():
"""Enable attaching to _this_ process, if host requires such an action.
Suitable for use as a preexec_fn in subprocess.Popen and similar."""
c = ctypes.CDLL(None)
PR_SET_PTRACER = ctypes.c_int(0x59616d61)
PR_SET_PTRACER_ANY = ctypes.c_ulong(-1)
c.prctl(PR_SET_PTRACER, PR_SET_PTRACER_ANY)
else:
enable_attach = None

View File

@@ -393,7 +393,6 @@ class _LocalProcess(_BaseProcess):
stdout=open(
os.devnull) if not self._trace_on else None,
stdin=PIPE,
preexec_fn=lldbplatformutil.enable_attach,
env=env)
def terminate(self):

View File

@@ -0,0 +1 @@
include Makefile.rules

View File

@@ -14,10 +14,12 @@ class TestGdbRemoteAttachWait(gdbremote_testcase.GdbRemoteTestCaseBase):
@skipIfWindows # This test is flaky on Windows
def test_attach_with_vAttachWait(self):
exe = '%s_%d' % (self.testMethodName, os.getpid())
exe_to_attach = exe
args = []
def launch_inferior():
inferior = self.launch_process_for_attach(
inferior_args=["sleep:60"],
inferior_args=args,
exe_path=self.getBuildArtifact(exe))
self.assertIsNotNone(inferior)
self.assertTrue(inferior.pid > 0)
@@ -26,7 +28,14 @@ class TestGdbRemoteAttachWait(gdbremote_testcase.GdbRemoteTestCaseBase):
inferior.pid, True))
return inferior
self.build(dictionary={'EXE': exe})
self.build(dictionary={'EXE': exe, 'CXX_SOURCES': 'main.cpp'})
if self.getPlatform() != "windows":
# Use a shim to ensure that the process is ready to be attached from
# the get-go.
args = [self.getBuildArtifact(exe)]
exe = "shim"
self.build(dictionary={'EXE': exe, 'CXX_SOURCES': 'shim.cpp'})
self.set_inferior_startup_attach_manually()
server = self.connect_to_debug_monitor()
@@ -38,7 +47,8 @@ class TestGdbRemoteAttachWait(gdbremote_testcase.GdbRemoteTestCaseBase):
self.do_handshake()
self.test_sequence.add_log_lines([
# Do the attach.
"read packet: $vAttachWait;{}#00".format(lldbgdbserverutils.gdbremote_hex_encode_string(exe)),
"read packet: $vAttachWait;{}#00".format(
lldbgdbserverutils.gdbremote_hex_encode_string(exe_to_attach)),
], True)
# Run the stream until attachWait.
context = self.expect_gdbremote_sequence()

View File

@@ -0,0 +1,8 @@
#include <thread>
int main()
{
// Wait to be attached.
std::this_thread::sleep_for(std::chrono::minutes(1));
return 0;
}

View File

@@ -0,0 +1,9 @@
#include <unistd.h>
#include <cstdio>
int main(int argc, char *argv[]) {
lldb_enable_attach();
execlp(argv[1], argv[1], nullptr);
perror("execlp");
return 1;
}