From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 44909 invoked by alias); 14 May 2015 16:02:11 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 44893 invoked by uid 89); 14 May 2015 16:02:10 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.4 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-HELO: relay1.mentorg.com Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 14 May 2015 16:02:04 +0000 Received: from svr-orw-fem-04.mgc.mentorg.com ([147.34.97.41]) by relay1.mentorg.com with esmtp id 1YsvaC-0003yG-E3 from Don_Breazeal@mentor.com ; Thu, 14 May 2015 09:02:00 -0700 Received: from NA-MBX-04.mgc.mentorg.com ([169.254.4.232]) by SVR-ORW-FEM-04.mgc.mentorg.com ([147.34.97.41]) with mapi id 14.03.0224.002; Thu, 14 May 2015 09:02:00 -0700 From: "Breazeal, Don" To: Vidya Praveen CC: "gdb-patches@sourceware.org" Subject: RE: [pushed][PATCH 4/7] Arch-specific remote follow fork Date: Thu, 14 May 2015 16:02:00 -0000 Message-ID: References: <1431451695-12273-1-git-send-email-donb@codesourcery.com> <1431451695-12273-4-git-send-email-donb@codesourcery.com> <20150514102532.GB2410@e103625-lin.cambridge.arm.com> In-Reply-To: <20150514102532.GB2410@e103625-lin.cambridge.arm.com> Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-SW-Source: 2015-05/txt/msg00366.txt.bz2 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 >=20 > Don, >=20 > 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)? >=20 > I atleast see aarch64 build of this fail. >=20 > Regards > VP. >=20 >=20 > 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) : Initialize new member. > > * linux-arm-low.c (arm_new_fork): New function. > > (the_low_target) : Initialize new member. > > * linux-low.c (handle_extended_wait): Call new target function > > new_fork. > > * linux-low.h (struct linux_target_ops) : New > member. > > * linux-mips-low.c (mips_add_watchpoint): New function > > extracted from mips_insert_point. > > (the_low_target) : 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) : 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 > > > > + * linux-aarch64-low.c (aarch64_linux_new_fork): New function. > > + (the_low_target) : Initialize new member. > > + * linux-arm-low.c (arm_new_fork): New function. > > + (the_low_target) : Initialize new member. > > + * linux-low.c (handle_extended_wait): Call new target function > > + new_fork. > > + * linux-low.h (struct linux_target_ops) : New > member. > > + * linux-mips-low.c (mips_add_watchpoint): New function > > + extracted from mips_insert_point. > > + (the_low_target) : 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) : Initialize new member. > > + > > +2015-05-12 Don Breazeal > > + > > * 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 =3D 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 !=3D NULL > > + && parent->private->arch_private !=3D NULL); > > + gdb_assert (child->private !=3D NULL > > + && child->private->arch_private !=3D 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 =3D *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 =3D > > 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 =3D info; > > } > > > > +static void > > +arm_new_fork (struct process_info *parent, struct process_info > > +*child) { > > + struct arch_process_info *parent_proc_info =3D > > +parent->private->arch_private; > > + struct arch_process_info *child_proc_info =3D > > +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 !=3D NULL > > + && parent->private->arch_private !=3D NULL); gdb_assert > > + (child->private !=3D NULL > > + && child->private->arch_private !=3D 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 =3D *parent_proc_info; > > + > > + /* Mark all the hardware breakpoints and watchpoints as changed to > > + make sure that the registers will be updated. */ > > + child_lwp =3D find_lwp_pid (ptid_of (child)); > > + child_lwp_info =3D child_lwp->arch_private; > > + for (i =3D 0; i < MAX_BPTS; i++) > > + child_lwp_info->bpts_changed[i] =3D 1; > > + for (i =3D 0; i < MAX_WPTS; i++) > > + child_lwp_info->wpts_changed[i] =3D 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 > > =3D { > > 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 =3D tdesc; > > child_lwp->must_set_ptrace_flags =3D 1; > > > > + /* Clone arch-specific process data. */ > > + if (the_low_target.new_fork !=3D NULL) > > + the_low_target.new_fork (parent_proc, child_proc); > > + > > /* Save fork info in the parent thread. */ > > event_lwp->waitstatus.kind =3D TARGET_WAITKIND_FORKED; > > event_lwp->waitstatus.value.related_pid =3D 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 =3D 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 =3D xmalloc (sizeof (struct mips_watchpoint)); > > + new_watch->addr =3D addr; new_watch->len =3D len; new_watch->type = =3D > > + watch_type; new_watch->next =3D NULL; > > + > > + pw =3D &private->current_watches; > > + while (*pw !=3D NULL) > > + pw =3D &(*pw)->next; > > + *pw =3D 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 !=3D NULL > > + && parent->private->arch_private !=3D NULL); gdb_assert > > + (child->private !=3D NULL > > + && child->private->arch_private !=3D 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 =3D parent->private->arch_private; child_private =3D > > + child->private->arch_private; > > + > > + child_private->watch_readback_valid =3D > > + parent_private->watch_readback_valid; > > + child_private->watch_readback =3D parent_private->watch_readback; > > + > > + for (wp =3D parent_private->current_watches; wp !=3D NULL; wp =3D wp- > >next) > > + mips_add_watchpoint (child_private, wp->addr, wp->len, wp->type); > > + > > + child_private->watch_mirror =3D 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 =3D current_process (); > > struct arch_process_info *priv =3D 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 =3D xmalloc (sizeof (struct mips_watchpoint)); > > - new_watch->addr =3D addr; > > - new_watch->len =3D len; > > - new_watch->type =3D watch_type; > > - new_watch->next =3D NULL; > > - > > - pw =3D &priv->current_watches; > > - while (*pw !=3D NULL) > > - pw =3D &(*pw)->next; > > - *pw =3D new_watch; > > + mips_add_watchpoint (priv, addr, len, watch_type); > > > > priv->watch_mirror =3D regs; > > > > @@ -845,6 +896,7 @@ struct linux_target_ops the_low_target =3D { > > 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 !=3D NULL > > + && parent->priv->arch_private !=3D NULL); > > + gdb_assert (child->priv !=3D NULL > > + && child->priv->arch_private !=3D 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 =3D *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 =3D > > 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 > >