JDevlieghere created this revision.
JDevlieghere added reviewers: teemperor, labath, mib, jingham.
Herald added a subscriber: pengfei.
JDevlieghere requested review of this revision.
When executing a `script` command in `HandleCommand(s)` we currently print its
output twice: once directly to the debugger's output stream, and once as part
of `HandleCommand`'s result. 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
* thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 1.1
frame #0: 0x0000000100003fad main.out`main at main.cpp:2:3
The issue is that `HandleCommands` uses a temporary `CommandReturnObject`. When
running a one-liner with `script`, the script interpreter redirects the I/O
from to the command object through `ScriptInterpreterIORedirect` which sets the
immediate output on the temporary result and causes the result to be printed
the first time. HandleCommand will then copy the result's output into its own
result and print it a second time, not knowing that it has already been printed.
I'm not entirely sure why `ScriptInterpreterIORedirect` relies on an immediate
output file, but there are several parts of LLDB that rely on it, so I'm
hesitant to change that. We could check in `HandleCommands` if someone set
immediate mode on our temporary result and not copy it into the final result
when that's the case. A better solution in my opinion, which I implemented in
this patch, is a "hermetic" mode for the `CommandReturnObject` which prevents
anyone from setting and immediate stream on the temporary result by the command
interpreter. This should prevent similar bugs if there are other places that
try to do this.
I added a new test using the breakpoint command and fixed a Lua test that
already suffered from this issue.
https://reviews.llvm.org/D103349
Files:
lldb/include/lldb/Interpreter/CommandReturnObject.h
lldb/source/Interpreter/CommandInterpreter.cpp
lldb/source/Interpreter/CommandReturnObject.cpp
lldb/test/Shell/Breakpoint/breakpoint-command.test
lldb/test/Shell/ScriptInterpreter/Lua/nested_sessions.test
Index: lldb/test/Shell/ScriptInterpreter/Lua/nested_sessions.test
===================================================================
--- lldb/test/Shell/ScriptInterpreter/Lua/nested_sessions.test
+++ 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
Index: lldb/test/Shell/Breakpoint/breakpoint-command.test
===================================================================
--- /dev/null
+++ 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
Index: lldb/source/Interpreter/CommandReturnObject.cpp
===================================================================
--- lldb/source/Interpreter/CommandReturnObject.cpp
+++ lldb/source/Interpreter/CommandReturnObject.cpp
@@ -43,7 +43,7 @@
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_interactive(true), m_hermetic(false) {}
void CommandReturnObject::AppendErrorWithFormat(const char *format, ...) {
if (!format)
@@ -152,6 +152,7 @@
m_status = eReturnStatusStarted;
m_did_change_process_state = false;
m_interactive = true;
+ m_hermetic = false;
}
bool CommandReturnObject::GetDidChangeProcessState() {
@@ -165,3 +166,7 @@
bool CommandReturnObject::GetInteractive() const { return m_interactive; }
void CommandReturnObject::SetInteractive(bool b) { m_interactive = b; }
+
+bool CommandReturnObject::GetHermetic() const { return m_hermetic; }
+
+void CommandReturnObject::SetHermetic(bool b) { m_hermetic = b; }
Index: lldb/source/Interpreter/CommandInterpreter.cpp
===================================================================
--- lldb/source/Interpreter/CommandInterpreter.cpp
+++ lldb/source/Interpreter/CommandInterpreter.cpp
@@ -2306,6 +2306,7 @@
CommandReturnObject tmp_result(m_debugger.GetUseColor());
tmp_result.SetInteractive(result.GetInteractive());
+ tmp_result.SetHermetic(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
Index: lldb/include/lldb/Interpreter/CommandReturnObject.h
===================================================================
--- lldb/include/lldb/Interpreter/CommandReturnObject.h
+++ lldb/include/lldb/Interpreter/CommandReturnObject.h
@@ -63,20 +63,28 @@
}
void SetImmediateOutputFile(lldb::FileSP file_sp) {
+ if (m_hermetic)
+ return;
lldb::StreamSP stream_sp(new StreamFile(file_sp));
m_out_stream.SetStreamAtIndex(eImmediateStreamIndex, stream_sp);
}
void SetImmediateErrorFile(lldb::FileSP file_sp) {
+ if (m_hermetic)
+ 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_hermetic)
+ return;
m_out_stream.SetStreamAtIndex(eImmediateStreamIndex, stream_sp);
}
void SetImmediateErrorStream(const lldb::StreamSP &stream_sp) {
+ if (m_hermetic)
+ return;
m_err_stream.SetStreamAtIndex(eImmediateStreamIndex, stream_sp);
}
@@ -142,6 +150,10 @@
void SetInteractive(bool b);
+ bool GetHermetic() const;
+
+ void SetHermetic(bool b);
+
private:
enum { eStreamStringIndex = 0, eImmediateStreamIndex = 1 };
@@ -150,8 +162,11 @@
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
+ /// If true, then the input handle from the debugger will be hooked up.
+ bool m_interactive;
+ /// If true, disallow immediate streams. Useful when using this return object
+ /// as a temporary.
+ bool m_hermetic;
};
} // namespace lldb_private
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
