[RFC]: x86 threaded watchpoint support [1/3]

[Jeff Johnston <jjohnstn@redhat.com>]

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

The following patch gets threaded watchpoint support working for the x86.  On 
x86 linux, the dr_status register is thread-specific.  This means that the 
current method which uses the PID to call PTRACE is wrong.  I have changed this 
to use the current lwp for the inferior_ptid.  Corresponding to this, the 
i386_stopped_data_address function switches the inferior_ptid to the trap_ptid. 
  Thus, we can see if we really stopped for a watchpoint or hardware breakpoint.

Because the thread-db.c code changes the trap_ptid into a high-level ptid (pid + 
tid), I had to add a new target vector interface which gives back the lwp for a 
given ptid.  This is used by the low level dr get routine.

With this change plus a change to breakpoint.c, threaded watchpoints work on the 
x86.  Part 2 will be the breakpoint.c change which is not x86-specific.  Part 3 
will be a test case when I figure out how to code it properly.  If you want to 
test it with the two patches applied, just debug gdb.threads/schedlock and issue:

b main
run
watch args[0]
watch args[1]
continue
continue
continue
etc...

Ok?

-- Jeff J.

2004-06-11  Jeff Johnston  <jjohnstn@redhat.com>

         * i386-linux-nat.c (i386_linux_dr_get): Use target_get_lwp
         function to get lwp of current ptid to use in the PTRACE
         call.
         * i386-nat.c: Add comments regarding the DR status register.
         (i386_stopped_data_address): Switch to the trap_ptid before
         getting the dr_status register.
         (i386_stopped_by_hwbp): Ditto.
         * lin-lwp.c (child_wait): Set the trap_ptid when appropriate.
         * target.h (struct target ops): Add new function to_get_lwp.
         (target_get_lwp): New macro.
         * target.c (INHERIT): Inherit to_get_lwp.
         (update_current_target): Default to_get_lwp to ptid_get_lwp.
         (init_dummy_target): Ditto.
         * thread-db.c (thread_db_get_lwp): New function.
         (thread_db_init): Expose thread_db_get_lwp in target vector.


[-- Attachment #2: x86-watchpoint.patch1 --]
[-- Type: text/plain, Size: 8214 bytes --]

Index: i386-linux-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/i386-linux-nat.c,v
retrieving revision 1.55
diff -u -p -r1.55 i386-linux-nat.c
--- i386-linux-nat.c	9 Apr 2004 16:31:01 -0000	1.55
+++ i386-linux-nat.c	11 Jun 2004 20:55:28 -0000
@@ -649,10 +649,11 @@ i386_linux_dr_get (int regnum)
   int tid;
   unsigned long value;
 
-  /* FIXME: kettenis/2001-01-29: It's not clear what we should do with
-     multi-threaded processes here.  For now, pretend there is just
-     one thread.  */
-  tid = PIDGET (inferior_ptid);
+  /* The debug status register is thread-specific and so we should
+     use the current lwp when fetching the debug registers.  */
+  tid = target_get_lwp (inferior_ptid);
+  if (tid == 0)
+    tid = PIDGET (inferior_ptid); /* Not a threaded program.  */
 
   /* FIXME: kettenis/2001-03-27: Calling perror_with_name if the
      ptrace call fails breaks debugging remote targets.  The correct
Index: i386-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/i386-nat.c,v
retrieving revision 1.8
diff -u -p -r1.8 i386-nat.c
--- i386-nat.c	5 Mar 2004 20:58:00 -0000	1.8
+++ i386-nat.c	11 Jun 2004 20:55:28 -0000
@@ -166,7 +166,10 @@
 #define ALL_DEBUG_REGISTERS(i)	for (i = 0; i < DR_NADDR; i++)
 
 /* Mirror the inferior's DRi registers.  We keep the status and
-   control registers separated because they don't hold addresses.  */
+   control registers separated because they don't hold addresses.  
+ 
+   Note that the DR_STATUS register is thread-specific and the
+   mirror value should be refreshed as necessary.  */
 static CORE_ADDR dr_mirror[DR_NADDR];
 static unsigned dr_status_mirror, dr_control_mirror;
 
@@ -567,13 +570,22 @@ i386_region_ok_for_watchpoint (CORE_ADDR
 /* If the inferior has some watchpoint that triggered, return the
    address associated with that watchpoint.  Otherwise, return zero.  */
 
+extern ptid_t trap_ptid;
+extern ptid_t inferior_ptid;
+
 CORE_ADDR
 i386_stopped_data_address (void)
 {
   CORE_ADDR addr = 0;
   int i;
+  ptid_t saved_ptid = inferior_ptid;
 
+  /* Debug status register is thread-specific.  A watchpoint will
+     cause a trap to occur.  Switch to trap ptid to get relevant 
+     status for that thread.  */
+  inferior_ptid = trap_ptid;
   dr_status_mirror = I386_DR_LOW_GET_STATUS ();
+  inferior_ptid = saved_ptid;
 
   ALL_DEBUG_REGISTERS(i)
     {
@@ -603,8 +615,16 @@ int
 i386_stopped_by_hwbp (void)
 {
   int i;
+  ptid_t saved_ptid = inferior_ptid;
+
+  /* Debug status register is thread-specific.  A hardware breakpoint
+     will cause a trap to occur.  Switch to trap ptid to get relevant 
+     status for that thread.  */
 
+  inferior_ptid = trap_ptid;
   dr_status_mirror = I386_DR_LOW_GET_STATUS ();
+  inferior_ptid = saved_ptid;
+
   if (maint_show_dr)
     i386_show_dr ("stopped_by_hwbp", 0, 0, hw_execute);
 
Index: lin-lwp.c
===================================================================
RCS file: /cvs/src/src/gdb/lin-lwp.c,v
retrieving revision 1.55
diff -u -p -r1.55 lin-lwp.c
--- lin-lwp.c	25 May 2004 14:58:28 -0000	1.55
+++ lin-lwp.c	11 Jun 2004 20:55:28 -0000
@@ -1200,21 +1200,29 @@ child_wait (ptid_t ptid, struct target_w
 	}
 
       /* Handle GNU/Linux's extended waitstatus for trace events.  */
-      if (pid != -1 && WIFSTOPPED (status) && WSTOPSIG (status) == SIGTRAP
-	  && status >> 16 != 0)
+      if (pid != -1 && WIFSTOPPED (status) && WSTOPSIG (status) == SIGTRAP)
 	{
-	  linux_handle_extended_wait (pid, status, ourstatus);
+	  /* Set trap_ptid like lin_lwp_wait does.  This is needed
+	     for watchpoint support.  For example, the x86 linux
+	     watchpoints need to know what thread an event occurred
+	     on so as to read the correct thread-specific status.  */
+          trap_ptid = pid_to_ptid (pid);
 
-	  /* If we see a clone event, detach the child, and don't
-	     report the event.  It would be nice to offer some way to
-	     switch into a non-thread-db based threaded mode at this
-	     point.  */
-	  if (ourstatus->kind == TARGET_WAITKIND_SPURIOUS)
+	  if (status >> 16 != 0)
 	    {
-	      ptrace (PTRACE_DETACH, ourstatus->value.related_pid, 0, 0);
-	      ourstatus->kind = TARGET_WAITKIND_IGNORE;
-	      pid = -1;
-	      save_errno = EINTR;
+	      linux_handle_extended_wait (pid, status, ourstatus);
+
+	      /* If we see a clone event, detach the child, and don't
+		 report the event.  It would be nice to offer some way to
+		 switch into a non-thread-db based threaded mode at this
+		 point.  */
+	      if (ourstatus->kind == TARGET_WAITKIND_SPURIOUS)
+		{ 
+		  ptrace (PTRACE_DETACH, ourstatus->value.related_pid, 0, 0);
+		  ourstatus->kind = TARGET_WAITKIND_IGNORE;
+		  pid = -1;
+		  save_errno = EINTR;
+		}
 	    }
 	}
 
Index: target.c
===================================================================
RCS file: /cvs/src/src/gdb/target.c,v
retrieving revision 1.75
diff -u -p -r1.75 target.c
--- target.c	9 Jun 2004 20:09:42 -0000	1.75
+++ target.c	11 Jun 2004 20:55:28 -0000
@@ -422,6 +422,7 @@ update_current_target (void)
       INHERIT (to_notice_signals, t);
       INHERIT (to_thread_alive, t);
       INHERIT (to_find_new_threads, t);
+      INHERIT (to_get_lwp, t);
       INHERIT (to_pid_to_str, t);
       INHERIT (to_extra_thread_info, t);
       INHERIT (to_stop, t);
@@ -601,6 +602,9 @@ update_current_target (void)
   de_fault (to_find_new_threads, 
 	    (void (*) (void)) 
 	    target_ignore);
+  de_fault (to_get_lwp,
+            (long (*) (ptid_t))
+            ptid_get_lwp);
   de_fault (to_extra_thread_info, 
 	    (char *(*) (struct thread_info *)) 
 	    return_zero);
@@ -1616,6 +1620,7 @@ init_dummy_target (void)
   dummy_target.to_find_memory_regions = dummy_find_memory_regions;
   dummy_target.to_make_corefile_notes = dummy_make_corefile_notes;
   dummy_target.to_xfer_partial = default_xfer_partial;
+  dummy_target.to_get_lwp = ptid_get_lwp;
   dummy_target.to_magic = OPS_MAGIC;
 }
 \f
Index: target.h
===================================================================
RCS file: /cvs/src/src/gdb/target.h,v
retrieving revision 1.60
diff -u -p -r1.60 target.h
--- target.h	7 Jun 2004 17:58:32 -0000	1.60
+++ target.h	11 Jun 2004 20:55:29 -0000
@@ -370,6 +370,7 @@ struct target_ops
     void (*to_notice_signals) (ptid_t ptid);
     int (*to_thread_alive) (ptid_t ptid);
     void (*to_find_new_threads) (void);
+    long (*to_get_lwp) (ptid_t ptid);
     char *(*to_pid_to_str) (ptid_t);
     char *(*to_extra_thread_info) (struct thread_info *);
     void (*to_stop) (void);
@@ -815,6 +816,10 @@ extern void target_load (char *arg, int 
 #define target_find_new_threads() \
      (*current_target.to_find_new_threads) (); \
 
+/* Get the lwp for a thread.  */
+#define target_get_lwp(ptid) \
+     (*current_target.to_get_lwp) (ptid); \
+
 /* Make target stop in a continuable fashion.  (For instance, under
    Unix, this should act like SIGSTOP).  This function is normally
    used by GUIs to implement a stop button.  */
Index: thread-db.c
===================================================================
RCS file: /cvs/src/src/gdb/thread-db.c,v
retrieving revision 1.42
diff -u -p -r1.42 thread-db.c
--- thread-db.c	7 Jun 2004 22:35:55 -0000	1.42
+++ thread-db.c	11 Jun 2004 20:55:29 -0000
@@ -408,6 +408,22 @@ lwp_from_thread (ptid_t ptid)
 }
 \f
 
+static long
+thread_db_get_lwp (ptid_t ptid)
+{
+  if (!is_thread (ptid))
+    return ptid.pid;
+  else
+    {
+      struct thread_info *thread_info = find_thread_pid (ptid);
+      if (!thread_info)
+        return 0;
+
+      thread_db_get_info (thread_info);
+      return thread_info->private->ti.ti_lid;
+    }
+}
+
 void
 thread_db_init (struct target_ops *target)
 {
@@ -1382,6 +1398,7 @@ init_thread_db_ops (void)
   thread_db_ops.to_mourn_inferior = thread_db_mourn_inferior;
   thread_db_ops.to_thread_alive = thread_db_thread_alive;
   thread_db_ops.to_find_new_threads = thread_db_find_new_threads;
+  thread_db_ops.to_get_lwp = thread_db_get_lwp;
   thread_db_ops.to_pid_to_str = thread_db_pid_to_str;
   thread_db_ops.to_stratum = thread_stratum;
   thread_db_ops.to_has_thread_control = tc_schedlock;