Re: [RFC] When debugging infrun, print stop_pc in all possible cases.

[Pedro Alves <pedro at codesourcery dot com>]

On Friday 13 March 2009 17:06:13, Doug Evans wrote:
> On Thu, Feb 26, 2009 at 11:01 AM, Doug Evans <dje@google.com> wrote:
> > Hi.
> >
> > I've been debugging a few issues and it's been really helpful to know
> > stop_pc in cases that aren't printed today.
> > This patch moves the debug printing of stop_pc up.
> >
> > I'm not submitting it RFA because I'm not entirely happy with it.
> > Printing stop_pc has been helpful, I'd like to improve what's there today,
> > but it's not clear to me that this patch reasonably handles the cases
> > when one can't read pc before the big switch() on ecs->ws.kind.

Hmm,:

 - not ignoring TARGET_WAITKIND_IGNORE.
 - accessing the target before the set_executing call.  It may end
   up being a problem in the future.
 - With displaced stepping, we lost the printing of the stop_pc
   *after* the fixup.
 - I'm not sure we want to move the watchpoint debug bits
   as well.

> Any comments on this approach?
> Would you do it differently?
> [E.g. One could add code to print the pc to all the appropriate cases
> in the switch(), but that seems clumsy.]

Dunno, perhaps, come up with a "set_stop_pc (CORE_ADDR)" function, do
the debug printing there, and replace all the appropriate accesses
to that global by a call to the new function?

This is probably one of the cases where I'd hack the debug
output, fix the issue, and then remove the hack and forget
about it.  :-)

> >
> > Comments?
> >
> > 2009-02-26  Doug Evans  <dje@google.com>
> >
> >        * infrun.c (handle_inferior_event): Move the debug printing of stop_pc,
> >        and possible watchpoint address, to an earlier point so we print it
> >        in all cases (where it's possible).
> >
> > Index: infrun.c
> > ===================================================================
> > RCS file: /cvs/src/src/gdb/infrun.c,v
> > retrieving revision 1.360
> > diff -u -p -r1.360 infrun.c
> > --- infrun.c    25 Feb 2009 02:14:22 -0000      1.360
> > +++ infrun.c    26 Feb 2009 17:52:28 -0000
> > @@ -2178,6 +2178,46 @@ handle_inferior_event (struct execution_
> >   /* Dependent on the current PC value modified by adjust_pc_after_break.  */
> >   reinit_frame_cache ();
> >
> > +  if (debug_infrun)
> > +    {
> > +      /* It's helpful to know the stop pc in all cases when we stop
> > +        (even if we're eventually going to resume).
> > +        To keep things simple we print it here before the big switch on
> > +        ecs->ws.kind.  If we leave printing of stop_pc until later,
> > +        we will miss the cases where we "return;" early.
> > +        There are a few cases where we can't read pc though.  */
> > +
> > +      switch (ecs->ws.kind)
> > +       {
> > +       case TARGET_WAITKIND_EXITED:
> > +       case TARGET_WAITKIND_SIGNALLED:
> > +         break;
> > +
> > +       default:
> > +         {
> > +           CORE_ADDR debug_stop_pc =
> > +             regcache_read_pc (get_thread_regcache (ecs->ptid));
> > +
> > +           fprintf_unfiltered (gdb_stdlog, "infrun: stop_pc = 0x%s\n",
> > +                               paddr_nz (debug_stop_pc));
> > +
> > +           if (STOPPED_BY_WATCHPOINT (&ecs->ws))
> > +             {
> > +               CORE_ADDR addr;
> > +
> > +               if (target_stopped_data_address (&current_target, &addr))
> > +                 fprintf_unfiltered (gdb_stdlog,
> > +                                     "infrun: stopped by watchpoint, data address = 0x%s\n",
> > +                                     paddr_nz (addr));
> > +               else
> > +                 fprintf_unfiltered (gdb_stdlog,
> > +                                     "infrun: stopped by watchpoint, no data address available\n");
> > +             }
> > +           break;
> > +         }
> > +       }
> > +    }
> > +
> >   if (ecs->ws.kind != TARGET_WAITKIND_IGNORE)
> >     {
> >       breakpoint_retire_moribund ();
> > @@ -2507,25 +2547,6 @@ targets should add new threads to the th
> >
> >   stop_pc = regcache_read_pc (get_thread_regcache (ecs->ptid));
> >
> > -  if (debug_infrun)
> > -    {
> > -      fprintf_unfiltered (gdb_stdlog, "infrun: stop_pc = 0x%s\n",
> > -                          paddr_nz (stop_pc));
> > -      if (STOPPED_BY_WATCHPOINT (&ecs->ws))
> > -       {
> > -          CORE_ADDR addr;
> > -         fprintf_unfiltered (gdb_stdlog, "infrun: stopped by watchpoint\n");
> > -
> > -          if (target_stopped_data_address (&current_target, &addr))
> > -            fprintf_unfiltered (gdb_stdlog,
> > -                                "infrun: stopped data address = 0x%s\n",
> > -                                paddr_nz (addr));
> > -          else
> > -            fprintf_unfiltered (gdb_stdlog,
> > -                                "infrun: (no data address available)\n");
> > -       }
> > -    }
> > -
> >   if (stepping_past_singlestep_breakpoint)
> >     {
> >       gdb_assert (singlestep_breakpoints_inserted_p);
> >
> 

-- 
Pedro Alves