This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA] Reverse Debugging, 5/5
- From: Eli Zaretskii <eliz at gnu dot org>
- To: Michael Snyder <msnyder at vmware dot com>
- Cc: gdb-patches at sourceware dot org, drow at false dot org, pedro at codesourcery dot com, teawater at gmail dot com
- Date: Thu, 02 Oct 2008 22:48:57 +0300
- Subject: Re: [RFA] Reverse Debugging, 5/5
- References: <48E3CD66.9020600@vmware.com>
- Reply-to: Eli Zaretskii <eliz at gnu dot org>
> Date: Wed, 01 Oct 2008 12:20:06 -0700
> From: Michael Snyder <msnyder@vmware.com>
> CC: Daniel Jacobowitz <drow@false.org>, Pedro Alves <pedro@codesourcery.com>, teawater <teawater@gmail.com>
>
> +static void
> +set_exec_direction_func (char *args, int from_tty,
> + struct cmd_list_element *cmd)
> +{
> + if (target_get_execution_direction () != EXEC_ERROR)
> + {
> + enum exec_direction_kind dir = EXEC_ERROR;
> +
> + if (!strcmp (exec_direction, exec_forward))
> + dir = EXEC_FORWARD;
> + else if (!strcmp (exec_direction, exec_reverse))
> + dir = EXEC_REVERSE;
> +
> + if (target_set_execution_direction (dir) != EXEC_ERROR)
> + return;
> + }
> +}
I'm not sure I get your intent with the last two lines: if
target_set_execution_direction returns anything but EXEC_ERROR, you
return, but if not, you ... return? What am I missing?
> + case EXEC_ERROR:
> + default:
> + fprintf_filtered, (out,
> + _("Target `%s' does not support execution-direction."),
> + target_shortname);
> + break;
Why print an error message? isn't it better to say the direction is
"forward" (which is documented as the default in your patch for the
manual)?
> + if (dir == EXEC_REVERSE)
> + error (_("Already in reverse mode. Use '%s' or 'set exec-dir forward'."),
> + cmd);
Isn't it better to silently do the equivalent of "cmd"?
> + add_com ("reverse-step", class_run, reverse_step, _("\
> +Step program backward until it reaches the beginning of another source line.\n\
> +Argument N means do this N times (or till program stops for another reason).")
This sounds as if you are single-stepping the program until it reaches
the previous line (as opposed to running uninterrupted until you hit
previous line). Are you?
> + add_com ("reverse-next", class_run, reverse_next, _("\
> +Step program backward, proceeding through subroutine calls.\n\
^
Won't commands like "apropos" stop at the first comma when they
display the short descriptions of commands?
> +@node Reverse Execution
> +@chapter Running programs backward
Please add a @cindex entry here.
> +program was executing normally. Variables, registers etc. should
Please put a @: after "etc.", to prevent the period being interpreted
by TeX as an end of a sentence.
> After executing
> +a piece of source code in reverse, all side effects of that code
> +should be ``undone''
ALL side effects? I thought some of them cannot be undone,
un-outputting to the various I/O devices etc.
> +Assuming you are debugging in a target environment that supports
"IF you are debugging in a target ..." sounds better, I think.
> Starting from
> +the first line of a function, @code{reverse-next} will take you back
> +to the caller of that function, @emph{before} the function was called.
Shouldn't we have some kind of caveat here regarding function prologue
and epilogue?
> +@item set exec-direction
There should be a @kindex entry here for this command.
Thanks!