Re: [PATCH v4 6/8] Call quit_force for gdb_exception_forced_quit in safe_execute_command

Pedro Alves
2023-01-30T19:02:01Z

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