[PATCH v3 0/7] glibc-2.34: Fix gdb.base/gdb-sigterm.exp failure/error

[PATCH v3 0/7] glibc-2.34: Fix gdb.base/gdb-sigterm.exp failure/error

[Kevin Buettner]

This series addresses Pedro's concerns regarding the v2 series posted
last year:

https://sourceware.org/pipermail/gdb-patches/2021-August/181575.html

In particular, it introduces a new exception which will be thrown
when GDB receives a SIGTERM.  That exception is handled at the
top level when possible, though for the extension languages, Python
and Guile, a somewhat different approach had to be taken.  In order to
ensure that this new exception isn't inadvertently swallowed on its
way up to the top level, I also did an audit of the try / catch code
involving gdb_exception, making adjustments where necessary.

Kevin Buettner (7):
  Introduce gdb_exception_forced_quit
  Handle gdb SIGTERM by throwing / catching gdb_exception_force_quit
  Catch gdb_exception_error instead of gdb_exception (in many places)
  Python QUIT processing updates
  Guile QUIT processing updates
  QUIT processing w/ explicit throw for gdb_exception_forced_quit
  Handle QUIT processing in the scoped_switch_fork_info destructor

 gdb/ada-lang.c                   |  2 +-
 gdb/breakpoint.c                 |  8 ++++----
 gdb/cli/cli-interp.c             |  2 +-
 gdb/event-top.c                  |  2 ++
 gdb/guile/guile-internal.h       |  5 +++++
 gdb/guile/scm-pretty-print.c     |  5 +++++
 gdb/guile/scm-type.c             |  5 +++++
 gdb/guile/scm-value.c            |  5 +++++
 gdb/i386-linux-tdep.c            |  2 +-
 gdb/inf-loop.c                   |  2 +-
 gdb/infcmd.c                     |  2 +-
 gdb/infrun.c                     |  2 +-
 gdb/jit.c                        |  2 +-
 gdb/linux-fork.c                 | 13 +++++++++++++
 gdb/main.c                       | 12 ++++++++++++
 gdb/mi/mi-cmd-break.c            |  2 +-
 gdb/mi/mi-interp.c               |  2 +-
 gdb/mi/mi-main.c                 |  4 ++++
 gdb/objc-lang.c                  |  2 +-
 gdb/parse.c                      |  2 +-
 gdb/printcmd.c                   |  2 +-
 gdb/python/py-finishbreakpoint.c |  5 +++++
 gdb/python/py-gdb-readline.c     |  4 ++++
 gdb/python/py-symbol.c           |  5 +++++
 gdb/python/py-utils.c            |  3 +++
 gdb/python/py-value.c            |  5 +++++
 gdb/record-btrace.c              |  2 +-
 gdb/record-full.c                |  2 +-
 gdb/remote-fileio.c              | 15 ++++++++++-----
 gdb/solib.c                      |  2 +-
 gdb/sparc64-linux-tdep.c         |  2 +-
 gdb/symfile-mem.c                |  2 +-
 gdb/top.h                        |  2 +-
 gdb/tui/tui-io.c                 |  4 ++++
 gdb/tui/tui.c                    |  4 ++++
 gdb/utils.c                      |  2 +-
 gdb/windows-nat.c                |  4 ++++
 gdbsupport/common-exceptions.cc  | 14 ++++++++++++++
 gdbsupport/common-exceptions.h   | 22 +++++++++++++++++++++-
 39 files changed, 153 insertions(+), 29 deletions(-)

-- 
2.35.1

[PATCH v3 1/7] Introduce gdb_exception_forced_quit

[Kevin Buettner]

This commit adds a new exception 'gdb_exception_forced_quit', reason
code 'REASON_FORCED_QUIT', return mask 'RETURN_MASK_FORCED_QUIT', and
a wrapper for throwing the exception, throw_forced_quit().

The addition of this exception plus supporting code will allow us to
recognize that a SIGTERM has been received by GDB and then propagate
recognition of that fact to the upper levels of GDB where it can be
correctly handled.  At the moment, when GDB receives a SIGTERM, it
will attempt to exit via a series of calls from the QUIT checking
code.  However, before it can exit, it must do various cleanups, such
as killing or detaching all inferiors.  Should these cleanups be
attempted while GDB is executing very low level code, such as reading
target memory from within ps_xfer_memory(), it can happen that some of
GDB's state is out of sync with regard to the cleanup code's
expectations.  In the case just mentioned, it's been observed that
inferior_ptid and the current_thread_ are not in sync; this triggers
an assert / internal error.

This commit only introduces the exception plus supporting machinery;
changes which use this new exception are in later commits in this
series.

Summary:

	* gdbsupport/common-exceptions.h (enum return_reason): Add
	new constant RETURN_FORCED_QUIT.
	(return_mask typedef): Add new constant RETURN_MASK_FORCED_QUIT.
	Update RETURN_MASK_ALL.
	(gdb_exception_forced_quit): New gdb_exception.
	(throw_forced_quit): Declare.
	* gdbsupport/common-exceptions.cc (throw_exception): Handle
	RETURN_FORCED_QUIT.
	(throw_it): Likewise.
	(throw_forced_quit): New function.
---
 gdbsupport/common-exceptions.cc | 14 ++++++++++++++
 gdbsupport/common-exceptions.h  | 22 +++++++++++++++++++++-
 2 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/gdbsupport/common-exceptions.cc b/gdbsupport/common-exceptions.cc
index 2d686c4ee65..cd97da2ead7 100644
--- a/gdbsupport/common-exceptions.cc
+++ b/gdbsupport/common-exceptions.cc
@@ -184,6 +184,8 @@ throw_exception (gdb_exception &&exception)
 {
   if (exception.reason == RETURN_QUIT)
     throw gdb_exception_quit (std::move (exception));
+  else if (exception.reason == RETURN_FORCED_QUIT)
+    throw gdb_exception_forced_quit (std::move (exception));
   else if (exception.reason == RETURN_ERROR)
     throw gdb_exception_error (std::move (exception));
   else
@@ -196,6 +198,8 @@ throw_it (enum return_reason reason, enum errors error, const char *fmt,
 {
   if (reason == RETURN_QUIT)
     throw gdb_exception_quit (fmt, ap);
+  else if (reason == RETURN_FORCED_QUIT)
+    throw gdb_exception_forced_quit (fmt, ap);
   else if (reason == RETURN_ERROR)
     throw gdb_exception_error (error, fmt, ap);
   else
@@ -233,3 +237,13 @@ throw_quit (const char *fmt, ...)
   throw_vquit (fmt, args);
   va_end (args);
 }
+
+void
+throw_forced_quit (const char *fmt, ...)
+{
+  va_list args;
+
+  va_start (args, fmt);
+  throw_it (RETURN_FORCED_QUIT, GDB_NO_ERROR, fmt, args);
+  va_end (args);
+}
diff --git a/gdbsupport/common-exceptions.h b/gdbsupport/common-exceptions.h
index b7e6f2dd7ca..e71246d5b32 100644
--- a/gdbsupport/common-exceptions.h
+++ b/gdbsupport/common-exceptions.h
@@ -31,6 +31,8 @@
 
 enum return_reason
   {
+    /* SIGTERM sent to GDB.  */
+    RETURN_FORCED_QUIT = -3,
     /* User interrupt.  */
     RETURN_QUIT = -2,
     /* Any other error.  */
@@ -41,9 +43,10 @@ enum return_reason
 
 typedef enum
 {
+  RETURN_MASK_FORCED_QUIT = RETURN_MASK (RETURN_FORCED_QUIT),
   RETURN_MASK_QUIT = RETURN_MASK (RETURN_QUIT),
   RETURN_MASK_ERROR = RETURN_MASK (RETURN_ERROR),
-  RETURN_MASK_ALL = (RETURN_MASK_QUIT | RETURN_MASK_ERROR)
+  RETURN_MASK_ALL = (RETURN_MASK_FORCED_QUIT | RETURN_MASK_QUIT | RETURN_MASK_ERROR)
 } return_mask;
 
 /* Describe all exceptions.  */
@@ -275,6 +278,21 @@ struct gdb_exception_quit : public gdb_exception
   }
 };
 
+struct gdb_exception_forced_quit : public gdb_exception
+{
+  gdb_exception_forced_quit (const char *fmt, va_list ap)
+    ATTRIBUTE_PRINTF (2, 0)
+    : gdb_exception (RETURN_FORCED_QUIT, GDB_NO_ERROR, fmt, ap)
+  {
+  }
+
+  explicit gdb_exception_forced_quit (gdb_exception &&ex) noexcept
+    : gdb_exception (std::move (ex))
+  {
+    gdb_assert (ex.reason == RETURN_FORCED_QUIT);
+  }
+};
+
 /* An exception type that inherits from both std::bad_alloc and a gdb
    exception.  This is necessary because operator new can only throw
    std::bad_alloc, and OTOH, we want exceptions thrown due to memory
@@ -317,5 +335,7 @@ extern void throw_error (enum errors error, const char *fmt, ...)
      ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (2, 3);
 extern void throw_quit (const char *fmt, ...)
      ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 2);
+extern void throw_forced_quit (const char *fmt, ...)
+     ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF (1, 2);
 
 #endif /* COMMON_COMMON_EXCEPTIONS_H */
-- 
2.35.1

[PATCH v3 2/7] Handle gdb SIGTERM by throwing / catching gdb_exception_force_quit

[Kevin Buettner]

When a GDB process receives the SIGTERM signal, handle_sigterm() in
event-top.c is called.  The global variable 'sync_quit_force_run' is
set by this signal handler.  It does some other things too, but the
setting of this global is the important bit for the SIGTERM part of
this discussion.

GDB will periodically check to see whether a Ctrl-C or SIGTERM has
been received.  This is performed via use of the QUIT macro in
GDB's code.  QUIT is defined to invoke maybe_quit(), which will be
periodically called during any lengthy operation.  This is supposed to
ensure that the user won't have to wait too long for a Ctrl-C or
SIGTERM to be acted upon.

When a Ctrl-C / SIGINT is received, quit_handler() will decide whether
to pass the SIGINT onto the inferior or to call quit() which causes
gdb_exception_quit to be thrown.  This exception (usually) propagates
to the top level.  Control is then returned to the top level event
loop.

At the moment, SIGTERM is handled very differently.  Instead of
throwing an exception, quit_force() is called.  This does eventually
cause GDB to exit(), but prior to that happening, the inferiors
are killed or detached and other target related cleanup occurs.
As shown in this discussion between Pedro Alves and myself...

https://sourceware.org/pipermail/gdb-patches/2021-July/180802.html
https://sourceware.org/pipermail/gdb-patches/2021-July/180902.html
https://sourceware.org/pipermail/gdb-patches/2021-July/180903.html

...we found that it is possible for inferior_ptid and current_thread_
to get out of sync.  When that happens, the "current_thread_ != nullptr"
assertion in inferior_thread() can fail resulting in a GDB internal
error.

Pedro recommended that we "let the normal quit exception propagate all
the way to the top level, and then have the top level call quit_force
if sync_quit_force_run is set."  However, after the v2 series for this
patch set, we tweaked that idea by introducing a new exception for
handling SIGTERM.

This commit implements the obvious part of Pedro's suggestion:
Instead of calling quit_force from quit(), throw_forced_quit() is now
called instead.  This causes the new exception 'gdb_exception_forced_quit'
to be thrown.

At the top level, I changed catch_command_errors() and captured_main()
to catch gdb_exception_forced_quit and then call quit_force() from the
catch block.  I also changed start_event_loop() to also catch
gdb_exception_forced_quit; while we could also call quit_force() from
that catch block, it's sufficient to simply rethrow the exception
since it'll be caught by the newly added code in captured_main().

Making these changes fixed the failure / regression that I was seeing
for gdb.base/gdb-sigterm.exp when run on a machine with glibc-2.34.
However, there are many other paths back to the top level which this
test case does not test.  I did an audit of all of the try / catch
code in GDB in which calls in the try-block might (eventually) call
QUIT.  I found many cases where gdb_exception_quit and the new
gdb_exception_forced_quit will be swallowed.  (When using GDB, have
you ever hit Ctrl-C and not have it do anything; if so, it could be
due to a swallowed gdb_exception_quit in one of the cases I've
identified.)  The rest of the patches in this series deal with this
concern.
---
 gdb/main.c  | 12 ++++++++++++
 gdb/utils.c |  2 +-
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/gdb/main.c b/gdb/main.c
index 8f944d86545..503d5d51347 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -420,6 +420,10 @@ start_event_loop ()
 	{
 	  result = gdb_do_one_event ();
 	}
+      catch (const gdb_exception_forced_quit &ex)
+	{
+	  throw;
+	}
       catch (const gdb_exception &ex)
 	{
 	  exception_print (gdb_stderr, ex);
@@ -528,6 +532,10 @@ catch_command_errors (catch_command_errors_const_ftype command,
       if (do_bp_actions)
 	bpstat_do_actions ();
     }
+  catch (const gdb_exception_forced_quit &e)
+    {
+      quit_force (NULL, 0);
+    }
   catch (const gdb_exception &e)
     {
       return handle_command_errors (e);
@@ -1347,6 +1355,10 @@ captured_main (void *data)
 	{
 	  captured_command_loop ();
 	}
+      catch (const gdb_exception_forced_quit &ex)
+        {
+	  quit_force (NULL, 0);
+	}
       catch (const gdb_exception &ex)
 	{
 	  exception_print (gdb_stderr, ex);
diff --git a/gdb/utils.c b/gdb/utils.c
index a8d6c96386d..28bd475a40e 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -671,7 +671,7 @@ quit (void)
   if (sync_quit_force_run)
     {
       sync_quit_force_run = 0;
-      quit_force (NULL, 0);
+      throw_forced_quit ("SIGTERM");
     }
 
 #ifdef __MSDOS__
-- 
2.35.1

[PATCH v3 3/7] Catch gdb_exception_error instead of gdb_exception (in many places)

[Kevin Buettner]

As described in the previous commit for this series, I became
concerned that there might be instances in which a QUIT (due to either
a SIGINT or SIGTERM) might not cause execution to return to the top
level.  In some (though very few) instances, it is okay to not
propagate the exception for a Ctrl-C / SIGINT, but I don't think that
it is ever okay to swallow the exception caused by a SIGTERM.
Allowing that to happen would definitely be a deviation from the
current behavior in which GDB exits upon receipt of a SIGTERM.

I looked at all cases where an exception handler catches a
gdb_exception.  Handlers which did NOT need modification were 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 and the new "forced quit" exception.

  3) There was (also) a catch for gdb_exception_quit.

Any try/catch blocks not meeting the above conditions could
potentially swallow a QUIT exception.

My first thought was to add catch blocks for gdb_exception_quit and
then rethrow the exception.  But Pedro pointed out that this can be
handled without adding additional code by simply catching
gdb_exception_error instead.  That's what this patch series does.

There are some oddball cases which needed to be handled differently,
plus the extension languages, but those are handled in later patches.
---
 gdb/ada-lang.c           | 2 +-
 gdb/breakpoint.c         | 8 ++++----
 gdb/cli/cli-interp.c     | 2 +-
 gdb/i386-linux-tdep.c    | 2 +-
 gdb/inf-loop.c           | 2 +-
 gdb/infcmd.c             | 2 +-
 gdb/infrun.c             | 2 +-
 gdb/jit.c                | 2 +-
 gdb/mi/mi-cmd-break.c    | 2 +-
 gdb/mi/mi-interp.c       | 2 +-
 gdb/objc-lang.c          | 2 +-
 gdb/parse.c              | 2 +-
 gdb/printcmd.c           | 2 +-
 gdb/record-btrace.c      | 2 +-
 gdb/record-full.c        | 2 +-
 gdb/solib.c              | 2 +-
 gdb/sparc64-linux-tdep.c | 2 +-
 gdb/symfile-mem.c        | 2 +-
 18 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index d2f620cbb04..fa10429038e 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -11789,7 +11789,7 @@ 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 &ex)
+  catch (const gdb_exception_error &ex)
     {
       exception_fprintf (gdb_stderr, ex,
 			 _("Error in testing exception condition:\n"));
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index a3cfeea6989..134bdaf8d35 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -2668,7 +2668,7 @@ insert_bp_location (struct bp_location *bl,
 		  if (val)
 		    bp_excpt = gdb_exception {RETURN_ERROR, GENERIC_ERROR};
 		}
-	      catch (gdb_exception &e)
+	      catch (gdb_exception_error &e)
 		{
 		  bp_excpt = std::move (e);
 		}
@@ -5067,7 +5067,7 @@ bpstat_check_watchpoint (bpstat *bs)
 	    {
 	      e = watchpoint_check (bs);
 	    }
-	  catch (const gdb_exception &ex)
+	  catch (const gdb_exception_error &ex)
 	    {
 	      exception_fprintf (gdb_stderr, ex,
 				 "Error evaluating expression "
@@ -5302,7 +5302,7 @@ bpstat_check_breakpoint_conditions (bpstat *bs, thread_info *thread)
 	    {
 	      condition_result = breakpoint_cond_eval (cond);
 	    }
-	  catch (const gdb_exception &ex)
+	  catch (const gdb_exception_error &ex)
 	    {
 	      exception_fprintf (gdb_stderr, ex,
 				 "Error in testing breakpoint condition:\n");
@@ -13855,7 +13855,7 @@ enable_breakpoint_disp (struct breakpoint *bpt, enum bpdisp disposition,
 	  bpt->enable_state = bp_enabled;
 	  update_watchpoint (w, 1 /* reparse */);
 	}
-      catch (const gdb_exception &e)
+      catch (const gdb_exception_error &e)
 	{
 	  bpt->enable_state = orig_enable_state;
 	  exception_fprintf (gdb_stderr, e, _("Cannot enable watchpoint %d: "),
diff --git a/gdb/cli/cli-interp.c b/gdb/cli/cli-interp.c
index 0190b4d32bc..abd5a04552e 100644
--- a/gdb/cli/cli-interp.c
+++ b/gdb/cli/cli-interp.c
@@ -368,7 +368,7 @@ safe_execute_command (struct ui_out *command_uiout, const char *command,
     {
       execute_command (command, from_tty);
     }
-  catch (gdb_exception &exception)
+  catch (gdb_exception_error &exception)
     {
       e = std::move (exception);
     }
diff --git a/gdb/i386-linux-tdep.c b/gdb/i386-linux-tdep.c
index 48191797cd5..0041b33c367 100644
--- a/gdb/i386-linux-tdep.c
+++ b/gdb/i386-linux-tdep.c
@@ -415,7 +415,7 @@ 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 &exception)
+  catch (const gdb_exception_error &exception)
     {
       return;
     }
diff --git a/gdb/inf-loop.c b/gdb/inf-loop.c
index 2002c4b8966..5455832a8f9 100644
--- a/gdb/inf-loop.c
+++ b/gdb/inf-loop.c
@@ -69,7 +69,7 @@ inferior_event_handler (enum inferior_event_type event_type)
 	    {
 	      bpstat_do_actions ();
 	    }
-	  catch (const gdb_exception &e)
+	  catch (const gdb_exception_error &e)
 	    {
 	      /* If the user was running a foreground execution
 		 command, then propagate the error so that the prompt
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 682bebe1229..34c985535cd 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -1565,7 +1565,7 @@ print_return_value (struct ui_out *uiout, struct return_value_info *rv)
 	 delete the breakpoint.  */
       print_return_value_1 (uiout, rv);
     }
-  catch (const gdb_exception &ex)
+  catch (const gdb_exception_error &ex)
     {
       exception_print (gdb_stdout, ex);
     }
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 5311822fcb8..b7d19a163a3 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -8581,7 +8581,7 @@ normal_stop (void)
 	{
 	  execute_cmd_pre_hook (stop_command);
 	}
-      catch (const gdb_exception &ex)
+      catch (const gdb_exception_error &ex)
 	{
 	  exception_fprintf (gdb_stderr, ex,
 			     "Error while running hook_stop:\n");
diff --git a/gdb/jit.c b/gdb/jit.c
index 9f57d521a8b..625a8c0df18 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -721,7 +721,7 @@ jit_reader_try_read_symtab (gdbarch *gdbarch, jit_code_entry *code_entry,
 			      code_entry->symfile_size))
 	status = 0;
     }
-  catch (const gdb_exception &e)
+  catch (const gdb_exception_error &e)
     {
       status = 0;
     }
diff --git a/gdb/mi/mi-cmd-break.c b/gdb/mi/mi-cmd-break.c
index 05eac3553ae..ac310f5e597 100644
--- a/gdb/mi/mi-cmd-break.c
+++ b/gdb/mi/mi-cmd-break.c
@@ -58,7 +58,7 @@ breakpoint_notify (struct breakpoint *b)
 	{
 	  print_breakpoint (b);
 	}
-      catch (const gdb_exception &ex)
+      catch (const gdb_exception_error &ex)
 	{
 	  exception_print (gdb_stderr, ex);
 	}
diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c
index 2c47024d5e6..d26194c87fd 100644
--- a/gdb/mi/mi-interp.c
+++ b/gdb/mi/mi-interp.c
@@ -856,7 +856,7 @@ mi_print_breakpoint_for_event (struct mi_interp *mi, breakpoint *bp)
 
       print_breakpoint (bp);
     }
-  catch (const gdb_exception &ex)
+  catch (const gdb_exception_error &ex)
     {
       exception_print (gdb_stderr, ex);
     }
diff --git a/gdb/objc-lang.c b/gdb/objc-lang.c
index 3bb9588ad48..1cffe2c4256 100644
--- a/gdb/objc-lang.c
+++ b/gdb/objc-lang.c
@@ -1279,7 +1279,7 @@ find_objc_msgcall_submethod (int (*f) (CORE_ADDR, CORE_ADDR *),
       if (f (pc, new_pc) == 0)
 	return 1;
     }
-  catch (const gdb_exception &ex)
+  catch (const gdb_exception_error &ex)
     {
       exception_fprintf (gdb_stderr, ex,
 			 "Unable to determine target of "
diff --git a/gdb/parse.c b/gdb/parse.c
index 23f0e66bf27..d495a77e567 100644
--- a/gdb/parse.c
+++ b/gdb/parse.c
@@ -510,7 +510,7 @@ parse_exp_in_context (const char **stringptr, CORE_ADDR pc,
     {
       lang->parser (&ps);
     }
-  catch (const gdb_exception &except)
+  catch (const gdb_exception_error &except)
     {
       /* If parsing for completion, allow this to succeed; but if no
 	 expression elements have been written, then there's nothing
diff --git a/gdb/printcmd.c b/gdb/printcmd.c
index 6f9be820b0c..ae3e865782b 100644
--- a/gdb/printcmd.c
+++ b/gdb/printcmd.c
@@ -2116,7 +2116,7 @@ do_one_display (struct display *d)
 	  d->exp = parse_expression (d->exp_string.c_str (), &tracker);
 	  d->block = tracker.block ();
 	}
-      catch (const gdb_exception &ex)
+      catch (const gdb_exception_error &ex)
 	{
 	  /* Can't re-parse the expression.  Disable this display item.  */
 	  d->enabled_p = false;
diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
index 3dfdf592dd5..f41f3e86428 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -2934,7 +2934,7 @@ cmd_record_btrace_start (const char *args, int from_tty)
     {
       execute_command ("target record-btrace", from_tty);
     }
-  catch (const gdb_exception &exception)
+  catch (const gdb_exception_error &exception)
     {
       record_btrace_conf.format = BTRACE_FORMAT_BTS;
 
diff --git a/gdb/record-full.c b/gdb/record-full.c
index bd8c49c1abe..94bca4b8ec8 100644
--- a/gdb/record-full.c
+++ b/gdb/record-full.c
@@ -784,7 +784,7 @@ record_full_message_wrapper_safe (struct regcache *regcache,
     {
       record_full_message (regcache, signal);
     }
-  catch (const gdb_exception &ex)
+  catch (const gdb_exception_error &ex)
     {
       exception_print (gdb_stderr, ex);
       return false;
diff --git a/gdb/solib.c b/gdb/solib.c
index b9b1d037187..9d1e02598ec 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -739,7 +739,7 @@ update_solib_list (int from_tty)
 	    {
 	      ops->open_symbol_file_object (from_tty);
 	    }
-	  catch (const gdb_exception &ex)
+	  catch (const gdb_exception_error &ex)
 	    {
 	      exception_fprintf (gdb_stderr, ex,
 				 "Error reading attached "
diff --git a/gdb/sparc64-linux-tdep.c b/gdb/sparc64-linux-tdep.c
index 9ea72331e93..23d00a4540d 100644
--- a/gdb/sparc64-linux-tdep.c
+++ b/gdb/sparc64-linux-tdep.c
@@ -139,7 +139,7 @@ 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 &exception)
+  catch (const gdb_exception_error &exception)
     {
       return;
     }
diff --git a/gdb/symfile-mem.c b/gdb/symfile-mem.c
index b4c359ccd31..e1dca07cef3 100644
--- a/gdb/symfile-mem.c
+++ b/gdb/symfile-mem.c
@@ -197,7 +197,7 @@ add_vsyscall_page (inferior *inf)
 				       name.c_str (),
 				       0 /* from_tty */);
 	}
-      catch (const gdb_exception &ex)
+      catch (const gdb_exception_error &ex)
 	{
 	  exception_print (gdb_stderr, ex);
 	}
-- 
2.35.1

[PATCH v3 4/7] Python QUIT processing updates

[Kevin Buettner]

See the previous patches in this series for the motivation behind
these changes.

This commit contains updates to Python's QUIT handling.  Ideally, we'd
like to throw gdb_exception_forced_quit through the extension
language; I made an attempt to do this for gdb_exception_quit in an
earlier version of this patch, but Pedro pointed out that it is
(almost certainly) not safe to do so.

Still, we definitely don't want to swallow the exception representing
a SIGTERM for GDB, nor do we want to force modules written in the
extension language to have to explicitly handle this case.  Since the
idea is for GDB to cleanup and quit for this exception, we'll simply
call quit_force() just as if the gdb_exception_forced_quit propagation
had managed to make it back to the top level.
---
 gdb/python/py-finishbreakpoint.c | 5 +++++
 gdb/python/py-gdb-readline.c     | 4 ++++
 gdb/python/py-symbol.c           | 5 +++++
 gdb/python/py-utils.c            | 3 +++
 gdb/python/py-value.c            | 5 +++++
 5 files changed, 22 insertions(+)

diff --git a/gdb/python/py-finishbreakpoint.c b/gdb/python/py-finishbreakpoint.c
index 083694fbcce..3e5adf45e06 100644
--- a/gdb/python/py-finishbreakpoint.c
+++ b/gdb/python/py-finishbreakpoint.c
@@ -20,6 +20,7 @@
 
 
 #include "defs.h"
+#include "top.h"		/* For quit_force().  */
 #include "python-internal.h"
 #include "breakpoint.h"
 #include "frame.h"
@@ -272,6 +273,10 @@ bpfinishpy_init (PyObject *self, PyObject *args, PyObject *kwargs)
 	    }
 	}
     }
+  catch (const gdb_exception_forced_quit &except)
+    {
+      quit_force (NULL, 0);
+    }
   catch (const gdb_exception &except)
     {
       /* Just swallow.  Either the return type or the function value
diff --git a/gdb/python/py-gdb-readline.c b/gdb/python/py-gdb-readline.c
index af388d5ed72..a112e6360a2 100644
--- a/gdb/python/py-gdb-readline.c
+++ b/gdb/python/py-gdb-readline.c
@@ -45,6 +45,10 @@ gdbpy_readline_wrapper (FILE *sys_stdin, FILE *sys_stdout,
       p = command_line_input (prompt, "python");
     }
   /* Handle errors by raising Python exceptions.  */
+  catch (const gdb_exception_forced_quit &e)
+    {
+      quit_force (NULL, 0);
+    }
   catch (const gdb_exception &except)
     {
       /* Detect user interrupt (Ctrl-C).  */
diff --git a/gdb/python/py-symbol.c b/gdb/python/py-symbol.c
index 819a51f7e41..c2d247590cb 100644
--- a/gdb/python/py-symbol.c
+++ b/gdb/python/py-symbol.c
@@ -18,6 +18,7 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #include "defs.h"
+#include "top.h"		/* For force_quit ().  */
 #include "block.h"
 #include "frame.h"
 #include "symtab.h"
@@ -501,6 +502,10 @@ gdbpy_lookup_static_symbol (PyObject *self, PyObject *args, PyObject *kw)
 	= get_selected_frame (_("No frame selected."));
       block = get_frame_block (selected_frame, NULL);
     }
+  catch (const gdb_exception_forced_quit &e)
+    {
+      quit_force (NULL, 0);
+    }
   catch (const gdb_exception &except)
     {
       /* Nothing.  */
diff --git a/gdb/python/py-utils.c b/gdb/python/py-utils.c
index 73c860bcc96..2bb452ed340 100644
--- a/gdb/python/py-utils.c
+++ b/gdb/python/py-utils.c
@@ -18,6 +18,7 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #include "defs.h"
+#include "top.h"		/* For quit_force ().  */
 #include "charset.h"
 #include "value.h"
 #include "python-internal.h"
@@ -237,6 +238,8 @@ gdbpy_convert_exception (const struct gdb_exception &exception)
 
   if (exception.reason == RETURN_QUIT)
     exc_class = PyExc_KeyboardInterrupt;
+  else if (exception.reason == RETURN_FORCED_QUIT)
+    quit_force (NULL, 0);
   else if (exception.error == MEMORY_ERROR)
     exc_class = gdbpy_gdb_memory_error;
   else
diff --git a/gdb/python/py-value.c b/gdb/python/py-value.c
index b546344da95..0a4b44c3b65 100644
--- a/gdb/python/py-value.c
+++ b/gdb/python/py-value.c
@@ -18,6 +18,7 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #include "defs.h"
+#include "top.h"		/* For quit_force ().  */
 #include "charset.h"
 #include "value.h"
 #include "language.h"
@@ -371,6 +372,10 @@ valpy_get_address (PyObject *self, void *closure)
 	  res_val = value_addr (val_obj->value);
 	  val_obj->address = value_to_value_object (res_val);
 	}
+      catch (const gdb_exception_forced_quit &except)
+	{
+	  quit_force (NULL, 0);
+	}
       catch (const gdb_exception &except)
 	{
 	  val_obj->address = Py_None;
-- 
2.35.1

[PATCH v3 5/7] Guile QUIT processing updates

[Kevin Buettner]

This commit contains QUIT processing updates for GDB's Guile support.
As with the Python updates, we don't want to permit this code to
swallow the exception, gdb_exception_forced_quit, which is associated
with GDB receiving a SIGTERM.

I've adopted the same solution that I used for Python; whereever
a gdb_exception is caught in try/catch code in the Guile extension
language support, a catch for gdb_exception_forced_quit has been
added; this catch block will simply call quit_force(), which will
cause the necessary cleanups to occur followed by GDB exiting.
---
 gdb/guile/guile-internal.h   | 5 +++++
 gdb/guile/scm-pretty-print.c | 5 +++++
 gdb/guile/scm-type.c         | 5 +++++
 gdb/guile/scm-value.c        | 5 +++++
 gdb/top.h                    | 2 +-
 5 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/gdb/guile/guile-internal.h b/gdb/guile/guile-internal.h
index 28e4889bfa9..965a965b1a7 100644
--- a/gdb/guile/guile-internal.h
+++ b/gdb/guile/guile-internal.h
@@ -28,6 +28,7 @@
 #include "extension-priv.h"
 #include "symtab.h"
 #include "libguile.h"
+#include "top.h"		/* For quit_force().  */
 
 struct block;
 struct frame_info;
@@ -712,6 +713,10 @@ gdbscm_wrap (Function &&func, Args &&... args)
     {
       result = func (std::forward<Args> (args)...);
     }
+  catch (const gdb_exception_forced_quit &e)
+    {
+      quit_force (NULL, 0);
+    }
   catch (const gdb_exception &except)
     {
       exc = unpack (except);
diff --git a/gdb/guile/scm-pretty-print.c b/gdb/guile/scm-pretty-print.c
index c64e8346938..8eded5b6176 100644
--- a/gdb/guile/scm-pretty-print.c
+++ b/gdb/guile/scm-pretty-print.c
@@ -21,6 +21,7 @@
    conventions, et.al.  */
 
 #include "defs.h"
+#include "top.h"		/* For quit_force().  */
 #include "charset.h"
 #include "symtab.h" /* Needed by language.h.  */
 #include "language.h"
@@ -558,6 +559,10 @@ ppscm_pretty_print_one_value (SCM printer, struct value **out_value,
 	    (_("invalid result from pretty-printer to-string"), result);
 	}
     }
+  catch (const gdb_exception_forced_quit &except)
+    {
+      quit_force (NULL, 0);
+    }
   catch (const gdb_exception &except)
     {
     }
diff --git a/gdb/guile/scm-type.c b/gdb/guile/scm-type.c
index 27d00f12a95..3e6a36cf6cd 100644
--- a/gdb/guile/scm-type.c
+++ b/gdb/guile/scm-type.c
@@ -21,6 +21,7 @@
    conventions, et.al.  */
 
 #include "defs.h"
+#include "top.h"		/* For quit_force().  */
 #include "arch-utils.h"
 #include "value.h"
 #include "gdbtypes.h"
@@ -112,6 +113,10 @@ tyscm_type_name (struct type *type)
 				    &type_print_raw_options);
       return stb.release ();
     }
+  catch (const gdb_exception_forced_quit &except)
+    {
+      quit_force (NULL, 0);
+    }
   catch (const gdb_exception &except)
     {
       excp = gdbscm_scm_from_gdb_exception (unpack (except));
diff --git a/gdb/guile/scm-value.c b/gdb/guile/scm-value.c
index 1c88a3add34..6c42a3dc967 100644
--- a/gdb/guile/scm-value.c
+++ b/gdb/guile/scm-value.c
@@ -21,6 +21,7 @@
    conventions, et.al.  */
 
 #include "defs.h"
+#include "top.h"		/* For quit_force().  */
 #include "arch-utils.h"
 #include "charset.h"
 #include "cp-abi.h"
@@ -416,6 +417,10 @@ gdbscm_value_address (SCM self)
 	    {
 	      address = vlscm_scm_from_value (value_addr (value));
 	    }
+	  catch (const gdb_exception_forced_quit &except)
+	    {
+	      quit_force (NULL, 0);
+	    }
 	  catch (const gdb_exception &except)
 	    {
 	    }
diff --git a/gdb/top.h b/gdb/top.h
index b26209e8c8d..3ea31a3858a 100644
--- a/gdb/top.h
+++ b/gdb/top.h
@@ -234,7 +234,7 @@ extern void read_command_file (FILE *);
 extern void init_history (void);
 extern void command_loop (void);
 extern int quit_confirm (void);
-extern void quit_force (int *, int);
+extern void quit_force (int *, int) ATTRIBUTE_NORETURN;
 extern void quit_command (const char *, int);
 extern void quit_cover (void);
 extern void execute_command (const char *, int);
-- 
2.35.1

[PATCH v3 6/7] QUIT processing w/ explicit throw for gdb_exception_forced_quit

[Kevin Buettner]

This commit contains changes which have an explicit throw for
gdb_exception_forced_quit, or, in a couple of cases for gdb_exception,
but with a throw following a check to see if 'reason' is
RETURN_FORCED_QUIT.

Most of these are straightforward - it made sense to continue to allow
an existing catch of gdb_exception to also catch gdb_exception_quit;
in these cases, a catch/throw for gdb_exception_forced_quit was added.

There are two cases, however, which deserve a more detailed
explanation.

1) remote_fileio_request in gdb/remote-fileio.c:

The try block calls do_remote_fileio_request which can (in turn)
call one of the functions in remote_fio_func_map[].  Taking the
first one, remote_fileio_func_open(), we have the following call
path to maybe_quit():

  remote_fileio_func_open(remote_target*, char*)
    -> target_read_memory(unsigned long, unsigned char*, long)
    -> target_read(target_ops*, target_object, char const*, unsigned char*, unsigned long, long)
    -> maybe_quit()

Since there is a path to maybe_quit(), we must ensure that the
catch block is not permitted to swallow a QUIT representing a
SIGTERM.

However, for this case, we must take care not to change the way that
Ctrl-C / SIGINT is handled; we want to send a suitable EINTR reply to
the remote target should that happen.  That being the case, I added a
catch/throw for gdb_exception_forced_quit.  I also did a bit of
rewriting here, adding a catch for gdb_exception_quit in favor of
checking the 'reason' code in the catch block for gdb_exception.

2) mi_execute_command in gdb/mi/mi-main.c:

The try block calls captured_mi_execute_command(); there exists
a call path to maybe_quit():

  captured_mi_execute_command(ui_out*, mi_parse*)
    -> mi_cmd_execute(mi_parse*)
    -> get_current_frame()
    -> get_prev_frame_always_1(frame_info*)
    -> frame_register_unwind_location(frame_info*, int, int*, lval_type*, unsigned long*, int*)
    -> frame_register_unwind(frame_info*, int, int*, int*, lval_type*, unsigned long*, int*, unsigned char*)
    -> value_entirely_available(value*)
    -> value_fetch_lazy(value*)
    -> value_fetch_lazy_memory(value*)
    -> read_value_memory(value*, long, int, unsigned long, unsigned char*, unsigned long)
    -> maybe_quit()

That being the case, we can't allow the exception handler (catch block)
to swallow a gdb_exception_quit for SIGTERM.  However, it does seem
reasonable to output the exception via the mi interface so that some
suitable message regarding SIGTERM might be printed; therefore, I
check the exception's 'reason' field for RETURN_FORCED_QUIT and
do a throw for this case.
---
 gdb/event-top.c     |  2 ++
 gdb/mi/mi-main.c    |  4 ++++
 gdb/remote-fileio.c | 15 ++++++++++-----
 gdb/tui/tui-io.c    |  4 ++++
 gdb/tui/tui.c       |  4 ++++
 gdb/windows-nat.c   |  4 ++++
 6 files changed, 28 insertions(+), 5 deletions(-)

diff --git a/gdb/event-top.c b/gdb/event-top.c
index 8890c4fae2d..bef70ccbcdc 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -1224,6 +1224,8 @@ async_disconnect (gdb_client_data arg)
       fputs_filtered ("Could not kill the program being debugged",
 		      gdb_stderr);
       exception_print (gdb_stderr, exception);
+      if (exception.reason == RETURN_FORCED_QUIT)
+        throw;
     }
 
   for (inferior *inf : all_inferiors ())
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 4860da7536a..b74a1e2034f 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1959,6 +1959,10 @@ mi_execute_command (const char *cmd, int from_tty)
 	     somewhere.  */
 	  mi_print_exception (command->token, result);
 	  mi_out_rewind (current_uiout);
+
+	  /* Throw to a higher level catch for SIGTERM sent to GDB.  */
+	  if (result.reason == RETURN_FORCED_QUIT)
+	    throw;
 	}
 
       bpstat_do_actions ();
diff --git a/gdb/remote-fileio.c b/gdb/remote-fileio.c
index 9963f1ebc01..1a0d9452d6f 100644
--- a/gdb/remote-fileio.c
+++ b/gdb/remote-fileio.c
@@ -1191,12 +1191,17 @@ remote_fileio_request (remote_target *remote, char *buf, int ctrlc_pending_p)
 	{
 	  do_remote_fileio_request (remote, buf);
 	}
-      catch (const gdb_exception &ex)
+      catch (const gdb_exception_forced_quit &ex)
+        {
+	  throw;
+	}
+      catch (const gdb_exception_quit &ex)
 	{
-	  if (ex.reason == RETURN_QUIT)
-	    remote_fileio_reply (remote, -1, FILEIO_EINTR);
-	  else
-	    remote_fileio_reply (remote, -1, FILEIO_EIO);
+	  remote_fileio_reply (remote, -1, FILEIO_EINTR);
+	}
+      catch (const gdb_exception &ex)
+        {
+	  remote_fileio_reply (remote, -1, FILEIO_EIO);
 	}
     }
 
diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c
index 8cac1c40f13..4f6f7513142 100644
--- a/gdb/tui/tui-io.c
+++ b/gdb/tui/tui-io.c
@@ -1259,6 +1259,10 @@ tui_getc (FILE *fp)
     {
       return tui_getc_1 (fp);
     }
+  catch (const gdb_exception_forced_quit &ex)
+    {
+      throw;
+    }
   catch (const gdb_exception &ex)
     {
       /* Just in case, don't ever let an exception escape to readline.
diff --git a/gdb/tui/tui.c b/gdb/tui/tui.c
index 776dccf0bb2..bba493fcdb8 100644
--- a/gdb/tui/tui.c
+++ b/gdb/tui/tui.c
@@ -109,6 +109,10 @@ tui_rl_switch_mode (int notused1, int notused2)
 	  tui_enable ();
 	}
     }
+  catch (const gdb_exception_forced_quit &ex)
+    {
+      throw;
+    }
   catch (const gdb_exception &ex)
     {
       exception_print (gdb_stderr, ex);
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 81e26fe4759..0f77bc94705 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -825,6 +825,10 @@ catch_errors (void (*func) ())
     {
       func ();
     }
+  catch (const gdb_exception_forced_quit &ex)
+    {
+      throw;
+    }
   catch (const gdb_exception &ex)
     {
       exception_print (gdb_stderr, ex);
-- 
2.35.1

[PATCH v3 7/7] Handle QUIT processing in the scoped_switch_fork_info destructor

[Kevin Buettner]

During my audit of the use of gdb_exception with regard to QUIT
processing, I found a try/catch in the scoped_switch_fork_info
destructor.

Static analysis found this call path from the destructor to
maybe_quit():

  scoped_switch_fork_info::~scoped_switch_fork_info()
    -> remove_breakpoints()
    -> remove_breakpoint(bp_location*)
    -> remove_breakpoint_1(bp_location*, remove_bp_reason)
    -> memory_validate_breakpoint(gdbarch*, bp_target_info*)
    -> target_read_memory(unsigned long, unsigned char*, long)
    -> target_read(target_ops*, target_object, char const*, unsigned char*, unsigned long, long)
    -> maybe_quit()

Since it's not safe to do a 'throw' from a destructor, we simply
call set_quit_flag and, for gdb_exception_forced_quit, also
set sync_quit_force_run.  This will cause the appropriate
exception to be rethrown at the next QUIT check.

Thanks to Pedro Alves for suggesting this idea.
---
 gdb/linux-fork.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/gdb/linux-fork.c b/gdb/linux-fork.c
index 4baab3c80c7..b8adb7396ba 100644
--- a/gdb/linux-fork.c
+++ b/gdb/linux-fork.c
@@ -430,6 +430,19 @@ class scoped_switch_fork_info
 	    fork_load_infrun_state (m_oldfp);
 	    insert_breakpoints ();
 	  }
+	catch (const gdb_exception_quit &ex)
+	  {
+	    /* We can't throw from a destructor, so re-set the quit flag
+	      for later QUIT checking.  */
+	    set_quit_flag ();
+	  }
+	catch (const gdb_exception_forced_quit &ex)
+	  {
+	    /* Like above, but (eventually) cause GDB to terminate by
+	       setting sync_quit_force_run.  */
+	    sync_quit_force_run = 1;
+	    set_quit_flag ();
+	  }
 	catch (const gdb_exception &ex)
 	  {
 	    warning (_("Couldn't restore checkpoint state in %s: %s"),
-- 
2.35.1

Re: [PATCH v3 3/7] Catch gdb_exception_error instead of gdb_exception (in many places)

[Pedro Alves]

Hi Kevin,

I've started giving the series an initial skim, but haven't read it all yet.  Meanwhile,
here's one spot I noticed isn't correct.

On 2022-02-27 00:00, Kevin Buettner wrote:
> --- a/gdb/cli/cli-interp.c
> +++ b/gdb/cli/cli-interp.c
> @@ -368,7 +368,7 @@ safe_execute_command (struct ui_out *command_uiout, const char *command,
>      {
>        execute_command (command, from_tty);
>      }
> -  catch (gdb_exception &exception)
> +  catch (gdb_exception_error &exception)
>      {
>        e = std::move (exception);
>      }

This one here, as is, can leave things in a messed up state.  Note that the function returns
the exception.  And then the caller does this:

gdb_exception
cli_interp::exec (const char *command_str)
{
  struct cli_interp *cli = this;
  struct ui_file *old_stream;
  struct gdb_exception result;

  /* gdb_stdout could change between the time cli_uiout was
     initialized and now.  Since we're probably using a different
     interpreter which has a new ui_file for gdb_stdout, use that one
     instead of the default.

     It is important that it gets reset everytime, since the user
     could set gdb to use a different interpreter.  */
  old_stream = cli->cli_uiout->set_stream (gdb_stdout);
  result = safe_execute_command (cli->cli_uiout, command_str, 1);
  cli->cli_uiout->set_stream (old_stream);       <<<<<<<<<<<< 
  return result;
}

Note the "<<<<" line.  If a QUIT exception propagates, the line doesn't execute.

Looking at the caller higher up the in chain, we get to interpreter_exec_cmd,
which does:

  for (i = 1; i < nrules; i++)
    {
      struct gdb_exception e = interp_exec (interp_to_use, prules[i]);

      if (e.reason < 0)
	{
	  interp_set (old_interp, 0);
	  error (_("error in command: \"%s\"."), prules[i]);
	}
    }

That throws plain error, so it won't be sufficient to just revert your hunk, as that
would convert a quit to a plain error.  I don't know off hand why
we need to have safe_execute_command and interp_exec catch the exception and
return it via the normal return path.  That's normally needed when we need to
cross C code, like readline or ncurses, but that's not the case here.  Maybe
we can just eliminate safe_execute_command and let exceptions propagate normally.

Re: [PATCH v3 6/7] QUIT processing w/ explicit throw for gdb_exception_forced_quit

[Pedro Alves]

On 2022-02-27 00:00, Kevin Buettner wrote:
> --- a/gdb/tui/tui-io.c
> +++ b/gdb/tui/tui-io.c
> @@ -1259,6 +1259,10 @@ tui_getc (FILE *fp)
>      {
>        return tui_getc_1 (fp);
>      }
> +  catch (const gdb_exception_forced_quit &ex)
> +    {
> +      throw;
> +    }
>    catch (const gdb_exception &ex)
>      {
>        /* Just in case, don't ever let an exception escape to readline.

This one's incorrect.  Note the comment in the context above, and also the comment
at the top of the function:

/* Get a character from the command window.  This is called from the
   readline package.  */

static int
tui_getc (FILE *fp)
{
  try
    {
      return tui_getc_1 (fp);
    }
  catch (const gdb_exception &ex)
    {
      /* Just in case, don't ever let an exception escape to readline.
	 This shouldn't ever happen, but if it does, print the
	 exception instead of just crashing GDB.  */
      exception_print (gdb_stderr, ex);

      /* If we threw an exception, it's because we recognized the
	 character.  */
      return 0;
    }
}


we really must not let a C++ exception propagate out to readline.  It will kill gdb
in configurations/architectures that don't default to -fasynchronous-unwind-tables.

As the comment says, this try/catch here is really just in case, I don't
think it is reachable.  But if we want to make it right for quits too,
just in case, then I think we instead need to swallow the exception, and call
set_quit_flag() / set sync_quit_force_run.



I'll continue auditing, and may come back to the same patches...

Re: [PATCH v3 0/7] glibc-2.34: Fix gdb.base/gdb-sigterm.exp failure/error

[Simon Marchi]

On 2022-02-26 19:00, Kevin Buettner via Gdb-patches wrote:
> This series addresses Pedro's concerns regarding the v2 series posted
> last year:
> 
> https://sourceware.org/pipermail/gdb-patches/2021-August/181575.html
> 
> In particular, it introduces a new exception which will be thrown
> when GDB receives a SIGTERM.  That exception is handled at the
> top level when possible, though for the extension languages, Python
> and Guile, a somewhat different approach had to be taken.  In order to
> ensure that this new exception isn't inadvertently swallowed on its
> way up to the top level, I also did an audit of the try / catch code
> involving gdb_exception, making adjustments where necessary.

I started to look at a gdb.base/gdb-sigterm.exp failure, searched the
list for mentions of that test, and found this series which I think
addresses exactly the failure I am seeing.  I'm just mentionning this to
bump the thread :).

Simon

Re: [PATCH v3 0/7] glibc-2.34: Fix gdb.base/gdb-sigterm.exp failure/error

[Tom de Vries]

On 7/20/22 03:53, Simon Marchi via Gdb-patches wrote:
> 
> 
> On 2022-02-26 19:00, Kevin Buettner via Gdb-patches wrote:
>> This series addresses Pedro's concerns regarding the v2 series posted
>> last year:
>>
>> https://sourceware.org/pipermail/gdb-patches/2021-August/181575.html
>>
>> In particular, it introduces a new exception which will be thrown
>> when GDB receives a SIGTERM.  That exception is handled at the
>> top level when possible, though for the extension languages, Python
>> and Guile, a somewhat different approach had to be taken.  In order to
>> ensure that this new exception isn't inadvertently swallowed on its
>> way up to the top level, I also did an audit of the try / catch code
>> involving gdb_exception, making adjustments where necessary.
> 
> I started to look at a gdb.base/gdb-sigterm.exp failure, searched the
> list for mentions of that test, and found this series which I think
> addresses exactly the failure I am seeing.  I'm just mentioning this to
> bump the thread :).

Hi,

likewise bumping the thread, and noting that the series can claim to fix 
PR https://sourceware.org/bugzilla/show_bug.cgi?id=26761 .

Thanks,
- Tom

Re: [PATCH v3 0/7] glibc-2.34: Fix gdb.base/gdb-sigterm.exp failure/error

[Pedro Alves]

Hi,

On 2023-01-05 1:35 p.m., Tom de Vries wrote:
> On 7/20/22 03:53, Simon Marchi via Gdb-patches wrote:
>>
>>
>> On 2022-02-26 19:00, Kevin Buettner via Gdb-patches wrote:
>>> This series addresses Pedro's concerns regarding the v2 series posted
>>> last year:
>>>
>>> https://sourceware.org/pipermail/gdb-patches/2021-August/181575.html
>>>
>>> In particular, it introduces a new exception which will be thrown
>>> when GDB receives a SIGTERM.  That exception is handled at the
>>> top level when possible, though for the extension languages, Python
>>> and Guile, a somewhat different approach had to be taken.  In order to
>>> ensure that this new exception isn't inadvertently swallowed on its
>>> way up to the top level, I also did an audit of the try / catch code
>>> involving gdb_exception, making adjustments where necessary.
>>
>> I started to look at a gdb.base/gdb-sigterm.exp failure, searched the
>> list for mentions of that test, and found this series which I think
>> addresses exactly the failure I am seeing.  I'm just mentioning this to
>> bump the thread :).
> 
> Hi,
> 
> likewise bumping the thread, and noting that the series can claim to fix PR https://sourceware.org/bugzilla/show_bug.cgi?id=26761 .
> 

I'm taking a look at this one.

> Thanks,
> - Tom