public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] [gdb/aarch64] pauth: Create new feature string for pauth to prevent crashing older gdb's
@ 2023-04-11 22:43 Luis Machado
  2023-04-12  6:05 ` Eli Zaretskii
  2023-04-12 12:27 ` [PATCH,v2] " Luis Machado
  0 siblings, 2 replies; 16+ messages in thread
From: Luis Machado @ 2023-04-11 22:43 UTC (permalink / raw)
  To: gdb-patches; +Cc: alex.bennee, richard.henderson, peter.maydell

Older gdb's (9, 10, 11 and 12) have a bug that causes them to crash whenever
a target reports the pauth feature string in the target description and also
provide additional register outside of gdb's known and expected feature
strings.

This was fixed in gdb 13 onwards, but that means we're stuck with gdb's out
there that will crash on connection to the above targets.

QEMU has postponed inclusion of the pauth feature string in version 8, and
instead we agreed to use a new feature name to prevent crashing those older
gdb's.

Initially there was a plan to backport a trivial fix all the way to gdb 9, but
given QEMU's choice, this is no longer needed.

This new feature string is org.gnu.gdb.aarch64.pauth_v2, and should be used
by all targets going forward, except native linux gdb and gdbserver, for
backwards compatibility with older gdb's/gdbserver's.

gdb/gdbserver will still emit the old feature string for Linux since it doesn't
report additional system registers and thus doesn't cause a crash of older
gdb's. We can revisit this in the future once the problematic gdb's are likely
no longer in use.

I've added some documentation to explain the situation.
---
 gdb/aarch64-tdep.c  | 15 ++++++++++++++-
 gdb/doc/gdb.texinfo | 16 ++++++++++++++++
 2 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index d8349e4ccdb..ec0e51bdaf7 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -3500,8 +3500,15 @@ aarch64_features_from_target_desc (const struct target_desc *tdesc)
     return features;
 
   features.vq = aarch64_get_tdesc_vq (tdesc);
+
+  /* We need to look for a couple pauth feature name variations.  */
   features.pauth
       = (tdesc_find_feature (tdesc, "org.gnu.gdb.aarch64.pauth") != nullptr);
+
+  if (!features.pauth)
+    features.pauth = (tdesc_find_feature (tdesc, "org.gnu.gdb.aarch64.pauth_v2")
+		      != nullptr);
+
   features.mte
       = (tdesc_find_feature (tdesc, "org.gnu.gdb.aarch64.mte") != nullptr);
 
@@ -3679,7 +3686,6 @@ aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   feature_core = tdesc_find_feature (tdesc,"org.gnu.gdb.aarch64.core");
   feature_fpu = tdesc_find_feature (tdesc, "org.gnu.gdb.aarch64.fpu");
   feature_sve = tdesc_find_feature (tdesc, "org.gnu.gdb.aarch64.sve");
-  feature_pauth = tdesc_find_feature (tdesc, "org.gnu.gdb.aarch64.pauth");
   const struct tdesc_feature *feature_mte
     = tdesc_find_feature (tdesc, "org.gnu.gdb.aarch64.mte");
   const struct tdesc_feature *feature_tls
@@ -3773,6 +3779,13 @@ aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
 	}
     }
 
+  /* We have two versions of the pauth target description due to a past bug
+     where GDB would crash when seeing the first version of the pauth target
+     description.  */
+  feature_pauth = tdesc_find_feature (tdesc, "org.gnu.gdb.aarch64.pauth");
+  if (feature_pauth == nullptr)
+    feature_pauth = tdesc_find_feature (tdesc, "org.gnu.gdb.aarch64.pauth_v2");
+
   /* Add the pauth registers.  */
   int pauth_masks = 0;
   if (feature_pauth != NULL)
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 45a0580bc29..aefeb63f75c 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -47980,6 +47980,22 @@ has a signed link register value that needs to be unmasked/decoded.
 Extra registers are allowed in this feature, but they will not affect
 @value{GDBN}.
 
+Due to a bug in previous versions of @value{GDBN} (versions 9, 10, 11 and 12),
+a new feature string was created to prevent targets causing a @value{GDBN}
+crash whenever they reported support for Pointer Authentication and also
+reported additional system registers that were not accounted for by
+@value{GDBN}.  This is common when using emulators and on bare-metal debugging
+scenarios.
+
+The new feature string is @samp{org.gnu.gdb.aarch64.pauth_v2}, and it has
+the same contents as feature string @samp{org.gnu.gdb.aarch64.pauth}.
+
+Targets reporting Pointer Authentication support via the remote protocol
+should always use the newer feature string @samp{org.gnu.gdb.aarch64.pauth_v2}.
+
+In the future the @samp{org.gnu.gdb.aarch64.pauth} feature string might be
+deprecated in favor of @samp{org.gnu.gdb.aarch64.pauth_v2}.
+
 @subsubsection AArch64 TLS registers feature
 
 The @samp{org.gnu.gdb.aarch64.tls} optional feature was introduced to expose
-- 
2.25.1


^ permalink raw reply	[flat|nested] 16+ messages in thread
end of thread, other threads:[~2023-04-14 12:45 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-11 22:43 [PATCH] [gdb/aarch64] pauth: Create new feature string for pauth to prevent crashing older gdb's Luis Machado
2023-04-12  6:05 ` Eli Zaretskii
2023-04-12  8:46   ` Luis Machado
2023-04-12  9:20     ` Eli Zaretskii
2023-04-12  9:25       ` Luis Machado
2023-04-12  9:54         ` Eli Zaretskii
2023-04-12 11:48         ` Peter Maydell
2023-04-12 11:57           ` Luis Machado
2023-04-13 13:55         ` Tom Tromey
2023-04-12 12:27 ` [PATCH,v2] " Luis Machado
2023-04-12 12:45   ` Eli Zaretskii
2023-04-13 13:56   ` [PATCH, v2] " Tom Tromey
2023-04-13 14:05     ` Luis Machado
2023-04-13 14:37   ` [PATCH, v3] " Luis Machado
2023-04-13 17:17     ` Tom Tromey
2023-04-14 12:45       ` 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).