From 5007dd9d0945938f2177142e9784e240490e7050 Mon Sep 17 00:00:00 2001 From: Charles Zablit Date: Thu, 26 Feb 2026 23:03:51 +0100 Subject: [PATCH] [lldb][repl] fix output interleaving with the status line (#183600) This patch fixes the REPL's output from interleaving with the status line by locking the stream before printing. --- lldb/include/lldb/Expression/REPL.h | 2 +- lldb/source/Expression/REPL.cpp | 108 ++++++++++--------- lldb/source/Plugins/REPL/Clang/ClangREPL.cpp | 13 ++- lldb/source/Plugins/REPL/Clang/ClangREPL.h | 3 +- 4 files changed, 72 insertions(+), 54 deletions(-) diff --git a/lldb/include/lldb/Expression/REPL.h b/lldb/include/lldb/Expression/REPL.h index b1dc830ec8bf..809b4d8e9f5c 100644 --- a/lldb/include/lldb/Expression/REPL.h +++ b/lldb/include/lldb/Expression/REPL.h @@ -144,7 +144,7 @@ protected: virtual lldb::LanguageType GetLanguage() = 0; virtual bool PrintOneVariable(Debugger &debugger, - lldb::StreamFileSP &output_sp, + lldb::LockableStreamFileSP &output_stream_sp, lldb::ValueObjectSP &valobj_sp, ExpressionVariable *var = nullptr) = 0; diff --git a/lldb/source/Expression/REPL.cpp b/lldb/source/Expression/REPL.cpp index c6cf6d87a722..2c73703bf441 100644 --- a/lldb/source/Expression/REPL.cpp +++ b/lldb/source/Expression/REPL.cpp @@ -189,29 +189,30 @@ int REPL::IOHandlerFixIndentation(IOHandler &io_handler, } static bool ReadCode(const std::string &path, std::string &code, - lldb::StreamFileSP &error_sp) { + lldb::LockableStreamFileSP &error_stream_sp) { auto &fs = FileSystem::Instance(); llvm::Twine pathTwine(path); if (!fs.Exists(pathTwine)) { - error_sp->Printf("no such file at path '%s'\n", path.c_str()); + error_stream_sp->Lock().Printf("no such file at path '%s'\n", path.c_str()); return false; } if (!fs.Readable(pathTwine)) { - error_sp->Printf("could not read file at path '%s'\n", path.c_str()); + error_stream_sp->Lock().Printf("could not read file at path '%s'\n", + path.c_str()); return false; } const size_t file_size = fs.GetByteSize(pathTwine); const size_t max_size = code.max_size(); if (file_size > max_size) { - error_sp->Printf("file at path '%s' too large: " - "file_size = %zu, max_size = %zu\n", - path.c_str(), file_size, max_size); + error_stream_sp->Lock().Printf("file at path '%s' too large: " + "file_size = %zu, max_size = %zu\n", + path.c_str(), file_size, max_size); return false; } auto data_sp = fs.CreateDataBuffer(pathTwine); if (data_sp == nullptr) { - error_sp->Printf("could not create buffer for file at path '%s'\n", - path.c_str()); + error_stream_sp->Lock().Printf( + "could not create buffer for file at path '%s'\n", path.c_str()); return false; } code.assign((const char *)data_sp->GetBytes(), data_sp->GetByteSize()); @@ -219,10 +220,10 @@ static bool ReadCode(const std::string &path, std::string &code, } void REPL::IOHandlerInputComplete(IOHandler &io_handler, std::string &code) { - lldb::StreamFileSP output_sp = std::make_shared( - io_handler.GetOutputStreamFileSP()->GetUnlockedFileSP()); - lldb::StreamFileSP error_sp = std::make_shared( - io_handler.GetErrorStreamFileSP()->GetUnlockedFileSP()); + lldb::LockableStreamFileSP output_stream_sp = + io_handler.GetOutputStreamFileSP(); + lldb::LockableStreamFileSP error_stream_sp = + io_handler.GetErrorStreamFileSP(); bool extra_line = false; bool did_quit = false; @@ -258,8 +259,10 @@ void REPL::IOHandlerInputComplete(IOHandler &io_handler, std::string &code) { // Execute the command CommandReturnObject result(debugger.GetUseColor()); - result.SetImmediateOutputStream(output_sp); - result.SetImmediateErrorStream(error_sp); + result.SetImmediateOutputStream(std::make_shared( + output_stream_sp->GetUnlockedFileSP())); + result.SetImmediateErrorStream( + std::make_shared(error_stream_sp->GetUnlockedFileSP())); ci.HandleCommand(code.c_str(), eLazyBoolNo, result); if (saved_prompt_on_quit) @@ -302,7 +305,7 @@ void REPL::IOHandlerInputComplete(IOHandler &io_handler, std::string &code) { // User wants to read code from a file. // Interpret rest of line as a literal path. auto path = llvm::StringRef(code.substr(1)).trim().str(); - if (!ReadCode(path, code, error_sp)) { + if (!ReadCode(path, code, error_stream_sp)) { return; } } @@ -318,7 +321,8 @@ void REPL::IOHandlerInputComplete(IOHandler &io_handler, std::string &code) { } } - const bool colorize_err = error_sp->GetFile().GetIsTerminalWithColors(); + const bool colorize_err = + error_stream_sp->Lock().GetFile().GetIsTerminalWithColors(); EvaluateExpressionOptions expr_options = m_expr_options; expr_options.SetCoerceToId(m_varobj_options.use_object_desc); @@ -347,7 +351,7 @@ void REPL::IOHandlerInputComplete(IOHandler &io_handler, std::string &code) { if (llvm::Error err = OnExpressionEvaluated(exe_ctx, code, expr_options, execution_results, result_valobj_sp, error)) { - *error_sp << llvm::toString(std::move(err)) << "\n"; + error_stream_sp->Lock() << llvm::toString(std::move(err)) << "\n"; } else if (process_sp && process_sp->IsAlive()) { bool add_to_code = true; bool handled = false; @@ -355,11 +359,12 @@ void REPL::IOHandlerInputComplete(IOHandler &io_handler, std::string &code) { lldb::Format format = m_format_options.GetFormat(); if (result_valobj_sp->GetError().Success()) { - handled |= PrintOneVariable(debugger, output_sp, result_valobj_sp); + handled |= + PrintOneVariable(debugger, output_stream_sp, result_valobj_sp); } else if (result_valobj_sp->GetError().GetError() == UserExpression::kNoResult) { if (format != lldb::eFormatVoid && debugger.GetNotifyVoid()) { - error_sp->PutCString("(void)\n"); + error_stream_sp->Lock().PutCString("(void)\n"); handled = true; } } @@ -372,48 +377,53 @@ void REPL::IOHandlerInputComplete(IOHandler &io_handler, std::string &code) { persistent_state->GetVariableAtIndex(vi); lldb::ValueObjectSP valobj_sp = persistent_var_sp->GetValueObject(); - PrintOneVariable(debugger, output_sp, valobj_sp, + PrintOneVariable(debugger, output_stream_sp, valobj_sp, persistent_var_sp.get()); } } if (!handled) { - bool useColors = error_sp->GetFile().GetIsTerminalWithColors(); + LockedStreamFile locked_error_stream = error_stream_sp->Lock(); + bool useColors = + locked_error_stream.GetFile().GetIsTerminalWithColors(); switch (execution_results) { case lldb::eExpressionSetupError: case lldb::eExpressionParseError: add_to_code = false; [[fallthrough]]; case lldb::eExpressionDiscarded: - error_sp->Printf("%s\n", error.AsCString()); + locked_error_stream.Printf("%s\n", error.AsCString()); break; case lldb::eExpressionCompleted: break; case lldb::eExpressionInterrupted: if (useColors) { - error_sp->Printf(ANSI_ESCAPE1(ANSI_FG_COLOR_RED)); - error_sp->Printf(ANSI_ESCAPE1(ANSI_CTRL_BOLD)); + locked_error_stream.Printf(ANSI_ESCAPE1(ANSI_FG_COLOR_RED)); + locked_error_stream.Printf(ANSI_ESCAPE1(ANSI_CTRL_BOLD)); } - error_sp->Printf("Execution interrupted. "); + locked_error_stream.Printf("Execution interrupted. "); if (useColors) - error_sp->Printf(ANSI_ESCAPE1(ANSI_CTRL_NORMAL)); - error_sp->Printf("Enter code to recover and continue.\nEnter LLDB " - "commands to investigate (type :help for " - "assistance.)\n"); + locked_error_stream.Printf(ANSI_ESCAPE1(ANSI_CTRL_NORMAL)); + locked_error_stream.Printf( + "Enter code to recover and continue.\nEnter LLDB " + "commands to investigate (type :help for " + "assistance.)\n"); break; case lldb::eExpressionHitBreakpoint: // Breakpoint was hit, drop into LLDB command interpreter if (useColors) { - error_sp->Printf(ANSI_ESCAPE1(ANSI_FG_COLOR_RED)); - error_sp->Printf(ANSI_ESCAPE1(ANSI_CTRL_BOLD)); + locked_error_stream.Printf(ANSI_ESCAPE1(ANSI_FG_COLOR_RED)); + locked_error_stream.Printf(ANSI_ESCAPE1(ANSI_CTRL_BOLD)); } - output_sp->Printf("Execution stopped at breakpoint. "); + output_stream_sp->Lock().Printf( + "Execution stopped at breakpoint. "); if (useColors) - error_sp->Printf(ANSI_ESCAPE1(ANSI_CTRL_NORMAL)); - output_sp->Printf("Enter LLDB commands to investigate (type help " - "for assistance.)\n"); + locked_error_stream.Printf(ANSI_ESCAPE1(ANSI_CTRL_NORMAL)); + locked_error_stream.Printf( + "Enter LLDB commands to investigate (type help " + "for assistance.)\n"); { lldb::IOHandlerSP io_handler_sp(ci.GetIOHandler()); if (io_handler_sp) { @@ -424,24 +434,24 @@ void REPL::IOHandlerInputComplete(IOHandler &io_handler, std::string &code) { break; case lldb::eExpressionTimedOut: - error_sp->Printf("error: timeout\n"); + locked_error_stream.Printf("error: timeout\n"); if (error.AsCString()) - error_sp->Printf("error: %s\n", error.AsCString()); + locked_error_stream.Printf("error: %s\n", error.AsCString()); break; case lldb::eExpressionResultUnavailable: // Shoulnd't happen??? - error_sp->Printf("error: could not fetch result -- %s\n", - error.AsCString()); + locked_error_stream.Printf("error: could not fetch result -- %s\n", + error.AsCString()); break; case lldb::eExpressionStoppedForDebug: // Shoulnd't happen??? - error_sp->Printf("error: stopped for debug -- %s\n", - error.AsCString()); + locked_error_stream.Printf("error: stopped for debug -- %s\n", + error.AsCString()); break; case lldb::eExpressionThreadVanished: // Shoulnd't happen??? - error_sp->Printf("error: expression thread vanished -- %s\n", - error.AsCString()); + locked_error_stream.Printf( + "error: expression thread vanished -- %s\n", error.AsCString()); break; } } @@ -466,8 +476,9 @@ void REPL::IOHandlerInputComplete(IOHandler &io_handler, std::string &code) { file.get()->Close(); } else { std::string message = llvm::toString(file.takeError()); - error_sp->Printf("error: couldn't open %s: %s\n", - m_repl_source_path.c_str(), message.c_str()); + error_stream_sp->Lock().Printf("error: couldn't open %s: %s\n", + m_repl_source_path.c_str(), + message.c_str()); } // Now set the default file and line to the REPL source file @@ -478,16 +489,15 @@ void REPL::IOHandlerInputComplete(IOHandler &io_handler, std::string &code) { static_cast(io_handler) .SetBaseLineNumber(m_code.GetSize() + 1); } - if (extra_line) { - output_sp->Printf("\n"); - } + if (extra_line) + output_stream_sp->Lock().Printf("\n"); } } // Don't complain about the REPL process going away if we are in the // process of quitting. if (!did_quit && (!process_sp || !process_sp->IsAlive())) { - error_sp->Printf( + error_stream_sp->Lock().Printf( "error: REPL process is no longer alive, exiting REPL\n"); io_handler.SetIsDone(true); } diff --git a/lldb/source/Plugins/REPL/Clang/ClangREPL.cpp b/lldb/source/Plugins/REPL/Clang/ClangREPL.cpp index 41e3e2706c1a..7e8c7827a000 100644 --- a/lldb/source/Plugins/REPL/Clang/ClangREPL.cpp +++ b/lldb/source/Plugins/REPL/Clang/ClangREPL.cpp @@ -85,7 +85,7 @@ lldb::offset_t ClangREPL::GetDesiredIndentation(const StringList &lines, lldb::LanguageType ClangREPL::GetLanguage() { return m_language; } bool ClangREPL::PrintOneVariable(Debugger &debugger, - lldb::StreamFileSP &output_sp, + lldb::LockableStreamFileSP &output_stream_sp, lldb::ValueObjectSP &valobj_sp, ExpressionVariable *var) { // If a ExpressionVariable was passed, check first if that variable is just @@ -95,8 +95,15 @@ bool ClangREPL::PrintOneVariable(Debugger &debugger, if (m_implicit_expr_result_regex.Execute(var->GetName().GetStringRef())) return true; } - if (llvm::Error error = valobj_sp->Dump(*output_sp)) - *output_sp << "error: " << toString(std::move(error)); + + { + // Suspend the statusline while printing to prevent its ANSI cursor + // save/restore sequences from interleaving with the output. + LockedStreamFile locked_stream = output_stream_sp->Lock(); + + if (llvm::Error error = valobj_sp->Dump(locked_stream)) + locked_stream << "error: " << toString(std::move(error)); + } return true; } diff --git a/lldb/source/Plugins/REPL/Clang/ClangREPL.h b/lldb/source/Plugins/REPL/Clang/ClangREPL.h index 7d219c46189e..f2573a078227 100644 --- a/lldb/source/Plugins/REPL/Clang/ClangREPL.h +++ b/lldb/source/Plugins/REPL/Clang/ClangREPL.h @@ -48,7 +48,8 @@ protected: lldb::LanguageType GetLanguage() override; - bool PrintOneVariable(Debugger &debugger, lldb::StreamFileSP &output_sp, + bool PrintOneVariable(Debugger &debugger, + lldb::LockableStreamFileSP &output_stream_sp, lldb::ValueObjectSP &valobj_sp, ExpressionVariable *var = nullptr) override;