Re: [PATCH 2/3] Adjust breakpoint address by clearing non-significant bits

Pedro Alves
2017-12-08T12:22:51Z

On 12/08/2017 10:04 AM, Yao Qi wrote:

> When program hits a breakpoint, the stopped pc reported by Linux kernel is
> the address *without* tag, so it is better the address recorded in
> breakpoint location is the one without tag too, so we can still match
> breakpoint location address and stopped pc reported by Linux kernel, by
> simple compare.
>

Agreed. LGTM with formatting nit below.

> 2017-10-24 Yao Qi <yao.qi@linaro.org>
>
> * gdb.arch/aarch64-tagged-pointer.c (main): Update.
> * gdb.arch/aarch64-tagged-pointer.exp: Add test for breakpoint.
> ---
> gdb/breakpoint.c | 20 +++++++++----------
> gdb/testsuite/gdb.arch/aarch64-tagged-pointer.c | 8 ++++++++
> gdb/testsuite/gdb.arch/aarch64-tagged-pointer.exp | 24 +++++++++++++++++++++++
> 3 files changed, 42 insertions(+), 10 deletions(-)
>
> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
> index 76bfd53..22b3069 100644
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -6987,12 +6987,7 @@ static CORE_ADDR
> adjust_breakpoint_address (struct gdbarch *gdbarch,
> CORE_ADDR bpaddr, enum bptype bptype)
> {
> - if (!gdbarch_adjust_breakpoint_address_p (gdbarch))
> - {
> - /* Very few targets need any kind of breakpoint adjustment. */
> - return bpaddr;
> - }
> - else if (bptype == bp_watchpoint
> + if (bptype == bp_watchpoint
> || bptype == bp_hardware_watchpoint
> || bptype == bp_read_watchpoint
> || bptype == bp_access_watchpoint

Please indent the rest of the condition before pushing.

OK with that fixed.

> @@ -7014,11 +7009,16 @@ adjust_breakpoint_address (struct gdbarch *gdbarch,
> }
> else

Thanks,
Pedro Alves