Author: Pavel Labath Date: 2022-07-07T14:38:33+02:00 New Revision: 82ba3f44657ac91b4bef95dbf647dfbab482e502
URL: https://github.com/llvm/llvm-project/commit/82ba3f44657ac91b4bef95dbf647dfbab482e502 DIFF: https://github.com/llvm/llvm-project/commit/82ba3f44657ac91b4bef95dbf647dfbab482e502.diff LOG: Recommit "[lldb/test] Don't use preexec_fn for launching inferiors" This recommits b15b1421, which reverted in was reverted in f51c47d98 due to failures on apple systems. The problem was that the patch introduced a race where the debug server could start the attach process before the first process (which isn't supposed to be attached to) was set up. This caused us to attach to the wrong process. The new version introduces additional synchronization to ensure that does not happen. Original commit message was: 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 e14c4f8c0d383..719131c9248e8 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 22851730153e0..d46e54f30bd55 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 0000000000000..22f1051530f87 --- /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 75% rename from lldb/test/API/tools/lldb-server/TestGdbRemoteAttachWait.py rename to lldb/test/API/tools/lldb-server/attach-wait/TestGdbRemoteAttachWait.py index ed31b5645c336..b1ec443788bfc 100644 --- a/lldb/test/API/tools/lldb-server/TestGdbRemoteAttachWait.py +++ b/lldb/test/API/tools/lldb-server/attach-wait/TestGdbRemoteAttachWait.py @@ -14,10 +14,13 @@ 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 + sync_file_path = lldbutil.append_to_process_working_directory(self, "process_ready") + args = [sync_file_path] 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 +29,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)] + args + exe = "shim" + self.build(dictionary={'EXE': exe, 'CXX_SOURCES': 'shim.cpp'}) + self.set_inferior_startup_attach_manually() server = self.connect_to_debug_monitor() @@ -34,11 +44,14 @@ def launch_inferior(): # Launch the first inferior (we shouldn't attach to this one). launch_inferior() - + + lldbutil.wait_for_file_on_target(self, sync_file_path) + 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 0000000000000..a64622a6c3b95 --- /dev/null +++ b/lldb/test/API/tools/lldb-server/attach-wait/main.cpp @@ -0,0 +1,12 @@ +#include <thread> +#include <fstream> + +int main(int argc, char *argv[]) +{ + if (argc >= 2) { + std::ofstream(argv[1]).close(); + } + // 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 0000000000000..60d0ba3753026 --- /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(); + execvp(argv[1], argv+1); + perror("execlp"); + return 1; +} _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
