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

[Pedro Alves <pedro@codesourcery.com>]

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