[lldb/Interpreter] Propagate script output back to command return object
#109440
Conversation
…bject When running a oneliner script expression, if the script interpreter returned a value, that value would be printed to the debugger standard output straight from the interpreter instead of being propagated back to the command return object, which would then forward it to its output stream. This implies that when evaluating a oneliner script expression (with `SBCommandInterpreter::HandleCommand`), the return value would get printed to stdout, but we would not be able to fetch it from the command return object. This patch solves this issue by extending the default Python `InteractiveConsole` class to keep track of the return value, before include it to the command return object. rdar://132420488 Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>
|
@llvm/pr-subscribers-lldb Author: Med Ismail Bennani (medismailben) ChangesWhen running a oneliner script expression, if the script interpreter returned a value, that value would be printed to the debugger standard output straight from the interpreter instead of being propagated back to the command return object, which would then forward it to its output stream. This implies that when evaluating a oneliner script expression (with This patch solves this issue by extending the default Python rdar://132420488 Full diff: https://github.com/llvm/llvm-project/pull/109440.diff 2 Files Affected:
diff --git a/lldb/source/Interpreter/embedded_interpreter.py b/lldb/source/Interpreter/embedded_interpreter.py
index a487592ef1aee5..fd5c44d0121fbd 100644
--- a/lldb/source/Interpreter/embedded_interpreter.py
+++ b/lldb/source/Interpreter/embedded_interpreter.py
@@ -8,6 +8,8 @@
import lldb
import traceback
+from io import StringIO
+
try:
import readline
import rlcompleter
@@ -116,19 +118,50 @@ def run_python_interpreter(local_dict):
print("Script exited with code %s" % e.code)
+class LLDBInteractiveConsole(code.InteractiveConsole):
+ def __init__(self, locals=None):
+ super().__init__(locals)
+ self.result_output = None
+
+ ### Implementation detail:
+ ### https://docs.python.org/3/library/code.html#code.InteractiveInterpreter.runsource
+ def runsource(self, source, filename="<input>", symbol="single"):
+ # Redirect stdout to capture print statements
+ old_stdout = sys.stdout
+ sys.stdout = result_output = StringIO()
+
+ try:
+ compiled_code = self.compile(source, filename, symbol)
+ if compiled_code is None:
+ return False
+
+ exec(compiled_code, self.locals)
+ return True
+ except Exception as e:
+ self.showsyntaxerror(filename)
+ return False
+ finally:
+ self.result_output = result_output
+ sys.stdout = old_stdout
+
+ def get_last_result(self):
+ return self.result_output.getvalue()
+
def run_one_line(local_dict, input_string):
global g_run_one_line_str
try:
input_string = strip_and_check_exit(input_string)
- repl = code.InteractiveConsole(local_dict)
+ repl = LLDBInteractiveConsole(local_dict)
if input_string:
# A newline is appended to support one-line statements containing
# control flow. For example "if True: print(1)" silently does
# nothing, but works with a newline: "if True: print(1)\n".
input_string += "\n"
- repl.runsource(input_string)
+ if repl.runsource(input_string):
+ return repl.get_last_result()
elif g_run_one_line_str:
- repl.runsource(g_run_one_line_str)
+ if repl.runsource(g_run_one_line_str):
+ return repl.get_last_result()
except LLDBExit:
pass
except SystemExit as e:
diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
index 63691d24f0dadb..30b67ce48a4be9 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -884,9 +884,12 @@ bool ScriptInterpreterPythonImpl::ExecuteOneLine(
PyRefType::Owned,
PyObject_CallObject(m_run_one_line_function.get(),
pargs.get()));
- if (return_value.IsValid())
+ if (return_value.IsValid()) {
success = true;
- else if (options.GetMaskoutErrors() && PyErr_Occurred()) {
+ PythonString repr = return_value.Repr();
+ if (repr && repr.GetSize())
+ result->AppendMessage(repr.GetString());
+ } else if (options.GetMaskoutErrors() && PyErr_Occurred()) {
PyErr_Print();
PyErr_Clear();
}
|
| ### Implementation detail: | ||
| ### https://docs.python.org/3/library/code.html#code.InteractiveInterpreter.runsource |
There was a problem hiding this comment.
Nit: the link isn't the implementation. You can probably just omit line 126.
| ### Implementation detail: | ||
| ### https://docs.python.org/3/library/code.html#code.InteractiveInterpreter.runsource | ||
| def runsource(self, source, filename="<input>", symbol="single"): | ||
| # Redirect stdout to capture print statements |
|
|
||
| ### Implementation detail: | ||
| ### https://docs.python.org/3/library/code.html#code.InteractiveInterpreter.runsource | ||
| def runsource(self, source, filename="<input>", symbol="single"): |
There was a problem hiding this comment.
If you don't need any of the arguments, can you use **kwargs and forward everything so that we don't need to maintain our own default values?
|
Now that successful results show up in the command return object, it's a little strange that if the expression fails - a parse error or whatever - that is not available in the command result. It would be more consistent if we did SetError on the command result object with the error. It looks like you could capture that and forward it to where the result is made up. |
| old_stdout = sys.stdout | ||
| sys.stdout = result_output = StringIO() |
There was a problem hiding this comment.
Apparently, there's even a standard utility to do this, but it comes with a big warning (which I fully agree with):
Note that the global side effect on [sys.stdout](https://docs.python.org/3/library/sys.html#sys.stdout) means that this context manager is not suitable for use in library code and most threaded applications.
We tick both of those boxes. Is this redirection really necessary? Does anything prevent two instances of this function from running in parallel (e.g. on two debugger objects). If not, we're just setting ourself for a nasty race where the stdout just suddenly goes dead (because it's been permanently redirected to StringIO). (Note that even if we can prevent races between in our own code, we can't prevent racing with anyone else attempting to do the same thing, nor we can stop inadvertently capturing the output of other threads).
Capturing the result of the command makes sense to me, but capturing stdout, seems like its going too far. Maybe we could provide the StringIO object to the executed code as some sort of an argument, so that if the user really wants print there it can do something like print(..., file=cmd_output) ?
When running a oneliner script expression, if the script interpreter returned a value, that value would be printed to the debugger standard output straight from the interpreter instead of being propagated back to the command return object, which would then forward it to its output stream.
This implies that when evaluating a oneliner script expression (with
SBCommandInterpreter::HandleCommand), the return value would get printed to stdout, but we would not be able to fetch it from the command return object.This patch solves this issue by extending the default Python
InteractiveConsoleclass to keep track of the return value, before include it to the command return object.rdar://132420488