public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] [AArch64] Fix removal of non-address bits for PAuth
@ 2022-07-05 14:00 Luis Machado
  2022-07-05 18:12 ` John Baldwin
                   ` (6 more replies)
  0 siblings, 7 replies; 37+ messages in thread
From: Luis Machado @ 2022-07-05 14:00 UTC (permalink / raw)
  To: gdb-patches

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.

Bug-url: https://sourceware.org/bugzilla/show_bug.cgi?id=28947
---
 gdb/aarch64-linux-nat.c                       |   2 +-
 gdb/aarch64-linux-tdep.c                      |  57 ++++++++-
 gdb/arch-utils.c                              |   8 ++
 gdb/arch-utils.h                              |   4 +
 gdb/arch/aarch64.c                            |  19 +++
 gdb/arch/aarch64.h                            |   9 ++
 gdb/breakpoint.c                              |   6 +-
 gdb/gdbarch-components.py                     |  20 ++-
 gdb/gdbarch-gen.h                             |  19 ++-
 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                |  40 ++++--
 16 files changed, 309 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 d58ad0143a2..3f41aae188e 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 453692df2f4..8398d0b4fc2 100644
--- a/gdb/aarch64-linux-tdep.c
+++ b/gdb/aarch64-linux-tdep.c
@@ -1586,7 +1586,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))
@@ -1612,7 +1612,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;
@@ -1651,7 +1651,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))
@@ -1708,7 +1708,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 ())
@@ -1782,7 +1782,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 ())
@@ -1803,6 +1804,49 @@ aarch64_linux_report_signal_info (struct gdbarch *gdbarch,
     }
 }
 
+/* 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
+    = (aarch64_gdbarch_tdep *) gdbarch_tdep (gdbarch);
+
+  /* By default, we assume TBI and discard the top 8 bits plus the VA range
+     select bit (55).  */
+  CORE_ADDR mask = (((CORE_ADDR) 1) << (64 - 9)) - 1;
+  mask = ~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.  */
+	  warning (_("C mask and D mask differ"));
+	  mask |= dmask > cmask? dmask : cmask;
+	}
+      else
+	mask |= cmask;
+    }
+
+  return aarch64_remove_top_bytes (pointer, mask);
+}
+
 static void
 aarch64_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
 {
@@ -1858,7 +1902,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 ff946ee3767..db77dc44c74 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..8578908049a 100644
--- a/gdb/arch/aarch64.c
+++ b/gdb/arch/aarch64.c
@@ -58,3 +58,22 @@ aarch64_create_target_description (const aarch64_features &features)
 
   return tdesc.release ();
 }
+
+/* See arch/aarch64.h.  */
+
+CORE_ADDR
+aarch64_remove_top_bytes (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)? true : false;
+
+  /* Remove the top non-address bits.  */
+  pointer &= ~mask;
+
+  /* Sign-extend if we have a kernel-space address.  */
+  if (kernel_address)
+    pointer |= mask;
+
+  return pointer;
+}
diff --git a/gdb/arch/aarch64.h b/gdb/arch/aarch64.h
index 8e3fd36726a..96d5fe4ce3c 100644
--- a/gdb/arch/aarch64.h
+++ b/gdb/arch/aarch64.h
@@ -67,6 +67,12 @@ 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_bytes (CORE_ADDR pointer, CORE_ADDR mask);
+
 /* 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 +102,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.  */
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index a3be12557f6..c7ccb6c4f0d 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -2113,7 +2113,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)
 		    {
@@ -7136,7 +7137,8 @@ adjust_breakpoint_address (struct gdbarch *gdbarch,
 	  adjusted_bpaddr = 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 fc10e8600ba..c6836b63c50 100644
--- a/gdb/gdbarch-components.py
+++ b/gdb/gdbarch-components.py
@@ -1116,15 +1116,21 @@ 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 is used mostly
+for data pointers, as opposed to code pointers.
 """,
-    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 ddcb4c55615..64930f7f239 100644
--- a/gdb/gdbarch-gen.h
+++ b/gdb/gdbarch-gen.h
@@ -609,13 +609,18 @@ 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 is used mostly
+   for data pointers, as opposed to code pointers. */
+
+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 68ef0480219..d536d8578af 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -140,7 +140,7 @@ struct gdbarch
   int frame_red_zone_size;
   gdbarch_convert_from_func_ptr_addr_ftype *convert_from_func_ptr_addr;
   gdbarch_addr_bits_remove_ftype *addr_bits_remove;
-  int significant_addr_bit;
+  gdbarch_remove_non_address_bits_ftype *remove_non_address_bits;
   gdbarch_memtag_to_string_ftype *memtag_to_string;
   gdbarch_tagged_address_p_ftype *tagged_address_p;
   gdbarch_memtag_matches_p_ftype *memtag_matches_p;
@@ -327,6 +327,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;
@@ -496,7 +497,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));
@@ -3147,21 +3148,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 18e53aa5d27..4ce1b9451a0 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 413a4f4d53b..f88d669c016 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..7faa7e0bc20 100644
--- a/gdbserver/linux-aarch64-low.cc
+++ b/gdbserver/linux-aarch64-low.cc
@@ -508,21 +508,37 @@ 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 = (((CORE_ADDR) 1) << (64 - 9)) - 1;
+  mask = ~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.  */
+	  warning (_("C mask and D mask differ"));
+	  mask |= (dmask > cmask)? dmask : cmask;
+	}
+    }
 
-  return addr;
+  return aarch64_remove_top_bytes (pointer, mask);
 }
 
 /* Implementation of linux target ops method "low_stopped_data_address".  */
@@ -549,7 +565,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));
-- 
2.25.1


^ permalink raw reply	[flat|nested] 37+ messages in thread

end of thread, other threads:[~2022-12-16 11:21 UTC | newest]

Thread overview: 37+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-05 14:00 [PATCH] [AArch64] Fix removal of non-address bits for PAuth 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
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

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).