Author: Jonas Devlieghere Date: 2026-07-10T17:02:01-07:00 New Revision: a35565161078268c253df85369b3878ca33c726b
URL: https://github.com/llvm/llvm-project/commit/a35565161078268c253df85369b3878ca33c726b DIFF: https://github.com/llvm/llvm-project/commit/a35565161078268c253df85369b3878ca33c726b.diff LOG: [lldb] Serialize scripted-command output with the statusline (#208609) A Python command's print() writes to sys.stdout, which the interpreter session pointed at the debugger's raw terminal descriptor via PyFile_FromFd. Those writes bypass the debugger's output lock, so they can race with the statusline, resulting in truncated output. Back the session's terminal stdout and stderr with a pipe instead. A reader thread drains the pipe and writes to the terminal through Debugger::PrintAsync, which takes the same output lock as the statusline. Only redirect when a statusline could actually be drawing: the pipe and reader thread are set up only when StatuslineSupported() holds (show-statusline is enabled and the output is an escape-code-capable terminal) and the target is the debugger's own terminal. The interactive interpreter opts out entirely, since input()'s readline line editing and echo need both sys.stdin and sys.stdout to be the real terminal. Python still gets a real line-buffered text file over the pipe, so output stays live and file objects passed to SB APIs (SetImmediateOutputFile, subprocess, fileno) keep working. On session teardown the wrappers are flushed before the pipe is closed so a retained sys.stdout does not strand a trailing line or write into a closed descriptor. rdar://181841574 Added: lldb/test/API/functionalities/statusline/statusline_flood.py Modified: lldb/include/lldb/Core/Debugger.h lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h lldb/test/API/functionalities/statusline/TestStatusline.py Removed: ################################################################################ diff --git a/lldb/include/lldb/Core/Debugger.h b/lldb/include/lldb/Core/Debugger.h index 11274abc247d3..2501eeabfe506 100644 --- a/lldb/include/lldb/Core/Debugger.h +++ b/lldb/include/lldb/Core/Debugger.h @@ -458,6 +458,10 @@ class Debugger : public std::enable_shared_from_this<Debugger>, /// Redraw the statusline if enabled. void RedrawStatusline(std::optional<ExecutionContextRef> exe_ctx_ref); + /// Whether the statusline can be drawn: show-statusline is enabled and the + /// output is an escape-code-capable terminal. + bool StatuslineSupported(); + /// Flush cached state (e.g. stale execution context in the statusline). void FlushStatusLine(); @@ -717,7 +721,6 @@ class Debugger : public std::enable_shared_from_this<Debugger>, /// @} bool IsEscapeCodeCapableTTY(); - bool StatuslineSupported(); void PushIOHandler(const lldb::IOHandlerSP &reader_sp, bool cancel_top_handler = true); diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp index 3b822f2585c5e..c86c2f5649497 100644 --- a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp +++ b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp @@ -25,6 +25,7 @@ #include "lldb/Core/ThreadedCommunication.h" #include "lldb/DataFormatters/TypeSummary.h" #include "lldb/Host/Config.h" +#include "lldb/Host/ConnectionFileDescriptor.h" #include "lldb/Host/FileSystem.h" #include "lldb/Host/HostInfo.h" #include "lldb/Host/Pipe.h" @@ -46,6 +47,10 @@ #include "llvm/Support/FileSystem.h" #include "llvm/Support/FormatAdapters.h" +#if defined(_WIN32) +#include "lldb/Host/windows/ConnectionGenericFileWindows.h" +#endif + #include <cstdio> #include <cstdlib> #include <memory> @@ -444,6 +449,88 @@ ScriptInterpreterPythonImpl::ScriptInterpreterPythonImpl(Debugger &debugger) RunSimpleString(run_string.GetData()); } +/// A Python sys.stdout/stderr file backed by a pipe whose read end is drained +/// by a reader thread that writes to the debugger's terminal under the output +/// lock (Debugger::PrintAsync). Handing Python the raw terminal descriptor +/// instead lets a script's print() race the statusline, which redraws on the +/// event thread under that lock. Its cursor save/restore then rewinds over and +/// eats the script's output. +class ScriptInterpreterPythonImpl::SessionIORedirect { +public: + static std::unique_ptr<SessionIORedirect> Create(lldb::user_id_t debugger_id, + bool is_stdout) { + Pipe pipe; + if (pipe.CreateNew().Fail()) + return nullptr; + + std::unique_ptr<SessionIORedirect> redirect( + new SessionIORedirect(debugger_id, is_stdout)); + +#if defined(_WIN32) + lldb::file_t read_handle = pipe.GetReadNativeHandle(); + pipe.ReleaseReadFileDescriptor(); + std::unique_ptr<Connection> conn = + std::make_unique<ConnectionGenericFile>(read_handle, true); +#else + std::unique_ptr<Connection> conn = + std::make_unique<ConnectionFileDescriptor>( + pipe.ReleaseReadFileDescriptor(), /*owns_fd=*/true); +#endif + if (!conn->IsConnected()) + return nullptr; + + redirect->m_communication.SetConnection(std::move(conn)); + redirect->m_communication.SetReadThreadBytesReceivedCallback( + ReadThreadBytesReceived, redirect.get()); + if (!redirect->m_communication.StartReadThread()) + return nullptr; + redirect->m_connected = true; + + // The write end is owned here. Python only borrows its descriptor. + redirect->m_write_file_sp = std::make_shared<NativeFile>( + pipe.ReleaseWriteFileDescriptor(), File::eOpenOptionWriteOnly, + NativeFile::Owned); + return redirect; + } + + ~SessionIORedirect() { + if (!m_connected) + return; + // Close the write end so the reader sees EOF and exits, then join it. + if (m_write_file_sp) + m_write_file_sp->Close(); + m_communication.JoinReadThread(); + m_communication.Disconnect(); + } + + int GetWriteDescriptor() const { + return m_write_file_sp ? m_write_file_sp->GetDescriptor() + : File::kInvalidDescriptor; + } + +private: + SessionIORedirect(lldb::user_id_t debugger_id, bool is_stdout) + : m_debugger_id(debugger_id), m_is_stdout(is_stdout), + m_communication("lldb.ScriptInterpreterPython.io-redirect") {} + + static void ReadThreadBytesReceived(void *baton, const void *src, + size_t src_len) { + if (!src || !src_len) + return; + auto *self = static_cast<SessionIORedirect *>(baton); + if (lldb::DebuggerSP debugger_sp = + Debugger::FindDebuggerWithID(self->m_debugger_id)) + debugger_sp->PrintAsync(static_cast<const char *>(src), src_len, + self->m_is_stdout); + } + + lldb::user_id_t m_debugger_id; + bool m_is_stdout; + lldb::FileSP m_write_file_sp; + ThreadedCommunication m_communication; + bool m_connected = false; +}; + ScriptInterpreterPythonImpl::~ScriptInterpreterPythonImpl() { // the session dictionary may hold objects with complex state which means // that they may need to be torn down with some level of smarts and that, in @@ -567,6 +654,26 @@ void ScriptInterpreterPythonImpl::LeaveSession() { if (PyThreadState_GetDict()) { PythonDictionary &sys_module_dict = GetSysModuleDictionary(); if (sys_module_dict.IsValid()) { + // Flush the pipe-backed wrappers while they are still sys.stdout/stderr. + // Line buffering already flushes on each newline, but a trailing + // unterminated line would otherwise be stranded (and later flushed into + // a closed descriptor) once we close the pipe write end below. + auto flush_redirect = [&](const char *py_name, + std::unique_ptr<SessionIORedirect> &redirect) { + if (!redirect) + return; + PythonObject file = + sys_module_dict.GetItemForKey(PythonString(py_name)); + if (!file.IsValid()) + return; + if (llvm::Expected<PythonObject> result = file.CallMethod("flush")) + (void)result; + else + llvm::consumeError(result.takeError()); + }; + flush_redirect("stdout", m_stdout_redirect); + flush_redirect("stderr", m_stderr_redirect); + if (m_saved_stdin.IsValid()) { sys_module_dict.SetItemForKey(PythonString("stdin"), m_saved_stdin); m_saved_stdin.Reset(); @@ -582,19 +689,86 @@ void ScriptInterpreterPythonImpl::LeaveSession() { } } + // Tear down the pipe redirects (closes each write end and joins its reader). + // The wrappers were flushed above, so nothing buffered is lost. + m_stdout_redirect.reset(); + m_stderr_redirect.reset(); + m_session_is_active = false; } +bool ScriptInterpreterPythonImpl::RedirectTerminalHandleThroughLock( + const char *py_name, PythonObject &save_file, const char *mode, + File &file) { + const bool is_stdout = ::strcmp(py_name, "stdout") == 0; + if (!is_stdout && ::strcmp(py_name, "stderr") != 0) + return false; + + // The statusline is the only writer that races Python's terminal output. + // When it isn't drawing there is nothing to serialize against, so keep the + // normal wrapping and skip the reader thread and pipe. + if (!m_debugger.StatuslineSupported()) + return false; + + // Only the debugger's own terminal races the statusline. A redirect to a + // pipe or user file (a diff erent descriptor) is wrapped normally. + lldb::FileSP debugger_file = + is_stdout ? m_debugger.GetOutputFileSP() : m_debugger.GetErrorFileSP(); + int fd = file.GetDescriptor(); + if (!debugger_file || fd == File::kInvalidDescriptor || + fd != debugger_file->GetDescriptor()) + return false; + + std::unique_ptr<SessionIORedirect> &redirect = + is_stdout ? m_stdout_redirect : m_stderr_redirect; + redirect = SessionIORedirect::Create(m_debugger.GetID(), is_stdout); + if (!redirect) + return false; + + // Line-buffer the wrapper so each print() reaches the reader (and the + // terminal) promptly: the pipe descriptor is not a tty, so the default + // buffering would hold output back until the buffer filled. + PyObject *pipe_file = PyFile_FromFd( + redirect->GetWriteDescriptor(), nullptr, mode, /*buffering=*/1, + /*encoding=*/nullptr, /*errors=*/"ignore", /*newline=*/nullptr, + /*closefd=*/0); + if (!pipe_file) { + // Fall back to the raw descriptor. That reopens the statusline race, so + // leave a breadcrumb rather than failing silently. + LLDB_LOG(GetLog(LLDBLog::Script), + "failed to wrap sys.{0} on a synchronized pipe; falling back to " + "the unsynchronized terminal descriptor", + py_name); + PyErr_Clear(); + redirect.reset(); + return false; + } + + PythonObject new_file(PyRefType::Owned, pipe_file); + PythonDictionary &sys_module_dict = GetSysModuleDictionary(); + save_file = sys_module_dict.GetItemForKey(PythonString(py_name)); + sys_module_dict.SetItemForKey(PythonString(py_name), new_file); + return true; +} + bool ScriptInterpreterPythonImpl::SetStdHandle(FileSP file_sp, const char *py_name, PythonObject &save_file, - const char *mode) { + const char *mode, + bool serialize_terminal_output) { if (!file_sp || !*file_sp) { save_file.Reset(); return false; } File &file = *file_sp; + // When stdout/stderr point at the debugger's own terminal, route Python's + // output through a pipe drained under the output lock so a script's print() + // cannot race the statusline. Any other target keeps the normal wrapping. + if (serialize_terminal_output && + RedirectTerminalHandleThroughLock(py_name, save_file, mode, file)) + return true; + // Flush the file before giving it to python to avoid interleaved output. file.Flush(); @@ -675,22 +849,31 @@ bool ScriptInterpreterPythonImpl::EnterSession(uint16_t on_entry_flags, if (on_entry_flags & Locker::NoSTDIN) { m_saved_stdin.Reset(); } else { - if (!SetStdHandle(in_sp, "stdin", m_saved_stdin, "r")) { + if (!SetStdHandle(in_sp, "stdin", m_saved_stdin, "r", + /*serialize_terminal_output=*/false)) { if (top_in_sp) - SetStdHandle(top_in_sp, "stdin", m_saved_stdin, "r"); + SetStdHandle(top_in_sp, "stdin", m_saved_stdin, "r", + /*serialize_terminal_output=*/false); } } - if (!SetStdHandle(out_sp, "stdout", m_saved_stdout, "w")) { + // Serialize terminal output for every session except those that opt out + // with NoOutputRedirect (see the flag for why). + const bool serialize_terminal_output = + !(on_entry_flags & Locker::NoOutputRedirect); + + if (!SetStdHandle(out_sp, "stdout", m_saved_stdout, "w", + serialize_terminal_output)) { if (top_out_sp) SetStdHandle(top_out_sp->GetUnlockedFileSP(), "stdout", m_saved_stdout, - "w"); + "w", serialize_terminal_output); } - if (!SetStdHandle(err_sp, "stderr", m_saved_stderr, "w")) { + if (!SetStdHandle(err_sp, "stderr", m_saved_stderr, "w", + serialize_terminal_output)) { if (top_err_sp) SetStdHandle(top_err_sp->GetUnlockedFileSP(), "stderr", m_saved_stderr, - "w"); + "w", serialize_terminal_output); } } diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h index 863cf27785824..9f83f0794b19b 100644 --- a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h +++ b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h @@ -200,7 +200,7 @@ class ScriptInterpreterPythonImpl : public ScriptInterpreterPython { bool GetLongHelpForCommandObject(StructuredData::GenericSP cmd_obj_sp, std::string &dest) override; - + StructuredData::ObjectSP GetOptionsForCommandObject(StructuredData::GenericSP cmd_obj_sp) override; @@ -209,7 +209,7 @@ class ScriptInterpreterPythonImpl : public ScriptInterpreterPython { bool SetOptionValueForCommandObject(StructuredData::GenericSP cmd_obj_sp, ExecutionContext *exe_ctx, - llvm::StringRef long_option, + llvm::StringRef long_option, llvm::StringRef value) override; void OptionParsingStartedForCommandObject( @@ -273,8 +273,7 @@ class ScriptInterpreterPythonImpl : public ScriptInterpreterPython { Status SetBreakpointCommandCallback(BreakpointOptions &bp_options, const char *command_body_text, StructuredData::ObjectSP extra_args_sp, - bool uses_extra_args, - bool is_callback); + bool uses_extra_args, bool is_callback); /// Set a one-liner as the callback for the watchpoint. void SetWatchpointCommandCallback(WatchpointOptions *wp_options, @@ -307,7 +306,12 @@ class ScriptInterpreterPythonImpl : public ScriptInterpreterPython { AcquireLock = 0x0001, InitSession = 0x0002, InitGlobals = 0x0004, - NoSTDIN = 0x0008 + NoSTDIN = 0x0008, + // Keep sys.stdout/stderr on the real terminal instead of routing them + // through the output-lock pipe (see EnterSession). Set by the interactive + // interpreter, whose input() needs both the real stdin and stdout for + // readline-based line editing and echo. + NoOutputRedirect = 0x0010 }; enum OnLeave { @@ -404,12 +408,31 @@ class ScriptInterpreterPythonImpl : public ScriptInterpreterPython { bool GetEmbeddedInterpreterModuleObjects(); + /// Point sys.\p py_name at \p file. When \p serialize_terminal_output is true + /// and \p file is the debugger's own terminal, the output is routed through a + /// lock-synchronized pipe instead (see RedirectTerminalHandleThroughLock) so + /// it cannot race the statusline redraw. bool SetStdHandle(lldb::FileSP file, const char *py_name, - python::PythonObject &save_file, const char *mode); + python::PythonObject &save_file, const char *mode, + bool serialize_terminal_output); + + /// Pipe-backed sys.stdout/stderr whose writes are serialized against the + /// statusline redraw. Defined and documented in the implementation file. + class SessionIORedirect; + + /// If \p file is the debugger's own terminal, point sys.\p py_name at a + /// pipe-backed file whose writes are serialized through the output lock (see + /// SessionIORedirect) and return true. Return false to let SetStdHandle wrap + /// \p file normally. + bool RedirectTerminalHandleThroughLock(const char *py_name, + python::PythonObject &save_file, + const char *mode, File &file); python::PythonObject m_saved_stdin; python::PythonObject m_saved_stdout; python::PythonObject m_saved_stderr; + std::unique_ptr<SessionIORedirect> m_stdout_redirect; + std::unique_ptr<SessionIORedirect> m_stderr_redirect; python::PythonModule m_main_module; python::PythonDictionary m_session_dict; python::PythonDictionary m_sys_module_dict; @@ -458,7 +481,8 @@ class IOHandlerPythonInterpreter : public IOHandler { m_python, ScriptInterpreterPythonImpl::Locker::AcquireLock | ScriptInterpreterPythonImpl::Locker::InitSession | - ScriptInterpreterPythonImpl::Locker::InitGlobals, + ScriptInterpreterPythonImpl::Locker::InitGlobals | + ScriptInterpreterPythonImpl::Locker::NoOutputRedirect, ScriptInterpreterPythonImpl::Locker::FreeAcquiredLock | ScriptInterpreterPythonImpl::Locker::TearDownSession); diff --git a/lldb/test/API/functionalities/statusline/TestStatusline.py b/lldb/test/API/functionalities/statusline/TestStatusline.py index 8891a561419cf..a77e18e9bc6e9 100644 --- a/lldb/test/API/functionalities/statusline/TestStatusline.py +++ b/lldb/test/API/functionalities/statusline/TestStatusline.py @@ -121,6 +121,52 @@ def test_no_target(self): self.expect("set set show-statusline true", ["no target"]) + def test_scripted_command_output_not_eaten(self): + """A scripted command's print() output must be serialized with the + statusline redraw. The statusline brackets its redraw with a cursor + save (ESC 7) and restore (ESC 8); if command output lands in between, + the restore rewinds the cursor back over it and the terminal + overwrites it, so the output is non-deterministically eaten. Assert + that no command output appears inside a save/restore pair.""" + self.launch(use_colors=False) + self.resize() + self.expect("settings set show-statusline true", ["no target"]) + + flood = os.path.join( + os.path.dirname(os.path.realpath(__file__)), "statusline_flood.py" + ) + self.expect('command script import "{}"'.format(flood)) + + tee = CaptureTee() + self.child.logfile_read = tee + self.child.sendline("statusline_flood") + self.child.expect("MARKER_0049") + self.child.expect("(lldb)") + self.child.logfile_read = None + + data = tee.data + # The test is only meaningful if the statusline actually redrew and the + # command actually produced output. + self.assertIn(b"\x1b7", data) + self.assertIn(b"MARKER_0049", data) + + # No command output may appear between a cursor save (ESC 7) and its + # matching restore (ESC 8); that would mean the two writers interleaved. + pos = 0 + while True: + save = data.find(b"\x1b7", pos) + if save == -1: + break + restore = data.find(b"\x1b8", save) + if restore == -1: + break + self.assertNotIn( + b"MARKER_", + data[save + 2 : restore], + "scripted command output was spliced into a statusline redraw", + ) + pos = restore + 2 + @skipIfEditlineSupportMissing def test_resize(self): """Test that move the cursor when resizing.""" diff --git a/lldb/test/API/functionalities/statusline/statusline_flood.py b/lldb/test/API/functionalities/statusline/statusline_flood.py new file mode 100644 index 0000000000000..2ce5af925453f --- /dev/null +++ b/lldb/test/API/functionalities/statusline/statusline_flood.py @@ -0,0 +1,17 @@ +"""Helper for TestStatusline.test_scripted_command_output_not_eaten. + +Registers a command that floods output while emitting progress events, so the +statusline redraws (on the event thread) concurrently with the command output. +""" + +import lldb + + [email protected]("statusline_flood") +def statusline_flood(debugger, command, result, internal_dict): + count = 50 + progress = lldb.SBProgress("flood", "working", count, debugger) + for i in range(count): + print("MARKER_{:04d}".format(i), flush=True) + progress.Increment(1, "step {}".format(i)) + progress.Finalize() _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
