public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Kevin Buettner <kevinb@redhat.com>
To: gdb-patches@sourceware.org
Cc: pedro@palves.net, simon.marchi@polymtl.ca
Subject: [PATCH v2] PR gdb/30219: Clear sync_quit_force_run in quit_force
Date: Fri, 24 Mar 2023 15:07:21 -0700	[thread overview]
Message-ID: <20230324150721.7afee776@f37-zws-nv> (raw)
In-Reply-To: <20230310231851.141981-1-kevinb@redhat.com>

PR 30219 shows an internal error due to a "Bad switch" in
print_exception() in gdb/exceptions.c.  The switch in question
contains cases for RETURN_QUIT and RETURN_ERROR, but is missing a case
for the recently added RETURN_FORCED_QUIT.  This commit adds that case.

Making the above change allows the errant test case to pass, but does
not fix the underlying problem, which I'll describe shortly.  Even
though the addition of a case for RETURN_FORCED_QUIT isn't the actual
fix, I still think it's important to add this case so that other
situations which lead to print_exeption() being called won't generate
that "Bad switch" internal error.

In order to understand the underlying problem, please examine
this portion of the backtrace from the bug report:

0x5576e4ff5780 print_exception
        /home/smarchi/src/binutils-gdb/gdb/exceptions.c:100
0x5576e4ff5930 exception_print(ui_file*, gdb_exception const&)
        /home/smarchi/src/binutils-gdb/gdb/exceptions.c:110
0x5576e6a896dd quit_force(int*, int)
        /home/smarchi/src/binutils-gdb/gdb/top.c:1849

The real problem is in quit_force; here's the try/catch which
eventually leads to the internal error:

  /* Get out of tfind mode, and kill or detach all inferiors.  */
  try
    {
      disconnect_tracing ();
      for (inferior *inf : all_inferiors ())
	kill_or_detach (inf, from_tty);
    }
  catch (const gdb_exception &ex)
    {
      exception_print (gdb_stderr, ex);
    }

While running the calls in the try-block, a QUIT check is being
performed.  This check finds that sync_quit_force_run is (still) set,
causing a gdb_exception_forced_quit to be thrown.  The exception
gdb_exception_forced_quit is derived from gdb_exception, causing
exception_print to be called.  As shown by the backtrace,
print_exception is then called, leading to the internal error.

The actual fix, also implemented by this commit, is to clear
sync_quit_force_run along with the quit flag.  This will allow the
various cleanup code, called by quit_force, to run without triggering
a gdb_exception_forced_quit.  (Though, if another SIGTERM is sent to
the gdb process, these flags will be set again and a QUIT check in the
cleanup code will detect it and throw the exception.)

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30219
Reviewed-by: Simon Marchi <simon.marchi@polymtl.ca>
---
 gdb/exceptions.c | 1 +
 gdb/top.c        | 8 ++++++++
 2 files changed, 9 insertions(+)

diff --git a/gdb/exceptions.c b/gdb/exceptions.c
index 4ab8dce58de..8b7858578a9 100644
--- a/gdb/exceptions.c
+++ b/gdb/exceptions.c
@@ -90,6 +90,7 @@ print_exception (struct ui_file *file, const struct gdb_exception &e)
   switch (e.reason)
     {
     case RETURN_QUIT:
+    case RETURN_FORCED_QUIT:
       annotate_quit ();
       break;
     case RETURN_ERROR:
diff --git a/gdb/top.c b/gdb/top.c
index aa2a6409d98..81f74f72f61 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -1824,6 +1824,14 @@ quit_force (int *exit_arg, int from_tty)
 {
   int exit_code = 0;
 
+  /* Clear the quit flag and sync_quit_force_run so that a
+     gdb_exception_forced_quit isn't inadvertently triggered by a QUIT
+     check while running the various cleanup/exit code below.  Note
+     that the call to 'check_quit_flag' clears the quit flag as a side
+     effect.  */
+  check_quit_flag ();
+  sync_quit_force_run = false;
+
   /* An optional expression may be used to cause gdb to terminate with the
      value of that expression.  */
   if (exit_arg)
-- 
2.40.0



  parent reply	other threads:[~2023-03-24 22:07 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-10 23:18 [PATCH] " Kevin Buettner
2023-03-23 15:26 ` Simon Marchi
2023-03-24 21:59   ` Kevin Buettner
2023-03-24 22:07 ` Kevin Buettner [this message]
2023-03-27 15:24   ` [PATCH v2] " Simon Marchi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230324150721.7afee776@f37-zws-nv \
    --to=kevinb@redhat.com \
    --cc=gdb-patches@sourceware.org \
    --cc=pedro@palves.net \
    --cc=simon.marchi@polymtl.ca \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).