Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: "Luis Machado (Code Review)" <gerrit@gnutoolchain-gerrit.osci.io>
To: gdb-patches@sourceware.org
Cc: Simon Marchi <simon.marchi@polymtl.ca>
Subject: [review v4] [AArch64, SVE] Improve target description check for SVE in gdbserver
Date: Tue, 19 Nov 2019 15:14:00 -0000	[thread overview]
Message-ID: <20191119151417.76C002816F@gnutoolchain-gerrit.osci.io> (raw)
In-Reply-To: <gerrit.1574172175000.I28b782cb1677560ca9a06a1be442974b25aabae4@gnutoolchain-gerrit.osci.io>

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/690
......................................................................

[AArch64, SVE] Improve target description check for SVE in gdbserver

The current code checks for the presence of a SVE target description by
comparing the number of registers.  This is a bit fragile since the number
of registers can change whenever we add new sets. Like PAC, for example.

If the comparison breaks, then we're left with SVE registers in the
description, but gdbserver doesn't send the registers to GDB, which in
turn displays stale information to the user.

The following patch changes the check to use the SVE feature string instead,
which hopefully should be more stable.

gdb/gdbserver/ChangeLog:

2019-11-18  Luis Machado  <luis.machado@linaro.org>

	* linux-aarch64-low.c (is_sve_tdesc): Check against target feature
	instead of register count.
	* tdesc.c (tdesc_contains_feature): New function.
	* tdesc.h (tdesc_contains_feature): New prototype.

Signed-off-by: Luis Machado <luis.machado@linaro.org>
Change-Id: I28b782cb1677560ca9a06a1be442974b25aabae4
---
M gdb/gdbserver/linux-aarch64-low.c
M gdb/gdbserver/tdesc.c
M gdb/gdbserver/tdesc.h
3 files changed, 22 insertions(+), 1 deletion(-)



diff --git a/gdb/gdbserver/linux-aarch64-low.c b/gdb/gdbserver/linux-aarch64-low.c
index 87a21a0..9e234e0 100644
--- a/gdb/gdbserver/linux-aarch64-low.c
+++ b/gdb/gdbserver/linux-aarch64-low.c
@@ -83,7 +83,7 @@
 {
   struct regcache *regcache = get_thread_regcache (current_thread, 0);
 
-  return regcache->tdesc->reg_defs.size () == AARCH64_SVE_NUM_REGS;
+  return tdesc_contains_feature (regcache->tdesc, "org.gnu.gdb.aarch64.sve");
 }
 
 static void
diff --git a/gdb/gdbserver/tdesc.c b/gdb/gdbserver/tdesc.c
index 92a0a60..817e69a 100644
--- a/gdb/gdbserver/tdesc.c
+++ b/gdb/gdbserver/tdesc.c
@@ -186,3 +186,19 @@
   tdesc->features.emplace_back (new_feature);
   return new_feature;
 }
+
+/* See gdbsupport/tdesc.h.  */
+
+bool
+tdesc_contains_feature (const target_desc *tdesc, const std::string &feature)
+{
+  gdb_assert (tdesc != nullptr);
+
+  for (const tdesc_feature_up &f : tdesc->features)
+    {
+      if (f->name == feature)
+	return true;
+    }
+
+  return false;
+}
diff --git a/gdb/gdbserver/tdesc.h b/gdb/gdbserver/tdesc.h
index b93f53c..da21cda 100644
--- a/gdb/gdbserver/tdesc.h
+++ b/gdb/gdbserver/tdesc.h
@@ -93,4 +93,9 @@
 
 const struct target_desc *current_target_desc (void);
 
+/* Return true if TDESC contains the feature described by string FEATURE.
+   Return false otherwise.  */
+bool tdesc_contains_feature (const target_desc *tdesc,
+			     const std::string &feature);
+
 #endif /* GDBSERVER_TDESC_H */

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I28b782cb1677560ca9a06a1be442974b25aabae4
Gerrit-Change-Number: 690
Gerrit-PatchSet: 4
Gerrit-Owner: Luis Machado <luis.machado@linaro.org>
Gerrit-Reviewer: Luis Machado <luis.machado@linaro.org>
Gerrit-CC: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-MessageType: newpatchset


  parent reply	other threads:[~2019-11-19 15:14 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-19 14:03 [review] " Luis Machado (Code Review)
2019-11-19 14:05 ` [review v2] " Luis Machado (Code Review)
2019-11-19 14:23 ` Simon Marchi (Code Review)
2019-11-19 14:30 ` Luis Machado (Code Review)
2019-11-19 14:43 ` [review v3] " Luis Machado (Code Review)
2019-11-19 15:03 ` Simon Marchi (Code Review)
2019-11-19 15:14 ` Luis Machado (Code Review) [this message]
2019-11-20 16:34   ` [review v4] " Alan Hayward
2019-11-20 16:51     ` Luis Machado
     [not found]     ` <c30acac0-f73f-c368-d980-94ece81e92e5@polymtl.ca>
2019-11-21  8:23       ` Alan Hayward
2019-11-20 16:59 ` [pushed] " Sourceware to Gerrit sync (Code Review)
2019-11-20 16:59 ` Sourceware to Gerrit sync (Code Review)

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=20191119151417.76C002816F@gnutoolchain-gerrit.osci.io \
    --to=gerrit@gnutoolchain-gerrit.osci.io \
    --cc=gdb-patches@sourceware.org \
    --cc=luis.machado@linaro.org \
    --cc=simon.marchi@polymtl.ca \
    /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