[RFA]: Watchpoints per thread patch

[Jeff Johnston <jjohnstn at redhat dot com>]

The following patch adds needed support for the ia64 and s390 platforms.  For 
these platforms, watchpoints need to be inserted / removed on each thread so as 
to work across all threads.  The patch adds support for detecting this at 
configuration time and setting a new flag WATCHPOINTS_PER_THREAD.  This flag is 
used when inserting/removing watchpoints and when a new thread event occurs.

This patch in itself does not give these platforms threaded watchpoint support 
to pass the watchthreads.exp test case, but I am breaking up my bigger patch 
that works for x86, x86_64, and ia64.  It still fails on the S390 but gets part 
marks because threaded watchpoints actually trigger properly however the S390 
cannot determine which watchpoint gets triggered when multiple watchpoint events 
occur. On ia64, the watchthreads.exp test case will fail differently than before 
because the low-level register and watchpoint code does not properly calculate 
the LWP for accessing registers so we end up eating through the maximum number 
of watchpoints quicker than anticipated.  I have a subsequent patch for getting 
the LWP reliably, but this subsequent patch may be made unnecessary depending on 
what Daniel does with ptids and the thread-db layer.  Regardless of Daniel's 
redesign, the watchpoints still must be inserted/removed on each thread.

Ok to commit?

-- Jeff J.

2004-10-19  Jeff Johnston  <jjohnstn@redhat.com>

        * configure.host: For ia64 and s390 platforms, support watchpoints
        per thread.
        * configure.in: Set WATCHPOINTS_PER_THREAD flag if platform
        has watchpoints per thread.
	* config.in: Regenerated.
	* configure: Ditto.
        * thread.c (restore_current_thread): Add new silent flag argument.
        Do not issue message if switching threads and the silent flag is
        set.  Change all existing callers to call with new argument.
        Add call to find_thread_pid to safeguard against accessing an
        incomplete or invalid thread list item.
        (thread_switch_and_call): New function.
        * breakpoint.h: Define incomplete struct thread_info type.
        (insert_watchpoints_for_new_thread): New prototype.
        * breakpoint.c (insert_watchpoints_for_new_thread): New function.
        (insert_one_watchpoint): Ditto.
        (insert_watchpoint): Ditto.
        (remove_one_watchpoint): Ditto.
        (remove_watchpoint): Ditto.
        (remove_breakpoint): Call remove_watchpoint instead of
        target_remove_watchpoint.
        (insert_bp_location): Call insert_watchpoint instead of
        target_insert_watchpoint.
        (print_it_typical): Support watchpoints occurring at the same
        time as thread events.
        * thread-db.c (attach_thread)[WATCHPOINTS_PER_THREAD]: Call
        insert_watchpoints_for_new_thread.

Index: configure.host
===================================================================
RCS file: /cvs/src/src/gdb/configure.host,v
retrieving revision 1.84
diff -u -p -r1.84 configure.host
--- configure.host	1 Sep 2004 20:46:41 -0000	1.84
+++ configure.host	19 Oct 2004 22:59:47 -0000
@@ -179,3 +179,17 @@ hppa*-*-linux*)
 	gdb_host_long_double_format=0
 	;;
 esac
+
+# Set various watchpoint flags based on host/cpu
+
+case "${host}" in
+ia64-*-*)
+	gdb_host_watchpoints_per_thread=1
+	;;
+s390*-*-*)
+	gdb_host_watchpoints_per_thread=1
+	;;
+*)
+	gdb_host_watchpoints_per_thread=
+	;;
+esac
Index: configure.in
===================================================================
RCS file: /cvs/src/src/gdb/configure.in,v
retrieving revision 1.176
diff -u -p -r1.176 configure.in
--- configure.in	10 Oct 2004 15:55:49 -0000	1.176
+++ configure.in	19 Oct 2004 22:59:47 -0000
@@ -1414,6 +1414,11 @@ AC_DEFINE_UNQUOTED(GDB_HOST_FLOAT_FORMAT
 AC_DEFINE_UNQUOTED(GDB_HOST_DOUBLE_FORMAT,$gdb_host_double_format,[Host double floatformat])
 AC_DEFINE_UNQUOTED(GDB_HOST_LONG_DOUBLE_FORMAT,$gdb_host_long_double_format,[Host long double floatformat])
 
+# List of host watchpoint flags.
+if test "x$gdb_host_watchpoints_per_thread" != "x"; then
+    AC_DEFINE_UNQUOTED(WATCHPOINTS_PER_THREAD,$gdb_host_watchpoints_per_thread,[Host watchpoints are thread-specific])
+fi
+
 # target_subdir is used by the testsuite to find the target libraries.
 target_subdir=
 if test "${host}" != "${target}"; then
Index: thread.c
===================================================================
RCS file: /cvs/src/src/gdb/thread.c,v
retrieving revision 1.38
diff -u -p -r1.38 thread.c
--- thread.c	25 Aug 2004 15:18:05 -0000	1.38
+++ thread.c	19 Oct 2004 22:59:47 -0000
@@ -61,7 +61,7 @@ static void thread_apply_all_command (ch
 static int thread_alive (struct thread_info *);
 static void info_threads_command (char *, int);
 static void thread_apply_command (char *, int);
-static void restore_current_thread (ptid_t);
+static void restore_current_thread (ptid_t, int);
 static void switch_to_thread (ptid_t ptid);
 static void prune_threads (void);
 
@@ -468,34 +468,38 @@ switch_to_thread (ptid_t ptid)
 }
 
 static void
-restore_current_thread (ptid_t ptid)
+restore_current_thread (ptid_t ptid, int silent)
 {
-  if (!ptid_equal (ptid, inferior_ptid))
+  if (!ptid_equal (ptid, inferior_ptid) &&
+      find_thread_pid (ptid))
     {
       switch_to_thread (ptid);
-      print_stack_frame (get_current_frame (), 1, SRC_LINE);
+      if (!silent)
+	print_stack_frame (get_current_frame (), 1, SRC_LINE);
     }
 }
 
 struct current_thread_cleanup
 {
   ptid_t inferior_ptid;
+  int silent;
 };
 
 static void
 do_restore_current_thread_cleanup (void *arg)
 {
   struct current_thread_cleanup *old = arg;
-  restore_current_thread (old->inferior_ptid);
+  restore_current_thread (old->inferior_ptid, old->silent);
   xfree (old);
 }
 
 static struct cleanup *
-make_cleanup_restore_current_thread (ptid_t inferior_ptid)
+make_cleanup_restore_current_thread (ptid_t inferior_ptid, int silent)
 {
   struct current_thread_cleanup *old
     = xmalloc (sizeof (struct current_thread_cleanup));
   old->inferior_ptid = inferior_ptid;
+  old->silent = silent;
   return make_cleanup (do_restore_current_thread_cleanup, old);
 }
 
@@ -519,7 +523,7 @@ thread_apply_all_command (char *cmd, int
   if (cmd == NULL || *cmd == '\000')
     error ("Please specify a command following the thread ID list");
 
-  old_chain = make_cleanup_restore_current_thread (inferior_ptid);
+  old_chain = make_cleanup_restore_current_thread (inferior_ptid, 0);
 
   /* It is safe to update the thread list now, before
      traversing it for "thread apply all".  MVS */
@@ -560,7 +564,7 @@ thread_apply_command (char *tidlist, int
   if (*cmd == '\000')
     error ("Please specify a command following the thread ID list");
 
-  old_chain = make_cleanup_restore_current_thread (inferior_ptid);
+  old_chain = make_cleanup_restore_current_thread (inferior_ptid, 0);
 
   /* Save a copy of the command in case it is clobbered by
      execute_command */
@@ -616,6 +620,47 @@ thread_apply_command (char *tidlist, int
   do_cleanups (old_chain);
 }
 
+int
+thread_switch_and_call (int thread_num, int (*callback) (void *), void *args)
+{
+  struct thread_info *tp;
+  struct cleanup *old_chain;
+  int rc = -1;
+
+  old_chain = make_cleanup_restore_current_thread (inferior_ptid, 1);
+
+  if (thread_num == -1)
+    {
+      if (thread_list != NULL)
+	{
+	  for (tp = thread_list; tp; tp = tp->next)
+	    {
+	      if (thread_alive (tp))
+		{
+		  switch_to_thread (tp->ptid);
+		  rc = (*callback) (args);
+		  if (rc)
+		    break;
+		}
+	    }
+	}
+      else
+	rc = (*callback) (args);
+    }
+  else
+    {
+      for (tp = thread_list; tp; tp = tp->next)
+	if (tp->num == thread_num && thread_alive (tp))
+	  {
+	    switch_to_thread (tp->ptid);
+	    rc = (*callback) (args);
+	  }
+    }
+
+  do_cleanups (old_chain);
+  return rc;
+}
+
 /* Switch to the specified thread.  Will dispatch off to thread_apply_command
    if prefix of arg is `apply'.  */
 
Index: breakpoint.h
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.h,v
retrieving revision 1.34
diff -u -p -r1.34 breakpoint.h
--- breakpoint.h	13 May 2004 16:39:11 -0000	1.34
+++ breakpoint.h	19 Oct 2004 22:59:47 -0000
@@ -30,6 +30,7 @@
 
 struct value;
 struct block;
+struct thread_info;
 
 /* This is the maximum number of bytes a breakpoint instruction can take.
    Feel free to increase it.  It's just used in a few places to size
@@ -662,6 +663,7 @@ extern void rwatch_command_wrapper (char
 extern void tbreak_command (char *, int);
 
 extern int insert_breakpoints (void);
+extern int insert_watchpoints_for_new_thread (struct thread_info *);
 
 extern int remove_breakpoints (void);
 
Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.183
diff -u -p -r1.183 breakpoint.c
--- breakpoint.c	8 Oct 2004 17:30:46 -0000	1.183
+++ breakpoint.c	19 Oct 2004 22:59:48 -0000
@@ -748,6 +748,124 @@ insert_catchpoint (struct ui_out *uo, vo
   return 0;
 }
 
+struct target_watchpoint_args
+{
+  CORE_ADDR addr;
+  int len;
+  int type;
+};
+
+static int
+insert_one_watchpoint (void *data)
+{
+  struct target_watchpoint_args *args = (struct target_watchpoint_args *)data;
+
+  return target_insert_watchpoint (args->addr, args->len, args->type);
+}
+
+static int
+insert_watchpoint (struct bp_location *bpt, CORE_ADDR addr, int len, int type)
+{
+  int val;
+  struct target_watchpoint_args args;
+
+  args.addr = addr;
+  args.len = len;
+  args.type = type;
+
+  /* If watchpoints do not apply to all threads automatically, we have to insert
+     and delete them for every thread.  Otherwise, we can insert or delete them
+     once from any thread.  */
+#ifdef WATCHPOINTS_PER_THREAD
+  val = thread_switch_and_call (bpt->owner->thread, &insert_one_watchpoint, &args);
+#else
+  val = insert_one_watchpoint (&args);
+#endif
+
+  return val;
+}
+
+int
+insert_watchpoints_for_new_thread (struct thread_info *ti)
+{
+  struct bp_location *b;
+  int val = 0;
+  int return_val = 0;
+  struct ui_file *tmp_error_stream = mem_fileopen ();
+  make_cleanup_ui_file_delete (tmp_error_stream);
+
+  /* Explicitly mark the warning -- this will only be printed if
+     there was an error.  */
+  fprintf_unfiltered (tmp_error_stream, "Warning:\n");
+
+  ALL_BP_LOCATIONS (b)
+    {
+      /* Skip disabled breakpoints.  */
+      if (!breakpoint_enabled (b->owner))
+	continue;
+
+      /* For every active watchpoint, we need to insert the watchpoint on the new
+	 thread.  */
+      if ((b->loc_type == bp_loc_hardware_watchpoint
+	   || b->owner->type == bp_watchpoint))
+	{
+	  struct value *v = b->owner->val_chain;
+
+	  /* Look at each value on the value chain.  */
+	  for (; v; v = v->next)
+	    {
+	      /* If it's a memory location, and GDB actually needed
+		 its contents to evaluate the expression, then we
+		 must watch it.  */
+	      if (VALUE_LVAL (v) == lval_memory
+		  && ! VALUE_LAZY (v))
+		{
+		  struct type *vtype = check_typedef (VALUE_TYPE (v));
+		  
+		  /* We only watch structs and arrays if user asked
+		     for it explicitly, never if they just happen to
+		     appear in the middle of some value chain.  */
+		  if (v == b->owner->val_chain
+		      || (TYPE_CODE (vtype) != TYPE_CODE_STRUCT
+			  && TYPE_CODE (vtype) != TYPE_CODE_ARRAY))
+		    {
+		      CORE_ADDR addr;
+		      int len, type;
+		      struct target_watchpoint_args args;
+		      
+		      addr = VALUE_ADDRESS (v) + VALUE_OFFSET (v);
+		      len = TYPE_LENGTH (VALUE_TYPE (v));
+		      type = hw_write;
+		      if (b->owner->type == bp_read_watchpoint)
+			type = hw_read;
+		      else if (b->owner->type == bp_access_watchpoint)
+			type = hw_access;
+		      args.addr = addr;
+		      args.len = len;
+		      args.type = type;
+		      val = thread_switch_and_call (ti->num, &insert_one_watchpoint, &args); 
+		    }
+		}
+	    }
+	}
+
+      if (val)
+	return_val = val;
+    }
+
+  /* Failure to insert a watchpoint on any memory value in the
+     value chain brings us here.  */
+  if (return_val)
+    {
+      fprintf_unfiltered (tmp_error_stream,
+			  "%s\n",
+			  "Could not insert hardware watchpoints on new thread."); 
+      target_terminal_ours_for_output ();
+      error_stream (tmp_error_stream);
+    }
+  return return_val;
+}
+
 /* Helper routine: free the value chain for a breakpoint (watchpoint).  */
 
 static void free_valchain (struct bp_location *b)
@@ -982,7 +1100,7 @@ insert_bp_location (struct bp_location *
 		      else if (bpt->owner->type == bp_access_watchpoint)
 			type = hw_access;
 
-		      val = target_insert_watchpoint (addr, len, type);
+		      val = insert_watchpoint (bpt, addr, len, type);
 		      if (val == -1)
 			{
 			  /* Don't exit the loop, try to insert
@@ -1404,6 +1522,36 @@ detach_breakpoints (int pid)
 }
 
 static int
+remove_one_watchpoint (void *data)
+{
+  struct target_watchpoint_args *args = (struct target_watchpoint_args *)data;
+
+  return target_remove_watchpoint (args->addr, args->len, args->type);
+}
+
+static int
+remove_watchpoint (struct bp_location *bpt, CORE_ADDR addr, int len, int type)
+{
+  int val;
+  struct target_watchpoint_args args;
+
+  args.addr = addr;
+  args.len = len;
+  args.type = type;
+
+  /* If watchpoints do not apply to all threads automatically, we have to insert
+     and delete them for every thread.  Otherwise, we can insert or delete them
+     once from any thread.  */
+#ifdef WATCHPOINTS_PER_THREAD
+  val = thread_switch_and_call (bpt->owner->thread, &remove_one_watchpoint, &args);
+#else
+  val = remove_one_watchpoint (&args);
+#endif
+
+  return val;
+}
+
+static int
 remove_breakpoint (struct bp_location *b, insertion_state_t is)
 {
   int val;
@@ -1512,7 +1660,7 @@ remove_breakpoint (struct bp_location *b
 		  else if (b->owner->type == bp_access_watchpoint)
 		    type = hw_access;
 
-		  val = target_remove_watchpoint (addr, len, type);
+		  val = remove_watchpoint (b, addr, len, type);
 		  if (val == -1)
 		    b->inserted = 1;
 		  val = 0;
@@ -2122,8 +2270,13 @@ print_it_typical (bpstat bs)
       break;
 
     case bp_thread_event:
-      /* Not sure how we will get here. 
-	 GDB should not stop for these breakpoints.  */
+      /* We can only get here legitimately if something further on the bs 
+       * list has caused the stop status to be noisy.  A valid example
+       * of this is a new thread event and a software watchpoint have
+       * both occurred.  */
+      if (bs->next)
+        return PRINT_UNKNOWN;
+
       printf_filtered ("Thread Event Breakpoint: gdb should not stop!\n");
       return PRINT_NOTHING;
       break;
Index: thread-db.c
===================================================================
RCS file: /cvs/src/src/gdb/thread-db.c,v
retrieving revision 1.46
diff -u -p -r1.46 thread-db.c
--- thread-db.c	8 Oct 2004 20:29:56 -0000	1.46
+++ thread-db.c	19 Oct 2004 22:59:48 -0000
@@ -762,6 +762,13 @@ attach_thread (ptid_t ptid, const td_thr
   ATTACH_LWP (BUILD_LWP (ti_p->ti_lid, GET_PID (ptid)), 0);
 #endif
 
+  /* For some platforms, watchpoints must be applied to each and every
+     thread.  When a new thread is created, all current watchpoints need
+     to be inserted.  */
+#ifdef WATCHPOINTS_PER_THREAD
+  insert_watchpoints_for_new_thread (tp);
+#endif
+
   /* Enable thread event reporting for this thread.  */
   err = td_thr_event_enable_p (th_p, 1);
   if (err != TD_OK)