public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Luis Machado <luis.machado@arm.com>
To: gdb-patches@sourceware.org
Cc: lsix@lancelotsix.com, thiago.bauermann@linaro.org
Subject: [PING][PATCH,v3] [aarch64] Fix removal of non-address bits for PAuth
Date: Tue, 25 Oct 2022 14:52:14 +0100	[thread overview]
Message-ID: <d08f1c3d-45d5-e80b-e215-67b6f14065d0@arm.com> (raw)
In-Reply-To: <20220823202936.1561526-1-luis.machado@arm.com>

On 8/23/22 21:29, Luis Machado via Gdb-patches wrote:
> v3:
> 
> - Updated warning messages.
> - Updated documentation.
> 
> v2:
> 
> - Renamed aarch64_remove_top_bytes to aarch64_remove_top_bits
> - Formatting issues.
> 
> --
> 
> The address_significant gdbarch setting was introduced as a way to remove
> non-address bits from pointers, and it is specified by a constant.  This
> constant represents the number of address bits in a pointer.
> 
> Right now AArch64 is the only architecture that uses it, and 56 was a
> correct option so far.
> 
> But if we are using Pointer Authentication (PAuth), we might use up to 2 bytes
> from the address space to store the required information.  We could also have
> cases where we're using both PAuth and MTE.
> 
> We could adjust the constant to 48 to cover those cases, but this doesn't
> cover the case where GDB needs to sign-extend kernel addresses after removal
> of the non-address bits.
> 
> This has worked so far because bit 55 is used to select between kernel-space
> and user-space addresses.  But trying to clear a range of bits crossing the
> bit 55 boundary requires the hook to be smarter.
> 
> The following patch renames the gdbarch hook from significant_addr_bit to
> remove_non_address_bits and passes a pointer as opposed to the number of
> bits.  The hook is now responsible for removing the required non-address bits
> and sign-extending the address if needed.
> 
> While at it, make GDB and GDBServer share some more code for aarch64 and add a
> new arch-specific testcase gdb.arch/aarch64-non-address-bits.exp.
> 
> Bug-url: https://sourceware.org/bugzilla/show_bug.cgi?id=28947
> ---
>   gdb/aarch64-linux-nat.c                       |   2 +-
>   gdb/aarch64-linux-tdep.c                      |  55 +++++++-
>   gdb/arch-utils.c                              |   8 ++
>   gdb/arch-utils.h                              |   4 +
>   gdb/arch/aarch64.c                            |  27 ++++
>   gdb/arch/aarch64.h                            |  17 +++
>   gdb/breakpoint.c                              |   6 +-
>   gdb/gdbarch-components.py                     |  22 +++-
>   gdb/gdbarch-gen.h                             |  21 ++-
>   gdb/gdbarch.c                                 |  25 ++--
>   gdb/target.c                                  |   2 +-
>   .../gdb.arch/aarch64-non-address-bits.c       |  25 ++++
>   .../gdb.arch/aarch64-non-address-bits.exp     | 121 ++++++++++++++++++
>   gdb/utils.c                                   |  22 ----
>   gdb/utils.h                                   |   3 -
>   gdbserver/linux-aarch64-low.cc                |  39 ++++--
>   16 files changed, 326 insertions(+), 73 deletions(-)
>   create mode 100644 gdb/testsuite/gdb.arch/aarch64-non-address-bits.c
>   create mode 100644 gdb/testsuite/gdb.arch/aarch64-non-address-bits.exp
> 
> diff --git a/gdb/aarch64-linux-nat.c b/gdb/aarch64-linux-nat.c
> index eda79ec6d35..06c7c6198ef 100644
> --- a/gdb/aarch64-linux-nat.c
> +++ b/gdb/aarch64-linux-nat.c
> @@ -843,7 +843,7 @@ aarch64_linux_nat_target::stopped_data_address (CORE_ADDR *addr_p)
>        kernel can potentially be tagged addresses.  */
>     struct gdbarch *gdbarch = thread_architecture (inferior_ptid);
>     const CORE_ADDR addr_trap
> -    = address_significant (gdbarch, (CORE_ADDR) siginfo.si_addr);
> +    = gdbarch_remove_non_address_bits (gdbarch, (CORE_ADDR) siginfo.si_addr);
>   
>     /* Check if the address matches any watched address.  */
>     state = aarch64_get_debug_reg_state (inferior_ptid.pid ());
> diff --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c
> index 15773c75da8..279c8d98f5d 100644
> --- a/gdb/aarch64-linux-tdep.c
> +++ b/gdb/aarch64-linux-tdep.c
> @@ -1591,7 +1591,7 @@ aarch64_linux_tagged_address_p (struct gdbarch *gdbarch, struct value *address)
>     CORE_ADDR addr = value_as_address (address);
>   
>     /* Remove the top byte for the memory range check.  */
> -  addr = address_significant (gdbarch, addr);
> +  addr = gdbarch_remove_non_address_bits (gdbarch, addr);
>   
>     /* Check if the page that contains ADDRESS is mapped with PROT_MTE.  */
>     if (!linux_address_in_memtag_page (addr))
> @@ -1617,7 +1617,7 @@ aarch64_linux_memtag_matches_p (struct gdbarch *gdbarch,
>   
>     /* Fetch the allocation tag for ADDRESS.  */
>     gdb::optional<CORE_ADDR> atag
> -    = aarch64_mte_get_atag (address_significant (gdbarch, addr));
> +    = aarch64_mte_get_atag (gdbarch_remove_non_address_bits (gdbarch, addr));
>   
>     if (!atag.has_value ())
>       return true;
> @@ -1656,7 +1656,7 @@ aarch64_linux_set_memtags (struct gdbarch *gdbarch, struct value *address,
>     else
>       {
>         /* Remove the top byte.  */
> -      addr = address_significant (gdbarch, addr);
> +      addr = gdbarch_remove_non_address_bits (gdbarch, addr);
>   
>         /* Make sure we are dealing with a tagged address to begin with.  */
>         if (!aarch64_linux_tagged_address_p (gdbarch, address))
> @@ -1713,7 +1713,7 @@ aarch64_linux_get_memtag (struct gdbarch *gdbarch, struct value *address,
>   	return nullptr;
>   
>         /* Remove the top byte.  */
> -      addr = address_significant (gdbarch, addr);
> +      addr = gdbarch_remove_non_address_bits (gdbarch, addr);
>         gdb::optional<CORE_ADDR> atag = aarch64_mte_get_atag (addr);
>   
>         if (!atag.has_value ())
> @@ -1787,7 +1787,8 @@ aarch64_linux_report_signal_info (struct gdbarch *gdbarch,
>         uiout->text ("\n");
>   
>         gdb::optional<CORE_ADDR> atag
> -	= aarch64_mte_get_atag (address_significant (gdbarch, fault_addr));
> +	= aarch64_mte_get_atag (gdbarch_remove_non_address_bits (gdbarch,
> +								 fault_addr));
>         gdb_byte ltag = aarch64_mte_get_ltag (fault_addr);
>   
>         if (!atag.has_value ())
> @@ -1961,6 +1962,47 @@ aarch64_linux_decode_memtag_section (struct gdbarch *gdbarch,
>     return tags;
>   }
>   
> +/* AArch64 implementation of the remove_non_address_bits gdbarch hook.  Remove
> +   non address bits from a pointer value.  */
> +
> +static CORE_ADDR
> +aarch64_remove_non_address_bits (struct gdbarch *gdbarch, CORE_ADDR pointer)
> +{
> +  aarch64_gdbarch_tdep *tdep = gdbarch_tdep<aarch64_gdbarch_tdep> (gdbarch);
> +
> +  /* By default, we assume TBI and discard the top 8 bits plus the VA range
> +     select bit (55).  */
> +  CORE_ADDR mask = AARCH64_TOP_BITS_MASK;
> +
> +  if (tdep->has_pauth ())
> +    {
> +      /* Fetch the PAC masks.  These masks are per-process, so we can just
> +	 fetch data from whatever thread we have at the moment.
> +
> +	 Also, we have both a code mask and a data mask.  For now they are the
> +	 same, but this may change in the future.  */
> +      struct regcache *regs = get_current_regcache ();
> +      CORE_ADDR cmask, dmask;
> +
> +      if (regs->cooked_read (tdep->pauth_reg_base, &dmask) != REG_VALID)
> +	dmask = mask;
> +
> +      if (regs->cooked_read (tdep->pauth_reg_base + 1, &cmask) != REG_VALID)
> +	cmask = mask;
> +
> +      if (dmask != cmask)
> +	{
> +	  /* Warn if the masks are different.  */
> +	  aarch64_pauth_mask_warning ();
> +	  mask |= dmask > cmask? dmask : cmask;
> +	}
> +      else
> +	mask |= cmask;
> +    }
> +
> +  return aarch64_remove_top_bits (pointer, mask);
> +}
> +
>   static void
>   aarch64_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>   {
> @@ -2016,7 +2058,8 @@ aarch64_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>     /* The top byte of a user space address known as the "tag",
>        is ignored by the kernel and can be regarded as additional
>        data associated with the address.  */
> -  set_gdbarch_significant_addr_bit (gdbarch, 56);
> +  set_gdbarch_remove_non_address_bits (gdbarch,
> +				       aarch64_remove_non_address_bits);
>   
>     /* MTE-specific settings and hooks.  */
>     if (tdep->has_mte ())
> diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c
> index 9bd4f0ddae6..12ed38ce6a9 100644
> --- a/gdb/arch-utils.c
> +++ b/gdb/arch-utils.c
> @@ -82,6 +82,14 @@ legacy_register_sim_regno (struct gdbarch *gdbarch, int regnum)
>       return LEGACY_SIM_REGNO_IGNORE;
>   }
>   
> +/* See arch-utils.h */
> +
> +CORE_ADDR
> +default_remove_non_address_bits (struct gdbarch *gdbarch, CORE_ADDR pointer)
> +{
> +  /* By default, just return the pointer value.  */
> +  return pointer;
> +}
>   
>   /* See arch-utils.h */
>   
> diff --git a/gdb/arch-utils.h b/gdb/arch-utils.h
> index f850e5fd6e7..e117599b171 100644
> --- a/gdb/arch-utils.h
> +++ b/gdb/arch-utils.h
> @@ -132,6 +132,10 @@ extern const struct floatformat **
>     default_floatformat_for_type (struct gdbarch *gdbarch,
>   				const char *name, int len);
>   
> +/* Default implementation of gdbarch_remove_non_address_bits.  */
> +CORE_ADDR default_remove_non_address_bits (struct gdbarch *gdbarch,
> +					   CORE_ADDR pointer);
> +
>   /* Default implementation of gdbarch_memtag_to_string.  */
>   extern std::string default_memtag_to_string (struct gdbarch *gdbarch,
>   					     struct value *tag);
> diff --git a/gdb/arch/aarch64.c b/gdb/arch/aarch64.c
> index 0f73286f145..d9c4b994850 100644
> --- a/gdb/arch/aarch64.c
> +++ b/gdb/arch/aarch64.c
> @@ -58,3 +58,30 @@ aarch64_create_target_description (const aarch64_features &features)
>   
>     return tdesc.release ();
>   }
> +
> +/* See arch/aarch64.h.  */
> +
> +CORE_ADDR
> +aarch64_remove_top_bits (CORE_ADDR pointer, CORE_ADDR mask)
> +{
> +  /* The VA range select bit is 55.  This bit tells us if we have a
> +     kernel-space address or a user-space address.  */
> +  bool kernel_address = (pointer & VA_RANGE_SELECT_BIT_MASK) != 0;
> +
> +  /* Remove the top non-address bits.  */
> +  pointer &= ~mask;
> +
> +  /* Sign-extend if we have a kernel-space address.  */
> +  if (kernel_address)
> +    pointer |= mask;
> +
> +  return pointer;
> +}
> +
> +/* See arch/aarch64.h.  */
> +
> +void
> +aarch64_pauth_mask_warning ()
> +{
> +  warning (_("Pointer authentication masks for code (C) and data (D) differ"));
> +}
> diff --git a/gdb/arch/aarch64.h b/gdb/arch/aarch64.h
> index 8e3fd36726a..19a3fb4d7c0 100644
> --- a/gdb/arch/aarch64.h
> +++ b/gdb/arch/aarch64.h
> @@ -67,6 +67,16 @@ namespace std
>   target_desc *
>     aarch64_create_target_description (const aarch64_features &features);
>   
> +/* Given a pointer value POINTER and a MASK of non-address bits, remove the
> +   non-address bits from the pointer and sign-extend the result if required.
> +   The sign-extension is required so we can handle kernel addresses
> +   correctly.  */
> +CORE_ADDR aarch64_remove_top_bits (CORE_ADDR pointer, CORE_ADDR mask);
> +
> +/* Produce a warning stating the D mask and the C mask have different
> +   values.  */
> +void aarch64_pauth_mask_warning ();
> +
>   /* Register numbers of various important registers.
>      Note that on SVE, the Z registers reuse the V register numbers and the V
>      registers become pseudo registers.  */
> @@ -96,6 +106,9 @@ enum aarch64_regnum
>     AARCH64_LAST_V_ARG_REGNUM = AARCH64_V0_REGNUM + 7
>   };
>   
> +/* Bit 55 is used to select between a kernel-space and user-space address.  */
> +#define VA_RANGE_SELECT_BIT_MASK 0x80000000000000
> +
>   #define V_REGISTER_SIZE 16
>   
>   /* Pseudo register base numbers.  */
> @@ -143,4 +156,8 @@ enum aarch64_regnum
>   /* Maximum supported VQ value.  Increase if required.  */
>   #define AARCH64_MAX_SVE_VQ  16
>   
> +/* Mask with 1's in bits 55~63, used to remove the top byte of pointers
> +   (Top Byte Ignore).  */
> +#define AARCH64_TOP_BITS_MASK (~((((CORE_ADDR) 1) << (64 - 9)) - 1))
> +
>   #endif /* ARCH_AARCH64_H */
> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
> index 2fe74ea0d3a..c26271017af 100644
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -2112,7 +2112,8 @@ update_watchpoint (struct watchpoint *b, int reparse)
>   		  loc->gdbarch = value_type (v)->arch ();
>   
>   		  loc->pspace = frame_pspace;
> -		  loc->address = address_significant (loc->gdbarch, addr);
> +		  loc->address
> +		    = gdbarch_remove_non_address_bits (loc->gdbarch, addr);
>   
>   		  if (bitsize != 0)
>   		    {
> @@ -7161,7 +7162,8 @@ adjust_breakpoint_address (struct gdbarch *gdbarch,
>   	    = gdbarch_adjust_breakpoint_address (gdbarch, bpaddr);
>   	}
>   
> -      adjusted_bpaddr = address_significant (gdbarch, adjusted_bpaddr);
> +      adjusted_bpaddr
> +	= gdbarch_remove_non_address_bits (gdbarch, adjusted_bpaddr);
>   
>         /* An adjusted breakpoint address can significantly alter
>   	 a user's expectations.  Print a warning if an adjustment
> diff --git a/gdb/gdbarch-components.py b/gdb/gdbarch-components.py
> index 71aa5991fbe..25b6d69d2d9 100644
> --- a/gdb/gdbarch-components.py
> +++ b/gdb/gdbarch-components.py
> @@ -1116,15 +1116,23 @@ possible it should be in TARGET_READ_PC instead).
>       invalid=False,
>   )
>   
> -Value(
> +Method(
>       comment="""
> -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.
> +On some architectures, not all bits of a pointer are significant.
> +On AArch64, for example, the top bits of a pointer may carry a "tag", which
> +can be ignored by the kernel and the hardware. The "tag" can be regarded as
> +additional data associated with the pointer, but it is not part of the address.
> +
> +Given a pointer for the architecture, this hook removes all the
> +non-significant bits and sign-extends things as needed. It gets used to remove
> +non-address bits from data pointers (for example, removing the AArch64 MTE tag
> +bits from a pointer) and from code pointers (removing the AArch64 PAC signature
> +from a pointer containing the return address).
>   """,
> -    type="int",
> -    name="significant_addr_bit",
> +    type="CORE_ADDR",
> +    name="remove_non_address_bits",
> +    params=[("CORE_ADDR", "pointer")],
> +    predefault="default_remove_non_address_bits",
>       invalid=False,
>   )
>   
> diff --git a/gdb/gdbarch-gen.h b/gdb/gdbarch-gen.h
> index 0504962e50d..99909946fb9 100644
> --- a/gdb/gdbarch-gen.h
> +++ b/gdb/gdbarch-gen.h
> @@ -609,13 +609,20 @@ typedef CORE_ADDR (gdbarch_addr_bits_remove_ftype) (struct gdbarch *gdbarch, COR
>   extern CORE_ADDR gdbarch_addr_bits_remove (struct gdbarch *gdbarch, CORE_ADDR addr);
>   extern void set_gdbarch_addr_bits_remove (struct gdbarch *gdbarch, gdbarch_addr_bits_remove_ftype *addr_bits_remove);
>   
> -/* 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. */
> -
> -extern int gdbarch_significant_addr_bit (struct gdbarch *gdbarch);
> -extern void set_gdbarch_significant_addr_bit (struct gdbarch *gdbarch, int significant_addr_bit);
> +/* On some architectures, not all bits of a pointer are significant.
> +   On AArch64, for example, the top bits of a pointer may carry a "tag", which
> +   can be ignored by the kernel and the hardware. The "tag" can be regarded as
> +   additional data associated with the pointer, but it is not part of the address.
> +
> +   Given a pointer for the architecture, this hook removes all the
> +   non-significant bits and sign-extends things as needed. It gets used to remove
> +   non-address bits from data pointers (for example, removing the AArch64 MTE tag
> +   bits from a pointer) and from code pointers (removing the AArch64 PAC signature
> +   from a pointer containing the return address). */
> +
> +typedef CORE_ADDR (gdbarch_remove_non_address_bits_ftype) (struct gdbarch *gdbarch, CORE_ADDR pointer);
> +extern CORE_ADDR gdbarch_remove_non_address_bits (struct gdbarch *gdbarch, CORE_ADDR pointer);
> +extern void set_gdbarch_remove_non_address_bits (struct gdbarch *gdbarch, gdbarch_remove_non_address_bits_ftype *remove_non_address_bits);
>   
>   /* Return a string representation of the memory tag TAG. */
>   
> diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
> index 0edae7f6f0a..b087aacd57e 100644
> --- a/gdb/gdbarch.c
> +++ b/gdb/gdbarch.c
> @@ -142,7 +142,7 @@ struct gdbarch
>     int frame_red_zone_size = 0;
>     gdbarch_convert_from_func_ptr_addr_ftype *convert_from_func_ptr_addr = nullptr;
>     gdbarch_addr_bits_remove_ftype *addr_bits_remove = nullptr;
> -  int significant_addr_bit = 0;
> +  gdbarch_remove_non_address_bits_ftype *remove_non_address_bits = nullptr;
>     gdbarch_memtag_to_string_ftype *memtag_to_string = nullptr;
>     gdbarch_tagged_address_p_ftype *tagged_address_p = nullptr;
>     gdbarch_memtag_matches_p_ftype *memtag_matches_p = nullptr;
> @@ -324,6 +324,7 @@ gdbarch_alloc (const struct gdbarch_info *info,
>     gdbarch->stabs_argument_has_addr = default_stabs_argument_has_addr;
>     gdbarch->convert_from_func_ptr_addr = convert_from_func_ptr_addr_identity;
>     gdbarch->addr_bits_remove = core_addr_identity;
> +  gdbarch->remove_non_address_bits = default_remove_non_address_bits;
>     gdbarch->memtag_to_string = default_memtag_to_string;
>     gdbarch->tagged_address_p = default_tagged_address_p;
>     gdbarch->memtag_matches_p = default_memtag_matches_p;
> @@ -493,7 +494,7 @@ verify_gdbarch (struct gdbarch *gdbarch)
>     /* Skip verify of frame_red_zone_size, invalid_p == 0 */
>     /* Skip verify of convert_from_func_ptr_addr, invalid_p == 0 */
>     /* Skip verify of addr_bits_remove, invalid_p == 0 */
> -  /* Skip verify of significant_addr_bit, invalid_p == 0 */
> +  /* Skip verify of remove_non_address_bits, invalid_p == 0 */
>     /* Skip verify of memtag_to_string, invalid_p == 0 */
>     /* Skip verify of tagged_address_p, invalid_p == 0 */
>     /* Skip verify of memtag_matches_p, invalid_p == 0 */
> @@ -974,8 +975,8 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file)
>                         "gdbarch_dump: addr_bits_remove = <%s>\n",
>                         host_address_to_string (gdbarch->addr_bits_remove));
>     gdb_printf (file,
> -                      "gdbarch_dump: significant_addr_bit = %s\n",
> -                      plongest (gdbarch->significant_addr_bit));
> +                      "gdbarch_dump: remove_non_address_bits = <%s>\n",
> +                      host_address_to_string (gdbarch->remove_non_address_bits));
>     gdb_printf (file,
>                         "gdbarch_dump: memtag_to_string = <%s>\n",
>                         host_address_to_string (gdbarch->memtag_to_string));
> @@ -3165,21 +3166,21 @@ set_gdbarch_addr_bits_remove (struct gdbarch *gdbarch,
>     gdbarch->addr_bits_remove = addr_bits_remove;
>   }
>   
> -int
> -gdbarch_significant_addr_bit (struct gdbarch *gdbarch)
> +CORE_ADDR
> +gdbarch_remove_non_address_bits (struct gdbarch *gdbarch, CORE_ADDR pointer)
>   {
>     gdb_assert (gdbarch != NULL);
> -  /* Skip verify of significant_addr_bit, invalid_p == 0 */
> +  gdb_assert (gdbarch->remove_non_address_bits != NULL);
>     if (gdbarch_debug >= 2)
> -    gdb_printf (gdb_stdlog, "gdbarch_significant_addr_bit called\n");
> -  return gdbarch->significant_addr_bit;
> +    gdb_printf (gdb_stdlog, "gdbarch_remove_non_address_bits called\n");
> +  return gdbarch->remove_non_address_bits (gdbarch, pointer);
>   }
>   
>   void
> -set_gdbarch_significant_addr_bit (struct gdbarch *gdbarch,
> -                                  int significant_addr_bit)
> +set_gdbarch_remove_non_address_bits (struct gdbarch *gdbarch,
> +                                     gdbarch_remove_non_address_bits_ftype remove_non_address_bits)
>   {
> -  gdbarch->significant_addr_bit = significant_addr_bit;
> +  gdbarch->remove_non_address_bits = remove_non_address_bits;
>   }
>   
>   std::string
> diff --git a/gdb/target.c b/gdb/target.c
> index 41144901fc1..47a2195c667 100644
> --- a/gdb/target.c
> +++ b/gdb/target.c
> @@ -1644,7 +1644,7 @@ memory_xfer_partial (struct target_ops *ops, enum target_object object,
>     if (len == 0)
>       return TARGET_XFER_EOF;
>   
> -  memaddr = address_significant (target_gdbarch (), memaddr);
> +  memaddr = gdbarch_remove_non_address_bits (target_gdbarch (), memaddr);
>   
>     /* Fill in READBUF with breakpoint shadows, or WRITEBUF with
>        breakpoint insns, thus hiding out from higher layers whether
> diff --git a/gdb/testsuite/gdb.arch/aarch64-non-address-bits.c b/gdb/testsuite/gdb.arch/aarch64-non-address-bits.c
> new file mode 100644
> index 00000000000..3cf6d63da17
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/aarch64-non-address-bits.c
> @@ -0,0 +1,25 @@
> +/* This file is part of GDB, the GNU debugger.
> +
> +   Copyright 2022 Free Software Foundation, Inc.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +static long l = 0;
> +static long *l_ptr = &l;
> +
> +int
> +main (int argc, char **argv)
> +{
> +  return *l_ptr;
> +}
> diff --git a/gdb/testsuite/gdb.arch/aarch64-non-address-bits.exp b/gdb/testsuite/gdb.arch/aarch64-non-address-bits.exp
> new file mode 100644
> index 00000000000..2b5a7b2f517
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/aarch64-non-address-bits.exp
> @@ -0,0 +1,121 @@
> +# Copyright 2022 Free Software Foundation, Inc.
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +#
> +# This file is part of the gdb testsuite.
> +#
> +# Test that GDB for AArch64/Linux can properly handle pointers with
> +# the upper 16 bits (PAC) or 8 bits (Tag) set, as well as the
> +# VA_RANGE_SELECT bit (55).
> +
> +if {![is_aarch64_target]} {
> +    verbose "Skipping ${gdb_test_file_name}."
> +    return
> +}
> +
> +global hex
> +global decimal
> +
> +standard_testfile
> +if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile}] } {
> +    return -1
> +}
> +
> +if ![runto_main] {
> +    return -1
> +}
> +
> +# We need to iterate over two distinct ranges, separated by a single bit.
> +# This bit is 55 (VA_RANGE_SELECT) which tells us if we have a kernel-space
> +# address or a user-space address.
> +
> +# The tag field has 8 bits.
> +set tag_bits_count 8
> +
> +# The pac field has 7 bits.
> +set pac_bits_count 7
> +
> +# A couple patterns that we reuse for the tests later.  One is for a successful
> +# memory read and the other is for a memory read failure.
> +set memory_read_ok_pattern "$hex\( <l>\)?:\[ \t\]+$hex"
> +set memory_read_fail_pattern "$hex:\[ \t\]+Cannot access memory at address $hex"
> +
> +set pac_enabled 0
> +
> +# Check if PAC is enabled.
> +gdb_test_multiple "ptype \$pauth_cmask" "fetch PAC cmask" {
> +    -re "type = long\r\n$gdb_prompt" {
> +	set pac_enabled 1
> +    }
> +    -re "type = void\r\n$gdb_prompt" {
> +    }
> +    -re ".*$gdb_prompt $" {
> +	fail $gdb_test_name
> +	return 1
> +    }
> +}
> +
> +# Value of the cmask register.
> +set cmask 0
> +
> +# If there are PAC registers, GDB uses those to unmask the PAC bits.
> +if {$pac_enabled} {
> +    set cmask [get_valueof "" "\$pauth_cmask >> 48" "0" "fetch PAC cmask"]
> +}
> +
> +# Cycle through the tag and pac bit ranges and check how GDB
> +# behaves when trying to access these addresses.
> +foreach upper_bits {"0x0" "0x1" "0x2" "0x4" "0x8" "0x10" "0x20" "0x40" "0x80"} {
> +    foreach lower_bits {"0x0" "0x1" "0x2" "0x4" "0x8" "0x10" "0x20" "0x40"} {
> +
> +	# A successful memory read pattern
> +	set pattern $memory_read_ok_pattern
> +
> +	if {!$pac_enabled} {
> +	    # If PAC is not supported, memory reads will fail if
> +	    # lower_bits != 0x0
> +	    if {$lower_bits != "0x0"} {
> +		set pattern $memory_read_fail_pattern
> +	    }
> +	} else {
> +	    # Otherwise, figure out if the memory read will succeed or not by
> +	    # checking cmask.
> +	    gdb_test_multiple "p/x (~${cmask}ULL & (${lower_bits}ULL))" "" {
> +		-re "= 0x0\r\n$gdb_prompt" {
> +		    # Either cmask is 0x7F or lower_bits is 0x0. Either way, the
> +		    # memory read should succeed.
> +		}
> +		-re "= $hex\r\n$gdb_prompt" {
> +		    if {$lower_bits != "0x0"} {
> +			# cmask doesn't mask off all the PAC bits, which
> +			# results in a memory read failure, with the actual
> +			# address being accessed differing from the one we
> +			# passed.
> +			set pattern $memory_read_fail_pattern
> +		    }
> +		}
> +	    }
> +	}
> +
> +	# Test without the VA_RANGE_SELECT bit set.
> +	gdb_test "x/gx ((unsigned long) l_ptr | ((${upper_bits}ULL << 56) | (${lower_bits}ULL << 48)))" \
> +	    $pattern \
> +	    "user-space memory access tag bits $upper_bits and pac bits $lower_bits"
> +
> +	# Now test with the VA_RANGE_SELECT bit set.
> +	gdb_test "x/gx ((unsigned long) l_ptr | ((${upper_bits}ULL << 56) | (${lower_bits}ULL << 48) | (1ULL << 55))) " \
> +	    $memory_read_fail_pattern \
> +	    "kernel-space memory access tag bits $upper_bits and pac bits $lower_bits"
> +    }
> +}
> diff --git a/gdb/utils.c b/gdb/utils.c
> index b0841e1fe5e..cf3db813777 100644
> --- a/gdb/utils.c
> +++ b/gdb/utils.c
> @@ -3139,28 +3139,6 @@ show_debug_timestamp (struct ui_file *file, int from_tty,
>   }
>   \f
>   
> -/* See utils.h.  */
> -
> -CORE_ADDR
> -address_significant (gdbarch *gdbarch, CORE_ADDR addr)
> -{
> -  /* Clear insignificant bits of a target address and sign extend resulting
> -     address, avoiding shifts larger or equal than the width of a CORE_ADDR.
> -     The local variable ADDR_BIT stops the compiler reporting a shift overflow
> -     when it won't occur.  Skip updating of target address if current target
> -     has not set gdbarch significant_addr_bit.  */
> -  int addr_bit = gdbarch_significant_addr_bit (gdbarch);
> -
> -  if (addr_bit && (addr_bit < (sizeof (CORE_ADDR) * HOST_CHAR_BIT)))
> -    {
> -      CORE_ADDR sign = (CORE_ADDR) 1 << (addr_bit - 1);
> -      addr &= ((CORE_ADDR) 1 << addr_bit) - 1;
> -      addr = (addr ^ sign) - sign;
> -    }
> -
> -  return addr;
> -}
> -
>   const char *
>   paddress (struct gdbarch *gdbarch, CORE_ADDR addr)
>   {
> diff --git a/gdb/utils.h b/gdb/utils.h
> index d2acf899ba2..237ef0a5d99 100644
> --- a/gdb/utils.h
> +++ b/gdb/utils.h
> @@ -278,9 +278,6 @@ extern void fputs_styled (const char *linebuffer,
>   extern void fputs_highlighted (const char *str, const compiled_regex &highlight,
>   			       struct ui_file *stream);
>   
> -/* Return the address only having significant bits.  */
> -extern CORE_ADDR address_significant (gdbarch *gdbarch, CORE_ADDR addr);
> -
>   /* Convert CORE_ADDR to string in platform-specific manner.
>      This is usually formatted similar to 0x%lx.  */
>   extern const char *paddress (struct gdbarch *gdbarch, CORE_ADDR addr);
> diff --git a/gdbserver/linux-aarch64-low.cc b/gdbserver/linux-aarch64-low.cc
> index db508696261..155706f1138 100644
> --- a/gdbserver/linux-aarch64-low.cc
> +++ b/gdbserver/linux-aarch64-low.cc
> @@ -508,21 +508,36 @@ aarch64_target::low_remove_point (raw_bkpt_type type, CORE_ADDR addr,
>     return ret;
>   }
>   
> -/* Return the address only having significant bits.  This is used to ignore
> -   the top byte (TBI).  */
> -
>   static CORE_ADDR
> -address_significant (CORE_ADDR addr)
> +aarch64_remove_non_address_bits (CORE_ADDR pointer)
>   {
> -  /* Clear insignificant bits of a target address and sign extend resulting
> -     address.  */
> -  int addr_bit = 56;
> +  /* By default, we assume TBI and discard the top 8 bits plus the
> +     VA range select bit (55).  */
> +  CORE_ADDR mask = AARCH64_TOP_BITS_MASK;
> +
> +  /* Check if PAC is available for this target.  */
> +  if (tdesc_contains_feature (current_process ()->tdesc,
> +			      "org.gnu.gdb.aarch64.pauth"))
> +    {
> +      /* Fetch the PAC masks.  These masks are per-process, so we can just
> +	 fetch data from whatever thread we have at the moment.
> +
> +	 Also, we have both a code mask and a data mask.  For now they are the
> +	 same, but this may change in the future.  */
>   
> -  CORE_ADDR sign = (CORE_ADDR) 1 << (addr_bit - 1);
> -  addr &= ((CORE_ADDR) 1 << addr_bit) - 1;
> -  addr = (addr ^ sign) - sign;
> +      struct regcache *regs = get_thread_regcache (current_thread, 1);
> +      CORE_ADDR dmask = regcache_raw_get_unsigned_by_name (regs, "pauth_dmask");
> +      CORE_ADDR cmask = regcache_raw_get_unsigned_by_name (regs, "pauth_cmask");
> +
> +      if (dmask != cmask && dmask != 0 && cmask != 0)
> +	{
> +	  /* Warn if the masks are different.  */
> +	  aarch64_pauth_mask_warning ();
> +	  mask |= (dmask > cmask)? dmask : cmask;
> +	}
> +    }
>   
> -  return addr;
> +  return aarch64_remove_top_bits (pointer, mask);
>   }
>   
>   /* Implementation of linux target ops method "low_stopped_data_address".  */
> @@ -549,7 +564,7 @@ aarch64_target::low_stopped_data_address ()
>        hardware watchpoint hit.  The stopped data addresses coming from the
>        kernel can potentially be tagged addresses.  */
>     const CORE_ADDR addr_trap
> -    = address_significant ((CORE_ADDR) siginfo.si_addr);
> +    = aarch64_remove_non_address_bits ((CORE_ADDR) siginfo.si_addr);
>   
>     /* Check if the address matches any watched address.  */
>     state = aarch64_get_debug_reg_state (pid_of (current_thread));


  parent reply	other threads:[~2022-10-25 13:52 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-05 14:00 [PATCH] [AArch64] " Luis Machado
2022-07-05 18:12 ` John Baldwin
2022-07-06 11:38 ` Lancelot SIX
2022-07-08 11:36   ` Luis Machado
2022-07-11 11:55 ` [PATCH,v2] [aarch64] " Luis Machado
2022-07-18  8:16   ` [Ping v1][PATCH,v2] " Luis Machado
2022-08-01 11:09     ` [Ping v2][PATCH,v2] " Luis Machado
2022-08-08 11:34   ` [Ping v3][PATCH,v2] " Luis Machado
2022-08-18 15:49   ` [Ping v4][PATCH,v2] " Luis Machado
2022-08-18 23:47   ` [PATCH,v2] " Thiago Jung Bauermann
2022-08-19  9:52     ` Luis Machado
2022-08-19 14:06       ` Thiago Jung Bauermann
2022-08-23 20:29 ` [PATCH,v3] " Luis Machado
2022-08-24 18:44   ` Thiago Jung Bauermann
2022-09-01  9:29   ` [PING][PATCH,v3] " Luis Machado
2022-09-07  8:21   ` Luis Machado
2022-09-12 12:47   ` Luis Machado
2022-09-20 12:26   ` Luis Machado
2022-09-22 12:59   ` [PATCH,v3] " Lancelot SIX
2022-09-22 16:39     ` Luis Machado
2022-09-23  7:58       ` Lancelot SIX
2022-10-03 11:37   ` [PING][PATCH,v3] " Luis Machado
2022-10-10 12:18   ` Luis Machado
2022-10-17 10:04   ` Luis Machado
2022-10-25 13:52   ` Luis Machado [this message]
2022-11-10  1:00   ` Luis Machado
2022-11-29 22:19   ` Luis Machado
2022-12-09 16:42   ` Luis Machado
2022-12-09 19:14   ` [PATCH,v3] " Simon Marchi
2022-12-12 14:21     ` Luis Machado
2022-12-12 15:07       ` Simon Marchi
2022-12-12 17:13 ` [PATCH v4] " Luis Machado
2022-12-12 18:54   ` Simon Marchi
2022-12-13  9:18     ` Luis Machado
2022-12-13 10:27 ` [PATCH v5] " Luis Machado
2022-12-16 10:57 ` [PATCH v6] " Luis Machado
2022-12-16 11:20   ` Luis Machado

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=d08f1c3d-45d5-e80b-e215-67b6f14065d0@arm.com \
    --to=luis.machado@arm.com \
    --cc=gdb-patches@sourceware.org \
    --cc=lsix@lancelotsix.com \
    --cc=thiago.bauermann@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).