Re: GDB MI Reverse Commands added [1 of 3]

Vladimir Prus
2009-09-02T08:16:16Z

Michael Snyder wrote:

> Index: mi-main.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/mi/mi-main.c,v
> retrieving revision 1.156
> diff -u -p -r1.156 mi-main.c
> --- mi-main.cÂ Â Â 2 Jul 2009 17:25:59 -0000Â Â Â Â Â Â Â 1.156
> +++ mi-main.cÂ Â Â 27 Aug 2009 01:45:23 -0000
> @@ -88,8 +88,8 @@ static void mi_cmd_execute (struct mi_pa
>
> static void mi_execute_cli_command (const char *cmd, int args_p,
> const char *args);
> -static void mi_execute_async_cli_command (char *cli_command,
> -Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â char **argv, int argc);
> +static void mi_execute_async_cli_command (char *cli_command,
> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  Â char **argv, int argc);
> static int register_changed_p (int regnum, struct regcache *,
> struct regcache *);
> static void get_register (struct frame_info *, int regnum, int format);
> @@ -119,35 +119,50 @@ void
> mi_cmd_exec_next (char *command, char **argv, int argc)
> {
> /* FIXME: Should call a libgdb function, not a cli wrapper. Â */
> - Â mi_execute_async_cli_command ("next", argv, argc);
> + Â if (argc > 0 && strcmp(argv[0], "--reverse") == 0)
> + Â  Â mi_execute_async_cli_command ("reverse-next", argv + 1, argc - 1);
> + Â else
> + Â  Â mi_execute_async_cli_command ("next", argv, argc);
> }
>
> void
> mi_cmd_exec_next_instruction (char *command, char **argv, int argc)
> {
> /* FIXME: Should call a libgdb function, not a cli wrapper. Â */
> - Â mi_execute_async_cli_command ("nexti", argv, argc);
> + Â if (argc > 0 && strcmp(argv[0], "--reverse") == 0)
> + Â  Â mi_execute_async_cli_command ("reverse-nexti", argv + 1, argc - 1);
> + Â else
> + Â  Â mi_execute_async_cli_command ("nexti", argv, argc);
> }
>
> void
> mi_cmd_exec_step (char *command, char **argv, int argc)
> {
> /* FIXME: Should call a libgdb function, not a cli wrapper. Â */
> - Â mi_execute_async_cli_command ("step", argv, argc);
> + Â if (argc > 0 && strcmp(argv[0], "--reverse") == 0)
> + Â  Â mi_execute_async_cli_command ("reverse-step", argv + 1, argc - 1);
> + Â else
> + Â  Â mi_execute_async_cli_command ("step", argv, argc);
> }
>
> void
> mi_cmd_exec_step_instruction (char *command, char **argv, int argc)
> {
> /* FIXME: Should call a libgdb function, not a cli wrapper. Â */
> - Â mi_execute_async_cli_command ("stepi", argv, argc);
> + Â if (argc > 0 && strcmp(argv[0], "--reverse") == 0)
> + Â  Â mi_execute_async_cli_command ("reverse-stepi", argv + 1, argc - 1);
> + Â else
> + Â  Â mi_execute_async_cli_command ("stepi", argv, argc);
> }
>
> void
> mi_cmd_exec_finish (char *command, char **argv, int argc)
> {
> /* FIXME: Should call a libgdb function, not a cli wrapper. Â */
> - Â mi_execute_async_cli_command ("finish", argv, argc);
> + Â if (argc > 0 && strcmp(argv[0], "--reverse") == 0)
> + Â  Â mi_execute_async_cli_command ("reverse-finish", argv + 1, argc - 1);
> + Â else
> + Â  Â mi_execute_async_cli_command ("finish", argv, argc);
> }

What will happen if MI client does not pass --reverse, but
execution_direction is set to reverse via CLI? It seems that this will execute
finish in reverse? I think this is not desirable -- rather -exec-finish
should always be forward. On the other hand, this is something that
I probably can fix as a follow-up patch, somewhere in MI core.

> void
> @@ -175,7 +190,7 @@ mi_cmd_exec_jump (char *args, char **arg
> /* FIXME: Should call a libgdb function, not a cli wrapper. Â */
> return mi_execute_async_cli_command ("jump", argv, argc);
> }
> -
> +

Stray whitespace change. The patch has a few of those, I imagine
that whoever ends up applying the patch to CVS once the copyright
assignment is sorted can undo this damage.

> static int
> proceed_thread_callback (struct thread_info *thread, void *arg)
> {
> @@ -193,8 +208,8 @@ proceed_thread_callback (struct thread_i
> return 0;
> }
>
> -void
> -mi_cmd_exec_continue (char *command, char **argv, int argc)
> +static void
> +exec_continue (char **argv, int argc)
> {
> if (argc == 0)
> continue_1 (0);
> @@ -212,10 +227,50 @@ mi_cmd_exec_continue (char *command, cha
>
> old_chain = make_cleanup_restore_current_thread ();
> iterate_over_threads (proceed_thread_callback, &pid);
> - Â  Â  Â do_cleanups (old_chain);
> + Â  Â  Â do_cleanups (old_chain);
> }
> else
> - Â  Â error ("Usage: -exec-continue [--all|--thread-group id]");
> + Â  Â error ("Usage: -exec-continue [--reverse] [--all|--thread-group id]");
> +}
> +
> +/* continue in reverse direction:
> + Â  XXX: code duplicated from reverse.c */
> +
> +static void
> +exec_direction_default (void *notused)
> +{
> + Â /* Return execution direction to default state. Â */
> + Â execution_direction = EXEC_FORWARD;
> +}
> +
> +static void
> +exec_reverse_continue (char **argv, int argc)
> +{
> + Â enum exec_direction_kind dir = execution_direction;
> + Â struct cleanup *old_chain;
> +
> + Â if (dir == EXEC_ERROR)
> + Â  Â error (_("Target %s does not support this command."), target_shortname);
> +
> + Â if (dir == EXEC_REVERSE)
> + Â  Â error (_("Already in reverse mode."));
> +
> + Â if (!target_can_execute_reverse)
> + Â  Â error (_("Target %s does not support this command."), target_shortname);
> +
> + Â old_chain = make_cleanup (exec_direction_default, NULL);
> + Â execution_direction = EXEC_REVERSE;
> + Â exec_continue (argv, argc);
> + Â do_cleanups (old_chain);
> +}

Why is this code "duplicated from reverse.c"? In other words, cannot
mi_cmd_exec_continue call 'reverse_continue'? If this is not possible,
there should be a comment explaining why.

Otherwise, this is OK. Thanks!

- Volodya