This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [rfc, gdbserver] Support hardware watchpoints on ARM
- From: Pedro Alves <pedro at codesourcery dot com>
- To: gdb-patches at sourceware dot org
- Cc: "Ulrich Weigand" <uweigand at de dot ibm dot com>, patches at linaro dot org
- Date: Wed, 21 Sep 2011 14:29:31 +0100
- Subject: Re: [rfc, gdbserver] Support hardware watchpoints on ARM
- References: <201109121723.p8CHN03n017032@d06av02.portsmouth.uk.ibm.com>
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