[vscode] Improve runInTerminal and support linux

Depends on D93874.

runInTerminal was using --wait-for, but it was some problems because it uses process polling looking for a single instance of the debuggee:

- it gets to know of the target late, which renders breakpoints in the main function almost impossible
- polling might fail if there are already other processes with the same name
- polling might also fail on some linux machine, as it's implemented with the ps command, and the ps command's args and output are not standard everywhere

As a better way to implement this so that it works well on Darwin and Linux, I'm using now the following process:

- lldb-vscode notices the runInTerminal, so it spawns lldb-vscode with a special flag --launch-target <target>. This flags tells lldb-vscode to wait to be attached and then it execs the target program. I'm using lldb-vscode itself to do this, because it makes finding the launcher program easier. Also no CMAKE INSTALL scripts are needed.
- Besides this, the debugger creates a temporary FIFO file where the launcher program will write its pid to. That way the debugger will be sure of which program to attach.
- Once attach happend, the debugger creates a second temporary file to notify the launcher program that it has been attached, so that it can then exec. I'm using this instead of using a signal or a similar mechanism because I don't want the launcher program to wait indefinitely to be attached in case the debugger crashed. That would pollute the process list with a lot of hanging processes. Instead, I'm setting a 20 seconds timeout (that's an overkill) and the launcher program seeks in intervals the second tepmorary file.

Some notes:
- I preferred not to use sockets because it requires a lot of code and I only need a pid. It would also require a lot of code when windows support is implemented.
- I didn't add Windows support, as I don't have a windows machine, but adding support for it should be easy, as the FIFO file can be implemented with a named pipe, which is standard on Windows and works pretty much the same way.

The existing test which didn't pass on Linux, now passes.

Differential Revision: https://reviews.llvm.org/D93951

Author: walter-erquinigo

lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py

@@ -282,7 +282,8 @@ def launch(self, program=None, args=None, cwd=None, env=None,
                trace=False, initCommands=None, preRunCommands=None,
                stopCommands=None, exitCommands=None, terminateCommands=None,
                sourcePath=None, debuggerRoot=None, launchCommands=None,
-               sourceMap=None, disconnectAutomatically=True, runInTerminal=False):
+               sourceMap=None, disconnectAutomatically=True, runInTerminal=False,
+               expectFailure=False):
         '''Sending launch request to vscode
         '''
 
@@ -317,14 +318,20 @@ def cleanup():
             debuggerRoot=debuggerRoot,
             launchCommands=launchCommands,
             sourceMap=sourceMap,
-            runInTerminal=runInTerminal)
+            runInTerminal=runInTerminal,
+            expectFailure=expectFailure)
+
+        if expectFailure:
+            return response
+
         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
 
 
     def build_and_launch(self, program, args=None, cwd=None, env=None,
@@ -340,7 +347,7 @@ def build_and_launch(self, program, args=None, cwd=None, env=None,
         self.build_and_create_debug_adaptor()
         self.assertTrue(os.path.exists(program), 'executable must exist')
 
-        self.launch(program, args, cwd, env, stopOnEntry, disableASLR,
+        return self.launch(program, args, cwd, env, stopOnEntry, disableASLR,
                     disableSTDIO, shellExpandArguments, trace,
                     initCommands, preRunCommands, stopCommands, exitCommands,
                     terminateCommands, sourcePath, debuggerRoot, runInTerminal=runInTerminal)

lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py

@@ -612,7 +612,7 @@ def request_launch(self, program, args=None, cwd=None, env=None,
                        stopCommands=None, exitCommands=None,
                        terminateCommands=None ,sourcePath=None,
                        debuggerRoot=None, launchCommands=None, sourceMap=None,
-                       runInTerminal=False):
+                       runInTerminal=False, expectFailure=False):
         args_dict = {
             'program': program
         }
@@ -660,9 +660,10 @@ def request_launch(self, program, args=None, cwd=None, env=None,
         }
         response = self.send_recv(command_dict)
 
-        # Wait for a 'process' and 'initialized' event in any order
-        self.wait_for_event(filter=['process', 'initialized'])
-        self.wait_for_event(filter=['process', 'initialized'])
+        if not expectFailure:
+            # Wait for a 'process' and 'initialized' event in any order
+            self.wait_for_event(filter=['process', 'initialized'])
+            self.wait_for_event(filter=['process', 'initialized'])
         return response
 
     def request_next(self, threadId):

lldb/test/API/tools/lldb-vscode/runInTerminal/TestVSCode_runInTerminal.py

@@ -11,22 +11,47 @@
 import lldbvscode_testcase
 import time
 import os
+import subprocess
+import shutil
+import json
+from threading import Thread
 
 
 class TestVSCode_runInTerminal(lldbvscode_testcase.VSCodeTestCaseBase):
 
     mydir = TestBase.compute_mydir(__file__)
 
-    @skipUnlessDarwin
+    def readPidMessage(self, fifo_file):
+        with open(fifo_file, "r") as file:
+            self.assertIn("pid", file.readline())
+
+    def sendDidAttachMessage(self, fifo_file):
+        with open(fifo_file, "w") as file:
+            file.write(json.dumps({"kind": "didAttach"}) + "\n")
+
+    def readErrorMessage(self, fifo_file):
+        with open(fifo_file, "r") as file:
+            return file.readline()
+
+    @skipIfWindows
     @skipIfRemote
     def test_runInTerminal(self):
         '''
             Tests the "runInTerminal" reverse request. It makes sure that the IDE can
             launch the inferior with the correct environment variables and arguments.
         '''
+        if "debug" in str(os.environ["LLDBVSCODE_EXEC"]).lower():
+            # We skip this test for debug builds because it takes too long parsing lldb's own
+            # debug info. Release builds are fine.
+            # Checking this environment variable seems to be a decent proxy for a quick
+            # detection
+            return
         program = self.getBuildArtifact("a.out")
         source = 'main.c'
-        self.build_and_launch(program, stopOnEntry=True, runInTerminal=True, args=["foobar"], env=["FOO=bar"])
+        self.build_and_launch(
+            program, stopOnEntry=True, runInTerminal=True, args=["foobar"],
+            env=["FOO=bar"])
+
         breakpoint_line = line_number(source, '// breakpoint')
 
         self.set_source_breakpoints(source, [breakpoint_line])
@@ -46,3 +71,87 @@ def test_runInTerminal(self):
         # We verify we were able to set the environment
         env = self.vscode.request_evaluate('foo')['body']['result']
         self.assertIn('bar', env)
+
+    @skipIfWindows
+    @skipIfRemote
+    def test_runInTerminalInvalidTarget(self):
+        self.build_and_create_debug_adaptor()
+        response = self.launch(
+            "INVALIDPROGRAM", stopOnEntry=True, 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'])
+
+    @skipIfWindows
+    @skipIfRemote
+    def test_missingArgInRunInTerminalLauncher(self):
+        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)
+
+    @skipIfWindows
+    @skipIfRemote
+    def test_FakeAttachedRunInTerminalLauncherWithInvalidProgram(self):
+        comm_file = os.path.join(self.getBuildDir(), "comm-file")
+        os.mkfifo(comm_file)
+
+        proc = subprocess.Popen(
+            [self.lldbVSCodeExec, "--comm-file", comm_file, "--launch-target", "INVALIDPROGRAM"],
+            universal_newlines=True, stderr=subprocess.PIPE)
+
+        self.readPidMessage(comm_file)
+        self.sendDidAttachMessage(comm_file)
+        self.assertIn("No such file or directory", self.readErrorMessage(comm_file))
+        
+        _, stderr = proc.communicate()
+        self.assertIn("No such file or directory", stderr)
+
+    @skipIfWindows
+    @skipIfRemote
+    def test_FakeAttachedRunInTerminalLauncherWithValidProgram(self):
+        comm_file = os.path.join(self.getBuildDir(), "comm-file")
+        os.mkfifo(comm_file)
+
+        proc = subprocess.Popen(
+            [self.lldbVSCodeExec, "--comm-file", comm_file, "--launch-target", "echo", "foo"],
+            universal_newlines=True, stdout=subprocess.PIPE)
+
+        self.readPidMessage(comm_file)
+        self.sendDidAttachMessage(comm_file)
+
+        stdout, _ = proc.communicate()
+        self.assertIn("foo", stdout)
+
+    @skipIfWindows
+    @skipIfRemote
+    def test_FakeAttachedRunInTerminalLauncherAndCheckEnvironment(self):
+        comm_file = os.path.join(self.getBuildDir(), "comm-file")
+        os.mkfifo(comm_file)
+
+        proc = subprocess.Popen(
+            [self.lldbVSCodeExec, "--comm-file", comm_file, "--launch-target", "env"],
+            universal_newlines=True, stdout=subprocess.PIPE,
+            env={**os.environ, "FOO": "BAR"})
+        
+        self.readPidMessage(comm_file)
+        self.sendDidAttachMessage(comm_file)
+
+        stdout, _ = proc.communicate()
+        self.assertIn("FOO=BAR", stdout)
+
+    @skipIfWindows
+    @skipIfRemote
+    def test_NonAttachedRunInTerminalLauncher(self):
+        comm_file = os.path.join(self.getBuildDir(), "comm-file")
+        os.mkfifo(comm_file)
+
+        proc = subprocess.Popen(
+            [self.lldbVSCodeExec, "--comm-file", comm_file, "--launch-target", "echo", "foo"],
+            universal_newlines=True, stderr=subprocess.PIPE,
+            env={**os.environ, "LLDB_VSCODE_RIT_TIMEOUT_IN_MS": "1000"})
+
+        self.readPidMessage(comm_file)
+        
+        _, stderr = proc.communicate()
+        self.assertIn("Timed out trying to get messages from the debug adaptor", stderr)

lldb/tools/lldb-vscode/CMakeLists.txt

@@ -27,10 +27,12 @@ add_lldb_tool(lldb-vscode
   lldb-vscode.cpp
   BreakpointBase.cpp
   ExceptionBreakpoint.cpp
+  FifoFiles.cpp
   FunctionBreakpoint.cpp
   IOStream.cpp
   JSONUtils.cpp
   LLDBUtils.cpp
+  RunInTerminal.cpp
   SourceBreakpoint.cpp
   VSCode.cpp
 

lldb/tools/lldb-vscode/FifoFiles.cpp

@@ -0,0 +1,91 @@
+//===-- FifoFiles.cpp -------------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#if !defined(WIN32)
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <unistd.h>
+#endif
+
+#include <chrono>
+#include <fstream>
+#include <future>
+#include <thread>
+
+#include "llvm/Support/FileSystem.h"
+
+#include "lldb/lldb-defines.h"
+
+#include "FifoFiles.h"
+
+using namespace llvm;
+
+namespace lldb_vscode {
+
+FifoFile::FifoFile(StringRef path) : m_path(path) {}
+
+FifoFile::~FifoFile() {
+#if !defined(WIN32)
+  unlink(m_path.c_str());
+#endif
+};
+
+Expected<std::shared_ptr<FifoFile>> CreateFifoFile(StringRef path) {
+#if defined(WIN32)
+  return createStringError(inconvertibleErrorCode(), "Unimplemented");
+#else
+  if (int err = mkfifo(path.data(), 0600))
+    return createStringError(std::error_code(err, std::generic_category()),
+                             "Couldn't create fifo file: %s", path.data());
+  return std::make_shared<FifoFile>(path);
+#endif
+}
+
+FifoFileIO::FifoFileIO(StringRef fifo_file, StringRef other_endpoint_name)
+    : m_fifo_file(fifo_file), m_other_endpoint_name(other_endpoint_name) {}
+
+Expected<json::Value> FifoFileIO::ReadJSON(std::chrono::milliseconds timeout) {
+  // We use a pointer for this future, because otherwise its normal destructor
+  // would wait for the getline to end, rendering the timeout useless.
+  Optional<std::string> line;
+  std::future<void> *future =
+      new std::future<void>(std::async(std::launch::async, [&]() {
+        std::ifstream reader(m_fifo_file, std::ifstream::in);
+        std::string buffer;
+        std::getline(reader, buffer);
+        if (!buffer.empty())
+          line = buffer;
+      }));
+  if (future->wait_for(timeout) == std::future_status::timeout ||
+      !line.hasValue())
+    return createStringError(inconvertibleErrorCode(),
+                             "Timed out trying to get messages from the " +
+                                 m_other_endpoint_name);
+  delete future;
+  return json::parse(*line);
+}
+
+Error FifoFileIO::SendJSON(const json::Value &json,
+                           std::chrono::milliseconds timeout) {
+  bool done = false;
+  std::future<void> *future =
+      new std::future<void>(std::async(std::launch::async, [&]() {
+        std::ofstream writer(m_fifo_file, std::ofstream::out);
+        writer << JSONToString(json) << std::endl;
+        done = true;
+      }));
+  if (future->wait_for(timeout) == std::future_status::timeout || !done) {
+    return createStringError(inconvertibleErrorCode(),
+                             "Timed out trying to send messages to the " +
+                                 m_other_endpoint_name);
+  }
+  delete future;
+  return Error::success();
+}
+
+} // namespace lldb_vscode

lldb/tools/lldb-vscode/FifoFiles.h

@@ -0,0 +1,84 @@
+//===-- FifoFiles.h ---------------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_TOOLS_LLDB_VSCODE_FIFOFILES_H
+#define LLDB_TOOLS_LLDB_VSCODE_FIFOFILES_H
+
+#include "llvm/Support/Error.h"
+
+#include "JSONUtils.h"
+
+namespace lldb_vscode {
+
+/// Struct that controls the life of a fifo file in the filesystem.
+///
+/// The file is destroyed when the destructor is invoked.
+struct FifoFile {
+  FifoFile(llvm::StringRef path);
+
+  ~FifoFile();
+
+  std::string m_path;
+};
+
+/// Create a fifo file in the filesystem.
+///
+/// \param[in] path
+///     The path for the fifo file.
+///
+/// \return
+///     A \a std::shared_ptr<FifoFile> if the file could be created, or an
+///     \a llvm::Error in case of failures.
+llvm::Expected<std::shared_ptr<FifoFile>> CreateFifoFile(llvm::StringRef path);
+
+class FifoFileIO {
+public:
+  /// \param[in] fifo_file
+  ///     The path to an input fifo file that exists in the file system.
+  ///
+  /// \param[in] other_endpoint_name
+  ///     A human readable name for the other endpoint that will communicate
+  ///     using this file. This is used for error messages.
+  FifoFileIO(llvm::StringRef fifo_file, llvm::StringRef other_endpoint_name);
+
+  /// Read the next JSON object from the underlying input fifo file.
+  ///
+  /// The JSON object is expected to be a single line delimited with \a
+  /// std::endl.
+  ///
+  /// \return
+  ///     An \a llvm::Error object indicating the success or failure of this
+  ///     operation. Failures arise if the timeout is hit, the next line of text
+  ///     from the fifo file is not a valid JSON object, or is it impossible to
+  ///     read from the file.
+  llvm::Expected<llvm::json::Value> ReadJSON(std::chrono::milliseconds timeout);
+
+  /// Serialize a JSON object and write it to the underlying output fifo file.
+  ///
+  /// \param[in] json
+  ///     The JSON object to send. It will be printed as a single line delimited
+  ///     with \a std::endl.
+  ///
+  /// \param[in] timeout
+  ///     A timeout for how long we should until for the data to be consumed.
+  ///
+  /// \return
+  ///     An \a llvm::Error object indicating whether the data was consumed by
+  ///     a reader or not.
+  llvm::Error SendJSON(
+      const llvm::json::Value &json,
+      std::chrono::milliseconds timeout = std::chrono::milliseconds(20000));
+
+private:
+  std::string m_fifo_file;
+  std::string m_other_endpoint_name;
+};
+
+} // namespace lldb_vscode
+
+#endif // LLDB_TOOLS_LLDB_VSCODE_FIFOFILES_H