This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] aarch64 multi-arch part 6: HW breakpoint on unaligned address
- From: Yao Qi <qiyaoltc at gmail dot com>
- To: Pedro Alves <palves at redhat dot com>
- Cc: Yao Qi <qiyaoltc at gmail dot com>, gdb-patches at sourceware dot org
- Date: Tue, 13 Oct 2015 16:26:35 +0100
- Subject: Re: [PATCH] aarch64 multi-arch part 6: HW breakpoint on unaligned address
- Authentication-results: sourceware.org; auth=none
- References: <1444731060-16237-1-git-send-email-yao dot qi at linaro dot org> <561CE5D2 dot 8030505 at redhat dot com>
Pedro Alves <palves@redhat.com> writes:
>> + {
>> + if (len == 3)
>> + len = 2;
>
> I think this warrants a comment. E.g., someone reading
> arm-linux-low.c:arm_linux_hw_point_initialize quite easily grasps
> what 3 means.
>
How about the comment like this?
if (len == 3)
{
/* LEN is 3 means the breakpoint is set on a 32-bit thumb
instruction. Set it to 2 to correctly encode length bit
mask in hardware/watchpoint control register. */
len = 2;
}
>> diff --git a/gdb/nat/aarch64-linux-hw-point.c b/gdb/nat/aarch64-linux-hw-point.c
>> index bca6ec1..d15e518 100644
>> --- a/gdb/nat/aarch64-linux-hw-point.c
>> +++ b/gdb/nat/aarch64-linux-hw-point.c
>> @@ -112,8 +112,17 @@ aarch64_point_encode_ctrl_reg (enum target_hw_bp_type type, int len)
>> static int
>> aarch64_point_is_aligned (int is_watchpoint, CORE_ADDR addr, int len)
>> {
>> - unsigned int alignment = is_watchpoint ? AARCH64_HWP_ALIGNMENT
>> - : AARCH64_HBP_ALIGNMENT;
>> + unsigned int alignment = 0;
>> +
>> + if (is_watchpoint)
>> + alignment = AARCH64_HWP_ALIGNMENT;
>> + else
>> + {
>> + /* Set alignment to 2 only if the current process is 32-bit,
>> + since thumb instruction can be 2-byte aligned. Otherwise, set
>> + alignment to AARCH64_HBP_ALIGNMENT. */
>> + alignment = 2;
>
> Is some other code doing what the comment says? I'm not seeing
> any obvious 32-bit check.
No, I don't do the 32-bit check here. Ideally, we should set alignment
to 2 only when the process is 32-bit, and still use 4 as alignment
otherwise. However, I don't find an easy way to do the 32-bit check
here, because this code is used by both GDB and GDBserver. We can do
the 32-bit check in GDB and GDBserver respectively, and pass the result
to nat/aarch64-linux-hw-point.c, but I don't like putting information down
multiple levels like this.
--
Yao (éå)