From patchwork Tue Oct 1 20:12:24 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tom Tromey X-Patchwork-Id: 34774 Received: (qmail 41663 invoked by alias); 1 Oct 2019 20:12:40 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 41600 invoked by uid 89); 1 Oct 2019 20:12:40 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-21.2 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_HELO_PASS autolearn=ham version=3.3.1 spammy=recovery, quit X-HELO: gateway22.websitewelcome.com Received: from gateway22.websitewelcome.com (HELO gateway22.websitewelcome.com) (192.185.47.168) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 01 Oct 2019 20:12:38 +0000 Received: from cm12.websitewelcome.com (cm12.websitewelcome.com [100.42.49.8]) by gateway22.websitewelcome.com (Postfix) with ESMTP id 47CDC51055 for ; Tue, 1 Oct 2019 15:12:36 -0500 (CDT) Received: from box5379.bluehost.com ([162.241.216.53]) by cmsmtp with SMTP id FOVgi8V3vW4frFOVgip3E8; Tue, 01 Oct 2019 15:12:36 -0500 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=tromey.com; s=default; h=References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From: Sender:Reply-To:MIME-Version:Content-Type:Content-Transfer-Encoding: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=SdNY+7bS5E4k2jGXZzeF2xqDHhIwdZILOco6A60Xzzk=; b=UGPR6HIe2qOTALMfnKeUp215P1 BMb6LMRbv5FmoPNuPdx+YEMFZZ+5q7CwHxi6RvP1lMzokyuCFxP0ZsDx35pVEDtiDlRdc4pJT7mqU TLY0Lq7tPoPTL3PD6UzwyCLhB; Received: from 75-166-72-156.hlrn.qwest.net ([75.166.72.156]:47310 helo=bapiya.Home) by box5379.bluehost.com with esmtpsa (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.92) (envelope-from ) id 1iFOVf-0023p5-W4; Tue, 01 Oct 2019 14:12:36 -0600 From: Tom Tromey To: gdb-patches@sourceware.org Cc: Tom Tromey Subject: [PATCH v4 08/11] Introduce thread-safe way to handle SIGSEGV Date: Tue, 1 Oct 2019 14:12:24 -0600 Message-Id: <20191001201227.8519-9-tom@tromey.com> In-Reply-To: <20191001201227.8519-1-tom@tromey.com> References: <20191001201227.8519-1-tom@tromey.com> The gdb demangler installs a SIGSEGV handler in order to protect gdb from demangler bugs. However, this is not thread-safe, as signal handlers are global to the process. This patch changes gdb to always install a global SIGSEGV handler, and then lets threads indicate their interest in handling the signal by setting a thread-local variable. This patch then arranges for the demangler code to use this; being sure to arrange for calls to warning and the like to be done on the main thread. One thing I wondered while writing this patch is if there are any systems that do not have "sigaction". If gdb could assume this, it would simplify this code. gdb/ChangeLog 2019-10-01 Tom Tromey * event-top.h (thread_local_segv_handler): Declare. * event-top.c (thread_local_segv_handler): New global. (install_handle_sigsegv, handle_sigsegv): New functions. (async_init_signals): Install SIGSEGV handler. * cp-support.c (gdb_demangle_jmp_buf): Change type. Now thread-local. (report_failed_demangle): New function. (gdb_demangle): Make core_dump_allowed atomic. Remove signal handler-setting code, instead use segv_handler. Run warning code on main thread. --- gdb/ChangeLog | 13 +++++ gdb/cp-support.c | 143 +++++++++++++++++++++++------------------------ gdb/event-top.c | 41 ++++++++++++++ gdb/event-top.h | 6 ++ 4 files changed, 130 insertions(+), 73 deletions(-) diff --git a/gdb/cp-support.c b/gdb/cp-support.c index cd732b60e7d..8fe9ea161f9 100644 --- a/gdb/cp-support.c +++ b/gdb/cp-support.c @@ -37,6 +37,9 @@ #include "gdbsupport/gdb_setjmp.h" #include "safe-ctype.h" #include "gdbsupport/selftest.h" +#include +#include "event-top.h" +#include "ser-event.h" #define d_left(dc) (dc)->u.s_binary.left #define d_right(dc) (dc)->u.s_binary.right @@ -1476,11 +1479,11 @@ static bool catch_demangler_crashes = true; /* Stack context and environment for demangler crash recovery. */ -static SIGJMP_BUF gdb_demangle_jmp_buf; +static thread_local SIGJMP_BUF *gdb_demangle_jmp_buf; /* If nonzero, attempt to dump core from the signal handler. */ -static int gdb_demangle_attempt_core_dump = 1; +static std::atomic gdb_demangle_attempt_core_dump; /* Signal handler for gdb_demangle. */ @@ -1492,10 +1495,46 @@ gdb_demangle_signal_handler (int signo) if (fork () == 0) dump_core (); - gdb_demangle_attempt_core_dump = 0; + gdb_demangle_attempt_core_dump = false; } - SIGLONGJMP (gdb_demangle_jmp_buf, signo); + SIGLONGJMP (*gdb_demangle_jmp_buf, signo); +} + +/* A helper for gdb_demangle that reports a demangling failure. */ + +static void +report_failed_demangle (const char *name, bool core_dump_allowed, + int crash_signal) +{ + static bool error_reported = false; + + if (!error_reported) + { + std::string short_msg + = string_printf (_("unable to demangle '%s' " + "(demangler failed with signal %d)"), + name, crash_signal); + + std::string long_msg + = string_printf ("%s:%d: %s: %s", __FILE__, __LINE__, + "demangler-warning", short_msg.c_str ()); + + target_terminal::scoped_restore_terminal_state term_state; + target_terminal::ours_for_output (); + + begin_line (); + if (core_dump_allowed) + fprintf_unfiltered (gdb_stderr, + _("%s\nAttempting to dump core.\n"), + long_msg.c_str ()); + else + warn_cant_dump_core (long_msg.c_str ()); + + demangler_warning (__FILE__, __LINE__, "%s", short_msg.c_str ()); + + error_reported = true; + } } #endif @@ -1509,38 +1548,18 @@ gdb_demangle (const char *name, int options) int crash_signal = 0; #ifdef HAVE_WORKING_FORK -#if defined (HAVE_SIGACTION) && defined (SA_RESTART) - struct sigaction sa, old_sa; -#else - sighandler_t ofunc; -#endif - static int core_dump_allowed = -1; - - if (core_dump_allowed == -1) - { - core_dump_allowed = can_dump_core (LIMIT_CUR); - - if (!core_dump_allowed) - gdb_demangle_attempt_core_dump = 0; - } - + scoped_restore restore_segv + = make_scoped_restore (&thread_local_segv_handler, + catch_demangler_crashes + ? gdb_demangle_signal_handler + : nullptr); + + bool core_dump_allowed = gdb_demangle_attempt_core_dump; + SIGJMP_BUF jmp_buf; + scoped_restore restore_jmp_buf + = make_scoped_restore (&gdb_demangle_jmp_buf, &jmp_buf); if (catch_demangler_crashes) - { -#if defined (HAVE_SIGACTION) && defined (SA_RESTART) - sa.sa_handler = gdb_demangle_signal_handler; - sigemptyset (&sa.sa_mask); -#ifdef HAVE_SIGALTSTACK - sa.sa_flags = SA_ONSTACK; -#else - sa.sa_flags = 0; -#endif - sigaction (SIGSEGV, &sa, &old_sa); -#else - ofunc = signal (SIGSEGV, gdb_demangle_signal_handler); -#endif - - crash_signal = SIGSETJMP (gdb_demangle_jmp_buf); - } + crash_signal = SIGSETJMP (jmp_buf); #endif if (crash_signal == 0) @@ -1549,45 +1568,20 @@ gdb_demangle (const char *name, int options) #ifdef HAVE_WORKING_FORK if (catch_demangler_crashes) { -#if defined (HAVE_SIGACTION) && defined (SA_RESTART) - sigaction (SIGSEGV, &old_sa, NULL); -#else - signal (SIGSEGV, ofunc); -#endif - if (crash_signal != 0) - { - static int error_reported = 0; - - if (!error_reported) - { - std::string short_msg - = string_printf (_("unable to demangle '%s' " - "(demangler failed with signal %d)"), - name, crash_signal); - - std::string long_msg - = string_printf ("%s:%d: %s: %s", __FILE__, __LINE__, - "demangler-warning", short_msg.c_str ()); - - target_terminal::scoped_restore_terminal_state term_state; - target_terminal::ours_for_output (); - - begin_line (); - if (core_dump_allowed) - fprintf_unfiltered (gdb_stderr, - _("%s\nAttempting to dump core.\n"), - long_msg.c_str ()); - else - warn_cant_dump_core (long_msg.c_str ()); - - demangler_warning (__FILE__, __LINE__, "%s", short_msg.c_str ()); - - error_reported = 1; - } - - result = NULL; - } + { + /* If there was a failure, we can't report it here, because + we might be in a background thread. Instead, arrange for + the reporting to happen on the main thread. */ + std::string copy = name; + run_on_main_thread ([=] () + { + report_failed_demangle (copy.c_str (), core_dump_allowed, + crash_signal); + }); + + result = NULL; + } } #endif @@ -2194,4 +2188,7 @@ display the offending symbol."), selftests::register_test ("cp_remove_params", selftests::test_cp_remove_params); #endif + + if (!can_dump_core (LIMIT_CUR)) + gdb_demangle_attempt_core_dump = false; } diff --git a/gdb/event-top.c b/gdb/event-top.c index 0b05b2f85a5..ad9e3ff4d9c 100644 --- a/gdb/event-top.c +++ b/gdb/event-top.c @@ -847,6 +847,45 @@ gdb_readline_no_editing_callback (gdb_client_data client_data) } +/* See event-top.h. */ + +thread_local void (*thread_local_segv_handler) (int); + +static void handle_sigsegv (int sig); + +/* Install the SIGSEGV handler. */ +static void +install_handle_sigsegv () +{ +#if defined (HAVE_SIGACTION) + struct sigaction sa; + sa.sa_handler = handle_sigsegv; + sigemptyset (&sa.sa_mask); +#ifdef HAVE_SIGALTSTACK + sa.sa_flags = SA_ONSTACK; +#else + sa.sa_flags = 0; +#endif + sigaction (SIGSEGV, &sa, nullptr); +#else + signal (SIGSEGV, handle_sigsegv); +#endif +} + +/* Handler for SIGSEGV. */ + +static void +handle_sigsegv (int sig) +{ + install_handle_sigsegv (); + + if (thread_local_segv_handler == nullptr) + abort (); + thread_local_segv_handler (sig); +} + + + /* The serial event associated with the QUIT flag. set_quit_flag sets this, and check_quit_flag clears it. Used by interruptible_select to be able to do interruptible I/O with no race with the SIGINT @@ -914,6 +953,8 @@ async_init_signals (void) sigtstp_token = create_async_signal_handler (async_sigtstp_handler, NULL); #endif + + install_handle_sigsegv (); } /* See defs.h. */ diff --git a/gdb/event-top.h b/gdb/event-top.h index 1dc7b13d4f8..e844125cbbf 100644 --- a/gdb/event-top.h +++ b/gdb/event-top.h @@ -70,4 +70,10 @@ extern void gdb_rl_callback_handler_install (const char *prompt); currently installed. */ extern void gdb_rl_callback_handler_reinstall (void); +/* The SIGSEGV handler for this thread, or NULL if there is none. GDB + always installs a global SIGSEGV handler, and then lets threads + indicate their interest in handling the signal by setting this + thread-local variable. */ +extern thread_local void (*thread_local_segv_handler) (int); + #endif