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

Doug Evans dje@google.com
Fri Mar 13 17:10:00 GMT 2009


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.
>
> 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);
>


Ping.

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.]



More information about the Gdb-patches mailing list