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

Reply via email to