[lldb-vscode] redirect stderr/stdout to the IDE's console

[wallace]

Summary

In certain occasions times, like when LLDB is initializing and evaluating the .lldbinit files, it tries to print to stderr and stdout directly. This confuses the IDE with malformed data, as it talks to lldb-vscode using stdin and stdout following the JSON RPC protocol. This ends up terminating the debug session with the user unaware of what's going on. There might be other situations in which this can happen, and they will be harder to debug than the .lldbinit case.

After several discussions with @clayborg, @yinghuitan and @aadsm, we realized that the best course of action is to simply redirect stdout and stderr to the console, without modifying LLDB itself. This will prove to be resilient to future bugs or features.

I made the simplest possible redirection logic I could come up with. It only works for POSIX, and to make it work with Windows should be merely changing pipe and dup2 for the windows equivalents like _pipe and _dup2. Sadly I don't have a Windows machine, so I'll do it later once my office reopens, or maybe someone else can do it.

I'm intentionally not adding a stop-redirecting logic, as I don't see it useful for the lldb-vscode case (why would we want to do that, really?).

I added a test.

Note: this is a simpler version of D80659. I first tried to implement a RIIA version of it, but it was problematic to manage the state of the thread and reverting the redirection came with some non trivial complexities, like what to do with unflushed data after the debug session has finished on the IDE's side.

[clayborg]

Just need to fix the default python arguments that were: "foo={}". This doesn't do what you think it will do. See https://docs.quantifiedcode.com/python-anti-patterns/correctness/mutable_default_value_as_argument.html

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

12

You must use None instead of {} otherwise all callers to this function will share the default dictionary

21

You must use None instead of {} otherwise all callers to this function will share the default dictionary

346–347

You must use None instead of {} otherwise all callers to this function will share the default dictionary

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

931

You must use None instead of {} otherwise all callers to this function will share the default dictionary

[wallace]

Address comments

[clayborg]

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

936

Python dictionaries have an update method that does what you want here: it will replace value for any keys that exist in env, and create one that don't exist.

lldb/tools/lldb-vscode/lldb-vscode.cpp

3094–3101

Do we need this code in lldb-vscode? Can't we just run some initCommands that print something from python?

3125–3126

I'd rather not build code into lldb-vscode for testing if we don't have to. You can easily just run "target list" from "initCommands" and it should print out "No targets."

[clayborg]

lldb/tools/lldb-vscode/lldb-vscode.cpp

3126

Come to think of it, this is the easiest way to do this. We can't do anything with and "initCommands" because that is too late as the in/out/err has been correctly set for the debugger by that time. We also don't want to have the test move the user's ~/.lldbinit or ~/.lldbinit-lldb-vscode file out of the way and replace it as that is just gnarly. So this is probably the best solution I can think of.

[wallace]

updated minor python detail

[wallace]

rebase

[JDevlieghere]

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

939

Should this be LLDB_VSCODE_LOG, similar to LLDB_VSCODE_TEST_STDOUT_STDERR_REDIRECTION? Most of our environment variables begin with LLDB_ so I think that would be most consistent.

[wallace]

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

939

I agree with you, but it might be hard to change it, as that might break some users' flow. The env var is also used inside of lldb-vscode.cpp itself

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

 
 from lldbsuite.test.lldbtest import *
 import os
 import vscode
 import time
 
 
 class VSCodeTestCaseBase(TestBase):
 
     NO_DEBUG_INFO_TESTCASE = True
 
-    def create_debug_adaptor(self):
+    def create_debug_adaptor(self, lldbVSCodeEnv=None):
         '''Create the Visual Studio Code debug adaptor'''
         self.assertTrue(os.path.exists(self.lldbVSCodeExec),
                         'lldb-vscode must exist')
         log_file_path = self.getBuildArtifact('vscode.txt')
         self.vscode = vscode.DebugAdaptor(
             executable=self.lldbVSCodeExec, init_commands=self.setUpCommands(),
-            log_file=log_file_path)
+            log_file=log_file_path, env=lldbVSCodeEnv)
 
-    def build_and_create_debug_adaptor(self):
+    def build_and_create_debug_adaptor(self, lldbVSCodeEnv=None):
         self.build()
-        self.create_debug_adaptor()
+        self.create_debug_adaptor(lldbVSCodeEnv)
 
     def set_source_breakpoints(self, source_path, lines, condition=None,
                                hitCondition=None):
         '''Sets source breakpoints and returns an array of strings containing
            the breakpoint IDs ("1", "2") for each breakpoint that was set.
         '''
         response = self.vscode.request_setBreakpoints(
             source_path, lines, condition=condition, hitCondition=hitCondition)
class VSCodeTestCaseBase(TestBase):
 
     def build_and_launch(self, program, args=None, cwd=None, env=None,
                          stopOnEntry=False, disableASLR=True,
                          disableSTDIO=False, shellExpandArguments=False,
                          trace=False, initCommands=None, preRunCommands=None,
                          stopCommands=None, exitCommands=None,
                          terminateCommands=None, sourcePath=None,
                          debuggerRoot=None, runInTerminal=False,
-                         disconnectAutomatically=True, postRunCommands=None):
+                         disconnectAutomatically=True, postRunCommands=None,
+                         lldbVSCodeEnv=None):
         '''Build the default Makefile target, create the VSCode debug adaptor,
            and launch the process.
         '''
-        self.build_and_create_debug_adaptor()
+        self.build_and_create_debug_adaptor(lldbVSCodeEnv)
         self.assertTrue(os.path.exists(program), 'executable must exist')
 
         return self.launch(program, args, cwd, env, stopOnEntry, disableASLR,
                     disableSTDIO, shellExpandArguments, trace,
                     initCommands, preRunCommands, stopCommands, exitCommands,
                     terminateCommands, sourcePath, debuggerRoot, runInTerminal=runInTerminal,
                     disconnectAutomatically=disconnectAutomatically,
                     postRunCommands=postRunCommands)

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

if line.startswith(prefix):
         json_str = f.read(length)
         if verbose:
             print('json: "%s"' % (json_str))
         if trace_file:
             trace_file.write('from adaptor:\n%s\n' % (json_str))
         # Decode the JSON bytes into a python dictionary
         return json.loads(json_str)
 
-    return None
+    raise Exception("unexpected malformed message from lldb-vscode: " + line)
 
 
 def packet_type_is(packet, packet_type):
     return 'type' in packet and packet['type'] == packet_type
 
 
 def read_packet_thread(vs_comm):
     done = False
def request_testGetTargetBreakpoints(self):
         return self.send_recv(command_dict)
 
     def terminate(self):
         self.send.close()
         # self.recv.close()
 
 
 class DebugAdaptor(DebugCommunication):
-    def __init__(self, executable=None, port=None, init_commands=[], log_file=None):
+    def __init__(self, executable=None, port=None, init_commands=[], log_file=None, env=None):
         self.process = None
         if executable is not None:
             adaptor_env = os.environ.copy()
+            if env is not None:
+                adaptor_env.update(env)
+
             if log_file:
                 adaptor_env['LLDBVSCODE_LOG'] = log_file
             self.process = subprocess.Popen([executable],
                                             stdin=subprocess.PIPE,
                                             stdout=subprocess.PIPE,
                                             stderr=subprocess.PIPE,
                                             env=adaptor_env)
             DebugCommunication.__init__(self, self.process.stdout,
                                         self.process.stdin, init_commands)
         elif port is not None:

lldb/test/API/tools/lldb-vscode/console/TestVSCode_redirection_to_console.py

+import unittest2
+import vscode
+import json
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+import lldbvscode_testcase
+
+
+class TestVSCode_redirection_to_console(lldbvscode_testcase.VSCodeTestCaseBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+
+    @skipIfWindows
+    @skipIfRemote
+    def test(self):
+        """
+            Without proper stderr and stdout redirection, the following code would throw an
+            exception, like the following:
+
+                Exception: unexpected malformed message from lldb-vscode
+        """
+        program = self.getBuildArtifact("a.out")
+        self.build_and_launch(
+            program,
+            lldbVSCodeEnv={"LLDB_VSCODE_TEST_STDOUT_STDERR_REDIRECTION": ""})
+
+        source = 'main.cpp'
+
+        breakpoint1_line = line_number(source, '// breakpoint 1')
+        breakpoint_ids = self.set_source_breakpoints(source, [breakpoint1_line])
+
+        self.assertEqual(len(breakpoint_ids), 1,
+                        "expect correct number of breakpoints")
+        self.continue_to_breakpoints(breakpoint_ids)
+
+        self.assertIn('argc', json.dumps(self.vscode.get_local_variables(frameIndex=1)))

lldb/tools/lldb-vscode/CMakeLists.txt

add_lldb_tool(lldb-vscode
   lldb-vscode.cpp
   BreakpointBase.cpp
   ExceptionBreakpoint.cpp
   FifoFiles.cpp
   FunctionBreakpoint.cpp
   IOStream.cpp
   JSONUtils.cpp
   LLDBUtils.cpp
+  OutputRedirector.cpp
   ProgressEvent.cpp
   RunInTerminal.cpp
   SourceBreakpoint.cpp
   VSCode.cpp
 
   LINK_LIBS
     liblldb
     ${host_lib}

lldb/tools/lldb-vscode/IOStream.cpp

 //===-- IOStream.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
 //
 //===----------------------------------------------------------------------===//
 
 #include "IOStream.h"
 
-#if defined(_WIN32) 
+#if defined(_WIN32)
 #include <io.h>
 #else
 #include <netinet/in.h>
 #include <sys/socket.h>
 #include <unistd.h>
 #endif
 
 #include <fstream>

lldb/tools/lldb-vscode/OutputRedirector.h

+//===-- OutputRedirector.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_OUTPUT_REDIRECTOR_H
+#define LLDB_TOOLS_LLDB_VSCODE_OUTPUT_REDIRECTOR_H
+
+#include <thread>
+
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Error.h"
+
+namespace lldb_vscode {
+
+/// Redirects the output of a given file descriptor to a callback.
+///
+/// \return
+///     \a Error::success if the redirection was set up correctly, or an error
+///     otherwise.
+llvm::Error RedirectFd(int fd, std::function<void(llvm::StringRef)> callback);
+
+} // namespace lldb_vscode
+
+#endif // LLDB_TOOLS_LLDB_VSCODE_OUTPUT_REDIRECTOR_H

lldb/tools/lldb-vscode/OutputRedirector.cpp

+//===-- OutputRedirector.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 <unistd.h>
+#endif
+
+#include "OutputRedirector.h"
+
+using namespace llvm;
+
+namespace lldb_vscode {
+
+Error RedirectFd(int fd, std::function<void(llvm::StringRef)> callback) {
+#if !defined(_WIN32)
+  int new_fd[2];
+  if (pipe(new_fd) == -1) {
+    int error = errno;
+    return createStringError(inconvertibleErrorCode(),
+                             "Couldn't create new pipe for fd %d. %s", fd,
+                             strerror(error));
+  }
+
+  if (dup2(new_fd[1], fd) == -1) {
+    int error = errno;
+    return createStringError(inconvertibleErrorCode(),
+                             "Couldn't override the fd %d. %s", fd,
+                             strerror(error));
+  }
+
+  int read_fd = new_fd[0];
+  std::thread t([read_fd, callback]() {
+    char buffer[4096];
+    while (true) {
+      ssize_t bytes_count = read(read_fd, &buffer, sizeof(buffer));
+      if (bytes_count == 0)
+        return;
+      if (bytes_count == -1) {
+        if (errno == EAGAIN || errno == EINTR)
+          continue;
+        break;
+      }
+      callback(StringRef(buffer, bytes_count).str());
+    }
+  });
+  t.detach();
+#endif
+  return Error::success();
+}
+
+} // namespace lldb_vscode

lldb/tools/lldb-vscode/lldb-vscode.cpp

 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/InitLLVM.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/PrettyStackTrace.h"
 #include "llvm/Support/raw_ostream.h"
 
 #include "JSONUtils.h"
 #include "LLDBUtils.h"
+#include "OutputRedirector.h"
 
 #if defined(_WIN32)
 #ifndef PATH_MAX
 #define PATH_MAX MAX_PATH
 #endif
 typedef int socklen_t;
 constexpr const char *dev_null_path = "nul";
 
#else
 
   std::string error = std::strerror(errno);
   comm_channel.NotifyError(error);
   llvm::errs() << error << "\n";
   exit(EXIT_FAILURE);
 #endif
 }
 
+/// used only by TestVSCode_redirection_to_console.py
+void redirection_test() {
+  printf("stdout message\n");
+  fprintf(stderr, "stderr message\n");
+  fflush(stdout);
+  fflush(stderr);
+}
+
+/// Redirect stdout and stderr fo the IDE's console output.
+///
+/// Errors in this operation will be printed to the log file and the IDE's
+/// console output as well.
+///
+/// \return
+///     A fd pointing to the original stdout.
+int SetupStdoutStderrRedirection() {
+  int new_stdout_fd = dup(fileno(stdout));
+  auto stdout_err_redirector_callback = [&](llvm::StringRef data) {
+    g_vsc.SendOutput(OutputType::Console, data);
+  };
+
+  for (int fd : {fileno(stdout), fileno(stderr)}) {
+    if (llvm::Error err = RedirectFd(fd, stdout_err_redirector_callback)) {
+      std::string error_message = llvm::toString(std::move(err));
+      if (g_vsc.log)
+        *g_vsc.log << error_message << std::endl;
+      stdout_err_redirector_callback(error_message);
+    }
+  }
+
+  /// used only by TestVSCode_redirection_to_console.py
+  if (getenv("LLDB_VSCODE_TEST_STDOUT_STDERR_REDIRECTION") != nullptr)
+    redirection_test();
+  return new_stdout_fd;
+}
+
 int main(int argc, char *argv[]) {
   llvm::InitLLVM IL(argc, argv, /*InstallPipeSignalExitHandler=*/false);
   llvm::PrettyStackTraceProgram X(argc, argv);
 
+  // stdout/stderr redirection to the IDE's console
+  int new_stdout_fd = SetupStdoutStderrRedirection();
+
   llvm::SmallString<256> program_path(argv[0]);
   llvm::sys::fs::make_absolute(program_path);
   g_vsc.debug_adaptor_path = program_path.str().str();
 
   LLDBVSCodeOptTable T;
   unsigned MAI, MAC;
   llvm::ArrayRef<const char *> ArgsArr = llvm::makeArrayRef(argv + 1, argc);
   llvm::opt::InputArgList input_args = T.ParseArgs(ArgsArr, MAI, MAC);
if (portno != -1) {
     if (socket_fd >= 0) {
       g_vsc.input.descriptor = StreamDescriptor::from_socket(socket_fd, true);
       g_vsc.output.descriptor = StreamDescriptor::from_socket(socket_fd, false);
     } else {
       return EXIT_FAILURE;
     }
   } else {
     g_vsc.input.descriptor = StreamDescriptor::from_file(fileno(stdin), false);
-    g_vsc.output.descriptor =
-        StreamDescriptor::from_file(fileno(stdout), false);
+    g_vsc.output.descriptor = StreamDescriptor::from_file(new_stdout_fd, false);
   }
+
   uint32_t packet_idx = 0;
   while (!g_vsc.sent_terminated_event) {
     llvm::json::Object object;
     lldb_vscode::PacketStatus status = g_vsc.GetNextObject(object);
     if (status == lldb_vscode::PacketStatus::EndOfFile)
       break;
     if (status != lldb_vscode::PacketStatus::Success)
       return 1; // Fatal error
 
     if (!g_vsc.HandleObject(object))
       return 1;
     ++packet_idx;
   }
 
   return EXIT_SUCCESS;
 }