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

["Jakob Engblom" <jakob@virtutech.com>]

I am now on parental leave, really, but I just gave it a try to try to get the
MI reverse commands fixed.  In particular, making them compatible with gdb7.  We
have an internal version at VT that does that, but that also contains the
offending piece of code that is duplicated from reverse.c. 

In our code, and in the submitted patches, in mi-main.c, we have:

----
static void
exec_continue (char **argv, int argc)
{
  if (argc == 0)
    continue_1 (0);
  else if (argc == 1 && strcmp (argv[0], "--all") == 0)
    continue_1 (1);
  else if (argc == 2 && strcmp (argv[0], "--thread-group") == 0)
    {
      struct cleanup *old_chain;
      int pid;
      if (argv[1] == NULL || argv[1] == '\0')
	error ("Thread group id not specified");
      pid = atoi (argv[1]);
      if (!in_inferior_list (pid))
	error ("Invalid thread group id '%s'", argv[1]);

      old_chain = make_cleanup_restore_current_thread ();
      iterate_over_threads (proceed_thread_callback, &pid);
      do_cleanups (old_chain);            
    }
  else
    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);
}

void
mi_cmd_exec_continue (char *command, char **argv, int argc)
{
  if (argc > 0 && strcmp(argv[0], "--reverse") == 0)
    exec_reverse_continue (argv + 1, argc - 1);
  else
    exec_continue (argv, argc);
}
----

This code is much more complex than for the other MI reverse commands, where the
logic is very simple:

----
void
mi_cmd_exec_step (char *command, char **argv, int argc)
{
  /* FIXME: Should call a libgdb function, not a cli wrapper.  */
  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);
}
----

To me, it is not clear why MI has to do such a special thing with the "continue"
command at all.  I assume there is a good reason for having the management of
-all and -thread-group inside the MI code?  

Note that the standard gdb7 non-reversible MI does this for the continue
command:

----
void
mi_cmd_exec_continue (char *command, char **argv, int argc)
{
  if (argc == 0)
    continue_1 (0);
  else if (argc == 1 && strcmp (argv[0], "--all") == 0)
    continue_1 (1);
  else if (argc == 2 && strcmp (argv[0], "--thread-group") == 0)
    {
      struct cleanup *old_chain;
      int pid;
      if (argv[1] == NULL || argv[1] == '\0')
	error ("Thread group id not specified");
      pid = atoi (argv[1]);
      if (!in_inferior_list (pid))
	error ("Invalid thread group id '%s'", argv[1]);

      old_chain = make_cleanup_restore_current_thread ();
      iterate_over_threads (proceed_thread_callback, &pid);
      do_cleanups (old_chain);            
    }
  else
    error ("Usage: -exec-continue [--reverse] [--all|--thread-group id]");
}
----

Given this, the safest thing to do appears to be to maintain the current special
handling for reverse continue.  This works as a wrapper around the core code
from mi_cmd_exec_continue, with an additional setting of direction before
calling it. 

/jakob