This is the mail archive of the gdb-patches@sourceware.org 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]

[patch 1/2] Fix watchpoints across fork #2


On Mon, 02 Jan 2012 20:04:12 +0100, Pedro Alves wrote:
> This already iterated over threads of the current inferior only.
> linux_nat_iterate_watchpoint_lwps does not do that, but leaves it to the
> next patch instead.  It'd be better to move that hunk into this patch
> already.

Changed, it was a patch rebase mistake.


> I guess this leaves a stale `pid_ptid' local behind.

Fixed.


> >  	  inferior_ptid = ptid_build (child_pid, child_pid, 0);
> >+	  reinit_frame_cache ();
> >+	  registers_changed ();
> 
> Only registers_changed is necessary.  That always invalidates the
> frame cache of inferior_ptid.
+
> But, I'm not sure I understand why are these calls are necessary.  We
> keep a  register cache list, indexed by ptid nowadays.  Could this
> be a latent bug elsewhere?

As this post does not include changes for ppc* I have dropped this part now.
Thanks for noticing me it is really not obvious, I agree it should work.
I will review it with the ppc*/s390* part to be ported later in a second patch
series.


> A [target_info exists gdb,no_hardware_watchpoints] check appears
> missing, either to skip to test, or call "set can-use-hw-watchpoints
> 0".  Similarly, [skip_hw_breakpoint_tests] for the hardware breakpoints
> bits.

Fixed.


> Took me a bit to convince myself i386_inferior_data_get was okay.
> I wish it was cleaner (could be if we had a separate struct
> process_info, which will end up with, if/when we merge
> gdb+gdbserver), but this will do for now.

I tried it various ways, I found this one the most feasible, this is
variant "f" - the 6th variant.


No regressions on {x86_64,x86_64-m32,i686}-fedorarawhide-linux-gnu and in
gdbserver mode and on ppc64-rhel62-linux-gnu.


Thanks,
Jan


gdb/
2012-01-20  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Fix watchpoints across inferior fork.
	* amd64-linux-nat.c (update_debug_registers_callback): Update the
	comment for linux_nat_iterate_watchpoint_lwps.
	(amd64_linux_dr_set_control, amd64_linux_dr_set_addr): Use
	linux_nat_iterate_watchpoint_lwps.
	(amd64_linux_prepare_to_resume): New comment on Linux kernel.
	* i386-linux-nat.c (update_debug_registers_callback): Update the
	comment for linux_nat_iterate_watchpoint_lwps.
	(i386_linux_dr_set_control, i386_linux_dr_set_addr): Use
	linux_nat_iterate_watchpoint_lwps.
	(i386_linux_prepare_to_resume): New comment on Linux kernel.
	* i386-nat.c: Include inferior.h.
	(dr_mirror): Remove.
	(i386_inferior_data, struct i386_inferior_data)
	(i386_inferior_data_get): New.
	(i386_debug_reg_state): Use i386_inferior_data_get.
	(i386_cleanup_dregs, i386_update_inferior_debug_regs)
	(i386_insert_watchpoint, i386_remove_watchpoint)
	(i386_stopped_data_address, i386_insert_hw_breakpoint)
	(i386_remove_hw_breakpoint): New variable state, use
	i386_debug_reg_state instead of DR_MIRROR.
	* linux-nat.c (delete_lwp): New declaration.
	(num_lwps): Move here from downwards.
	(delete_lwp_cleanup): New.
	(linux_child_follow_fork): Create new child_lp, call
	linux_nat_new_thread and linux_nat_prepare_to_resume before calling
	PTRACE_DETACH.
	(num_lwps): Move upwards.
	(linux_nat_iterate_watchpoint_lwps): New.
	* linux-nat.h (linux_nat_iterate_watchpoint_lwps_ftype): New.
	(linux_nat_iterate_watchpoint_lwps_ftype): New declaration.

gdb/testsuite/
2012-01-20  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Fix watchpoints across inferior fork.
	* gdb.threads/watchpoint-fork-child.c: New file.
	* gdb.threads/watchpoint-fork-mt.c: New file.
	* gdb.threads/watchpoint-fork-parent.c: New file.
	* gdb.threads/watchpoint-fork-st.c: New file.
	* gdb.threads/watchpoint-fork.exp: New file.
	* gdb.threads/watchpoint-fork.h: New file.

--- a/gdb/amd64-linux-nat.c
+++ b/gdb/amd64-linux-nat.c
@@ -336,8 +336,8 @@ amd64_linux_dr_get_status (void)
   return amd64_linux_dr_get (inferior_ptid, DR_STATUS);
 }
 
-/* Callback for iterate_over_lwps.  Update the debug registers of
-   LWP.  */
+/* Callback for linux_nat_iterate_watchpoint_lwps.  Update the debug registers
+   of LWP.  */
 
 static int
 update_debug_registers_callback (struct lwp_info *lwp, void *arg)
@@ -363,9 +363,7 @@ update_debug_registers_callback (struct lwp_info *lwp, void *arg)
 static void
 amd64_linux_dr_set_control (unsigned long control)
 {
-  ptid_t pid_ptid = pid_to_ptid (ptid_get_pid (inferior_ptid));
-
-  iterate_over_lwps (pid_ptid, update_debug_registers_callback, NULL);
+  linux_nat_iterate_watchpoint_lwps (update_debug_registers_callback, NULL);
 }
 
 /* Set address REGNUM (zero based) to ADDR in all LWPs of the current
@@ -374,11 +372,9 @@ amd64_linux_dr_set_control (unsigned long control)
 static void
 amd64_linux_dr_set_addr (int regnum, CORE_ADDR addr)
 {
-  ptid_t pid_ptid = pid_to_ptid (ptid_get_pid (inferior_ptid));
-
   gdb_assert (regnum >= 0 && regnum <= DR_LASTADDR - DR_FIRSTADDR);
 
-  iterate_over_lwps (pid_ptid, update_debug_registers_callback, NULL);
+  linux_nat_iterate_watchpoint_lwps (update_debug_registers_callback, NULL);
 }
 
 /* Called when resuming a thread.
@@ -400,6 +396,13 @@ amd64_linux_prepare_to_resume (struct lwp_info *lwp)
       struct i386_debug_reg_state *state = i386_debug_reg_state ();
       int i;
 
+      /* On Linux kernel before 2.6.33 commit
+	 72f674d203cd230426437cdcf7dd6f681dad8b0d
+	 if you enable a breakpoint by the DR_CONTROL bits you need to have
+	 already written the corresponding DR_FIRSTADDR...DR_LASTADDR registers.
+
+	 Ensure DR_CONTROL gets written as the very last register here.  */
+
       for (i = DR_FIRSTADDR; i <= DR_LASTADDR; i++)
 	if (state->dr_ref_count[i] > 0)
 	  {
--- a/gdb/i386-linux-nat.c
+++ b/gdb/i386-linux-nat.c
@@ -708,8 +708,8 @@ i386_linux_dr_get_status (void)
   return i386_linux_dr_get (inferior_ptid, DR_STATUS);
 }
 
-/* Callback for iterate_over_lwps.  Update the debug registers of
-   LWP.  */
+/* Callback for linux_nat_iterate_watchpoint_lwps.  Update the debug registers
+   of LWP.  */
 
 static int
 update_debug_registers_callback (struct lwp_info *lwp, void *arg)
@@ -735,9 +735,7 @@ update_debug_registers_callback (struct lwp_info *lwp, void *arg)
 static void
 i386_linux_dr_set_control (unsigned long control)
 {
-  ptid_t pid_ptid = pid_to_ptid (ptid_get_pid (inferior_ptid));
-
-  iterate_over_lwps (pid_ptid, update_debug_registers_callback, NULL);
+  linux_nat_iterate_watchpoint_lwps (update_debug_registers_callback, NULL);
 }
 
 /* Set address REGNUM (zero based) to ADDR in all LWPs of the current
@@ -750,7 +748,7 @@ i386_linux_dr_set_addr (int regnum, CORE_ADDR addr)
 
   gdb_assert (regnum >= 0 && regnum <= DR_LASTADDR - DR_FIRSTADDR);
 
-  iterate_over_lwps (pid_ptid, update_debug_registers_callback, NULL);
+  linux_nat_iterate_watchpoint_lwps (update_debug_registers_callback, NULL);
 }
 
 /* Called when resuming a thread.
@@ -772,6 +770,9 @@ i386_linux_prepare_to_resume (struct lwp_info *lwp)
       struct i386_debug_reg_state *state = i386_debug_reg_state ();
       int i;
 
+      /* See amd64_linux_prepare_to_resume for Linux kernel note on
+	 i386_linux_dr_set calls ordering.  */
+
       for (i = DR_FIRSTADDR; i <= DR_LASTADDR; i++)
 	if (state->dr_ref_count[i] > 0)
 	  {
--- a/gdb/i386-nat.c
+++ b/gdb/i386-nat.c
@@ -25,6 +25,7 @@
 #include "gdbcmd.h"
 #include "target.h"
 #include "gdb_assert.h"
+#include "inferior.h"
 
 /* Support for hardware watchpoints and breakpoints using the i386
    debug registers.
@@ -170,14 +171,73 @@ i386_init_dregs (struct i386_debug_reg_state *state)
   state->dr_status_mirror  = 0;
 }
 
-/* The local mirror of the inferior's debug registers.  Currently this
-   is a global, but it should really be per-inferior.  */
-static struct i386_debug_reg_state dr_mirror;
+/* Per-inferior data key.  */
+static const struct inferior_data *i386_inferior_data;
+
+/* Per-inferior data.  */
+struct i386_inferior_data
+  {
+    /* Copy of i386 hardware debug registers for performance reasons.  */
+    struct i386_debug_reg_state state;
+  };
+
+/* Get data specific for INFERIOR_PTID LWP.  Return special data area
+   for processes being detached.  */
+
+static struct i386_inferior_data *
+i386_inferior_data_get (void)
+{
+  /* Intermediate patch stub.  */
+  static struct i386_inferior_data inf_data_local;
+  struct inferior *inf = current_inferior ();
+  struct i386_inferior_data *inf_data = &inf_data_local;
+
+  if (inf->pid != ptid_get_pid (inferior_ptid))
+    {
+      /* INFERIOR_PTID is being detached from the inferior INF.
+	 Provide local cache specific for the detached LWP.  */
+
+      static struct i386_inferior_data detached_inf_data_local;
+      static int detached_inf_pid = -1;
+
+      if (detached_inf_pid != ptid_get_pid (inferior_ptid))
+	{
+	  /* Reinitialize the local cache if INFERIOR_PTID is
+	     different from the LWP last detached.
+ 
+	     Linux kernel before 2.6.33 commit
+	     72f674d203cd230426437cdcf7dd6f681dad8b0d
+	     will inherit hardware debug registers from parent
+	     on fork/vfork/clone.  Newer Linux kernels create such tasks with
+	     zeroed debug registers.
+
+	     GDB will remove all breakpoints (and watchpoints) from the forked
+	     off process.  We also need to reset the debug registers in that
+	     process to be compatible with the older Linux kernels.
+
+	     Copy the debug registers mirrors into the new process so that all
+	     breakpoints and watchpoints can be removed together.  The debug
+	     registers mirror will become zeroed in the end before detaching
+	     the forked off process.  */
+
+	  detached_inf_pid = ptid_get_pid (inferior_ptid);
+	  memcpy (&detached_inf_data_local, inf_data,
+		  sizeof (detached_inf_data_local));
+	}
+
+      return &detached_inf_data_local;
+    }
+
+  return inf_data;
+}
+
+/* Get debug registers state for INFERIOR_PTID, see
+   i386_inferior_data_get.  */
 
 struct i386_debug_reg_state *
 i386_debug_reg_state (void)
 {
-  return &dr_mirror;
+  return &i386_inferior_data_get ()->state;
 }
 
 /* Whether or not to print the mirrored debug registers.  */
@@ -230,7 +290,9 @@ static int i386_handle_nonaligned_watchpoint (struct i386_debug_reg_state *state
 void
 i386_cleanup_dregs (void)
 {
-  i386_init_dregs (&dr_mirror);
+  struct i386_debug_reg_state *state = i386_debug_reg_state ();
+
+  i386_init_dregs (state);
 }
 
 /* Print the values of the mirrored debug registers.  This is called
@@ -494,20 +556,21 @@ Invalid value %d of operation in i386_handle_nonaligned_watchpoint.\n"),
 static void
 i386_update_inferior_debug_regs (struct i386_debug_reg_state *new_state)
 {
+  struct i386_debug_reg_state *state = i386_debug_reg_state ();
   int i;
 
   ALL_DEBUG_REGISTERS (i)
     {
-      if (I386_DR_VACANT (new_state, i) != I386_DR_VACANT (&dr_mirror, i))
+      if (I386_DR_VACANT (new_state, i) != I386_DR_VACANT (state, i))
 	i386_dr_low.set_addr (i, new_state->dr_mirror[i]);
       else
-	gdb_assert (new_state->dr_mirror[i] == dr_mirror.dr_mirror[i]);
+	gdb_assert (new_state->dr_mirror[i] == state->dr_mirror[i]);
     }
 
-  if (new_state->dr_control_mirror != dr_mirror.dr_control_mirror)
+  if (new_state->dr_control_mirror != state->dr_control_mirror)
     i386_dr_low.set_control (new_state->dr_control_mirror);
 
-  dr_mirror = *new_state;
+  *state = *new_state;
 }
 
 /* Insert a watchpoint to watch a memory region which starts at
@@ -518,10 +581,11 @@ static int
 i386_insert_watchpoint (CORE_ADDR addr, int len, int type,
 			struct expression *cond)
 {
+  struct i386_debug_reg_state *state = i386_debug_reg_state ();
   int retval;
   /* Work on a local copy of the debug registers, and on success,
      commit the change back to the inferior.  */
-  struct i386_debug_reg_state local_state = dr_mirror;
+  struct i386_debug_reg_state local_state = *state;
 
   if (type == hw_read)
     return 1; /* unsupported */
@@ -542,7 +606,7 @@ i386_insert_watchpoint (CORE_ADDR addr, int len, int type,
     i386_update_inferior_debug_regs (&local_state);
 
   if (maint_show_dr)
-    i386_show_dr (&dr_mirror, "insert_watchpoint", addr, len, type);
+    i386_show_dr (state, "insert_watchpoint", addr, len, type);
 
   return retval;
 }
@@ -554,10 +618,11 @@ static int
 i386_remove_watchpoint (CORE_ADDR addr, int len, int type,
 			struct expression *cond)
 {
+  struct i386_debug_reg_state *state = i386_debug_reg_state ();
   int retval;
   /* Work on a local copy of the debug registers, and on success,
      commit the change back to the inferior.  */
-  struct i386_debug_reg_state local_state = dr_mirror;
+  struct i386_debug_reg_state local_state = *state;
 
   if (((len != 1 && len !=2 && len !=4) && !(TARGET_HAS_DR_LEN_8 && len == 8))
       || addr % len != 0)
@@ -575,7 +640,7 @@ i386_remove_watchpoint (CORE_ADDR addr, int len, int type,
     i386_update_inferior_debug_regs (&local_state);
 
   if (maint_show_dr)
-    i386_show_dr (&dr_mirror, "remove_watchpoint", addr, len, type);
+    i386_show_dr (state, "remove_watchpoint", addr, len, type);
 
   return retval;
 }
@@ -586,11 +651,12 @@ i386_remove_watchpoint (CORE_ADDR addr, int len, int type,
 static int
 i386_region_ok_for_watchpoint (CORE_ADDR addr, int len)
 {
+  struct i386_debug_reg_state *state = i386_debug_reg_state ();
   int nregs;
 
   /* Compute how many aligned watchpoints we would need to cover this
      region.  */
-  nregs = i386_handle_nonaligned_watchpoint (&dr_mirror,
+  nregs = i386_handle_nonaligned_watchpoint (state,
 					     WP_COUNT, addr, len, hw_write);
   return nregs <= DR_NADDR ? 1 : 0;
 }
@@ -602,6 +668,7 @@ i386_region_ok_for_watchpoint (CORE_ADDR addr, int len)
 static int
 i386_stopped_data_address (struct target_ops *ops, CORE_ADDR *addr_p)
 {
+  struct i386_debug_reg_state *state = i386_debug_reg_state ();
   CORE_ADDR addr = 0;
   int i;
   int rc = 0;
@@ -615,25 +682,24 @@ i386_stopped_data_address (struct target_ops *ops, CORE_ADDR *addr_p)
   unsigned control = 0;
 
   /* In non-stop/async, threads can be running while we change the
-     global dr_mirror (and friends).  Say, we set a watchpoint, and
-     let threads resume.  Now, say you delete the watchpoint, or
-     add/remove watchpoints such that dr_mirror changes while threads
-     are running.  On targets that support non-stop,
-     inserting/deleting watchpoints updates the global dr_mirror only.
-     It does not update the real thread's debug registers; that's only
-     done prior to resume.  Instead, if threads are running when the
-     mirror changes, a temporary and transparent stop on all threads
-     is forced so they can get their copy of the debug registers
-     updated on re-resume.  Now, say, a thread hit a watchpoint before
-     having been updated with the new dr_mirror contents, and we
-     haven't yet handled the corresponding SIGTRAP.  If we trusted
-     dr_mirror below, we'd mistake the real trapped address (from the
-     last time we had updated debug registers in the thread) with
-     whatever was currently in dr_mirror.  So to fix this, dr_mirror
-     always represents intention, what we _want_ threads to have in
-     debug registers.  To get at the address and cause of the trap, we
-     need to read the state the thread still has in its debug
-     registers.
+     STATE (and friends).  Say, we set a watchpoint, and let threads
+     resume.  Now, say you delete the watchpoint, or add/remove
+     watchpoints such that STATE changes while threads are running.
+     On targets that support non-stop, inserting/deleting watchpoints
+     updates the STATE only.  It does not update the real thread's
+     debug registers; that's only done prior to resume.  Instead, if
+     threads are running when the mirror changes, a temporary and
+     transparent stop on all threads is forced so they can get their
+     copy of the debug registers updated on re-resume.  Now, say,
+     a thread hit a watchpoint before having been updated with the new
+     STATE contents, and we haven't yet handled the corresponding
+     SIGTRAP.  If we trusted STATE below, we'd mistake the real
+     trapped address (from the last time we had updated debug
+     registers in the thread) with whatever was currently in STATE.
+     So to fix this, STATE always represents intention, what we _want_
+     threads to have in debug registers.  To get at the address and
+     cause of the trap, we need to read the state the thread still has
+     in its debug registers.
 
      In sum, always get the current debug register values the current
      thread has, instead of trusting the global mirror.  If the thread
@@ -663,11 +729,11 @@ i386_stopped_data_address (struct target_ops *ops, CORE_ADDR *addr_p)
 	  addr = i386_dr_low.get_addr (i);
 	  rc = 1;
 	  if (maint_show_dr)
-	    i386_show_dr (&dr_mirror, "watchpoint_hit", addr, -1, hw_write);
+	    i386_show_dr (state, "watchpoint_hit", addr, -1, hw_write);
 	}
     }
   if (maint_show_dr && addr == 0)
-    i386_show_dr (&dr_mirror, "stopped_data_addr", 0, 0, hw_write);
+    i386_show_dr (state, "stopped_data_addr", 0, 0, hw_write);
 
   if (rc)
     *addr_p = addr;
@@ -687,11 +753,12 @@ static int
 i386_insert_hw_breakpoint (struct gdbarch *gdbarch,
 			   struct bp_target_info *bp_tgt)
 {
+  struct i386_debug_reg_state *state = i386_debug_reg_state ();
   unsigned len_rw = i386_length_and_rw_bits (1, hw_execute);
   CORE_ADDR addr = bp_tgt->placed_address;
   /* Work on a local copy of the debug registers, and on success,
      commit the change back to the inferior.  */
-  struct i386_debug_reg_state local_state = dr_mirror;
+  struct i386_debug_reg_state local_state = *state;
   int retval = i386_insert_aligned_watchpoint (&local_state,
 					       addr, len_rw) ? EBUSY : 0;
 
@@ -699,7 +766,7 @@ i386_insert_hw_breakpoint (struct gdbarch *gdbarch,
     i386_update_inferior_debug_regs (&local_state);
 
   if (maint_show_dr)
-    i386_show_dr (&dr_mirror, "insert_hwbp", addr, 1, hw_execute);
+    i386_show_dr (state, "insert_hwbp", addr, 1, hw_execute);
 
   return retval;
 }
@@ -711,11 +778,12 @@ static int
 i386_remove_hw_breakpoint (struct gdbarch *gdbarch,
 			   struct bp_target_info *bp_tgt)
 {
+  struct i386_debug_reg_state *state = i386_debug_reg_state ();
   unsigned len_rw = i386_length_and_rw_bits (1, hw_execute);
   CORE_ADDR addr = bp_tgt->placed_address;
   /* Work on a local copy of the debug registers, and on success,
      commit the change back to the inferior.  */
-  struct i386_debug_reg_state local_state = dr_mirror;
+  struct i386_debug_reg_state local_state = *state;
   int retval = i386_remove_aligned_watchpoint (&local_state,
 					       addr, len_rw);
 
@@ -723,7 +791,7 @@ i386_remove_hw_breakpoint (struct gdbarch *gdbarch,
     i386_update_inferior_debug_regs (&local_state);
 
   if (maint_show_dr)
-    i386_show_dr (&dr_mirror, "remove_hwbp", addr, 1, hw_execute);
+    i386_show_dr (state, "remove_hwbp", addr, 1, hw_execute);
 
   return retval;
 }
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -288,6 +288,7 @@ static void restore_child_signals_mask (sigset_t *prev_mask);
 struct lwp_info;
 static struct lwp_info *add_lwp (ptid_t ptid);
 static void purge_lwp_list (int pid);
+static void delete_lwp (ptid_t ptid);
 static struct lwp_info *find_lwp_pid (ptid_t ptid);
 
 
@@ -584,6 +585,31 @@ linux_child_post_startup_inferior (ptid_t ptid)
   linux_enable_tracesysgood (ptid);
 }
 
+/* Return the number of known LWPs in the tgid given by PID.  */
+
+static int
+num_lwps (int pid)
+{
+  int count = 0;
+  struct lwp_info *lp;
+
+  for (lp = lwp_list; lp; lp = lp->next)
+    if (ptid_get_pid (lp->ptid) == pid)
+      count++;
+
+  return count;
+}
+
+/* Call delete_lwp with prototype compatible for make_cleanup.  */
+
+static void
+delete_lwp_cleanup (void *lp_voidp)
+{
+  struct lwp_info *lp = lp_voidp;
+
+  delete_lwp (lp->ptid);
+}
+
 static int
 linux_child_follow_fork (struct target_ops *ops, int follow_child)
 {
@@ -630,6 +656,8 @@ holding the child stopped.  Try \"set detach-on-fork\" or \
       /* Detach new forked process?  */
       if (detach_fork)
 	{
+	  struct cleanup *old_chain;
+
 	  /* Before detaching from the child, remove all breakpoints
 	     from it.  If we forked, then this has already been taken
 	     care of by infrun.c.  If we vforked however, any
@@ -652,7 +680,28 @@ holding the child stopped.  Try \"set detach-on-fork\" or \
 				child_pid);
 	    }
 
+	  old_chain = save_inferior_ptid ();
+	  inferior_ptid = ptid_build (child_pid, child_pid, 0);
+
+	  child_lp = add_lwp (inferior_ptid);
+	  child_lp->stopped = 1;
+	  child_lp->last_resume_kind = resume_stop;
+	  make_cleanup (delete_lwp_cleanup, child_lp);
+
+	  /* CHILD_LP has new PID, therefore linux_nat_new_thread is not called for it.
+	     See i386_inferior_data_get for the Linux kernel specifics.
+	     Ensure linux_nat_prepare_to_resume will reset the hardware debug
+	     registers.  It is done by the linux_nat_new_thread call, which is
+	     being skipped in add_lwp above for the first lwp of a pid.  */
+	  gdb_assert (num_lwps (GET_PID (child_lp->ptid)) == 1);
+	  if (linux_nat_new_thread != NULL)
+	    linux_nat_new_thread (child_lp);
+
+	  if (linux_nat_prepare_to_resume != NULL)
+	    linux_nat_prepare_to_resume (child_lp);
 	  ptrace (PTRACE_DETACH, child_pid, 0, 0);
+
+	  do_cleanups (old_chain);
 	}
       else
 	{
@@ -1111,21 +1160,6 @@ purge_lwp_list (int pid)
     }
 }
 
-/* Return the number of known LWPs in the tgid given by PID.  */
-
-static int
-num_lwps (int pid)
-{
-  int count = 0;
-  struct lwp_info *lp;
-
-  for (lp = lwp_list; lp; lp = lp->next)
-    if (ptid_get_pid (lp->ptid) == pid)
-      count++;
-
-  return count;
-}
-
 /* Add the LWP specified by PID to the list.  Return a pointer to the
    structure describing the new LWP.  The LWP should already be stopped
    (with an exception for the very first LWP).  */
@@ -1235,6 +1269,46 @@ iterate_over_lwps (ptid_t filter,
   return NULL;
 }
 
+/* Iterate like iterate_over_lwps does except when forking-off a child call
+   CALLBACK with CALLBACK_DATA specifically only for that new child PID.  */
+
+void
+linux_nat_iterate_watchpoint_lwps
+  (linux_nat_iterate_watchpoint_lwps_ftype callback, void *callback_data)
+{
+  int inferior_pid = ptid_get_pid (inferior_ptid);
+  struct inferior *inf = current_inferior ();
+
+  if (inf->pid == inferior_pid)
+    {
+      /* Iterate all the threads of the current inferior.  Without specifying
+	 INFERIOR_PID it would iterate all threads of all inferiors, which is
+	 inappropriate for watchpoints.  */
+
+      iterate_over_lwps (pid_to_ptid (inferior_pid), callback, callback_data);
+    }
+  else
+    {
+      /* Detaching a new child PID temporarily present in INFERIOR_PID.  */
+
+      struct lwp_info *child_lp;
+      struct cleanup *old_chain;
+      pid_t child_pid = GET_PID (inferior_ptid);
+      ptid_t child_ptid = ptid_build (child_pid, child_pid, 0);
+
+      gdb_assert (!is_lwp (inferior_ptid));
+      gdb_assert (find_lwp_pid (child_ptid) == NULL);
+      child_lp = add_lwp (child_ptid);
+      child_lp->stopped = 1;
+      child_lp->last_resume_kind = resume_stop;
+      old_chain = make_cleanup (delete_lwp_cleanup, child_lp);
+
+      callback (child_lp, callback_data);
+
+      do_cleanups (old_chain);
+    }
+}
+
 /* Update our internal state when changing from one checkpoint to
    another indicated by NEW_PTID.  We can only switch single-threaded
    applications, so we only create one new LWP, and the previous list
--- a/gdb/linux-nat.h
+++ b/gdb/linux-nat.h
@@ -158,6 +158,12 @@ struct lwp_info *iterate_over_lwps (ptid_t filter,
 						     void *), 
 				    void *data);
 
+typedef int (*linux_nat_iterate_watchpoint_lwps_ftype) (struct lwp_info *lwp,
+							void *arg);
+
+extern void linux_nat_iterate_watchpoint_lwps
+  (linux_nat_iterate_watchpoint_lwps_ftype callback, void *callback_data);
+
 /* Create a prototype generic GNU/Linux target.  The client can
    override it with local methods.  */
 struct target_ops * linux_target (void);
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/watchpoint-fork-child.c
@@ -0,0 +1,129 @@
+/* Test case for forgotten hw-watchpoints after fork()-off of a process.
+
+   Copyright 2012 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 2 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, write to the Free Software
+   Foundation, Inc., 59 Temple Place - Suite 330,
+   Boston, MA 02111-1307, USA.  */
+
+#include <string.h>
+#include <errno.h>
+#include <unistd.h>
+#include <assert.h>
+#include <signal.h>
+#include <stdio.h>
+
+#include "watchpoint-fork.h"
+
+/* `pid_t' may not be available.  */
+
+static volatile int usr1_got;
+
+static void
+handler_usr1 (int signo)
+{
+  usr1_got++;
+}
+
+void
+forkoff (int nr)
+{
+  int child, save_parent = getpid ();
+  int i;
+  struct sigaction act, oldact;
+#ifdef THREAD
+  void *thread_result;
+#endif
+
+  memset (&act, 0, sizeof act);
+  act.sa_flags = SA_RESTART;
+  act.sa_handler = handler_usr1;
+  sigemptyset (&act.sa_mask);
+  i = sigaction (SIGUSR1, &act, &oldact);
+  assert (i == 0);
+
+  child = fork ();
+  switch (child)
+    {
+    case -1:
+      assert (0);
+    default:
+      printf ("parent%d: %d\n", nr, (int) child);
+
+      /* Sleep for a while to possibly get incorrectly ATTACH_THREADed by GDB
+	 tracing the child fork with no longer valid thread/lwp entries of the
+	 parent.  */
+
+      i = sleep (2);
+      assert (i == 0);
+
+      /* We must not get caught here (against a forgotten breakpoint).  */
+
+      var++;
+      marker ();
+
+#ifdef THREAD
+      /* And neither got caught our thread.  */
+
+      step = 99;
+      i = pthread_join (thread, &thread_result);
+      assert (i == 0);
+      assert (thread_result == (void *) 99UL);
+#endif
+
+      /* Be sure our child knows we did not get caught above.  */
+
+      i = kill (child, SIGUSR1);
+      assert (i == 0);
+
+      /* Sleep for a while to check GDB's `info threads' no longer tracks us in
+	 the child fork.  */
+
+      i = sleep (2);
+      assert (i == 0);
+
+      _exit (0);
+    case 0:
+      printf ("child%d: %d\n", nr, (int) getpid ());
+
+      /* Let the parent signal us about its success.  Be careful of races.  */
+
+      for (;;)
+	{
+	  /* Parent either died (and USR1_GOT is zero) or it succeeded.  */
+	  if (getppid () != save_parent)
+	    break;
+	  if (kill (getppid (), 0) != 0)
+	    break;
+	  /* Parent succeeded?  */
+	  if (usr1_got)
+	    break;
+
+#ifdef THREAD
+	  i = pthread_yield ();
+	  assert (i == 0);
+#endif
+	}
+      assert (usr1_got);
+
+      /* We must get caught here (against a false watchpoint removal).  */
+
+      marker ();
+    }
+
+  i = sigaction (SIGUSR1, &oldact, NULL);
+  assert (i == 0);
+}
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/watchpoint-fork-mt.c
@@ -0,0 +1,174 @@
+/* Test case for forgotten hw-watchpoints after fork()-off of a process.
+
+   Copyright 2012 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 2 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, write to the Free Software
+   Foundation, Inc., 59 Temple Place - Suite 330,
+   Boston, MA 02111-1307, USA.  */
+
+#include <assert.h>
+#include <unistd.h>
+#include <sys/wait.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <pthread.h>
+
+#include <asm/unistd.h>
+#include <unistd.h>
+#define gettid() syscall (__NR_gettid)
+
+#include "watchpoint-fork.h"
+
+/* Non-atomic `var++' should not hurt as we synchronize the threads by the STEP
+   variable.  Hit-comments need to be duplicite there to catch both at-stops
+   and behind-stops, depending on the target.  */
+
+volatile int var;
+
+void
+marker (void)
+{
+}
+
+static void
+empty (void)
+{
+}
+
+static void
+mark_exit (void)
+{
+}
+
+pthread_t thread;
+volatile int step;
+
+static void *
+start (void *arg)
+{
+  int i;
+
+  if (step >= 3)
+    goto step_3;
+
+  while (step != 1)
+    {
+      i = pthread_yield ();
+      assert (i == 0);
+    }
+
+  var++;	/* validity-thread-B */
+  empty ();	/* validity-thread-B */
+  step = 2;
+  while (step != 3)
+    {
+      if (step == 99)
+	goto step_99;
+
+      i = pthread_yield ();
+      assert (i == 0);
+    }
+
+step_3:
+  if (step >= 5)
+    goto step_5;
+
+  var++;	/* after-fork1-B */
+  empty ();	/* after-fork1-B */
+  step = 4;
+  while (step != 5)
+    {
+      if (step == 99)
+	goto step_99;
+
+      i = pthread_yield ();
+      assert (i == 0);
+    }
+
+step_5:
+  var++;	/* after-fork2-B */
+  empty ();	/* after-fork2-B */
+  return (void *) 5UL;
+
+step_99:
+  /* We must not get caught here (against a forgotten breakpoint).  */
+  var++;
+  marker ();
+  return (void *) 99UL;
+}
+
+int
+main (void)
+{
+  int i;
+  void *thread_result;
+
+  setbuf (stdout, NULL);
+  printf ("main: %d\n", (int) gettid ());
+
+  /* General hardware breakpoints and watchpoints validity.  */
+  marker ();
+  var++;	/* validity-first */
+  empty ();	/* validity-first */
+
+  i = pthread_create (&thread, NULL, start, NULL);
+  assert (i == 0);
+
+  var++;	/* validity-thread-A */
+  empty ();	/* validity-thread-A */
+  step = 1;
+  while (step != 2)
+    {
+      i = pthread_yield ();
+      assert (i == 0);
+    }
+
+  /* Hardware watchpoints got disarmed here.  */
+  forkoff (1);
+
+  var++;	/* after-fork1-A */
+  empty ();	/* after-fork1-A */
+  step = 3;
+#ifdef FOLLOW_CHILD
+  /* Spawn new thread as it was deleted in the child of FORK.  */
+  i = pthread_create (&thread, NULL, start, NULL);
+  assert (i == 0);
+#endif
+  while (step != 4)
+    {
+      i = pthread_yield ();
+      assert (i == 0);
+    }
+
+  /* A sanity check for double hardware watchpoints removal.  */
+  forkoff (2);
+
+  var++;	/* after-fork2-A */
+  empty ();	/* after-fork2-A */
+  step = 5;
+#ifdef FOLLOW_CHILD
+  /* Spawn new thread as it was deleted in the child of FORK.  */
+  i = pthread_create (&thread, NULL, start, NULL);
+  assert (i == 0);
+#endif
+
+  i = pthread_join (thread, &thread_result);
+  assert (i == 0);
+  assert (thread_result == (void *) 5UL);
+
+  mark_exit ();
+  return 0;
+}
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/watchpoint-fork-parent.c
@@ -0,0 +1,74 @@
+/* Test case for forgotten hw-watchpoints after fork()-off of a process.
+
+   Copyright 2012 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 2 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, write to the Free Software
+   Foundation, Inc., 59 Temple Place - Suite 330,
+   Boston, MA 02111-1307, USA.  */
+
+#include <string.h>
+#include <errno.h>
+#include <sys/types.h>
+#include <unistd.h>
+#include <assert.h>
+#include <stdio.h>
+#include <sys/wait.h>
+
+#include "watchpoint-fork.h"
+
+void
+forkoff (int nr)
+{
+  pid_t child, pid_got;
+  int exit_code = 42 + nr;
+  int status, i;
+
+  child = fork ();
+  switch (child)
+    {
+    case -1:
+      assert (0);
+    case 0:
+      printf ("child%d: %d\n", nr, (int) getpid ());
+      /* Delay to get both the "child%d" and "parent%d" message printed without
+	 a race breaking expect by its endless wait on `$gdb_prompt$':
+	 Breakpoint 3, marker () at ../../../gdb/testsuite/gdb.threads/watchpoint-fork.c:33
+	 33      }
+	 (gdb) parent2: 14223  */
+      i = sleep (1);
+      assert (i == 0);
+
+      /* We must not get caught here (against a forgotten breakpoint).  */
+      var++;
+      marker ();
+
+      _exit (exit_code);
+    default:
+      printf ("parent%d: %d\n", nr, (int) child);
+      /* Delay to get both the "child%d" and "parent%d" message printed, see
+	 above.  */
+      i = sleep (1);
+      assert (i == 0);
+
+      pid_got = wait (&status);
+      assert (pid_got == child);
+      assert (WIFEXITED (status));
+      assert (WEXITSTATUS (status) == exit_code);
+
+      /* We must get caught here (against a false watchpoint removal).  */
+      marker ();
+    }
+}
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/watchpoint-fork-st.c
@@ -0,0 +1,61 @@
+/* Test case for forgotten hw-watchpoints after fork()-off of a process.
+
+   Copyright 2012 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 2 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, write to the Free Software
+   Foundation, Inc., 59 Temple Place - Suite 330,
+   Boston, MA 02111-1307, USA.  */
+
+#include <assert.h>
+#include <unistd.h>
+#include <sys/wait.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+#include "watchpoint-fork.h"
+
+volatile int var;
+
+void
+marker (void)
+{
+}
+
+static void
+mark_exit (void)
+{
+}
+
+int
+main (void)
+{
+  setbuf (stdout, NULL);
+  printf ("main: %d\n", (int) getpid ());
+
+  /* General hardware breakpoints and watchpoints validity.  */
+  marker ();
+  var++;
+  /* Hardware watchpoints got disarmed here.  */
+  forkoff (1);
+  /* This watchpoint got lost before.  */
+  var++;
+  /* A sanity check for double hardware watchpoints removal.  */
+  forkoff (2);
+  var++;
+
+  mark_exit ();
+  return 0;
+}
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/watchpoint-fork.exp
@@ -0,0 +1,158 @@
+# Copyright 2012 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test case for forgotten hw-watchpoints after fork()-off of a process.
+
+set testfile watchpoint-fork
+
+if [is_remote target] {
+    kfail "remote/13584" "gdbserver does not support debugging across fork"
+    return
+}
+
+proc test {type symbol} {
+    global testfile objdir subdir srcdir gdb_prompt
+
+    global pf_prefix
+    set prefix_test $pf_prefix
+    lappend pf_prefix "$type:"
+    set prefix_mt $pf_prefix
+
+    set srcfile_type ${srcdir}/${subdir}/${testfile}-${type}.c
+
+
+    # no threads
+
+    set pf_prefix $prefix_mt
+    lappend pf_prefix "singlethreaded:"
+
+    set executable ${testfile}-${type}-st
+    set srcfile_main ${srcdir}/${subdir}/${testfile}-st.c
+    if { [gdb_compile "${srcfile_main} ${srcfile_type}" ${objdir}/${subdir}/${executable} executable [list debug additional_flags=-D$symbol]] != "" } {
+	untested ${testfile}.exp
+	return
+    }
+    clean_restart $executable
+
+    if [target_info exists gdb,no_hardware_watchpoints] {
+	# The software watchpoint functionality is in GDB an unrelated test.
+	gdb_test_no_output "set can-use-hw-watchpoints 0"
+    }
+
+    gdb_test "show detach-on-fork" "Whether gdb will detach the child of a fork is on\\."
+    gdb_test_no_output "set follow-fork-mode $type"
+    gdb_test "show follow-fork-mode" "Debugger response to a program call of fork or vfork is \"$type\"\\."
+    # Testcase uses it for the `follow-fork-mode child' type.
+    gdb_test "handle SIGUSR1 nostop noprint pass" "No\[ \t\]+No\[ \t\]+Yes.*"
+
+    if ![runto_main] {
+	return
+    }
+
+    gdb_test "watch var" "atchpoint \[0-9\]+: var" "Set the watchpoint"
+
+    # It is never hit but it should not be left over in the fork()ed-off child.
+    if [skip_hw_breakpoint_tests] {
+	set hbreak "break"
+    } else {
+	set hbreak "hbreak"
+    }
+    gdb_test "$hbreak marker"
+
+    gdb_breakpoint "mark_exit"
+
+    gdb_test "continue" \
+	     "reakpoint \[0-9\]+, marker.*" "hardware breakpoints work"
+    gdb_test "continue" \
+	     "atchpoint \[0-9\]+: var.*Old value = 0.*New value = 1.*forkoff *\\(1\\).*" "watchpoints work"
+    gdb_test "continue" \
+	     "reakpoint \[0-9\]+, marker.*" "breakpoint after the first fork"
+    gdb_test "continue" \
+	     "atchpoint \[0-9\]+: var.*Old value = 1.*New value = 2.*forkoff *\\(2\\).*" "watchpoint after the first fork"
+    gdb_test "continue" \
+	     "reakpoint \[0-9\]+, marker.*" "breakpoint after the second fork"
+    gdb_test "continue" \
+	     "atchpoint \[0-9\]+: var.*Old value = 2.*New value = 3.*mark_exit \\(\\);" "watchpoint after the second fork"
+    gdb_test "continue" "Continuing\\..*\r\nBreakpoint \[0-9\]+, mark_exit .*" "finish"
+
+
+    # threads
+
+    if [target_info exists gdb,no_hardware_watchpoints] {
+	# Watchpoint hits would get detected in unexpected threads.
+	return
+    }
+
+    set pf_prefix $prefix_mt
+    lappend pf_prefix "multithreaded:"
+
+    set executable ${testfile}-${type}-mt
+    set srcfile_main ${srcdir}/${subdir}/${testfile}-mt.c
+    if { [gdb_compile_pthreads "${srcfile_main} ${srcfile_type}" ${objdir}/${subdir}/${executable} executable [list debug "additional_flags=-D$symbol -DTHREAD"]] != "" } {
+	untested ${testfile}.exp
+	return
+    }
+    clean_restart $executable
+
+    gdb_test_no_output "set follow-fork-mode $type"
+    # Testcase uses it for the `follow-fork-mode child' type.
+    gdb_test "handle SIGUSR1 nostop noprint pass" "No\[ \t\]+No\[ \t\]+Yes.*"
+
+    if ![runto_main] {
+	return
+    }
+
+    gdb_test "watch var" "atchpoint \[0-9\]+: var" "Set the watchpoint"
+
+    # It should not be left over in the fork()ed-off child.
+    gdb_test "$hbreak marker" {reakpoint [0-9]+.*}
+
+    gdb_breakpoint "mark_exit"
+
+    gdb_test "continue" \
+	     "reakpoint \[0-9\]+, marker.*" "hardware breakpoints work"
+    gdb_test "continue" \
+	     "atchpoint \[0-9\]+: var.*Old value = 0.*New value = 1.*validity-first.*" "singlethread watchpoints work"
+    gdb_test "continue" \
+	     "atchpoint \[0-9\]+: var.*Old value = 1.*New value = 2.*validity-thread-A.*" "multithreaded watchpoints work at A"
+    gdb_test "continue" \
+	     "atchpoint \[0-9\]+: var.*Old value = 2.*New value = 3.*validity-thread-B.*" "multithreaded watchpoints work at B"
+    gdb_test "continue" \
+	     "reakpoint \[0-9\]+, marker.*" "breakpoint (A) after the first fork"
+    gdb_test "continue" \
+	     "atchpoint \[0-9\]+: var.*Old value = 3.*New value = 4.*after-fork1-A.*" "watchpoint A after the first fork"
+    gdb_test "continue" \
+	     "atchpoint \[0-9\]+: var.*Old value = 4.*New value = 5.*after-fork1-B.*" "watchpoint B after the first fork"
+    gdb_test "continue" \
+	     "reakpoint \[0-9\]+, marker.*" "breakpoint (A) after the second fork"
+    gdb_test "continue" \
+	     "atchpoint \[0-9\]+: var.*Old value = 5.*New value = 6.*after-fork2-A.*" "watchpoint A after the second fork"
+    gdb_test "continue" \
+	     "atchpoint \[0-9\]+: var.*Old value = 6.*New value = 7.*after-fork2-B.*" "watchpoint B after the second fork"
+    gdb_test "continue" "Continuing\\..*\r\nBreakpoint \[0-9\]+, mark_exit .*" "finish"
+
+
+    # cleanup
+    set pf_prefix $prefix_test
+}
+
+test parent FOLLOW_PARENT
+
+# Only GNU/Linux is known to support `set follow-fork-mode child'.
+if [istarget "*-*-linux*"] {
+    test child FOLLOW_CHILD
+} else {
+    untested "${testfile}: child"
+}
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/watchpoint-fork.h
@@ -0,0 +1,32 @@
+/* Test case for forgotten hw-watchpoints after fork()-off of a process.
+
+   Copyright 2012 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 2 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, write to the Free Software
+   Foundation, Inc., 59 Temple Place - Suite 330,
+   Boston, MA 02111-1307, USA.  */
+
+#ifdef THREAD
+#include <pthread.h>
+
+extern volatile int step;
+extern pthread_t thread;
+#endif /* THREAD */
+
+extern volatile int var;
+
+extern void marker (void);
+extern void forkoff (int nr);


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