From 1a216fb15a188f9ac7de6d79267b3cfb2946f792 Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere Date: Tue, 8 Jun 2021 10:46:43 -0700 Subject: [PATCH] [lldb] Don't print script output twice in HandleCommand When executing a script command in HandleCommand(s) we currently print its output twice You can see this issue in action when adding a breakpoint command: (lldb) b main Breakpoint 1: where = main.out`main + 13 at main.cpp:2:3, address = 0x0000000100003fad (lldb) break command add 1 -o "script print(\"Hey!\")" (lldb) r Process 76041 launched: '/tmp/main.out' (x86_64) Hey! (lldb) script print("Hey!") Hey! Process 76041 stopped The issue is caused by HandleCommands using a temporary CommandReturnObject and one of the commands (`script` in this case) setting an immediate output stream. This causes the result to be printed twice: once directly to the immediate output stream and once when printing the result of HandleCommands. This patch fixes the issue by introducing a new option to suppress immediate output for temporary CommandReturnObjects. Differential revision: https://reviews.llvm.org/D103349 --- .../lldb/Interpreter/CommandReturnObject.h | 23 +++++++++++++++---- .../Shell/Breakpoint/breakpoint-command.test | 5 ++++ .../source/Interpreter/CommandInterpreter.cpp | 1 + .../Interpreter/CommandReturnObject.cpp | 13 ++++++++--- .../Shell/Breakpoint/breakpoint-command.test | 5 ++++ .../Lua/nested_sessions.test | 1 - 6 files changed, 40 insertions(+), 8 deletions(-) create mode 100644 lldb/lldb/test/Shell/Breakpoint/breakpoint-command.test create mode 100644 lldb/test/Shell/Breakpoint/breakpoint-command.test diff --git a/lldb/include/lldb/Interpreter/CommandReturnObject.h b/lldb/include/lldb/Interpreter/CommandReturnObject.h index c638b4bc70b2..e2b237c6d0f1 100644 --- a/lldb/include/lldb/Interpreter/CommandReturnObject.h +++ b/lldb/include/lldb/Interpreter/CommandReturnObject.h @@ -63,20 +63,28 @@ public: } void SetImmediateOutputFile(lldb::FileSP file_sp) { + if (m_suppress_immediate_output) + return; lldb::StreamSP stream_sp(new StreamFile(file_sp)); m_out_stream.SetStreamAtIndex(eImmediateStreamIndex, stream_sp); } void SetImmediateErrorFile(lldb::FileSP file_sp) { + if (m_suppress_immediate_output) + return; lldb::StreamSP stream_sp(new StreamFile(file_sp)); m_err_stream.SetStreamAtIndex(eImmediateStreamIndex, stream_sp); } void SetImmediateOutputStream(const lldb::StreamSP &stream_sp) { + if (m_suppress_immediate_output) + return; m_out_stream.SetStreamAtIndex(eImmediateStreamIndex, stream_sp); } void SetImmediateErrorStream(const lldb::StreamSP &stream_sp) { + if (m_suppress_immediate_output) + return; m_err_stream.SetStreamAtIndex(eImmediateStreamIndex, stream_sp); } @@ -142,16 +150,23 @@ public: void SetInteractive(bool b); + bool GetSuppressImmediateOutput() const; + + void SetSuppressImmediateOutput(bool b); + private: enum { eStreamStringIndex = 0, eImmediateStreamIndex = 1 }; StreamTee m_out_stream; StreamTee m_err_stream; - lldb::ReturnStatus m_status; - bool m_did_change_process_state; - bool m_interactive; // If true, then the input handle from the debugger will - // be hooked up + lldb::ReturnStatus m_status = lldb::eReturnStatusStarted; + + bool m_did_change_process_state = false; + bool m_suppress_immediate_output = false; + + /// If true, then the input handle from the debugger will be hooked up. + bool m_interactive = true; }; } // namespace lldb_private diff --git a/lldb/lldb/test/Shell/Breakpoint/breakpoint-command.test b/lldb/lldb/test/Shell/Breakpoint/breakpoint-command.test new file mode 100644 index 000000000000..6104713cde5a --- /dev/null +++ b/lldb/lldb/test/Shell/Breakpoint/breakpoint-command.test @@ -0,0 +1,5 @@ +# RUN: %build %p/Inputs/dummy-target.c -o %t.out +# RUN: %lldb %t.out -o 'b main' -o 'break command add 1 -o "script print(95000 + 126)"' -o 'r' + +# CHECK: 95125 +# CHECK-NOT: 95126 diff --git a/lldb/source/Interpreter/CommandInterpreter.cpp b/lldb/source/Interpreter/CommandInterpreter.cpp index 75c26c42e18d..85025b92d367 100644 --- a/lldb/source/Interpreter/CommandInterpreter.cpp +++ b/lldb/source/Interpreter/CommandInterpreter.cpp @@ -2306,6 +2306,7 @@ void CommandInterpreter::HandleCommands(const StringList &commands, CommandReturnObject tmp_result(m_debugger.GetUseColor()); tmp_result.SetInteractive(result.GetInteractive()); + tmp_result.SetSuppressImmediateOutput(true); // We might call into a regex or alias command, in which case the // add_to_history will get lost. This m_command_source_depth dingus is the diff --git a/lldb/source/Interpreter/CommandReturnObject.cpp b/lldb/source/Interpreter/CommandReturnObject.cpp index 980d39bfb168..531c1f246bd8 100644 --- a/lldb/source/Interpreter/CommandReturnObject.cpp +++ b/lldb/source/Interpreter/CommandReturnObject.cpp @@ -41,9 +41,7 @@ static void DumpStringToStreamWithNewline(Stream &strm, const std::string &s) { } CommandReturnObject::CommandReturnObject(bool colors) - : m_out_stream(colors), m_err_stream(colors), - m_status(eReturnStatusStarted), m_did_change_process_state(false), - m_interactive(true) {} + : m_out_stream(colors), m_err_stream(colors) {} void CommandReturnObject::AppendErrorWithFormat(const char *format, ...) { SetStatus(eReturnStatusFailed); @@ -154,6 +152,7 @@ void CommandReturnObject::Clear() { static_cast(stream_sp.get())->Clear(); m_status = eReturnStatusStarted; m_did_change_process_state = false; + m_suppress_immediate_output = false; m_interactive = true; } @@ -168,3 +167,11 @@ void CommandReturnObject::SetDidChangeProcessState(bool b) { bool CommandReturnObject::GetInteractive() const { return m_interactive; } void CommandReturnObject::SetInteractive(bool b) { m_interactive = b; } + +bool CommandReturnObject::GetSuppressImmediateOutput() const { + return m_suppress_immediate_output; +} + +void CommandReturnObject::SetSuppressImmediateOutput(bool b) { + m_suppress_immediate_output = b; +} diff --git a/lldb/test/Shell/Breakpoint/breakpoint-command.test b/lldb/test/Shell/Breakpoint/breakpoint-command.test new file mode 100644 index 000000000000..6104713cde5a --- /dev/null +++ b/lldb/test/Shell/Breakpoint/breakpoint-command.test @@ -0,0 +1,5 @@ +# RUN: %build %p/Inputs/dummy-target.c -o %t.out +# RUN: %lldb %t.out -o 'b main' -o 'break command add 1 -o "script print(95000 + 126)"' -o 'r' + +# CHECK: 95125 +# CHECK-NOT: 95126 diff --git a/lldb/test/Shell/ScriptInterpreter/Lua/nested_sessions.test b/lldb/test/Shell/ScriptInterpreter/Lua/nested_sessions.test index a81418b6af61..87305654b2fa 100644 --- a/lldb/test/Shell/ScriptInterpreter/Lua/nested_sessions.test +++ b/lldb/test/Shell/ScriptInterpreter/Lua/nested_sessions.test @@ -7,6 +7,5 @@ # CHECK-NEXT: foo foo # CHECK-NEXT: foo bar # CHECK-NEXT: foo bar -# CHECK-NEXT: foo bar # CHECK: script # CHECK-NEXT: bar bar