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

[Michael Snyder <msnyder@vmware.com>]

Oh, excuse me, reading email out of date order.
I see the questions have been answered.
Please disregard.  ;-)

Michael Snyder wrote:
> Maybe these questions were intended for Jakob?
> 
> 
> 
> Vladimir Prus wrote:
>> 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
>>
>>
>>
>