From: "Breazeal, Don" <Don_Breazeal@mentor.com>
To: Vidya Praveen <vidyapraveen@arm.com>
Cc: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
Subject: RE: [pushed][PATCH 4/7] Arch-specific remote follow fork
Date: Thu, 14 May 2015 16:02:00 -0000 [thread overview]
Message-ID: <DA279C53C4A5884A907135DFCD7A059A37D4B8FB@NA-MBX-04.mgc.mentorg.com> (raw)
In-Reply-To: <20150514102532.GB2410@e103625-lin.cambridge.arm.com>
Thanks, I will take a look. --Don
> -----Original Message-----
> From: Vidya Praveen [mailto:vidyapraveen@arm.com]
> Sent: Thursday, May 14, 2015 3:26 AM
> To: Breazeal, Don
> Cc: gdb-patches@sourceware.org
> Subject: Re: [pushed][PATCH 4/7] Arch-specific remote follow fork
>
> Don,
>
> There are several instances of xyz->private->arch_private where xyz is
> (struct process_info*). Assume it is a typo (should've been priv instead
> of private)?
>
> I atleast see aarch64 build of this fail.
>
> Regards
> VP.
>
>
> On Tue, May 12, 2015 at 06:28:12PM +0100, Don Breazeal wrote:
> > This is the version of the patch that I pushed.
> > Thanks
> > --Don
> >
> >
> > This patch implements the architecture-specific pieces of follow-fork
> > for remote and extended-remote Linux targets, 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.
> >
> > This follows the native implementation as closely as possible 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 and an ARM board. Aarch64 hasn't been tested.
> >
> > gdb/gdbserver/ChangeLog:
> >
> > * 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/ChangeLog | 17 +++++++++
> > 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 +++++++++++++++
> > 7 files changed, 187 insertions(+), 12 deletions(-)
> >
> > diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog index
> > 0ef695c..0bdcdf3 100644
> > --- a/gdb/gdbserver/ChangeLog
> > +++ b/gdb/gdbserver/ChangeLog
> > @@ -1,5 +1,22 @@
> > 2015-05-12 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.
> > +
> > +2015-05-12 Don Breazeal <donb@codesourcery.com>
> > +
> > * linux-low.c (handle_extended_wait): Implement return value,
> > rename argument 'event_child' to 'event_lwp', handle
> > PTRACE_EVENT_FORK, call internal_error for unrecognized event.
> > diff --git a/gdb/gdbserver/linux-aarch64-low.c
> > b/gdb/gdbserver/linux-aarch64-low.c
> > index d44175e..7e6e9ac 100644
> > --- a/gdb/gdbserver/linux-aarch64-low.c
> > +++ b/gdb/gdbserver/linux-aarch64-low.c
> > @@ -1135,6 +1135,33 @@ aarch64_linux_new_thread (struct lwp_info *lwp)
> > lwp->arch_private = 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. */
> >
> > @@ -1299,6 +1326,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 d408dcd..3420dea 100644
> > --- a/gdb/gdbserver/linux-arm-low.c
> > +++ b/gdb/gdbserver/linux-arm-low.c
> > @@ -717,6 +717,47 @@ arm_new_thread (struct lwp_info *lwp)
> > lwp->arch_private = 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 b280c17..d9053ad 100644
> > --- a/gdb/gdbserver/linux-low.c
> > +++ b/gdb/gdbserver/linux-low.c
> > @@ -489,6 +489,10 @@ handle_extended_wait (struct lwp_info *event_lwp,
> 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 in the parent thread. */
> > event_lwp->waitstatus.kind = TARGET_WAITKIND_FORKED;
> > event_lwp->waitstatus.value.related_pid = ptid; diff --git
> > a/gdb/gdbserver/linux-low.h b/gdb/gdbserver/linux-low.h index
> > 41067d6..3300da9 100644
> > --- a/gdb/gdbserver/linux-low.h
> > +++ b/gdb/gdbserver/linux-low.h
> > @@ -187,6 +187,9 @@ struct linux_target_ops
> > allocate it here. */
> > void (*new_thread) (struct lwp_info *);
> >
> > + /* 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 7988d07..4601ad0 100644
> > --- a/gdb/gdbserver/linux-mips-low.c
> > +++ b/gdb/gdbserver/linux-mips-low.c
> > @@ -344,6 +344,68 @@ mips_linux_new_thread (struct lwp_info *lwp)
> > lwp->arch_private = 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 *priv = proc->priv->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 = &priv->current_watches;
> > - while (*pw != NULL)
> > - pw = &(*pw)->next;
> > - *pw = new_watch;
> > + mips_add_watchpoint (priv, addr, len, watch_type);
> >
> > priv->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 763df08..4aef7b7 100644
> > --- a/gdb/gdbserver/linux-x86-low.c
> > +++ b/gdb/gdbserver/linux-x86-low.c
> > @@ -634,6 +634,34 @@ x86_linux_new_process (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->priv != NULL
> > + && parent->priv->arch_private != NULL);
> > + gdb_assert (child->priv != NULL
> > + && child->priv->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->priv->arch_private = *parent->priv->arch_private; }
> > +
> > /* See nat/x86-dregs.h. */
> >
> > struct x86_debug_reg_state *
> > @@ -3263,6 +3291,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.8.1.1
> >
prev parent reply other threads:[~2015-05-14 16:02 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-05-12 17:28 [pushed][PATCH 1/7] Identify remote fork event support Don Breazeal
2015-05-12 17:28 ` [pushed][PATCH 2/7] Clone remote breakpoints Don Breazeal
2015-05-12 17:28 ` [pushed][PATCH 3/7] Extended-remote Linux follow fork Don Breazeal
2015-05-12 17:29 ` [pushed][PATCH 6/7] Extended-remote fork catch Don Breazeal
2015-05-14 22:11 ` Sergio Durigan Junior
2015-05-14 23:26 ` Don Breazeal
2015-05-14 23:41 ` Sergio Durigan Junior
2015-05-28 22:22 ` Don Breazeal
2015-05-12 17:29 ` [pushed][PATCH 7/7] Extended-remote fork event docs Don Breazeal
2015-05-12 17:29 ` [pushed][PATCH 5/7] Extended-remote follow vfork Don Breazeal
2015-05-12 17:29 ` [pushed][PATCH 4/7] Arch-specific remote follow fork Don Breazeal
2015-05-14 10:25 ` Vidya Praveen
2015-05-14 16:02 ` Breazeal, Don [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DA279C53C4A5884A907135DFCD7A059A37D4B8FB@NA-MBX-04.mgc.mentorg.com \
--to=don_breazeal@mentor.com \
--cc=gdb-patches@sourceware.org \
--cc=vidyapraveen@arm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).