This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFA] Resubmit reverse debugging [4/5]


A Wednesday 08 October 2008 03:19:37, Michael Snyder escreveu:
> 2008-10-07 ÂMichael Snyder Â<msnyder@vmware.com>
> 
> ÂÂÂÂÂÂÂÂ* breakpoint.c (make_breakpoint_silent): New function.
> ÂÂÂÂÂÂÂÂ* breakpoint.h (make_breakpoint_silent): Export.
> ÂÂÂÂÂÂÂÂ* infcmd.c (finish_command): Check for reverse exec direction.
> ÂÂÂÂÂÂÂÂ(finish_backward): New function, handle finish cmd in reverse.
> 
> Index: breakpoint.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/breakpoint.c,v
> retrieving revision 1.352
> diff -u -p -r1.352 breakpoint.c
> --- breakpoint.cÂÂÂÂÂÂÂÂ16 Sep 2008 18:55:01 -0000ÂÂÂÂÂÂ1.352
> +++ breakpoint.cÂÂÂÂÂÂÂÂ8 Oct 2008 01:25:26 -0000
> @@ -7741,6 +7741,13 @@ breakpoint_clear_ignore_counts (void)
> Â Â Âb->ignore_count = 0;
> Â}
> Â
> +void
> +make_breakpoint_silent (struct breakpoint *b)
> +{
> + Â/* Silence the breakpoint. Â*/
> + Âb->silent = 1;
> +}
> +
> Â/* Command to set ignore-count of breakpoint N to COUNT. Â*/
> Â
> Âstatic void
> Index: breakpoint.h
> ===================================================================
> RCS file: /cvs/src/src/gdb/breakpoint.h,v
> retrieving revision 1.79
> diff -u -p -r1.79 breakpoint.h
> --- breakpoint.hÂÂÂÂÂÂÂÂ22 Sep 2008 15:26:53 -0000ÂÂÂÂÂÂ1.79
> +++ breakpoint.hÂÂÂÂÂÂÂÂ8 Oct 2008 01:25:26 -0000
> @@ -884,4 +884,7 @@ extern int breakpoints_always_inserted_m
> Â Â in our opinion won't ever trigger. Â*/
> Âextern void breakpoint_retire_moribund (void);
> Â
> +/* Tell a breakpoint to be quiet. Â*/
> +extern void make_breakpoint_silent (struct breakpoint *);
> +
> Â#endif /* !defined (BREAKPOINT_H) */
> Index: infcmd.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/infcmd.c,v
> retrieving revision 1.212
> diff -u -p -r1.212 infcmd.c
> --- infcmd.cÂÂÂÂ22 Sep 2008 15:20:08 -0000ÂÂÂÂÂÂ1.212
> +++ infcmd.cÂÂÂÂ8 Oct 2008 01:25:26 -0000
> @@ -1366,6 +1366,67 @@ finish_command_continuation_free_arg (vo
> Â Âxfree (arg);
> Â}
> Â
> +/* finish_backward -- helper function for finish_command. Â*/
> +
> +static void
> +finish_backward (struct symbol *function, struct thread_info *tp)
> +{
> + Âstruct symtab_and_line sal;
> + Âstruct breakpoint *breakpoint;
> + Âstruct cleanup *old_chain;
> + ÂCORE_ADDR func_addr;
> + Âint back_up;
> +
> + Âif (find_pc_partial_function (get_frame_pc (get_current_frame ()),
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂNULL, &func_addr, NULL) == 0)
> + Â Âinternal_error (__FILE__, __LINE__,
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ Â Â_("Finish: couldn't find function."));
> +

Still internal_error?

> + Âsal = find_pc_line (func_addr, 0);
> +
> + Â/* TODO: Let's not worry about async until later. Â*/
> +

Should be an error here instead of on finish_command ...
(keep reading)

> + Â/* We don't need a return value. Â*/
> + Âtp->proceed_to_finish = 0;
> + Â/* Special case: if we're sitting at the function entry point,
> + Â Â then all we need to do is take a reverse singlestep. ÂWe
> + Â Â don't need to set a breakpoint, and indeed it would do us
> + Â Â no good to do so.
> +
> + Â Â Note that this can only happen at frame #0, since there's
> + Â Â no way that a function up the stack can have a return address
> + Â Â that's equal to its entry point. Â*/
> +
> + Âif (sal.pc != read_pc ())
> + Â Â{
> + Â Â Â/* Set breakpoint and continue. Â*/
> + Â Â Âbreakpoint =
> +ÂÂÂÂÂÂÂset_momentary_breakpoint (sal,
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ Âget_frame_id (get_selected_frame (NULL)),
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ Âbp_breakpoint);
> + Â Â Â/* Tell the breakpoint to keep quiet. ÂWe won't be done
> + Â Â Â Â until we've done another reverse single-step. Â*/
> + Â Â Âmake_breakpoint_silent (breakpoint);
> + Â Â Âold_chain = make_cleanup_delete_breakpoint (breakpoint);
> + Â Â Âproceed ((CORE_ADDR) -1, TARGET_SIGNAL_DEFAULT, 0);
> + Â Â Â/* We will be stopped when proceed returns. Â*/
> + Â Â Âback_up = bpstat_find_breakpoint (tp->stop_bpstat, breakpoint) != NULL;
> + Â Â Âdo_cleanups (old_chain);
> + Â Â}
> + Âelse
> + Â Âback_up = 1;
> + Âif (back_up)
> + Â Â{
> + Â Â Â/* If in fact we hit the step-resume breakpoint (and not
> +ÂÂÂÂÂÂÂ some other breakpoint), then we're almost there --
> +ÂÂÂÂÂÂÂ we just need to back up by one more single-step. Â*/
> + Â Â Â/* (Kludgy way of letting wait_for_inferior know...) */
> + Â Â Âtp->step_range_start = tp->step_range_end = 1;
> + Â Â Âproceed ((CORE_ADDR) -1, TARGET_SIGNAL_DEFAULT, 1);
> + Â Â}

I guess calling step_once doesn't help then?  Otherwise, "kludgy" here
sounds unfair... This is, after all, the interface used
to request a single-step even in forward direction ...

> + Âreturn;

Useless.

> +}
> +
> Â/* "finish": Set a temporary breakpoint at the place the selected
> Â Â frame will return to, then continue. Â*/
> Â
> @@ -1391,6 +1452,10 @@ finish_command (char *arg, int from_tty)
> Â Âif (async_exec && !target_can_async_p ())
> Â Â Âerror (_("Asynchronous execution not supported on this target."));
> Â
> + Â/* Don't try to async in reverse. Â*/
> + Âif (async_exec && execution_direction == EXEC_REVERSE)
> + Â Âerror (_("Asynchronous 'finish' not supported in reverse."));
> +
> Â Â/* If we are not asked to run in the bg, then prepare to run in the
> Â Â Â foreground, synchronously. Â*/
> Â Âif (!async_exec && target_can_async_p ())
> @@ -1412,13 +1477,6 @@ finish_command (char *arg, int from_tty)
> Â
> Â Âclear_proceed_status ();
> Â
> - Âsal = find_pc_line (get_frame_pc (frame), 0);
> - Âsal.pc = get_frame_pc (frame);
> -
> - Âbreakpoint = set_momentary_breakpoint (sal, get_frame_id (frame), bp_finish);
> -
> - Âold_chain = make_cleanup_delete_breakpoint (breakpoint);
> -
> Â Â/* Find the function we will return from. Â*/
> Â
> Â Âfunction = find_pc_function (get_frame_pc (get_selected_frame (NULL)));
> @@ -1427,10 +1485,29 @@ finish_command (char *arg, int from_tty)
> Â Â Â source. Â*/
> Â Âif (from_tty)
> Â Â Â{
> - Â Â Âprintf_filtered (_("Run till exit from "));
> + Â Â Âif (execution_direction == EXEC_REVERSE)
> +ÂÂÂÂÂÂÂprintf_filtered (_("Run back to call of "));
> + Â Â Âelse
> +ÂÂÂÂÂÂÂprintf_filtered (_("Run till exit from "));
> +
> Â Â Â Âprint_stack_frame (get_selected_frame (NULL), 1, LOCATION);
> Â Â Â}
> Â
> + Âif (execution_direction == EXEC_REVERSE)
> + Â Â{
> + Â Â Â/* Split off at this point. Â*/
> + Â Â Âfinish_backward (function, tp);
> + Â Â Âreturn;
> + Â Â}

Didn't Joel request you to not do this, but do,

finish_command
  if (execution_direction == EXEC_REVERSE)
   finish_backward ();
  else
   finish_forward ();

instead ?

(then, it makes sense to put the async + reverse error
inside finish_backward, IMHO.)

> +
> + Âsal = find_pc_line (get_frame_pc (frame), 0);
> + Âsal.pc = get_frame_pc (frame);
> +
> + Âbreakpoint = set_momentary_breakpoint (sal, get_frame_id (frame), 
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ bp_finish);
> +
> + Âold_chain = make_cleanup_delete_breakpoint (breakpoint);
> +
> Â Âtp->proceed_to_finish = 1;ÂÂÂ/* We want stop_registers, please... Â*/
> Â Âmake_cleanup_restore_integer (&suppress_stop_observer);
> Â Âsuppress_stop_observer = 1;

-- 
Pedro Alves


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]