gdb/riscv: Update API for looking up target descriptions

In preparation for adding the RISC-V gdbserver, this commit
restructures the API for looking up target descriptions.

The current API is riscv_create_target_description, which creates a
target description from a riscv_gdbarch_features, but also caches the
created target descriptions so that for a given features object we
always get back the same target description object.  This is important
for GDB due to the way gdbarch objects are reused.

As the same target description is always returned to GDB, and can be
returned multiple times, it is returned as a const, however, the
current cache actually stores a non-const target description.  This is
improved in this patch so that the cache holds a const target
description.

For gdbsever, this caching of the target descriptions is not needed,
the gdbserver looks up one target description to describe the target
it is actually running on and that is it.  Further the gdbserver
actually needs to modify the target description that is looked up, so
for the gdbsever, returning a const target description is not
acceptable.

This commit aims to address this by creating two parallel target
description APIs, on is the old riscv_create_target_description,
however, this no longer performs any caching, and just creates a new
target description, and returns it as non-const.

The second API is riscv_lookup_target_description, this one performs
the caching, and calls riscv_create_target_description to create a
target description when needed.

In order to make sure the correct API is used in the correct place I
have guarded the code using the GDBSERVER define.  For GDB the
riscv_create_target_description is static, and not generally usable
throughout GDB, only the lookup API is global.  In gdbserver, the
lookup functions, and the cache are not defined or created at all,
only the riscv_create_target_description API is available.

There should be no user visible changes after this commit.

gdb/ChangeLog:

	* arch/riscv.c (struct riscv_gdbarch_features_hasher): Only define
	if GDBSERVER is not defined.
	(riscv_tdesc_cache): Likewise, also store const target_desc.
	(STATIC_IN_GDB): Define.
	(riscv_create_target_description): Update declaration with
	STATIC_IN_GDB.
	(riscv_lookup_target_description): New function, only define if
	GDBSERVER is not defined.
	* arch/riscv.h (riscv_create_target_description): Declare only
	when GDBSERVER is defined.
	(riscv_lookup_target_description): New declaration when GDBSERVER
	is not defined.
	* nat/riscv-linux-tdesc.c (riscv_linux_read_description): Rename to...
	(riscv_linux_read_features): ...this, and return
	riscv_gdbarch_features instead of target_desc.
	* nat/riscv-linux-tdesc.h: Include 'arch/riscv.h'.
	(riscv_linux_read_description): Rename to...
	(riscv_linux_read_features): ...this.
	* riscv-linux-nat.c (riscv_linux_nat_target::read_description):
	Update to use riscv_gdbarch_features and
	riscv_lookup_target_description.
	* riscv-tdep.c (riscv_find_default_target_description): Use
	riscv_lookup_target_description instead of
	riscv_create_target_description.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 9d295682e8deb0c22df34bf5722758708b224261..144fd4f256af52ee8fc0dbb60f6ace3601738023 100644 (file)
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,30 @@
+2020-02-19  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+       * arch/riscv.c (struct riscv_gdbarch_features_hasher): Only define
+       if GDBSERVER is not defined.
+       (riscv_tdesc_cache): Likewise, also store const target_desc.
+       (STATIC_IN_GDB): Define.
+       (riscv_create_target_description): Update declaration with
+       STATIC_IN_GDB.
+       (riscv_lookup_target_description): New function, only define if
+       GDBSERVER is not defined.
+       * arch/riscv.h (riscv_create_target_description): Declare only
+       when GDBSERVER is defined.
+       (riscv_lookup_target_description): New declaration when GDBSERVER
+       is not defined.
+       * nat/riscv-linux-tdesc.c (riscv_linux_read_description): Rename to...
+       (riscv_linux_read_features): ...this, and return
+       riscv_gdbarch_features instead of target_desc.
+       * nat/riscv-linux-tdesc.h: Include 'arch/riscv.h'.
+       (riscv_linux_read_description): Rename to...
+       (riscv_linux_read_features): ...this.
+       * riscv-linux-nat.c (riscv_linux_nat_target::read_description):
+       Update to use riscv_gdbarch_features and
+       riscv_lookup_target_description.
+       * riscv-tdep.c (riscv_find_default_target_description): Use
+       riscv_lookup_target_description instead of
+       riscv_create_target_description.
+
 2020-02-18  Simon Marchi  <simon.marchi@efficios.com>
 
        * valprint.c (generic_val_print_enum_1): When printing a flag

diff --git a/gdb/arch/riscv.c b/gdb/arch/riscv.c
index a3ab8a92909f082754dfd6eb9a70f19153b8684f..a02c18b4c916907f7205ee693c4838e4f21776c6 100644 (file)
--- a/gdb/arch/riscv.c
+++ b/gdb/arch/riscv.c
@@ -25,37 +25,17 @@
 #include "../features/riscv/32bit-fpu.c"
 #include "../features/riscv/64bit-fpu.c"
 
-/* Wrapper used by std::unordered_map to generate hash for feature set.  */
-struct riscv_gdbarch_features_hasher
-{
-  std::size_t
-  operator() (const riscv_gdbarch_features &features) const noexcept
-  {
-    return features.hash ();
-  }
-};
-
-/* Cache of previously seen target descriptions, indexed by the feature set
-   that created them.  */
-static std::unordered_map<riscv_gdbarch_features,
-                          target_desc *,
-                          riscv_gdbarch_features_hasher> riscv_tdesc_cache;
+#ifndef GDBSERVER
+#define STATIC_IN_GDB static
+#else
+#define STATIC_IN_GDB
+#endif
 
 /* See arch/riscv.h.  */
 
-const target_desc *
-riscv_create_target_description (struct riscv_gdbarch_features features)
+STATIC_IN_GDB target_desc *
+riscv_create_target_description (const struct riscv_gdbarch_features features)
 {
-  /* Have we seen this feature set before?  If we have return the same
-     target description.  GDB expects that if two target descriptions are
-     the same (in content terms) then they will actually be the same
-     instance.  This is important when trying to lookup gdbarch objects as
-     GDBARCH_LIST_LOOKUP_BY_INFO performs a pointer comparison on target
-     descriptions to find candidate gdbarch objects.  */
-  const auto it = riscv_tdesc_cache.find (features);
-  if (it != riscv_tdesc_cache.end ())
-    return it->second;
-
   /* Now we should create a new target description.  */
   target_desc *tdesc = allocate_target_description ();
 
@@ -93,8 +73,43 @@ riscv_create_target_description (struct riscv_gdbarch_features features)
   else if (features.flen == 8)
     regnum = create_feature_riscv_64bit_fpu (tdesc, regnum);
 
+  return tdesc;
+}
+
+#ifndef GDBSERVER
+
+/* Wrapper used by std::unordered_map to generate hash for feature set.  */
+struct riscv_gdbarch_features_hasher
+{
+  std::size_t
+  operator() (const riscv_gdbarch_features &features) const noexcept
+  {
+    return features.hash ();
+  }
+};
+
+/* Cache of previously seen target descriptions, indexed by the feature set
+   that created them.  */
+static std::unordered_map<riscv_gdbarch_features,
+                         const target_desc *,
+                         riscv_gdbarch_features_hasher> riscv_tdesc_cache;
+
+/* See arch/riscv.h.  */
+
+const target_desc *
+riscv_lookup_target_description (const struct riscv_gdbarch_features features)
+{
+  /* Lookup in the cache.  */
+  const auto it = riscv_tdesc_cache.find (features);
+  if (it != riscv_tdesc_cache.end ())
+    return it->second;
+
+  target_desc *tdesc = riscv_create_target_description (features);
+
   /* Add to the cache.  */
   riscv_tdesc_cache.emplace (features, tdesc);
 
   return tdesc;
 }
+
+#endif /* !GDBSERVER */

diff --git a/gdb/arch/riscv.h b/gdb/arch/riscv.h
index 016a5fcd2f0ee31a151e0b3e37e8fe4676608fa2..d40862cbd8e2a18e7aa4f684b3c9769c83b3ed9d 100644 (file)
--- a/gdb/arch/riscv.h
+++ b/gdb/arch/riscv.h
@@ -66,10 +66,28 @@ struct riscv_gdbarch_features
   }
 };
 
-/* Create and return a target description that is compatible with
-   FEATURES.  */
+#ifdef GDBSERVER
+
+/* Create and return a target description that is compatible with FEATURES.
+   This is only used directly from the gdbserver where the created target
+   description is modified after it is return.  */
+
+target_desc *riscv_create_target_description
+       (const struct riscv_gdbarch_features features);
+
+#else
+
+/* Lookup an already existing target description matching FEATURES, or
+   create a new target description if this is the first time we have seen
+   FEATURES.  For the same FEATURES the same target_desc is always
+   returned.  This is important when trying to lookup gdbarch objects as
+   GDBARCH_LIST_LOOKUP_BY_INFO performs a pointer comparison on target
+   descriptions to find candidate gdbarch objects.  */
+
+const target_desc *riscv_lookup_target_description
+       (const struct riscv_gdbarch_features features);
+
+#endif /* GDBSERVER */
 
-const target_desc *riscv_create_target_description
-       (struct riscv_gdbarch_features features);
 
 #endif /* ARCH_RISCV_H */

diff --git a/gdb/nat/riscv-linux-tdesc.c b/gdb/nat/riscv-linux-tdesc.c
index 1b625cf38fc467032290b9462fb23225a0e73a23..3220725b87590bf7a3488158fb71b8419631a6c6 100644 (file)
--- a/gdb/nat/riscv-linux-tdesc.c
+++ b/gdb/nat/riscv-linux-tdesc.c
@@ -31,10 +31,10 @@
 # define NFPREG 33
 #endif
 
-/* Determine XLEN and FLEN and return a corresponding target description.  */
+/* See nat/riscv-linux-tdesc.h.  */
 
-const struct target_desc *
-riscv_linux_read_description (int tid)
+struct riscv_gdbarch_features
+riscv_linux_read_features (int tid)
 {
   struct riscv_gdbarch_features features;
   elf_fpregset_t regs;
@@ -79,5 +79,5 @@ riscv_linux_read_description (int tid)
       break;
     }
 
-  return riscv_create_target_description (features);
+  return features;
 }

diff --git a/gdb/nat/riscv-linux-tdesc.h b/gdb/nat/riscv-linux-tdesc.h
index 9b57a9e99d5f2311f46c8ba1a685a6a5c77f184d..e9ee64aa5d27fe42cf0423a0bc4016b855efa61c 100644 (file)
--- a/gdb/nat/riscv-linux-tdesc.h
+++ b/gdb/nat/riscv-linux-tdesc.h
@@ -19,9 +19,10 @@
 #ifndef NAT_RISCV_LINUX_TDESC_H
 #define NAT_RISCV_LINUX_TDESC_H
 
-struct target_desc;
+#include "arch/riscv.h"
 
-/* Return a target description for the LWP identified by TID.  */
-const struct target_desc *riscv_linux_read_description (int tid);
+/* Determine XLEN and FLEN for the LWP identified by TID, and return a
+   corresponding features object.  */
+struct riscv_gdbarch_features riscv_linux_read_features (int tid);
 
 #endif /* NAT_RISCV_LINUX_TDESC_H */

diff --git a/gdb/riscv-linux-nat.c b/gdb/riscv-linux-nat.c
index 2622f1b4399576c54b88da2ad79b693f662d5569..4f78dfdc6a20f447863b1821a598c0434afaa5e7 100644 (file)
--- a/gdb/riscv-linux-nat.c
+++ b/gdb/riscv-linux-nat.c
@@ -201,7 +201,9 @@ fill_fpregset (const struct regcache *regcache, prfpregset_t *fpregs,
 const struct target_desc *
 riscv_linux_nat_target::read_description ()
 {
-  return riscv_linux_read_description (inferior_ptid.lwp ());
+  const struct riscv_gdbarch_features features
+    = riscv_linux_read_features (inferior_ptid.lwp ());
+  return riscv_lookup_target_description (features);
 }
 
 /* Fetch REGNUM (or all registers if REGNUM == -1) from the target

diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
index a14ef4db0830465e773b8e41a61283d6b9d57ee4..0515729f4e92046f8755a9d07d770719677be76e 100644 (file)
--- a/gdb/riscv-tdep.c
+++ b/gdb/riscv-tdep.c
@@ -2973,7 +2973,7 @@ riscv_find_default_target_description (const struct gdbarch_info info)
     features.xlen = 8;
 
   /* Now build a target description based on the feature set.  */
-  return riscv_create_target_description (features);
+  return riscv_lookup_target_description (features);
 }
 
 /* All of the registers in REG_SET are checked for in FEATURE, TDESC_DATA