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

[Doug Evans <dje@google.com>]

[-- Attachment #1: Type: text/plain, Size: 1838 bytes --]

On Sun, May 24, 2009 at 9:47 AM, Pedro Alves <pedro@codesourcery.com> wrote:
>
>> 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.
>
> Looks OK to me.  Small nit below.
>
> On Thursday 07 May 2009 04:11:08, Doug Evans wrote:
>>    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;
>>      }
>
> Can we rename that `stop_pc' variable to, say, `pc', so we're
> consistent throughout?  We can think of the `stop_pc' as having
> always the decr_pc_after_break adjustment applied.

Thanks.

Attached is what I checked in.

[-- Attachment #2: gdb-090524-gdbserver-get-pc-3.patch.txt --]
[-- Type: text/plain, Size: 2486 bytes --]

2009-05-06  Doug Evans  <dje@google.com>

	* 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.103
diff -u -p -r1.103 linux-low.c
--- linux-low.c	24 May 2009 01:09:22 -0000	1.103
+++ linux-low.c	24 May 2009 17:27:10 -0000
@@ -354,10 +354,13 @@ 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 0x%lx\n", (long) stop_pc);
+
+  return stop_pc;
 }
 
 static void *
@@ -814,7 +817,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 0x%lx\n", (long) stop_pc);
+      (*the_low_target.set_pc) (stop_pc);
+    }
 
   /* We consumed the pending SIGTRAP.  */
   event_child->pending_is_breakpoint = 0;
@@ -942,14 +949,16 @@ retry:
     }
 
   if (debug_threads
-      && WIFSTOPPED (*wstatp))
+      && WIFSTOPPED (*wstatp)
+      && the_low_target.get_pc != NULL)
     {
       struct thread_info *saved_inferior = current_inferior;
+      CORE_ADDR 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 ();
+      pc = (*the_low_target.get_pc) ();
+      fprintf (stderr, "linux_wait_for_lwp: pc is 0x%lx\n", (long) pc);
       current_inferior = saved_inferior;
     }
 
@@ -1706,8 +1715,8 @@ linux_resume_one_lwp (struct lwp_info *l
 
   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