Re: Always run GDB command post-hook after pre-hook has been run

Stephen Cross
2015-12-02T18:24:32Z

I was wondering if anyone has had a chance to look at this patch?
(I've also CC'ed the gdb list since I'm looking for input on the
approach.)

Thanks,
Stephen

On Mon, Nov 30, 2015 at 6:17 PM, Stephen Cross <scross@undo-software.com> wrote:
> Hello,
>
> We've observed that a command post-hook isn't run if an exception is
> thrown (inside GDB) when running the command. Here's a trace of this:
>
> (gdb) define hook-print
>>echo hook-print\n
>>end
> (gdb) define hookpost-print
>>echo hookpost-print\n
>>end
> (gdb) print test
> hook-print
> No symbol table is loaded. Use the "file" command.
> (gdb) print "test"
> hook-print
> $1 = "test"
> hookpost-print
>
> This issue can be fixed by adding a cleanup action for the post-hook
> call via the patch below. With this change we get the following trace:
>
> (gdb) define hook-print
>>echo hook-print\n
>>end
> (gdb) define hookpost-print
>>echo hookpost-print\n
>>end
> (gdb) print test
> hook-print
> hookpost-print
> No symbol table is loaded. Use the "file" command.
> (gdb) print "test"
> hook-print
> $1 = "test"
> hookpost-print
>
> As you can see the post-hook is now always being called, even if the
> command fails.
>
> diff --git a/gdb/top.c b/gdb/top.c
> index d1e2271..43b3b7f 100644
> --- a/gdb/top.c
> +++ b/gdb/top.c
> @@ -388,13 +388,19 @@ maybe_wait_sync_command_done (int was_sync)
> wait_sync_command_done ();
> }
>
> +static void
> +call_post_hook_cleanup(void* p)
> +{
> + execute_cmd_post_hook (p);
> +}
> +
> /* Execute the line P as a command, in the current user context.
> Pass FROM_TTY as second argument to the defining function. */
>
> void
> execute_command (char *p, int from_tty)
> {
> - struct cleanup *cleanup_if_error, *cleanup;
> + struct cleanup *cleanup_if_error, *cleanup, *cmd_cleanup;
> struct cmd_list_element *c;
> char *line;
>
> @@ -456,6 +462,7 @@ execute_command (char *p, int from_tty)
>
> /* If this command has been pre-hooked, run the hook first. */
> execute_cmd_pre_hook (c);
> + cmd_cleanup = make_cleanup (call_post_hook_cleanup, c);
>
> if (c->deprecated_warn_user)
> deprecated_cmd_warning (line);
> @@ -477,7 +484,7 @@ execute_command (char *p, int from_tty)
> maybe_wait_sync_command_done (was_sync);
>
> /* If this command has been post-hooked, run the hook last. */
> - execute_cmd_post_hook (c);
> + do_cleanups (cmd_cleanup);
>
> }
>
> I've run the test suite before and after this change and I can't see
> any relevant failures. Presumably the patch will also need to include
> a test, but I'm hoping to just get comments on the implementation
> change first.
>
> Thanks,
> Stephen
>
> --
> Stephen Cross
>
> Software Engineer at Undo Software