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

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 <[email protected]> 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



-- 
Stephen Cross

Software Engineer at Undo Software