From c05c9914b1875d9edca946532d02daafc82c8615 Mon Sep 17 00:00:00 2001 From: Tom Tromey Date: Mon, 10 Mar 2025 08:51:01 -0600 Subject: [PATCH] Use flags enum for cooked_index_entry::full_name I found a small bug coming from a couple of recent patches of mine for cooked_index_entry::full_name. First, commit aab26529b30 (Add "Ada linkage" mode to cooked_index_entry::full_name) added a small hack to optionally compute the Ada linkage name. Then, commit aab2ac34d7f (Avoid excessive CU expansion on failed matches) changed the relevant expand_symtabs_matching implementation to use this feature. However, the feature was used unconditionally, causing a bad side effect: the non-canonical name is now used for all languages, not just Ada. But, for C++ this is wrong. Furthermore, consider the declaration of full_name: const char *full_name (struct obstack *storage, bool for_main = false, bool for_ada_linkage = false, const char *default_sep = nullptr) const; ... and then consider this call in cooked_index::dump: gdb_printf (" qualified: %s\n", entry->full_name (&temp_storage, false, "::")); Oops! The "::" is silently converted to 'true' here. To fix both of these problems, this patch changes full_name to accept a flags enum rather than booleans. This avoids the type-safety problem. Then, full_name is changed to remove the "Ada" flag when the entry is not in fact an Ada symbol. Regression tested on x86-64 Fedora 40. Approved-By: Simon Marchi --- gdb/dwarf2/cooked-index.c | 32 +++++++++++++++++------------ gdb/dwarf2/cooked-index.h | 42 ++++++++++++++++++++++++++------------- gdb/dwarf2/index-write.c | 2 +- gdb/dwarf2/parent-map.c | 2 +- gdb/dwarf2/read.c | 2 +- 5 files changed, 50 insertions(+), 30 deletions(-) diff --git a/gdb/dwarf2/cooked-index.c b/gdb/dwarf2/cooked-index.c index 6612585649f..457ea44cc23 100644 --- a/gdb/dwarf2/cooked-index.c +++ b/gdb/dwarf2/cooked-index.c @@ -221,11 +221,11 @@ cooked_index_entry::matches (domain_search_flags kind) const /* See cooked-index.h. */ const char * -cooked_index_entry::full_name (struct obstack *storage, bool for_main, - bool for_ada_linkage, +cooked_index_entry::full_name (struct obstack *storage, + cooked_index_full_name_flag name_flags, const char *default_sep) const { - const char *local_name = for_main ? name : canonical; + const char *local_name = ((name_flags & FOR_MAIN) != 0) ? name : canonical; if ((flags & IS_LINKAGE) != 0 || get_parent () == nullptr) return local_name; @@ -240,7 +240,7 @@ cooked_index_entry::full_name (struct obstack *storage, bool for_main, break; case language_ada: - if (for_ada_linkage) + if ((name_flags & FOR_ADA_LINKAGE_NAME) != 0) { sep = "__"; break; @@ -257,7 +257,12 @@ cooked_index_entry::full_name (struct obstack *storage, bool for_main, break; } - get_parent ()->write_scope (storage, sep, for_main, for_ada_linkage); + /* The FOR_ADA_LINKAGE_NAME flag should only affect Ada entries, so + disable it here if we don't need it. */ + if (lang != language_ada) + name_flags &= ~FOR_ADA_LINKAGE_NAME; + + get_parent ()->write_scope (storage, sep, name_flags); obstack_grow0 (storage, local_name, strlen (local_name)); return (const char *) obstack_finish (storage); } @@ -267,14 +272,15 @@ cooked_index_entry::full_name (struct obstack *storage, bool for_main, void cooked_index_entry::write_scope (struct obstack *storage, const char *sep, - bool for_main, - bool for_ada_linkage) const + cooked_index_full_name_flag flags) const { if (get_parent () != nullptr) - get_parent ()->write_scope (storage, sep, for_main, for_ada_linkage); + get_parent ()->write_scope (storage, sep, flags); /* When computing the Ada linkage name, the entry might not have been canonicalized yet, so use the 'name'. */ - const char *local_name = (for_main || for_ada_linkage) ? name : canonical; + const char *local_name = ((flags & (FOR_MAIN | FOR_ADA_LINKAGE_NAME)) != 0 + ? name + : canonical); obstack_grow (storage, local_name, strlen (local_name)); obstack_grow (storage, sep, strlen (sep)); } @@ -471,8 +477,8 @@ cooked_index_shard::finalize (const parent_map_map *parent_maps) code in cooked_index_entry::full_name). */ if (entry->get_parent () != nullptr) { - const char *fullname = entry->full_name (&m_storage, false, - true); + const char *fullname + = entry->full_name (&m_storage, FOR_ADA_LINKAGE_NAME); cooked_index_entry *linkage = create (entry->die_offset, entry->tag, (entry->flags @@ -834,7 +840,7 @@ cooked_index::get_main_name (struct obstack *obstack, enum language *lang) return nullptr; *lang = entry->lang; - return entry->full_name (obstack, true); + return entry->full_name (obstack, FOR_MAIN); } /* See cooked_index.h. */ @@ -902,7 +908,7 @@ cooked_index::dump (gdbarch *arch) gdb_printf (" name: %s\n", entry->name); gdb_printf (" canonical: %s\n", entry->canonical); gdb_printf (" qualified: %s\n", - entry->full_name (&temp_storage, false, "::")); + entry->full_name (&temp_storage, 0, "::")); gdb_printf (" DWARF tag: %s\n", dwarf_tag_name (entry->tag)); gdb_printf (" flags: %s\n", to_string (entry->flags).c_str ()); gdb_printf (" DIE offset: %s\n", sect_offset_str (entry->die_offset)); diff --git a/gdb/dwarf2/cooked-index.h b/gdb/dwarf2/cooked-index.h index f6586359770..4c35d5b65dd 100644 --- a/gdb/dwarf2/cooked-index.h +++ b/gdb/dwarf2/cooked-index.h @@ -65,6 +65,17 @@ enum cooked_index_flag_enum : unsigned char }; DEF_ENUM_FLAGS_TYPE (enum cooked_index_flag_enum, cooked_index_flag); +/* Flags used when requesting the full name of an entry. */ +enum cooked_index_full_name_enum : unsigned char +{ + /* Set when requesting the name of "main". See the method for the + full description. */ + FOR_MAIN = 1, + /* Set when requesting the linkage name for an Ada entry. */ + FOR_ADA_LINKAGE_NAME = 2, +}; +DEF_ENUM_FLAGS_TYPE (enum cooked_index_full_name_enum, cooked_index_full_name_flag); + /* Type representing either a resolved or deferred cooked_index_entry. */ union cooked_index_entry_ref @@ -139,18 +150,22 @@ struct cooked_index_entry : public allocate_on_obstack /* Construct the fully-qualified name of this entry and return a pointer to it. If allocation is needed, it will be done on - STORAGE. FOR_MAIN is true if we are computing the name of the - "main" entry -- one marked DW_AT_main_subprogram. This matters - for avoiding name canonicalization and also a related race (if - "main" computation is done during finalization). If - FOR_ADA_LINKAGE is true, then Ada-language symbols will have - their "linkage-style" name computed. The default is - source-style. If the language - doesn't prescribe a separator, one can be specified using - DEFAULT_SEP. */ + STORAGE. + + FLAGS affects the result. If the FOR_MAIN flag is set, we are + computing the name of the "main" entry -- one marked + DW_AT_main_subprogram. This matters for avoiding name + canonicalization and also a related race (if "main" computation + is done during finalization). + + If the FOR_ADA_LINKAGE_NAME flag is set, then Ada-language + symbols will have their "linkage-style" name computed. The + default is source-style. + + If the language doesn't prescribe a separator, one can be + specified using DEFAULT_SEP. */ const char *full_name (struct obstack *storage, - bool for_main = false, - bool for_ada_linkage = false, + cooked_index_full_name_flag name_flags = 0, const char *default_sep = nullptr) const; /* Comparison modes for the 'compare' function. See the function @@ -255,10 +270,9 @@ private: /* A helper method for full_name. Emits the full scope of this object, followed by the separator, to STORAGE. If this entry has a parent, its write_scope method is called first. See full_name - for a description of the FOR_MAIN and FOR_ADA_LINKAGE - parameters. */ + for a description of the FLAGS parameter. */ void write_scope (struct obstack *storage, const char *sep, - bool for_main, bool for_ada_linkage) const; + cooked_index_full_name_flag flags) const; /* The parent entry. This is NULL for top-level entries. Otherwise, it points to the parent entry, such as a namespace or diff --git a/gdb/dwarf2/index-write.c b/gdb/dwarf2/index-write.c index cfe65cb81f5..9d876b1b93c 100644 --- a/gdb/dwarf2/index-write.c +++ b/gdb/dwarf2/index-write.c @@ -1280,7 +1280,7 @@ write_shortcuts_table (cooked_index *table, data_buf &shortcuts, if (dw_lang != 0) { auto_obstack obstack; - const auto main_name = main_info->full_name (&obstack, true); + const auto main_name = main_info->full_name (&obstack, FOR_MAIN); main_name_offset = cpool.size (); cpool.append_cstr0 (main_name); diff --git a/gdb/dwarf2/parent-map.c b/gdb/dwarf2/parent-map.c index 956f1f287f8..d029a76294a 100644 --- a/gdb/dwarf2/parent-map.c +++ b/gdb/dwarf2/parent-map.c @@ -60,7 +60,7 @@ dump_parent_map (dwarf2_per_bfd *per_bfd, const struct addrmap *map) gdb_printf (outfile, " -> (0x%" PRIx64 ": %s)", to_underlying (parent_entry->die_offset), - parent_entry->full_name (&temp_storage, false)); + parent_entry->full_name (&temp_storage)); }; addrmap_dump (const_cast (map), gdb_stdlog, nullptr, diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c index 3b0f74cfec7..a58c83257c3 100644 --- a/gdb/dwarf2/read.c +++ b/gdb/dwarf2/read.c @@ -14942,7 +14942,7 @@ cooked_index_functions::expand_symtabs_matching { auto_obstack temp_storage; const char *full_name = entry->full_name (&temp_storage, - false, true); + FOR_ADA_LINKAGE_NAME); if (symbol_matcher == nullptr) { symbol_name_matcher_ftype *name_matcher -- 2.39.5