diff mbox series

[2/3] Make bcache more type-safe

Message ID 20240320-bcache-type-v1-2-fe616105e9ae@adacore.com
State New
Headers
Series Better type safety for bcache |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Testing passed

Commit Message

Tom Tromey March 20, 2024, 8:23 p.m. UTC 
  The bcache uses memcpy to make copies of the data passed to it.  In
C++, this is only safe for trivially-copyable types.

This patch changes bcache to require this property, and slightly
changes the API to make it easier to use when copying a single object.
It also makes the new 'insert' template methods return the correct
type.
---
 gdb/bcache.h   | 25 ++++++++++++++++++++++++-
 gdb/gdbtypes.c |  2 +-
 gdb/macrotab.c | 12 ++++++------
 gdb/psymtab.c  |  6 ++----
 4 files changed, 33 insertions(+), 12 deletions(-)

Comments

Alexandra Petlanova Hajkova March 27, 2024, 2:15 p.m. UTC | #1 
On Wed, Mar 20, 2024 at 9:24 PM Tom Tromey <tromey@adacore.com> wrote:

> The bcache uses memcpy to make copies of the data passed to it.  In
> C++, this is only safe for trivially-copyable types.
>
> This patch changes bcache to require this property, and slightly
> changes the API to make it easier to use when copying a single object.
> It also makes the new 'insert' template methods return the correct
> type.
> ---
>  gdb/bcache.h   | 25 ++++++++++++++++++++++++-
>  gdb/gdbtypes.c |  2 +-
>  gdb/macrotab.c | 12 ++++++------
>  gdb/psymtab.c  |  6 ++----
>  4 files changed, 33 insertions(+), 12 deletions(-)
>
>
> I can confirm this change does not cause any regressions on Fedora Rawhide
aarch64.
diff mbox series

Patch

diff --git a/gdb/bcache.h b/gdb/bcache.h
index 17a09880d99..a77cd005ea6 100644
--- a/gdb/bcache.h
+++ b/gdb/bcache.h
@@ -152,7 +152,26 @@  struct bcache
      were newly added to the cache, or to false if the bytes were
      found in the cache.  */
 
-  const void *insert (const void *addr, int length, bool *added = nullptr);
+  template<typename T, typename = gdb::Requires<std::is_trivially_copyable<T>>>
+  const T *insert (const T *addr, int length, bool *added = nullptr)
+  {
+    return (const T *) this->insert ((const void *) addr, length, added);
+  }
+
+  /* Find a copy of OBJECT in this bcache.  If BCACHE has never seen
+     those bytes before, add a copy of them to BCACHE.  In either
+     case, return a pointer to BCACHE's copy of that string.  Since
+     the cached value is meant to be read-only, return a const buffer.
+     If ADDED is not NULL, set *ADDED to true if the bytes were newly
+     added to the cache, or to false if the bytes were found in the
+     cache.  */
+
+  template<typename T, typename = gdb::Requires<std::is_trivially_copyable<T>>>
+  const T *insert (const T &object, bool *added = nullptr)
+  {
+    return (const T *) this->insert ((const void *) &object, sizeof (object),
+				     added);
+  }
 
   /* Print statistics on this bcache's memory usage and efficacity at
      eliminating duplication.  TYPE should be a string describing the
@@ -173,6 +192,10 @@  struct bcache
 
 private:
 
+  /* Implementation of the templated 'insert' methods.  */
+
+  const void *insert (const void *addr, int length, bool *added);
+
   /* All the bstrings are allocated here.  */
   struct obstack m_cache {};
 
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 960a7f49e45..4fcfbc587f6 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -4373,7 +4373,7 @@  check_types_worklist (std::vector<type_equality_entry> *worklist,
 
       /* If the type pair has already been visited, we know it is
 	 ok.  */
-      cache->insert (&entry, sizeof (entry), &added);
+      cache->insert (entry, &added);
       if (!added)
 	continue;
 
diff --git a/gdb/macrotab.c b/gdb/macrotab.c
index 5329d2a5a93..a9c425b63c9 100644
--- a/gdb/macrotab.c
+++ b/gdb/macrotab.c
@@ -110,8 +110,9 @@  macro_free (void *object, struct macro_table *t)
 /* If the macro table T has a bcache, then cache the LEN bytes at ADDR
    there, and return the cached copy.  Otherwise, just xmalloc a copy
    of the bytes, and return a pointer to that.  */
-static const void *
-macro_bcache (struct macro_table *t, const void *addr, int len)
+template<typename U>
+static const U *
+macro_bcache (struct macro_table *t, const U *addr, int len)
 {
   if (t->bcache)
     return t->bcache->insert (addr, len);
@@ -120,7 +121,7 @@  macro_bcache (struct macro_table *t, const void *addr, int len)
       void *copy = xmalloc (len);
 
       memcpy (copy, addr, len);
-      return copy;
+      return (const U *) copy;
     }
 }
 
@@ -131,7 +132,7 @@  macro_bcache (struct macro_table *t, const void *addr, int len)
 static const char *
 macro_bcache_str (struct macro_table *t, const char *s)
 {
-  return (const char *) macro_bcache (t, s, strlen (s) + 1);
+  return macro_bcache (t, s, strlen (s) + 1);
 }
 
 
@@ -572,8 +573,7 @@  new_macro_definition (struct macro_table *t,
 	cached_argv[i] = macro_bcache_str (t, argv[i]);
 
       /* Now bcache the array of argument pointers itself.  */
-      d->argv = ((const char * const *)
-		 macro_bcache (t, cached_argv, cached_argv_size));
+      d->argv = macro_bcache (t, cached_argv, cached_argv_size);
     }
 
   /* We don't bcache the entire definition structure because it's got
diff --git a/gdb/psymtab.c b/gdb/psymtab.c
index f1aabd5b10b..ca638515d61 100644
--- a/gdb/psymtab.c
+++ b/gdb/psymtab.c
@@ -1062,10 +1062,8 @@  partial_symtab::add_psymbol (const partial_symbol &psymbol,
   bool added;
 
   /* Stash the partial symbol away in the cache.  */
-  partial_symbol *psym
-    = ((struct partial_symbol *)
-       partial_symtabs->psymbol_cache.insert
-       (&psymbol, sizeof (struct partial_symbol), &added));
+  const partial_symbol *psym = partial_symtabs->psymbol_cache.insert (psymbol,
+								      &added);
 
   /* Do not duplicate global partial symbols.  */
   if (where == psymbol_placement::GLOBAL && !added)