This is the mail archive of the gdb-patches@sources.redhat.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFA]: Watchpoints per thread patch


Daniel Jacobowitz wrote:
On Wed, Oct 20, 2004 at 01:27:15PM -0400, Andrew Cagney wrote:

the underlying target (ia64-linux-nat, ...) can locally override the method and handle the problem. The code's the same, but how it is wired up is different

Sound reasonable to all?


I think that sounds pretty good.  Hopefully the changes involved will
be pretty small, since I imagine that most GNU/Linux targets with
hardware watchpoints will want it.


The attached patch is the rework of my original attempt. It no longer uses configuration or magic defines. Per Mark's suggestion, it uses an observer to handle inserting watchpoints on a new thread and only the low-level code knows about inserting/removing watchpoints on all threads.


Ok to commit?

-- Jeff J.

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

        * breakpoint.c (insert_watchpoints_for_new_thread): New function.
        * breakpoint.h (insert_watchpoints_for_new_thread): New prototype.
        * ia64-linux-nat.c (ia64_linux_insert_one_watchpoint): New function.
        (ia64_linux_insert_watchpoint_callback): Ditto.
        (ia64_linux_insert_watchpoint): Change to iterate through lwps
        and insert the specified watchpoint per thread.
        (ia64_linux_remove_one_watchpoint): New function.
        (ia64_linux_remove_watchpoint_callback): Ditto.
        (ia64_linux_remove_watchpoint): Change to iterate through lwps and
        remove the specified watchpoint for each thread.
        (ia64_linux_new_thread): New thread observer.
        (_initialize_ia64_linux_nat): New function.  Initialize
        new thread observer.
        * thread-db.c (attach_thread): Notify any observers of the new
        thread event.
        * s390-nat.c (s390_tid): New function.
        (s390_inferior_tid): Change to call s390_tid.
        (s390_remove_one_watchpoint): New function.
        (s390_remove_watchpoint_callback): Ditto.
        (s390_remove_watchpoint): Change to iterate through lwps and
        remove the specified watchpoint for each thread.
        (s390_insert_one_watchpoint): New function.
        (s390_insert_watchpoint_callback): Ditto.
        (s390_insert_watchpoint): Change to iterate through lwps and
        insert the specified watchpoint on each thread.
        (s390_new_thread): New thread observer.
        (_initialize_s390_nat): New function.  Initialize
        new thread observer.

doc/ChangeLog:

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

* observer.texi (new_thread): New observer.

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	27 Oct 2004 21:43:50 -0000
@@ -748,6 +748,91 @@ insert_catchpoint (struct ui_out *uo, vo
   return 0;
 }
 
+/* External function to insert all existing watchpoints on a newly
+   attached thread.  IWPFN is a callback function to perform
+   the target insert watchpoint.  This function is used to support
+   platforms whereby a watchpoint must be inserted/removed on each
+   individual thread (e.g. ia64-linux and s390-linux).  For
+   ia64 and s390 linux, this function is called via a new thread
+   observer.  */
+int
+insert_watchpoints_for_new_thread (ptid_t new_thread, 
+				   insert_watchpoint_ftype *iwpfn)
+{
+  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;
+		      
+		      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;
+		      val = (*iwpfn) (new_thread, addr, len, type); 
+		    }
+		}
+	    }
+	}
+
+      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)
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	27 Oct 2004 21:43:50 -0000
@@ -663,6 +663,12 @@ extern void tbreak_command (char *, int)
 
 extern int insert_breakpoints (void);
 
+/* The following provides a callback mechanism to insert watchpoints
+   for a new thread.  This is needed, for example, on ia64 linux.  */
+typedef int (insert_watchpoint_ftype) (ptid_t, CORE_ADDR, int, int);
+extern int insert_watchpoints_for_new_thread (ptid_t ptid,
+					      insert_watchpoint_ftype *fn);
+
 extern int remove_breakpoints (void);
 
 /* This function can be used to physically insert eventpoints from the
Index: ia64-linux-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/ia64-linux-nat.c,v
retrieving revision 1.26
diff -u -p -r1.26 ia64-linux-nat.c
--- ia64-linux-nat.c	13 Oct 2004 21:40:41 -0000	1.26
+++ ia64-linux-nat.c	27 Oct 2004 21:43:50 -0000
@@ -39,6 +39,8 @@
 
 #include <asm/ptrace_offsets.h>
 #include <sys/procfs.h>
+#include "observer.h"
+#include "linux-nat.h"
 
 /* Prototypes for supply_gregset etc. */
 #include "gregset.h"
@@ -559,8 +561,9 @@ is_power_of_2 (int val)
   return onecount <= 1;
 }
 
-int
-ia64_linux_insert_watchpoint (ptid_t ptid, CORE_ADDR addr, int len, int rw)
+/* Internal routine to insert one watchpoint for a specified thread.  */
+static int
+ia64_linux_insert_one_watchpoint (ptid_t ptid, CORE_ADDR addr, int len, int rw)
 {
   int idx;
   long dbr_addr, dbr_mask;
@@ -606,8 +609,38 @@ ia64_linux_insert_watchpoint (ptid_t pti
   return 0;
 }
 
+/* Internal callback routine which can be used via iterate_over_lwps
+   to insert a specific watchpoint from all active threads.  */
+static int
+ia64_linux_insert_watchpoint_callback (struct lwp_info *lwp, void *data)
+{
+  struct target_watchpoint *args = (struct target_watchpoint *)data;
+
+  return ia64_linux_insert_one_watchpoint (lwp->ptid, args->addr,
+		 		     	   args->len, args->type);
+}
+
+/* Insert a watchpoint for all threads.  */
 int
-ia64_linux_remove_watchpoint (ptid_t ptid, CORE_ADDR addr, int len)
+ia64_linux_insert_watchpoint (ptid_t ptid, CORE_ADDR addr, int len, int rw)
+{
+  struct target_watchpoint args;
+
+  args.addr = addr;
+  args.len = len;
+  args.type = rw;
+
+  /* For ia64, watchpoints must be inserted/removed on each thread so
+     we iterate over the lwp list.  */
+  if (iterate_over_lwps (&ia64_linux_insert_watchpoint_callback, &args))
+    return -1;
+
+  return 0;
+}
+
+/* Internal routine to remove one watchpoint for a specified thread.  */
+static int
+ia64_linux_remove_one_watchpoint (ptid_t ptid, CORE_ADDR addr, int len)
 {
   int idx;
   long dbr_addr, dbr_mask;
@@ -630,6 +663,34 @@ ia64_linux_remove_watchpoint (ptid_t pti
   return -1;
 }
 
+/* Internal callback routine which can be used via iterate_over_lwps
+   to remove a specific watchpoint from all active threads.  */
+static int
+ia64_linux_remove_watchpoint_callback (struct lwp_info *lwp, void *data)
+{
+  struct target_watchpoint *args = (struct target_watchpoint *)data;
+
+  return ia64_linux_remove_one_watchpoint (lwp->ptid, args->addr,
+		 		     	   args->len);
+}
+
+/* Remove a watchpoint for all threads.  */
+int
+ia64_linux_remove_watchpoint (ptid_t ptid, CORE_ADDR addr, int len)
+{
+  struct target_watchpoint args;
+
+  args.addr = addr;
+  args.len = len;
+
+  /* For ia64, watchpoints must be inserted/removed on each thread so
+     we iterate over the lwp list.  */
+  if (iterate_over_lwps (&ia64_linux_remove_watchpoint_callback, &args))
+    return -1;
+
+  return 0;
+}
+
 int
 ia64_linux_stopped_data_address (CORE_ADDR *addr_p)
 {
@@ -674,3 +735,18 @@ ia64_linux_xfer_unwind_table (struct tar
 {
   return syscall (__NR_getunwind, readbuf, len);
 }
+
+/* Observer function for a new thread attach.  We need to insert
+   existing watchpoints on the new thread.  */
+void
+ia64_linux_new_thread (ptid_t ptid)
+{
+  insert_watchpoints_for_new_thread (ptid, &ia64_linux_insert_one_watchpoint);
+}
+
+void
+_initialize_ia64_linux_nat (void)
+{
+  observer_attach_new_thread (ia64_linux_new_thread);
+}
+    
Index: s390-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/s390-nat.c,v
retrieving revision 1.12
diff -u -p -r1.12 s390-nat.c
--- s390-nat.c	18 Feb 2004 04:17:35 -0000	1.12
+++ s390-nat.c	27 Oct 2004 21:43:50 -0000
@@ -112,18 +112,25 @@ fill_fpregset (fpregset_t *regp, int reg
 			      ((char *)regp) + regmap_fpregset[i]);
 }
 
-/* Find the TID for the current inferior thread to use with ptrace.  */
+/* Find the TID for use with ptrace.  */
 static int
-s390_inferior_tid (void)
+s390_tid (ptid_t ptid)
 {
   /* GNU/Linux LWP ID's are process ID's.  */
-  int tid = TIDGET (inferior_ptid);
+  int tid = TIDGET (ptid);
   if (tid == 0)
-    tid = PIDGET (inferior_ptid); /* Not a threaded program.  */
+    tid = PIDGET (ptid); /* Not a threaded program.  */
 
   return tid;
 }
 
+/* Find the TID for the current inferior thread to use with ptrace.  */
+static int
+s390_inferior_tid (void)
+{
+  return s390_tid (inferior_ptid);
+}
+
 /* Fetch all general-purpose registers from process/thread TID and
    store their values in GDB's register cache.  */
 static void
@@ -269,9 +276,9 @@ s390_stopped_by_watchpoint (void)
 }
 
 static void
-s390_fix_watch_points (void)
+s390_fix_watch_points (ptid_t ptid)
 {
-  int tid = s390_inferior_tid ();
+  int tid = s390_tid (ptid);
 
   per_struct per_info;
   ptrace_area parea;
@@ -308,8 +315,9 @@ s390_fix_watch_points (void)
     perror_with_name ("Couldn't modify watchpoint status");
 }
 
-int
-s390_insert_watchpoint (CORE_ADDR addr, int len)
+/* Insert a specified watchpoint on a specified thread.  */
+static int
+s390_insert_one_watchpoint (ptid_t ptid, CORE_ADDR addr, int len, int type)
 {
   struct watch_area *area = xmalloc (sizeof (struct watch_area));
   if (!area)
@@ -321,12 +329,36 @@ s390_insert_watchpoint (CORE_ADDR addr, 
   area->next = watch_base;
   watch_base = area;
 
-  s390_fix_watch_points ();
+  s390_fix_watch_points (ptid);
   return 0;
 }
 
+/* Callback routine to use with iterate_over_lwps to insert a specified
+   watchpoint from all threads.  */
+static int
+s390_insert_watchpoint_callback (struct lwp_info *lwp, void *data)
+{
+  struct target_watchpoint *args = (struct target_watchpoint *)data;
+
+  return s390_insert_one_watchpoint (lwp->ptid, args->addr, args->len, 0);
+}
+
+/* Insert a specified watchpoint on all threads.  */
 int
-s390_remove_watchpoint (CORE_ADDR addr, int len)
+s390_insert_watchpoint (CORE_ADDR addr, int len)
+{
+  struct target_watchpoint args;
+
+  args.addr = addr;
+  args.len = len;
+  /* For the S390, a watchpoint must be inserted/removed for each
+     thread so we iterate over the list of existing lwps.  */
+  return iterate_over_lwps (&s390_insert_watchpoint_callback, &args);
+}
+
+/* Remove a specified watchpoint from a specified thread.  */
+static int
+s390_remove_one_watchpoint (ptid_t ptid, CORE_ADDR addr, int len)
 {
   struct watch_area *area, **parea;
 
@@ -346,10 +378,32 @@ s390_remove_watchpoint (CORE_ADDR addr, 
   *parea = area->next;
   xfree (area);
 
-  s390_fix_watch_points ();
+  s390_fix_watch_points (ptid);
   return 0;
 }
 
+/* Callback routine to use with iterate_over_lwps to remove a specified
+   watchpoint from all threads.  */
+static int
+s390_remove_watchpoint_callback (struct lwp_info *lwp, void *data)
+{
+  struct s390_watchpoint_args *args = (struct s390_watchpoint_args *)data;
+
+  return s390_remove_one_watchpoint (lwp->ptid, args->addr, args->len);
+}
+
+/* Remove a specified watchpoint from all threads.  */
+int
+s390_remove_watchpoint (CORE_ADDR addr, int len)
+{
+  struct s390_watchpoint_args args;
+
+  args.addr = addr;
+  args.len = len;
+  /* For the S390, a watchpoint must be inserted/removed for each
+     thread so we iterate over the list of existing lwps.  */
+  return iterate_over_lwps (&s390_remove_watchpoint_callback, &args);
+}
 
 int
 kernel_u_size (void)
@@ -357,3 +411,18 @@ kernel_u_size (void)
   return sizeof (struct user);
 }
 
+/* New thread observer that inserts all existing watchpoints on the
+   new thread.  */
+static int
+s390_new_thread (ptid_t ptid)
+{
+  return insert_watchpoints_for_new_thread (ptid, &s390_insert_one_watchpoint);
+}
+
+void
+_initialize_s390_nat (void)
+{
+  observer_attach_new_thread (s390_new_thread);
+}
+
+
Index: target.h
===================================================================
RCS file: /cvs/src/src/gdb/target.h,v
retrieving revision 1.65
diff -u -p -r1.65 target.h
--- target.h	8 Oct 2004 20:29:55 -0000	1.65
+++ target.h	27 Oct 2004 21:43:51 -0000
@@ -178,6 +178,15 @@ extern char *target_signal_to_name (enum
 /* Given a name (SIGHUP, etc.), return its signal.  */
 enum target_signal target_signal_from_name (char *);
 
+
+/* Watchpoint specification.  */
+struct target_watchpoint
+  {
+    CORE_ADDR addr;
+    int len;
+    int type;
+  };
+
 /* Request the transfer of up to LEN 8-bit bytes of the target's
    OBJECT.  The OFFSET, for a seekable object, specifies the starting
    point.  The ANNEX can be used to provide additional data-specific
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	27 Oct 2004 21:43:51 -0000
@@ -34,6 +34,7 @@
 #include "target.h"
 #include "regcache.h"
 #include "solib-svr4.h"
+#include "observer.h"
 
 #ifdef HAVE_GNU_LIBC_VERSION_H
 #include <gnu/libc-version.h>
@@ -722,6 +723,7 @@ attach_thread (ptid_t ptid, const td_thr
 {
   struct thread_info *tp;
   td_err_e err;
+  ptid_t new_ptid;
 
   /* If we're being called after a TD_CREATE event, we may already
      know about this thread.  There are two ways this can happen.  We
@@ -757,11 +759,16 @@ attach_thread (ptid_t ptid, const td_thr
   if (ti_p->ti_state == TD_THR_UNKNOWN || ti_p->ti_state == TD_THR_ZOMBIE)
     return;			/* A zombie thread -- do not attach.  */
 
+  new_ptid = BUILD_LWP (ti_p->ti_lid, GET_PID (ptid));
+
   /* Under GNU/Linux, we have to attach to each and every thread.  */
 #ifdef ATTACH_LWP
-  ATTACH_LWP (BUILD_LWP (ti_p->ti_lid, GET_PID (ptid)), 0);
+  ATTACH_LWP (new_ptid, 0);
 #endif
 
+  /* Inform any observers of new attached thread.  */
+  observer_notify_new_thread (new_ptid);
+
   /* Enable thread event reporting for this thread.  */
   err = td_thr_event_enable_p (th_p, 1);
   if (err != TD_OK)
Index: doc/observer.texi
===================================================================
RCS file: /cvs/src/src/gdb/doc/observer.texi,v
retrieving revision 1.8
diff -u -p -r1.8 observer.texi
--- doc/observer.texi	1 Sep 2004 17:59:37 -0000	1.8
+++ doc/observer.texi	27 Oct 2004 21:43:51 -0000
@@ -95,3 +95,7 @@ inferior, and before any information on 
 The specified shared library has been discovered to be unloaded.
 @end deftypefun
 
+@deftypefun void new_thread (ptid_t @var{ptid})
+A new thread has been attached to.
+@end deftypefun
+

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]