From 64f47c1e58a10de160ce3fb3afbc50c0243e2977 Mon Sep 17 00:00:00 2001 From: Walter Erquinigo Date: Wed, 21 Apr 2021 14:20:17 -0700 Subject: [PATCH] [lldb-vscode] redirect stderr/stdout to the IDE's console 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. --- .../test/tools/lldb-vscode/lldbvscode_testcase.py | 13 ++--- .../lldbsuite/test/tools/lldb-vscode/vscode.py | 7 ++- .../console/TestVSCode_redirection_to_console.py | 37 ++++++++++++++ lldb/tools/lldb-vscode/CMakeLists.txt | 1 + lldb/tools/lldb-vscode/IOStream.cpp | 2 +- lldb/tools/lldb-vscode/OutputRedirector.cpp | 56 ++++++++++++++++++++++ lldb/tools/lldb-vscode/OutputRedirector.h | 28 +++++++++++ lldb/tools/lldb-vscode/lldb-vscode.cpp | 44 ++++++++++++++++- 8 files changed, 177 insertions(+), 11 deletions(-) create mode 100644 lldb/test/API/tools/lldb-vscode/console/TestVSCode_redirection_to_console.py create mode 100644 lldb/tools/lldb-vscode/OutputRedirector.cpp create mode 100644 lldb/tools/lldb-vscode/OutputRedirector.h diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py index d073e36..5f5e4b4 100644 --- a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py +++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py @@ -9,18 +9,18 @@ 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): @@ -343,11 +343,12 @@ class VSCodeTestCaseBase(TestBase): 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, diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py index 02e9b65..9eeebdb 100644 --- a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py +++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py @@ -81,7 +81,7 @@ def read_packet(f, verbose=False, trace_file=None): # 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): @@ -928,10 +928,13 @@ class DebugCommunication(object): 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], diff --git a/lldb/test/API/tools/lldb-vscode/console/TestVSCode_redirection_to_console.py b/lldb/test/API/tools/lldb-vscode/console/TestVSCode_redirection_to_console.py new file mode 100644 index 0000000..1e028e5 --- /dev/null +++ b/lldb/test/API/tools/lldb-vscode/console/TestVSCode_redirection_to_console.py @@ -0,0 +1,37 @@ +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))) diff --git a/lldb/tools/lldb-vscode/CMakeLists.txt b/lldb/tools/lldb-vscode/CMakeLists.txt index eb2f651..41c1f10 100644 --- a/lldb/tools/lldb-vscode/CMakeLists.txt +++ b/lldb/tools/lldb-vscode/CMakeLists.txt @@ -32,6 +32,7 @@ add_lldb_tool(lldb-vscode IOStream.cpp JSONUtils.cpp LLDBUtils.cpp + OutputRedirector.cpp ProgressEvent.cpp RunInTerminal.cpp SourceBreakpoint.cpp diff --git a/lldb/tools/lldb-vscode/IOStream.cpp b/lldb/tools/lldb-vscode/IOStream.cpp index cd22d90..4b11b90 100644 --- a/lldb/tools/lldb-vscode/IOStream.cpp +++ b/lldb/tools/lldb-vscode/IOStream.cpp @@ -8,7 +8,7 @@ #include "IOStream.h" -#if defined(_WIN32) +#if defined(_WIN32) #include #else #include diff --git a/lldb/tools/lldb-vscode/OutputRedirector.cpp b/lldb/tools/lldb-vscode/OutputRedirector.cpp new file mode 100644 index 0000000..7432a82 --- /dev/null +++ b/lldb/tools/lldb-vscode/OutputRedirector.cpp @@ -0,0 +1,56 @@ +//===-- 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 +#endif + +#include "OutputRedirector.h" + +using namespace llvm; + +namespace lldb_vscode { + +Error RedirectFd(int fd, std::function 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 diff --git a/lldb/tools/lldb-vscode/OutputRedirector.h b/lldb/tools/lldb-vscode/OutputRedirector.h new file mode 100644 index 0000000..c728367 --- /dev/null +++ b/lldb/tools/lldb-vscode/OutputRedirector.h @@ -0,0 +1,28 @@ +//===-- 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 + +#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 callback); + +} // namespace lldb_vscode + +#endif // LLDB_TOOLS_LLDB_VSCODE_OUTPUT_REDIRECTOR_H diff --git a/lldb/tools/lldb-vscode/lldb-vscode.cpp b/lldb/tools/lldb-vscode/lldb-vscode.cpp index fd502db..b282274 100644 --- a/lldb/tools/lldb-vscode/lldb-vscode.cpp +++ b/lldb/tools/lldb-vscode/lldb-vscode.cpp @@ -57,6 +57,7 @@ #include "JSONUtils.h" #include "LLDBUtils.h" +#include "OutputRedirector.h" #if defined(_WIN32) #ifndef PATH_MAX @@ -3090,10 +3091,49 @@ void LaunchRunInTerminalTarget(llvm::opt::Arg &target_arg, #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(); @@ -3163,9 +3203,9 @@ int main(int argc, char *argv[]) { } } 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; -- 2.7.4