[lldb-dap] Add stdio redirection for integrated and external terminals #161089
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-lldb Author: Sergei Druzhkov (DrSergei) ChangesThis patch extends stdio redirection support to integrated and external terminals. Currently, these cases are not covered by the standard logic because Also, fixed a small typo in a comparison that was leading to out-of-bound access, and added Full diff: https://github.com/llvm/llvm-project/pull/161089.diff 8 Files Affected:
diff --git a/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py b/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py
index ceef95dfcd0d5..8db2316e73fc8 100644
--- a/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py
+++ b/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py
@@ -632,7 +632,27 @@ def test_stdio_redirection(self):
program = self.getBuildArtifact("a.out")
with tempfile.NamedTemporaryFile("rt") as f:
- self.launch(program, stdio=[None, f.name, None])
+ self.launch(program, stdio=[None, f.name])
+ self.continue_to_exit()
+ lines = f.readlines()
+ self.assertIn(
+ program, lines[0], "make sure program path is in first argument"
+ )
+
+ @skipIfAsan
+ @skipIfWindows
+ @skipIf(oslist=["linux"], archs=no_match(["x86_64"]))
+ def test_stdio_redirection_and_console(self):
+ """
+ Test stdio redirection and console.
+ """
+ self.build_and_create_debug_adapter()
+ program = self.getBuildArtifact("a.out")
+
+ with tempfile.NamedTemporaryFile("rt") as f:
+ self.launch(
+ program, console="integratedTerminal", stdio=[None, f.name, None]
+ )
self.continue_to_exit()
lines = f.readlines()
self.assertIn(
diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.cpp b/lldb/tools/lldb-dap/Handler/RequestHandler.cpp
index 773891353db6a..e7d9b89653f1c 100644
--- a/lldb/tools/lldb-dap/Handler/RequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/RequestHandler.cpp
@@ -57,7 +57,7 @@ SetupIORedirection(const std::vector<std::optional<std::string>> &stdio,
size_t n = std::max(stdio.size(), static_cast<size_t>(3));
for (size_t i = 0; i < n; i++) {
std::optional<std::string> path;
- if (stdio.size() < i)
+ if (stdio.size() <= i)
path = stdio.back();
else
path = stdio[i];
@@ -107,7 +107,7 @@ RunInTerminal(DAP &dap, const protocol::LaunchRequestArguments &arguments) {
llvm::json::Object reverse_request = CreateRunInTerminalReverseRequest(
arguments.configuration.program, arguments.args, arguments.env,
- arguments.cwd, comm_file.m_path, debugger_pid,
+ arguments.cwd, comm_file.m_path, debugger_pid, arguments.stdio,
arguments.console == protocol::eConsoleExternalTerminal);
dap.SendReverseRequest<LogFailureResponseHandler>("runInTerminal",
std::move(reverse_request));
diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp
index 4f26599a49bac..9ecb3a36543ce 100644
--- a/lldb/tools/lldb-dap/JSONUtils.cpp
+++ b/lldb/tools/lldb-dap/JSONUtils.cpp
@@ -866,7 +866,8 @@ llvm::json::Value CreateCompileUnit(lldb::SBCompileUnit &unit) {
llvm::json::Object CreateRunInTerminalReverseRequest(
llvm::StringRef program, const std::vector<std::string> &args,
const llvm::StringMap<std::string> &env, llvm::StringRef cwd,
- llvm::StringRef comm_file, lldb::pid_t debugger_pid, bool external) {
+ llvm::StringRef comm_file, lldb::pid_t debugger_pid,
+ const std::vector<std::optional<std::string>> &stdio, bool external) {
llvm::json::Object run_in_terminal_args;
if (external) {
// This indicates the IDE to open an external terminal window.
@@ -885,6 +886,18 @@ llvm::json::Object CreateRunInTerminalReverseRequest(
}
req_args.push_back("--launch-target");
req_args.push_back(program.str());
+ if (!stdio.empty()) {
+ req_args.push_back("--stdio");
+ std::stringstream ss;
+ for (const auto &file : stdio) {
+ if (file)
+ ss << *file;
+ ss << ":";
+ }
+ std::string files = ss.str();
+ files.pop_back();
+ req_args.push_back(std::move(files));
+ }
req_args.insert(req_args.end(), args.begin(), args.end());
run_in_terminal_args.try_emplace("args", req_args);
diff --git a/lldb/tools/lldb-dap/JSONUtils.h b/lldb/tools/lldb-dap/JSONUtils.h
index e9094f67b94ec..0c865a33a6ce4 100644
--- a/lldb/tools/lldb-dap/JSONUtils.h
+++ b/lldb/tools/lldb-dap/JSONUtils.h
@@ -388,6 +388,10 @@ llvm::json::Value CreateCompileUnit(lldb::SBCompileUnit &unit);
/// launcher uses it on Linux tell the kernel that it should allow the
/// debugger process to attach.
///
+/// \param[in] stdio
+/// An array of file paths for redirecting the program's standard IO
+/// streams.
+///
/// \param[in] external
/// If set to true, the program will run in an external terminal window
/// instead of IDE's integrated terminal.
@@ -398,7 +402,8 @@ llvm::json::Value CreateCompileUnit(lldb::SBCompileUnit &unit);
llvm::json::Object CreateRunInTerminalReverseRequest(
llvm::StringRef program, const std::vector<std::string> &args,
const llvm::StringMap<std::string> &env, llvm::StringRef cwd,
- llvm::StringRef comm_file, lldb::pid_t debugger_pid, bool external);
+ llvm::StringRef comm_file, lldb::pid_t debugger_pid,
+ const std::vector<std::optional<std::string>> &stdio, bool external);
/// Create a "Terminated" JSON object that contains statistics
///
diff --git a/lldb/tools/lldb-dap/Options.td b/lldb/tools/lldb-dap/Options.td
index c8492c6a62b25..5e9dd7a1d6419 100644
--- a/lldb/tools/lldb-dap/Options.td
+++ b/lldb/tools/lldb-dap/Options.td
@@ -47,6 +47,12 @@ def debugger_pid: S<"debugger-pid">,
HelpText<"The PID of the lldb-dap instance that sent the launchInTerminal "
"request when using --launch-target.">;
+def stdio: S<"stdio">,
+ MetaVarName<"<stdin:stdout:stderr:...>">,
+ HelpText<"An array of file paths for redirecting the program's standard IO "
+ "streams. A colon-separated list of entries. Empty value means no "
+ "redirection.">;
+
def repl_mode
: S<"repl-mode">,
MetaVarName<"<mode>">,
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
index 92dada2295841..a85a68b87014c 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
@@ -300,6 +300,7 @@ struct LaunchRequestArguments {
/// terminal or external terminal.
Console console = eConsoleInternal;
+ /// An array of file paths for redirecting the program's standard IO streams.
std::vector<std::optional<std::string>> stdio;
/// @}
diff --git a/lldb/tools/lldb-dap/package.json b/lldb/tools/lldb-dap/package.json
index e961c2e48b258..3f0f150c0d98e 100644
--- a/lldb/tools/lldb-dap/package.json
+++ b/lldb/tools/lldb-dap/package.json
@@ -626,7 +626,10 @@
"stdio": {
"type": "array",
"items": {
- "type": "string"
+ "type": [
+ "string",
+ "null"
+ ]
},
"description": "The stdio property specifies the redirection targets for the debuggee's stdio streams. A null value redirects a stream to the default debug terminal. String can be a path to file, named pipe or TTY device. If less than three values are provided, the list will be padded with the last value. Specifying more than three values will create additional file descriptors (4, 5, etc.).",
"default": []
diff --git a/lldb/tools/lldb-dap/tool/lldb-dap.cpp b/lldb/tools/lldb-dap/tool/lldb-dap.cpp
index 93446c051eb54..709d1fa6ff880 100644
--- a/lldb/tools/lldb-dap/tool/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/tool/lldb-dap.cpp
@@ -16,6 +16,7 @@
#include "lldb/API/SBStream.h"
#include "lldb/Host/Config.h"
#include "lldb/Host/File.h"
+#include "lldb/Host/FileSystem.h"
#include "lldb/Host/MainLoop.h"
#include "lldb/Host/MainLoopBase.h"
#include "lldb/Host/MemoryMonitor.h"
@@ -25,6 +26,7 @@
#include "lldb/lldb-forward.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/ScopeExit.h"
+#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringExtras.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Option/Arg.h"
@@ -143,6 +145,55 @@ static void PrintVersion() {
llvm::outs() << "liblldb: " << lldb::SBDebugger::GetVersionString() << '\n';
}
+#if not defined(_WIN32)
+static llvm::Error RedirectToFile(int fd, llvm::StringRef file, bool read,
+ bool write) {
+ int flags = 0;
+ if (read && write)
+ flags = O_NOCTTY | O_CREAT | O_RDWR;
+ else if (read)
+ flags = O_NOCTTY | O_RDONLY;
+ else
+ flags = O_NOCTTY | O_CREAT | O_WRONLY | O_TRUNC;
+ int target_fd = lldb_private::FileSystem::Instance().Open(file.str().c_str(),
+ flags, 0666);
+ if (target_fd == -1)
+ return llvm::errorCodeToError(
+ std::error_code(errno, std::generic_category()));
+ if (target_fd == fd)
+ return llvm::Error::success();
+ if (dup2(target_fd, fd) == -1)
+ return llvm::errorCodeToError(
+ std::error_code(errno, std::generic_category()));
+ close(target_fd);
+ return llvm::Error::success();
+}
+
+static llvm::Error
+SetupIORedirection(const llvm::SmallVectorImpl<llvm::StringRef> &files) {
+ for (std::size_t i = 0; i < files.size(); i++) {
+ if (files[i].empty())
+ continue;
+ switch (i) {
+ case 0:
+ if (llvm::Error err = RedirectToFile(i, files[i], true, false))
+ return err;
+ break;
+ case 1:
+ case 2:
+ if (llvm::Error err = RedirectToFile(i, files[i], false, true))
+ return err;
+ break;
+ default:
+ if (llvm::Error err = RedirectToFile(i, files[i], true, true))
+ return err;
+ break;
+ }
+ }
+ return llvm::Error::success();
+}
+#endif
+
// If --launch-target is provided, this instance of lldb-dap becomes a
// runInTerminal launcher. It will ultimately launch the program specified in
// the --launch-target argument, which is the original program the user wanted
@@ -165,6 +216,7 @@ static void PrintVersion() {
static llvm::Error LaunchRunInTerminalTarget(llvm::opt::Arg &target_arg,
llvm::StringRef comm_file,
lldb::pid_t debugger_pid,
+ llvm::StringRef stdio,
char *argv[]) {
#if defined(_WIN32)
return llvm::createStringError(
@@ -179,6 +231,15 @@ static llvm::Error LaunchRunInTerminalTarget(llvm::opt::Arg &target_arg,
(void)prctl(PR_SET_PTRACER, debugger_pid, 0, 0, 0);
#endif
+ if (!stdio.empty()) {
+ llvm::SmallVector<llvm::StringRef, 3> files;
+ stdio.split(files, ':');
+ while (files.size() < 3)
+ files.push_back(files.back());
+ if (llvm::Error err = SetupIORedirection(files))
+ return err;
+ }
+
RunInTerminalLauncherCommChannel comm_channel(comm_file);
if (llvm::Error err = comm_channel.NotifyPid())
return err;
@@ -484,9 +545,10 @@ int main(int argc, char *argv[]) {
break;
}
}
+ llvm::StringRef stdio = input_args.getLastArgValue(OPT_stdio);
if (llvm::Error err =
LaunchRunInTerminalTarget(*target_arg, comm_file->getValue(), pid,
- argv + target_args_pos)) {
+ stdio, argv + target_args_pos)) {
llvm::errs() << llvm::toString(std::move(err)) << '\n';
return EXIT_FAILURE;
}
|
DrSergei
force-pushed
the
extend-stdio-redirection
branch
2 times, most recently
from
d8247a3 to
0fed265
Compare
walter-erquinigo
approved these changes
Sep 29, 2025
lldb/tools/lldb-dap/JSONUtils.cpp
Outdated
| if (!stdio.empty()) { | ||
| req_args.push_back("--stdio"); | ||
| std::stringstream ss; | ||
| for (const auto &file : stdio) { |
| HelpText<"The PID of the lldb-dap instance that sent the launchInTerminal " | ||
| "request when using --launch-target.">; | ||
|
|
||
| def stdio: S<"stdio">, |
There was a problem hiding this comment.
would it be possible to redirect 2 to 1, for example, instead of providing full paths? If that works, state a couple of examples of different kinds of inputs here.
There was a problem hiding this comment.
I expect only paths here because we have full control over this option when starting the runInTerminal launcher. Therefore, we can resolve everything on the callee side
|
I updated the redirection logic to handle the case where multiple descriptors are redirected to the same file to avoid opening it twice. |
JDevlieghere
approved these changes
Oct 7, 2025
Comment on lines
161
to
167
| int flags = 0; | ||
| if (fdg.read && fdg.write) | ||
| flags = O_NOCTTY | O_CREAT | O_RDWR; | ||
| else if (fdg.read) | ||
| flags = O_NOCTTY | O_RDONLY; | ||
| else | ||
| flags = O_NOCTTY | O_CREAT | O_WRONLY | O_TRUNC; |
There was a problem hiding this comment.
Maybe consider making this FDGroup::GetFlags() and have some early returns to avoid all the else cases? Something like:
int GetFlags() const {
if (fdg.read && fdg.write)
return O_NOCTTY | O_CREAT | O_RDWR;
if (fdg.read)
return O_NOCTTY | O_RDONLY;
return O_NOCTTY | O_CREAT | O_WRONLY | O_TRUNC;
}
DrSergei
force-pushed
the
extend-stdio-redirection
branch
from
725ed72 to
c970d17
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch extends stdio redirection support to integrated and external terminals. Currently, these cases are not covered by the standard logic because
attachis used instead oflaunch. To be honest,runInTerminalin VSCode request supports>and<for redirection, but not2>. We could use theargsCanBeInterpretedByShelloption to use full power of shell, however it requieres proper escaping of arguments on lldb-dap side. So, I think it will be better to have the only one option to control stdio redirection that works consistently across all console modes.Also, fixed a small typo in a comparison that was leading to out-of-bound access, and added
nullas a possible value forstdioarray in package.json.