Re: [PATCH 3/3] [AArch64] Remove tag from address for watchpoint

[Pedro Alves <palves at redhat dot com>]

On 10/26/2017 09:29 AM, Yao Qi wrote:

> +typedef CORE_ADDR (gdbarch_addr_tag_remove_ftype) (struct gdbarch *gdbarch, CORE_ADDR addr);
> +extern CORE_ADDR gdbarch_addr_tag_remove (struct gdbarch *gdbarch, CORE_ADDR addr);
> +extern void set_gdbarch_addr_tag_remove (struct gdbarch *gdbarch, gdbarch_addr_tag_remove_ftype *addr_tag_remove);
> +
>  /* FIXME/cagney/2001-01-18: This should be split in two.  A target method that
>     indicates if the target needs software single step.  An ISA method to
>     implement it.
> diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
> index 6459b12..1f673e7 100755
> --- a/gdb/gdbarch.sh
> +++ b/gdb/gdbarch.sh
> @@ -621,6 +621,11 @@ m;CORE_ADDR;convert_from_func_ptr_addr;CORE_ADDR addr, struct target_ops *targ;a
>  # possible it should be in TARGET_READ_PC instead).
>  m;CORE_ADDR;addr_bits_remove;CORE_ADDR addr;addr;;core_addr_identity;;0
>  
> +# On some machines, there are bits in address which are ignored by the
> +# kernel, the hardeware, etc.  They are called "tag", which can be
> +# regarded as additional data associated with the address.
> +m;CORE_ADDR;addr_tag_remove;CORE_ADDR addr;addr;;core_addr_identity;;0

typo: "hardeware".

Hmmm.  We have gdbarch_addr_bit / addr_bit to represent the size
of a target address.  I'm thinking that instead of addr_tag_remove,
this would a bit more in line with the current scheme if this were
a new "significant_addr_bit" gdbarch property?  I.e.:

 /* On some machines, not all bits of an address word are significant.
    For example, on Aarch64, the top bits of an address known as the "tag"
    are ignored by the kernel, the hardware, etc. and can be regarded as
    additional data associated with the address.  */
 int gdbarch_significant_addr_bit (struct gdbarch *gdbarch);

significant_addr_bit would default to addr_bit.

And then at places where we need to save or compare memory addresses,
like in the watchpoint location addresses case we strip out / ignore
non-significant bits.

And the next question is: if you're adding a gdbarch hook such as
this one (either significant_addr_bit or addr_tag_remove)
why not use it for all the cases handled by the different patches in
this series, instead of using different solutions for each case?
I.e., for memory access, saving breakpoint and watchpoint
location addresses, the dcache, and any other future case we run
into, like e.g., maybe agent expressions.

Thanks,
Pedro Alves