From patchwork Sat Mar 9 17:22:59 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tom Tromey X-Patchwork-Id: 31804 Received: (qmail 29949 invoked by alias); 9 Mar 2019 17:23:10 -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 29863 invoked by uid 89); 9 Mar 2019 17:23:10 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-22.1 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=quit, protect, thread-local, crash_signal X-HELO: gateway31.websitewelcome.com Received: from gateway31.websitewelcome.com (HELO gateway31.websitewelcome.com) (192.185.143.46) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sat, 09 Mar 2019 17:23:06 +0000 Received: from cm16.websitewelcome.com (cm16.websitewelcome.com [100.42.49.19]) by gateway31.websitewelcome.com (Postfix) with ESMTP id F375438089 for ; Sat, 9 Mar 2019 11:23:03 -0600 (CST) Received: from box5379.bluehost.com ([162.241.216.53]) by cmsmtp with SMTP id 2fgdhB8KX4FKp2fgdhR6ED; Sat, 09 Mar 2019 11:23:03 -0600 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=zqTN4Yschs779aVpeBbqIOX+IOkAHp8k4bZ4wwIjtlY=; b=VFn5aQayp0KrJJkrlBEAn4LFIQ /XjwcGCiOJNwOGu42uQGcil4nj4cSTPRgL9Z4m9j69ipnQWG8Ad6VhzNzvwbEzxOGb2tLHcLadc/F wsUPn5cQ2vcuYF2wYWd2CrzEi; Received: from 75-166-85-218.hlrn.qwest.net ([75.166.85.218]:56494 helo=bapiya.Home) by box5379.bluehost.com with esmtpsa (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.91) (envelope-from ) id 1h2fgd-003et2-Pm; Sat, 09 Mar 2019 11:23:03 -0600 From: Tom Tromey To: gdb-patches@sourceware.org Cc: Tom Tromey Subject: [RFC 5/6] Introduce thread-safe way to handle SIGSEGV Date: Sat, 9 Mar 2019 10:22:59 -0700 Message-Id: <20190309172300.2764-6-tom@tromey.com> In-Reply-To: <20190309172300.2764-1-tom@tromey.com> References: <20190309172300.2764-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 thread 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. 2019-03-08 Tom Tromey * event-top.h (segv_handler): Declare. * event-top.c (segv_handler): New global. (handle_sigsegv): New function. (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 | 114 +++++++++++++++++++++++------------------------ gdb/event-top.c | 37 +++++++++++++++ gdb/event-top.h | 3 ++ 4 files changed, 109 insertions(+), 58 deletions(-) diff --git a/gdb/cp-support.c b/gdb/cp-support.c index 79457f991fa..ae7f7e2abc2 100644 --- a/gdb/cp-support.c +++ b/gdb/cp-support.c @@ -37,6 +37,9 @@ #include "common/gdb_setjmp.h" #include "safe-ctype.h" #include "common/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 @@ -1470,7 +1473,7 @@ static int catch_demangler_crashes = 1; /* 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. */ @@ -1489,7 +1492,43 @@ gdb_demangle_signal_handler (int signo) gdb_demangle_attempt_core_dump = 0; } - 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, int core_dump_allowed, + int crash_signal) +{ + 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; + } } #endif @@ -1503,12 +1542,7 @@ 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; + static std::atomic core_dump_allowed (-1); if (core_dump_allowed == -1) { @@ -1518,23 +1552,15 @@ gdb_demangle (const char *name, int options) gdb_demangle_attempt_core_dump = 0; } - 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 + scoped_restore restore_segv + = make_scoped_restore (&segv_handler, + catch_demangler_crashes + ? gdb_demangle_signal_handler + : nullptr); - crash_signal = SIGSETJMP (gdb_demangle_jmp_buf); - } + SIGJMP_BUF jmp_buf; + if (catch_demangler_crashes) + crash_signal = SIGSETJMP (jmp_buf); #endif if (crash_signal == 0) @@ -1543,42 +1569,14 @@ 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; - } + std::string copy = name; + run_on_main_thread ([=] () + { + report_failed_demangle (copy.c_str (), core_dump_allowed, + crash_signal); + }); result = NULL; } diff --git a/gdb/event-top.c b/gdb/event-top.c index 6e4031165c7..3cb2046aeb3 100644 --- a/gdb/event-top.c +++ b/gdb/event-top.c @@ -845,6 +845,29 @@ gdb_readline_no_editing_callback (gdb_client_data client_data) } +/* See event-top.h. */ + +thread_local void (*segv_handler) (int); + +/* Handler for SIGSEGV. */ + +static void +handle_sigsegv (int sig) +{ + if (segv_handler == nullptr) + { + signal (sig, nullptr); + raise (sig); + } + else + { + signal (sig, handle_sigsegv); + 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 @@ -912,6 +935,20 @@ async_init_signals (void) sigtstp_token = create_async_signal_handler (async_sigtstp_handler, NULL); #endif + +#if defined (HAVE_SIGACTION) && defined (SA_RESTART) + struct sigaction sa, old_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, &old_sa); +#else + signal (SIGSEGV, handle_sigsegv); +#endif } /* See defs.h. */ diff --git a/gdb/event-top.h b/gdb/event-top.h index 4c3e6cb8612..849f40d9192 100644 --- a/gdb/event-top.h +++ b/gdb/event-top.h @@ -70,4 +70,7 @@ 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. */ +extern thread_local void (*segv_handler) (int); + #endif