[lldb] Fix typed commands not shown on the screen#174216
Conversation
The issue is that in python3.14, `fcntl.ioctl` now throws a buffer overflow error when the buffer is too small (see python/cpython#132919). This caused the Python interpreter to fail terminal detection and not properly echo user commands back to the screen. Fix by dropping the custom terminal size check entirely and using the built-in `sys.stdin.isatty()` instead. Fixes llvm#173302
|
@llvm/pr-subscribers-lldb Author: Ebuka Ezike (da-viper) ChangesThe cause is that in Fix by dropping the custom terminal size check entirely and using the built-in Fixes #173302 Full diff: https://github.com/llvm/llvm-project/pull/174216.diff 3 Files Affected:
diff --git a/lldb/packages/Python/lldbsuite/test/lldbpexpect.py b/lldb/packages/Python/lldbsuite/test/lldbpexpect.py
index 3279e1fd39f8c..03b2500fbda52 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbpexpect.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbpexpect.py
@@ -10,6 +10,7 @@
@skipIfRemote
+@skipIfWindows
@add_test_categories(["pexpect"])
class PExpectTest(TestBase):
NO_DEBUG_INFO_TESTCASE = True
diff --git a/lldb/source/Interpreter/embedded_interpreter.py b/lldb/source/Interpreter/embedded_interpreter.py
index 42a9ab5fc367a..5eb582e131871 100644
--- a/lldb/source/Interpreter/embedded_interpreter.py
+++ b/lldb/source/Interpreter/embedded_interpreter.py
@@ -1,8 +1,6 @@
import sys
import builtins
import code
-import lldb
-import traceback
try:
import readline
@@ -31,19 +29,6 @@ def is_libedit():
g_run_one_line_str = None
-
-def get_terminal_size(fd):
- try:
- import fcntl
- import termios
- import struct
-
- hw = struct.unpack("hh", fcntl.ioctl(fd, termios.TIOCGWINSZ, "1234"))
- except:
- hw = (0, 0)
- return hw
-
-
class LLDBExit(SystemExit):
pass
@@ -74,50 +59,21 @@ def readfunc_stdio(prompt):
def run_python_interpreter(local_dict):
# Pass in the dictionary, for continuity from one session to the next.
try:
- fd = sys.stdin.fileno()
- interacted = False
- if get_terminal_size(fd)[1] == 0:
- try:
- import termios
-
- old = termios.tcgetattr(fd)
- if old[3] & termios.ECHO:
- # Need to turn off echoing and restore
- new = termios.tcgetattr(fd)
- new[3] = new[3] & ~termios.ECHO
- try:
- termios.tcsetattr(fd, termios.TCSADRAIN, new)
- interacted = True
- code.interact(
- banner="Python Interactive Interpreter. To exit, type 'quit()', 'exit()'.",
- readfunc=readfunc_stdio,
- local=local_dict,
- )
- finally:
- termios.tcsetattr(fd, termios.TCSADRAIN, old)
- except:
- pass
- # Don't need to turn off echoing
- if not interacted:
- code.interact(
- banner="Python Interactive Interpreter. To exit, type 'quit()', 'exit()' or Ctrl-D.",
- readfunc=readfunc_stdio,
- local=local_dict,
- )
- else:
- # We have a real interactive terminal
- code.interact(
- banner="Python Interactive Interpreter. To exit, type 'quit()', 'exit()' or Ctrl-D.",
- readfunc=readfunc,
- local=local_dict,
- )
+ banner = "Python Interactive Interpreter. To exit, type 'quit()', 'exit()'."
+ input_func = readfunc_stdio
+
+ is_atty = sys.stdin.isatty()
+ if is_atty:
+ banner = "Python Interactive Interpreter. To exit, type 'quit()', 'exit()' or Ctrl-D."
+ input_func = readfunc
+
+ code.interact(banner=banner, readfunc=input_func, local=local_dict)
except LLDBExit:
pass
except SystemExit as e:
if e.code:
print("Script exited with code %s" % e.code)
-
def run_one_line(local_dict, input_string):
global g_run_one_line_str
try:
diff --git a/lldb/test/API/terminal/TestPythonInterpreterEcho.py b/lldb/test/API/terminal/TestPythonInterpreterEcho.py
new file mode 100644
index 0000000000000..afa8bca2c9be8
--- /dev/null
+++ b/lldb/test/API/terminal/TestPythonInterpreterEcho.py
@@ -0,0 +1,62 @@
+"""
+Test that typing python expression in the terminal is echoed back to stdout.
+"""
+
+from lldbsuite.test.decorators import skipIfAsan
+from lldbsuite.test.lldbpexpect import PExpectTest
+
+
+@skipIfAsan
+class PythonInterpreterEchoTest(PExpectTest):
+ PYTHON_PROMPT = ">>> "
+
+ def verify_command_echo(
+ self, command: str, expected_output: str = "", is_regex: bool = False
+ ):
+ assert self.child != None
+ child = self.child
+ self.assertIsNotNone(self.child, "expected a running lldb process.")
+
+ child.sendline(command)
+
+ # Build pattern list: match whichever comes first (output or prompt)
+ # This prevents waiting for a timeout if there's no match
+ pattern = []
+ match_expected = expected_output and len(expected_output) > 0
+
+ if match_expected:
+ pattern.append(expected_output)
+ pattern.append(self.PYTHON_PROMPT)
+
+ expect_func = child.expect if is_regex else child.expect_exact
+ match_idx = expect_func(pattern)
+ if match_expected:
+ self.assertEqual(
+ match_idx, 0, "Expected output `{expected_output}` in stdout."
+ )
+
+ self.assertIsNotNone(self.child.before, "Expected output before prompt")
+ self.assertIsInstance(self.child.before, bytes)
+ echoed_text: str = self.child.before.decode("ascii").strip()
+ self.assertEqual(
+ command, echoed_text, f"Command '{command}' should be echoed to stdout."
+ )
+
+ if match_expected:
+ child.expect_exact(self.PYTHON_PROMPT)
+
+ def test_python_interpreter_echo(self):
+ """Test that that the user typed commands is echoed to stdout"""
+
+ self.launch(use_colors=False, dimensions=(100, 100))
+
+ # enter the python interpreter
+ self.verify_command_echo(
+ "script --language python --", expected_output="Python.*\\.", is_regex=True
+ )
+ self.child_in_script_interpreter = True
+
+ self.verify_command_echo("val = 300")
+ self.verify_command_echo(
+ "print('result =', 300)", expected_output="result = 300"
+ )
|
medismailben
left a comment
There was a problem hiding this comment.
This seems fine but I'm unsure of how sys.stdin.isatty behaves on older python installs. @JDevlieghere might know better.
JDevlieghere
left a comment
There was a problem hiding this comment.
This looks like a good improvement (I see a lot of deleted lines!) but I agree with Ismail that we need to make sure this behaves correctly with all our supported Python versions.
The new PExpect test covers the case of using a TTY, but do we have existing tests that do the same when we're redirecting input and setting the input programmatically through the SB API?
9c15daf to
f5b00e3
Compare
I currently run test on the minimum version (3.8) and 3.14.
We already have tests in python for the one line command ( (lldb) script
Python Interactive Interpreter. To exit, type 'quit()', 'exit()' or Ctrl-D.
>>> print(hello)I have added two new basic test for both sending the input through the This does not cover testing the below scenarios because, it did not work prior to this commit on any python version. It greatly increases the scope of the PR and should be in a different commit:
test with version
script --language python --
print("first time")
exit
script --language python --
print("in here a second time")
exit
versionwrong output $ lldb < input
(lldb) version
lldb version 22.0.0git (git@github.com:da-viper/llvm-project.git revision f8bb85a52efef0b75503e7e3404476dcf5e97e25)
clang revision f8bb85a52efef0b75503e7e3404476dcf5e97e25
llvm revision f8bb85a52efef0b75503e7e3404476dcf5e97e25
(lldb) script --language python --
Python Interactive Interpreter. To exit, type 'quit()', 'exit()'.
>>> first time
>>> now exiting InteractiveConsole... |
|
The reason we did not catch this earlier is because we currently do not report python exceptions. when using |
JDevlieghere
left a comment
There was a problem hiding this comment.
LGTM. Python 3.9 is the other important release to test, which is covered by GreenDragon.
|
Note the buildbot lldb-x86_64-win is broken after this patch. Please take a look. FAIL: lldb-api::TestFileHandle.py FAIL: lldb-shell::io.test |
|
linked PR #175055 |
Please note #175055 (comment) |
There is no indication this ever worked on windows as this is the first test that checks python interactive console from a file. Looking at the error from the CI, It closed the interpreter before running any python commands. Will reconfirm this when I have access to a windows machine. From #174216
…75055) There is no indication this ever worked on windows as this is the first test that checks python interactive console from a file. Looking at the error from the CI, It closed the interpreter before running any python commands. Will reconfirm this when I have access to a windows machine. From llvm/llvm-project#174216
The cause is that in `python3.14`, `fcntl.ioctl` now throws a buffer overflow error when the buffer is too small or too large (see python/cpython#132919). This caused the Python interpreter to fail terminal detection and not properly echo user commands back to the screen. Fix by dropping the custom terminal size check entirely and using the built-in `sys.stdin.isatty()` instead. Fixes llvm#173302
There is no indication this ever worked on windows as this is the first test that checks python interactive console from a file. Looking at the error from the CI, It closed the interpreter before running any python commands. Will reconfirm this when I have access to a windows machine. From llvm#174216
There is no indication this ever worked on windows as this is the first test that checks python interactive console from a file. Looking at the error from the CI, It closed the interpreter before running any python commands. Will reconfirm this when I have access to a windows machine. From llvm/llvm-project#174216 (cherry picked from commit 21a1e6e)
The cause is that in `python3.14`, `fcntl.ioctl` now throws a buffer overflow error when the buffer is too small or too large (see python/cpython#132919). This caused the Python interpreter to fail terminal detection and not properly echo user commands back to the screen. Fix by dropping the custom terminal size check entirely and using the built-in `sys.stdin.isatty()` instead. Fixes llvm#173302
There is no indication this ever worked on windows as this is the first test that checks python interactive console from a file. Looking at the error from the CI, It closed the interpreter before running any python commands. Will reconfirm this when I have access to a windows machine. From llvm#174216
The cause is that in `python3.14`, `fcntl.ioctl` now throws a buffer overflow error when the buffer is too small or too large (see python/cpython#132919). This caused the Python interpreter to fail terminal detection and not properly echo user commands back to the screen. Fix by dropping the custom terminal size check entirely and using the built-in `sys.stdin.isatty()` instead. Fixes llvm#173302 (cherry picked from commit 43cb463)
There is no indication this ever worked on windows as this is the first test that checks python interactive console from a file. Looking at the error from the CI, It closed the interpreter before running any python commands. Will reconfirm this when I have access to a windows machine. From llvm#174216 (cherry picked from commit 21a1e6e)
[lldb] Fix typed commands not shown on the screen (llvm#174216)
The cause is that in
python3.14,fcntl.ioctlnow throws a buffer overflow errorwhen the buffer is too small or too large (see python/cpython#132919). This caused the Python interpreter to fail terminal detection and not properly echo user commands back to the screen.
Fix by dropping the custom terminal size check entirely and using the built-in
sys.stdin.isatty()instead.Fixes #173302