From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9592 invoked by alias); 21 Sep 2011 13:29:50 -0000 Received: (qmail 9577 invoked by uid 22791); 21 Sep 2011 13:29:49 -0000 X-SWARE-Spam-Status: No, hits=-1.8 required=5.0 tests=AWL,BAYES_00 X-Spam-Check-By: sourceware.org Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 21 Sep 2011 13:29:35 +0000 Received: from nat-ies.mentorg.com ([192.94.31.2] helo=EU1-MAIL.mgc.mentorg.com) by relay1.mentorg.com with esmtp id 1R6Mrm-0002a1-U4 from pedro_alves@mentor.com ; Wed, 21 Sep 2011 06:29:35 -0700 Received: from scottsdale.localnet ([172.16.63.104]) by EU1-MAIL.mgc.mentorg.com with Microsoft SMTPSVC(6.0.3790.1830); Wed, 21 Sep 2011 14:29:33 +0100 From: Pedro Alves To: gdb-patches@sourceware.org Subject: Re: [rfc, gdbserver] Support hardware watchpoints on ARM Date: Wed, 21 Sep 2011 13:46:00 -0000 User-Agent: KMail/1.13.6 (Linux/2.6.38-11-generic; KDE/4.7.0; x86_64; ; ) Cc: "Ulrich Weigand" , patches@linaro.org References: <201109121723.p8CHN03n017032@d06av02.portsmouth.uk.ibm.com> In-Reply-To: <201109121723.p8CHN03n017032@d06av02.portsmouth.uk.ibm.com> MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201109211429.31335.pedro@codesourcery.com> X-IsSubscribed: yes 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 X-SW-Source: 2011-09/txt/msg00381.txt.bz2 Hi Ulrich, I was just looking over the patch before lunch, and meanwhile you've committed it. :-) It looks fine to me in any case. :-) I just had a couple minor remarks. On Monday 12 September 2011 18:23:00, Ulrich Weigand wrote: > + if (hwbp_type == arm_hwbp_break) > + { > + /* For breakpoints, the length field encodes the mode. */ > + switch (len) > + { > + case 2: /* 16-bit Thumb mode breakpoint */ > + case 3: /* 32-bit Thumb mode breakpoint */ > + mask = 0x3 << (addr & 2); > + break; > + case 4: /* 32-bit ARM mode breakpoint */ > + mask = 0xf; > + break; > + default: > + /* Unsupported. */ > + return -1; > + } > + > + addr &= ~3; Is this ~3 correct for 16-bit Thumb? > + } > +static void > +arm_prepare_to_resume (struct lwp_info *lwp) > +{ > + int pid = lwpid_of (lwp); > + struct process_info *proc = find_process_pid (pid_of (lwp)); > + struct arch_process_info *proc_info = proc->private->arch_private; > + struct arch_lwp_info *lwp_info = lwp->arch_private; > + int i; > + > + for (i = 0; i < arm_linux_get_hw_breakpoint_count (); i++) It's a bit unfortunate that arm_linux_get_hw_breakpoint_count relies on the current_inferior global having been set to LWP by the callers. We try to avoid that when we have an LWP handy. Can we make arm_linux_get_hw_breakpoint_count take an LWP argument? On Monday 12 September 2011 18:23:00, Ulrich Weigand wrote: > + if (arm_hwbp_control_is_enabled (proc_info->bpts[i].control)) > + if (ptrace (PTRACE_SETHBPREGS, pid, ((i << 1) + 1), > + &proc_info->bpts[i].address) < 0) > + error (_("Unexpected error setting breakpoint address")); > + > + if (arm_hwbp_control_is_initialized (proc_info->bpts[i].control)) > + if (ptrace (PTRACE_SETHBPREGS, pid, ((i << 1) + 2), > + &proc_info->bpts[i].control) < 0) > + error (_("Unexpected error setting breakpoint")); I think perror_with_name would be better, so we can see on the logs the errno ptrace set on error. -- Pedro Alves