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]

[RFA]: Modified Watchthreads Patch


The following is a modified version of my thread watchpoint patch from October/November. It removes the code I had used to switch between lwp ptids and thread ptids now that Daniel's lwp patch is in place. It uses the former version of my observer that is linux-specific and is activated in attach_thread in linux-thread-db.c. Eli, I renamed the observer as asked to indicate this.

I also addressed Ulrich's comments regarding simplifying the S390 code and using the s390_fix_watch_points call to actually put the watchpoints on the new thread.

Ulrich/Daniel can you take a look to verify everything is in place. Daniel, I realize that this touches files that are currently in patch state for you. I have no problem waiting for your latest patch to apply and retrofitting my changes at check-in if necessary.

As I mentioned before, more is required to get ia64 threaded watchpoints to work. For S390, this change allows it to set and recognize threaded watchpoints.

-- Jeff J.

2004-12-09 Jeff Johnston <jjohnstn@redhat.com>

        * breakpoint.c (insert_watchpoints_for_new_thread): New function.
        (print_it_typical): Do not issue an error for bp_thread_event
        if a subsequent event is on the chain.
        * 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_stopped_data_address): Call target_get_lwp to ensure
        the ptid has its lwp field filled in.
        (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 linux thread observer.
        * linux-nat.h (struct linux_watchpoint): New structure.
        * linux-thread-db.c (attach_thread): Notify all linux new thread
        observers.
        * s390-nat.c (s390_tid): 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_watchpoint_callback): New function.
        (s390_insert_watchpoint): Change to iterate through lwps and
        insert the specified watchpoint on each thread.
        (s390_linux_new_thread): New linux thread observer.
        (_initialize_s390_nat): New function.  Initialize
        new linux thread observer.
        * Makefile.in (s390-nat.o, ia64-linux-nat.o): Add new header file
        dependencies.

doc/ChangeLog

2004-12-09 Jeff Johnston <jjohnstn@redhat.com>

* observer.texi (linux_new_thread): New observer.


Index: Makefile.in
===================================================================
RCS file: /cvs/src/src/gdb/Makefile.in,v
retrieving revision 1.676
diff -u -p -r1.676 Makefile.in
--- Makefile.in	7 Dec 2004 22:17:59 -0000	1.676
+++ Makefile.in	9 Dec 2004 23:18:41 -0000
@@ -2059,7 +2059,8 @@ ia64-aix-nat.o: ia64-aix-nat.c $(defs_h)
 	$(objfiles_h) $(gdb_stat_h)
 ia64-aix-tdep.o: ia64-aix-tdep.c $(defs_h)
 ia64-linux-nat.o: ia64-linux-nat.c $(defs_h) $(gdb_string_h) $(inferior_h) \
-	$(target_h) $(gdbcore_h) $(regcache_h) $(gdb_wait_h) $(gregset_h)
+	$(target_h) $(gdbcore_h) $(regcache_h) $(gdb_wait_h) $(gregset_h) \
+	$(linux_nat_h) $(observer_h)
 ia64-linux-tdep.o: ia64-linux-tdep.c $(defs_h) $(ia64_tdep_h) \
 	$(arch_utils_h) $(gdbcore_h) $(regcache_h)
 ia64-tdep.o: ia64-tdep.c $(defs_h) $(inferior_h) $(gdbcore_h) \
@@ -2440,7 +2441,7 @@ rs6000-tdep.o: rs6000-tdep.c $(defs_h) $
 	$(ppc_tdep_h) $(gdb_assert_h) $(dis_asm_h) $(trad_frame_h) \
 	$(frame_unwind_h) $(frame_base_h)
 s390-nat.o: s390-nat.c $(defs_h) $(tm_h) $(regcache_h) $(inferior_h) \
-	$(s390_tdep_h)
+	$(s390_tdep_h) $(observer_h) $(linux_nat_h)
 s390-tdep.o: s390-tdep.c $(defs_h) $(arch_utils_h) $(frame_h) $(inferior_h) \
 	$(symtab_h) $(target_h) $(gdbcore_h) $(gdbcmd_h) $(objfiles_h) \
 	$(tm_h) $(__bfd_bfd_h) $(floatformat_h) $(regcache_h) \
Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.186
diff -u -p -r1.186 breakpoint.c
--- breakpoint.c	1 Dec 2004 06:54:56 -0000	1.186
+++ breakpoint.c	9 Dec 2004 23:18:42 -0000
@@ -738,6 +738,90 @@ 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 where 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)
+	{
+	  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)
@@ -2112,8 +2196,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: 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	9 Dec 2004 23:18:42 -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	9 Dec 2004 23:18:42 -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 linux_watchpoint *args = (struct linux_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 linux_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 linux_watchpoint *args = (struct linux_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 linux_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,19 @@ 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.  */
+static 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_linux_new_thread (ia64_linux_new_thread);
+}
+    
Index: linux-nat.h
===================================================================
RCS file: /cvs/src/src/gdb/linux-nat.h,v
retrieving revision 1.6
diff -u -p -r1.6 linux-nat.h
--- linux-nat.h	29 Mar 2004 18:07:14 -0000	1.6
+++ linux-nat.h	9 Dec 2004 23:18:42 -0000
@@ -63,6 +63,14 @@ struct lwp_info
   struct lwp_info *next;
 };
 
+/* Watchpoint description.  */
+struct linux_watchpoint
+{
+  CORE_ADDR addr;
+  int len;
+  int type;
+};
+
 /* Read/write to target memory via the Linux kernel's "proc file
    system".  */
 struct mem_attrib;
Index: linux-thread-db.c
===================================================================
RCS file: /cvs/src/src/gdb/linux-thread-db.c,v
retrieving revision 1.2
diff -u -p -r1.2 linux-thread-db.c
--- linux-thread-db.c	8 Dec 2004 15:10:30 -0000	1.2
+++ linux-thread-db.c	9 Dec 2004 23:18:42 -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>
@@ -713,6 +714,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
@@ -748,11 +750,18 @@ 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
 
+  /* Notify any observers of a new linux thread.  This
+     would include any linux platforms that have to insert hardware
+     watchpoints on every thread.  */
+  observer_notify_linux_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: 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	9 Dec 2004 23:18:42 -0000
@@ -27,6 +27,8 @@
 #include "inferior.h"
 
 #include "s390-tdep.h"
+#include "linux-nat.h"
+#include "observer.h"
 
 #include <asm/ptrace.h>
 #include <sys/ptrace.h>
@@ -112,14 +114,14 @@ 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;
 }
@@ -203,7 +205,7 @@ store_fpregs (int tid, int regnum)
 void
 fetch_inferior_registers (int regnum)
 {
-  int tid = s390_inferior_tid ();
+  int tid = s390_tid (inferior_ptid);
 
   if (regnum == -1 
       || (regnum < S390_NUM_REGS && regmap_gregset[regnum] != -1))
@@ -219,7 +221,7 @@ fetch_inferior_registers (int regnum)
 void
 store_inferior_registers (int regnum)
 {
-  int tid = s390_inferior_tid ();
+  int tid = s390_tid (inferior_ptid);
 
   if (regnum == -1 
       || (regnum < S390_NUM_REGS && regmap_gregset[regnum] != -1))
@@ -261,7 +263,7 @@ s390_stopped_by_watchpoint (void)
   parea.len = sizeof (per_lowcore);
   parea.process_addr = (addr_t) & per_lowcore;
   parea.kernel_addr = offsetof (struct user_regs_struct, per_info.lowcore);
-  if (ptrace (PTRACE_PEEKUSR_AREA, s390_inferior_tid (), &parea) < 0)
+  if (ptrace (PTRACE_PEEKUSR_AREA, s390_tid (inferior_ptid), &parea) < 0)
     perror_with_name ("Couldn't retrieve watchpoint status");
 
   return per_lowcore.perc_storage_alteration == 1
@@ -269,9 +271,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,6 +310,16 @@ s390_fix_watch_points (void)
     perror_with_name ("Couldn't modify watchpoint status");
 }
 
+/* Callback routine to use with iterate_over_lwps to insert a specified
+   watchpoint on all threads.  */
+static int
+s390_insert_watchpoint_callback (struct lwp_info *lwp, void *data)
+{
+  s390_fix_watch_points (lwp->ptid);
+  return 0;
+}
+
+/* Insert a specified watchpoint on all threads.  */
 int
 s390_insert_watchpoint (CORE_ADDR addr, int len)
 {
@@ -321,10 +333,24 @@ s390_insert_watchpoint (CORE_ADDR addr, 
   area->next = watch_base;
   watch_base = area;
 
-  s390_fix_watch_points ();
+  /* For the S390, a watchpoint must be inserted/removed for each
+     thread so we iterate over the list of existing lwps.  */
+  if (iterate_over_lwps (&s390_insert_watchpoint_callback, NULL))
+    return -1;
+
+  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)
+{
+  s390_fix_watch_points (lwp->ptid);
   return 0;
 }
 
+/* Remove a specified watchpoint from all threads.  */
 int
 s390_remove_watchpoint (CORE_ADDR addr, int len)
 {
@@ -346,7 +372,11 @@ s390_remove_watchpoint (CORE_ADDR addr, 
   *parea = area->next;
   xfree (area);
 
-  s390_fix_watch_points ();
+  /* For the S390, a watchpoint must be inserted/removed for each
+     thread so we iterate over the list of existing lwps.  */
+  if (iterate_over_lwps (&s390_remove_watchpoint_callback, NULL))
+    return -1;
+
   return 0;
 }
 
@@ -357,3 +387,19 @@ kernel_u_size (void)
   return sizeof (struct user);
 }
 
+/* New thread observer that inserts all existing watchpoints on the
+   new thread.  */
+static void
+s390_linux_new_thread (ptid_t ptid)
+{
+  /* Add existing watchpoints to new thread.  */
+  s390_fix_watch_points (ptid);
+}
+
+void
+_initialize_s390_nat (void)
+{
+  observer_attach_linux_new_thread (s390_linux_new_thread);
+}
+
+
Index: observer.texi
===================================================================
RCS file: /cvs/src/src/gdb/doc/observer.texi,v
retrieving revision 1.8
diff -u -p -r1.8 observer.texi
--- observer.texi	1 Sep 2004 17:59:37 -0000	1.8
+++ observer.texi	9 Dec 2004 23:31:58 -0000
@@ -95,3 +95,8 @@ inferior, and before any information on 
 The specified shared library has been discovered to be unloaded.
 @end deftypefun
 
+@deftypefun void linux_new_thread (ptid_t @var{ptid})
+A new linux thread described by @var{ptid} has been officially attached
+to by gdb.
+@end deftypefun
+

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