From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id UEieMrXcImEkGwAAWB0awg (envelope-from ) for ; Sun, 22 Aug 2021 19:24:37 -0400 Received: by simark.ca (Postfix, from userid 112) id CB5D71EE18; Sun, 22 Aug 2021 19:24:37 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-1.1 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,MAILING_LIST_MULTI,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (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 simark.ca (Postfix) with ESMTPS id 5A7151E813 for ; Sun, 22 Aug 2021 19:24:36 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 1796E3857413 for ; Sun, 22 Aug 2021 23:24:36 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 1796E3857413 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1629674676; bh=Fh9zgtYAR0pvYQLvDWvjUYqmsil/D7g9WMfEWUAGuZM=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=mGncXUfOSuf05lF4EKrMLIQK02M4xfztorDx6xmDF2NBA1McpwFZ+CkAjSSas2Emd 9a/oL+GnXs6tdnEklv7DUDP98yadBbaHrlP9bQgARlviAoTnuURTIo8EbZV7qG+UP+ kAcQMoBzyQ8gy3ojEwcDkyvIzDJRg/CPIYwqw56A= Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTP id B802A385782D for ; Sun, 22 Aug 2021 23:24:16 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org B802A385782D Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-538-NcnRh5Y_OWCa8nY5-NcWaA-1; Sun, 22 Aug 2021 19:24:15 -0400 X-MC-Unique: NcnRh5Y_OWCa8nY5-NcWaA-1 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 4D930108088C; Sun, 22 Aug 2021 23:24:14 +0000 (UTC) Received: from f34-1.lan (ovpn-112-47.phx2.redhat.com [10.3.112.47]) by smtp.corp.redhat.com (Postfix) with ESMTP id C3CE110013C1; Sun, 22 Aug 2021 23:24:12 +0000 (UTC) To: gdb-patches@sourceware.org Subject: [PATCH v2 3/6] Catch and (re)throw gdb_exception_quit Date: Sun, 22 Aug 2021 16:20:00 -0700 Message-Id: <20210822231959.184061-4-kevinb@redhat.com> In-Reply-To: <20210822231959.184061-1-kevinb@redhat.com> References: <20210822231959.184061-1-kevinb@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset="US-ASCII" X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Kevin Buettner via Gdb-patches Reply-To: Kevin Buettner Errors-To: gdb-patches-bounces+public-inbox=simark.ca@sourceware.org Sender: "Gdb-patches" As described in previous commit for this series, I became concerned that there might be instances where a QUIT (due to either a SIGINT or SIGTERM) might not cause execution to return to the top level. In some instances, it is okay (desired even) to swallow a Ctrl-C / SIGINT, but I don't think there are any instances where we want an exception handler to swallow a SIGTERM. (Allowing this would definitely be a deviation from the current behavior.) I looked at all cases where an exception handler catches a gdb_exception. Handlers which did NOT need modification where those which satisifed one or more of the following conditions: 1) There is no call path to maybe_quit() in the try block. I used a static analysis tool to help make this determination. In instances where the tool didn't provide an answer of "yes, this call path can result in maybe_quit() being called", I reviewed it by hand. 2) The catch block contains a throw for conditions that it doesn't want to handle; these "not handled" conditions must include the quit exception. 3) The catch block contains a call to exception_print(). An earlier patch in this series modified exception_print() to check sync_quit_force_run (which indicates that GDB was sent a SIGTERM signal), calling quit_force() when set. quit_force() does various cleanups and then causes GDB to exit. Therefore, catch blocks which contain a call to exception_print() will cause GDB to terminate when GDB receives a SIGTERM signal. Any try/catch blocks not meeting the above conditions could potentially swallow a QUIT exception. This commit (mostly) adds a new catch of the following form prior to the gdb_exception catch: catch (const gdb_exception_quit &ex) { throw; } There is one case - the first hunk in gdb/breakpoint.c - which is a little different, but I didn't consider it to be different enough to split it out into its own patch. Guile and Python QUIT processing updates are split into their own commits. There are also two other tricky (but somewhat related) cases which I've placed in a separate commit. There is one case that I examined which I could not figure out how to handle. It is the scoped_switch_fork_info destructor in gdb/linux-fork.c. It looks looks like this: ~scoped_switch_fork_info () { if (m_oldfp != nullptr) { /* Switch back to inferior_ptid. */ try { remove_breakpoints (); fork_load_infrun_state (m_oldfp); insert_breakpoints (); } catch (const gdb_exception &ex) { warning (_("Couldn't restore checkpoint state in %s: %s"), target_pid_to_str (m_oldfp->ptid).c_str (), ex.what ()); } } } The static analysis tool determined that there is a call path to maybe_quit(): "_ZN23scoped_switch_fork_infoD2Ev" -> "_Z18remove_breakpointsv" -> "_ZL17remove_breakpointP11bp_location" -> "_ZL19remove_breakpoint_1P11bp_location16remove_bp_reason" -> "_Z26memory_validate_breakpointP7gdbarchP14bp_target_info" -> "_Z18target_read_memorymPhl" -> "_Z11target_readP10target_ops13target_objectPKcPhml" -> "_Z10maybe_quitv"; The catch block simply prints a warning, thus (potentially) swallowing a QUIT exception. It's not clear to me whether rethrowing the exception is safe here. --- gdb/ada-lang.c | 4 ++++ gdb/breakpoint.c | 15 +++++++++++++++ gdb/i386-linux-tdep.c | 4 ++++ gdb/infrun.c | 4 ++++ gdb/jit.c | 4 ++++ gdb/objc-lang.c | 4 ++++ gdb/parse.c | 4 ++++ gdb/printcmd.c | 4 ++++ gdb/record-btrace.c | 4 ++++ gdb/solib.c | 4 ++++ gdb/sparc64-linux-tdep.c | 4 ++++ 11 files changed, 55 insertions(+) diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c index b098991612d..f2f6acf0bfe 100644 --- a/gdb/ada-lang.c +++ b/gdb/ada-lang.c @@ -11679,6 +11679,10 @@ should_stop_exception (const struct bp_location *bl) stop = value_true (evaluate_expression (ada_loc->excep_cond_expr.get ())); value_free_to_mark (mark); } + catch (const gdb_exception_quit &ex) + { + throw; + } catch (const gdb_exception &ex) { exception_fprintf (gdb_stderr, ex, diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 5cc37430e36..6271545073b 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -2699,6 +2699,9 @@ insert_bp_location (struct bp_location *bl, { /* Can't set the breakpoint. */ + if (bp_excpt.reason == RETURN_QUIT) + throw bp_excpt; + /* If the target has closed then it will have deleted any breakpoints inserted within the target inferior, as a result any further attempts to interact with the breakpoint objects @@ -5082,6 +5085,10 @@ bpstat_check_watchpoint (bpstat bs) { e = watchpoint_check (bs); } + catch (const gdb_exception_quit &ex) + { + throw; + } catch (const gdb_exception &ex) { exception_fprintf (gdb_stderr, ex, @@ -5317,6 +5324,10 @@ bpstat_check_breakpoint_conditions (bpstat bs, thread_info *thread) { condition_result = breakpoint_cond_eval (cond); } + catch (const gdb_exception_quit &ex) + { + throw; + } catch (const gdb_exception &ex) { exception_fprintf (gdb_stderr, ex, @@ -14385,6 +14396,10 @@ enable_breakpoint_disp (struct breakpoint *bpt, enum bpdisp disposition, bpt->enable_state = bp_enabled; update_watchpoint (w, 1 /* reparse */); } + catch (const gdb_exception_quit &ex) + { + throw; + } catch (const gdb_exception &e) { bpt->enable_state = orig_enable_state; diff --git a/gdb/i386-linux-tdep.c b/gdb/i386-linux-tdep.c index c0df916ce6d..73b7b465b78 100644 --- a/gdb/i386-linux-tdep.c +++ b/gdb/i386-linux-tdep.c @@ -415,6 +415,10 @@ i386_linux_report_signal_info (struct gdbarch *gdbarch, struct ui_out *uiout, access = parse_and_eval_long ("$_siginfo._sifields._sigfault.si_addr"); } + catch (const gdb_exception_quit &ex) + { + throw; + } catch (const gdb_exception &exception) { return; diff --git a/gdb/infrun.c b/gdb/infrun.c index 8cfd58b267b..b72bb6d9cb1 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -8634,6 +8634,10 @@ normal_stop (void) { execute_cmd_pre_hook (stop_command); } + catch (const gdb_exception_quit &ex) + { + throw; + } catch (const gdb_exception &ex) { exception_fprintf (gdb_stderr, ex, diff --git a/gdb/jit.c b/gdb/jit.c index b3c52c02eee..f29e8390516 100644 --- a/gdb/jit.c +++ b/gdb/jit.c @@ -655,6 +655,10 @@ jit_reader_try_read_symtab (struct jit_code_entry *code_entry, code_entry->symfile_size)) status = 0; } + catch (const gdb_exception_quit &ex) + { + throw; + } catch (const gdb_exception &e) { status = 0; diff --git a/gdb/objc-lang.c b/gdb/objc-lang.c index 077ac772f79..e8ed0cf1c43 100644 --- a/gdb/objc-lang.c +++ b/gdb/objc-lang.c @@ -1277,6 +1277,10 @@ find_objc_msgcall_submethod (int (*f) (CORE_ADDR, CORE_ADDR *), if (f (pc, new_pc) == 0) return 1; } + catch (const gdb_exception_quit &ex) + { + throw; + } catch (const gdb_exception &ex) { exception_fprintf (gdb_stderr, ex, diff --git a/gdb/parse.c b/gdb/parse.c index b2f23eb67bb..632841c9f79 100644 --- a/gdb/parse.c +++ b/gdb/parse.c @@ -509,6 +509,10 @@ parse_exp_in_context (const char **stringptr, CORE_ADDR pc, { lang->parser (&ps); } + catch (const gdb_exception_quit &ex) + { + throw; + } catch (const gdb_exception &except) { /* If parsing for completion, allow this to succeed; but if no diff --git a/gdb/printcmd.c b/gdb/printcmd.c index 3cd42f817f5..883492ccc4f 100644 --- a/gdb/printcmd.c +++ b/gdb/printcmd.c @@ -2094,6 +2094,10 @@ do_one_display (struct display *d) d->exp = parse_expression (d->exp_string.c_str (), &tracker); d->block = tracker.block (); } + catch (const gdb_exception_quit &ex) + { + throw; + } catch (const gdb_exception &ex) { /* Can't re-parse the expression. Disable this display item. */ diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c index bf899ebeb24..1c81f285570 100644 --- a/gdb/record-btrace.c +++ b/gdb/record-btrace.c @@ -2953,6 +2953,10 @@ cmd_record_btrace_start (const char *args, int from_tty) { execute_command ("target record-btrace", from_tty); } + catch (const gdb_exception_quit &ex) + { + throw; + } catch (const gdb_exception &exception) { record_btrace_conf.format = BTRACE_FORMAT_BTS; diff --git a/gdb/solib.c b/gdb/solib.c index 317f7eb485e..fff69de910b 100644 --- a/gdb/solib.c +++ b/gdb/solib.c @@ -736,6 +736,10 @@ update_solib_list (int from_tty) { ops->open_symbol_file_object (from_tty); } + catch (const gdb_exception_quit &ex) + { + throw; + } catch (const gdb_exception &ex) { exception_fprintf (gdb_stderr, ex, diff --git a/gdb/sparc64-linux-tdep.c b/gdb/sparc64-linux-tdep.c index ea692ac00ef..cc36c1b79bb 100644 --- a/gdb/sparc64-linux-tdep.c +++ b/gdb/sparc64-linux-tdep.c @@ -139,6 +139,10 @@ sparc64_linux_report_signal_info (struct gdbarch *gdbarch, struct ui_out *uiout, if (si_code >= SEGV_ACCADI && si_code <= SEGV_ADIPERR) addr = parse_and_eval_long ("$_siginfo._sifields._sigfault.si_addr"); } + catch (const gdb_exception_quit &ex) + { + throw; + } catch (const gdb_exception &exception) { return; -- 2.31.1