Re: [RFA]: Clean up debug printing of pc in gdbserver

[Doug Evans <dje at google dot com>]

Ping.
Attached is an updated patch (now that x86 gdbserver is biarch).

On Wed, May 6, 2009 at 8:11 PM, Doug Evans <dje@google.com> wrote:
> Hi.
>
> The debug printing of pc in linux-i386-low.c/linux-x86-64-low.c can
> use some clean up.
>
> - why restrict the printing to just x86?
> - the text that is printed for linux_resume_one_lwp is confusing
>
> Ok to check in?
>
> 2009-05-06 ?Doug Evans ?<dje@google.com>
>
> ? ? ? ?* linux-x86-64-low.c (debug_threads): Remove declaration.
> ? ? ? ?(x86_64_get_pc,x86_64_set_pc): Remove debug printing of pc.
> ? ? ? ?* linux-i386-low.c (debug_threads): Remove declaration.
> ? ? ? ?(i386_get_pc,i386_set_pc): Remove debug printing of pc.
> ? ? ? ?* linux-low.c (get_stop_pc): Print pc if debug_threads.
> ? ? ? ?(check_removed_breakpoint, linux_wait_for_lwp): Ditto.
> ? ? ? ?(linux_resume_one_lwp): Ditto.
>
> Index: linux-low.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/gdbserver/linux-low.c,v
> retrieving revision 1.99
> diff -u -p -r1.99 linux-low.c
> --- linux-low.c 6 May 2009 17:32:59 -0000 ? ? ? 1.99
> +++ linux-low.c 7 May 2009 02:10:55 -0000
> @@ -290,10 +290,14 @@ get_stop_pc (void)
> ?{
> ? CORE_ADDR stop_pc = (*the_low_target.get_pc) ();
>
> - ?if (get_thread_lwp (current_inferior)->stepping)
> - ? ?return stop_pc;
> - ?else
> - ? ?return stop_pc - the_low_target.decr_pc_after_break;
> + ?if (! get_thread_lwp (current_inferior)->stepping)
> + ? ?stop_pc -= the_low_target.decr_pc_after_break;
> +
> + ?if (debug_threads)
> + ? ?fprintf (stderr, "stop pc is %08lx\n",
> + ? ? ? ? ? ?(long) stop_pc);
> +
> + ?return stop_pc;
> ?}
>
> ?static void *
> @@ -750,7 +754,11 @@ check_removed_breakpoint (struct lwp_inf
> ? ? ?decrement. ?We go immediately from this function to resuming,
> ? ? ?and can not safely call get_stop_pc () again. ?*/
> ? if (the_low_target.set_pc != NULL)
> - ? ?(*the_low_target.set_pc) (stop_pc);
> + ? ?{
> + ? ? ?if (debug_threads)
> + ? ? ? fprintf (stderr, "Set pc to %08lx.\n", (long) stop_pc);
> + ? ? ?(*the_low_target.set_pc) (stop_pc);
> + ? ?}
>
> ? /* We consumed the pending SIGTRAP. ?*/
> ? event_child->pending_is_breakpoint = 0;
> @@ -878,14 +886,14 @@ retry:
> ? ? }
>
> ? if (debug_threads
> - ? ? ?&& WIFSTOPPED (*wstatp))
> + ? ? ?&& WIFSTOPPED (*wstatp)
> + ? ? ?&& the_low_target.get_pc != NULL)
> ? ? {
> ? ? ? struct thread_info *saved_inferior = current_inferior;
> + ? ? ?CORE_ADDR stop_pc = (*the_low_target.get_pc) ();
> ? ? ? current_inferior = (struct thread_info *)
> ? ? ? ?find_inferior_id (&all_threads, child->head.id);
> - ? ? ?/* For testing only; i386_stop_pc prints out a diagnostic. ?*/
> - ? ? ?if (the_low_target.get_pc != NULL)
> - ? ? ? get_stop_pc ();
> + ? ? ?fprintf (stderr, "linux_wait_for_lwp: pc is %08lx\n", (long) stop_pc);
> ? ? ? current_inferior = saved_inferior;
> ? ? }
>
> @@ -1644,8 +1652,8 @@ linux_resume_one_lwp (struct inferior_li
>
> ? if (debug_threads && the_low_target.get_pc != NULL)
> ? ? {
> - ? ? ?fprintf (stderr, " ?");
> - ? ? ?(*the_low_target.get_pc) ();
> + ? ? ?CORE_ADDR pc = (*the_low_target.get_pc) ();
> + ? ? ?fprintf (stderr, " ?resuming from pc 0x%lx\n", (long) pc);
> ? ? }
>
> ? /* If we have pending signals, consume one unless we are trying to reinsert
> Index: linux-x86-64-low.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/gdbserver/linux-x86-64-low.c,v
> retrieving revision 1.22
> diff -u -p -r1.22 linux-x86-64-low.c
> --- linux-x86-64-low.c ?22 Mar 2009 23:57:10 -0000 ? ? ?1.22
> +++ linux-x86-64-low.c ?7 May 2009 03:03:09 -0000
> @@ -128,8 +128,6 @@ struct regset_info target_regsets[] = {
> ?static const unsigned char x86_64_breakpoint[] = { 0xCC };
> ?#define x86_64_breakpoint_len 1
>
> -extern int debug_threads;
> -
> ?static CORE_ADDR
> ?x86_64_get_pc ()
> ?{
> @@ -137,16 +135,12 @@ x86_64_get_pc ()
>
> ? collect_register_by_name ("rip", &pc);
>
> - ?if (debug_threads)
> - ? ?fprintf (stderr, "stop pc (before any decrement) is %08lx\n", pc);
> ? return pc;
> ?}
>
> ?static void
> ?x86_64_set_pc (CORE_ADDR newpc)
> ?{
> - ?if (debug_threads)
> - ? ?fprintf (stderr, "set pc to %08lx\n", (long) newpc);
> ? supply_register_by_name ("rip", &newpc);
> ?}
>
> Index: linux-i386-low.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/gdbserver/linux-i386-low.c,v
> retrieving revision 1.19
> diff -u -p -r1.19 linux-i386-low.c
> --- linux-i386-low.c ? ?22 Mar 2009 23:57:10 -0000 ? ? ?1.19
> +++ linux-i386-low.c ? ?7 May 2009 03:03:09 -0000
> @@ -154,8 +154,6 @@ struct regset_info target_regsets[] = {
> ?static const unsigned char i386_breakpoint[] = { 0xCC };
> ?#define i386_breakpoint_len 1
>
> -extern int debug_threads;
> -
> ?static CORE_ADDR
> ?i386_get_pc ()
> ?{
> @@ -163,16 +161,12 @@ i386_get_pc ()
>
> ? collect_register_by_name ("eip", &pc);
>
> - ?if (debug_threads)
> - ? ?fprintf (stderr, "stop pc (before any decrement) is %08lx\n", pc);
> ? return pc;
> ?}
>
> ?static void
> ?i386_set_pc (CORE_ADDR newpc)
> ?{
> - ?if (debug_threads)
> - ? ?fprintf (stderr, "set pc to %08lx\n", (long) newpc);
> ? supply_register_by_name ("eip", &newpc);
> ?}
>
>

Attachment: gdb-090523-gdbserver-get-pc-2.patch.txt
Description: Text document