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

Stephen Cross
2015-11-30T18:17:42Z

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.