Re: [PATCH, ppc] Fix hw *points for embedded ppc in a threaded environment

[Pedro Alves <palves at redhat dot com>]

I hacked GDB some more, and I have further insight.
GDB _is_ to blame.  Updated GDB/test hack patch at the bottom.

New gdb.log snippet:

infrun: resume (step=0, signal=0), trap_expected=0, current thread [Thread 0x7ffff7fcb700 (LWP 29305)] at 0x3d25a068d2
LLR: Preparing to resume process 29301, 0, inferior_ptid Thread 0x7ffff7fcb700 (LWP 29305)
RC: Resuming sibling Thread 0x7ffff77ca700 (LWP 29306), 0, resume
RC: Not resuming sibling Thread 0x7ffff7fcb700 (LWP 29305) (not stopped)
RC: Resuming sibling Thread 0x7ffff7fcc740 (LWP 29301), 0, resume
Thread 1 [0xf77ca700 (LWP 29306)] changing watch_thread[1] data from 4 -> 5
LLR: PTRACE_CONT process 29305, 0 (resume event thread)
sigchld
infrun: prepare_to_wait
linux_nat_wait: [process -1], []
LLW: enter
LNW: waitpid(-1, ...) returned 29306, ERRNO-OK
LLW: waitpid 29306 received Trace/breakpoint trap (stopped)
LLTA: KILL(SIG0) Thread 0x7ffff77ca700 (LWP 29306) (OK)
LLW: Candidate event Trace/breakpoint trap (stopped) in Thread 0x7ffff77ca700 (LWP 29306).
SC:  kill Thread 0x7ffff7fcb700 (LWP 29305) **<SIGSTOP>**
SC:  lwp kill 0 ERRNO-OK
SC:  kill Thread 0x7ffff7fcc740 (LWP 29301) **<SIGSTOP>**
SC:  lwp kill 0 ERRNO-OK
WL: waitpid Thread 0x7ffff7fcb700 (LWP 29305) received Trace/breakpoint trap (stopped)
SWC: Pending event Trace/breakpoint trap (stopped) in Thread 0x7ffff7fcb700 (LWP 29305)
WL: waitpid Thread 0x7ffff7fcc740 (LWP 29301) received Stopped (signal) (stopped)
SWC: Delayed SIGSTOP caught for Thread 0x7ffff7fcc740 (LWP 29301).
SEL: Found 2 SIGTRAP events, selecting #1
LLW: trap ptid is Thread 0x7ffff7fcb700 (LWP 29305).
LLW: exit
sigchld
infrun: target_wait (-1, status) =
infrun:   29301 [Thread 0x7ffff7fcb700 (LWP 29305)],
infrun:   status->kind = stopped, signal = SIGTRAP
infrun: infwait_normal_state
infrun: TARGET_WAITKIND_STOPPED
infrun: stop_pc = 0x3d25a0f680
breakpoint: forced reparse
breakpoint: value reparsed. New=5
breakpoint: forced reparse
breakpoint: value reparsed. New=5
infrun: BPSTAT_WHAT_SINGLE
infrun: no stepping, continue
infrun: resume (step=1, signal=0), trap_expected=1, current thread [Thread 0x7ffff7fcb700 (LWP 29305)] at 0x3d25a0f680
LLR: Preparing to step Thread 0x7ffff7fcb700 (LWP 29305), 0, inferior_ptid Thread 0x7ffff7fcb700 (LWP 29305)
LLR: PTRACE_SINGLESTEP process 29305, 0 (resume event thread)
sigchld
infrun: prepare_to_wait
linux_nat_wait: [process -1], []
LLW: enter
LNW: waitpid(-1, ...) returned 29305, ERRNO-OK
LLW: waitpid 29305 received Stopped (signal) (stopped)
LLTA: KILL(SIG0) Thread 0x7ffff7fcb700 (LWP 29305) (OK)
LLW: Delayed SIGSTOP caught for Thread 0x7ffff7fcb700 (LWP 29305).
LLW: PTRACE_SINGLESTEP Thread 0x7ffff7fcb700 (LWP 29305), 0, 0 (discard SIGSTOP)
LNW: waitpid(-1, ...) returned 29305, ERRNO-OK
LLW: waitpid 29305 received Trafce/breakpoint trap (stopped)
LLTA: KILL(SIG0) Thread 0x7ffff7fcb700 (LWP 29305) (OK)
LLW: Candidate event Trace/breakpoint trap (stopped) in Thread 0x7ffff7fcb700 (LWP 29305).
SEL: Select single-step Thread 0x7ffff7fcb700 (LWP 29305)
LLW: trap ptid is Thread 0x7ffff7fcb700 (LWP 29305).
LLW: exit
sigchld
infrun: target_wait (-1, status) =
infrun:   29301 [Thread 0x7ffff7fcb700 (LWP 29305)],
infrun:   status->kind = stopped, signal = SIGTRAP
infrun: infwait_normal_state
infrun: TARGET_WAITKIND_STOPPED
infrun: stop_pc = 0x3d25a130a5
infrun: no stepping, continue
infrun: resume (step=0, signal=0), trap_expected=0, current thread [Thread 0x7ffff7fcb700 (LWP 29305)] at 0x3d25a130a5
LLR: Preparing to resume process 29301, 0, inferior_ptid Thread 0x7ffff7fcb700 (LWP 29305)
RC: Not resuming sibling Thread 0x7ffff77ca700 (LWP 29306) (has pending)
RC: Not resuming sibling Thread 0x7ffff7fcb700 (LWP 29305) (not stopped)
RC: Resuming sibling Thread 0x7ffff7fcc740 (LWP 29301), 0, resume
LLR: PTRACE_CONT process 29305, 0 (resume event thread)
sigchld
infrun: prepare_to_wait
linux_nat_wait: [process -1], []
LLW: enter
LLW: Using pending wait status Trace/breakpoint trap (stopped) for Thread 0x7ffff77ca700 (LWP 29306).
LLW: Candidate event Trace/breakpoint trap (stopped) in Thread 0x7ffff77ca700 (LWP 29306).
SC:  kill Thread 0x7ffff7fcb700 (LWP 29305) **<SIGSTOP>**
SC:  lwp kill 0 ERRNO-OK
SC:  kill Thread 0x7ffff7fcc740 (LWP 29301) **<SIGSTOP>**
SC:  lwp kill 0 ERRNO-OK
WL: waitpid Thread 0x7ffff7fcb700 (LWP 29305) received Trace/breakpoint trap (stopped)
SWC: Pending event Trace/breakpoint trap (stopped) in Thread 0x7ffff7fcb700 (LWP 29305)
WL: waitpid Thread 0x7ffff7fcc740 (LWP 29301) received Stopped (signal) (stopped)
SWC: Delayed SIGSTOP caught for Thread 0x7ffff7fcc740 (LWP 29301).
SEL: Found 2 SIGTRAP events, selecting #0
CB: Push back breakpoint for Thread 0x7ffff7fcb700 (LWP 29305)
LLW: trap ptid is Thread 0x7ffff77ca700 (LWP 29306).
LLW: exit
sigchld
infrun: target_wait (-1, status) =
infrun:   29301 [Thread 0x7ffff77ca700 (LWP 29306)],
infrun:   status->kind = stopped, signal = SIGTRAP
infrun: infwait_normal_state
infrun: TARGET_WAITKIND_STOPPED
infrun: stop_pc = 0x400b71
infrun: stopped by watchpoint
infrun: stopped data address = 0x6012e4
infrun: context switch
infrun: Switching context from Thread 0x7ffff7fcb700 (LWP 29305) to Thread 0x7ffff77ca700 (LWP 29306)
breakpoint: nothing changed. Old=5, New=5




So we can now see from the above that the watchpoint is ignored
because Old=5 and New=5.  But why does GDB think the old value of
the watchpoint is 5?  Something must have forced the watchpoint's
value to be refetched without informing the user.  And indeed,
extra logging in breakpoint.c shows:

breakpoint: forced reparse
breakpoint: value reparsed. New=5

The patch that triggers that is re_set_watchpoint.  Okay,
so why do we have watchpoint re-sets going on?  The log
shows threads stopping at 0x3d25a0f680, which is,

(gdb) info symbol 0x3d25a0f680
_dl_debug_state in section .text of /lib64/ld-linux-x86-64.so.2

ahhhhhhhhhh.  So to program is stopping at DSO events (though
the user doesn't see this).  With "set stop-on-solib-events 1",
we see:

(gdb) bt
#0  __GI__dl_debug_state () at dl-debug.c:76
#1  0x0000003d25a068d2 in _dl_map_object_from_fd (name=name@entry=0x3d26a10acf "libgcc_s.so.1", fd=fd@entry=12, fbp=fbp@entry=0x7ffff77ca5f0,
    realname=0x7ffff00008c0 "/lib64/libgcc_s.so.1", loader=loader@entry=0x0, l_type=l_type@entry=2, mode=mode@entry=-1879048191,
    stack_endp=stack_endp@entry=0x7ffff77ca5e8, nsid=nsid@entry=0) at dl-load.c:1044
#2  0x0000003d25a082f3 in _dl_map_object (loader=0x0, name=name@entry=0x3d26a10acf "libgcc_s.so.1", type=type@entry=2, trace_mode=trace_mode@entry=0,
    mode=mode@entry=-1879048191, nsid=nsid@entry=0) at dl-load.c:2355
#3  0x0000003d25a12fec in dl_open_worker (a=a@entry=0x7ffff77cab90) at dl-open.c:226
#4  0x0000003d25a0eca6 in _dl_catch_error (objname=objname@entry=0x7ffff77cab80, errstring=errstring@entry=0x7ffff77cab88, mallocedp=mallocedp@entry=0x7ffff77cab70,
    operate=operate@entry=0x3d25a12ec0 <dl_open_worker>, args=args@entry=0x7ffff77cab90) at dl-error.c:178
#5  0x0000003d25a12b1c in _dl_open (file=0x3d26a10acf "libgcc_s.so.1", mode=-2147483647, caller_dlopen=<optimized out>, nsid=-2, argc=1, argv=0x7fffffffdc08,
    env=0x7fffffffdc18) at dl-open.c:652
#6  0x0000003d25f2d212 in do_dlopen () from /lib64/libc.so.6
#7  0x0000003d25a0eca6 in _dl_catch_error (objname=0x7ffff77cad80, errstring=0x7ffff77cad90, mallocedp=0x7ffff77cad70, operate=0x3d25f2d1d0 <do_dlopen>,
    args=0x7ffff77cada0) at dl-error.c:178
#8  0x0000003d25f2d2d2 in __libc_dlopen_mode () from /lib64/libc.so.6
#9  0x0000003d26a0f704 in pthread_cancel_init () at ../nptl/sysdeps/pthread/unwind-forcedunwind.c:53
#10 0x0000003d26a0f8dc in _Unwind_ForcedUnwind (exc=<optimized out>, stop=stop@entry=0x3d26a0da60 <unwind_stop>, stop_argument=<optimized out>)
    at ../nptl/sysdeps/pthread/unwind-forcedunwind.c:130
#11 0x0000003d26a0dbf0 in __GI___pthread_unwind (buf=<optimized out>) at unwind.c:130
#12 0x0000003d26a08d35 in __do_cancel () at pthreadP.h:265
#13 __pthread_exit (value=<optimized out>) at pthread_exit.c:30
#14 0x0000000000400bab in thread_function (arg=0x1) at ../../../src/gdb/testsuite/gdb.threads/wp-replication.c:146
#15 0x0000003d26a07d14 in start_thread (arg=0x7ffff77cb700) at pthread_create.c:309
#16 0x0000003d25ef168d in clone () from /lib64/libc.so.6
(gdb)

That's a thread exiting normally.  (Notice #13).

IOW, when the first thread exits, pthread_cancel_init does:

   handle = __libc_dlopen (LIBGCC_S_SO);

and GDB catches that and forces a watchpoint re-set, losing
the watchpoint's old value...

So GDB is to blame.  Fixing this, however, would be a different
story, and doesn't look that simple.

So the test could work around this by making sure that threads
don't exit until after all watchpoints have been tested.

--------
diff --git c/gdb/breakpoint.c w/gdb/breakpoint.c
index 2757c6b..35d3411 100644
--- c/gdb/breakpoint.c
+++ w/gdb/breakpoint.c
@@ -106,6 +106,9 @@ static void map_breakpoint_numbers (char *, void (*) (struct breakpoint *,
 						      void *),
 				    void *);

+static void
+watchpoint_value_print (struct value *val, struct ui_file *stream);
+
 static void ignore_command (char *, int);

 static int breakpoint_re_set_one (void *);
@@ -1788,6 +1791,8 @@ update_watchpoint (struct watchpoint *b, int reparse)
       b->val = NULL;
       b->val_valid = 0;

+      fprintf_unfiltered (gdb_stdlog, "breakpoint: forced reparse\n");
+
       /* Note that unlike with breakpoints, the watchpoint's condition
 	 expression is stored in the breakpoint object, not in the
 	 locations (re)created below.  */
@@ -1831,6 +1836,18 @@ update_watchpoint (struct watchpoint *b, int reparse)
 	 watchpoints.  */
       if (!b->val_valid && !is_masked_watchpoint (&b->base))
 	{
+	  struct cleanup *old_chain;
+	  struct ui_file *new_stb = mem_fileopen ();
+	  long len;
+
+	  old_chain = make_cleanup_ui_file_delete (new_stb);
+
+	  watchpoint_value_print (v, new_stb);
+
+	  fprintf_unfiltered (gdb_stdlog, "breakpoint: value reparsed. New=%s\n",
+			      ui_file_xstrdup (new_stb, &len));
+	  do_cleanups (old_chain);
+
 	  b->val = v;
 	  b->val_valid = 1;
 	}
@@ -4800,6 +4817,27 @@ watchpoint_check (void *p)
 	      release_value (new_val);
 	      value_free_to_mark (mark);
 	    }
+
+
+
+	  {
+	  struct cleanup *old_chain;
+	  struct ui_file *old_stb = mem_fileopen ();
+	  struct ui_file *new_stb = mem_fileopen ();
+	  long len;
+
+	  old_chain = make_cleanup_ui_file_delete (old_stb);
+	  make_cleanup_ui_file_delete (new_stb);
+
+	  watchpoint_value_print (b->val, old_stb);
+	  watchpoint_value_print (new_val, new_stb);
+
+	  fprintf_unfiltered (gdb_stdlog, "breakpoint: value changed. Old=%s, New=%s\n",
+			      ui_file_xstrdup (old_stb, &len),
+			      ui_file_xstrdup (new_stb, &len));
+	  do_cleanups (old_chain);
+	  }
+
 	  bs->old_val = b->val;
 	  b->val = new_val;
 	  b->val_valid = 1;
@@ -4807,6 +4845,22 @@ watchpoint_check (void *p)
 	}
       else
 	{
+	  struct cleanup *old_chain;
+	  struct ui_file *old_stb = mem_fileopen ();
+	  struct ui_file *new_stb = mem_fileopen ();
+	  long len;
+
+	  old_chain = make_cleanup_ui_file_delete (old_stb);
+	  make_cleanup_ui_file_delete (new_stb);
+
+	  watchpoint_value_print (b->val, old_stb);
+	  watchpoint_value_print (new_val, new_stb);
+
+	  fprintf_unfiltered (gdb_stdlog, "breakpoint: nothing changed. Old=%s, New=%s\n",
+			      ui_file_xstrdup (old_stb, &len),
+			      ui_file_xstrdup (new_stb, &len));
+	  do_cleanups (old_chain);
+
 	  /* Nothing changed.  */
 	  value_free_to_mark (mark);
 	  return WP_VALUE_NOT_CHANGED;
@@ -14722,6 +14776,8 @@ invalidate_bp_value_on_memory_change (struct inferior *inferior,
 		  value_free (wp->val);
 		  wp->val = NULL;
 		  wp->val_valid = 0;
+
+		  fprintf_unfiltered (gdb_stdlog, "breakpoint: on memory change\n");
 		}
 	  }
       }
diff --git c/gdb/testsuite/gdb.threads/wp-replication.c w/gdb/testsuite/gdb.threads/wp-replication.c
index 4d79d56..0543b67 100644
--- c/gdb/testsuite/gdb.threads/wp-replication.c
+++ w/gdb/testsuite/gdb.threads/wp-replication.c
@@ -44,7 +44,7 @@ int test_ready = 0;
 int watch_count_done = 0;

 /* Array with elements we can create watchpoints for.  */
-static int watched_data[NR_BYTES];
+static int watched_data[NR_TRIGGERS_PER_THREAD * NR_THREADS];
 pthread_mutex_t data_mutex;

 /* Wait function to keep threads busy while the testcase does
@@ -109,6 +109,9 @@ thread_started ()
 {
 }

+#include <sys/syscall.h>
+#define gettid() syscall (__NR_gettid)
+
 void *
 thread_function (void *arg)
 {
@@ -121,24 +124,24 @@ thread_function (void *arg)
   while (!test_ready)
     usleep (1);

+  pthread_mutex_lock (&data_mutex);
   for (i = 0; i < NR_TRIGGERS_PER_THREAD; ++i)
     {
       for (j = 0; j < NR_THREADS; j++)
 	{
-   pthread_mutex_lock (&data_mutex);
 	  /* For debugging.  */
-   printf ("Thread %ld changing watch_thread[%d] data"
-     " from %d -> %d\n", thread_number, j,
+	  printf ("Thread %ld [0x%x (LWP %d)] changing watch_thread[%d] data"
+		  " from %d -> %d\n", thread_number, pthread_self (), gettid (), j,
 		  watched_data[j], watched_data[j] + 1);
 	  /* The call to usleep is so that when the watchpoint triggers,
 	     the pc is still on the same line.  */
 	  /* Increment the watched data field.  */
 	  ++watched_data[j];
-   usleep (1);
+	  usleep (5);

-   pthread_mutex_unlock (&data_mutex);
 	}
     }
+  pthread_mutex_unlock (&data_mutex);

   pthread_exit (NULL);
 }
diff --git c/gdb/testsuite/gdb.threads/wp-replication.exp w/gdb/testsuite/gdb.threads/wp-replication.exp
index 449423f..937c9e6 100644
--- c/gdb/testsuite/gdb.threads/wp-replication.exp
+++ w/gdb/testsuite/gdb.threads/wp-replication.exp
@@ -19,8 +19,8 @@
 # threads when the hardware watchpoint is created.


-set NR_THREADS 4
-set NR_TRIGGERS_PER_THREAD 10
+set NR_THREADS 2
+set NR_TRIGGERS_PER_THREAD 4
 set NR_BYTES 100

 # This test verifies that a hardware watchpoint gets replicated to
@@ -105,6 +105,9 @@ gdb_test "break thread_started" \
 	 "Breakpoint \[0-9\]+ at .*: file .*${srcfile}, line .*" \
 	 "Breakpoint on thread_started"

+verbose -log "hwatch_count = $hwatch_count"
+verbose -log "NR_THREADS = $NR_THREADS"
+
 # Move all threads to where they're supposed to be for testing.
 for { set i 0 } { $i < $NR_THREADS } { incr i} {

@@ -117,6 +120,8 @@ for { set i 0 } { $i < $NR_THREADS } { incr i} {
       gdb_test "watch watched_data\[$i\]" \
 	"Hardware watchpoint .*" \
 	"watch watched_data\[$i\]"
+    } else {
+	verbose -log "not setting watchpoint for watched_data\[$i\]\n"
     }

     gdb_test continue "Continuing\\..*Breakpoint \[0-9\]+, thread_started \\(\\) at .*$srcfile.*" \
@@ -131,6 +136,9 @@ gdb_test_no_output "set var test_ready=1" \
 # Set the number of expected watchpoint triggers.
 set TRIGGERS [expr "$NR_THREADS * $NR_THREADS * $NR_TRIGGERS_PER_THREAD"]

+gdb_test_no_output "set debug infrun 1"
+gdb_test_no_output "set debug lin-lwp 1"
+
 # Move the threads and hit the watchpoints
 # TRIGGERS times.
 for { set i 0 } { $i < $TRIGGERS } { incr i} {