Author: Pavel Labath Date: 2022-07-01T14:36:01+02:00 New Revision: b15b1421bc9a11b318b65b489e5fd58dd917db1f
URL: https://github.com/llvm/llvm-project/commit/b15b1421bc9a11b318b65b489e5fd58dd917db1f DIFF: https://github.com/llvm/llvm-project/commit/b15b1421bc9a11b318b65b489e5fd58dd917db1f.diff LOG: [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 a997a1d7fbe229433fb458bb0035b32424ecf3bd. Added: lldb/test/API/tools/lldb-server/attach-wait/Makefile lldb/test/API/tools/lldb-server/attach-wait/TestGdbRemoteAttachWait.py lldb/test/API/tools/lldb-server/attach-wait/main.cpp lldb/test/API/tools/lldb-server/attach-wait/shim.cpp Modified: lldb/packages/Python/lldbsuite/test/lldbplatformutil.py lldb/packages/Python/lldbsuite/test/lldbtest.py Removed: lldb/test/API/tools/lldb-server/TestGdbRemoteAttachWait.py ################################################################################ diff --git a/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py b/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py index e14c4f8c0d38..719131c9248e 100644 --- a/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py +++ b/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py @@ -4,12 +4,11 @@ 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 diff --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py b/lldb/packages/Python/lldbsuite/test/lldbtest.py index 22851730153e..d46e54f30bd5 100644 --- a/lldb/packages/Python/lldbsuite/test/lldbtest.py +++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py @@ -393,7 +393,6 @@ def launch(self, executable, args, extra_env): stdout=open( os.devnull) if not self._trace_on else None, stdin=PIPE, - preexec_fn=lldbplatformutil.enable_attach, env=env) def terminate(self): diff --git a/lldb/test/API/tools/lldb-server/attach-wait/Makefile b/lldb/test/API/tools/lldb-server/attach-wait/Makefile new file mode 100644 index 000000000000..22f1051530f8 --- /dev/null +++ b/lldb/test/API/tools/lldb-server/attach-wait/Makefile @@ -0,0 +1 @@ +include Makefile.rules diff --git a/lldb/test/API/tools/lldb-server/TestGdbRemoteAttachWait.py b/lldb/test/API/tools/lldb-server/attach-wait/TestGdbRemoteAttachWait.py similarity index 80% rename from lldb/test/API/tools/lldb-server/TestGdbRemoteAttachWait.py rename to lldb/test/API/tools/lldb-server/attach-wait/TestGdbRemoteAttachWait.py index ed31b5645c33..abcc98d22090 100644 --- a/lldb/test/API/tools/lldb-server/TestGdbRemoteAttachWait.py +++ b/lldb/test/API/tools/lldb-server/attach-wait/TestGdbRemoteAttachWait.py @@ -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 @@ def launch_inferior(): 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 @@ def launch_inferior(): 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() diff --git a/lldb/test/API/tools/lldb-server/attach-wait/main.cpp b/lldb/test/API/tools/lldb-server/attach-wait/main.cpp new file mode 100644 index 000000000000..891dc9f5e7d1 --- /dev/null +++ b/lldb/test/API/tools/lldb-server/attach-wait/main.cpp @@ -0,0 +1,8 @@ +#include <thread> + +int main() +{ + // Wait to be attached. + std::this_thread::sleep_for(std::chrono::minutes(1)); + return 0; +} diff --git a/lldb/test/API/tools/lldb-server/attach-wait/shim.cpp b/lldb/test/API/tools/lldb-server/attach-wait/shim.cpp new file mode 100644 index 000000000000..33d75b195762 --- /dev/null +++ b/lldb/test/API/tools/lldb-server/attach-wait/shim.cpp @@ -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; +} _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
