jgorbe updated this revision to Diff 511818. jgorbe added a comment. Added a `--debugger-pid` flag that needs to be passed with `--launch-target` so that we can restrict `PR_SET_PTRACER` to the main lldb-vscode instance instead of using `PR_SET_PTRACER_ANY`.
CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147805/new/ https://reviews.llvm.org/D147805 Files: lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py lldb/test/API/tools/lldb-vscode/runInTerminal/TestVSCode_runInTerminal.py lldb/tools/lldb-vscode/JSONUtils.cpp lldb/tools/lldb-vscode/JSONUtils.h lldb/tools/lldb-vscode/Options.td lldb/tools/lldb-vscode/lldb-vscode.cpp
Index: lldb/tools/lldb-vscode/lldb-vscode.cpp =================================================================== --- lldb/tools/lldb-vscode/lldb-vscode.cpp +++ lldb/tools/lldb-vscode/lldb-vscode.cpp @@ -32,6 +32,10 @@ #include <unistd.h> #endif +#if defined(__linux__) +#include <sys/prctl.h> +#endif + #include <algorithm> #include <chrono> #include <fstream> @@ -1562,8 +1566,12 @@ RunInTerminalDebugAdapterCommChannel comm_channel(comm_file.m_path); + unsigned long debugger_pid = 0; +#if !defined(_WIN32) + debugger_pid = getpid(); +#endif llvm::json::Object reverse_request = CreateRunInTerminalReverseRequest( - launch_request, g_vsc.debug_adaptor_path, comm_file.m_path); + launch_request, g_vsc.debug_adaptor_path, comm_file.m_path, debugger_pid); llvm::json::Object reverse_response; lldb_vscode::PacketStatus status = g_vsc.SendReverseRequest(reverse_request, reverse_response); @@ -3141,11 +3149,20 @@ // In case of errors launching the target, a suitable error message will be // emitted to the debug adaptor. void LaunchRunInTerminalTarget(llvm::opt::Arg &target_arg, - llvm::StringRef comm_file, char *argv[]) { + llvm::StringRef comm_file, + unsigned long debugger_pid, char *argv[]) { #if defined(_WIN32) llvm::errs() << "runInTerminal is only supported on POSIX systems\n"; exit(EXIT_FAILURE); #else + + // On Linux with the Yama security module enabled, a process can only attach + // to its descendants by default. In the runInTerminal case the target + // process is launched by the client so we need to allow tracing explicitly. +#if defined(__linux__) + (void)prctl(PR_SET_PTRACER, debugger_pid, 0, 0, 0); +#endif + RunInTerminalLauncherCommChannel comm_channel(comm_file); if (llvm::Error err = comm_channel.NotifyPid()) { llvm::errs() << llvm::toString(std::move(err)) << "\n"; @@ -3237,18 +3254,27 @@ } if (llvm::opt::Arg *target_arg = input_args.getLastArg(OPT_launch_target)) { - if (llvm::opt::Arg *comm_file = input_args.getLastArg(OPT_comm_file)) { + llvm::opt::Arg *debugger_pid = input_args.getLastArg(OPT_debugger_pid); + llvm::opt::Arg *comm_file = input_args.getLastArg(OPT_comm_file); + if (comm_file && debugger_pid) { + char *remainder; + unsigned long pid = strtoul(debugger_pid->getValue(), &remainder, 0); + if (remainder == debugger_pid->getValue() || *remainder != '\0') { + llvm::errs() << "'" << debugger_pid->getValue() << "' is not a valid " + "PID\n"; + return EXIT_FAILURE; + } int target_args_pos = argc; for (int i = 0; i < argc; i++) if (strcmp(argv[i], "--launch-target") == 0) { target_args_pos = i + 1; break; } - LaunchRunInTerminalTarget(*target_arg, comm_file->getValue(), + LaunchRunInTerminalTarget(*target_arg, comm_file->getValue(), pid, argv + target_args_pos); } else { - llvm::errs() << "\"--launch-target\" requires \"--comm-file\" to be " - "specified\n"; + llvm::errs() << "\"--launch-target\" requires \"--comm-file\" and " + "\"--debugger-pid\" to be specified\n"; return EXIT_FAILURE; } } Index: lldb/tools/lldb-vscode/Options.td =================================================================== --- lldb/tools/lldb-vscode/Options.td +++ lldb/tools/lldb-vscode/Options.td @@ -28,9 +28,14 @@ MetaVarName<"<target>">, HelpText<"Launch a target for the launchInTerminal request. Any argument " "provided after this one will be passed to the target. The parameter " - "--comm-files-prefix must also be specified.">; + "--comm-file must also be specified.">; def comm_file: Separate<["--", "-"], "comm-file">, MetaVarName<"<file>">, - HelpText<"The fifo file used to communicate the with the debug adaptor" + HelpText<"The fifo file used to communicate the with the debug adaptor " "when using --launch-target.">; + +def debugger_pid: Separate<["--", "-"], "debugger-pid">, + MetaVarName<"<pid>">, + HelpText<"The PID of the lldb-vscode instance that sent the launchInTerminal " + "request when using --launch-target.">; Index: lldb/tools/lldb-vscode/JSONUtils.h =================================================================== --- lldb/tools/lldb-vscode/JSONUtils.h +++ lldb/tools/lldb-vscode/JSONUtils.h @@ -479,13 +479,19 @@ /// \param[in] comm_file /// The fifo file used to communicate the with the target launcher. /// +/// \param[in] debugger_pid +/// The PID of the lldb-vscode instance that will attach to the target. The +/// launcher uses it on Linux tell the kernel that it should allow the +/// debugger process to attach. +/// /// \return /// A "runInTerminal" JSON object that follows the specification outlined by /// Microsoft. llvm::json::Object CreateRunInTerminalReverseRequest(const llvm::json::Object &launch_request, llvm::StringRef debug_adaptor_path, - llvm::StringRef comm_file); + llvm::StringRef comm_file, + unsigned long debugger_pid); /// Create a "Terminated" JSON object that contains statistics /// Index: lldb/tools/lldb-vscode/JSONUtils.cpp =================================================================== --- lldb/tools/lldb-vscode/JSONUtils.cpp +++ lldb/tools/lldb-vscode/JSONUtils.cpp @@ -1102,7 +1102,8 @@ llvm::json::Object CreateRunInTerminalReverseRequest(const llvm::json::Object &launch_request, llvm::StringRef debug_adaptor_path, - llvm::StringRef comm_file) { + llvm::StringRef comm_file, + unsigned long debugger_pid) { llvm::json::Object reverse_request; reverse_request.try_emplace("type", "request"); reverse_request.try_emplace("command", "runInTerminal"); @@ -1116,6 +1117,7 @@ // The program path must be the first entry in the "args" field std::vector<std::string> args = { debug_adaptor_path.str(), "--comm-file", comm_file.str(), + "--debugger-pid", std::to_string(debugger_pid), "--launch-target", GetString(launch_request_arguments, "program").str()}; std::vector<std::string> target_args = GetStrings(launch_request_arguments, "args"); Index: lldb/test/API/tools/lldb-vscode/runInTerminal/TestVSCode_runInTerminal.py =================================================================== --- lldb/test/API/tools/lldb-vscode/runInTerminal/TestVSCode_runInTerminal.py +++ lldb/test/API/tools/lldb-vscode/runInTerminal/TestVSCode_runInTerminal.py @@ -57,8 +57,7 @@ program = self.getBuildArtifact("a.out") source = 'main.c' self.build_and_launch( - program, stopOnEntry=True, runInTerminal=True, args=["foobar"], - env=["FOO=bar"]) + program, runInTerminal=True, args=["foobar"], env=["FOO=bar"]) breakpoint_line = line_number(source, '// breakpoint') @@ -88,7 +87,7 @@ return self.build_and_create_debug_adaptor() response = self.launch( - "INVALIDPROGRAM", stopOnEntry=True, runInTerminal=True, args=["foobar"], env=["FOO=bar"], expectFailure=True) + "INVALIDPROGRAM", runInTerminal=True, args=["foobar"], env=["FOO=bar"], expectFailure=True) self.assertFalse(response['success']) self.assertIn("Could not create a target for a program 'INVALIDPROGRAM': unable to find executable", response['message']) @@ -102,7 +101,7 @@ proc = subprocess.run([self.lldbVSCodeExec, "--launch-target", "INVALIDPROGRAM"], capture_output=True, universal_newlines=True) self.assertTrue(proc.returncode != 0) - self.assertIn('"--launch-target" requires "--comm-file" to be specified', proc.stderr) + self.assertIn('"--launch-target" requires "--comm-file" and "--debugger-pid" to be specified', proc.stderr) @skipIfWindows @skipIfRemote @@ -114,7 +113,7 @@ os.mkfifo(comm_file) proc = subprocess.Popen( - [self.lldbVSCodeExec, "--comm-file", comm_file, "--launch-target", "INVALIDPROGRAM"], + [self.lldbVSCodeExec, "--comm-file", comm_file, "--debugger-pid", "1234", "--launch-target", "INVALIDPROGRAM"], universal_newlines=True, stderr=subprocess.PIPE) self.readPidMessage(comm_file) @@ -134,7 +133,7 @@ os.mkfifo(comm_file) proc = subprocess.Popen( - [self.lldbVSCodeExec, "--comm-file", comm_file, "--launch-target", "echo", "foo"], + [self.lldbVSCodeExec, "--comm-file", comm_file, "--debugger-pid", "1234", "--launch-target", "echo", "foo"], universal_newlines=True, stdout=subprocess.PIPE) self.readPidMessage(comm_file) @@ -153,7 +152,7 @@ os.mkfifo(comm_file) proc = subprocess.Popen( - [self.lldbVSCodeExec, "--comm-file", comm_file, "--launch-target", "env"], + [self.lldbVSCodeExec, "--comm-file", comm_file, "--debugger-pid", "1234", "--launch-target", "env"], universal_newlines=True, stdout=subprocess.PIPE, env={**os.environ, "FOO": "BAR"}) @@ -173,7 +172,7 @@ os.mkfifo(comm_file) proc = subprocess.Popen( - [self.lldbVSCodeExec, "--comm-file", comm_file, "--launch-target", "echo", "foo"], + [self.lldbVSCodeExec, "--comm-file", comm_file, "--debugger-pid", "1234", "--launch-target", "echo", "foo"], universal_newlines=True, stderr=subprocess.PIPE, env={**os.environ, "LLDB_VSCODE_RIT_TIMEOUT_IN_MS": "1000"}) Index: lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py =================================================================== --- lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py +++ lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py @@ -331,10 +331,6 @@ if not (response and response['success']): self.assertTrue(response['success'], 'launch failed (%s)' % (response['message'])) - # We need to trigger a request_configurationDone after we've successfully - # attached a runInTerminal process to finish initialization. - if runInTerminal: - self.vscode.request_configurationDone() return response
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits