llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Jonas Devlieghere (JDevlieghere) <details> <summary>Changes</summary> The driver's async SIGINT handler called SBDebugger::DispatchInputInterrupt directly. That is not async-signal-safe and can lead to a crash. Register SIGINT with the existing signal-thread MainLoop instead so DispatchInputInterrupt runs in normal thread context. The Windows path is unchanged and keeps the legacy async handler. While DispatchInputInterrupt runs, the callback temporarily installs SIG_DFL so a second Ctrl-C still hard-terminates the process, preserving the escape hatch users rely on when the debugger is unresponsive. Moving SIGINT off the main thread means a Ctrl-C no longer interrupts blocking syscalls there (e.g. a Python REPL waiting on input or sleeping), so Python never observes the queued interrupt and KeyboardInterrupt is not raised. To restore that behavior, after dispatching the interrupt the callback re-raises SIGINT on the main thread via pthread_kill; the resulting EINTR lets Python pick up the pending interrupt. A skip flag suppresses the re-entry that this self-send produces. Because the callback only ever runs on the signal thread, the flag and the captured main-thread id live in the lambda's captures and need no synchronization. rdar://158218595 --- Full diff: https://github.com/llvm/llvm-project/pull/196687.diff 1 Files Affected: - (modified) lldb/tools/driver/Driver.cpp (+56-4) ``````````diff diff --git a/lldb/tools/driver/Driver.cpp b/lldb/tools/driver/Driver.cpp index d47d3daf1c3fc..e58286f9ff41e 100644 --- a/lldb/tools/driver/Driver.cpp +++ b/lldb/tools/driver/Driver.cpp @@ -43,6 +43,9 @@ #include <clocale> #include <csignal> #include <future> +#ifndef _WIN32 +#include <pthread.h> +#endif #include <string> #include <thread> #include <utility> @@ -651,11 +654,10 @@ void Driver::UpdateWindowSize() { } } -void sigint_handler(int signo) { #ifdef _WIN32 +void sigint_handler(int signo) { // Restore handler as it is not persistent on Windows. signal(SIGINT, sigint_handler); -#endif static std::atomic_flag g_interrupt_sent = ATOMIC_FLAG_INIT; if (g_driver != nullptr) { @@ -668,6 +670,7 @@ void sigint_handler(int signo) { _exit(signo); } +#endif static void printHelp(LLDBOptTable &table, llvm::StringRef tool_name) { std::string usage_str = tool_name.str() + " [options]"; @@ -781,15 +784,64 @@ int main(int argc, char const *argv[]) { // Setup LLDB signal handlers once the debugger has been initialized. SBDebugger::PrintDiagnosticsOnError(); - // FIXME: Migrate the SIGINT handler to be handled by the signal loop below. +#ifdef _WIN32 signal(SIGINT, sigint_handler); -#if !defined(_WIN32) +#else signal(SIGPIPE, SIG_IGN); + // Capture the main thread's id so the signal thread can target it. + pthread_t main_thread = pthread_self(); + + // Set when the signal thread sends itself a SIGINT to wake the main thread. + // The next callback invocation observes this flag and skips the work. A + // plain bool is sufficient because the callback only ever runs on the + // signal thread; it lives outside the lambda because MainLoopPosix copies + // the callback on every dispatch, which would discard in-lambda state. + bool skip_next_sigint = false; + // Handle signals in a MainLoop running on a separate thread. MainLoop signal_loop; Status signal_status; + auto sigint_handler = signal_loop.RegisterSignal( + SIGINT, + [&, main_thread](MainLoopBase &) { + // Skip the self-sent wakeup SIGINT queued at the end of the previous + // invocation. + if (std::exchange(skip_next_sigint, false)) + return; + + // Temporarily restore the default disposition so that a second SIGINT + // delivered while DispatchInputInterrupt is running hard-terminates + // the process. This preserves the "double Ctrl-C to force exit" + // escape hatch users rely on when the debugger is unresponsive. + struct sigaction old_action; + struct sigaction new_action = {}; + new_action.sa_handler = SIG_DFL; + sigemptyset(&new_action.sa_mask); + + int ret = sigaction(SIGINT, &new_action, &old_action); + UNUSED_IF_ASSERT_DISABLED(ret); + assert(ret == 0 && "sigaction failed"); + + if (g_driver) + g_driver->GetDebugger().DispatchInputInterrupt(); + + ret = sigaction(SIGINT, &old_action, nullptr); + UNUSED_IF_ASSERT_DISABLED(ret); + assert(ret == 0 && "sigaction failed"); + + // Wake the main thread so any blocking syscall (e.g. the Python REPL + // waiting on input or sleeping) returns with EINTR. This lets Python + // observe the pending interrupt queued by DispatchInputInterrupt and + // raise KeyboardInterrupt. Flag the resulting callback invocation so + // it's skipped rather than re-running DispatchInputInterrupt. + skip_next_sigint = true; + pthread_kill(main_thread, SIGINT); + }, + signal_status); + assert(sigint_handler && signal_status.Success()); + auto sigwinch_handler = signal_loop.RegisterSignal( SIGWINCH, [&](MainLoopBase &) { `````````` </details> https://github.com/llvm/llvm-project/pull/196687 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
