* [PATCH 0/3] Better type safety for bcache
@ 2024-03-20 20:23 Tom Tromey
2024-03-20 20:23 ` [PATCH 1/3] Some constification in psymtab Tom Tromey
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Tom Tromey @ 2024-03-20 20:23 UTC (permalink / raw)
To: gdb-patches
This series changes bcache to only allow trivially-copyable types.
Some other minor API improvements are also made.
Regression tested on x86-64 Fedora 38
---
Tom Tromey (3):
Some constification in psymtab
Make bcache more type-safe
Remove a couple unnecessary casts
gdb/bcache.h | 25 +++++++++++++++++++++-
gdb/gdbtypes.c | 2 +-
gdb/macrotab.c | 12 +++++------
gdb/objfiles.h | 4 ++--
gdb/psymtab.c | 65 +++++++++++++++++++++++++++-------------------------------
gdb/psymtab.h | 4 ++--
6 files changed, 65 insertions(+), 47 deletions(-)
---
base-commit: d5e9331b6ba459fa211229e56d63b11daaab3330
change-id: 20240320-bcache-type-408f5e90ca2a
Best regards,
--
Tom Tromey <tromey@adacore.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/3] Some constification in psymtab
2024-03-20 20:23 [PATCH 0/3] Better type safety for bcache Tom Tromey
@ 2024-03-20 20:23 ` Tom Tromey
2024-03-25 15:00 ` Alexandra Petlanova Hajkova
2024-03-20 20:23 ` [PATCH 2/3] Make bcache more type-safe Tom Tromey
2024-03-20 20:24 ` [PATCH 3/3] Remove a couple unnecessary casts Tom Tromey
2 siblings, 1 reply; 7+ messages in thread
From: Tom Tromey @ 2024-03-20 20:23 UTC (permalink / raw)
To: gdb-patches
This patch changes some spots in psymtab.[ch] to use 'const'. This is
just preparation for a subsequent patch. Note that psymbols are
conceptually const, and since they were changed to be
objfile-indepdendent, they are truly never modified -- which is what
makes this patch reasonably short.
---
gdb/psymtab.c | 59 ++++++++++++++++++++++++++++-------------------------------
gdb/psymtab.h | 4 ++--
2 files changed, 30 insertions(+), 33 deletions(-)
diff --git a/gdb/psymtab.c b/gdb/psymtab.c
index a831e5eec40..f1aabd5b10b 100644
--- a/gdb/psymtab.c
+++ b/gdb/psymtab.c
@@ -37,18 +37,15 @@
#include <set>
#include "gdbsupport/buildargv.h"
-static struct partial_symbol *lookup_partial_symbol (struct objfile *,
- struct partial_symtab *,
- const lookup_name_info &,
- int,
- domain_search_flags);
+static const struct partial_symbol *lookup_partial_symbol
+ (struct objfile *, struct partial_symtab *, const lookup_name_info &,
+ int, domain_search_flags);
static const char *psymtab_to_fullname (struct partial_symtab *ps);
-static struct partial_symbol *find_pc_sect_psymbol (struct objfile *,
- struct partial_symtab *,
- CORE_ADDR,
- struct obj_section *);
+static const struct partial_symbol *find_pc_sect_psymbol
+ (struct objfile *, struct partial_symtab *, CORE_ADDR,
+ struct obj_section *);
static struct compunit_symtab *psymtab_to_symtab (struct objfile *objfile,
struct partial_symtab *pst);
@@ -117,7 +114,7 @@ find_pc_sect_psymtab_closer (struct objfile *objfile,
{
if (pc >= tpst->text_low (objfile) && pc < tpst->text_high (objfile))
{
- struct partial_symbol *p;
+ const struct partial_symbol *p;
CORE_ADDR this_addr;
/* NOTE: This assumes that every psymbol has a
@@ -212,12 +209,12 @@ psymbol_functions::find_pc_sect_compunit_symtab
/* Find which partial symbol within a psymtab matches PC and SECTION.
Return NULL if none. */
-static struct partial_symbol *
+static const struct partial_symbol *
find_pc_sect_psymbol (struct objfile *objfile,
struct partial_symtab *psymtab, CORE_ADDR pc,
struct obj_section *section)
{
- struct partial_symbol *best = NULL;
+ const struct partial_symbol *best = NULL;
CORE_ADDR best_pc;
const CORE_ADDR textlow = psymtab->text_low (objfile);
@@ -229,7 +226,7 @@ find_pc_sect_psymbol (struct objfile *objfile,
/* Search the global symbols as well as the static symbols, so that
find_pc_partial_function doesn't use a minimal symbol and thus
cache a bad endaddr. */
- for (partial_symbol *p : psymtab->global_psymbols)
+ for (const partial_symbol *p : psymtab->global_psymbols)
{
if (p->domain == VAR_DOMAIN
&& p->aclass == LOC_BLOCK
@@ -249,7 +246,7 @@ find_pc_sect_psymbol (struct objfile *objfile,
}
}
- for (partial_symbol *p : psymtab->static_psymbols)
+ for (const partial_symbol *p : psymtab->static_psymbols)
{
if (p->domain == VAR_DOMAIN
&& p->aclass == LOC_BLOCK
@@ -289,7 +286,7 @@ psymbol_functions::lookup_global_symbol_language (struct objfile *objfile,
for (partial_symtab *ps : partial_symbols (objfile))
{
- struct partial_symbol *psym;
+ const struct partial_symbol *psym;
if (ps->readin_p (objfile))
continue;
@@ -307,7 +304,7 @@ psymbol_functions::lookup_global_symbol_language (struct objfile *objfile,
/* Returns true if PSYM matches LOOKUP_NAME. */
static bool
-psymbol_name_matches (partial_symbol *psym,
+psymbol_name_matches (const partial_symbol *psym,
const lookup_name_info &lookup_name)
{
const language_defn *lang = language_def (psym->ginfo.language ());
@@ -320,14 +317,14 @@ psymbol_name_matches (partial_symbol *psym,
LOOKUP_NAME. Check the global symbols if GLOBAL, the static
symbols if not. */
-static struct partial_symbol *
+static const struct partial_symbol *
lookup_partial_symbol (struct objfile *objfile,
struct partial_symtab *pst,
const lookup_name_info &lookup_name,
int global, domain_search_flags domain)
{
- struct partial_symbol **start, **psym;
- struct partial_symbol **top, **real_top, **bottom, **center;
+ const struct partial_symbol **start, **psym;
+ const struct partial_symbol **top, **real_top, **bottom, **center;
int length = (global
? pst->global_psymbols.size ()
: pst->static_psymbols.size ());
@@ -497,11 +494,11 @@ psymbol_functions::forget_cached_source_info (struct objfile *objfile)
static void
print_partial_symbols (struct gdbarch *gdbarch, struct objfile *objfile,
- const std::vector<partial_symbol *> &symbols,
+ const std::vector<const partial_symbol *> &symbols,
const char *what, struct ui_file *outfile)
{
gdb_printf (outfile, " %s partial symbols:\n", what);
- for (partial_symbol *p : symbols)
+ for (const partial_symbol *p : symbols)
{
QUIT;
gdb_printf (outfile, " `%s'", p->ginfo.linkage_name ());
@@ -830,15 +827,15 @@ recursively_search_psymtabs
}
}
- partial_symbol **gbound = (ps->global_psymbols.data ()
- + ps->global_psymbols.size ());
- partial_symbol **sbound = (ps->static_psymbols.data ()
- + ps->static_psymbols.size ());
- partial_symbol **bound = gbound;
+ const partial_symbol **gbound = (ps->global_psymbols.data ()
+ + ps->global_psymbols.size ());
+ const partial_symbol **sbound = (ps->static_psymbols.data ()
+ + ps->static_psymbols.size ());
+ const partial_symbol **bound = gbound;
/* Go through all of the symbols stored in a partial
symtab in one loop. */
- partial_symbol **psym = ps->global_psymbols.data ();
+ const partial_symbol **psym = ps->global_psymbols.data ();
if ((search_flags & SEARCH_GLOBAL_BLOCK) == 0)
{
@@ -1006,7 +1003,7 @@ partial_symtab::end ()
/* Sort the global list; don't sort the static list. */
std::sort (global_psymbols.begin (),
global_psymbols.end (),
- [] (partial_symbol *s1, partial_symbol *s2)
+ [] (const partial_symbol *s1, const partial_symbol *s2)
{
return strcmp_iw_ordered (s1->ginfo.search_name (),
s2->ginfo.search_name ()) < 0;
@@ -1075,7 +1072,7 @@ partial_symtab::add_psymbol (const partial_symbol &psymbol,
return;
/* Save pointer to partial symbol in psymtab, growing symtab if needed. */
- std::vector<partial_symbol *> &list
+ std::vector<const partial_symbol *> &list
= (where == psymbol_placement::STATIC
? static_psymbols
: global_psymbols);
@@ -1496,7 +1493,7 @@ maintenance_check_psymtabs (const char *ignore, int from_tty)
continue;
bv = cust->blockvector ();
b = bv->static_block ();
- for (partial_symbol *psym : ps->static_psymbols)
+ for (const partial_symbol *psym : ps->static_psymbols)
{
/* Skip symbols for inlined functions without address. These may
or may not have a match in the full symtab. */
@@ -1518,7 +1515,7 @@ maintenance_check_psymtabs (const char *ignore, int from_tty)
}
}
b = bv->global_block ();
- for (partial_symbol *psym : ps->global_psymbols)
+ for (const partial_symbol *psym : ps->global_psymbols)
{
lookup_name_info lookup_name
(psym->ginfo.search_name (), symbol_name_match_type::SEARCH_NAME);
diff --git a/gdb/psymtab.h b/gdb/psymtab.h
index 557a7cf4c08..f41f2418639 100644
--- a/gdb/psymtab.h
+++ b/gdb/psymtab.h
@@ -444,7 +444,7 @@ struct partial_symtab
improve access. Binary search will be the usual method of
finding a symbol within it. */
- std::vector<partial_symbol *> global_psymbols;
+ std::vector<const partial_symbol *> global_psymbols;
/* Static symbol list. This list will *not* be sorted after readin;
to find a symbol in it, exhaustive search must be used. This is
@@ -453,7 +453,7 @@ struct partial_symtab
to take a *lot* of time; check) or an error (and we don't care
how long errors take). */
- std::vector<partial_symbol *> static_psymbols;
+ std::vector<const partial_symbol *> static_psymbols;
/* True if the name of this partial symtab is not a source file name. */
--
2.43.0
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/3] Make bcache more type-safe
2024-03-20 20:23 [PATCH 0/3] Better type safety for bcache Tom Tromey
2024-03-20 20:23 ` [PATCH 1/3] Some constification in psymtab Tom Tromey
@ 2024-03-20 20:23 ` Tom Tromey
2024-03-27 14:15 ` Alexandra Petlanova Hajkova
2024-03-20 20:24 ` [PATCH 3/3] Remove a couple unnecessary casts Tom Tromey
2 siblings, 1 reply; 7+ messages in thread
From: Tom Tromey @ 2024-03-20 20:23 UTC (permalink / raw)
To: gdb-patches
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(-)
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)
--
2.43.0
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 3/3] Remove a couple unnecessary casts
2024-03-20 20:23 [PATCH 0/3] Better type safety for bcache Tom Tromey
2024-03-20 20:23 ` [PATCH 1/3] Some constification in psymtab Tom Tromey
2024-03-20 20:23 ` [PATCH 2/3] Make bcache more type-safe Tom Tromey
@ 2024-03-20 20:24 ` Tom Tromey
2024-03-27 19:50 ` Alexandra Petlanova Hajkova
2 siblings, 1 reply; 7+ messages in thread
From: Tom Tromey @ 2024-03-20 20:24 UTC (permalink / raw)
To: gdb-patches
After the previous bcache change, a couple of casts in objfiles.h are
now redundant.
---
gdb/objfiles.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/gdb/objfiles.h b/gdb/objfiles.h
index 8b8b7182e87..ab655906508 100644
--- a/gdb/objfiles.h
+++ b/gdb/objfiles.h
@@ -234,14 +234,14 @@ struct objfile_per_bfd_storage
const char *intern (const char *str)
{
- return (const char *) string_cache.insert (str, strlen (str) + 1);
+ return string_cache.insert (str, strlen (str) + 1);
}
/* Same as the above, but for an std::string. */
const char *intern (const std::string &str)
{
- return (const char *) string_cache.insert (str.c_str (), str.size () + 1);
+ return string_cache.insert (str.c_str (), str.size () + 1);
}
/* Get the BFD this object is associated to. */
--
2.43.0
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] Some constification in psymtab
2024-03-20 20:23 ` [PATCH 1/3] Some constification in psymtab Tom Tromey
@ 2024-03-25 15:00 ` Alexandra Petlanova Hajkova
0 siblings, 0 replies; 7+ messages in thread
From: Alexandra Petlanova Hajkova @ 2024-03-25 15:00 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 874 bytes --]
On Wed, Mar 20, 2024 at 9:24 PM Tom Tromey <tromey@adacore.com> wrote:
> This patch changes some spots in psymtab.[ch] to use 'const'. This is
> just preparation for a subsequent patch. Note that psymbols are
> conceptually const, and since they were changed to be
> objfile-indepdendent, they are truly never modified -- which is what
> makes this patch reasonably short.
> ---
> gdb/psymtab.c | 59
> ++++++++++++++++++++++++++++-------------------------------
> gdb/psymtab.h | 4 ++--
> 2 files changed, 30 insertions(+), 33 deletions(-)
>
> diff --git a/gdb/psymtab.c b/gdb/psymtab.c
> index a831e5eec40..f1aabd5b10b 100644
> --- a/gdb/psymtab.c
> +++ b/gdb/psymtab.c
> @@ -37,18 +37,15 @@
> #include <set>
> #include "gdbsupport/buildargv.h"
>
> This patch looks reasonable and causes no regressions for Fedora Rawhide
> on aarch64.
>
[-- Attachment #2: Type: text/html, Size: 1249 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/3] Make bcache more type-safe
2024-03-20 20:23 ` [PATCH 2/3] Make bcache more type-safe Tom Tromey
@ 2024-03-27 14:15 ` Alexandra Petlanova Hajkova
0 siblings, 0 replies; 7+ messages in thread
From: Alexandra Petlanova Hajkova @ 2024-03-27 14:15 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 728 bytes --]
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.
[-- Attachment #2: Type: text/html, Size: 1090 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 3/3] Remove a couple unnecessary casts
2024-03-20 20:24 ` [PATCH 3/3] Remove a couple unnecessary casts Tom Tromey
@ 2024-03-27 19:50 ` Alexandra Petlanova Hajkova
0 siblings, 0 replies; 7+ messages in thread
From: Alexandra Petlanova Hajkova @ 2024-03-27 19:50 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 343 bytes --]
On Wed, Mar 20, 2024 at 9:25 PM Tom Tromey <tromey@adacore.com> wrote:
> After the previous bcache change, a couple of casts in objfiles.h are
> now redundant.
> ---
> gdb/objfiles.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> It's nice to get rid of redundant things. Causes no regressions as
> expected.
>
[-- Attachment #2: Type: text/html, Size: 670 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-03-27 19:51 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-20 20:23 [PATCH 0/3] Better type safety for bcache Tom Tromey
2024-03-20 20:23 ` [PATCH 1/3] Some constification in psymtab Tom Tromey
2024-03-25 15:00 ` Alexandra Petlanova Hajkova
2024-03-20 20:23 ` [PATCH 2/3] Make bcache more type-safe Tom Tromey
2024-03-27 14:15 ` Alexandra Petlanova Hajkova
2024-03-20 20:24 ` [PATCH 3/3] Remove a couple unnecessary casts Tom Tromey
2024-03-27 19:50 ` Alexandra Petlanova Hajkova
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox