diff mbox series

[v4,6/8] Call quit_force for gdb_exception_forced_quit in safe_execute_command

Message ID 20230112015630.32999-7-kevinb@redhat.com
State New
Headers
Series Fix gdb.base/gdb-sigterm.exp failure/error |

Commit Message

Kevin Buettner Jan. 12, 2023, 1:56 a.m. UTC 
  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(+)

Comments

Pedro Alves Jan. 30, 2023, 7:01 p.m. UTC | #1 
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);
>
diff mbox series

Patch

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