Re: [RFA] Reverse Debugging, 1/5

["Doug Evans" <dje@google.com>]

On Wed, Oct 1, 2008 at 12:17 PM, Michael Snyder <msnyder@vmware.com> wrote:
>
>
> This first patch of the reverse debugging sequence affects
> only the target_ops interface (adding several target methods
> and some enums and such).
>
> Adding this patch alone does not affect gdb's behavior, and
> I have run testsuites under RHEL with no change in results.
>
>
> 2008-09-30  Michael Snyder  <msnyder@vmware.com>
>        Target interface for reverse debugging.
>        * target.h (enum target_waitkind):
>        Add new wait event, TARGET_WAITKIND_NO_HISTORY.
>        (enum exec_direction_kind): New enum.
>        (struct target_ops): New methods to_set_execdir, to_get_execdir.
>        * target.c (target_get_execdir): New generic method.
>        (target_set_execdir): Ditto.
>
> Index: target.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/target.c,v
> retrieving revision 1.184
> diff -u -p -r1.184 target.c
> --- target.c    22 Sep 2008 15:21:30 -0000      1.184
> +++ target.c    30 Sep 2008 18:19:00 -0000
> @@ -455,6 +455,8 @@ update_current_target (void)
>       INHERIT (to_find_memory_regions, t);
>       INHERIT (to_make_corefile_notes, t);
>       INHERIT (to_get_thread_local_address, t);
> +      INHERIT (to_get_execdir, t);
> +      INHERIT (to_set_execdir, t);
>       /* Do not inherit to_read_description.  */
>       /* Do not inherit to_search_memory.  */
>       INHERIT (to_magic, t);
> Index: target.h
> ===================================================================
> RCS file: /cvs/src/src/gdb/target.h,v
> retrieving revision 1.130
> diff -u -p -r1.130 target.h
> --- target.h    19 Aug 2008 13:22:14 -0000      1.130
> +++ target.h    30 Sep 2008 18:19:00 -0000
> @@ -128,7 +128,11 @@ enum target_waitkind
>        inferior, rather than being stuck in the remote_async_wait()
>        function. This way the event loop is responsive to other events,
>        like for instance the user typing.  */
> -    TARGET_WAITKIND_IGNORE
> +    TARGET_WAITKIND_IGNORE,
> +
> +    /* The target has run out of history information,
> +       and cannot run backward any further.  */
> +    TARGET_WAITKIND_NO_HISTORY
>   };
>
>  struct target_waitstatus
> @@ -147,6 +151,14 @@ struct target_waitstatus
>     value;
>   };
>
> +/* Reverse execution.  */
> +enum exec_direction_kind
> +  {
> +    EXEC_FORWARD,
> +    EXEC_REVERSE,
> +    EXEC_ERROR
> +  };
> +
>  /* Possible types of events that the inferior handler will have to
>    deal with.  */
>  enum inferior_event_type
> @@ -523,6 +535,11 @@ struct target_ops
>                             const gdb_byte *pattern, ULONGEST pattern_len,
>                             CORE_ADDR *found_addrp);
>
> +    /* Set execution direction (forward/reverse).  */
> +    int (*to_set_execdir) (enum exec_direction_kind);
> +    /* Get execution direction (forward/reverse).  */
> +    enum exec_direction_kind (*to_get_execdir) (void);
> +
>     int to_magic;
>     /* Need sub-structure for target machine related rather than comm
> related?
>      */
> @@ -1127,6 +1144,18 @@ extern int target_stopped_data_address_p
>  #define target_watchpoint_addr_within_range(target, addr, start, length) \
>   (*target.to_watchpoint_addr_within_range) (target, addr, start, length)
>
> +/* Forward/reverse execution direction.
> +   These will only be implemented by a target that supports reverse
> execution.
> +*/
> +#define target_get_execution_direction() \
> +    (current_target.to_get_execdir ? \
> +     (*current_target.to_get_execdir) () : EXEC_ERROR)
> +
> +#define target_set_execution_direction(DIR) \
> +    (current_target.to_set_execdir ? \
> +     (*current_target.to_set_execdir) (DIR) : EXEC_ERROR)
> +
> +
>  extern const struct target_desc *target_read_description (struct target_ops
> *);
>
>  /* Utility implementation of searching memory.  */

Hi.

Nit: Can all occurrences of "execdir" be replaced with
"exec_direction"?  I look at "execdir" and think "execution
directory".  [I realize often the context provides enough clues that
prevent this misinterpretation, but that's not always true.]