This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGcef1e07cc6d0: [lldb] Fix that the embedded Python REPL
crashes if it receives SIGINT (authored by teemperor).
Herald added a subscriber: lldb-commits.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D104886/new/
https://reviews.llvm.org/D104886
Files:
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
lldb/test/API/iohandler/sigint/TestIOHandlerPythonREPLSigint.py
Index: lldb/test/API/iohandler/sigint/TestIOHandlerPythonREPLSigint.py
===================================================================
--- /dev/null
+++ lldb/test/API/iohandler/sigint/TestIOHandlerPythonREPLSigint.py
@@ -0,0 +1,73 @@
+"""
+Test sending SIGINT to the embedded Python REPL.
+"""
+
+import os
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.lldbpexpect import PExpectTest
+
+class TestCase(PExpectTest):
+
+ mydir = TestBase.compute_mydir(__file__)
+
+ def start_python_repl(self):
+ """ Starts up the embedded Python REPL."""
+ self.launch()
+ # Start the embedded Python REPL via the 'script' command.
+ self.child.send("script -l python --\n")
+ # Wait for the Python REPL prompt.
+ self.child.expect(">>>")
+
+ # PExpect uses many timeouts internally and doesn't play well
+ # under ASAN on a loaded machine..
+ @skipIfAsan
+ def test_while_evaluating_code(self):
+ """ Tests SIGINT handling while Python code is being evaluated."""
+ self.start_python_repl()
+
+ # Start a long-running command that we try to abort with SIGINT.
+ # Note that we dont actually wait 10000s in this code as pexpect or
+ # lit will kill the test way before that.
+ self.child.send("import time; print('running' + 'now'); time.sleep(10000);\n")
+
+ # Make sure the command is actually being evaluated at the moment by
+ # looking at the string that the command is printing.
+ # Don't check for a needle that also occurs in the program itself to
+ # prevent that echoing will make this check pass unintentionally.
+ self.child.expect("runningnow")
+
+ # Send SIGINT to the LLDB process.
+ self.child.sendintr()
+
+ # This should get transformed to a KeyboardInterrupt which is the same
+ # behaviour as the standalone Python REPL. It should also interrupt
+ # the evaluation of our sleep statement.
+ self.child.expect("KeyboardInterrupt")
+ # Send EOF to quit the Python REPL.
+ self.child.sendeof()
+
+ self.quit()
+
+ # PExpect uses many timeouts internally and doesn't play well
+ # under ASAN on a loaded machine..
+ @skipIfAsan
+ # FIXME: On Linux the Python code that reads from stdin seems to block until
+ # it has finished reading a line before handling any queued signals.
+ @skipIfLinux
+ def test_while_waiting_on_input(self):
+ """ Tests SIGINT handling while the REPL is waiting on input from
+ stdin."""
+ self.start_python_repl()
+
+ # Send SIGINT to the LLDB process.
+ self.child.sendintr()
+ # This should get transformed to a KeyboardInterrupt which is the same
+ # behaviour as the standalone Python REPL.
+ self.child.expect("KeyboardInterrupt")
+ # Send EOF to quit the Python REPL.
+ self.child.sendeof()
+
+ self.quit()
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===================================================================
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -1067,6 +1067,23 @@
}
bool ScriptInterpreterPythonImpl::Interrupt() {
+ // PyErr_SetInterrupt was introduced in 3.2.
+#if (PY_MAJOR_VERSION == 3 && PY_MINOR_VERSION >= 2) || (PY_MAJOR_VERSION > 3)
+ // If the interpreter isn't evaluating any Python at the moment then return
+ // false to signal that this function didn't handle the interrupt and the
+ // next component should try handling it.
+ if (!IsExecutingPython())
+ return false;
+
+ // Tell Python that it should pretend to have received a SIGINT.
+ PyErr_SetInterrupt();
+ // PyErr_SetInterrupt has no way to return an error so we can only pretend the
+ // signal got successfully handled and return true.
+ // Python 3.10 introduces PyErr_SetInterruptEx that could return an error, but
+ // the error handling is limited to checking the arguments which would be
+ // just our (hardcoded) input signal code SIGINT, so that's not useful at all.
+ return true;
+#else
Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_SCRIPT));
if (IsExecutingPython()) {
@@ -1088,6 +1105,7 @@
"ScriptInterpreterPythonImpl::Interrupt() python code not running, "
"can't interrupt");
return false;
+#endif
}
bool ScriptInterpreterPythonImpl::ExecuteOneLineWithReturn(
@@ -3293,6 +3311,28 @@
return py_lock;
}
+namespace {
+/// Saves the current signal handler for the specified signal and restores
+/// it at the end of the current scope.
+struct RestoreSignalHandlerScope {
+ /// The signal handler.
+ struct sigaction m_prev_handler;
+ int m_signal_code;
+ RestoreSignalHandlerScope(int signal_code) : m_signal_code(signal_code) {
+ // Initialize sigaction to their default state.
+ std::memset(&m_prev_handler, 0, sizeof(m_prev_handler));
+ // Don't install a new handler, just read back the old one.
+ struct sigaction *new_handler = nullptr;
+ int signal_err = ::sigaction(m_signal_code, new_handler, &m_prev_handler);
+ lldbassert(signal_err == 0 && "sigaction failed to read handler");
+ }
+ ~RestoreSignalHandlerScope() {
+ int signal_err = ::sigaction(m_signal_code, &m_prev_handler, nullptr);
+ lldbassert(signal_err == 0 && "sigaction failed to restore old handler");
+ }
+};
+} // namespace
+
void ScriptInterpreterPythonImpl::InitializePrivate() {
if (g_initialized)
return;
@@ -3328,6 +3368,25 @@
"lldb.embedded_interpreter; from "
"lldb.embedded_interpreter import run_python_interpreter; "
"from lldb.embedded_interpreter import run_one_line");
+
+#if (PY_MAJOR_VERSION == 3 && PY_MINOR_VERSION >= 2) || (PY_MAJOR_VERSION > 3)
+ // Python will not just overwrite its internal SIGINT handler but also the
+ // one from the process. Backup the current SIGINT handler to prevent that
+ // Python deletes it.
+ RestoreSignalHandlerScope save_sigint(SIGINT);
+
+ // Setup a default SIGINT signal handler that works the same way as the
+ // normal Python REPL signal handler which raises a KeyboardInterrupt.
+ // Also make sure to not pollute the user's REPL with the signal module nor
+ // our utility function.
+ PyRun_SimpleString("def lldb_setup_sigint_handler():\n"
+ " import signal;\n"
+ " def signal_handler(sig, frame):\n"
+ " raise KeyboardInterrupt()\n"
+ " signal.signal(signal.SIGINT, signal_handler);\n"
+ "lldb_setup_sigint_handler();\n"
+ "del lldb_setup_sigint_handler\n");
+#endif
}
void ScriptInterpreterPythonImpl::AddToSysPath(AddLocation location,
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
