From patchwork Fri Sep 19 21:26:45 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Don Breazeal X-Patchwork-Id: 2931 Received: (qmail 15165 invoked by alias); 19 Sep 2014 21:26:52 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 15152 invoked by uid 89); 19 Sep 2014 21:26:51 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL, BAYES_00 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; Fri, 19 Sep 2014 21:26:49 +0000 Received: from svr-orw-exc-10.mgc.mentorg.com ([147.34.98.58]) by relay1.mentorg.com with esmtp id 1XV5hW-0003Qg-4Y from Don_Breazeal@mentor.com for gdb-patches@sourceware.org; Fri, 19 Sep 2014 14:26:46 -0700 Received: from [127.0.0.1] ([172.30.12.90]) by SVR-ORW-EXC-10.mgc.mentorg.com with Microsoft SMTPSVC(6.0.3790.4675); Fri, 19 Sep 2014 14:26:45 -0700 Message-ID: <541C9F95.1080607@codesourcery.com> Date: Fri, 19 Sep 2014 14:26:45 -0700 From: "Breazeal, Don" User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Thunderbird/31.1.1 MIME-Version: 1.0 To: gdb-patches@sourceware.org Subject: Re: [PATCH 07/16 v2] Extended-remote arch-specific follow fork References: <1407434395-19089-1-git-send-email-donb@codesourcery.com> <1408580964-27916-8-git-send-email-donb@codesourcery.com> In-Reply-To: <1408580964-27916-8-git-send-email-donb@codesourcery.com> X-IsSubscribed: yes 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 > > * 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/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(-) @@ -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, 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. */