public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v4 0/8] Fix gdb.base/gdb-sigterm.exp failure/error
@ 2023-01-12  1:56 Kevin Buettner
  2023-01-12  1:56 ` [PATCH v4 1/8] Introduce gdb_exception_forced_quit Kevin Buettner
                   ` (8 more replies)
  0 siblings, 9 replies; 21+ messages in thread
From: Kevin Buettner @ 2023-01-12  1:56 UTC (permalink / raw)
  To: gdb-patches; +Cc: pedro, simark, tdevries, Kevin Buettner

This series fixes the failure in gdb.base/gdb-sigterm.exp when
running in environments with glibc-2.34 or later.

It addresses Pedro's concerns regarding the v3 series:

https://sourceware.org/pipermail/gdb-patches/2022-February/186150.html

Patches 3, 6, and 7 from the earlier series has changed as follows:

Patch 3 / "Catch gdb_exception_error instead of gdb_exception (in many
places)" no longer contains a change for safe_execute_command().  This
is a tricky case; I've reworked it and moved it into its own commit
titled "Call quit_force for gdb_exception_forced_quit in
safe_execute_command".

Patch 6 / "QUIT processing w/ explicit throw for
gdb_exception_forced_quit" no longer contains changes to
gdb/tui/tui-io.c.  These changes have been moved to patch 8 (which
was #7 in the v3 series) since the technique used for circumventing
the problem with not being able to do a throw are the same for both.

Patch 7 (from v3) / "Handle QUIT processing in the scoped_switch_fork_info
destructor" is now patch 8 in v4.  It's been retitled as "Forced quit cases
handled by resetting sync_quit_force_run"  It contains changes for the
destructor as well as tui_getc().

Patch 6 (v4) / "Call quit_force for gdb_exception_forced_quit in
safe_execute_command" is a new patch containing a rework of changes
to safe_execute_command that was previously in Patch 3.

Kevin Buettner (8):
  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
  Call quit_force for gdb_exception_forced_quit in safe_execute_command
  QUIT processing w/ explicit throw for gdb_exception_forced_quit
  Forced quit cases handled by resetting sync_quit_force_run

 gdb/ada-lang.c                   |  2 +-
 gdb/breakpoint.c                 |  8 ++++----
 gdb/cli/cli-interp.c             | 11 +++++++++++
 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                 |  9 +++++++++
 gdb/tui/tui.c                    |  4 ++++
 gdb/utils.c                      |  2 +-
 gdbsupport/common-exceptions.cc  | 14 ++++++++++++++
 gdbsupport/common-exceptions.h   | 22 +++++++++++++++++++++-
 38 files changed, 164 insertions(+), 28 deletions(-)

-- 
2.34.3


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v4 1/8] Introduce gdb_exception_forced_quit
  2023-01-12  1:56 [PATCH v4 0/8] Fix gdb.base/gdb-sigterm.exp failure/error Kevin Buettner
@ 2023-01-12  1:56 ` Kevin Buettner
  2023-01-30 18:56   ` Pedro Alves
  2023-01-12  1:56 ` [PATCH v4 2/8] Handle gdb SIGTERM by throwing / catching gdb_exception_force_quit Kevin Buettner
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 21+ messages in thread
From: Kevin Buettner @ 2023-01-12  1:56 UTC (permalink / raw)
  To: gdb-patches; +Cc: pedro, simark, tdevries, 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.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=26761
---
 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 d0dd8081c41..edc1b56181a 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 a2a4f5a3892..f9b59ece736 100644
--- a/gdbsupport/common-exceptions.h
+++ b/gdbsupport/common-exceptions.h
@@ -32,6 +32,8 @@
 
 enum return_reason
   {
+    /* SIGTERM sent to GDB.  */
+    RETURN_FORCED_QUIT = -3,
     /* User interrupt.  */
     RETURN_QUIT = -2,
     /* Any other error.  */
@@ -42,9 +44,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.  */
@@ -294,6 +297,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
@@ -336,5 +354,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.34.3


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v4 2/8] Handle gdb SIGTERM by throwing / catching gdb_exception_force_quit
  2023-01-12  1:56 [PATCH v4 0/8] Fix gdb.base/gdb-sigterm.exp failure/error Kevin Buettner
  2023-01-12  1:56 ` [PATCH v4 1/8] Introduce gdb_exception_forced_quit Kevin Buettner
@ 2023-01-12  1:56 ` Kevin Buettner
  2023-01-30 18:57   ` Pedro Alves
  2023-01-12  1:56 ` [PATCH v4 3/8] Catch gdb_exception_error instead of gdb_exception (in many places) Kevin Buettner
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 21+ messages in thread
From: Kevin Buettner @ 2023-01-12  1:56 UTC (permalink / raw)
  To: gdb-patches; +Cc: pedro, simark, tdevries, 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.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=26761
---
 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 c04d37a45f9..323d99321c7 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -410,6 +410,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);
@@ -518,6 +522,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);
@@ -1309,6 +1317,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 734c5bf7f70..0e126cf103b 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -673,7 +673,7 @@ quit (void)
   if (sync_quit_force_run)
     {
       sync_quit_force_run = 0;
-      quit_force (NULL, 0);
+      throw_forced_quit ("SIGTERM");
     }
 
 #ifdef __MSDOS__
-- 
2.34.3


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v4 3/8] Catch gdb_exception_error instead of gdb_exception (in many places)
  2023-01-12  1:56 [PATCH v4 0/8] Fix gdb.base/gdb-sigterm.exp failure/error Kevin Buettner
  2023-01-12  1:56 ` [PATCH v4 1/8] Introduce gdb_exception_forced_quit Kevin Buettner
  2023-01-12  1:56 ` [PATCH v4 2/8] Handle gdb SIGTERM by throwing / catching gdb_exception_force_quit Kevin Buettner
@ 2023-01-12  1:56 ` Kevin Buettner
  2023-01-30 19:00   ` Pedro Alves
  2023-01-12  1:56 ` [PATCH v4 4/8] Python QUIT processing updates Kevin Buettner
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 21+ messages in thread
From: Kevin Buettner @ 2023-01-12  1:56 UTC (permalink / raw)
  To: gdb-patches; +Cc: pedro, simark, tdevries, 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.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=26761
---
 gdb/ada-lang.c           | 2 +-
 gdb/breakpoint.c         | 8 ++++----
 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 +-
 17 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 40f85914a56..241dd1b9f24 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -12320,7 +12320,7 @@ should_stop_exception (const struct bp_location *bl)
       scoped_value_mark mark;
       stop = value_true (evaluate_expression (ada_loc->excep_cond_expr.get ()));
     }
-  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 8cfc46e0bed..af8b62e4880 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -2833,7 +2833,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)
 		{
 		  rethrow_on_target_close_error (e);
 		  bp_excpt = std::move (e);
@@ -5295,7 +5295,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 "
@@ -5539,7 +5539,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");
@@ -13476,7 +13476,7 @@ enable_breakpoint_disp (struct breakpoint *bpt, enum bpdisp disposition,
 	  bpt->enable_state = bp_enabled;
 	  update_watchpoint (w, true /* 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/i386-linux-tdep.c b/gdb/i386-linux-tdep.c
index ad0377de5a6..a6adeca1b97 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 f49d1c3c872..b9f25008247 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 0497ad05091..895d84025cf 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -1588,7 +1588,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 181d961d80d..9cfafda5ace 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -8730,7 +8730,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 48bfef0a026..7f6b289cdd3 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -715,7 +715,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 8b0483803b6..75957b75bad 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 cf3d738edce..ba58044f868 100644
--- a/gdb/mi/mi-interp.c
+++ b/gdb/mi/mi-interp.c
@@ -847,7 +847,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 e17a4c406c0..054895c02e3 100644
--- a/gdb/objc-lang.c
+++ b/gdb/objc-lang.c
@@ -1286,7 +1286,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 2f7d58061ab..9aa080ff974 100644
--- a/gdb/parse.c
+++ b/gdb/parse.c
@@ -514,7 +514,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 c3c2e5ad612..fe26771abe4 100644
--- a/gdb/printcmd.c
+++ b/gdb/printcmd.c
@@ -2109,7 +2109,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 0daba893813..61de8491bb9 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -2932,7 +2932,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 e3cfa3f45d5..15c5b7d682e 100644
--- a/gdb/record-full.c
+++ b/gdb/record-full.c
@@ -785,7 +785,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 60bdf46cb91..ec0fcfb5beb 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -785,7 +785,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 beff812eeef..685df066f3a 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 d36b9090e7e..7ac3ac709ec 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.34.3


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v4 4/8] Python QUIT processing updates
  2023-01-12  1:56 [PATCH v4 0/8] Fix gdb.base/gdb-sigterm.exp failure/error Kevin Buettner
                   ` (2 preceding siblings ...)
  2023-01-12  1:56 ` [PATCH v4 3/8] Catch gdb_exception_error instead of gdb_exception (in many places) Kevin Buettner
@ 2023-01-12  1:56 ` Kevin Buettner
  2023-01-30 19:01   ` Pedro Alves
  2023-01-12  1:56 ` [PATCH v4 5/8] Guile " Kevin Buettner
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 21+ messages in thread
From: Kevin Buettner @ 2023-01-12  1:56 UTC (permalink / raw)
  To: gdb-patches; +Cc: pedro, simark, tdevries, 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.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=26761
---
 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 d4d129110e9..1a224b35779 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"
@@ -271,6 +272,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 ea0f78c9ad8..b9294ad9afc 100644
--- a/gdb/python/py-gdb-readline.c
+++ b/gdb/python/py-gdb-readline.c
@@ -46,6 +46,10 @@ gdbpy_readline_wrapper (FILE *sys_stdin, FILE *sys_stdout,
       p = command_line_input (buffer, 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 b8777966c47..899b0787582 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"
@@ -515,6 +516,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 624b90a827f..d5b07a80d82 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"
@@ -219,6 +220,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 dcc92e51b60..9473468035b 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.34.3


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v4 5/8] Guile QUIT processing updates
  2023-01-12  1:56 [PATCH v4 0/8] Fix gdb.base/gdb-sigterm.exp failure/error Kevin Buettner
                   ` (3 preceding siblings ...)
  2023-01-12  1:56 ` [PATCH v4 4/8] Python QUIT processing updates Kevin Buettner
@ 2023-01-12  1:56 ` Kevin Buettner
  2023-01-12  1:56 ` [PATCH v4 6/8] Call quit_force for gdb_exception_forced_quit in safe_execute_command Kevin Buettner
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 21+ messages in thread
From: Kevin Buettner @ 2023-01-12  1:56 UTC (permalink / raw)
  To: gdb-patches; +Cc: pedro, simark, tdevries, 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.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=26761
---
 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 42ecb3c24f9..b04ef17a5ad 100644
--- a/gdb/guile/guile-internal.h
+++ b/gdb/guile/guile-internal.h
@@ -29,6 +29,7 @@
 #include "symtab.h"
 #include "libguile.h"
 #include "objfiles.h"
+#include "top.h"		/* For quit_force().  */
 
 struct block;
 struct frame_info;
@@ -704,6 +705,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 cb6677c8831..c618101bad8 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 da16d22990c..008e792cc34 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"
@@ -132,6 +133,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 b9948303bc1..0b92b211df7 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 9f9a0281be4..5a1352ef81c 100644
--- a/gdb/top.h
+++ b/gdb/top.h
@@ -236,7 +236,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.34.3


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v4 6/8] Call quit_force for gdb_exception_forced_quit in safe_execute_command
  2023-01-12  1:56 [PATCH v4 0/8] Fix gdb.base/gdb-sigterm.exp failure/error Kevin Buettner
                   ` (4 preceding siblings ...)
  2023-01-12  1:56 ` [PATCH v4 5/8] Guile " Kevin Buettner
@ 2023-01-12  1:56 ` Kevin Buettner
  2023-01-30 19:01   ` Pedro Alves
  2023-01-12  1:56 ` [PATCH v4 7/8] QUIT processing w/ explicit throw for gdb_exception_forced_quit Kevin Buettner
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 21+ messages in thread
From: Kevin Buettner @ 2023-01-12  1:56 UTC (permalink / raw)
  To: gdb-patches; +Cc: pedro, simark, tdevries, Kevin Buettner

In gdb/cli/cli-interp.c, safe_execute_command catches gdb_exception.
In the previous series, I had changed it to instead catch
gdb_exception_error, which would allow gdb_exception_quit and
gdb_exception_forced_quit to not be caught, thus propagating as
normal.

Pedro found some problems with doing this for safe_execute_command.
See:

https://sourceware.org/pipermail/gdb-patches/2022-March/186320.html

Pedro suggested: "Maybe we can just eliminate safe_execute_command and
let exceptions propagate normally."

I'm not doing that here, though I think it might be worth trying.

Instead, what I've done is to catch gdb_exception_forced_quit and,
when caught, call quit_force() thus forcing GDB to terminate.

After (re)reading Pedro's remarks, I think there's still a problem
with this area of the code (in which gdb_exception_quit is turned into
an error in interpreter_exec_cmd), but that problem existed before this
patch series and I think that fully addressing it should be the
subject of a different set of patches.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=26761
---
 gdb/cli/cli-interp.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/gdb/cli/cli-interp.c b/gdb/cli/cli-interp.c
index 5f2ff726f26..066e55b64f1 100644
--- a/gdb/cli/cli-interp.c
+++ b/gdb/cli/cli-interp.c
@@ -353,6 +353,17 @@ safe_execute_command (struct ui_out *command_uiout, const char *command,
     {
       execute_command (command, from_tty);
     }
+  catch (gdb_exception_forced_quit &exception)
+    {
+      /* Due to the way that the cli_inter::exec is structured, it is
+	 not safe to only catch gdb_exception_error below, thus
+	 allowing quit exceptions to propagate through.  (The CLI
+	 stream won't be reset as it should be.) So, for
+	 gdb_exception_forced_quit, which corresponds to GDB having
+	 received a SIGTERM signal, call quit_force() here which will
+	 cause GDB to terminate.  */
+      quit_force (NULL, 0);
+    }
   catch (gdb_exception &exception)
     {
       e = std::move (exception);
-- 
2.34.3


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v4 7/8] QUIT processing w/ explicit throw for gdb_exception_forced_quit
  2023-01-12  1:56 [PATCH v4 0/8] Fix gdb.base/gdb-sigterm.exp failure/error Kevin Buettner
                   ` (5 preceding siblings ...)
  2023-01-12  1:56 ` [PATCH v4 6/8] Call quit_force for gdb_exception_forced_quit in safe_execute_command Kevin Buettner
@ 2023-01-12  1:56 ` Kevin Buettner
  2023-01-30 19:02   ` Pedro Alves
  2023-01-12  1:56 ` [PATCH v4 8/8] Forced quit cases handled by resetting sync_quit_force_run Kevin Buettner
  2023-01-12 12:37 ` [PATCH v4 0/8] Fix gdb.base/gdb-sigterm.exp failure/error Tom de Vries
  8 siblings, 1 reply; 21+ messages in thread
From: Kevin Buettner @ 2023-01-12  1:56 UTC (permalink / raw)
  To: gdb-patches; +Cc: pedro, simark, tdevries, 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.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=26761
---
 gdb/event-top.c     |  2 ++
 gdb/mi/mi-main.c    |  4 ++++
 gdb/remote-fileio.c | 15 ++++++++++-----
 gdb/tui/tui.c       |  4 ++++
 4 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/gdb/event-top.c b/gdb/event-top.c
index 4a46e1b9346..9d34e574653 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -1281,6 +1281,8 @@ async_disconnect (gdb_client_data arg)
       gdb_puts ("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 e0cade2edc3..d7ca7843d6f 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1952,6 +1952,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 3ff2a65b0ec..c804b0b2098 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.c b/gdb/tui/tui.c
index cdae9ffe02b..23b47ee163c 100644
--- a/gdb/tui/tui.c
+++ b/gdb/tui/tui.c
@@ -122,6 +122,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);
-- 
2.34.3


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v4 8/8] Forced quit cases handled by resetting sync_quit_force_run
  2023-01-12  1:56 [PATCH v4 0/8] Fix gdb.base/gdb-sigterm.exp failure/error Kevin Buettner
                   ` (6 preceding siblings ...)
  2023-01-12  1:56 ` [PATCH v4 7/8] QUIT processing w/ explicit throw for gdb_exception_forced_quit Kevin Buettner
@ 2023-01-12  1:56 ` Kevin Buettner
  2023-01-30 19:02   ` Pedro Alves
  2023-01-12 12:37 ` [PATCH v4 0/8] Fix gdb.base/gdb-sigterm.exp failure/error Tom de Vries
  8 siblings, 1 reply; 21+ messages in thread
From: Kevin Buettner @ 2023-01-12  1:56 UTC (permalink / raw)
  To: gdb-patches; +Cc: pedro, simark, tdevries, 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.

Another case is the try / catch in tui_getc() in tui-io.c.  The
existing catch swallows the exception.  I've added a catch for
'gdb_exception_forced_quit', which also swallows the exception,
but also sets sync_quit_force_run and calls set_quit_flag in
order to restart forced quit processing at the next QUIT check.
This is required because it isn't safe to throw into/through
readline.

Thanks to Pedro Alves for suggesting this idea.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=26761
---
 gdb/linux-fork.c | 13 +++++++++++++
 gdb/tui/tui-io.c |  9 +++++++++
 2 files changed, 22 insertions(+)

diff --git a/gdb/linux-fork.c b/gdb/linux-fork.c
index 61545b859ea..fc2f00d0766 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"),
diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c
index 2f39e34df2f..ac3c7296499 100644
--- a/gdb/tui/tui-io.c
+++ b/gdb/tui/tui-io.c
@@ -1275,6 +1275,15 @@ tui_getc (FILE *fp)
     {
       return tui_getc_1 (fp);
     }
+  catch (const gdb_exception_forced_quit &ex)
+    {
+      /* As noted below, it's not safe to let an exception escape
+         to newline, so, for this case,  reset the quit flag for
+	 later QUIT checking.  */
+      sync_quit_force_run = 1;
+      set_quit_flag ();
+      return 0;
+    }
   catch (const gdb_exception &ex)
     {
       /* Just in case, don't ever let an exception escape to readline.
-- 
2.34.3


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 0/8] Fix gdb.base/gdb-sigterm.exp failure/error
  2023-01-12  1:56 [PATCH v4 0/8] Fix gdb.base/gdb-sigterm.exp failure/error Kevin Buettner
                   ` (7 preceding siblings ...)
  2023-01-12  1:56 ` [PATCH v4 8/8] Forced quit cases handled by resetting sync_quit_force_run Kevin Buettner
@ 2023-01-12 12:37 ` Tom de Vries
  2023-01-27 22:03   ` Kevin Buettner
  8 siblings, 1 reply; 21+ messages in thread
From: Tom de Vries @ 2023-01-12 12:37 UTC (permalink / raw)
  To: Kevin Buettner, gdb-patches; +Cc: pedro, simark

On 1/12/23 02:56, Kevin Buettner wrote:
> This series fixes the failure in gdb.base/gdb-sigterm.exp when
> running in environments with glibc-2.34 or later.
> 

I've tested this (with python and guile enabled) on:
- openSUSE Tumbleweed x86_64, where I did see FAIL before, and
   this series fixes it
- openSUSE Leap 15.4 x86_64
No problems found.

FWIW, I ran into this when applying:
...
$ git am ~/patches/grouped/eml/*
Applying: Introduce gdb_exception_forced_quit
Applying: Handle gdb SIGTERM by throwing / catching gdb_exception_force_quit
.git/rebase-apply/patch:37: indent with spaces.
         {
warning: 1 line adds whitespace errors.
Applying: Catch gdb_exception_error instead of gdb_exception (in many 
places)
Applying: Python QUIT processing updates
Applying: Guile QUIT processing updates
Applying: Call quit_force for gdb_exception_forced_quit in 
safe_execute_command
Applying: QUIT processing w/ explicit throw for gdb_exception_forced_quit
.git/rebase-apply/patch:17: indent with spaces.
         throw;
.git/rebase-apply/patch:46: indent with spaces.
         {
.git/rebase-apply/patch:58: indent with spaces.
         {
warning: 3 lines add whitespace errors.
Applying: Forced quit cases handled by resetting sync_quit_force_run
.git/rebase-apply/patch:41: indent with spaces.
          to newline, so, for this case,  reset the quit flag for
warning: 1 line adds whitespace errors.
...

Thanks,
- Tom

> It addresses Pedro's concerns regarding the v3 series:
> 
> https://sourceware.org/pipermail/gdb-patches/2022-February/186150.html
> 
> Patches 3, 6, and 7 from the earlier series has changed as follows:
> 
> Patch 3 / "Catch gdb_exception_error instead of gdb_exception (in many
> places)" no longer contains a change for safe_execute_command().  This
> is a tricky case; I've reworked it and moved it into its own commit
> titled "Call quit_force for gdb_exception_forced_quit in
> safe_execute_command".
> 
> Patch 6 / "QUIT processing w/ explicit throw for
> gdb_exception_forced_quit" no longer contains changes to
> gdb/tui/tui-io.c.  These changes have been moved to patch 8 (which
> was #7 in the v3 series) since the technique used for circumventing
> the problem with not being able to do a throw are the same for both.
> 
> Patch 7 (from v3) / "Handle QUIT processing in the scoped_switch_fork_info
> destructor" is now patch 8 in v4.  It's been retitled as "Forced quit cases
> handled by resetting sync_quit_force_run"  It contains changes for the
> destructor as well as tui_getc().
> 
> Patch 6 (v4) / "Call quit_force for gdb_exception_forced_quit in
> safe_execute_command" is a new patch containing a rework of changes
> to safe_execute_command that was previously in Patch 3.
> 
> Kevin Buettner (8):
>    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
>    Call quit_force for gdb_exception_forced_quit in safe_execute_command
>    QUIT processing w/ explicit throw for gdb_exception_forced_quit
>    Forced quit cases handled by resetting sync_quit_force_run
> 
>   gdb/ada-lang.c                   |  2 +-
>   gdb/breakpoint.c                 |  8 ++++----
>   gdb/cli/cli-interp.c             | 11 +++++++++++
>   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                 |  9 +++++++++
>   gdb/tui/tui.c                    |  4 ++++
>   gdb/utils.c                      |  2 +-
>   gdbsupport/common-exceptions.cc  | 14 ++++++++++++++
>   gdbsupport/common-exceptions.h   | 22 +++++++++++++++++++++-
>   38 files changed, 164 insertions(+), 28 deletions(-)
> 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 0/8] Fix gdb.base/gdb-sigterm.exp failure/error
  2023-01-12 12:37 ` [PATCH v4 0/8] Fix gdb.base/gdb-sigterm.exp failure/error Tom de Vries
@ 2023-01-27 22:03   ` Kevin Buettner
  0 siblings, 0 replies; 21+ messages in thread
From: Kevin Buettner @ 2023-01-27 22:03 UTC (permalink / raw)
  To: Tom de Vries; +Cc: gdb-patches, pedro

On Thu, 12 Jan 2023 13:37:00 +0100
Tom de Vries <tdevries@suse.de> wrote:

> FWIW, I ran into this when applying:
> ...
> $ git am ~/patches/grouped/eml/*
> Applying: Introduce gdb_exception_forced_quit
> Applying: Handle gdb SIGTERM by throwing / catching gdb_exception_force_quit
> .git/rebase-apply/patch:37: indent with spaces.
>          {
> warning: 1 line adds whitespace errors.
> Applying: Catch gdb_exception_error instead of gdb_exception (in many 
> places)
> Applying: Python QUIT processing updates
> Applying: Guile QUIT processing updates
> Applying: Call quit_force for gdb_exception_forced_quit in 
> safe_execute_command
> Applying: QUIT processing w/ explicit throw for gdb_exception_forced_quit
> .git/rebase-apply/patch:17: indent with spaces.
>          throw;
> .git/rebase-apply/patch:46: indent with spaces.
>          {
> .git/rebase-apply/patch:58: indent with spaces.
>          {
> warning: 3 lines add whitespace errors.
> Applying: Forced quit cases handled by resetting sync_quit_force_run
> .git/rebase-apply/patch:41: indent with spaces.
>           to newline, so, for this case,  reset the quit flag for
> warning: 1 line adds whitespace errors.
> ...

Thanks!

I've fixed these whitespace problems in my local sources.  I've
also added a "Tested-by" attribution for you to each commit message.

Kevin


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 1/8] Introduce gdb_exception_forced_quit
  2023-01-12  1:56 ` [PATCH v4 1/8] Introduce gdb_exception_forced_quit Kevin Buettner
@ 2023-01-30 18:56   ` Pedro Alves
  0 siblings, 0 replies; 21+ messages in thread
From: Pedro Alves @ 2023-01-30 18:56 UTC (permalink / raw)
  To: Kevin Buettner, gdb-patches; +Cc: simark, tdevries

On 2023-01-12 1:56 a.m., Kevin Buettner wrote:
> 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.

OK.

> 
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=26761
> ---
>  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 d0dd8081c41..edc1b56181a 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 a2a4f5a3892..f9b59ece736 100644
> --- a/gdbsupport/common-exceptions.h
> +++ b/gdbsupport/common-exceptions.h
> @@ -32,6 +32,8 @@
>  
>  enum return_reason
>    {
> +    /* SIGTERM sent to GDB.  */
> +    RETURN_FORCED_QUIT = -3,
>      /* User interrupt.  */
>      RETURN_QUIT = -2,
>      /* Any other error.  */
> @@ -42,9 +44,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.  */
> @@ -294,6 +297,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
> @@ -336,5 +354,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 */
> 


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 2/8] Handle gdb SIGTERM by throwing / catching gdb_exception_force_quit
  2023-01-12  1:56 ` [PATCH v4 2/8] Handle gdb SIGTERM by throwing / catching gdb_exception_force_quit Kevin Buettner
@ 2023-01-30 18:57   ` Pedro Alves
  0 siblings, 0 replies; 21+ messages in thread
From: Pedro Alves @ 2023-01-30 18:57 UTC (permalink / raw)
  To: Kevin Buettner, gdb-patches; +Cc: simark, tdevries

On 2023-01-12 1:56 a.m., Kevin Buettner wrote:
> 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.
> 
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=26761
> ---
>  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 c04d37a45f9..323d99321c7 100644
> --- a/gdb/main.c
> +++ b/gdb/main.c
> @@ -410,6 +410,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);
> @@ -518,6 +522,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);
> @@ -1309,6 +1317,10 @@ captured_main (void *data)
>  	{
>  	  captured_command_loop ();
>  	}
> +      catch (const gdb_exception_forced_quit &ex)
> +        {
> +	  quit_force (NULL, 0);
> +	}

Tabs vs spaces above.

Otherwise this LGTM.

Pedro Alves

>        catch (const gdb_exception &ex)
>  	{
>  	  exception_print (gdb_stderr, ex);
> diff --git a/gdb/utils.c b/gdb/utils.c
> index 734c5bf7f70..0e126cf103b 100644
> --- a/gdb/utils.c
> +++ b/gdb/utils.c
> @@ -673,7 +673,7 @@ quit (void)
>    if (sync_quit_force_run)
>      {
>        sync_quit_force_run = 0;
> -      quit_force (NULL, 0);
> +      throw_forced_quit ("SIGTERM");
>      }
>  
>  #ifdef __MSDOS__
> 


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 3/8] Catch gdb_exception_error instead of gdb_exception (in many places)
  2023-01-12  1:56 ` [PATCH v4 3/8] Catch gdb_exception_error instead of gdb_exception (in many places) Kevin Buettner
@ 2023-01-30 19:00   ` Pedro Alves
  2023-02-16 10:52     ` Pedro Alves
  0 siblings, 1 reply; 21+ messages in thread
From: Pedro Alves @ 2023-01-30 19:00 UTC (permalink / raw)
  To: Kevin Buettner, gdb-patches; +Cc: simark, tdevries

Hi Kevin,

This patch took me a bit to convince myself is good as is.  See below why.

On 2023-01-12 1:56 a.m., Kevin Buettner wrote:
> 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.
> 
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=26761
> ---
>  gdb/ada-lang.c           | 2 +-
>  gdb/breakpoint.c         | 8 ++++----
>  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 +-
>  17 files changed, 20 insertions(+), 20 deletions(-)
> 
> diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
> index 40f85914a56..241dd1b9f24 100644
> --- a/gdb/ada-lang.c
> +++ b/gdb/ada-lang.c
> @@ -12320,7 +12320,7 @@ should_stop_exception (const struct bp_location *bl)
>        scoped_value_mark mark;
>        stop = value_true (evaluate_expression (ada_loc->excep_cond_expr.get ()));
>      }
> -  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 8cfc46e0bed..af8b62e4880 100644
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -2833,7 +2833,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)
>  		{
>  		  rethrow_on_target_close_error (e);
>  		  bp_excpt = std::move (e);
> @@ -5295,7 +5295,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 "
> @@ -5539,7 +5539,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");
> @@ -13476,7 +13476,7 @@ enable_breakpoint_disp (struct breakpoint *bpt, enum bpdisp disposition,
>  	  bpt->enable_state = bp_enabled;
>  	  update_watchpoint (w, true /* 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/i386-linux-tdep.c b/gdb/i386-linux-tdep.c
> index ad0377de5a6..a6adeca1b97 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 f49d1c3c872..b9f25008247 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 0497ad05091..895d84025cf 100644
> --- a/gdb/infcmd.c
> +++ b/gdb/infcmd.c
> @@ -1588,7 +1588,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);
>      }


Most of this patch makes a little nervous, as in several cases it doesn't look like
just letting an exception propagate will do the right thing, particularly
if it's a ctrl-c.  For example, in the code touched by the hunk above we have:

 void
 print_return_value (struct ui_out *uiout, struct return_value_info *rv)
 {
   if (rv->type == nullptr
       || check_typedef (rv->type)->code () == TYPE_CODE_VOID)
     return;
 
   try
     {
       /* print_return_value_1 can throw an exception in some
 	 circumstances.  We need to catch this so that we still
 	 delete the breakpoint.  */
       print_return_value_1 (uiout, rv);
     }
   catch (const gdb_exception &ex)
     {
       exception_print (gdb_stdout, ex);
     }
 }

Note:

 "We need to catch this so that we still delete the breakpoint."

That's referring to the internal breakpoint inserted by the
"finish" command.

Actually, it's not that easy to think about how to best handle a Ctrl-C exception
thrown while handling an event, within fetch_inferior_event & friends.

When the inferior is running in the foreground, then a Ctrl-C typed while
handling some event will end up translated into a call to target_pass_ctrlc(),
so the quit-exception now escaping case doesn't happen then:

 void
 default_quit_handler (void)
 {
   if (check_quit_flag ())
     {
       if (target_terminal::is_ours ())
 	quit ();
       else
 	target_pass_ctrlc ();
      }
 }

It's when the inferior is running on the background, say with "c&" that is
more concerning (the is_ours() case above).  In that case a Ctrl-C really results in a Quit,
which if called within fetch_inferior_event & friends can abort run control handling, and most
probably leave run control in an inconsistent state.  It should be possible to contruct e.g. a
testcase that sets a thread constantly hitting a breakpoint with a condition that evals
false, a watchpoint etc. to maximize chances of a QUIT macro call done while an event is being
handled, and then do "c &" and then periodically press Ctrl-C.  Since the run control handling,
and breakpoint and watchpoint evaluation, etc. are running in the background from the perspective
of the CLI, when users type ctrl-c in this situation, they're thinking of aborting whatever
other command they were typing or running at the prompt, not the run control side, not the previous
"c&" command.

So I'm thinking that we should probably install a custom quit_handler while inside 
fetch_inferior_event, like we disable pagination and other things, one that just does nothing,
or at least, does nothing until the user to tries ctrl-c a number of times before querying the
user what to do, in case GDB really gets stuck.

And if we agree with that direction, then I guess my concern about making all these try/catch that can
be run during run control, the ones in infrun.c, breakpoint.c, etc., escape gdb_exception_quit mostly
goes away.

So I think with that direction this patch is OK.

Pedro Alves

> diff --git a/gdb/infrun.c b/gdb/infrun.c
> index 181d961d80d..9cfafda5ace 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -8730,7 +8730,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 48bfef0a026..7f6b289cdd3 100644
> --- a/gdb/jit.c
> +++ b/gdb/jit.c
> @@ -715,7 +715,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 8b0483803b6..75957b75bad 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 cf3d738edce..ba58044f868 100644
> --- a/gdb/mi/mi-interp.c
> +++ b/gdb/mi/mi-interp.c
> @@ -847,7 +847,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 e17a4c406c0..054895c02e3 100644
> --- a/gdb/objc-lang.c
> +++ b/gdb/objc-lang.c
> @@ -1286,7 +1286,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 2f7d58061ab..9aa080ff974 100644
> --- a/gdb/parse.c
> +++ b/gdb/parse.c
> @@ -514,7 +514,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 c3c2e5ad612..fe26771abe4 100644
> --- a/gdb/printcmd.c
> +++ b/gdb/printcmd.c
> @@ -2109,7 +2109,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 0daba893813..61de8491bb9 100644
> --- a/gdb/record-btrace.c
> +++ b/gdb/record-btrace.c
> @@ -2932,7 +2932,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 e3cfa3f45d5..15c5b7d682e 100644
> --- a/gdb/record-full.c
> +++ b/gdb/record-full.c
> @@ -785,7 +785,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 60bdf46cb91..ec0fcfb5beb 100644
> --- a/gdb/solib.c
> +++ b/gdb/solib.c
> @@ -785,7 +785,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 beff812eeef..685df066f3a 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 d36b9090e7e..7ac3ac709ec 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);
>  	}
> 


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 4/8] Python QUIT processing updates
  2023-01-12  1:56 ` [PATCH v4 4/8] Python QUIT processing updates Kevin Buettner
@ 2023-01-30 19:01   ` Pedro Alves
  2023-02-20  2:52     ` Kevin Buettner
  0 siblings, 1 reply; 21+ messages in thread
From: Pedro Alves @ 2023-01-30 19:01 UTC (permalink / raw)
  To: Kevin Buettner, gdb-patches; +Cc: simark, tdevries

On 2023-01-12 1:56 a.m., Kevin Buettner wrote:
> 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.
> 
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=26761

I guess this is OK as long as we don't hook up Python at a very low level
where running quit_force() would be dangerous.

Alternatively, couldn't we force some Python exception/error which is not
catch-able from Python code?  Set some error flag in the Python interpreter
or something, so that it aborts the Python code as soon as we return to the
Python interpreter.  We'd use the set_quit_flag() trick here where you're calling
quit_force (and instead of calling quit_force), so that as soon as we get out of
the Python interpreter on the other end, we'd re-raise gdb_exception_forced_quit.

Pedro Alves

> ---
>  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 d4d129110e9..1a224b35779 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"
> @@ -271,6 +272,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 ea0f78c9ad8..b9294ad9afc 100644
> --- a/gdb/python/py-gdb-readline.c
> +++ b/gdb/python/py-gdb-readline.c
> @@ -46,6 +46,10 @@ gdbpy_readline_wrapper (FILE *sys_stdin, FILE *sys_stdout,
>        p = command_line_input (buffer, 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 b8777966c47..899b0787582 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"
> @@ -515,6 +516,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 624b90a827f..d5b07a80d82 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"
> @@ -219,6 +220,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 dcc92e51b60..9473468035b 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;
> 


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 6/8] Call quit_force for gdb_exception_forced_quit in safe_execute_command
  2023-01-12  1:56 ` [PATCH v4 6/8] Call quit_force for gdb_exception_forced_quit in safe_execute_command Kevin Buettner
@ 2023-01-30 19:01   ` Pedro Alves
  0 siblings, 0 replies; 21+ messages in thread
From: Pedro Alves @ 2023-01-30 19:01 UTC (permalink / raw)
  To: Kevin Buettner, gdb-patches; +Cc: simark, tdevries

On 2023-01-12 1:56 a.m., Kevin Buettner wrote:
> In gdb/cli/cli-interp.c, safe_execute_command catches gdb_exception.
> In the previous series, I had changed it to instead catch
> gdb_exception_error, which would allow gdb_exception_quit and
> gdb_exception_forced_quit to not be caught, thus propagating as
> normal.
> 
> Pedro found some problems with doing this for safe_execute_command.
> See:
> 
> https://sourceware.org/pipermail/gdb-patches/2022-March/186320.html
> 
> Pedro suggested: "Maybe we can just eliminate safe_execute_command and
> let exceptions propagate normally."
> 
> I'm not doing that here, though I think it might be worth trying.
> 
> Instead, what I've done is to catch gdb_exception_forced_quit and,
> when caught, call quit_force() thus forcing GDB to terminate.
> 
> After (re)reading Pedro's remarks, I think there's still a problem
> with this area of the code (in which gdb_exception_quit is turned into
> an error in interpreter_exec_cmd), but that problem existed before this
> patch series and I think that fully addressing it should be the
> subject of a different set of patches.
> 

I sent a patch implementing my suggestion, here:

  https://inbox.sourceware.org/gdb-patches/20230127230545.77750-1-pedro@palves.net/

I think that when that goes in, this patch can be dropped.

Pedro Alves


> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=26761
> ---
>  gdb/cli/cli-interp.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/gdb/cli/cli-interp.c b/gdb/cli/cli-interp.c
> index 5f2ff726f26..066e55b64f1 100644
> --- a/gdb/cli/cli-interp.c
> +++ b/gdb/cli/cli-interp.c
> @@ -353,6 +353,17 @@ safe_execute_command (struct ui_out *command_uiout, const char *command,
>      {
>        execute_command (command, from_tty);
>      }
> +  catch (gdb_exception_forced_quit &exception)
> +    {
> +      /* Due to the way that the cli_inter::exec is structured, it is
> +	 not safe to only catch gdb_exception_error below, thus
> +	 allowing quit exceptions to propagate through.  (The CLI
> +	 stream won't be reset as it should be.) So, for
> +	 gdb_exception_forced_quit, which corresponds to GDB having
> +	 received a SIGTERM signal, call quit_force() here which will
> +	 cause GDB to terminate.  */
> +      quit_force (NULL, 0);
> +    }
>    catch (gdb_exception &exception)
>      {
>        e = std::move (exception);
> 


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 7/8] QUIT processing w/ explicit throw for gdb_exception_forced_quit
  2023-01-12  1:56 ` [PATCH v4 7/8] QUIT processing w/ explicit throw for gdb_exception_forced_quit Kevin Buettner
@ 2023-01-30 19:02   ` Pedro Alves
  0 siblings, 0 replies; 21+ messages in thread
From: Pedro Alves @ 2023-01-30 19:02 UTC (permalink / raw)
  To: Kevin Buettner, gdb-patches; +Cc: simark, tdevries

On 2023-01-12 1:56 a.m., Kevin Buettner wrote:
> 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.
> 
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=26761
> ---
>  gdb/event-top.c     |  2 ++
>  gdb/mi/mi-main.c    |  4 ++++
>  gdb/remote-fileio.c | 15 ++++++++++-----
>  gdb/tui/tui.c       |  4 ++++
>  4 files changed, 20 insertions(+), 5 deletions(-)
> 
> diff --git a/gdb/event-top.c b/gdb/event-top.c
> index 4a46e1b9346..9d34e574653 100644
> --- a/gdb/event-top.c
> +++ b/gdb/event-top.c
> @@ -1281,6 +1281,8 @@ async_disconnect (gdb_client_data arg)
>        gdb_puts ("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 e0cade2edc3..d7ca7843d6f 100644
> --- a/gdb/mi/mi-main.c
> +++ b/gdb/mi/mi-main.c
> @@ -1952,6 +1952,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 3ff2a65b0ec..c804b0b2098 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;
> +	}

Tabs vs spaces above.

> +      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);
>  	}

Ditto.

>      }
>  
> diff --git a/gdb/tui/tui.c b/gdb/tui/tui.c
> index cdae9ffe02b..23b47ee163c 100644
> --- a/gdb/tui/tui.c
> +++ b/gdb/tui/tui.c
> @@ -122,6 +122,10 @@ tui_rl_switch_mode (int notused1, int notused2)
>  	  tui_enable ();
>  	}
>      }
> +  catch (const gdb_exception_forced_quit &ex)
> +    {
> +      throw;
> +    }

We can't let the exception propagate here.  See comment just above, which reads:

  /* Don't let exceptions escape.  We're in the middle of a readline
     callback that isn't prepared for that.  */
  try
    {
  ...


I think we need to use the set_quit_flag()-to-rethrow-later trick here.

>    catch (const gdb_exception &ex)
>      {
>        exception_print (gdb_stderr, ex);
> 


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 8/8] Forced quit cases handled by resetting sync_quit_force_run
  2023-01-12  1:56 ` [PATCH v4 8/8] Forced quit cases handled by resetting sync_quit_force_run Kevin Buettner
@ 2023-01-30 19:02   ` Pedro Alves
  0 siblings, 0 replies; 21+ messages in thread
From: Pedro Alves @ 2023-01-30 19:02 UTC (permalink / raw)
  To: Kevin Buettner, gdb-patches; +Cc: simark, tdevries

On 2023-01-12 1:56 a.m., Kevin Buettner wrote:
> 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.
> 
> Another case is the try / catch in tui_getc() in tui-io.c.  The
> existing catch swallows the exception.  I've added a catch for
> 'gdb_exception_forced_quit', which also swallows the exception,
> but also sets sync_quit_force_run and calls set_quit_flag in
> order to restart forced quit processing at the next QUIT check.
> This is required because it isn't safe to throw into/through
> readline.
> 
> Thanks to Pedro Alves for suggesting this idea.
> 
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=26761
> ---
>  gdb/linux-fork.c | 13 +++++++++++++
>  gdb/tui/tui-io.c |  9 +++++++++
>  2 files changed, 22 insertions(+)
> 
> diff --git a/gdb/linux-fork.c b/gdb/linux-fork.c
> index 61545b859ea..fc2f00d0766 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 ();

I think it'd be nice if we had a function that did both of these for us.
Something like "set_force_quit_flag()".

> +	  }
>  	catch (const gdb_exception &ex)
>  	  {
>  	    warning (_("Couldn't restore checkpoint state in %s: %s"),
> diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c
> index 2f39e34df2f..ac3c7296499 100644
> --- a/gdb/tui/tui-io.c
> +++ b/gdb/tui/tui-io.c
> @@ -1275,6 +1275,15 @@ tui_getc (FILE *fp)
>      {
>        return tui_getc_1 (fp);
>      }
> +  catch (const gdb_exception_forced_quit &ex)
> +    {
> +      /* As noted below, it's not safe to let an exception escape
> +         to newline, so, for this case,  reset the quit flag for

Spurious double space before "reset".

> +	 later QUIT checking.  */
> +      sync_quit_force_run = 1;
> +      set_quit_flag ();
> +      return 0;
> +    }
>    catch (const gdb_exception &ex)
>      {
>        /* Just in case, don't ever let an exception escape to readline.
> 


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 3/8] Catch gdb_exception_error instead of gdb_exception (in many places)
  2023-01-30 19:00   ` Pedro Alves
@ 2023-02-16 10:52     ` Pedro Alves
  0 siblings, 0 replies; 21+ messages in thread
From: Pedro Alves @ 2023-02-16 10:52 UTC (permalink / raw)
  To: Kevin Buettner, gdb-patches; +Cc: simark, tdevries

Hi Kevin,

On 2023-01-30 7:00 p.m., Pedro Alves wrote:
> Hi Kevin,
> 
> This patch took me a bit to convince myself is good as is.  See below why.

...

> So I'm thinking that we should probably install a custom quit_handler while inside 
> fetch_inferior_event, like we disable pagination and other things, one that just does nothing,
> or at least, does nothing until the user to tries ctrl-c a number of times before querying the
> user what to do, in case GDB really gets stuck.
> 
> And if we agree with that direction, then I guess my concern about making all these try/catch that can
> be run during run control, the ones in infrun.c, breakpoint.c, etc., escape gdb_exception_quit mostly
> goes away.
> 
> So I think with that direction this patch is OK.
> 

Just wanted to explicitly state that changes implementing the above are all in master now,
since yesterday.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 4/8] Python QUIT processing updates
  2023-01-30 19:01   ` Pedro Alves
@ 2023-02-20  2:52     ` Kevin Buettner
  2023-02-23 12:50       ` Pedro Alves
  0 siblings, 1 reply; 21+ messages in thread
From: Kevin Buettner @ 2023-02-20  2:52 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

On Mon, 30 Jan 2023 19:01:16 +0000
Pedro Alves <pedro@palves.net> wrote:

> On 2023-01-12 1:56 a.m., Kevin Buettner wrote:
> > 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.
> > 
> > Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=26761  
> 
> I guess this is OK as long as we don't hook up Python at a very low level
> where running quit_force() would be dangerous.
> 
> Alternatively, couldn't we force some Python exception/error which
> is not catch-able from Python code?  Set some error flag in the
> Python interpreter or something, so that it aborts the Python code
> as soon as we return to the Python interpreter.  We'd use the
> set_quit_flag() trick here where you're calling quit_force (and
> instead of calling quit_force), so that as soon as we get out of the
> Python interpreter on the other end, we'd re-raise
> gdb_exception_forced_quit.

I've looked into doing this, but I haven't found a way to force/throw
an uncatchable Python error or exception.

We could define a Python exception, say GDBForcedQuitException, that's
derived from BaseException, but it can still be caught either via:

    try:
      ...
    except GDBForcedQuitException:
      ...

Or:

    try:
      ...
    except BaseException:
      ...

Or even:

    try:
      ...
    except:
      ...

The latter construct will basically catch everything.  This is why
there are no uncatchable exceptions (or errors) in Python.  That said,
deriving GDBForcedQuitException from BaseException (instead of Exception)
means that it can't be caught via "except Exception:", which is a catch-all
for normal exceptions.

I also took a look at calling PyErr_SetInterruptEx (SIGTERM) which
would simulate the effect of the Python layer receiving a SIGTERM
signal.  For Ctrl-C/SIGINT, We use something similar - a call to
PyErr_SetInterrupt() which is equivalent to PyErr_SetInterruptEx(SIGINT).
However, for SIGINT, python defines the default signal handler for
SIGINT to raise the KeyboardInterrupt exception.  However, from my
reading of the documentation, Python doesn't have a default handler
for SIGTERM.  The documentation says that such signals will be ignored.

Even though it's not uncatchable, it might be worth pursuing your
idea via an exception derived from BaseException as described above.
I started looking into this, but doing it looks somewhat involved.
So, instead of including it in this series, I'd prefer to push this
commit as-is and work on raising a GDBForcedQuitException in a separate
series.

Kevin


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 4/8] Python QUIT processing updates
  2023-02-20  2:52     ` Kevin Buettner
@ 2023-02-23 12:50       ` Pedro Alves
  0 siblings, 0 replies; 21+ messages in thread
From: Pedro Alves @ 2023-02-23 12:50 UTC (permalink / raw)
  To: Kevin Buettner; +Cc: gdb-patches

Hi Kevin,

On 2023-02-20 2:52 a.m., Kevin Buettner wrote:
> On Mon, 30 Jan 2023 19:01:16 +0000
> Pedro Alves <pedro@palves.net> wrote:
> 
>> On 2023-01-12 1:56 a.m., Kevin Buettner wrote:
>>> 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.
>>>
>>> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=26761  
>>
>> I guess this is OK as long as we don't hook up Python at a very low level
>> where running quit_force() would be dangerous.
>>
>> Alternatively, couldn't we force some Python exception/error which
>> is not catch-able from Python code?  Set some error flag in the
>> Python interpreter or something, so that it aborts the Python code
>> as soon as we return to the Python interpreter.  We'd use the
>> set_quit_flag() trick here where you're calling quit_force (and
>> instead of calling quit_force), so that as soon as we get out of the
>> Python interpreter on the other end, we'd re-raise
>> gdb_exception_forced_quit.
> 
> I've looked into doing this, but I haven't found a way to force/throw
> an uncatchable Python error or exception.

Thanks for checking.  I was hoping that we could at least do something
like calling something like Py_Finalize() from within a Python callback,
but of course a version of that function that signaled that were bringing
down the interpreter, without causing Python to crash as soon as we're
out of the callback back into Python.

> 
> We could define a Python exception, say GDBForcedQuitException, that's
> derived from BaseException, but it can still be caught either via:
> 
>     try:
>       ...
>     except GDBForcedQuitException:
>       ...
> 
> Or:
> 
>     try:
>       ...
>     except BaseException:
>       ...
> 
> Or even:
> 
>     try:
>       ...
>     except:
>       ...
> 
> The latter construct will basically catch everything.  This is why
> there are no uncatchable exceptions (or errors) in Python.  That said,
> deriving GDBForcedQuitException from BaseException (instead of Exception)
> means that it can't be caught via "except Exception:", which is a catch-all
> for normal exceptions.
> 
> I also took a look at calling PyErr_SetInterruptEx (SIGTERM) which
> would simulate the effect of the Python layer receiving a SIGTERM
> signal.  For Ctrl-C/SIGINT, We use something similar - a call to
> PyErr_SetInterrupt() which is equivalent to PyErr_SetInterruptEx(SIGINT).
> However, for SIGINT, python defines the default signal handler for
> SIGINT to raise the KeyboardInterrupt exception.  However, from my
> reading of the documentation, Python doesn't have a default handler
> for SIGTERM.  The documentation says that such signals will be ignored.
> 
> Even though it's not uncatchable, it might be worth pursuing your
> idea via an exception derived from BaseException as described above.
> I started looking into this, but doing it looks somewhat involved.
> So, instead of including it in this series, I'd prefer to push this
> commit as-is and work on raising a GDBForcedQuitException in a separate
> series.

Yes, let's do that.

Thanks again for the investigation.

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2023-02-23 12:50 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-12  1:56 [PATCH v4 0/8] Fix gdb.base/gdb-sigterm.exp failure/error Kevin Buettner
2023-01-12  1:56 ` [PATCH v4 1/8] Introduce gdb_exception_forced_quit Kevin Buettner
2023-01-30 18:56   ` Pedro Alves
2023-01-12  1:56 ` [PATCH v4 2/8] Handle gdb SIGTERM by throwing / catching gdb_exception_force_quit Kevin Buettner
2023-01-30 18:57   ` Pedro Alves
2023-01-12  1:56 ` [PATCH v4 3/8] Catch gdb_exception_error instead of gdb_exception (in many places) Kevin Buettner
2023-01-30 19:00   ` Pedro Alves
2023-02-16 10:52     ` Pedro Alves
2023-01-12  1:56 ` [PATCH v4 4/8] Python QUIT processing updates Kevin Buettner
2023-01-30 19:01   ` Pedro Alves
2023-02-20  2:52     ` Kevin Buettner
2023-02-23 12:50       ` Pedro Alves
2023-01-12  1:56 ` [PATCH v4 5/8] Guile " Kevin Buettner
2023-01-12  1:56 ` [PATCH v4 6/8] Call quit_force for gdb_exception_forced_quit in safe_execute_command Kevin Buettner
2023-01-30 19:01   ` Pedro Alves
2023-01-12  1:56 ` [PATCH v4 7/8] QUIT processing w/ explicit throw for gdb_exception_forced_quit Kevin Buettner
2023-01-30 19:02   ` Pedro Alves
2023-01-12  1:56 ` [PATCH v4 8/8] Forced quit cases handled by resetting sync_quit_force_run Kevin Buettner
2023-01-30 19:02   ` Pedro Alves
2023-01-12 12:37 ` [PATCH v4 0/8] Fix gdb.base/gdb-sigterm.exp failure/error Tom de Vries
2023-01-27 22:03   ` Kevin Buettner

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).