From patchwork Mon Nov 18 19:57:17 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Guinevere Larsen X-Patchwork-Id: 101465 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 010EF3858C62 for ; Mon, 18 Nov 2024 19:58:11 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 010EF3858C62 Authentication-Results: sourceware.org; dkim=fail reason="signature verification failed" (1024-bit key, unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=PHb0jEyK X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTP id 9AE7A3858D21 for ; Mon, 18 Nov 2024 19:57:26 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 9AE7A3858D21 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 9AE7A3858D21 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.129.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1731959846; cv=none; b=aKACUoiVi0Htz/mtlC1wSeajG2oKskO9gEHi7cMtJlulUM9Zp4molI9rwnjN5+GUUVrLX2Uqfqf0DBdUszDGPy9WsIrwQnIi4VYO6YKmex0TyYzwQEw1B07Jb1dZysmSMBMoXS5Jnh2CB5rg/ausbSEKoDZM+yUvJL4tzjNQx14= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1731959846; c=relaxed/simple; bh=fjjPYVhxc3R+L78IGhR3+tc+5GxJ5hfaqh29IhB8bVI=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=b2aXz4I4EGv3uCC+vd25Yx7cuNm1D8eiWpviTSt2ex9AYWpThZSkyOoNyVryiR5LRgJ/QIS3n7haGpmxPs2ZPauJn8FUbijk5e2A1UBTJLlNIldpUSekGaQgA1HuTSNfdEhP/fDSdwx854g/B7RAQJvVHhChOuJkMKwxMkwa4r0= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 9AE7A3858D21 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1731959846; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=ZkhHKIJ+F5NQUkGN1oCRWxkkIk2qhLDdah18dgIebYU=; b=PHb0jEyKT9zn+QfwrekgoDITLhsxi+px2L8YZfNzIXlDoJu9JOEn4It4rR9PcM5aNTjSSk dB7gW6RfOgiAPn5DRyL99HV6cpNiLOpcGU/gvdEfhuqvePawuK67LwTRG56WLDgSKPSb7J bA9AlfdB6zQNMvQ4xahtD7zEjjszNwU= Received: from mx-prod-mc-02.mail-002.prod.us-west-2.aws.redhat.com (ec2-54-186-198-63.us-west-2.compute.amazonaws.com [54.186.198.63]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-303-r7Q-XxWZNsy5rJv8ivF43Q-1; Mon, 18 Nov 2024 14:57:24 -0500 X-MC-Unique: r7Q-XxWZNsy5rJv8ivF43Q-1 X-Mimecast-MFC-AGG-ID: r7Q-XxWZNsy5rJv8ivF43Q Received: from mx-prod-int-02.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-02.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.15]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-02.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id 31EF21953943 for ; Mon, 18 Nov 2024 19:57:24 +0000 (UTC) Received: from fedora.redhat.com (unknown [10.96.134.66]) by mx-prod-int-02.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id F2F8D1956086; Mon, 18 Nov 2024 19:57:22 +0000 (UTC) From: Guinevere Larsen To: gdb-patches@sourceware.org Cc: Guinevere Larsen Subject: [PATCH v5] gdb: Introduce RAII signal handler setter Date: Mon, 18 Nov 2024 16:57:17 -0300 Message-ID: <20241118195717.783180-1-guinevere@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.0 on 10.30.177.15 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: AbXmdFb_uW9i4pQAqjPxoUPZtsptZ-ImbQuH_VEeOGw_1731959844 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-9.8 required=5.0 tests=BAYES_00, DKIM_INVALID, DKIM_SIGNED, GIT_PATCH_0, KAM_DMARC_NONE, KAM_DMARC_STATUS, KAM_SHORT, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces~patchwork=sourceware.org@sourceware.org This patch is motivated by the wait function for the record-full target, that would install a custom signal handler for SIGINT, but could throw an exception and never reset the SIGINT handler. This is clearly a bad idea, so this patch introduces the class scoped_signal_handler in a new .h file. The file is added to gdbsupport, even though only gdb code is using it, because it feels like an addition that would be useful for more than just directly gdb. The implementation of the RAII class is based on the implementation on gdb/utils.c. That is, it uses preprocessor ifdefs to probe for sigaction support, and uses it if possible, defaulting to a raw call to signal only if sigaction isn't supported. sigaction is preferred based on the "portability" section of the manual page for the signal function. There are 3 places where this class can just be dropped in, gdb/record-full.c, gdb/utils.c and gdb/extension.c. This third place already had a specialized RAII signal handler setter, but it is substituted for the new general purpose one. --- gdb/extension.c | 25 +---------- gdb/record-full.c | 5 +-- gdb/utils.c | 19 +------- gdbsupport/scoped_signal_handler.h | 71 ++++++++++++++++++++++++++++++ 4 files changed, 77 insertions(+), 43 deletions(-) create mode 100644 gdbsupport/scoped_signal_handler.h diff --git a/gdb/extension.c b/gdb/extension.c index ec1aa138872..47f56914802 100644 --- a/gdb/extension.c +++ b/gdb/extension.c @@ -33,6 +33,7 @@ #include "guile/guile.h" #include #include "inferior.h" +#include "gdbsupport/scoped_signal_handler.h" static script_sourcer_func source_gdb_script; static objfile_script_sourcer_func source_gdb_objfile_script; @@ -297,28 +298,6 @@ ext_lang_auto_load_enabled (const struct extension_language_defn *extlang) } -/* RAII class used to temporarily return SIG to its default handler. */ - -template -struct scoped_default_signal -{ - scoped_default_signal () - { m_old_sig_handler = signal (SIG, SIG_DFL); } - - ~scoped_default_signal () - { signal (SIG, m_old_sig_handler); } - - DISABLE_COPY_AND_ASSIGN (scoped_default_signal); - -private: - /* The previous signal handler that needs to be restored. */ - sighandler_t m_old_sig_handler; -}; - -/* Class to temporarily return SIGINT to its default handler. */ - -using scoped_default_sigint = scoped_default_signal; - /* Functions that iterate over all extension languages. These only iterate over external extension languages, not including GDB's own extension/scripting language, unless otherwise indicated. */ @@ -334,7 +313,7 @@ ext_lang_initialization (void) if (extlang->ops != nullptr && extlang->ops->initialize != NULL) { - scoped_default_sigint set_sigint_to_default_handler; + scoped_signal_handler set_sigint_to_default_handler (SIG_DFL); extlang->ops->initialize (extlang); } } diff --git a/gdb/record-full.c b/gdb/record-full.c index d48718a2793..7d7959ffe68 100644 --- a/gdb/record-full.c +++ b/gdb/record-full.c @@ -39,6 +39,7 @@ #include "infrun.h" #include "gdbsupport/gdb_unlinker.h" #include "gdbsupport/byte-vector.h" +#include "gdbsupport/scoped_signal_handler.h" #include "async-event.h" #include "top.h" #include "valprint.h" @@ -1159,6 +1160,7 @@ record_full_wait_1 (struct target_ops *ops, { scoped_restore restore_operation_disable = record_full_gdb_operation_disable_set (); + scoped_signal_handler interrupt_handler (record_full_sig_handler); if (record_debug) gdb_printf (gdb_stdlog, @@ -1179,7 +1181,6 @@ record_full_wait_1 (struct target_ops *ops, } record_full_get_sig = 0; - signal (SIGINT, record_full_sig_handler); record_full_stop_reason = TARGET_STOPPED_BY_NO_REASON; @@ -1468,8 +1469,6 @@ record_full_wait_1 (struct target_ops *ops, } } - signal (SIGINT, handle_sigint); - return inferior_ptid; } diff --git a/gdb/utils.c b/gdb/utils.c index 2932efc3fc0..28a262d7182 100644 --- a/gdb/utils.c +++ b/gdb/utils.c @@ -19,6 +19,7 @@ #include #include "gdbsupport/gdb_wait.h" +#include "gdbsupport/scoped_signal_handler.h" #include "event-top.h" #include "gdbthread.h" #include "fnmatch.h" @@ -3467,18 +3468,7 @@ wait_to_die_with_timeout (pid_t pid, int *status, int timeout) if (timeout > 0) { #ifdef SIGALRM -#if defined (HAVE_SIGACTION) && defined (SA_RESTART) - struct sigaction sa, old_sa; - - sa.sa_handler = sigalrm_handler; - sigemptyset (&sa.sa_mask); - sa.sa_flags = 0; - sigaction (SIGALRM, &sa, &old_sa); -#else - sighandler_t ofunc; - - ofunc = signal (SIGALRM, sigalrm_handler); -#endif + scoped_signal_handler alarm_restore (sigalrm_handler); alarm (timeout); #endif @@ -3487,11 +3477,6 @@ wait_to_die_with_timeout (pid_t pid, int *status, int timeout) #ifdef SIGALRM alarm (0); -#if defined (HAVE_SIGACTION) && defined (SA_RESTART) - sigaction (SIGALRM, &old_sa, NULL); -#else - signal (SIGALRM, ofunc); -#endif #endif } else diff --git a/gdbsupport/scoped_signal_handler.h b/gdbsupport/scoped_signal_handler.h new file mode 100644 index 00000000000..c6c207a1019 --- /dev/null +++ b/gdbsupport/scoped_signal_handler.h @@ -0,0 +1,71 @@ +/* RAII class to install a separate handler for a given signal + + Copyright (C) 2024 Free Software Foundation, Inc. + + This file is part of GDB. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . */ + +#ifndef SCOPED_SIGNAL_HANDLER_H +#define SCOPED_SIGNAL_HANDLER_H + +#include + +/* RAII class to set a signal handler for a scope, that will take care of + unsetting the handler when the scope is left. + This class will try to use sigaction whenever available, following the + recommendation on the man page for signal, and only fallback to signal + if necessary. */ +template +class scoped_signal_handler +{ +public: + scoped_signal_handler (sighandler_t handler) + { +#if defined (HAVE_SIGACTION) + struct sigaction act; + + act.sa_handler = handler; + sigemptyset (&act.sa_mask); + act.sa_flags = 0; + sigaction (SIG, &act, &m_prev_handler); +#else + /* The return of the function call is the previous signal handler, or + SIG_ERR if the function doesn't succeed. */ + m_prev_handler = signal (SIG, handler); + /* According to the GNU libc manual, the only way signal fails is if + the signum given is invalid, so we should be safe to assert. */ + gdb_assert (m_prev_handler != SIG_ERR); +#endif + } + + ~scoped_signal_handler () + { +#if defined (HAVE_SIGACTION) + sigaction (SIG, &m_prev_handler, nullptr); +#else + signal (SIG, m_prev_handler); +#endif + } + + DISABLE_COPY_AND_ASSIGN (scoped_signal_handler); +private: +#if defined (HAVE_SIGACTION) + struct sigaction m_prev_handler; +#else + typedef sighandler_t m_prev_handler; +#endif +}; + +#endif /* SCOPED_SIGNAL_HANDLER_H */