From patchwork Wed May 29 21:29:14 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tom Tromey X-Patchwork-Id: 32914 Received: (qmail 129141 invoked by alias); 29 May 2019 22:03:32 -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 129131 invoked by uid 89); 29 May 2019 22:03:32 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-18.1 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_BL_SPAMCOP_NET, RCVD_IN_DNSWL_NONE, SPF_HELO_PASS autolearn=ham version=3.3.1 spammy=__line__, __LINE__ X-HELO: gateway21.websitewelcome.com Received: from gateway21.websitewelcome.com (HELO gateway21.websitewelcome.com) (192.185.46.109) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 29 May 2019 22:03:29 +0000 Received: from cm11.websitewelcome.com (cm11.websitewelcome.com [100.42.49.5]) by gateway21.websitewelcome.com (Postfix) with ESMTP id E647F405A0D43 for ; Wed, 29 May 2019 16:29:21 -0500 (CDT) Received: from box5379.bluehost.com ([162.241.216.53]) by cmsmtp with SMTP id W68PhX3PidnCeW68PhBm5K; Wed, 29 May 2019 16:29:21 -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=MoXmmFKWXSwlv5thehl0z/TGMsLl3vY5KEfL0cxzkz4=; b=YqQv5GSttsx03CaaiOBkDa79uS uw/tk8uDZyDpMIjNdrXdGZqRbli3A/C9MsbBDxcg9270Idjyczr6zTQJwfxpz7gH5TGxQevEGdL9B nxmRpu+dwAvIFoKJqWGz/v05E; Received: from 174-29-48-168.hlrn.qwest.net ([174.29.48.168]:41198 helo=bapiya.Home) by box5379.bluehost.com with esmtpsa (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.91) (envelope-from ) id 1hW68P-002qwZ-LY; Wed, 29 May 2019 16:29:21 -0500 From: Tom Tromey To: gdb-patches@sourceware.org Cc: Tom Tromey Subject: [PATCH v3 6/8] Introduce thread-safe way to handle SIGSEGV Date: Wed, 29 May 2019 15:29:14 -0600 Message-Id: <20190529212916.23721-7-tom@tromey.com> In-Reply-To: <20190529212916.23721-1-tom@tromey.com> References: <20190529212916.23721-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. gdb/ChangeLog 2019-05-29 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 4afb79a4ea9..60bbbc23ec3 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 @@ -1480,7 +1483,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. */ @@ -1499,7 +1502,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 @@ -1513,12 +1552,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) { @@ -1528,23 +1562,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) @@ -1553,42 +1579,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 93b7d2d28bc..f1466c0c2ba 100644 --- a/gdb/event-top.c +++ b/gdb/event-top.c @@ -849,6 +849,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 @@ -916,6 +939,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