Merge pull request #3533 from Teemperor/cherry/cef1e07cc6d00b5b429d77133201e1f404a8023c

[lldb] Fix that the embedded Python REPL crashes if it receives SIGINT

Author: JDevlieghere

lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp

@@ -1050,6 +1050,23 @@ void ScriptInterpreterPythonImpl::ExecuteInterpreterLoop() {
 }
 
 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()) {
@@ -1071,6 +1088,7 @@ bool ScriptInterpreterPythonImpl::Interrupt() {
             "ScriptInterpreterPythonImpl::Interrupt() python code not running, "
             "can't interrupt");
   return false;
+#endif
 }
 
 bool ScriptInterpreterPythonImpl::ExecuteOneLineWithReturn(
@@ -3276,6 +3294,28 @@ ScriptInterpreterPythonImpl::AcquireInterpreterLock() {
   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;
@@ -3311,6 +3351,25 @@ void ScriptInterpreterPythonImpl::InitializePrivate() {
                      "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/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()