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]

Re: [PATCH 07/16 v2] Extended-remote arch-specific follow fork


This is an update to this patch that makes inheritance of hardware
watchpoints across a fork work on ARM targets.  I had noted previously
that I had not tested inheritance of hardware watchpoints across a fork
on ARM.  I have not completed that testing, which exposed a couple of
problems.

The difference between the previous patch and this one is that the
function gdbserver/linux-arm-low.c:arm_new_fork, primarily so that the
arrays of flags used to mark hardware breakpoints and watchpoints as
"changed" are set for the new forked process.

Note that I still have no way to test the aarch64 target, but I did
examine the code and determined that it doesn't require the changes made
for the ARM target.

Thanks,
--Don

On 8/20/2014 5:29 PM, Don Breazeal wrote:
> This patch implements the architecture-specific pieces of follow-fork,
> which in the current implementation copyies the parent's debug register
> state into the new child's data structures.  This is required for x86,
> arm, aarch64, and mips.
> 
> I followed the native implementation as closely as I could by
> implementing a new linux_target_ops function 'new_fork', which is
> analogous to 'linux_nat_new_fork' in linux-nat.c.  In gdbserver, the debug
> registers are stored in the process list, instead of an
> architecture-specific list, so the function arguments are process_info
> pointers instead of an lwp_info and a pid as in the native implementation.
> 
> In the MIPS implementation the debug register mirror is stored differently
> from x86, ARM, and aarch64, so instead of doing a simple structure assignment
> I had to clone the list of watchpoint structures.
> 
> Tested using gdb.threads/watchpoint-fork.exp on x86, and ran manual tests
> on a MIPS board.
> 
> I ran manual tests on an ARM board, but on the boards I had access to at
> the time, hardware watchpoints did not work, even in the unmodified version
> of the debugger.  The kernel on one of the boards should have been new
> enough to provide the support.  I didn't debug it further at the time.  If
> someone is able to test this easily, please do.  Otherwise I plan to 
> come back to this.
> 
> I don't currently have access to an aarch64 board, so again if someone is
> able to test this easily, please do.
> 
> Thanks
> --Don
> 
> gdb/gdbserver/
> 2014-08-20  Don Breazeal  <donb@codesourcery.com>
> 
> 	* linux-aarch64-low.c (aarch64_linux_new_fork): New function.
> 	(the_low_target) <new_fork>: Initialize new member.
> 	* linux-arm-low.c (arm_new_fork): New function.
> 	(the_low_target) <new_fork>: Initialize new member.
> 	* linux-low.c (handle_extended_wait): Call new target function
> 	new_fork.
> 	* linux-low.h (struct linux_target_ops) <new_fork>: New member.
> 	* linux-mips-low.c (mips_add_watchpoint): New function
> 	extracted from mips_insert_point.
> 	(the_low_target) <new_fork>: Initialize new member.
> 	(mips_linux_new_fork): New function.
> 	(mips_insert_point): Call mips_add_watchpoint.
> 	* linux-x86-low.c (x86_linux_new_fork): New function.
> 	(the_low_target) <new_fork>: Initialize new member.
> 

---
 gdb/gdbserver/linux-aarch64-low.c |   28 +++++++++++++
 gdb/gdbserver/linux-arm-low.c     |   42 ++++++++++++++++++++
 gdb/gdbserver/linux-low.c         |    4 ++
 gdb/gdbserver/linux-low.h         |    3 +
 gdb/gdbserver/linux-mips-low.c    |   76
+++++++++++++++++++++++++++++++------
 gdb/gdbserver/linux-x86-low.c     |   29 ++++++++++++++
 6 files changed, 170 insertions(+), 12 deletions(-)

diff --git a/gdb/gdbserver/linux-aarch64-low.c
b/gdb/gdbserver/linux-aarch64-low.c
index 654b319..31ec0d7 100644
--- a/gdb/gdbserver/linux-aarch64-low.c
+++ b/gdb/gdbserver/linux-aarch64-low.c
@@ -1129,6 +1129,33 @@ aarch64_linux_new_thread (void)
   return info;
 }

+static void
+aarch64_linux_new_fork (struct process_info *parent,
+			struct process_info *child)
+{
+  /* These are allocated by linux_add_process.  */
+  gdb_assert (parent->private != NULL
+	      && parent->private->arch_private != NULL);
+  gdb_assert (child->private != NULL
+	      && child->private->arch_private != NULL);
+
+  /* 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 core assumes the child inherits the watchpoints/hw
+     breakpoints of the parent, and will remove them all from the
+     forked off process.  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, thus making
+     this compatible with older Linux kernels too.  */
+
+  *child->private->arch_private = *parent->private->arch_private;
+}
+
 /* Called when resuming a thread.
    If the debug regs have changed, update the thread's copies.  */

@@ -1293,6 +1320,7 @@ struct linux_target_ops the_low_target =
   NULL,
   aarch64_linux_new_process,
   aarch64_linux_new_thread,
+  aarch64_linux_new_fork,
   aarch64_linux_prepare_to_resume,
 };

diff --git a/gdb/gdbserver/linux-arm-low.c b/gdb/gdbserver/linux-arm-low.c
index 8b72523..fe9c34e 100644
--- a/gdb/gdbserver/linux-arm-low.c
+++ b/gdb/gdbserver/linux-arm-low.c
@@ -717,6 +717,47 @@ arm_new_thread (void)
   return info;
 }

+static void
+arm_new_fork (struct process_info *parent, struct process_info *child)
+{
+  struct arch_process_info *parent_proc_info =
parent->private->arch_private;
+  struct arch_process_info *child_proc_info = child->private->arch_private;
+  struct lwp_info *child_lwp;
+  struct arch_lwp_info *child_lwp_info;
+  int i;
+
+  /* These are allocated by linux_add_process.  */
+  gdb_assert (parent->private != NULL
+	      && parent->private->arch_private != NULL);
+  gdb_assert (child->private != NULL
+	      && child->private->arch_private != NULL);
+
+  /* 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 core assumes the child inherits the watchpoints/hw
+     breakpoints of the parent, and will remove them all from the
+     forked off process.  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, thus making
+     this compatible with older Linux kernels too.  */
+
+  *child_proc_info = *parent_proc_info;
+
+  /* Mark all the hardware breakpoints and watchpoints as changed to
+     make sure that the registers will be updated.  */
+  child_lwp = find_lwp_pid (ptid_of (child));
+  child_lwp_info = child_lwp->arch_private;
+  for (i = 0; i < MAX_BPTS; i++)
+    child_lwp_info->bpts_changed[i] = 1;
+  for (i = 0; i < MAX_WPTS; i++)
+    child_lwp_info->wpts_changed[i] = 1;
+}
+
 /* Called when resuming a thread.
    If the debug regs have changed, update the thread's copies.  */
 static void
@@ -920,6 +961,7 @@ struct linux_target_ops the_low_target = {
   NULL, /* siginfo_fixup */
   arm_new_process,
   arm_new_thread,
+  arm_new_fork,
   arm_prepare_to_resume,
 };

diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 56993bd..1fc9be9 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -442,6 +442,10 @@ handle_extended_wait (struct lwp_info *event_child,
int wstat)
 	  child_proc->tdesc = tdesc;
 	  child_lwp->must_set_ptrace_flags = 1;

+	  /* Clone arch-specific process data.  */
+	  if (the_low_target.new_fork != NULL)
+	    the_low_target.new_fork (parent_proc, child_proc);
+
 	  /* Save fork info for target processing.  */
 	  current_inferior->pending_follow.kind = TARGET_WAITKIND_FORKED;
 	  current_inferior->pending_follow.value.related_pid = ptid;
diff --git a/gdb/gdbserver/linux-low.h b/gdb/gdbserver/linux-low.h
index a903430..53d1c24 100644
--- a/gdb/gdbserver/linux-low.h
+++ b/gdb/gdbserver/linux-low.h
@@ -185,6 +185,9 @@ struct linux_target_ops
      allocate it here.  */
   struct arch_lwp_info * (*new_thread) (void);

+  /* Hook to call, if any, when a new fork is attached.  */
+  void (*new_fork) (struct process_info *parent, struct process_info
*child);
+
   /* Hook to call prior to resuming a thread.  */
   void (*prepare_to_resume) (struct lwp_info *);

diff --git a/gdb/gdbserver/linux-mips-low.c b/gdb/gdbserver/linux-mips-low.c
index 0fc8cb4..d5db79d 100644
--- a/gdb/gdbserver/linux-mips-low.c
+++ b/gdb/gdbserver/linux-mips-low.c
@@ -344,6 +344,68 @@ mips_linux_new_thread (void)
   return info;
 }

+/* Create a new mips_watchpoint and add it to the list.  */
+
+static void
+mips_add_watchpoint (struct arch_process_info *private, CORE_ADDR addr,
+		     int len, int watch_type)
+{
+  struct mips_watchpoint *new_watch;
+  struct mips_watchpoint **pw;
+
+  new_watch = xmalloc (sizeof (struct mips_watchpoint));
+  new_watch->addr = addr;
+  new_watch->len = len;
+  new_watch->type = watch_type;
+  new_watch->next = NULL;
+
+  pw = &private->current_watches;
+  while (*pw != NULL)
+    pw = &(*pw)->next;
+  *pw = new_watch;
+}
+
+/* Hook to call when a new fork is attached.  */
+
+static void
+mips_linux_new_fork (struct process_info *parent,
+			struct process_info *child)
+{
+  struct arch_process_info *parent_private;
+  struct arch_process_info *child_private;
+  struct mips_watchpoint *wp;
+
+  /* These are allocated by linux_add_process.  */
+  gdb_assert (parent->private != NULL
+	      && parent->private->arch_private != NULL);
+  gdb_assert (child->private != NULL
+	      && child->private->arch_private != NULL);
+
+  /* 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 core assumes the child inherits the watchpoints/hw
+     breakpoints of the parent, and will remove them all from the
+     forked off process.  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, thus making
+     this compatible with older Linux kernels too.  */
+
+  parent_private = parent->private->arch_private;
+  child_private = child->private->arch_private;
+
+  child_private->watch_readback_valid =
parent_private->watch_readback_valid;
+  child_private->watch_readback = parent_private->watch_readback;
+
+  for (wp = parent_private->current_watches; wp != NULL; wp = wp->next)
+    mips_add_watchpoint (child_private, wp->addr, wp->len, wp->type);
+
+  child_private->watch_mirror = parent_private->watch_mirror;
+}
 /* This is the implementation of linux_target_ops method
    prepare_to_resume.  If the watch regs have changed, update the
    thread's copies.  */
@@ -397,8 +459,6 @@ mips_insert_point (enum raw_bkpt_type type,
CORE_ADDR addr,
   struct process_info *proc = current_process ();
   struct arch_process_info *private = proc->private->arch_private;
   struct pt_watch_regs regs;
-  struct mips_watchpoint *new_watch;
-  struct mips_watchpoint **pw;
   int pid;
   long lwpid;
   enum target_hw_bp_type watch_type;
@@ -425,16 +485,7 @@ mips_insert_point (enum raw_bkpt_type type,
CORE_ADDR addr,
     return -1;

   /* It fit.  Stick it on the end of the list.  */
-  new_watch = xmalloc (sizeof (struct mips_watchpoint));
-  new_watch->addr = addr;
-  new_watch->len = len;
-  new_watch->type = watch_type;
-  new_watch->next = NULL;
-
-  pw = &private->current_watches;
-  while (*pw != NULL)
-    pw = &(*pw)->next;
-  *pw = new_watch;
+  mips_add_watchpoint (private, addr, len, watch_type);

   private->watch_mirror = regs;

@@ -845,6 +896,7 @@ struct linux_target_ops the_low_target = {
   NULL, /* siginfo_fixup */
   mips_linux_new_process,
   mips_linux_new_thread,
+  mips_linux_new_fork,
   mips_linux_prepare_to_resume
 };

diff --git a/gdb/gdbserver/linux-x86-low.c b/gdb/gdbserver/linux-x86-low.c
index 838e7c9..10b8a56 100644
--- a/gdb/gdbserver/linux-x86-low.c
+++ b/gdb/gdbserver/linux-x86-low.c
@@ -768,6 +768,34 @@ x86_linux_new_thread (void)
   return info;
 }

+/* Target routine for linux_new_fork.  */
+
+static void
+x86_linux_new_fork (struct process_info *parent, struct process_info
*child)
+{
+  /* These are allocated by linux_add_process.  */
+  gdb_assert (parent->private != NULL
+	      && parent->private->arch_private != NULL);
+  gdb_assert (child->private != NULL
+	      && child->private->arch_private != NULL);
+
+  /* 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 core assumes the child inherits the watchpoints/hw
+     breakpoints of the parent, and will remove them all from the
+     forked off process.  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, thus making
+     this compatible with older Linux kernels too.  */
+
+  *child->private->arch_private = *parent->private->arch_private;
+}
+
 /* Called when resuming a thread.
    If the debug regs have changed, update the thread's copies.  */

@@ -3428,6 +3456,7 @@ struct linux_target_ops the_low_target =
   x86_siginfo_fixup,
   x86_linux_new_process,
   x86_linux_new_thread,
+  x86_linux_new_fork,
   x86_linux_prepare_to_resume,
   x86_linux_process_qsupported,
   x86_supports_tracepoints,
-- 
1.7.0.4



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