Re: [rfc, gdbserver] Support hardware watchpoints on ARM

Pedro Alves
2011-09-21T13:29:50Z

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