* [PATCH v2 01/28] Add another minor hack to cooked_index_entry::full_name
2025-04-02 23:44 [PATCH v2 00/28] Search symbols via quick API Tom Tromey
@ 2025-04-02 23:45 ` Tom Tromey
2025-04-02 23:45 ` [PATCH v2 02/28] Change ada_decode to preserve upper-case in some situations Tom Tromey
` (28 subsequent siblings)
29 siblings, 0 replies; 50+ messages in thread
From: Tom Tromey @ 2025-04-02 23:45 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This patch adds another minor hack to cooked_index_entry::full_name.
In particular, if GNAT emits non-hierarchical names (still the default
as the hierarchical series is blocked on one tricky problem), then a
request to compute the "linkage-style" name will now just return the
'name' field.
Without this tweak, this series would regress ada-cold-name.exp,
because the search would look for "name.cold" but the index would
return "name[cold]" as the "linkage" name (which would be wrong).
This area is a bit difficult to unravel. The best plan here, IMO, is
to change Ada to work like the other languages in gdb: store the
natural name and do searches with that name. I think this is
achievable, but I didn't want to try it here.
I've updated the relevant bug (tagged below) to reflect this.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32766
---
gdb/dwarf2/cooked-index-entry.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/gdb/dwarf2/cooked-index-entry.c b/gdb/dwarf2/cooked-index-entry.c
index 3e322f140e5c0c1ca4c81d1a5affb0a5beb8750b..d0dac2f30004603254b211682c50109873a59015 100644
--- a/gdb/dwarf2/cooked-index-entry.c
+++ b/gdb/dwarf2/cooked-index-entry.c
@@ -188,8 +188,15 @@ cooked_index_entry::full_name (struct obstack *storage,
break;
case language_ada:
+ /* If GNAT emits hierarchical names (patches not in at the time
+ of writing), then we need to compute the linkage name here.
+ However for traditional GNAT, the linkage name will be in
+ 'name'. Detect this by looking for "__"; see also
+ cooked_index_shard::finalize. */
if ((name_flags & FOR_ADA_LINKAGE_NAME) != 0)
{
+ if (strstr (name, "__") != nullptr)
+ return name;
sep = "__";
break;
}
--
2.46.1
^ permalink raw reply [flat|nested] 50+ messages in thread* [PATCH v2 02/28] Change ada_decode to preserve upper-case in some situations
2025-04-02 23:44 [PATCH v2 00/28] Search symbols via quick API Tom Tromey
2025-04-02 23:45 ` [PATCH v2 01/28] Add another minor hack to cooked_index_entry::full_name Tom Tromey
@ 2025-04-02 23:45 ` Tom Tromey
2025-04-02 23:45 ` [PATCH v2 03/28] Emit some type declarations in .gdb_index Tom Tromey
` (27 subsequent siblings)
29 siblings, 0 replies; 50+ messages in thread
From: Tom Tromey @ 2025-04-02 23:45 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This patch is needed to avoid regressions later in the series.
The issue here is that ada_decode, when called with wide=false, would
act as though the input needed verbatim quoting. That would happen
because the 'W' character would be passed through; and then a later
loop would reject the result due to that character.
Similarly, with operators=false the upper-case-checking loop would be
skipped, but then some names that did need verbatim quoting would pass
through.
Furthermore I noticed that there isn't a need to distinguish between
the "wide" and "operators" cases -- all callers pass identical values
to both.
This patch cleans up the above, consolidating the parameters and
changing how upper-case detection is handled, so that both the
operator and wide cases pass-through without issue. I've added new
unit tests for this.
---
gdb/ada-lang.c | 83 +++++++++++++++++++++++++++++------------
gdb/ada-lang.h | 15 +++-----
gdb/dwarf2/cooked-index-shard.c | 2 +-
gdb/symtab.h | 2 +-
4 files changed, 68 insertions(+), 34 deletions(-)
diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index a55ee12ce70d02082e64d85634b87dd27f5a0670..4bb6a808fd8c1a7f8e4b2344fdf935f94c602ed1 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -1308,7 +1308,7 @@ convert_from_hex_encoded (std::string &out, const char *str, int n)
/* See ada-lang.h. */
std::string
-ada_decode (const char *encoded, bool wrap, bool operators, bool wide)
+ada_decode (const char *encoded, bool wrap, bool translate)
{
int i;
int len0;
@@ -1403,7 +1403,7 @@ ada_decode (const char *encoded, bool wrap, bool operators, bool wide)
while (i < len0)
{
/* Is this a symbol function? */
- if (operators && at_start_name && encoded[i] == 'O')
+ if (at_start_name && encoded[i] == 'O')
{
int k;
@@ -1414,7 +1414,10 @@ ada_decode (const char *encoded, bool wrap, bool operators, bool wide)
op_len - 1) == 0)
&& !isalnum (encoded[i + op_len]))
{
- decoded.append (ada_opname_table[k].decoded);
+ if (translate)
+ decoded.append (ada_opname_table[k].decoded);
+ else
+ decoded.append (ada_opname_table[k].encoded);
at_start_name = 0;
i += op_len;
break;
@@ -1502,28 +1505,59 @@ ada_decode (const char *encoded, bool wrap, bool operators, bool wide)
i++;
}
- if (wide && i < len0 + 3 && encoded[i] == 'U' && isxdigit (encoded[i + 1]))
+ /* Handle wide characters while respecting the arguments to the
+ function: we may want to copy them verbatim, but in this case
+ we do not want to register that we've copied an upper-case
+ character. */
+ if (i < len0 + 3 && encoded[i] == 'U' && isxdigit (encoded[i + 1]))
{
- if (convert_from_hex_encoded (decoded, &encoded[i + 1], 2))
+ if (translate)
{
- i += 3;
+ if (convert_from_hex_encoded (decoded, &encoded[i + 1], 2))
+ {
+ i += 3;
+ continue;
+ }
+ }
+ else
+ {
+ decoded.push_back (encoded[i]);
+ ++i;
continue;
}
}
- else if (wide && i < len0 + 5 && encoded[i] == 'W' && isxdigit (encoded[i + 1]))
+ else if (i < len0 + 5 && encoded[i] == 'W' && isxdigit (encoded[i + 1]))
{
- if (convert_from_hex_encoded (decoded, &encoded[i + 1], 4))
+ if (translate)
+ {
+ if (convert_from_hex_encoded (decoded, &encoded[i + 1], 4))
+ {
+ i += 5;
+ continue;
+ }
+ }
+ else
{
- i += 5;
+ decoded.push_back (encoded[i]);
+ ++i;
continue;
}
}
- else if (wide && i < len0 + 10 && encoded[i] == 'W' && encoded[i + 1] == 'W'
+ else if (i < len0 + 10 && encoded[i] == 'W' && encoded[i + 1] == 'W'
&& isxdigit (encoded[i + 2]))
{
- if (convert_from_hex_encoded (decoded, &encoded[i + 2], 8))
+ if (translate)
{
- i += 10;
+ if (convert_from_hex_encoded (decoded, &encoded[i + 2], 8))
+ {
+ i += 10;
+ continue;
+ }
+ }
+ else
+ {
+ decoded.push_back (encoded[i]);
+ ++i;
continue;
}
}
@@ -1550,6 +1584,12 @@ ada_decode (const char *encoded, bool wrap, bool operators, bool wide)
at_start_name = 1;
i += 2;
}
+ else if (isupper (encoded[i]) || encoded[i] == ' ')
+ {
+ /* Decoded names should never contain any uppercase
+ character. */
+ goto Suppress;
+ }
else
{
/* It's a character part of the decoded name, so just copy it
@@ -1559,16 +1599,6 @@ ada_decode (const char *encoded, bool wrap, bool operators, bool wide)
}
}
- /* Decoded names should never contain any uppercase character.
- Double-check this, and abort the decoding if we find one. */
-
- if (operators)
- {
- for (i = 0; i < decoded.length(); ++i)
- if (isupper (decoded[i]) || decoded[i] == ' ')
- goto Suppress;
- }
-
/* If the compiler added a suffix, append it now. */
if (suffix >= 0)
decoded = decoded + "[" + &encoded[suffix] + "]";
@@ -1594,6 +1624,13 @@ ada_decode_tests ()
/* This isn't valid, but used to cause a crash. PR gdb/30639. The
result does not really matter very much. */
SELF_CHECK (ada_decode ("44") == "44");
+
+ /* Check that the settings used by the DWARF reader have the desired
+ effect. */
+ SELF_CHECK (ada_decode ("symada__cS", false, false) == "");
+ SELF_CHECK (ada_decode ("pkg__Oxor", false, false) == "pkg.Oxor");
+ SELF_CHECK (ada_decode ("pack__func_W017b", false, false)
+ == "pack.func_W017b");
}
#endif
@@ -13311,7 +13348,7 @@ ada_lookup_name_info::ada_lookup_name_info (const lookup_name_info &lookup_name)
else
m_standard_p = false;
- m_decoded_name = ada_decode (m_encoded_name.c_str (), true, false, false);
+ m_decoded_name = ada_decode (m_encoded_name.c_str (), true, false);
/* If the name contains a ".", then the user is entering a fully
qualified entity name, and the match must not be done in wild
diff --git a/gdb/ada-lang.h b/gdb/ada-lang.h
index 3582082a1a1b702595b803072ff9c345b7f3e0f7..a96a1f6e01737b03c6e6dea5024fbdd253647201 100644
--- a/gdb/ada-lang.h
+++ b/gdb/ada-lang.h
@@ -218,16 +218,13 @@ extern const char *ada_decode_symbol (const struct general_symbol_info *);
simply wrapped in <...>. If WRAP is false, then the empty string
will be returned.
- When OPERATORS is false, operator names will not be decoded. By
- default, they are decoded, e.g., 'Oadd' will be transformed to
- '"+"'.
-
- When WIDE is false, wide characters will be left as-is. By
- default, they converted from their hex encoding to the host
- charset. */
+ TRANSLATE has two effects. When true (the default), operator names
+ and wide characters will be decoded. E.g., 'Oadd' will be
+ transformed to '"+"', and wide characters converted from their hex
+ encoding to the host charset. When false, these will be left
+ alone. */
extern std::string ada_decode (const char *name, bool wrap = true,
- bool operators = true,
- bool wide = true);
+ bool translate = true);
extern std::vector<struct block_symbol> ada_lookup_symbol_list
(const char *, const struct block *, domain_search_flags);
diff --git a/gdb/dwarf2/cooked-index-shard.c b/gdb/dwarf2/cooked-index-shard.c
index 683feb2ce9615be23a39f3934e922b53574fa5ab..29a8aea513786e4c1c1ed77dee8610fc329d1c8a 100644
--- a/gdb/dwarf2/cooked-index-shard.c
+++ b/gdb/dwarf2/cooked-index-shard.c
@@ -108,7 +108,7 @@ cooked_index_shard::handle_gnat_encoded_entry
characters are left as-is. This is done to make name matching a
bit simpler; and for wide characters, it means the choice of Ada
source charset does not affect the indexer directly. */
- std::string canonical = ada_decode (entry->name, false, false, false);
+ std::string canonical = ada_decode (entry->name, false, false);
if (canonical.empty ())
{
entry->canonical = entry->name;
diff --git a/gdb/symtab.h b/gdb/symtab.h
index 7927380fca3f115fd43ecdaf683ecc07a0ff22e0..83913b1806f4a5fe39987978bb7059efc606a594 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -145,7 +145,7 @@ class ada_lookup_name_info final
std::string m_encoded_name;
/* The decoded lookup name. This is formed by calling ada_decode
- with both 'operators' and 'wide' set to false. */
+ with 'translate' set to false. */
std::string m_decoded_name;
/* Whether the user-provided lookup name was Ada encoded. If so,
--
2.46.1
^ permalink raw reply [flat|nested] 50+ messages in thread* [PATCH v2 03/28] Emit some type declarations in .gdb_index
2025-04-02 23:44 [PATCH v2 00/28] Search symbols via quick API Tom Tromey
2025-04-02 23:45 ` [PATCH v2 01/28] Add another minor hack to cooked_index_entry::full_name Tom Tromey
2025-04-02 23:45 ` [PATCH v2 02/28] Change ada_decode to preserve upper-case in some situations Tom Tromey
@ 2025-04-02 23:45 ` Tom Tromey
2025-04-21 2:50 ` Simon Marchi
2025-04-02 23:45 ` [PATCH v2 04/28] Ada import functions not in index Tom Tromey
` (26 subsequent siblings)
29 siblings, 1 reply; 50+ messages in thread
From: Tom Tromey @ 2025-04-02 23:45 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
If you run struct-decl.exp with the .gdb_index board, you will see
that "the_type" is not emitted in the index. This would cause a
failure in this series. The fix is to ensure that certain necessary
type declarations are emitted.
However, a naive fix here will regress stub-array-size.exp, where a
type declaration and a type definition are both seen -- but the
declaration is seen first and causes a failure. This is handled by
adding some code (including a mild hack) to filter out type
declarations when a corresponding type definition is seen.
---
gdb/dwarf2/index-write.c | 79 +++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 74 insertions(+), 5 deletions(-)
diff --git a/gdb/dwarf2/index-write.c b/gdb/dwarf2/index-write.c
index da1f6cd1e0869620a99959639718a554882bab2c..389fcaa727ce15706a735ce82c047374fbc57d2b 100644
--- a/gdb/dwarf2/index-write.c
+++ b/gdb/dwarf2/index-write.c
@@ -55,7 +55,7 @@
#define DW2_GDB_INDEX_SYMBOL_KIND_SET_VALUE(cu_index, value) \
do { \
gdb_assert ((value) >= GDB_INDEX_SYMBOL_KIND_TYPE \
- && (value) <= GDB_INDEX_SYMBOL_KIND_OTHER); \
+ && (value) <= GDB_INDEX_SYMBOL_KIND_UNUSED5); \
GDB_INDEX_SYMBOL_KIND_SET_VALUE((cu_index), (value)); \
} while (0)
@@ -422,10 +422,53 @@ symtab_index_entry::minimize ()
if (name == nullptr || cu_indices.empty ())
return;
- std::sort (cu_indices.begin (), cu_indices.end ());
+ /* We sort the indexes in a funny way: GDB_INDEX_SYMBOL_KIND_UNUSED5
+ is always sorted last; then otherwise we sort by numeric value.
+ This ensures that we prefer the definition when both a definition
+ and a declaration (stub type) are seen. */
+ std::sort (cu_indices.begin (), cu_indices.end (),
+ [] (offset_type vala, offset_type valb)
+ {
+ gdb_index_symbol_kind kinda
+ = GDB_INDEX_SYMBOL_KIND_VALUE (vala);
+ gdb_index_symbol_kind kindb
+ = GDB_INDEX_SYMBOL_KIND_VALUE (valb);
+ if (kinda == GDB_INDEX_SYMBOL_KIND_UNUSED5)
+ {
+ if (kindb == GDB_INDEX_SYMBOL_KIND_UNUSED5)
+ return vala < valb;
+ /* Declaration sorts last. */
+ return false;
+ }
+ else if (kindb == GDB_INDEX_SYMBOL_KIND_UNUSED5)
+ {
+ /* Declaration sorts last. */
+ return true;
+ }
+ return vala < valb;
+ });
auto from = std::unique (cu_indices.begin (), cu_indices.end ());
cu_indices.erase (from, cu_indices.end ());
+ /* Rewrite GDB_INDEX_SYMBOL_KIND_UNUSED5. This ensures that a type
+ declaration will be deleted by the subsequent squashing step, if
+ warranted. */
+ for (auto &val : cu_indices)
+ {
+ gdb_index_symbol_kind kind = GDB_INDEX_SYMBOL_KIND_VALUE (val);
+ if (kind != GDB_INDEX_SYMBOL_KIND_UNUSED5)
+ continue;
+
+ offset_type newval = 0;
+ DW2_GDB_INDEX_CU_SET_VALUE (newval, GDB_INDEX_CU_VALUE (val));
+ DW2_GDB_INDEX_SYMBOL_STATIC_SET_VALUE
+ (newval, GDB_INDEX_SYMBOL_STATIC_VALUE (val));
+ DW2_GDB_INDEX_SYMBOL_KIND_SET_VALUE (newval,
+ GDB_INDEX_SYMBOL_KIND_TYPE);
+
+ val = newval;
+ }
+
/* We don't want to enter a type more than once, so
remove any such duplicates from the list as well. When doing
this, we want to keep the entry from the first CU -- but this is
@@ -1212,6 +1255,22 @@ write_cooked_index (cooked_index *table,
const cu_index_map &cu_index_htab,
struct mapped_symtab *symtab)
{
+ /* A type declaration that is used as a (non-trivial) parent entry
+ must be written out. */
+ gdb::unordered_set<const cooked_index_entry *> required_entries;
+ for (const cooked_index_entry *entry : table->all_entries ())
+ {
+ if ((entry->flags & IS_TYPE_DECLARATION) == 0
+ && entry->get_parent () != nullptr)
+ {
+ for (const cooked_index_entry *parent = entry->get_parent ();
+ parent != nullptr;
+ parent = parent->get_parent ())
+ if ((parent->flags & IS_TYPE_DECLARATION) != 0)
+ required_entries.insert (parent);
+ }
+ }
+
for (const cooked_index_entry *entry : table->all_entries ())
{
const auto it = cu_index_htab.find (entry->per_cu);
@@ -1239,8 +1298,9 @@ write_cooked_index (cooked_index *table,
}
else if ((entry->flags & IS_TYPE_DECLARATION) != 0)
{
- /* Don't add type declarations to the index. */
- continue;
+ /* Don't add most type declarations to the index. */
+ if (!required_entries.contains (entry))
+ continue;
}
gdb_index_symbol_kind kind;
@@ -1252,7 +1312,16 @@ write_cooked_index (cooked_index *table,
|| entry->tag == DW_TAG_enumerator)
kind = GDB_INDEX_SYMBOL_KIND_VARIABLE;
else if (tag_is_type (entry->tag))
- kind = GDB_INDEX_SYMBOL_KIND_TYPE;
+ {
+ /* If we added a type declaration, we want to note this
+ fact for later, because we don't want a type declaration
+ to cause the real definition to be omitted from the
+ index. GDB_INDEX_SYMBOL_KIND_UNUSED5 is used here, but
+ rewritten later before the index is written. */
+ kind = ((entry->flags & IS_TYPE_DECLARATION) == 0
+ ? GDB_INDEX_SYMBOL_KIND_TYPE
+ : GDB_INDEX_SYMBOL_KIND_UNUSED5);
+ }
else
kind = GDB_INDEX_SYMBOL_KIND_OTHER;
--
2.46.1
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH v2 03/28] Emit some type declarations in .gdb_index
2025-04-02 23:45 ` [PATCH v2 03/28] Emit some type declarations in .gdb_index Tom Tromey
@ 2025-04-21 2:50 ` Simon Marchi
2025-04-21 14:50 ` Tom Tromey
0 siblings, 1 reply; 50+ messages in thread
From: Simon Marchi @ 2025-04-21 2:50 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
On 2025-04-02 19:45, Tom Tromey wrote:
> If you run struct-decl.exp with the .gdb_index board, you will see
> that "the_type" is not emitted in the index. This would cause a
> failure in this series. The fix is to ensure that certain necessary
> type declarations are emitted.
>
> However, a naive fix here will regress stub-array-size.exp, where a
> type declaration and a type definition are both seen -- but the
> declaration is seen first and causes a failure. This is handled by
> adding some code (including a mild hack) to filter out type
> declarations when a corresponding type definition is seen.
I don't really understand the use of GDB_INDEX_SYMBOL_KIND_UNUSED5, what
does it mean?
Simon
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH v2 03/28] Emit some type declarations in .gdb_index
2025-04-21 2:50 ` Simon Marchi
@ 2025-04-21 14:50 ` Tom Tromey
2025-04-23 4:11 ` Simon Marchi
0 siblings, 1 reply; 50+ messages in thread
From: Tom Tromey @ 2025-04-21 14:50 UTC (permalink / raw)
To: Simon Marchi; +Cc: Tom Tromey, gdb-patches
>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:
Simon> On 2025-04-02 19:45, Tom Tromey wrote:
>> If you run struct-decl.exp with the .gdb_index board, you will see
>> that "the_type" is not emitted in the index. This would cause a
>> failure in this series. The fix is to ensure that certain necessary
>> type declarations are emitted.
>>
>> However, a naive fix here will regress stub-array-size.exp, where a
>> type declaration and a type definition are both seen -- but the
>> declaration is seen first and causes a failure. This is handled by
>> adding some code (including a mild hack) to filter out type
>> declarations when a corresponding type definition is seen.
Simon> I don't really understand the use of GDB_INDEX_SYMBOL_KIND_UNUSED5, what
Simon> does it mean?
I thought it was reasonably well explained in the comments.
Could you say which part needs more text to be clear?
+ /* We sort the indexes in a funny way: GDB_INDEX_SYMBOL_KIND_UNUSED5
+ is always sorted last; then otherwise we sort by numeric value.
+ This ensures that we prefer the definition when both a definition
+ and a declaration (stub type) are seen. */
...
+ /* Rewrite GDB_INDEX_SYMBOL_KIND_UNUSED5. This ensures that a type
+ declaration will be deleted by the subsequent squashing step, if
+ warranted. */
...
+ /* If we added a type declaration, we want to note this
+ fact for later, because we don't want a type declaration
+ to cause the real definition to be omitted from the
+ index. GDB_INDEX_SYMBOL_KIND_UNUSED5 is used here, but
+ rewritten later before the index is written. */
Tom
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH v2 03/28] Emit some type declarations in .gdb_index
2025-04-21 14:50 ` Tom Tromey
@ 2025-04-23 4:11 ` Simon Marchi
2025-04-23 20:54 ` Tom Tromey
0 siblings, 1 reply; 50+ messages in thread
From: Simon Marchi @ 2025-04-23 4:11 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
On 2025-04-21 10:50, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:
>
> Simon> On 2025-04-02 19:45, Tom Tromey wrote:
>>> If you run struct-decl.exp with the .gdb_index board, you will see
>>> that "the_type" is not emitted in the index. This would cause a
>>> failure in this series. The fix is to ensure that certain necessary
>>> type declarations are emitted.
>>>
>>> However, a naive fix here will regress stub-array-size.exp, where a
>>> type declaration and a type definition are both seen -- but the
>>> declaration is seen first and causes a failure. This is handled by
>>> adding some code (including a mild hack) to filter out type
>>> declarations when a corresponding type definition is seen.
>
> Simon> I don't really understand the use of GDB_INDEX_SYMBOL_KIND_UNUSED5, what
> Simon> does it mean?
>
> I thought it was reasonably well explained in the comments.
> Could you say which part needs more text to be clear?
>
>
> + /* We sort the indexes in a funny way: GDB_INDEX_SYMBOL_KIND_UNUSED5
> + is always sorted last; then otherwise we sort by numeric value.
> + This ensures that we prefer the definition when both a definition
> + and a declaration (stub type) are seen. */
> ...
> + /* Rewrite GDB_INDEX_SYMBOL_KIND_UNUSED5. This ensures that a type
> + declaration will be deleted by the subsequent squashing step, if
> + warranted. */
> ...
> + /* If we added a type declaration, we want to note this
> + fact for later, because we don't want a type declaration
> + to cause the real definition to be omitted from the
> + index. GDB_INDEX_SYMBOL_KIND_UNUSED5 is used here, but
> + rewritten later before the index is written. */
I think this is clear enough once you know the structure of the code in
this file (which I didn't know beforehand). It doesn't help that I keep
trying to review this series late at night, when my capacity to learn
new things it near zero...
Simon
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH v2 03/28] Emit some type declarations in .gdb_index
2025-04-23 4:11 ` Simon Marchi
@ 2025-04-23 20:54 ` Tom Tromey
0 siblings, 0 replies; 50+ messages in thread
From: Tom Tromey @ 2025-04-23 20:54 UTC (permalink / raw)
To: Simon Marchi; +Cc: Tom Tromey, gdb-patches
>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:
Simon> I think this is clear enough once you know the structure of the code in
Simon> this file (which I didn't know beforehand). It doesn't help that I keep
Simon> trying to review this series late at night, when my capacity to learn
Simon> new things it near zero...
No worries.
One thought I had is I could add a new #define alias for this constant,
then use that through the file. That would provide a spot to write an
overview comment.
Tom
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH v2 04/28] Ada import functions not in index
2025-04-02 23:44 [PATCH v2 00/28] Search symbols via quick API Tom Tromey
` (2 preceding siblings ...)
2025-04-02 23:45 ` [PATCH v2 03/28] Emit some type declarations in .gdb_index Tom Tromey
@ 2025-04-02 23:45 ` Tom Tromey
2025-04-02 23:45 ` [PATCH v2 05/28] Fix index's handling of DW_TAG_imported_declaration Tom Tromey
` (25 subsequent siblings)
29 siblings, 0 replies; 50+ messages in thread
From: Tom Tromey @ 2025-04-02 23:45 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
The cooked index does not currently contain entries for Ada import
functions. This means that whether or not these are visible to
"break" depends on which CUs were previously expanded -- clearly a
bug.
This patch fixes the issue. I think the comments in the patch explain
the fix reasonably well.
Perhaps one to-do item here is to change GNAT to use
DW_TAG_imported_declaration for these imports. This may eventually
let us remove some of the current hacks.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32511
---
gdb/dwarf2/abbrev.c | 7 ++++++-
gdb/dwarf2/abbrev.h | 8 ++++++++
gdb/dwarf2/cooked-indexer.c | 6 ++++--
gdb/dwarf2/read.c | 9 +++------
gdb/dwarf2/read.h | 6 ++++++
gdb/testsuite/gdb.ada/import.exp | 28 ++++++++++++++--------------
6 files changed, 41 insertions(+), 23 deletions(-)
diff --git a/gdb/dwarf2/abbrev.c b/gdb/dwarf2/abbrev.c
index 9f7ead88f1fa717985004709e73aa8e916acb1e1..5b7e99abd24ead974595993f72c7452ad0d2ce4c 100644
--- a/gdb/dwarf2/abbrev.c
+++ b/gdb/dwarf2/abbrev.c
@@ -242,7 +242,12 @@ abbrev_table::read (struct dwarf2_section_info *section,
}
else if (has_hardcoded_declaration
&& (cur_abbrev->tag != DW_TAG_variable || !has_external))
- cur_abbrev->interesting = false;
+ {
+ cur_abbrev->interesting = false;
+ if (cur_abbrev->tag == DW_TAG_subprogram && has_name
+ && has_linkage_name)
+ cur_abbrev->maybe_ada_import = true;
+ }
else if (!tag_interesting_for_index (cur_abbrev->tag))
cur_abbrev->interesting = false;
else
diff --git a/gdb/dwarf2/abbrev.h b/gdb/dwarf2/abbrev.h
index 29914f9c86dfa46bbab90f5e87a7c248760d942d..41e60adb3aaafb9e70b525cc9fb671f7846a7168 100644
--- a/gdb/dwarf2/abbrev.h
+++ b/gdb/dwarf2/abbrev.h
@@ -51,6 +51,14 @@ struct abbrev_info
/* True if the DIE has children. */
bool has_children;
bool interesting;
+ /* In Ada, an imported subprogram DIE will be marked as a
+ declaration, but will have both a name and a linkage name. This
+ declaration may be the only spot where that name is associated
+ with an object, so it has to show up in the index. But, because
+ abbrevs are CU-independent, we can't check the language when
+ computing them and instead we keep a separate flag to indicate
+ that the scanner should check this DIE. */
+ bool maybe_ada_import;
unsigned short size_if_constant;
unsigned short sibling_offset;
/* Number of attributes. */
diff --git a/gdb/dwarf2/cooked-indexer.c b/gdb/dwarf2/cooked-indexer.c
index 1f3a2357958f04e75ba4674c17464ddc14b64dab..7b9ffa79fe8fd6575a7d8c57424895073e790fba 100644
--- a/gdb/dwarf2/cooked-indexer.c
+++ b/gdb/dwarf2/cooked-indexer.c
@@ -20,6 +20,7 @@
#include "dwarf2/cooked-indexer.h"
#include "dwarf2/cooked-index-worker.h"
#include "dwarf2/error.h"
+#include "dwarf2/read.h"
/* See cooked-indexer.h. */
@@ -301,7 +302,7 @@ cooked_indexer::scan_attributes (dwarf2_per_cu *scanning_per_cu,
|| abbrev->tag == DW_TAG_namespace)
&& abbrev->has_children)
*flags |= IS_TYPE_DECLARATION;
- else
+ else if (!is_ada_import_or_export (reader->cu (), *name, *linkage_name))
{
*linkage_name = nullptr;
*name = nullptr;
@@ -514,7 +515,8 @@ cooked_indexer::index_dies (cutu_reader *reader,
/* If a DIE parent is a DW_TAG_subprogram, then the DIE is only
interesting if it's a DW_TAG_subprogram or a DW_TAG_entry_point. */
bool die_interesting
- = (abbrev->interesting
+ = ((abbrev->interesting
+ || (m_language == language_ada && abbrev->maybe_ada_import))
&& (parent_entry == nullptr
|| parent_entry->tag != DW_TAG_subprogram
|| abbrev->tag == DW_TAG_subprogram
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 17f06ff134220d7753e7d9ac25e424965e9e55ed..ae8369266be1b1d7e5cc99cb1f44966a2392ec2b 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -973,9 +973,6 @@ static void queue_comp_unit (dwarf2_per_cu *per_cu,
static void process_queue (dwarf2_per_objfile *per_objfile);
-static bool is_ada_import_or_export (dwarf2_cu *cu, const char *name,
- const char *linkagename);
-
/* Class, the destructor of which frees all allocated queue entries. This
will only have work to do if an error was thrown while processing the
dwarf. If no error was thrown then the queue entries should have all
@@ -16627,14 +16624,14 @@ add_ada_export_symbol (struct symbol *orig, const char *new_name,
add_symbol_to_list (copy, list_to_add);
}
-/* A helper function that decides if a given symbol is an Ada Pragma
- Import or Pragma Export. */
+/* See read.h. */
-static bool
+bool
is_ada_import_or_export (dwarf2_cu *cu, const char *name,
const char *linkagename)
{
return (cu->lang () == language_ada
+ && name != nullptr
&& linkagename != nullptr
&& !streq (name, linkagename)
/* The following exclusions are necessary because symbols
diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h
index 7f56dac32f186a321978190c388885999a7ea3fd..c39945d7da4e5a4bd33292849437bcbe396fbb0a 100644
--- a/gdb/dwarf2/read.h
+++ b/gdb/dwarf2/read.h
@@ -1299,4 +1299,10 @@ extern int dwarf2_ranges_read (unsigned offset, unrelocated_addr *low_return,
extern file_and_directory &find_file_and_directory (die_info *die,
dwarf2_cu *cu);
+/* A helper function that decides if a given symbol is an Ada Pragma
+ Import or Pragma Export. */
+
+extern bool is_ada_import_or_export (dwarf2_cu *cu, const char *name,
+ const char *linkagename);
+
#endif /* GDB_DWARF2_READ_H */
diff --git a/gdb/testsuite/gdb.ada/import.exp b/gdb/testsuite/gdb.ada/import.exp
index 04bb7e1a0d9f18e732855114b008a03348235086..6f1026f545fcfd5bc19d1f6e72321d05ffe12960 100644
--- a/gdb/testsuite/gdb.ada/import.exp
+++ b/gdb/testsuite/gdb.ada/import.exp
@@ -54,6 +54,9 @@ gdb_test "print pkg.imported_var_ada" " = 42"
gdb_test "print pkg.exported_var_ada" " = 99"
gdb_test "print exported_var_ada" " = 99"
+gdb_breakpoint "local_imported_func" message
+gdb_test "print copy" " = 42"
+
# This passes with gcc 10 but fails with gcc 9. With gcc 9, we have:
# <1><1659>: Abbrev Number: 4 (DW_TAG_subprogram)
# <165a> DW_AT_external : 1
@@ -76,19 +79,16 @@ gdb_test "print exported_var_ada" " = 99"
# The fact that things start to work when adding the DW_AT_declaration is
# consistent with what is described in commit ff9baa5f1c5, so xfail this
# (without pinpointing it to a specific gcc PR or commit).
-if { [gcc_major_version] < 10 } {
- setup_xfail *-*-*
-}
-gdb_breakpoint "pkg.imported_func_ada" message
-gdb_breakpoint "imported_func" message
-if { [gcc_major_version] < 10 } {
- setup_xfail *-*-*
+foreach func {"pkg.imported_func_ada" "imported_func"} {
+ clean_restart $testfile
+ if { [gcc_major_version] < 10 } {
+ setup_xfail *-*-*
+ }
+ gdb_breakpoint $func message
}
-gdb_breakpoint "imported_func_ada" message
-gdb_breakpoint "local_imported_func" message
-gdb_breakpoint "pkg.exported_func_ada" message
-gdb_breakpoint "exported_func_ada" message
-gdb_breakpoint "exported_func" message
-
-gdb_test "print copy" " = 42"
+foreach func {"imported_func_ada" "pkg.exported_func_ada" \
+ "exported_func_ada" "exported_func"} {
+ clean_restart $testfile
+ gdb_breakpoint $func message
+}
--
2.46.1
^ permalink raw reply [flat|nested] 50+ messages in thread* [PATCH v2 05/28] Fix index's handling of DW_TAG_imported_declaration
2025-04-02 23:44 [PATCH v2 00/28] Search symbols via quick API Tom Tromey
` (3 preceding siblings ...)
2025-04-02 23:45 ` [PATCH v2 04/28] Ada import functions not in index Tom Tromey
@ 2025-04-02 23:45 ` Tom Tromey
2025-04-02 23:45 ` [PATCH v2 06/28] Put all CTF symbols in global scope Tom Tromey
` (24 subsequent siblings)
29 siblings, 0 replies; 50+ messages in thread
From: Tom Tromey @ 2025-04-02 23:45 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
Currently the full symbol reader puts DW_TAG_imported_declaration in
TYPE_DOMAIN, in the global scope. This patch changes the cooked
indexer to follow.
Without this patch, a later patch in the series would cause
nsalias.exp to regress.
This also updates read-gdb-index.c to do something similar.
---
gdb/dwarf2/cooked-indexer.c | 5 +++++
gdb/dwarf2/read-gdb-index.c | 2 +-
gdb/dwarf2/tag.h | 4 ++++
3 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/gdb/dwarf2/cooked-indexer.c b/gdb/dwarf2/cooked-indexer.c
index 7b9ffa79fe8fd6575a7d8c57424895073e790fba..bf9f1384781c54e067cb7e8441a5900613207a20 100644
--- a/gdb/dwarf2/cooked-indexer.c
+++ b/gdb/dwarf2/cooked-indexer.c
@@ -552,6 +552,11 @@ cooked_indexer::index_dies (cutu_reader *reader,
flags &= ~IS_STATIC;
flags |= parent_entry->flags & IS_STATIC;
}
+ else if (abbrev->tag == DW_TAG_imported_declaration)
+ {
+ /* Match the full reader. */
+ flags &= ~IS_STATIC;
+ }
if (abbrev->tag == DW_TAG_namespace
&& m_language == language_cplus
diff --git a/gdb/dwarf2/read-gdb-index.c b/gdb/dwarf2/read-gdb-index.c
index 343916309d7c4ca1072d3b11e4fa144d5ec435f4..eeaa38a502c09a0acdd1b60a1c0b8403843aa9fe 100644
--- a/gdb/dwarf2/read-gdb-index.c
+++ b/gdb/dwarf2/read-gdb-index.c
@@ -1098,7 +1098,7 @@ dw2_expand_marked_cus (dwarf2_per_objfile *per_objfile, offset_type idx,
mask = SEARCH_TYPE_DOMAIN | SEARCH_STRUCT_DOMAIN;
break;
case GDB_INDEX_SYMBOL_KIND_OTHER:
- mask = SEARCH_MODULE_DOMAIN;
+ mask = SEARCH_MODULE_DOMAIN | SEARCH_TYPE_DOMAIN;
break;
}
if ((kind & mask) == 0)
diff --git a/gdb/dwarf2/tag.h b/gdb/dwarf2/tag.h
index 865c2bc109d1bf41aa4365a5cf9bda66a1ced65b..ef1ad217c6d34fbacdf5f6372b0d29ad3b6e7bbd 100644
--- a/gdb/dwarf2/tag.h
+++ b/gdb/dwarf2/tag.h
@@ -102,6 +102,10 @@ tag_matches_domain (dwarf_tag tag, domain_search_flags search, language lang)
}
break;
+ case DW_TAG_imported_declaration:
+ /* DW_TAG_imported_declaration isn't necessarily a type, but the
+ scanner doesn't track the referent, and the full reader
+ also currently puts it in TYPE_DOMAIN. */
case DW_TAG_padding:
case DW_TAG_array_type:
case DW_TAG_pointer_type:
--
2.46.1
^ permalink raw reply [flat|nested] 50+ messages in thread* [PATCH v2 06/28] Put all CTF symbols in global scope
2025-04-02 23:44 [PATCH v2 00/28] Search symbols via quick API Tom Tromey
` (4 preceding siblings ...)
2025-04-02 23:45 ` [PATCH v2 05/28] Fix index's handling of DW_TAG_imported_declaration Tom Tromey
@ 2025-04-02 23:45 ` Tom Tromey
2025-04-02 23:45 ` [PATCH v2 07/28] Restore "ingestion" of .debug_str when writing .debug_names Tom Tromey
` (23 subsequent siblings)
29 siblings, 0 replies; 50+ messages in thread
From: Tom Tromey @ 2025-04-02 23:45 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
The new approach to searching (solely via the quick API) is more
sensitive to discrepancies between the partial and full readers. In
CTF, there is some disagreement about which scope to use. CTF doesn't
seem to really distinguish between the file and global scope, so this
patch takes the simple approach of putting all CTF symbols into the
global scope.
This changes one test as well. It seems to me that the behavior here
is arbitrary and the test is making unwarranted assumptions.
---
gdb/ctfread.c | 6 +++---
gdb/testsuite/gdb.ctf/cross-tu-cyclic.exp | 4 ++--
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/gdb/ctfread.c b/gdb/ctfread.c
index ee7c30f7d8737cc38aa7fae54d339438ae45377f..b9f8d7350538b52076f0fb91a18c8d6139701d32 100644
--- a/gdb/ctfread.c
+++ b/gdb/ctfread.c
@@ -516,7 +516,7 @@ new_symbol (struct ctf_context *ccp, struct type *type, ctf_id_t tid)
break;
}
- add_symbol_to_list (sym, ccp->builder->get_file_symbols ());
+ add_symbol_to_list (sym, ccp->builder->get_global_symbols ());
}
return sym;
@@ -1171,7 +1171,7 @@ ctf_add_var_cb (const char *name, ctf_id_t id, void *arg)
sym->set_domain (VAR_DOMAIN);
sym->set_aclass_index (LOC_OPTIMIZED_OUT);
sym->compute_and_set_names (name, false, ccp->of->per_bfd);
- add_symbol_to_list (sym, ccp->builder->get_file_symbols ());
+ add_symbol_to_list (sym, ccp->builder->get_global_symbols ());
break;
default:
complaint (_("ctf_add_var_cb: kind unsupported (%d)"), kind);
@@ -1510,7 +1510,7 @@ ctf_psymtab_type_cb (ctf_id_t tid, void *arg)
ccp->pst->add_psymbol (name, false,
domain, aclass, section,
- psymbol_placement::STATIC,
+ psymbol_placement::GLOBAL,
unrelocated_addr (0),
language_c, ccp->partial_symtabs, ccp->of);
diff --git a/gdb/testsuite/gdb.ctf/cross-tu-cyclic.exp b/gdb/testsuite/gdb.ctf/cross-tu-cyclic.exp
index 7c6cb0d3bf14759f3fa40ab3926b0da8d7988dac..06242d3e74d4c8586826460d46266cb25406aa68 100644
--- a/gdb/testsuite/gdb.ctf/cross-tu-cyclic.exp
+++ b/gdb/testsuite/gdb.ctf/cross-tu-cyclic.exp
@@ -37,6 +37,6 @@ gdb_test_no_output "set always-read-ctf on"
gdb_load $binfile
# Same thing with struct and union.
-gdb_test "ptype struct A" "type = struct A \{\[\r\n\]+\[ \t\]+struct B \\*foo;\[\r\n\]+\}.*" "ptype structure A"
-gdb_test "ptype struct B" "type = struct B \{\[\r\n\]+\[ \t\]+struct B \\*next;\[\r\n\]+\}.*" "ptype structure B"
+gdb_test "ptype struct A" "type = struct A \{\[\r\n\]+\[ \t\]+long a;\[\r\n\]+\[ \t\]+struct B \\*foo;\[\r\n\]+\}.*" "ptype structure A"
+gdb_test "ptype struct B" "type = struct B \{\[\r\n\]+\[ \t\]+int foo;\[\r\n\]+\[ \t\]+struct A \\*bar;\[\r\n\]+\}.*" "ptype structure B"
gdb_test "ptype struct C" "type = struct C \{\[\r\n\]+\[ \t\]+struct B \\*foo;\[\r\n\]+\[ \t\]+int b;\[\r\n\]+\}.*" "ptype structure C"
--
2.46.1
^ permalink raw reply [flat|nested] 50+ messages in thread* [PATCH v2 07/28] Restore "ingestion" of .debug_str when writing .debug_names
2025-04-02 23:44 [PATCH v2 00/28] Search symbols via quick API Tom Tromey
` (5 preceding siblings ...)
2025-04-02 23:45 ` [PATCH v2 06/28] Put all CTF symbols in global scope Tom Tromey
@ 2025-04-02 23:45 ` Tom Tromey
2025-04-02 23:45 ` [PATCH v2 08/28] Entries from anon-struct.exp not in cooked index Tom Tromey
` (22 subsequent siblings)
29 siblings, 0 replies; 50+ messages in thread
From: Tom Tromey @ 2025-04-02 23:45 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
When I rewrote the .debug_names writer (commit 91a42a61), I changed
the writer to not import .debug_str into the debug_str_lookup object.
However, a later patch in this series needed this again. The issue
here was that if a name occurs in the DWARF, and is also allocated,
then there is a race, where the created index depends on which DIE is
read first. This can cause index-file.exp failures.
This patch restores the old approach, avoiding this problem. I also
applied a couple of small cleanups to the class. And, I removed the
old complaint from the "ingestion" function, as this was not
necessary.
---
gdb/dwarf2/index-write.c | 24 ++++++++++++++----------
1 file changed, 14 insertions(+), 10 deletions(-)
diff --git a/gdb/dwarf2/index-write.c b/gdb/dwarf2/index-write.c
index 389fcaa727ce15706a735ce82c047374fbc57d2b..8945b844e2a9e08049902decde2bb6ab7366b1be 100644
--- a/gdb/dwarf2/index-write.c
+++ b/gdb/dwarf2/index-write.c
@@ -763,7 +763,7 @@ class debug_names
});
m_name_table_string_offs.push_back_reorder
- (m_debugstrlookup.lookup (name.c_str ())); /* ??? */
+ (m_debugstrlookup.lookup (name.c_str ()));
m_name_table_entry_offs.push_back_reorder (m_entry_pool.size ());
for (const cooked_index_entry *entry : these_entries)
@@ -928,10 +928,21 @@ class debug_names
{
public:
- /* Object constructor to be called for current DWARF2_PER_BFD. */
- debug_str_lookup (dwarf2_per_bfd *per_bfd)
+ /* Object constructor to be called for current DWARF2_PER_BFD.
+ All .debug_str section strings are automatically stored. */
+ explicit debug_str_lookup (dwarf2_per_bfd *per_bfd)
: m_per_bfd (per_bfd)
{
+ gdb_assert (per_bfd->str.readin);
+ const gdb_byte *data = per_bfd->str.buffer;
+ if (data == nullptr)
+ return;
+ while (data < per_bfd->str.buffer + per_bfd->str.size)
+ {
+ const char *const s = reinterpret_cast<const char *> (data);
+ m_str_table.emplace (c_str_view (s), data - per_bfd->str.buffer);
+ data += strlen (s) + 1;
+ }
}
/* Return offset of symbol name S in the .debug_str section. Add
@@ -939,13 +950,6 @@ class debug_names
yet. */
size_t lookup (const char *s)
{
- /* Most strings will have come from the string table
- already. */
- const gdb_byte *b = (const gdb_byte *) s;
- if (b >= m_per_bfd->str.buffer
- && b < m_per_bfd->str.buffer + m_per_bfd->str.size)
- return b - m_per_bfd->str.buffer;
-
const auto it = m_str_table.find (c_str_view (s));
if (it != m_str_table.end ())
return it->second;
--
2.46.1
^ permalink raw reply [flat|nested] 50+ messages in thread* [PATCH v2 08/28] Entries from anon-struct.exp not in cooked index
2025-04-02 23:44 [PATCH v2 00/28] Search symbols via quick API Tom Tromey
` (6 preceding siblings ...)
2025-04-02 23:45 ` [PATCH v2 07/28] Restore "ingestion" of .debug_str when writing .debug_names Tom Tromey
@ 2025-04-02 23:45 ` Tom Tromey
2025-04-02 23:45 ` [PATCH v2 09/28] Remove dwarf2_per_cu_data::mark Tom Tromey
` (21 subsequent siblings)
29 siblings, 0 replies; 50+ messages in thread
From: Tom Tromey @ 2025-04-02 23:45 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
g++ will sometimes use a typedef to give a name to an otherwise
anonymous type for linkage purposes. gdb tries to handle this odd
scenario, which is enforced by anon-struct.exp.
It's difficult to detect this problem in the current tree, but the
cooked index does not include an entry for these DIEs.
This patch changes gdb to add these to the index. This is needed by
subsequent changes in this series.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32519
---
gdb/dwarf2/cooked-index-shard.h | 7 +++++++
gdb/dwarf2/cooked-index-worker.h | 7 +++++++
gdb/dwarf2/cooked-indexer.c | 26 ++++++++++++++++++++++++++
3 files changed, 40 insertions(+)
diff --git a/gdb/dwarf2/cooked-index-shard.h b/gdb/dwarf2/cooked-index-shard.h
index eb809264cbb80612c864bcf4c04c68ff5d4e99fb..14f5c588c15b6a7ac6d558caabfd9a84b071895f 100644
--- a/gdb/dwarf2/cooked-index-shard.h
+++ b/gdb/dwarf2/cooked-index-shard.h
@@ -48,6 +48,13 @@ class cooked_index_shard
cooked_index_entry_ref parent_entry,
dwarf2_per_cu *per_cu);
+ /* Add a copy of NAME to the index. Return a pointer to the
+ copy. */
+ const char *add (std::string_view name)
+ {
+ return m_names.insert (name);
+ }
+
/* Install a new fixed addrmap from the given mutable addrmap. */
void install_addrmap (addrmap_mutable *map)
{
diff --git a/gdb/dwarf2/cooked-index-worker.h b/gdb/dwarf2/cooked-index-worker.h
index df5c31d572dfe89c067c1118d34e7d07e8a084aa..c6e005c14dea1bb891680969c17172d494a7de46 100644
--- a/gdb/dwarf2/cooked-index-worker.h
+++ b/gdb/dwarf2/cooked-index-worker.h
@@ -85,6 +85,13 @@ class cooked_index_worker_result
name, parent_entry, per_cu);
}
+ /* Add a copy of NAME to the index. Return a pointer to the
+ copy. */
+ const char *add (std::string_view name)
+ {
+ return m_shard->add (name);
+ }
+
/* Install the current addrmap into the shard being constructed,
then transfer ownership of the index to the caller. */
cooked_index_shard_up release_shard ()
diff --git a/gdb/dwarf2/cooked-indexer.c b/gdb/dwarf2/cooked-indexer.c
index bf9f1384781c54e067cb7e8441a5900613207a20..473dd451a704541668c91a26ad709daae1aa5343 100644
--- a/gdb/dwarf2/cooked-indexer.c
+++ b/gdb/dwarf2/cooked-indexer.c
@@ -21,6 +21,8 @@
#include "dwarf2/cooked-index-worker.h"
#include "dwarf2/error.h"
#include "dwarf2/read.h"
+#include "cp-support.h"
+#include "demangle.h"
/* See cooked-indexer.h. */
@@ -569,6 +571,30 @@ cooked_indexer::index_dies (cutu_reader *reader,
name = nullptr;
}
+ /* An otherwise anonymous type might be given a name (via
+ typedef) for linkage purposes, and gdb tries to handle this
+ case. See anon-struct.exp. In this case, GCC will emit a
+ funny thing: a linkage name for the type, but in "type" form.
+ That is, it will not start with _Z. */
+ if ((abbrev->tag == DW_TAG_class_type
+ || abbrev->tag == DW_TAG_structure_type
+ || abbrev->tag == DW_TAG_union_type)
+ && m_language == language_cplus
+ && name == nullptr
+ && linkage_name != nullptr)
+ {
+ gdb::unique_xmalloc_ptr<char> dem
+ = gdb_demangle (linkage_name, DMGL_GNU_V3 | DMGL_TYPES);
+ if (dem != nullptr)
+ {
+ /* We're only interested in the last component. */
+ std::vector<std::string_view> split
+ = split_name (dem.get (), split_style::CXX);
+ name = m_index_storage->add (split.back ());
+ linkage_name = nullptr;
+ }
+ }
+
cooked_index_entry *this_entry = nullptr;
if (name != nullptr)
{
--
2.46.1
^ permalink raw reply [flat|nested] 50+ messages in thread* [PATCH v2 09/28] Remove dwarf2_per_cu_data::mark
2025-04-02 23:44 [PATCH v2 00/28] Search symbols via quick API Tom Tromey
` (7 preceding siblings ...)
2025-04-02 23:45 ` [PATCH v2 08/28] Entries from anon-struct.exp not in cooked index Tom Tromey
@ 2025-04-02 23:45 ` Tom Tromey
2025-04-21 3:09 ` Simon Marchi
2025-04-02 23:45 ` [PATCH v2 10/28] Have expand_symtabs_matching work for already-expanded CUs Tom Tromey
` (20 subsequent siblings)
29 siblings, 1 reply; 50+ messages in thread
From: Tom Tromey @ 2025-04-02 23:45 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This removes dwarf2_per_cu_data::mark, replacing it with a
locally-allocated boolean vector. It also inverts the sense of the
flag -- now, the flag is true when a CU should be skipped, and false
when the CU should be further examined. Also, the validity of the
flag is no longer dependent on 'file_matcher != NULL'.
This patch makes the subsequent patch to searching a bit simpler, so
I've separated it out.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=16994
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=16998
---
gdb/dwarf2/read-gdb-index.c | 14 +++++-----
gdb/dwarf2/read.c | 64 ++++++++++++++++++++++++++-------------------
gdb/dwarf2/read.h | 35 +++++++++++++++++++++----
3 files changed, 75 insertions(+), 38 deletions(-)
diff --git a/gdb/dwarf2/read-gdb-index.c b/gdb/dwarf2/read-gdb-index.c
index eeaa38a502c09a0acdd1b60a1c0b8403843aa9fe..7bcf50d347609f0b27068228cd78633dd57e1556 100644
--- a/gdb/dwarf2/read-gdb-index.c
+++ b/gdb/dwarf2/read-gdb-index.c
@@ -1030,6 +1030,7 @@ dwarf2_gdb_index::dump (struct objfile *objfile)
static bool
dw2_expand_marked_cus (dwarf2_per_objfile *per_objfile, offset_type idx,
+ auto_bool_vector &marked,
expand_symtabs_file_matcher file_matcher,
expand_symtabs_expansion_listener expansion_notify,
block_search_flags search_flags,
@@ -1114,9 +1115,9 @@ dw2_expand_marked_cus (dwarf2_per_objfile *per_objfile, offset_type idx,
}
dwarf2_per_cu *per_cu = per_objfile->per_bfd->get_cu (cu_index);
-
- if (!dw2_expand_symtabs_matching_one (per_cu, per_objfile, file_matcher,
- expansion_notify, lang_matcher))
+ if (!dw2_expand_symtabs_matching_one (per_cu, per_objfile, marked,
+ file_matcher, expansion_notify,
+ lang_matcher))
return false;
}
@@ -1136,7 +1137,8 @@ dwarf2_gdb_index::expand_symtabs_matching
{
dwarf2_per_objfile *per_objfile = get_dwarf2_per_objfile (objfile);
- dw_expand_symtabs_matching_file_matcher (per_objfile, file_matcher);
+ auto_bool_vector marked;
+ dw_expand_symtabs_matching_file_matcher (per_objfile, marked, file_matcher);
/* This invariant is documented in quick-functions.h. */
gdb_assert (lookup_name != nullptr || symbol_matcher == nullptr);
@@ -1147,7 +1149,7 @@ dwarf2_gdb_index::expand_symtabs_matching
QUIT;
if (!dw2_expand_symtabs_matching_one (per_cu, per_objfile,
- file_matcher,
+ marked, file_matcher,
expansion_notify,
lang_matcher))
return false;
@@ -1164,7 +1166,7 @@ dwarf2_gdb_index::expand_symtabs_matching
symbol_matcher,
[&] (offset_type idx)
{
- if (!dw2_expand_marked_cus (per_objfile, idx, file_matcher,
+ if (!dw2_expand_marked_cus (per_objfile, idx, marked, file_matcher,
expansion_notify, search_flags, domain,
lang_matcher))
return false;
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index ae8369266be1b1d7e5cc99cb1f44966a2392ec2b..4b971173815f439ad26da4f78c6aadd0cd18446b 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -1913,11 +1913,13 @@ bool
dw2_expand_symtabs_matching_one
(dwarf2_per_cu *per_cu,
dwarf2_per_objfile *per_objfile,
+ auto_bool_vector &marked,
expand_symtabs_file_matcher file_matcher,
expand_symtabs_expansion_listener expansion_notify,
expand_symtabs_lang_matcher lang_matcher)
{
- if (file_matcher != nullptr && !per_cu->mark)
+ /* Already visited, or intentionally skipped. */
+ if (marked.is_set (per_cu->index))
return true;
if (lang_matcher != nullptr)
@@ -1944,7 +1946,9 @@ dw2_expand_symtabs_matching_one
void
dw_expand_symtabs_matching_file_matcher
- (dwarf2_per_objfile *per_objfile, expand_symtabs_file_matcher file_matcher)
+ (dwarf2_per_objfile *per_objfile,
+ auto_bool_vector &marked,
+ expand_symtabs_file_matcher file_matcher)
{
if (file_matcher == NULL)
return;
@@ -1960,54 +1964,57 @@ dw_expand_symtabs_matching_file_matcher
QUIT;
if (per_cu->is_debug_types)
- continue;
- per_cu->mark = 0;
+ {
+ marked.set (per_cu->index, true);
+ continue;
+ }
/* We only need to look at symtabs not already expanded. */
if (per_objfile->symtab_set_p (per_cu.get ()))
- continue;
+ {
+ marked.set (per_cu->index, true);
+ continue;
+ }
if (per_cu->fnd != nullptr)
{
file_and_directory *fnd = per_cu->fnd.get ();
if (file_matcher (fnd->get_name (), false))
- {
- per_cu->mark = 1;
- continue;
- }
+ continue;
/* Before we invoke realpath, which can get expensive when many
files are involved, do a quick comparison of the basenames. */
if ((basenames_may_differ
|| file_matcher (lbasename (fnd->get_name ()), true))
&& file_matcher (fnd->get_fullname (), false))
- {
- per_cu->mark = 1;
- continue;
- }
+ continue;
}
quick_file_names *file_data = dw2_get_file_names (per_cu.get (),
per_objfile);
if (file_data == NULL)
- continue;
+ {
+ marked.set (per_cu->index, true);
+ continue;
+ }
if (visited_not_found.contains (file_data))
- continue;
- else if (visited_found.contains (file_data))
{
- per_cu->mark = 1;
+ marked.set (per_cu->index, true);
continue;
}
+ else if (visited_found.contains (file_data))
+ continue;
+ bool matched = false;
for (int j = 0; j < file_data->num_file_names; ++j)
{
const char *this_real_name;
if (file_matcher (file_data->file_names[j], false))
{
- per_cu->mark = 1;
+ matched = true;
break;
}
@@ -2021,15 +2028,18 @@ dw_expand_symtabs_matching_file_matcher
this_real_name = dw2_get_real_path (per_objfile, file_data, j);
if (file_matcher (this_real_name, false))
{
- per_cu->mark = 1;
+ matched = true;
break;
}
}
- if (per_cu->mark)
+ if (matched)
visited_found.insert (file_data);
else
- visited_not_found.insert (file_data);
+ {
+ marked.set (per_cu->index, true);
+ visited_not_found.insert (file_data);
+ }
}
}
@@ -14435,7 +14445,8 @@ cooked_index_functions::expand_symtabs_matching
cooked_index *table = wait (objfile, true);
- dw_expand_symtabs_matching_file_matcher (per_objfile, file_matcher);
+ auto_bool_vector marked;
+ dw_expand_symtabs_matching_file_matcher (per_objfile, marked, file_matcher);
/* This invariant is documented in quick-functions.h. */
gdb_assert (lookup_name != nullptr || symbol_matcher == nullptr);
@@ -14446,7 +14457,7 @@ cooked_index_functions::expand_symtabs_matching
QUIT;
if (!dw2_expand_symtabs_matching_one (per_cu, per_objfile,
- file_matcher,
+ marked, file_matcher,
expansion_notify,
lang_matcher))
return false;
@@ -14527,9 +14538,8 @@ cooked_index_functions::expand_symtabs_matching
if (per_objfile->symtab_set_p (entry->per_cu))
continue;
- /* If file-matching was done, we don't need to consider
- symbols from unmarked CUs. */
- if (file_matcher != nullptr && !entry->per_cu->mark)
+ /* We don't need to consider symbols from some CUs. */
+ if (marked.is_set (entry->per_cu->index))
continue;
/* See if the symbol matches the type filter. */
@@ -14614,7 +14624,7 @@ cooked_index_functions::expand_symtabs_matching
}
if (!dw2_expand_symtabs_matching_one (entry->per_cu, per_objfile,
- file_matcher,
+ marked, file_matcher,
expansion_notify, nullptr))
return false;
}
diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h
index c39945d7da4e5a4bd33292849437bcbe396fbb0a..6cc08993a3ce7cdfa8b80350e8f0b965ed69b3e3 100644
--- a/gdb/dwarf2/read.h
+++ b/gdb/dwarf2/read.h
@@ -125,7 +125,6 @@ struct dwarf2_per_cu
lto_artificial (false),
queued (false),
m_header_read_in (false),
- mark (false),
files_read (false),
scanned (false),
section (section),
@@ -193,10 +192,6 @@ struct dwarf2_per_cu
it private at the moment. */
mutable packed<bool, 1> m_header_read_in;
- /* A temporary mark bit used when iterating over all CUs in
- expand_symtabs_matching. */
- packed<unsigned int, 1> mark;
-
/* True if we've tried to read the file table. There will be no
point in trying to read it again next time. */
packed<bool, 1> files_read;
@@ -1179,6 +1174,34 @@ struct dwarf2_base_index_functions : public quick_symbol_functions
bool need_fullname) override;
};
+/* This is used to track whether a CU has already been visited during
+ symbol expansion. It is an auto-resizing bool vector. */
+class auto_bool_vector
+{
+public:
+
+ auto_bool_vector () = default;
+
+ /* Return true if element I is set. */
+ bool is_set (size_t i) const
+ {
+ if (i < m_vec.size ())
+ return m_vec[i];
+ return false;
+ }
+
+ /* Set a value in this vector, growing it automatically. */
+ void set (size_t i, bool value)
+ {
+ if (m_vec.size () < i + 1)
+ m_vec.resize (i + 1);
+ m_vec[i] = value;
+ }
+
+private:
+ std::vector<bool> m_vec;
+};
+
/* If FILE_MATCHER is NULL or if PER_CU has
dwarf2_per_cu_quick_data::MARK set (see
dw_expand_symtabs_matching_file_matcher), expand the CU and call
@@ -1187,6 +1210,7 @@ struct dwarf2_base_index_functions : public quick_symbol_functions
extern bool dw2_expand_symtabs_matching_one
(dwarf2_per_cu *per_cu,
dwarf2_per_objfile *per_objfile,
+ auto_bool_vector &marked,
expand_symtabs_file_matcher file_matcher,
expand_symtabs_expansion_listener expansion_notify,
expand_symtabs_lang_matcher lang_matcher);
@@ -1197,6 +1221,7 @@ extern bool dw2_expand_symtabs_matching_one
extern void dw_expand_symtabs_matching_file_matcher
(dwarf2_per_objfile *per_objfile,
+ auto_bool_vector &marked,
expand_symtabs_file_matcher file_matcher);
/* Return pointer to string at .debug_str offset STR_OFFSET. */
--
2.46.1
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH v2 09/28] Remove dwarf2_per_cu_data::mark
2025-04-02 23:45 ` [PATCH v2 09/28] Remove dwarf2_per_cu_data::mark Tom Tromey
@ 2025-04-21 3:09 ` Simon Marchi
2025-04-21 15:38 ` Tom Tromey
0 siblings, 1 reply; 50+ messages in thread
From: Simon Marchi @ 2025-04-21 3:09 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
On 2025-04-02 19:45, Tom Tromey wrote:
> This removes dwarf2_per_cu_data::mark, replacing it with a
> locally-allocated boolean vector. It also inverts the sense of the
> flag -- now, the flag is true when a CU should be skipped, and false
> when the CU should be further examined. Also, the validity of the
> flag is no longer dependent on 'file_matcher != NULL'.
>
> This patch makes the subsequent patch to searching a bit simpler, so
> I've separated it out.
>
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=16994
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=16998
> ---
> gdb/dwarf2/read-gdb-index.c | 14 +++++-----
> gdb/dwarf2/read.c | 64 ++++++++++++++++++++++++++-------------------
> gdb/dwarf2/read.h | 35 +++++++++++++++++++++----
> 3 files changed, 75 insertions(+), 38 deletions(-)
>
> diff --git a/gdb/dwarf2/read-gdb-index.c b/gdb/dwarf2/read-gdb-index.c
> index eeaa38a502c09a0acdd1b60a1c0b8403843aa9fe..7bcf50d347609f0b27068228cd78633dd57e1556 100644
> --- a/gdb/dwarf2/read-gdb-index.c
> +++ b/gdb/dwarf2/read-gdb-index.c
> @@ -1030,6 +1030,7 @@ dwarf2_gdb_index::dump (struct objfile *objfile)
>
> static bool
> dw2_expand_marked_cus (dwarf2_per_objfile *per_objfile, offset_type idx,
> + auto_bool_vector &marked,
> expand_symtabs_file_matcher file_matcher,
> expand_symtabs_expansion_listener expansion_notify,
> block_search_flags search_flags,
Can you choose a different name for these "marked" parameters? That
name doesn't convey what a mark means. Perhaps "visited_cus", or
something like that.
Also, does the "marked" in "dw2_expand_marked_cus" have the same meaning
as the "marked" that changed meaning in this patch? If so, does this
function need to be renamed (does it do the opposite of the what its
name imply now)?
Simon
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH v2 09/28] Remove dwarf2_per_cu_data::mark
2025-04-21 3:09 ` Simon Marchi
@ 2025-04-21 15:38 ` Tom Tromey
2025-04-23 4:12 ` Simon Marchi
0 siblings, 1 reply; 50+ messages in thread
From: Tom Tromey @ 2025-04-21 15:38 UTC (permalink / raw)
To: Simon Marchi; +Cc: Tom Tromey, gdb-patches
>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:
Simon> Can you choose a different name for these "marked" parameters? That
Simon> name doesn't convey what a mark means. Perhaps "visited_cus", or
Simon> something like that.
I used cus_to_skip.
"visited" won't really be accurate by the end of the series, because by
then a CU that's already expanded will still be searched.
Simon> Also, does the "marked" in "dw2_expand_marked_cus" have the same meaning
Simon> as the "marked" that changed meaning in this patch? If so, does this
Simon> function need to be renamed (does it do the opposite of the what its
Simon> name imply now)?
I think that is what it mean, and furthermore the function seems
mis-named in other ways, since it isn't primarily concerned with marked
CUs so much as deciding which CUs to expand.
However I would rather not bother to rename it at this point, since it's
deleted entirely in a subsequent patch in the series.
Tom
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH v2 09/28] Remove dwarf2_per_cu_data::mark
2025-04-21 15:38 ` Tom Tromey
@ 2025-04-23 4:12 ` Simon Marchi
0 siblings, 0 replies; 50+ messages in thread
From: Simon Marchi @ 2025-04-23 4:12 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
On 2025-04-21 11:38, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:
>
> Simon> Can you choose a different name for these "marked" parameters? That
> Simon> name doesn't convey what a mark means. Perhaps "visited_cus", or
> Simon> something like that.
>
> I used cus_to_skip.
>
> "visited" won't really be accurate by the end of the series, because by
> then a CU that's already expanded will still be searched.
Fine with me, thanks.
> Simon> Also, does the "marked" in "dw2_expand_marked_cus" have the same meaning
> Simon> as the "marked" that changed meaning in this patch? If so, does this
> Simon> function need to be renamed (does it do the opposite of the what its
> Simon> name imply now)?
>
> I think that is what it mean, and furthermore the function seems
> mis-named in other ways, since it isn't primarily concerned with marked
> CUs so much as deciding which CUs to expand.
>
> However I would rather not bother to rename it at this point, since it's
> deleted entirely in a subsequent patch in the series.
Works for me, thanks!.
Simon
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH v2 10/28] Have expand_symtabs_matching work for already-expanded CUs
2025-04-02 23:44 [PATCH v2 00/28] Search symbols via quick API Tom Tromey
` (8 preceding siblings ...)
2025-04-02 23:45 ` [PATCH v2 09/28] Remove dwarf2_per_cu_data::mark Tom Tromey
@ 2025-04-02 23:45 ` Tom Tromey
2025-04-23 15:53 ` Simon Marchi
2025-04-02 23:45 ` [PATCH v2 11/28] Rewrite the .gdb_index reader Tom Tromey
` (19 subsequent siblings)
29 siblings, 1 reply; 50+ messages in thread
From: Tom Tromey @ 2025-04-02 23:45 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
Currently, gdb will search the already-expanded symtabs in one loop,
and then also expand matching symtabs in another loop. However, this
is somewhat inefficient -- when searching the already-expanded
symtabs, all such symtabs are examined. However, the various "quick"
implementations already know which subset of symtabs might have a
match.
This changes the contract of expand_symtabs_matching to also call the
callback for an already-expanded symtab. With this change, the number
of searched symtabs should sometimes be reduced. This also cuts down
on the amount of redundant code.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=16994
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=16998
---
gdb/dwarf2/read.c | 42 ++++++++++++++++++++++++++++--------------
gdb/psymtab.c | 3 ---
2 files changed, 28 insertions(+), 17 deletions(-)
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 4b971173815f439ad26da4f78c6aadd0cd18446b..f1084530221e82f33efb35f542d6d842fe3df2ab 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -1544,6 +1544,23 @@ struct readnow_functions : public dwarf2_base_index_functions
domain_search_flags domain,
expand_symtabs_lang_matcher lang_matcher) override
{
+ dwarf2_per_objfile *per_objfile = get_dwarf2_per_objfile (objfile);
+ auto_bool_vector marked;
+ dw_expand_symtabs_matching_file_matcher (per_objfile, marked,
+ file_matcher);
+
+ for (const auto &per_cu : per_objfile->per_bfd->all_units)
+ {
+ QUIT;
+
+ if (per_cu->is_debug_types)
+ continue;
+ if (!dw2_expand_symtabs_matching_one (per_cu.get (), per_objfile,
+ marked, file_matcher,
+ expansion_notify,
+ lang_matcher))
+ return false;
+ }
return true;
}
};
@@ -1931,13 +1948,15 @@ dw2_expand_symtabs_matching_one
return true;
}
- bool symtab_was_null = !per_objfile->symtab_set_p (per_cu);
compunit_symtab *symtab
= dw2_instantiate_symtab (per_cu, per_objfile, false);
gdb_assert (symtab != nullptr);
- if (expansion_notify != NULL && symtab_was_null)
- return expansion_notify (symtab);
+ if (expansion_notify != nullptr)
+ {
+ marked.set (per_cu->index, true);
+ return expansion_notify (symtab);
+ }
return true;
}
@@ -1969,13 +1988,6 @@ dw_expand_symtabs_matching_file_matcher
continue;
}
- /* We only need to look at symtabs not already expanded. */
- if (per_objfile->symtab_set_p (per_cu.get ()))
- {
- marked.set (per_cu->index, true);
- continue;
- }
-
if (per_cu->fnd != nullptr)
{
file_and_directory *fnd = per_cu->fnd.get ();
@@ -4211,6 +4223,12 @@ load_full_comp_unit (dwarf2_per_cu *this_cu, dwarf2_per_objfile *per_objfile,
if (reader.is_dummy ())
return;
+ /* We always need the file names filled in so that
+ expand_symtabs_matching can match filenames. It's convenient to
+ do this here. */
+ if (!this_cu->files_read)
+ dw2_get_file_names_reader (reader.cu (), reader.top_level_die ());
+
reader.read_all_dies ();
/* Save this dwarf2_cu in the per_objfile. The per_objfile owns it
@@ -14534,10 +14552,6 @@ cooked_index_functions::expand_symtabs_matching
{
QUIT;
- /* No need to consider symbols from expanded CUs. */
- if (per_objfile->symtab_set_p (entry->per_cu))
- continue;
-
/* We don't need to consider symbols from some CUs. */
if (marked.is_set (entry->per_cu->index))
continue;
diff --git a/gdb/psymtab.c b/gdb/psymtab.c
index 28455ba0bc58990a515f0899a873c9995fb4cca8..d12f8f1c5c18018fe96b0d647f3957dfd0f4d594 100644
--- a/gdb/psymtab.c
+++ b/gdb/psymtab.c
@@ -914,9 +914,6 @@ psymbol_functions::expand_symtabs_matching
{
QUIT;
- if (ps->readin_p (objfile))
- continue;
-
if (file_matcher)
{
bool match;
--
2.46.1
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH v2 10/28] Have expand_symtabs_matching work for already-expanded CUs
2025-04-02 23:45 ` [PATCH v2 10/28] Have expand_symtabs_matching work for already-expanded CUs Tom Tromey
@ 2025-04-23 15:53 ` Simon Marchi
2025-04-23 20:39 ` Tom Tromey
0 siblings, 1 reply; 50+ messages in thread
From: Simon Marchi @ 2025-04-23 15:53 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
On 2025-04-02 19:45, Tom Tromey wrote:
> Currently, gdb will search the already-expanded symtabs in one loop,
> and then also expand matching symtabs in another loop. However, this
> is somewhat inefficient -- when searching the already-expanded
> symtabs, all such symtabs are examined. However, the various "quick"
> implementations already know which subset of symtabs might have a
> match.
>
> This changes the contract of expand_symtabs_matching to also call the
> callback for an already-expanded symtab. With this change, the number
> of searched symtabs should sometimes be reduced. This also cuts down
> on the amount of redundant code.
Not particularly important, but I'm not sure I understand how this patch
alone would result in fewer symtabs being searched. I understand that
it's a step towards the ultimate goal of this series, but I suppose you
need the changes to the symbol lookup functions (that come later in this
series) to reduce the number of searched symtabs?
I don't know if it comes later in the series, but it would be good to
do some renaming, because "expand matching symtabs" is no longer
accurate. It's more like "find matching symtab". That can be done later
of course.
> @@ -4211,6 +4223,12 @@ load_full_comp_unit (dwarf2_per_cu *this_cu, dwarf2_per_objfile *per_objfile,
> if (reader.is_dummy ())
> return;
>
> + /* We always need the file names filled in so that
> + expand_symtabs_matching can match filenames. It's convenient to
> + do this here. */
> + if (!this_cu->files_read)
> + dw2_get_file_names_reader (reader.cu (), reader.top_level_die ());
In other spots, this is initialized lazily when something calls
dw2_get_file_names. Why would it not work with expand_symtabs_matching?
Simon
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH v2 10/28] Have expand_symtabs_matching work for already-expanded CUs
2025-04-23 15:53 ` Simon Marchi
@ 2025-04-23 20:39 ` Tom Tromey
2025-04-23 20:57 ` Tom Tromey
0 siblings, 1 reply; 50+ messages in thread
From: Tom Tromey @ 2025-04-23 20:39 UTC (permalink / raw)
To: Simon Marchi; +Cc: Tom Tromey, gdb-patches
>> This changes the contract of expand_symtabs_matching to also call the
>> callback for an already-expanded symtab. With this change, the number
>> of searched symtabs should sometimes be reduced. This also cuts down
>> on the amount of redundant code.
Simon> Not particularly important, but I'm not sure I understand how this patch
Simon> alone would result in fewer symtabs being searched. I understand that
Simon> it's a step towards the ultimate goal of this series, but I suppose you
Simon> need the changes to the symbol lookup functions (that come later in this
Simon> series) to reduce the number of searched symtabs?
Yeah. I added some text to the commit message here.
>> + /* We always need the file names filled in so that
>> + expand_symtabs_matching can match filenames. It's convenient to
>> + do this here. */
>> + if (!this_cu->files_read)
>> + dw2_get_file_names_reader (reader.cu (), reader.top_level_die ());
Simon> In other spots, this is initialized lazily when something calls
Simon> dw2_get_file_names. Why would it not work with expand_symtabs_matching?
It's been a long time since I wrote this but my recollection is that if
the CU is already expanded, then this is never done.
I will take another look.
Tom
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH v2 10/28] Have expand_symtabs_matching work for already-expanded CUs
2025-04-23 20:39 ` Tom Tromey
@ 2025-04-23 20:57 ` Tom Tromey
0 siblings, 0 replies; 50+ messages in thread
From: Tom Tromey @ 2025-04-23 20:57 UTC (permalink / raw)
To: Tom Tromey; +Cc: Simon Marchi, gdb-patches
>>> + if (!this_cu->files_read)
>>> + dw2_get_file_names_reader (reader.cu (), reader.top_level_die ());
Simon> In other spots, this is initialized lazily when something calls
Simon> dw2_get_file_names. Why would it not work with expand_symtabs_matching?
Tom> It's been a long time since I wrote this but my recollection is that if
Tom> the CU is already expanded, then this is never done.
Tom> I will take another look.
FWIW one benefit to doing the call here is that the cutu_reader has
already been constructed. So, it is more efficient.
I'll remove it and try to reproduce the crash though.
Tom
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH v2 11/28] Rewrite the .gdb_index reader
2025-04-02 23:44 [PATCH v2 00/28] Search symbols via quick API Tom Tromey
` (9 preceding siblings ...)
2025-04-02 23:45 ` [PATCH v2 10/28] Have expand_symtabs_matching work for already-expanded CUs Tom Tromey
@ 2025-04-02 23:45 ` Tom Tromey
2025-04-23 17:22 ` Simon Marchi
2025-04-24 14:37 ` Pedro Alves
2025-04-02 23:45 ` [PATCH v2 12/28] Convert default_collect_symbol_completion_matches_break_on Tom Tromey
` (18 subsequent siblings)
29 siblings, 2 replies; 50+ messages in thread
From: Tom Tromey @ 2025-04-02 23:45 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This patch rewrites the .gdb_index reader to create the same data
structures that are created by the cooked indexer and the .debug_names
reader.
This is done in support of this series; but also because, from what I
can tell, the "templates.exp" change didn't really work properly with
this reader.
In addition to fixing that problem, this patch removes a lot of code.
Implementing this required a couple of hacks, as .gdb_index does not
contain all the information that's used by the cooked index
implementation.
* The index-searching code likes to differentiate between the various
DWARF tags when matching, but .gdb_index lumps many things into a
single "other" category. To handle this, we introduce a phony tag
that's used so that the match method can match on multiple domains.
* Similarly, .gdb_index doesn't distinguish between the type and
struct domains, so another phony tag is used for this.
* Support for older versions of .gdb_index is removed entirely.
* The reader must attempt to guess the language of various symbols.
This is somewhat finicky. "Plain" (unqualified) symbols are marked
as language_unknown and then a couple of hacks are used to handle
these -- one in expand_symtabs_matching and another when recognizing
"main".
For what it's worth, I consider .gdb_index to be near the end of its
life. While .debug_names is not perfect -- we found a number of bugs
in the standard while implementing it -- it is better than .gdb_index
and also better documented.
After this patch, we could conceivably remove dwarf_scanner_base.
However, I have not done this.
Finally, this patch also changes this reader to dump the content of
the index, as the other DWARF readers do. This can be handy when
debugging gdb.
---
gdb/dwarf2/cooked-index-shard.c | 11 +-
gdb/dwarf2/cooked-index-worker.h | 10 +-
gdb/dwarf2/read-gdb-index.c | 1270 +++++++-------------------------------
gdb/dwarf2/read-gdb-index.h | 14 +
gdb/dwarf2/read.c | 58 +-
gdb/dwarf2/tag.h | 8 +
6 files changed, 302 insertions(+), 1069 deletions(-)
diff --git a/gdb/dwarf2/cooked-index-shard.c b/gdb/dwarf2/cooked-index-shard.c
index 29a8aea513786e4c1c1ed77dee8610fc329d1c8a..888a0fa345b9124e2814aa722e71a9d2fd5adf8e 100644
--- a/gdb/dwarf2/cooked-index-shard.c
+++ b/gdb/dwarf2/cooked-index-shard.c
@@ -86,7 +86,16 @@ cooked_index_shard::add (sect_offset die_offset, enum dwarf_tag tag,
implicit "main" discovery. */
if ((flags & IS_MAIN) != 0)
m_main = result;
- else if ((flags & IS_PARENT_DEFERRED) == 0
+ /* The language check here is subtle: it exists solely to work
+ around a bug in .gdb_index. That index does not record
+ languages, but it might emit an entry for "main". However,
+ recognizing this "main" as being the main program would be wrong
+ -- for example, an Ada program has a C "main" but this is not the
+ desired target of the "start" command. Requiring the language to
+ be set here avoids over-eagerly setting the "main" when using
+ .gdb_index. Should .gdb_index ever be removed (PR symtab/31363),
+ the language_unknown check here could also be removed. */
+ else if (lang != language_unknown
&& parent_entry.resolved == nullptr
&& m_main == nullptr
&& language_may_use_plain_main (lang)
diff --git a/gdb/dwarf2/cooked-index-worker.h b/gdb/dwarf2/cooked-index-worker.h
index c6e005c14dea1bb891680969c17172d494a7de46..a6b4780373c54a51760e46997a24aca57c340969 100644
--- a/gdb/dwarf2/cooked-index-worker.h
+++ b/gdb/dwarf2/cooked-index-worker.h
@@ -108,6 +108,12 @@ class cooked_index_worker_result
return &m_addrmap;
}
+ /* Set the mutable addrmap. */
+ void set_addrmap (addrmap_mutable new_map)
+ {
+ m_addrmap = std::move (new_map);
+ }
+
/* Return the parent_map that is currently being created. */
parent_map *get_parent_map ()
{
@@ -203,10 +209,10 @@ enum class cooked_state
/* An object of this type controls the scanning of the DWARF. It
schedules the worker tasks and tracks the current state. Once
scanning is done, this object is discarded.
-
+
This is an abstract base class that defines the basic behavior of
scanners. Separate concrete implementations exist for scanning
- .debug_names and .debug_info. */
+ .debug_names, .gdb_index, and .debug_info. */
class cooked_index_worker
{
diff --git a/gdb/dwarf2/read-gdb-index.c b/gdb/dwarf2/read-gdb-index.c
index 7bcf50d347609f0b27068228cd78633dd57e1556..1d7704b8c5bb7b5487b2373ea064c11ab1630e72 100644
--- a/gdb/dwarf2/read-gdb-index.c
+++ b/gdb/dwarf2/read-gdb-index.c
@@ -27,16 +27,68 @@
#include "event-top.h"
#include "gdb/gdb-index.h"
#include "gdbsupport/gdb-checked-static-cast.h"
-#include "mapped-index.h"
+#include "cooked-index.h"
#include "read.h"
#include "extract-store-integer.h"
#include "cp-support.h"
#include "symtab.h"
#include "gdbsupport/selftest.h"
+#include "tag.h"
+#include "gdbsupport/gdb-safe-ctype.h"
/* When true, do not reject deprecated .gdb_index sections. */
static bool use_deprecated_index_sections = false;
+struct dwarf2_gdb_index : public cooked_index_functions
+{
+ /* This dumps minimal information about the index.
+ It is called via "mt print objfiles".
+ One use is to verify .gdb_index has been loaded by the
+ gdb.dwarf2/gdb-index.exp testcase. */
+ void dump (struct objfile *objfile) override;
+};
+
+/* This is like a cooked index, but as it has been ingested from
+ .gdb_index, it can't be used to write out an index. */
+
+class cooked_gdb_index : public cooked_index
+{
+public:
+
+ cooked_gdb_index (std::unique_ptr<cooked_index_worker> worker,
+ int version)
+ : cooked_index (std::move (worker)),
+ version (version)
+ { }
+
+ cooked_index *index_for_writing () override
+ { return nullptr; }
+
+ quick_symbol_functions_up make_quick_functions () const override
+ { return quick_symbol_functions_up (new dwarf2_gdb_index); }
+
+ bool version_check () const override
+ {
+ return version >= 8;
+ }
+
+ /* Index data format version. */
+ int version;
+};
+
+/* See above. */
+
+void
+dwarf2_gdb_index::dump (struct objfile *objfile)
+{
+ dwarf2_per_objfile *per_objfile = get_dwarf2_per_objfile (objfile);
+
+ cooked_gdb_index *index = (gdb::checked_static_cast<cooked_gdb_index *>
+ (per_objfile->per_bfd->index_table.get ()));
+ gdb_printf (".gdb_index: version %d\n", index->version);
+ cooked_index_functions::dump (objfile);
+}
+
/* This is a view into the index that converts from bytes to an
offset_type, and allows indexing. Unaligned bytes are specifically
allowed here, and handled via unpacking. */
@@ -77,43 +129,11 @@ class offset_view
gdb::array_view<const gdb_byte> m_bytes;
};
-/* An index into a (C++) symbol name component in a symbol name as
- recorded in the mapped_index's symbol table. For each C++ symbol
- in the symbol table, we record one entry for the start of each
- component in the symbol in a table of name components, and then
- sort the table, in order to be able to binary search symbol names,
- ignoring leading namespaces, both completion and regular look up.
- For example, for symbol "A::B::C", we'll have an entry that points
- to "A::B::C", another that points to "B::C", and another for "C".
- Note that function symbols in GDB index have no parameter
- information, just the function/method names. You can convert a
- name_component to a "const char *" using the
- 'mapped_index::symbol_name_at(offset_type)' method. */
-
-struct name_component
-{
- /* Offset in the symbol name where the component starts. Stored as
- a (32-bit) offset instead of a pointer to save memory and improve
- locality on 64-bit architectures. */
- offset_type name_offset;
-
- /* The symbol's index in the symbol and constant pool tables of a
- mapped_index. */
- offset_type idx;
-};
-
-/* A description of .gdb_index index. The file format is described in
- a comment by the code that writes the index. */
+/* A worker for reading .gdb_index. The file format is described in
+ the manual. */
-struct mapped_gdb_index : public dwarf_scanner_base
+struct mapped_gdb_index
{
- /* The name_component table (a sorted vector). See name_component's
- description above. */
- std::vector<name_component> name_components;
-
- /* How NAME_COMPONENTS is sorted. */
- enum case_sensitivity name_components_casing;
-
/* Index data format version. */
int version = 0;
@@ -132,6 +152,14 @@ struct mapped_gdb_index : public dwarf_scanner_base
/* An address map that maps from PC to dwarf2_per_cu. */
addrmap_fixed *index_addrmap = nullptr;
+ /* The name of 'main', or nullptr if not known. */
+ const char *main_name = nullptr;
+ /* The language of 'main', if known. */
+ enum language main_lang = language_minimal;
+
+ /* The result we're constructing. */
+ cooked_index_worker_result result;
+
/* Return the index into the constant pool of the name of the IDXth
symbol in the symbol table. */
offset_type symbol_name_index (offset_type idx) const
@@ -148,221 +176,41 @@ struct mapped_gdb_index : public dwarf_scanner_base
/* Return whether the name at IDX in the symbol table should be
ignored. */
- virtual bool symbol_name_slot_invalid (offset_type idx) const
+ bool symbol_name_slot_invalid (offset_type idx) const
{
- return (symbol_name_index (idx) == 0
- && symbol_vec_index (idx) == 0);
+ return symbol_name_index (idx) == 0 && symbol_vec_index (idx) == 0;
}
/* Convenience method to get at the name of the symbol at IDX in the
symbol table. */
- virtual const char *symbol_name_at
- (offset_type idx, dwarf2_per_objfile *per_objfile) const
+ const char *symbol_name_at (offset_type idx,
+ dwarf2_per_objfile *per_objfile) const
{
return (const char *) (this->constant_pool.data ()
+ symbol_name_index (idx));
}
- virtual size_t symbol_name_count () const
+ size_t symbol_name_count () const
{ return this->symbol_table.size () / 2; }
+ /* Set the name and language of the main function from the shortcut
+ table. */
+ void set_main_name (dwarf2_per_objfile *per_objfile);
+
/* Build the symbol name component sorted vector, if we haven't
yet. */
void build_name_components (dwarf2_per_objfile *per_objfile);
-
- /* Returns the lower (inclusive) and upper (exclusive) bounds of the
- possible matches for LN_NO_PARAMS in the name component
- vector. */
- std::pair<std::vector<name_component>::const_iterator,
- std::vector<name_component>::const_iterator>
- find_name_components_bounds (const lookup_name_info &ln_no_params,
- enum language lang,
- dwarf2_per_objfile *per_objfile) const;
-
- quick_symbol_functions_up make_quick_functions () const override;
-
- bool version_check () const override
- {
- return version >= 8;
- }
-
- dwarf2_per_cu *lookup (unrelocated_addr addr) override
- {
- if (index_addrmap == nullptr)
- return nullptr;
-
- void *obj = index_addrmap->find (static_cast<CORE_ADDR> (addr));
- return static_cast<dwarf2_per_cu *> (obj);
- }
-
- cooked_index *index_for_writing () override
- { return nullptr; }
};
-
-/* Starting from a search name, return the string that finds the upper
- bound of all strings that start with SEARCH_NAME in a sorted name
- list. Returns the empty string to indicate that the upper bound is
- the end of the list. */
-
-static std::string
-make_sort_after_prefix_name (const char *search_name)
-{
- /* When looking to complete "func", we find the upper bound of all
- symbols that start with "func" by looking for where we'd insert
- the closest string that would follow "func" in lexicographical
- order. Usually, that's "func"-with-last-character-incremented,
- i.e. "fund". Mind non-ASCII characters, though. Usually those
- will be UTF-8 multi-byte sequences, but we can't be certain.
- Especially mind the 0xff character, which is a valid character in
- non-UTF-8 source character sets (e.g. Latin1 'ÿ'), and we can't
- rule out compilers allowing it in identifiers. Note that
- conveniently, strcmp/strcasecmp are specified to compare
- characters interpreted as unsigned char. So what we do is treat
- the whole string as a base 256 number composed of a sequence of
- base 256 "digits" and add 1 to it. I.e., adding 1 to 0xff wraps
- to 0, and carries 1 to the following more-significant position.
- If the very first character in SEARCH_NAME ends up incremented
- and carries/overflows, then the upper bound is the end of the
- list. The string after the empty string is also the empty
- string.
-
- Some examples of this operation:
-
- SEARCH_NAME => "+1" RESULT
-
- "abc" => "abd"
- "ab\xff" => "ac"
- "\xff" "a" "\xff" => "\xff" "b"
- "\xff" => ""
- "\xff\xff" => ""
- "" => ""
-
- Then, with these symbols for example:
-
- func
- func1
- fund
-
- completing "func" looks for symbols between "func" and
- "func"-with-last-character-incremented, i.e. "fund" (exclusive),
- which finds "func" and "func1", but not "fund".
-
- And with:
-
- funcÿ (Latin1 'ÿ' [0xff])
- funcÿ1
- fund
-
- completing "funcÿ" looks for symbols between "funcÿ" and "fund"
- (exclusive), which finds "funcÿ" and "funcÿ1", but not "fund".
-
- And with:
-
- ÿÿ (Latin1 'ÿ' [0xff])
- ÿÿ1
-
- completing "ÿ" or "ÿÿ" looks for symbols between between "ÿÿ" and
- the end of the list.
- */
- std::string after = search_name;
- while (!after.empty () && (unsigned char) after.back () == 0xff)
- after.pop_back ();
- if (!after.empty ())
- after.back () = (unsigned char) after.back () + 1;
- return after;
-}
-
-/* See declaration. */
-
-std::pair<std::vector<name_component>::const_iterator,
- std::vector<name_component>::const_iterator>
-mapped_gdb_index::find_name_components_bounds
- (const lookup_name_info &lookup_name_without_params, language lang,
- dwarf2_per_objfile *per_objfile) const
-{
- auto *name_cmp
- = this->name_components_casing == case_sensitive_on ? strcmp : strcasecmp;
-
- const char *lang_name
- = lookup_name_without_params.language_lookup_name (lang);
-
- /* Comparison function object for lower_bound that matches against a
- given symbol name. */
- auto lookup_compare_lower = [&] (const name_component &elem,
- const char *name)
- {
- const char *elem_qualified = this->symbol_name_at (elem.idx, per_objfile);
- const char *elem_name = elem_qualified + elem.name_offset;
- return name_cmp (elem_name, name) < 0;
- };
-
- /* Comparison function object for upper_bound that matches against a
- given symbol name. */
- auto lookup_compare_upper = [&] (const char *name,
- const name_component &elem)
- {
- const char *elem_qualified = this->symbol_name_at (elem.idx, per_objfile);
- const char *elem_name = elem_qualified + elem.name_offset;
- return name_cmp (name, elem_name) < 0;
- };
-
- auto begin = this->name_components.begin ();
- auto end = this->name_components.end ();
-
- /* Find the lower bound. */
- auto lower = [&] ()
- {
- if (lookup_name_without_params.completion_mode () && lang_name[0] == '\0')
- return begin;
- else
- return std::lower_bound (begin, end, lang_name, lookup_compare_lower);
- } ();
-
- /* Find the upper bound. */
- auto upper = [&] ()
- {
- if (lookup_name_without_params.completion_mode ())
- {
- /* In completion mode, we want UPPER to point past all
- symbols names that have the same prefix. I.e., with
- these symbols, and completing "func":
-
- function << lower bound
- function1
- other_function << upper bound
-
- We find the upper bound by looking for the insertion
- point of "func"-with-last-character-incremented,
- i.e. "fund". */
- std::string after = make_sort_after_prefix_name (lang_name);
- if (after.empty ())
- return end;
- return std::lower_bound (lower, end, after.c_str (),
- lookup_compare_lower);
- }
- else
- return std::upper_bound (lower, end, lang_name, lookup_compare_upper);
- } ();
-
- return {lower, upper};
-}
-
/* See declaration. */
void
mapped_gdb_index::build_name_components (dwarf2_per_objfile *per_objfile)
{
- if (!this->name_components.empty ())
- return;
-
- this->name_components_casing = case_sensitivity;
- auto *name_cmp
- = this->name_components_casing == case_sensitive_on ? strcmp : strcasecmp;
+ std::vector<std::pair<std::string_view, std::vector<cooked_index_entry *>>>
+ need_parents;
+ gdb::unordered_map<std::string_view, cooked_index_entry *> by_name;
- /* The code below only knows how to break apart components of C++
- symbol names (and other languages that use '::' as
- namespace/module separator) and Ada symbol names. */
auto count = this->symbol_name_count ();
for (offset_type idx = 0; idx < count; idx++)
{
@@ -371,815 +219,186 @@ mapped_gdb_index::build_name_components (dwarf2_per_objfile *per_objfile)
const char *name = this->symbol_name_at (idx, per_objfile);
- /* Add each name component to the name component table. */
- unsigned int previous_len = 0;
+ /* This code only knows how to break apart components of C++
+ symbol names (and other languages that use '::' as
+ namespace/module separator) and Ada symbol names.
+ It's unfortunate that we need the language, but since it is
+ really only used to rebuild full names, pairing it with the
+ split method is fine. */
+ enum language lang;
+ std::vector<std::string_view> components;
if (strstr (name, "::") != nullptr)
{
- for (unsigned int current_len = cp_find_first_component (name);
- name[current_len] != '\0';
- current_len += cp_find_first_component (name + current_len))
- {
- gdb_assert (name[current_len] == ':');
- this->name_components.push_back ({previous_len, idx});
- /* Skip the '::'. */
- current_len += 2;
- previous_len = current_len;
- }
+ components = split_name (name, split_style::CXX);
+ lang = language_cplus;
}
- else
+ else if (strchr (name, '<') != nullptr)
{
- /* Handle the Ada encoded (aka mangled) form here. */
- for (const char *iter = strstr (name, "__");
- iter != nullptr;
- iter = strstr (iter, "__"))
- {
- this->name_components.push_back ({previous_len, idx});
- iter += 2;
- previous_len = iter - name;
- }
+ /* Guess that this is a template and so a C++ name. */
+ components.emplace_back (name);
+ lang = language_cplus;
}
-
- this->name_components.push_back ({previous_len, idx});
- }
-
- /* Sort name_components elements by name. */
- auto name_comp_compare = [&] (const name_component &left,
- const name_component &right)
- {
- const char *left_qualified
- = this->symbol_name_at (left.idx, per_objfile);
- const char *right_qualified
- = this->symbol_name_at (right.idx, per_objfile);
-
- const char *left_name = left_qualified + left.name_offset;
- const char *right_name = right_qualified + right.name_offset;
-
- return name_cmp (left_name, right_name) < 0;
- };
-
- std::sort (this->name_components.begin (),
- this->name_components.end (),
- name_comp_compare);
-}
-
-/* Helper for dw2_expand_symtabs_matching that works with a
- mapped_index_base instead of the containing objfile. This is split
- to a separate function in order to be able to unit test the
- name_components matching using a mock mapped_index_base. For each
- symbol name that matches, calls MATCH_CALLBACK, passing it the
- symbol's index in the mapped_index_base symbol table. */
-
-static bool
-dw2_expand_symtabs_matching_symbol
- (mapped_gdb_index &index,
- const lookup_name_info &lookup_name_in,
- expand_symtabs_symbol_matcher symbol_matcher,
- gdb::function_view<bool (offset_type)> match_callback,
- dwarf2_per_objfile *per_objfile,
- expand_symtabs_lang_matcher lang_matcher)
-{
- lookup_name_info lookup_name_without_params
- = lookup_name_in.make_ignore_params ();
-
- /* Build the symbol name component sorted vector, if we haven't
- yet. */
- index.build_name_components (per_objfile);
-
- /* The same symbol may appear more than once in the range though.
- E.g., if we're looking for symbols that complete "w", and we have
- a symbol named "w1::w2", we'll find the two name components for
- that same symbol in the range. To be sure we only call the
- callback once per symbol, we first collect the symbol name
- indexes that matched in a temporary vector and ignore
- duplicates. */
- std::vector<offset_type> matches;
-
- struct name_and_matcher
- {
- symbol_name_matcher_ftype *matcher;
- const char *name;
-
- bool operator== (const name_and_matcher &other) const
- {
- return matcher == other.matcher && strcmp (name, other.name) == 0;
- }
- };
-
- /* A vector holding all the different symbol name matchers, for all
- languages. */
- std::vector<name_and_matcher> matchers;
-
- for (int i = 0; i < nr_languages; i++)
- {
- enum language lang_e = (enum language) i;
- if (lang_matcher != nullptr && !lang_matcher (lang_e))
- continue;
-
- const language_defn *lang = language_def (lang_e);
- symbol_name_matcher_ftype *name_matcher
- = lang->get_symbol_name_matcher (lookup_name_without_params);
-
- name_and_matcher key {
- name_matcher,
- lookup_name_without_params.language_lookup_name (lang_e)
- };
-
- /* Don't insert the same comparison routine more than once.
- Note that we do this linear walk. This is not a problem in
- practice because the number of supported languages is
- low. */
- if (std::find (matchers.begin (), matchers.end (), key)
- != matchers.end ())
- continue;
- matchers.push_back (std::move (key));
-
- auto bounds
- = index.find_name_components_bounds (lookup_name_without_params,
- lang_e, per_objfile);
-
- /* Now for each symbol name in range, check to see if we have a name
- match, and if so, call the MATCH_CALLBACK callback. */
-
- for (; bounds.first != bounds.second; ++bounds.first)
+ else if (strstr (name, "__") != nullptr)
{
- const char *qualified
- = index.symbol_name_at (bounds.first->idx, per_objfile);
-
- if (!name_matcher (qualified, lookup_name_without_params, NULL)
- || (symbol_matcher != NULL && !symbol_matcher (qualified)))
- continue;
-
- matches.push_back (bounds.first->idx);
+ /* The Ada case is handled during finalization, because gdb
+ does not write the synthesized package names into the
+ index. */
+ components.emplace_back (name);
+ lang = language_ada;
}
- }
-
- std::sort (matches.begin (), matches.end ());
-
- /* Finally call the callback, once per match. */
- ULONGEST prev = -1;
- bool result = true;
- for (offset_type idx : matches)
- {
- if (prev != idx)
+ else
{
- if (!match_callback (idx))
- {
- result = false;
- break;
- }
- prev = idx;
+ components = split_name (name, split_style::DOT_STYLE);
+ /* Mark ordinary names as having an unknown language. This
+ is a hack to avoid problems with some Ada names. */
+ lang = (components.size () == 1) ? language_unknown : language_go;
}
- }
-
- /* Above we use a type wider than idx's for 'prev', since 0 and
- (offset_type)-1 are both possible values. */
- static_assert (sizeof (prev) > sizeof (offset_type), "");
-
- return result;
-}
-
-#if GDB_SELF_TEST
-
-namespace selftests { namespace dw2_expand_symtabs_matching {
-
-/* A mock .gdb_index/.debug_names-like name index table, enough to
- exercise dw2_expand_symtabs_matching_symbol, which works with the
- mapped_index_base interface. Builds an index from the symbol list
- passed as parameter to the constructor. */
-class mock_mapped_index : public mapped_gdb_index
-{
-public:
- mock_mapped_index (gdb::array_view<const char *> symbols)
- : m_symbol_table (symbols)
- {}
-
- DISABLE_COPY_AND_ASSIGN (mock_mapped_index);
-
- bool symbol_name_slot_invalid (offset_type idx) const override
- { return false; }
-
- /* Return the number of names in the symbol table. */
- size_t symbol_name_count () const override
- {
- return m_symbol_table.size ();
- }
-
- /* Get the name of the symbol at IDX in the symbol table. */
- const char *symbol_name_at
- (offset_type idx, dwarf2_per_objfile *per_objfile) const override
- {
- return m_symbol_table[idx];
- }
-
- quick_symbol_functions_up make_quick_functions () const override
- {
- return nullptr;
- }
-
-private:
- gdb::array_view<const char *> m_symbol_table;
-};
-
-/* Convenience function that converts a NULL pointer to a "<null>"
- string, to pass to print routines. */
-
-static const char *
-string_or_null (const char *str)
-{
- return str != NULL ? str : "<null>";
-}
-
-/* Check if a lookup_name_info built from
- NAME/MATCH_TYPE/COMPLETION_MODE matches the symbols in the mock
- index. EXPECTED_LIST is the list of expected matches, in expected
- matching order. If no match expected, then an empty list is
- specified. Returns true on success. On failure prints a warning
- indicating the file:line that failed, and returns false. */
-
-static bool
-check_match (const char *file, int line,
- mock_mapped_index &mock_index,
- const char *name, symbol_name_match_type match_type,
- bool completion_mode,
- std::initializer_list<const char *> expected_list,
- dwarf2_per_objfile *per_objfile)
-{
- lookup_name_info lookup_name (name, match_type, completion_mode);
-
- bool matched = true;
-
- auto mismatch = [&] (const char *expected_str,
- const char *got)
- {
- warning (_("%s:%d: match_type=%s, looking-for=\"%s\", "
- "expected=\"%s\", got=\"%s\"\n"),
- file, line,
- (match_type == symbol_name_match_type::FULL
- ? "FULL" : "WILD"),
- name, string_or_null (expected_str), string_or_null (got));
- matched = false;
- };
-
- auto expected_it = expected_list.begin ();
- auto expected_end = expected_list.end ();
-
- dw2_expand_symtabs_matching_symbol (mock_index, lookup_name,
- nullptr,
- [&] (offset_type idx)
- {
- const char *matched_name = mock_index.symbol_name_at (idx, per_objfile);
- const char *expected_str
- = expected_it == expected_end ? NULL : *expected_it++;
-
- if (expected_str == NULL || strcmp (expected_str, matched_name) != 0)
- mismatch (expected_str, matched_name);
- return true;
- }, per_objfile, nullptr);
-
- const char *expected_str
- = expected_it == expected_end ? NULL : *expected_it++;
- if (expected_str != NULL)
- mismatch (expected_str, NULL);
-
- return matched;
-}
-
-/* The symbols added to the mock mapped_index for testing (in
- canonical form). */
-static const char *test_symbols[] = {
- "function",
- "std::bar",
- "std::zfunction",
- "std::zfunction2",
- "w1::w2",
- "ns::foo<char*>",
- "ns::foo<int>",
- "ns::foo<long>",
- "ns2::tmpl<int>::foo2",
- "(anonymous namespace)::A::B::C",
-
- /* These are used to check that the increment-last-char in the
- matching algorithm for completion doesn't match "t1_fund" when
- completing "t1_func". */
- "t1_func",
- "t1_func1",
- "t1_fund",
- "t1_fund1",
-
- /* A UTF-8 name with multi-byte sequences to make sure that
- cp-name-parser understands this as a single identifier ("função"
- is "function" in PT). */
- (const char *)u8"u8função",
-
- /* Test a symbol name that ends with a 0xff character, which is a
- valid character in non-UTF-8 source character sets (e.g. Latin1
- 'ÿ'), and we can't rule out compilers allowing it in identifiers.
- We test this because the completion algorithm finds the upper
- bound of symbols by looking for the insertion point of
- "func"-with-last-character-incremented, i.e. "fund", and adding 1
- to 0xff should wraparound and carry to the previous character.
- See comments in make_sort_after_prefix_name. */
- "yfunc\377",
-
- /* Some more symbols with \377 (0xff). See above. */
- "\377",
- "\377\377123",
-
- /* A name with all sorts of complications. Starts with "z" to make
- it easier for the completion tests below. */
-#define Z_SYM_NAME \
- "z::std::tuple<(anonymous namespace)::ui*, std::bar<(anonymous namespace)::ui> >" \
- "::tuple<(anonymous namespace)::ui*, " \
- "std::default_delete<(anonymous namespace)::ui>, void>"
-
- Z_SYM_NAME
-};
-
-/* Returns true if the mapped_index_base::find_name_component_bounds
- method finds EXPECTED_SYMS in INDEX when looking for SEARCH_NAME,
- in completion mode. */
-
-static bool
-check_find_bounds_finds (mapped_gdb_index &index,
- const char *search_name,
- gdb::array_view<const char *> expected_syms,
- dwarf2_per_objfile *per_objfile)
-{
- lookup_name_info lookup_name (search_name,
- symbol_name_match_type::FULL, true);
-
- auto bounds = index.find_name_components_bounds (lookup_name,
- language_cplus,
- per_objfile);
-
- size_t distance = std::distance (bounds.first, bounds.second);
- if (distance != expected_syms.size ())
- return false;
-
- for (size_t exp_elem = 0; exp_elem < distance; exp_elem++)
- {
- auto nc_elem = bounds.first + exp_elem;
- const char *qualified = index.symbol_name_at (nc_elem->idx, per_objfile);
- if (strcmp (qualified, expected_syms[exp_elem]) != 0)
- return false;
- }
-
- return true;
-}
-
-/* Test the lower-level mapped_index::find_name_component_bounds
- method. */
-
-static void
-test_mapped_index_find_name_component_bounds ()
-{
- mock_mapped_index mock_index (test_symbols);
-
- mock_index.build_name_components (NULL /* per_objfile */);
-
- /* Test the lower-level mapped_index::find_name_component_bounds
- method in completion mode. */
- {
- static const char *expected_syms[] = {
- "t1_func",
- "t1_func1",
- };
-
- SELF_CHECK (check_find_bounds_finds
- (mock_index, "t1_func", expected_syms,
- NULL /* per_objfile */));
- }
-
- /* Check that the increment-last-char in the name matching algorithm
- for completion doesn't get confused with Ansi1 'ÿ' / 0xff. See
- make_sort_after_prefix_name. */
- {
- static const char *expected_syms1[] = {
- "\377",
- "\377\377123",
- };
- SELF_CHECK (check_find_bounds_finds
- (mock_index, "\377", expected_syms1, NULL /* per_objfile */));
-
- static const char *expected_syms2[] = {
- "\377\377123",
- };
- SELF_CHECK (check_find_bounds_finds
- (mock_index, "\377\377", expected_syms2,
- NULL /* per_objfile */));
- }
-}
-
-/* Test dw2_expand_symtabs_matching_symbol. */
-
-static void
-test_dw2_expand_symtabs_matching_symbol ()
-{
- mock_mapped_index mock_index (test_symbols);
-
- /* We let all tests run until the end even if some fails, for debug
- convenience. */
- bool any_mismatch = false;
-
- /* Create the expected symbols list (an initializer_list). Needed
- because lists have commas, and we need to pass them to CHECK,
- which is a macro. */
-#define EXPECT(...) { __VA_ARGS__ }
-
- /* Wrapper for check_match that passes down the current
- __FILE__/__LINE__. */
-#define CHECK_MATCH(NAME, MATCH_TYPE, COMPLETION_MODE, EXPECTED_LIST) \
- any_mismatch |= !check_match (__FILE__, __LINE__, \
- mock_index, \
- NAME, MATCH_TYPE, COMPLETION_MODE, \
- EXPECTED_LIST, NULL)
-
- /* Identity checks. */
- for (const char *sym : test_symbols)
- {
- /* Should be able to match all existing symbols. */
- CHECK_MATCH (sym, symbol_name_match_type::FULL, false,
- EXPECT (sym));
-
- /* Should be able to match all existing symbols with
- parameters. */
- std::string with_params = std::string (sym) + "(int)";
- CHECK_MATCH (with_params.c_str (), symbol_name_match_type::FULL, false,
- EXPECT (sym));
-
- /* Should be able to match all existing symbols with
- parameters and qualifiers. */
- with_params = std::string (sym) + " ( int ) const";
- CHECK_MATCH (with_params.c_str (), symbol_name_match_type::FULL, false,
- EXPECT (sym));
-
- /* This should really find sym, but cp-name-parser.y doesn't
- know about lvalue/rvalue qualifiers yet. */
- with_params = std::string (sym) + " ( int ) &&";
- CHECK_MATCH (with_params.c_str (), symbol_name_match_type::FULL, false,
- {});
- }
-
- /* Check that the name matching algorithm for completion doesn't get
- confused with Latin1 'ÿ' / 0xff. See
- make_sort_after_prefix_name. */
- {
- static const char str[] = "\377";
- CHECK_MATCH (str, symbol_name_match_type::FULL, true,
- EXPECT ("\377", "\377\377123"));
- }
-
- /* Check that the increment-last-char in the matching algorithm for
- completion doesn't match "t1_fund" when completing "t1_func". */
- {
- static const char str[] = "t1_func";
- CHECK_MATCH (str, symbol_name_match_type::FULL, true,
- EXPECT ("t1_func", "t1_func1"));
- }
-
- /* Check that completion mode works at each prefix of the expected
- symbol name. */
- {
- static const char str[] = "function(int)";
- size_t len = strlen (str);
- std::string lookup;
-
- for (size_t i = 1; i < len; i++)
- {
- lookup.assign (str, i);
- CHECK_MATCH (lookup.c_str (), symbol_name_match_type::FULL, true,
- EXPECT ("function"));
- }
- }
-
- /* While "w" is a prefix of both components, the match function
- should still only be called once. */
- {
- CHECK_MATCH ("w", symbol_name_match_type::FULL, true,
- EXPECT ("w1::w2"));
- CHECK_MATCH ("w", symbol_name_match_type::WILD, true,
- EXPECT ("w1::w2"));
- }
-
- /* Same, with a "complicated" symbol. */
- {
- static const char str[] = Z_SYM_NAME;
- size_t len = strlen (str);
- std::string lookup;
-
- for (size_t i = 1; i < len; i++)
- {
- lookup.assign (str, i);
- CHECK_MATCH (lookup.c_str (), symbol_name_match_type::FULL, true,
- EXPECT (Z_SYM_NAME));
- }
- }
-
- /* In FULL mode, an incomplete symbol doesn't match. */
- {
- CHECK_MATCH ("std::zfunction(int", symbol_name_match_type::FULL, false,
- {});
- }
-
- /* A complete symbol with parameters matches any overload, since the
- index has no overload info. */
- {
- CHECK_MATCH ("std::zfunction(int)", symbol_name_match_type::FULL, true,
- EXPECT ("std::zfunction", "std::zfunction2"));
- CHECK_MATCH ("zfunction(int)", symbol_name_match_type::WILD, true,
- EXPECT ("std::zfunction", "std::zfunction2"));
- CHECK_MATCH ("zfunc", symbol_name_match_type::WILD, true,
- EXPECT ("std::zfunction", "std::zfunction2"));
- }
-
- /* Check that whitespace is ignored appropriately. A symbol with a
- template argument list. */
- {
- static const char expected[] = "ns::foo<int>";
- CHECK_MATCH ("ns :: foo < int > ", symbol_name_match_type::FULL, false,
- EXPECT (expected));
- CHECK_MATCH ("foo < int > ", symbol_name_match_type::WILD, false,
- EXPECT (expected));
- }
-
- /* Check that whitespace is ignored appropriately. A symbol with a
- template argument list that includes a pointer. */
- {
- static const char expected[] = "ns::foo<char*>";
- /* Try both completion and non-completion modes. */
- static const bool completion_mode[2] = {false, true};
- for (size_t i = 0; i < 2; i++)
- {
- CHECK_MATCH ("ns :: foo < char * >", symbol_name_match_type::FULL,
- completion_mode[i], EXPECT (expected));
- CHECK_MATCH ("foo < char * >", symbol_name_match_type::WILD,
- completion_mode[i], EXPECT (expected));
-
- CHECK_MATCH ("ns :: foo < char * > (int)", symbol_name_match_type::FULL,
- completion_mode[i], EXPECT (expected));
- CHECK_MATCH ("foo < char * > (int)", symbol_name_match_type::WILD,
- completion_mode[i], EXPECT (expected));
- }
- }
-
- {
- /* Check method qualifiers are ignored. */
- static const char expected[] = "ns::foo<char*>";
- CHECK_MATCH ("ns :: foo < char * > ( int ) const",
- symbol_name_match_type::FULL, true, EXPECT (expected));
- CHECK_MATCH ("ns :: foo < char * > ( int ) &&",
- symbol_name_match_type::FULL, true, EXPECT (expected));
- CHECK_MATCH ("foo < char * > ( int ) const",
- symbol_name_match_type::WILD, true, EXPECT (expected));
- CHECK_MATCH ("foo < char * > ( int ) &&",
- symbol_name_match_type::WILD, true, EXPECT (expected));
- }
-
- /* Test lookup names that don't match anything. */
- {
- CHECK_MATCH ("bar2", symbol_name_match_type::WILD, false,
- {});
-
- CHECK_MATCH ("doesntexist", symbol_name_match_type::FULL, false,
- {});
- }
-
- /* Some wild matching tests, exercising "(anonymous namespace)",
- which should not be confused with a parameter list. */
- {
- static const char *syms[] = {
- "A::B::C",
- "B::C",
- "C",
- "A :: B :: C ( int )",
- "B :: C ( int )",
- "C ( int )",
- };
-
- for (const char *s : syms)
- {
- CHECK_MATCH (s, symbol_name_match_type::WILD, false,
- EXPECT ("(anonymous namespace)::A::B::C"));
- }
- }
-
- {
- static const char expected[] = "ns2::tmpl<int>::foo2";
- CHECK_MATCH ("tmp", symbol_name_match_type::WILD, true,
- EXPECT (expected));
- CHECK_MATCH ("tmpl<", symbol_name_match_type::WILD, true,
- EXPECT (expected));
- }
-
- SELF_CHECK (!any_mismatch);
-#undef EXPECT
-#undef CHECK_MATCH
-}
-
-static void
-run_test ()
-{
- test_mapped_index_find_name_component_bounds ();
- test_dw2_expand_symtabs_matching_symbol ();
-}
-
-}} /* namespace selftests::dw2_expand_symtabs_matching */
-
-#endif /* GDB_SELF_TEST */
-
-struct dwarf2_gdb_index : public dwarf2_base_index_functions
-{
- /* This dumps minimal information about the index.
- It is called via "mt print objfiles".
- One use is to verify .gdb_index has been loaded by the
- gdb.dwarf2/gdb-index.exp testcase. */
- void dump (struct objfile *objfile) override;
-
- bool expand_symtabs_matching
- (struct objfile *objfile,
- expand_symtabs_file_matcher file_matcher,
- const lookup_name_info *lookup_name,
- expand_symtabs_symbol_matcher symbol_matcher,
- expand_symtabs_expansion_listener expansion_notify,
- block_search_flags search_flags,
- domain_search_flags domain,
- expand_symtabs_lang_matcher lang_matcher) override;
-};
-
-/* This dumps minimal information about the index.
- It is called via "mt print objfiles".
- One use is to verify .gdb_index has been loaded by the
- gdb.dwarf2/gdb-index.exp testcase. */
-
-void
-dwarf2_gdb_index::dump (struct objfile *objfile)
-{
- dwarf2_per_objfile *per_objfile = get_dwarf2_per_objfile (objfile);
-
- mapped_gdb_index *index = (gdb::checked_static_cast<mapped_gdb_index *>
- (per_objfile->per_bfd->index_table.get ()));
- gdb_printf (".gdb_index: version %d\n", index->version);
- gdb_printf ("\n");
-}
-
-/* Helper for dw2_expand_matching symtabs. Called on each symbol
- matched, to expand corresponding CUs that were marked. IDX is the
- index of the symbol name that matched. */
-
-static bool
-dw2_expand_marked_cus (dwarf2_per_objfile *per_objfile, offset_type idx,
- auto_bool_vector &marked,
- expand_symtabs_file_matcher file_matcher,
- expand_symtabs_expansion_listener expansion_notify,
- block_search_flags search_flags,
- domain_search_flags kind,
- expand_symtabs_lang_matcher lang_matcher)
-{
- offset_type vec_len, vec_idx;
- bool global_seen = false;
- mapped_gdb_index &index
- = *(gdb::checked_static_cast<mapped_gdb_index *>
- (per_objfile->per_bfd->index_table.get ()));
-
- offset_view vec (index.constant_pool.slice (index.symbol_vec_index (idx)));
- vec_len = vec[0];
- for (vec_idx = 0; vec_idx < vec_len; ++vec_idx)
- {
- offset_type cu_index_and_attrs = vec[vec_idx + 1];
- /* This value is only valid for index versions >= 7. */
- int is_static = GDB_INDEX_SYMBOL_STATIC_VALUE (cu_index_and_attrs);
- gdb_index_symbol_kind symbol_kind =
- GDB_INDEX_SYMBOL_KIND_VALUE (cu_index_and_attrs);
- int cu_index = GDB_INDEX_CU_VALUE (cu_index_and_attrs);
- /* Only check the symbol attributes if they're present.
- Indices prior to version 7 don't record them,
- and indices >= 7 may elide them for certain symbols
- (gold does this). */
- int attrs_valid =
- (index.version >= 7
- && symbol_kind != GDB_INDEX_SYMBOL_KIND_NONE);
-
- /* Work around gold/15646. */
- if (attrs_valid
- && !is_static
- && symbol_kind == GDB_INDEX_SYMBOL_KIND_TYPE)
+ std::vector<cooked_index_entry *> these_entries;
+ offset_view vec (constant_pool.slice (symbol_vec_index (idx)));
+ offset_type vec_len = vec[0];
+ bool global_seen = false;
+ for (offset_type vec_idx = 0; vec_idx < vec_len; ++vec_idx)
{
- if (global_seen)
+ offset_type cu_index_and_attrs = vec[vec_idx + 1];
+ gdb_index_symbol_kind symbol_kind
+ = GDB_INDEX_SYMBOL_KIND_VALUE (cu_index_and_attrs);
+ /* Only use a symbol if the attributes are present. Indices
+ prior to version 7 don't record them, and indices >= 7
+ may elide them for certain symbols (gold does this). */
+ if (symbol_kind == GDB_INDEX_SYMBOL_KIND_NONE)
continue;
- global_seen = true;
- }
+ int is_static = GDB_INDEX_SYMBOL_STATIC_VALUE (cu_index_and_attrs);
- /* Only check the symbol's kind if it has one. */
- if (attrs_valid)
- {
- if (is_static)
- {
- if ((search_flags & SEARCH_STATIC_BLOCK) == 0)
- continue;
- }
- else
+ int cu_index = GDB_INDEX_CU_VALUE (cu_index_and_attrs);
+ /* Don't crash on bad data. */
+ if (cu_index >= per_objfile->per_bfd->all_units.size ())
{
- if ((search_flags & SEARCH_GLOBAL_BLOCK) == 0)
- continue;
+ complaint (_(".gdb_index entry has bad CU index"
+ " [in module %s]"),
+ objfile_name (per_objfile->objfile));
+ continue;
}
+ dwarf2_per_cu *per_cu = per_objfile->per_bfd->get_cu (cu_index);
- domain_search_flags mask = 0;
+ enum language this_lang = lang;
+ dwarf_tag tag;
switch (symbol_kind)
{
case GDB_INDEX_SYMBOL_KIND_VARIABLE:
- mask = SEARCH_VAR_DOMAIN;
+ tag = DW_TAG_variable;
break;
case GDB_INDEX_SYMBOL_KIND_FUNCTION:
- mask = SEARCH_FUNCTION_DOMAIN;
+ tag = DW_TAG_subprogram;
break;
case GDB_INDEX_SYMBOL_KIND_TYPE:
- mask = SEARCH_TYPE_DOMAIN | SEARCH_STRUCT_DOMAIN;
+ if (is_static)
+ tag = (dwarf_tag) DW_TAG_GDB_INDEX_TYPE;
+ else
+ {
+ /* Work around gold/15646. */
+ if (global_seen)
+ continue;
+ global_seen = true;
+
+ tag = DW_TAG_structure_type;
+ this_lang = language_cplus;
+ }
break;
+ /* The "default" should not happen, but we mention it to
+ avoid an uninitialized warning. */
+ default:
case GDB_INDEX_SYMBOL_KIND_OTHER:
- mask = SEARCH_MODULE_DOMAIN | SEARCH_TYPE_DOMAIN;
+ tag = (dwarf_tag) DW_TAG_GDB_INDEX_OTHER;
break;
}
- if ((kind & mask) == 0)
- continue;
+
+ cooked_index_flag flags = 0;
+ if (is_static)
+ flags |= IS_STATIC;
+ if (main_name != nullptr
+ && tag == DW_TAG_subprogram
+ && strcmp (name, main_name) == 0)
+ {
+ flags |= IS_MAIN;
+ this_lang = main_lang;
+ /* Don't bother looking for another. */
+ main_name = nullptr;
+ }
+
+ /* Note that this assumes the final component ends in \0. */
+ cooked_index_entry *entry = result.add (per_cu->sect_off, tag,
+ flags, this_lang,
+ components.back ().data (),
+ nullptr, per_cu);
+ /* Don't bother pushing if we do not need a panrent. */
+ if (components.size () > 1)
+ these_entries.push_back (entry);
+
+ /* We don't care exactly which entry ends up in this
+ map. */
+ by_name[std::string_view (name)] = entry;
}
- /* Don't crash on bad data. */
- if (cu_index >= per_objfile->per_bfd->all_units.size ())
+ if (components.size () > 1)
{
- complaint (_(".gdb_index entry has bad CU index"
- " [in module %s]"), objfile_name (per_objfile->objfile));
- continue;
- }
+ std::string_view penultimate = components[components.size () - 2];
+ std::string_view prefix (name, &penultimate.back () + 1 - name);
- dwarf2_per_cu *per_cu = per_objfile->per_bfd->get_cu (cu_index);
- if (!dw2_expand_symtabs_matching_one (per_cu, per_objfile, marked,
- file_matcher, expansion_notify,
- lang_matcher))
- return false;
+ need_parents.emplace_back (prefix, std::move (these_entries));
+ }
}
- return true;
+ for (const auto &[prefix, entries] : need_parents)
+ {
+ auto iter = by_name.find (prefix);
+ /* If we can't find the parent entry, just lose. It should
+ always be there. We could synthesize it from the components,
+ if we kept those, but that seems like overkill. */
+ if (iter != by_name.end ())
+ {
+ for (cooked_index_entry *entry : entries)
+ entry->set_parent (iter->second);
+ }
+ }
}
-bool
-dwarf2_gdb_index::expand_symtabs_matching
- (objfile *objfile,
- expand_symtabs_file_matcher file_matcher,
- const lookup_name_info *lookup_name,
- expand_symtabs_symbol_matcher symbol_matcher,
- expand_symtabs_expansion_listener expansion_notify,
- block_search_flags search_flags,
- domain_search_flags domain,
- expand_symtabs_lang_matcher lang_matcher)
+/* The worker that reads a mapped index and fills in a
+ cooked_index_worker_result. */
+
+class gdb_index_worker : public cooked_index_worker
{
- dwarf2_per_objfile *per_objfile = get_dwarf2_per_objfile (objfile);
+public:
- auto_bool_vector marked;
- dw_expand_symtabs_matching_file_matcher (per_objfile, marked, file_matcher);
+ gdb_index_worker (dwarf2_per_objfile *per_objfile,
+ std::unique_ptr<mapped_gdb_index> map)
+ : cooked_index_worker (per_objfile),
+ map (std::move (map))
+ { }
- /* This invariant is documented in quick-functions.h. */
- gdb_assert (lookup_name != nullptr || symbol_matcher == nullptr);
- if (lookup_name == nullptr)
- {
- for (dwarf2_per_cu *per_cu : all_units_range (per_objfile->per_bfd))
- {
- QUIT;
+ void do_reading () override;
- if (!dw2_expand_symtabs_matching_one (per_cu, per_objfile,
- marked, file_matcher,
- expansion_notify,
- lang_matcher))
- return false;
- }
- return true;
- }
+ /* The map we're reading. */
+ std::unique_ptr<mapped_gdb_index> map;
+};
+
+void
+gdb_index_worker::do_reading ()
+{
+ complaint_interceptor complaint_handler;
+ map->build_name_components (m_per_objfile);
- mapped_gdb_index &index
- = *(gdb::checked_static_cast<mapped_gdb_index *>
- (per_objfile->per_bfd->index_table.get ()));
+ m_results.push_back (std::move (map->result));
+ m_results[0].done_reading (complaint_handler.release ());
- bool result
- = dw2_expand_symtabs_matching_symbol (index, *lookup_name,
- symbol_matcher,
- [&] (offset_type idx)
- {
- if (!dw2_expand_marked_cus (per_objfile, idx, marked, file_matcher,
- expansion_notify, search_flags, domain,
- lang_matcher))
- return false;
- return true;
- }, per_objfile, lang_matcher);
+ /* No longer needed. */
+ map.reset ();
- return result;
-}
+ done_reading ();
-quick_symbol_functions_up
-mapped_gdb_index::make_quick_functions () const
-{
- return quick_symbol_functions_up (new dwarf2_gdb_index);
+ bfd_thread_cleanup ();
}
/* A helper function that reads the .gdb_index from BUFFER and fills
@@ -1208,11 +427,9 @@ read_gdb_index_from_buffer (const char *filename,
/* Version check. */
offset_type version = metadata[0];
- /* Versions earlier than 3 emitted every copy of a psymbol. This
- causes the index to behave very poorly for certain requests. Version 3
- contained incomplete addrmap. So, it seems better to just ignore such
- indices. */
- if (version < 4)
+ /* GDB now requires the symbol attributes, which were added in
+ version 7. */
+ if (version < 7)
{
static int warning_printed = 0;
if (!warning_printed)
@@ -1223,30 +440,6 @@ read_gdb_index_from_buffer (const char *filename,
}
return 0;
}
- /* Index version 4 uses a different hash function than index version
- 5 and later.
-
- Versions earlier than 6 did not emit psymbols for inlined
- functions. Using these files will cause GDB not to be able to
- set breakpoints on inlined functions by name, so we ignore these
- indices unless the user has done
- "set use-deprecated-index-sections on". */
- if (version < 6 && !deprecated_ok)
- {
- static int warning_printed = 0;
- if (!warning_printed)
- {
- warning (_("\
-Skipping deprecated .gdb_index section in %s.\n\
-Do \"%ps\" before the file is read\n\
-to use the section anyway."),
- filename,
- styled_string (command_style.style (),
- "set use-deprecated-index-sections on"));
- warning_printed = 1;
- }
- return 0;
- }
/* Version 7 indices generated by gold refer to the CU for a symbol instead
of the TU (for symbols coming from TUs),
http://sourceware.org/bugzilla/show_bug.cgi?id=15021.
@@ -1431,23 +624,19 @@ create_addrmap_from_gdb_index (dwarf2_per_objfile *per_objfile,
mutable_map.set_empty (lo, hi - 1, per_bfd->get_cu (cu_index));
}
- index->index_addrmap
- = new (&per_bfd->obstack) addrmap_fixed (&per_bfd->obstack, &mutable_map);
+ index->result.set_addrmap (std::move (mutable_map));
}
-/* Sets the name and language of the main function from the shortcut table. */
-
-static void
-set_main_name_from_gdb_index (dwarf2_per_objfile *per_objfile,
- mapped_gdb_index *index)
+void
+mapped_gdb_index::set_main_name (dwarf2_per_objfile *per_objfile)
{
const auto expected_size = 2 * sizeof (offset_type);
- if (index->shortcut_table.size () < expected_size)
+ if (this->shortcut_table.size () < expected_size)
/* The data in the section is not present, is corrupted or is in a version
we don't know about. Regardless, we can't make use of it. */
return;
- auto ptr = index->shortcut_table.data ();
+ auto ptr = this->shortcut_table.data ();
const auto dw_lang = extract_unsigned_integer (ptr, 4, BFD_ENDIAN_LITTLE);
if (dw_lang >= DW_LANG_hi_user)
{
@@ -1463,13 +652,11 @@ set_main_name_from_gdb_index (dwarf2_per_objfile *per_objfile,
}
ptr += 4;
- const auto lang = dwarf_lang_to_enum_language (dw_lang);
+ main_lang = dwarf_lang_to_enum_language (dw_lang);
const auto name_offset = extract_unsigned_integer (ptr,
sizeof (offset_type),
BFD_ENDIAN_LITTLE);
- const auto name = (const char *) (index->constant_pool.data () + name_offset);
-
- set_objfile_main_name (per_objfile->objfile, name, (enum language) lang);
+ main_name = (const char *) (this->constant_pool.data () + name_offset);
}
/* See read-gdb-index.h. */
@@ -1557,9 +744,15 @@ dwarf2_read_gdb_index
create_addrmap_from_gdb_index (per_objfile, map.get ());
- set_main_name_from_gdb_index (per_objfile, map.get ());
+ map->set_main_name (per_objfile);
- per_bfd->index_table = std::move (map);
+ int version = map->version;
+ auto worker = std::make_unique<gdb_index_worker> (per_objfile,
+ std::move (map));
+ auto idx = std::make_unique<cooked_gdb_index> (std::move (worker),
+ version);
+
+ per_bfd->start_reading (std::move (idx));
return true;
}
@@ -1580,9 +773,4 @@ Warning: This option must be enabled before gdb reads the file."),
NULL,
NULL,
&setlist, &showlist);
-
-#if GDB_SELF_TEST
- selftests::register_test ("dw2_expand_symtabs_matching",
- selftests::dw2_expand_symtabs_matching::run_test);
-#endif
}
diff --git a/gdb/dwarf2/read-gdb-index.h b/gdb/dwarf2/read-gdb-index.h
index e38a8318db8fac78336208097bac288f8321be3f..f605985f648104d4508e934e727c608eb0928bff 100644
--- a/gdb/dwarf2/read-gdb-index.h
+++ b/gdb/dwarf2/read-gdb-index.h
@@ -27,6 +27,20 @@ struct dwarf2_per_objfile;
struct dwz_file;
struct objfile;
+/* .gdb_index doesn't distinguish between the various "other" symbols
+ -- but the symbol search machinery really wants to. For example,
+ an imported decl is "other" but is really a namespace and thus in
+ TYPE_DOMAIN; whereas a Fortran module is also "other" but is in the
+ MODULE_DOMAIN. We use this value internally to represent the
+ "other" case so that matching can work. The exact value does not
+ matter, all that matters here is that it won't overlap with any
+ symbol that gdb might create. */
+#define DW_TAG_GDB_INDEX_OTHER 0xffff
+
+/* Similarly, .gdb_index can't distinguish between the type and struct
+ domains. This is a special tag that inhabits both. */
+#define DW_TAG_GDB_INDEX_TYPE 0xfffe
+
/* Callback types for dwarf2_read_gdb_index. */
typedef gdb::function_view
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index f1084530221e82f33efb35f542d6d842fe3df2ab..6eee9631623435cbb39830116b790ba4b1f0c21d 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -14570,13 +14570,26 @@ cooked_index_functions::expand_symtabs_matching
continue;
}
+ /* This is a bit of a hack to support .gdb_index. Since
+ .gdb_index does not record languages, and since we want
+ to know the language to avoid excessive CU expansion due
+ to false matches, if we see a symbol with an unknown
+ language we find the CU's language. Only the .gdb_index
+ reader creates such symbols. */
+ enum language entry_lang = entry->lang;
+ if (entry_lang == language_unknown)
+ {
+ entry->per_cu->ensure_lang (per_objfile);
+ entry_lang = entry->per_cu->lang ();
+ }
+
/* We've found the base name of the symbol; now walk its
parentage chain, ensuring that each component
matches. */
bool found = true;
const cooked_index_entry *parent = entry->get_parent ();
- const language_defn *lang_def = language_def (entry->lang);
+ const language_defn *lang_def = language_def (entry_lang);
for (int i = name_vec.size () - 1; i > 0; --i)
{
/* If we ran out of entries, or if this segment doesn't
@@ -14586,17 +14599,15 @@ cooked_index_functions::expand_symtabs_matching
found = false;
break;
}
- if (parent->lang != language_unknown)
+
+ symbol_name_matcher_ftype *name_matcher
+ = (lang_def->get_symbol_name_matcher
+ (segment_lookup_names[i-1]));
+ if (!name_matcher (parent->canonical,
+ segment_lookup_names[i-1], nullptr))
{
- symbol_name_matcher_ftype *name_matcher
- = lang_def->get_symbol_name_matcher
- (segment_lookup_names[i-1]);
- if (!name_matcher (parent->canonical,
- segment_lookup_names[i-1], nullptr))
- {
- found = false;
- break;
- }
+ found = false;
+ break;
}
parent = parent->get_parent ();
@@ -14619,23 +14630,20 @@ cooked_index_functions::expand_symtabs_matching
seems like the loop above could just examine every
element of the name, avoiding the need to check here; but
this is hard. See PR symtab/32733. */
- if (symbol_matcher != nullptr || entry->lang != language_unknown)
+ auto_obstack temp_storage;
+ const char *full_name = entry->full_name (&temp_storage,
+ FOR_ADA_LINKAGE_NAME);
+ if (symbol_matcher == nullptr)
{
- auto_obstack temp_storage;
- const char *full_name = entry->full_name (&temp_storage,
- FOR_ADA_LINKAGE_NAME);
- if (symbol_matcher == nullptr)
- {
- symbol_name_matcher_ftype *name_matcher
- = (lang_def->get_symbol_name_matcher
- (lookup_name_without_params));
- if (!name_matcher (full_name, lookup_name_without_params,
- nullptr))
- continue;
- }
- else if (!symbol_matcher (full_name))
+ symbol_name_matcher_ftype *name_matcher
+ = (lang_def->get_symbol_name_matcher
+ (lookup_name_without_params));
+ if (!name_matcher (full_name, lookup_name_without_params,
+ nullptr))
continue;
}
+ else if (!symbol_matcher (full_name))
+ continue;
if (!dw2_expand_symtabs_matching_one (entry->per_cu, per_objfile,
marked, file_matcher,
diff --git a/gdb/dwarf2/tag.h b/gdb/dwarf2/tag.h
index ef1ad217c6d34fbacdf5f6372b0d29ad3b6e7bbd..e2f056033702c303f3b3354810e09fe27126625d 100644
--- a/gdb/dwarf2/tag.h
+++ b/gdb/dwarf2/tag.h
@@ -22,6 +22,7 @@
#include "dwarf2.h"
#include "symtab.h"
+#include "read-gdb-index.h"
/* Return true if TAG represents a type, false otherwise. */
@@ -144,6 +145,13 @@ tag_matches_domain (dwarf_tag tag, domain_search_flags search, language lang)
else
flags = SEARCH_MODULE_DOMAIN;
break;
+
+ case DW_TAG_GDB_INDEX_OTHER:
+ flags = SEARCH_MODULE_DOMAIN | SEARCH_TYPE_DOMAIN;
+ break;
+ case DW_TAG_GDB_INDEX_TYPE:
+ flags = SEARCH_STRUCT_DOMAIN | SEARCH_TYPE_DOMAIN;
+ break;
}
return (flags & search) != 0;
--
2.46.1
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH v2 11/28] Rewrite the .gdb_index reader
2025-04-02 23:45 ` [PATCH v2 11/28] Rewrite the .gdb_index reader Tom Tromey
@ 2025-04-23 17:22 ` Simon Marchi
2025-04-23 20:50 ` Tom Tromey
2025-04-24 14:37 ` Pedro Alves
1 sibling, 1 reply; 50+ messages in thread
From: Simon Marchi @ 2025-04-23 17:22 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
On 2025-04-02 19:45, Tom Tromey wrote:
> This patch rewrites the .gdb_index reader to create the same data
> structures that are created by the cooked indexer and the .debug_names
> reader.
>
> This is done in support of this series; but also because, from what I
> can tell, the "templates.exp" change didn't really work properly with
> this reader.
>
> In addition to fixing that problem, this patch removes a lot of code.
>
> Implementing this required a couple of hacks, as .gdb_index does not
> contain all the information that's used by the cooked index
> implementation.
>
> * The index-searching code likes to differentiate between the various
> DWARF tags when matching, but .gdb_index lumps many things into a
> single "other" category. To handle this, we introduce a phony tag
> that's used so that the match method can match on multiple domains.
>
> * Similarly, .gdb_index doesn't distinguish between the type and
> struct domains, so another phony tag is used for this.
>
> * Support for older versions of .gdb_index is removed entirely.
>
> * The reader must attempt to guess the language of various symbols.
> This is somewhat finicky. "Plain" (unqualified) symbols are marked
> as language_unknown and then a couple of hacks are used to handle
> these -- one in expand_symtabs_matching and another when recognizing
> "main".
>
> For what it's worth, I consider .gdb_index to be near the end of its
> life. While .debug_names is not perfect -- we found a number of bugs
> in the standard while implementing it -- it is better than .gdb_index
> and also better documented.
>
> After this patch, we could conceivably remove dwarf_scanner_base.
> However, I have not done this.
>
> Finally, this patch also changes this reader to dump the content of
> the index, as the other DWARF readers do. This can be handy when
> debugging gdb.
I'm not knowledgeable enough with the .gdb_index quirks to have an
informed opinion about this. I am happy with this change because it
brings it in line with the other two we already have (.debug_info and
.debug_names).
I noted some random comments below.
> diff --git a/gdb/dwarf2/cooked-index-shard.c b/gdb/dwarf2/cooked-index-shard.c
> index 29a8aea513786e4c1c1ed77dee8610fc329d1c8a..888a0fa345b9124e2814aa722e71a9d2fd5adf8e 100644
> --- a/gdb/dwarf2/cooked-index-shard.c
> +++ b/gdb/dwarf2/cooked-index-shard.c
> @@ -86,7 +86,16 @@ cooked_index_shard::add (sect_offset die_offset, enum dwarf_tag tag,
> implicit "main" discovery. */
> if ((flags & IS_MAIN) != 0)
> m_main = result;
> - else if ((flags & IS_PARENT_DEFERRED) == 0
> + /* The language check here is subtle: it exists solely to work
> + around a bug in .gdb_index. That index does not record
> + languages, but it might emit an entry for "main". However,
> + recognizing this "main" as being the main program would be wrong
> + -- for example, an Ada program has a C "main" but this is not the
> + desired target of the "start" command. Requiring the language to
> + be set here avoids over-eagerly setting the "main" when using
> + .gdb_index. Should .gdb_index ever be removed (PR symtab/31363),
> + the language_unknown check here could also be removed. */
> + else if (lang != language_unknown
Did you remove the IS_PARENT_DEFERRED check on purpose?
> @@ -203,10 +209,10 @@ enum class cooked_state
> /* An object of this type controls the scanning of the DWARF. It
> schedules the worker tasks and tracks the current state. Once
> scanning is done, this object is discarded.
> -
> +
Trailing spaces ^.
> +/* This is like a cooked index, but as it has been ingested from
> + .gdb_index, it can't be used to write out an index. */
> +
> +class cooked_gdb_index : public cooked_index
Technically, I think it should be "This is a cooked index as ingested
from .gdb_index". You know, the "is-a" relationship. Currently, the
base class is the "from .debug_info" case and we have some derived
classes for the indexes. I feel like we should (eventually) move to
having an abstract base class and add a derived class for the "from
.debug_info" case.
> + /* Note that this assumes the final component ends in \0. */
> + cooked_index_entry *entry = result.add (per_cu->sect_off, tag,
> + flags, this_lang,
> + components.back ().data (),
> + nullptr, per_cu);
> + /* Don't bother pushing if we do not need a panrent. */
panrent -> parent
Simon
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH v2 11/28] Rewrite the .gdb_index reader
2025-04-02 23:45 ` [PATCH v2 11/28] Rewrite the .gdb_index reader Tom Tromey
2025-04-23 17:22 ` Simon Marchi
@ 2025-04-24 14:37 ` Pedro Alves
1 sibling, 0 replies; 50+ messages in thread
From: Pedro Alves @ 2025-04-24 14:37 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
Hi!
On 2025-04-03 00:45, Tom Tromey wrote:
> * Support for older versions of .gdb_index is removed entirely.
This deserves a NEWS entry.
Pedro Alves
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH v2 12/28] Convert default_collect_symbol_completion_matches_break_on
2025-04-02 23:44 [PATCH v2 00/28] Search symbols via quick API Tom Tromey
` (10 preceding siblings ...)
2025-04-02 23:45 ` [PATCH v2 11/28] Rewrite the .gdb_index reader Tom Tromey
@ 2025-04-02 23:45 ` Tom Tromey
2025-04-02 23:45 ` [PATCH v2 13/28] Convert gdbpy_lookup_static_symbols Tom Tromey
` (17 subsequent siblings)
29 siblings, 0 replies; 50+ messages in thread
From: Tom Tromey @ 2025-04-02 23:45 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This converts default_collect_symbol_completion_matches_break_on to
the callback approach, merging the search loop and the call to
expand_symtabs_matching.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=16994
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=16998
---
gdb/symtab.c | 33 ++++++++++++++-------------------
1 file changed, 14 insertions(+), 19 deletions(-)
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 06e2cd5b87756a2d53c6a4c427cec1e44cdeddb0..b52fa07da8737c39d39d5c86bb380beb8c81ea62 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -6130,25 +6130,20 @@ default_collect_symbol_completion_matches_break_on
/* Add completions for all currently loaded symbol tables. */
for (objfile *objfile : current_program_space->objfiles ())
{
- for (compunit_symtab *cust : objfile->compunits ())
- add_symtab_completions (cust, tracker, mode, lookup_name,
- sym_text, word, code);
- }
-
- /* Look through the partial symtabs for all symbols which begin by
- matching SYM_TEXT. Expand all CUs that you find to the list. */
- expand_symtabs_matching (NULL,
- lookup_name,
- NULL,
- [&] (compunit_symtab *symtab) /* expansion notify */
- {
- add_symtab_completions (symtab,
- tracker, mode, lookup_name,
- sym_text, word, code);
- return true;
- },
- SEARCH_GLOBAL_BLOCK | SEARCH_STATIC_BLOCK,
- SEARCH_ALL_DOMAINS);
+ /* Look through the partial symtabs for all symbols which begin by
+ matching SYM_TEXT. Expand all CUs that you find to the list. */
+ objfile->expand_symtabs_matching
+ (nullptr, &lookup_name, nullptr,
+ [&] (compunit_symtab *symtab)
+ {
+ add_symtab_completions (symtab,
+ tracker, mode, lookup_name,
+ sym_text, word, code);
+ return true;
+ },
+ SEARCH_GLOBAL_BLOCK | SEARCH_STATIC_BLOCK,
+ SEARCH_ALL_DOMAINS);
+ }
/* Search upwards from currently selected frame (so that we can
complete on local vars). Also catch fields of types defined in
--
2.46.1
^ permalink raw reply [flat|nested] 50+ messages in thread* [PATCH v2 13/28] Convert gdbpy_lookup_static_symbols
2025-04-02 23:44 [PATCH v2 00/28] Search symbols via quick API Tom Tromey
` (11 preceding siblings ...)
2025-04-02 23:45 ` [PATCH v2 12/28] Convert default_collect_symbol_completion_matches_break_on Tom Tromey
@ 2025-04-02 23:45 ` Tom Tromey
2025-04-02 23:45 ` [PATCH v2 14/28] Convert ada_add_global_exceptions Tom Tromey
` (16 subsequent siblings)
29 siblings, 0 replies; 50+ messages in thread
From: Tom Tromey @ 2025-04-02 23:45 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This changes gdbpy_lookup_static_symbols to the callback approach,
merging the search loop and the call to expand_symtabs_matching.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=16994
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=16998
---
gdb/python/py-symbol.c | 29 ++++++++++++++++++++---------
1 file changed, 20 insertions(+), 9 deletions(-)
diff --git a/gdb/python/py-symbol.c b/gdb/python/py-symbol.c
index 3028a307205eb9ea70dd0c5f7fcbeda01d89b96d..5268b5a242c4cab801e342b2580fa8d99fdbbd1d 100644
--- a/gdb/python/py-symbol.c
+++ b/gdb/python/py-symbol.c
@@ -605,17 +605,17 @@ gdbpy_lookup_static_symbols (PyObject *self, PyObject *args, PyObject *kw)
/* Expand any symtabs that contain potentially matching symbols. */
lookup_name_info lookup_name (name, symbol_name_match_type::FULL);
- expand_symtabs_matching (NULL, lookup_name, NULL, NULL,
- SEARCH_STATIC_BLOCK, flags);
for (objfile *objfile : current_program_space->objfiles ())
{
- for (compunit_symtab *cust : objfile->compunits ())
+ bool error_found = false;
+
+ auto callback = [&] (compunit_symtab *cust)
{
/* Skip included compunits to prevent including compunits from
being searched twice. */
if (cust->user != nullptr)
- continue;
+ return true;
const struct blockvector *bv = cust->blockvector ();
const struct block *block = bv->static_block ();
@@ -628,13 +628,24 @@ gdbpy_lookup_static_symbols (PyObject *self, PyObject *args, PyObject *kw)
if (symbol != nullptr)
{
PyObject *sym_obj = symbol_to_symbol_object (symbol);
- if (sym_obj == nullptr)
- return nullptr;
- if (PyList_Append (return_list.get (), sym_obj) == -1)
- return nullptr;
+ if (sym_obj == nullptr
+ || PyList_Append (return_list.get (), sym_obj) == -1)
+ {
+ error_found = true;
+ return false;
+ }
}
}
- }
+
+ return true;
+ };
+
+ objfile->expand_symtabs_matching
+ (nullptr, &lookup_name, nullptr, callback,
+ SEARCH_STATIC_BLOCK, flags);
+
+ if (error_found)
+ return nullptr;
}
}
catch (const gdb_exception &except)
--
2.46.1
^ permalink raw reply [flat|nested] 50+ messages in thread* [PATCH v2 14/28] Convert ada_add_global_exceptions
2025-04-02 23:44 [PATCH v2 00/28] Search symbols via quick API Tom Tromey
` (12 preceding siblings ...)
2025-04-02 23:45 ` [PATCH v2 13/28] Convert gdbpy_lookup_static_symbols Tom Tromey
@ 2025-04-02 23:45 ` Tom Tromey
2025-04-02 23:45 ` [PATCH v2 15/28] Convert ada_language_defn::collect_symbol_completion_matches Tom Tromey
` (15 subsequent siblings)
29 siblings, 0 replies; 50+ messages in thread
From: Tom Tromey @ 2025-04-02 23:45 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This converts ada_add_global_exceptions to the callback approach,
merging the search loop and the call to expand_symtabs_matching.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=16994
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=16998
---
gdb/ada-lang.c | 49 ++++++++++++++++++++++++++-----------------------
1 file changed, 26 insertions(+), 23 deletions(-)
diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 4bb6a808fd8c1a7f8e4b2344fdf935f94c602ed1..0a20bfb8c68666326b92a6b9298df09dedcf845c 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -13098,32 +13098,12 @@ ada_add_global_exceptions (compiled_regex *preg,
return preg == nullptr || preg->exec (name, 0, NULL, 0) == 0;
};
-
- /* In Ada, the symbol "search name" is a linkage name, whereas the
- regular expression used to do the matching refers to the natural
- name. So match against the decoded name. */
- expand_symtabs_matching (NULL,
- lookup_name_info::match_any (),
- [&] (const char *search_name)
- {
- std::string decoded = ada_decode (search_name);
- return name_matches_regex (decoded.c_str ());
- },
- NULL,
- SEARCH_GLOBAL_BLOCK | SEARCH_STATIC_BLOCK,
- SEARCH_VAR_DOMAIN,
- [&] (enum language lang)
- {
- /* Try to skip non-Ada CUs. */
- return lang == language_ada;
- });
-
/* Iterate over all objfiles irrespective of scope or linker namespaces
so we get all exceptions anywhere in the progspace. */
for (objfile *objfile : current_program_space->objfiles ())
{
- for (compunit_symtab *s : objfile->compunits ())
- {
+ auto callback = [&] (compunit_symtab *s)
+ {
const struct blockvector *bv = s->blockvector ();
int i;
@@ -13141,7 +13121,30 @@ ada_add_global_exceptions (compiled_regex *preg,
exceptions->push_back (info);
}
}
- }
+
+ return true;
+ };
+
+ /* In Ada, the symbol "search name" is a linkage name, whereas
+ the regular expression used to do the matching refers to the
+ natural name. So match against the decoded name. */
+ auto any = lookup_name_info::match_any ();
+ objfile->expand_symtabs_matching
+ (nullptr,
+ &any,
+ [&] (const char *search_name)
+ {
+ std::string decoded = ada_decode (search_name);
+ return name_matches_regex (decoded.c_str ());
+ },
+ callback,
+ SEARCH_GLOBAL_BLOCK | SEARCH_STATIC_BLOCK,
+ SEARCH_VAR_DOMAIN,
+ [&] (enum language lang)
+ {
+ /* Try to skip non-Ada CUs. */
+ return lang == language_ada;
+ });
}
}
--
2.46.1
^ permalink raw reply [flat|nested] 50+ messages in thread* [PATCH v2 15/28] Convert ada_language_defn::collect_symbol_completion_matches
2025-04-02 23:44 [PATCH v2 00/28] Search symbols via quick API Tom Tromey
` (13 preceding siblings ...)
2025-04-02 23:45 ` [PATCH v2 14/28] Convert ada_add_global_exceptions Tom Tromey
@ 2025-04-02 23:45 ` Tom Tromey
2025-04-02 23:45 ` [PATCH v2 16/28] Convert ada-lang.c:map_matching_symbols Tom Tromey
` (14 subsequent siblings)
29 siblings, 0 replies; 50+ messages in thread
From: Tom Tromey @ 2025-04-02 23:45 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This converts ada_language_defn::collect_symbol_completion_matches to
the callback approach, merging the search loop and the call to
expand_symtabs_matching.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=16994
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=16998
---
gdb/ada-lang.c | 67 +++++++++++++++++++++++-----------------------------------
1 file changed, 27 insertions(+), 40 deletions(-)
diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 0a20bfb8c68666326b92a6b9298df09dedcf845c..3dc48125dee8aa424bc787a89323264d75837926 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -13652,20 +13652,12 @@ class ada_language : public language_defn
const char *text, const char *word,
enum type_code code) const override
{
- const struct block *b, *surrounding_static_block = 0;
+ const struct block *surrounding_static_block = 0;
gdb_assert (code == TYPE_CODE_UNDEF);
lookup_name_info lookup_name (text, name_match_type, true);
- /* First, look at the partial symtab symbols. */
- expand_symtabs_matching (NULL,
- lookup_name,
- NULL,
- NULL,
- SEARCH_GLOBAL_BLOCK | SEARCH_STATIC_BLOCK,
- SEARCH_ALL_DOMAINS);
-
/* At this point scan through the misc symbol vectors and add each
symbol you find to the list. Eventually we want to ignore
anything that isn't a text symbol (everything else will be
@@ -13707,7 +13699,9 @@ class ada_language : public language_defn
/* Search upwards from currently selected frame (so that we can
complete on local vars. */
- for (b = get_selected_block (0); b != NULL; b = b->superblock ())
+ for (const block *b = get_selected_block (0);
+ b != nullptr;
+ b = b->superblock ())
{
if (b->is_static_block ())
surrounding_static_block = b; /* For elmin of dups */
@@ -13729,43 +13723,36 @@ class ada_language : public language_defn
for (objfile *objfile : current_program_space->objfiles ())
{
- for (compunit_symtab *s : objfile->compunits ())
+ auto callback = [&] (compunit_symtab *s)
{
QUIT;
- b = s->blockvector ()->global_block ();
- for (struct symbol *sym : block_iterator_range (b))
+ for (const block *b = s->blockvector ()->static_block ();
+ b != nullptr;
+ b = b->superblock ())
{
- if (completion_skip_symbol (mode, sym))
- continue;
+ /* Don't do this block twice. */
+ if (b == surrounding_static_block)
+ break;
+
+ for (struct symbol *sym : block_iterator_range (b))
+ {
+ if (completion_skip_symbol (mode, sym))
+ continue;
- completion_list_add_name (tracker,
- sym->language (),
- sym->linkage_name (),
- lookup_name, text, word);
+ completion_list_add_name (tracker,
+ sym->language (),
+ sym->linkage_name (),
+ lookup_name, text, word);
+ }
}
- }
- }
- for (objfile *objfile : current_program_space->objfiles ())
- {
- for (compunit_symtab *s : objfile->compunits ())
- {
- QUIT;
- b = s->blockvector ()->static_block ();
- /* Don't do this block twice. */
- if (b == surrounding_static_block)
- continue;
- for (struct symbol *sym : block_iterator_range (b))
- {
- if (completion_skip_symbol (mode, sym))
- continue;
+ return true;
+ };
- completion_list_add_name (tracker,
- sym->language (),
- sym->linkage_name (),
- lookup_name, text, word);
- }
- }
+ objfile->expand_symtabs_matching
+ (nullptr, &lookup_name, nullptr, callback,
+ SEARCH_GLOBAL_BLOCK | SEARCH_STATIC_BLOCK,
+ SEARCH_ALL_DOMAINS);
}
}
--
2.46.1
^ permalink raw reply [flat|nested] 50+ messages in thread* [PATCH v2 16/28] Convert ada-lang.c:map_matching_symbols
2025-04-02 23:44 [PATCH v2 00/28] Search symbols via quick API Tom Tromey
` (14 preceding siblings ...)
2025-04-02 23:45 ` [PATCH v2 15/28] Convert ada_language_defn::collect_symbol_completion_matches Tom Tromey
@ 2025-04-02 23:45 ` Tom Tromey
2025-04-02 23:45 ` [PATCH v2 17/28] Remove expand_symtabs_matching Tom Tromey
` (13 subsequent siblings)
29 siblings, 0 replies; 50+ messages in thread
From: Tom Tromey @ 2025-04-02 23:45 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This converts ada-lang.c:map_matching_symbols to the callback
approach, merging the search loop and the call to
expand_symtabs_matching.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=16994
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=16998
---
gdb/ada-lang.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 3dc48125dee8aa424bc787a89323264d75837926..a8fc70688591d2f4bcd00a21fd164c386d4269d8 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -5529,22 +5529,22 @@ map_matching_symbols (struct objfile *objfile,
match_data &data)
{
data.objfile = objfile;
- objfile->expand_symtabs_matching (nullptr, &lookup_name,
- nullptr, nullptr,
- global
- ? SEARCH_GLOBAL_BLOCK
- : SEARCH_STATIC_BLOCK,
- domain);
const int block_kind = global ? GLOBAL_BLOCK : STATIC_BLOCK;
- for (compunit_symtab *symtab : objfile->compunits ())
+ auto callback = [&] (compunit_symtab *symtab)
{
const struct block *block
= symtab->blockvector ()->block (block_kind);
- if (!iterate_over_symbols_terminated (block, lookup_name,
- domain, data))
- break;
- }
+ return iterate_over_symbols_terminated (block, lookup_name,
+ domain, data);
+ };
+
+ objfile->expand_symtabs_matching (nullptr, &lookup_name,
+ nullptr, callback,
+ global
+ ? SEARCH_GLOBAL_BLOCK
+ : SEARCH_STATIC_BLOCK,
+ domain);
}
/* Add to RESULT all non-local symbols whose name and domain match
--
2.46.1
^ permalink raw reply [flat|nested] 50+ messages in thread* [PATCH v2 17/28] Remove expand_symtabs_matching
2025-04-02 23:44 [PATCH v2 00/28] Search symbols via quick API Tom Tromey
` (15 preceding siblings ...)
2025-04-02 23:45 ` [PATCH v2 16/28] Convert ada-lang.c:map_matching_symbols Tom Tromey
@ 2025-04-02 23:45 ` Tom Tromey
2025-04-02 23:45 ` [PATCH v2 18/28] Simplify basic_lookup_transparent_type Tom Tromey
` (12 subsequent siblings)
29 siblings, 0 replies; 50+ messages in thread
From: Tom Tromey @ 2025-04-02 23:45 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
The last caller of the global expand_symtabs_matching function has
been changed, so now we can remove this function.
---
gdb/symfile.c | 25 -------------------------
gdb/symfile.h | 9 ---------
2 files changed, 34 deletions(-)
diff --git a/gdb/symfile.c b/gdb/symfile.c
index b0c178a5aeb75c23862d6dd25a9e4bc6235ee231..8e229719b39f6cc4081448e5ec5fa82a0ead691f 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -3756,31 +3756,6 @@ symfile_free_objfile (struct objfile *objfile)
objfile->pspace ()->remove_target_sections (objfile);
}
-/* Wrapper around the quick_symbol_functions expand_symtabs_matching "method".
- Expand all symtabs that match the specified criteria.
- See quick_symbol_functions.expand_symtabs_matching for details. */
-
-bool
-expand_symtabs_matching (expand_symtabs_file_matcher file_matcher,
- const lookup_name_info &lookup_name,
- expand_symtabs_symbol_matcher symbol_matcher,
- expand_symtabs_expansion_listener expansion_notify,
- block_search_flags search_flags,
- domain_search_flags domain,
- expand_symtabs_lang_matcher lang_matcher)
-{
- for (objfile *objfile : current_program_space->objfiles ())
- if (!objfile->expand_symtabs_matching (file_matcher,
- &lookup_name,
- symbol_matcher,
- expansion_notify,
- search_flags,
- domain,
- lang_matcher))
- return false;
- return true;
-}
-
/* Wrapper around the quick_symbol_functions map_symbol_filenames "method".
Map function FUN over every file.
See quick_symbol_functions.map_symbol_filenames for details. */
diff --git a/gdb/symfile.h b/gdb/symfile.h
index c2e5142c045dab72fad9d73abd259813f9caef1e..e72030f5f0e1e5e0b094d288e4a81c58264222b8 100644
--- a/gdb/symfile.h
+++ b/gdb/symfile.h
@@ -345,15 +345,6 @@ symfile_segment_data_up get_symfile_segment_data (bfd *abfd);
extern scoped_restore_tmpl<int> increment_reading_symtab (void);
-bool expand_symtabs_matching
- (expand_symtabs_file_matcher file_matcher,
- const lookup_name_info &lookup_name,
- expand_symtabs_symbol_matcher symbol_matcher,
- expand_symtabs_expansion_listener expansion_notify,
- block_search_flags search_flags,
- domain_search_flags kind,
- expand_symtabs_lang_matcher lang_matcher = nullptr);
-
void map_symbol_filenames (symbol_filename_listener fun, bool need_fullname);
/* Target-agnostic function to load the sections of an executable into memory.
--
2.46.1
^ permalink raw reply [flat|nested] 50+ messages in thread* [PATCH v2 18/28] Simplify basic_lookup_transparent_type
2025-04-02 23:44 [PATCH v2 00/28] Search symbols via quick API Tom Tromey
` (16 preceding siblings ...)
2025-04-02 23:45 ` [PATCH v2 17/28] Remove expand_symtabs_matching Tom Tromey
@ 2025-04-02 23:45 ` Tom Tromey
2025-04-02 23:45 ` [PATCH v2 19/28] Remove objfile::expand_symtabs_for_function Tom Tromey
` (11 subsequent siblings)
29 siblings, 0 replies; 50+ messages in thread
From: Tom Tromey @ 2025-04-02 23:45 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This patch changes basic_lookup_transparent_type to always work via
the "quick" API -- that is, no separate search of the already-expanded
symtabs is needed.
This is more efficient when many CUs have already been expanded. It
also makes the lookup more consistent, as the result is no longer
dependent on the order in which CUs were previously expanded.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=16994
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=16998
---
gdb/symtab.c | 60 +++---------------------------------------------------------
1 file changed, 3 insertions(+), 57 deletions(-)
diff --git a/gdb/symtab.c b/gdb/symtab.c
index b52fa07da8737c39d39d5c86bb380beb8c81ea62..508374f1358e92108db7546246e2323c5d7b5905 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -2776,35 +2776,6 @@ basic_lookup_transparent_type_quick (struct objfile *objfile,
return sym->type ();
}
-/* Subroutine of basic_lookup_transparent_type to simplify it.
- Look up the non-opaque definition of NAME in BLOCK_INDEX of OBJFILE.
- BLOCK_INDEX is either GLOBAL_BLOCK or STATIC_BLOCK. */
-
-static struct type *
-basic_lookup_transparent_type_1 (struct objfile *objfile,
- enum block_enum block_index,
- domain_search_flags flags,
- const lookup_name_info &name)
-{
- const struct blockvector *bv;
- const struct block *block;
- const struct symbol *sym;
-
- for (compunit_symtab *cust : objfile->compunits ())
- {
- bv = cust->blockvector ();
- block = bv->block (block_index);
- sym = block_find_symbol (block, name, flags, nullptr);
- if (sym != nullptr)
- {
- gdb_assert (!TYPE_IS_OPAQUE (sym->type ()));
- return sym->type ();
- }
- }
-
- return NULL;
-}
-
/* The standard implementation of lookup_transparent_type. This code
was modeled on lookup_symbol -- the parts not relevant to looking
up types were just left out. In particular it's assumed here that
@@ -2818,19 +2789,7 @@ basic_lookup_transparent_type (const char *name, domain_search_flags flags)
lookup_name_info lookup_name (name, symbol_name_match_type::FULL);
- /* Now search all the global symbols. Do the symtab's first, then
- check the psymtab's. If a psymtab indicates the existence
- of the desired name as a global, then do psymtab-to-symtab
- conversion on the fly and return the found symbol. */
-
- for (objfile *objfile : current_program_space->objfiles ())
- {
- t = basic_lookup_transparent_type_1 (objfile, GLOBAL_BLOCK,
- flags, lookup_name);
- if (t)
- return t;
- }
-
+ /* Search all the global symbols. */
for (objfile *objfile : current_program_space->objfiles ())
{
t = basic_lookup_transparent_type_quick (objfile, GLOBAL_BLOCK,
@@ -2839,21 +2798,8 @@ basic_lookup_transparent_type (const char *name, domain_search_flags flags)
return t;
}
- /* Now search the static file-level symbols.
- Not strictly correct, but more useful than an error.
- Do the symtab's first, then
- check the psymtab's. If a psymtab indicates the existence
- of the desired name as a file-level static, then do psymtab-to-symtab
- conversion on the fly and return the found symbol. */
-
- for (objfile *objfile : current_program_space->objfiles ())
- {
- t = basic_lookup_transparent_type_1 (objfile, STATIC_BLOCK,
- flags, lookup_name);
- if (t)
- return t;
- }
-
+ /* Now search the static file-level symbols. Not strictly correct,
+ but more useful than an error. */
for (objfile *objfile : current_program_space->objfiles ())
{
t = basic_lookup_transparent_type_quick (objfile, STATIC_BLOCK,
--
2.46.1
^ permalink raw reply [flat|nested] 50+ messages in thread* [PATCH v2 19/28] Remove objfile::expand_symtabs_for_function
2025-04-02 23:44 [PATCH v2 00/28] Search symbols via quick API Tom Tromey
` (17 preceding siblings ...)
2025-04-02 23:45 ` [PATCH v2 18/28] Simplify basic_lookup_transparent_type Tom Tromey
@ 2025-04-02 23:45 ` Tom Tromey
2025-04-02 23:45 ` [PATCH v2 20/28] Convert linespec.c:iterate_over_all_matching_symtabs Tom Tromey
` (10 subsequent siblings)
29 siblings, 0 replies; 50+ messages in thread
From: Tom Tromey @ 2025-04-02 23:45 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
objfile::expand_symtabs_for_function only has a single caller now, so
it can be removed. This also allows us to merge the expansion and
searching phases, as done in other patches in this series.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=16994
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=16998
---
gdb/cp-support.c | 28 +++++++++++++++++-----------
gdb/objfiles.h | 4 ----
gdb/symfile-debug.c | 22 ----------------------
3 files changed, 17 insertions(+), 37 deletions(-)
diff --git a/gdb/cp-support.c b/gdb/cp-support.c
index 6dd364df1c4dc60a42c20482f195ee3f2350b5ca..22124d3f6091b53a7c1e100c4881027f716ce233 100644
--- a/gdb/cp-support.c
+++ b/gdb/cp-support.c
@@ -1455,17 +1455,15 @@ add_symbol_overload_list_qualified (const char *func_name,
? selected_block->objfile ()
: nullptr);
+ lookup_name_info base_lookup (func_name, symbol_name_match_type::FULL);
+ lookup_name_info lookup_name = base_lookup.make_ignore_params ();
+
gdbarch_iterate_over_objfiles_in_search_order
(current_objfile ? current_objfile->arch () : current_inferior ()->arch (),
- [func_name, surrounding_static_block, &overload_list]
+ [func_name, surrounding_static_block, &overload_list, lookup_name]
(struct objfile *obj)
{
- /* Look through the partial symtabs for all symbols which
- begin by matching FUNC_NAME. Make sure we read that
- symbol table in. */
- obj->expand_symtabs_for_function (func_name);
-
- for (compunit_symtab *cust : obj->compunits ())
+ auto callback = [&] (compunit_symtab *cust)
{
QUIT;
const struct block *b = cust->blockvector ()->global_block ();
@@ -1473,11 +1471,19 @@ add_symbol_overload_list_qualified (const char *func_name,
b = cust->blockvector ()->static_block ();
/* Don't do this block twice. */
- if (b == surrounding_static_block)
- continue;
+ if (b != surrounding_static_block)
+ add_symbol_overload_list_block (func_name, b, overload_list);
+ return true;
+ };
- add_symbol_overload_list_block (func_name, b, overload_list);
- }
+ /* Look through the partial symtabs for all symbols which
+ begin by matching FUNC_NAME. Make sure we read that
+ symbol table in. */
+ obj->expand_symtabs_matching (nullptr, &lookup_name,
+ nullptr, callback,
+ (SEARCH_GLOBAL_BLOCK
+ | SEARCH_STATIC_BLOCK),
+ SEARCH_FUNCTION_DOMAIN);
return 0;
}, current_objfile);
diff --git a/gdb/objfiles.h b/gdb/objfiles.h
index 05e1e01d6352f6c0295e71fd611d6d6a19853673..6e119a5071f3b7846e060e9e433ea4729a286f14 100644
--- a/gdb/objfiles.h
+++ b/gdb/objfiles.h
@@ -577,10 +577,6 @@ struct objfile : intrusive_list_node<objfile>
/* See quick_symbol_functions. */
void dump ();
- /* Find all the symbols in OBJFILE named FUNC_NAME, and ensure that
- the corresponding symbol tables are loaded. */
- void expand_symtabs_for_function (const char *func_name);
-
/* See quick_symbol_functions. */
void expand_all_symtabs ();
diff --git a/gdb/symfile-debug.c b/gdb/symfile-debug.c
index 937839856b27fa7e38d472b9dccd3c125d0af878..856b10558a3ec471a1fc0fd288041134e2cd03ac 100644
--- a/gdb/symfile-debug.c
+++ b/gdb/symfile-debug.c
@@ -310,28 +310,6 @@ objfile::dump ()
iter->dump (this);
}
-void
-objfile::expand_symtabs_for_function (const char *func_name)
-{
- if (debug_symfile)
- gdb_printf (gdb_stdlog,
- "qf->expand_symtabs_for_function (%s, \"%s\")\n",
- objfile_debug_name (this), func_name);
-
- lookup_name_info base_lookup (func_name, symbol_name_match_type::FULL);
- lookup_name_info lookup_name = base_lookup.make_ignore_params ();
-
- for (const auto &iter : qf)
- iter->expand_symtabs_matching (this,
- nullptr,
- &lookup_name,
- nullptr,
- nullptr,
- (SEARCH_GLOBAL_BLOCK
- | SEARCH_STATIC_BLOCK),
- SEARCH_FUNCTION_DOMAIN);
-}
-
void
objfile::expand_all_symtabs ()
{
--
2.46.1
^ permalink raw reply [flat|nested] 50+ messages in thread* [PATCH v2 20/28] Convert linespec.c:iterate_over_all_matching_symtabs
2025-04-02 23:44 [PATCH v2 00/28] Search symbols via quick API Tom Tromey
` (18 preceding siblings ...)
2025-04-02 23:45 ` [PATCH v2 19/28] Remove objfile::expand_symtabs_for_function Tom Tromey
@ 2025-04-02 23:45 ` Tom Tromey
2025-04-02 23:45 ` [PATCH v2 21/28] Simplify block_lookup_symbol_primary Tom Tromey
` (9 subsequent siblings)
29 siblings, 0 replies; 50+ messages in thread
From: Tom Tromey @ 2025-04-02 23:45 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This converts linespec.c:iterate_over_all_matching_symtabs to the
callback approach, merging the search loop and the call to
expand_symtabs_matching.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=16994
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=16998
---
gdb/linespec.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/gdb/linespec.c b/gdb/linespec.c
index 5eef96c44e7dc283c3855690359fd718fe2c0a55..b500930b1feeb447aac81e45ba51439a021c4b48 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -1134,12 +1134,7 @@ iterate_over_all_matching_symtabs
for (objfile *objfile : pspace->objfiles ())
{
- objfile->expand_symtabs_matching (NULL, &lookup_name, NULL, NULL,
- (SEARCH_GLOBAL_BLOCK
- | SEARCH_STATIC_BLOCK),
- domain);
-
- for (compunit_symtab *cu : objfile->compunits ())
+ auto expand_callback = [&] (compunit_symtab *cu)
{
struct symtab *symtab = cu->primary_filetab ();
@@ -1166,7 +1161,15 @@ iterate_over_all_matching_symtabs
});
}
}
- }
+
+ return true;
+ };
+
+ objfile->expand_symtabs_matching (nullptr, &lookup_name,
+ nullptr, expand_callback,
+ (SEARCH_GLOBAL_BLOCK
+ | SEARCH_STATIC_BLOCK),
+ domain);
}
}
}
--
2.46.1
^ permalink raw reply [flat|nested] 50+ messages in thread* [PATCH v2 21/28] Simplify block_lookup_symbol_primary
2025-04-02 23:44 [PATCH v2 00/28] Search symbols via quick API Tom Tromey
` (19 preceding siblings ...)
2025-04-02 23:45 ` [PATCH v2 20/28] Convert linespec.c:iterate_over_all_matching_symtabs Tom Tromey
@ 2025-04-02 23:45 ` Tom Tromey
2025-04-02 23:45 ` [PATCH v2 22/28] Pass lookup_name_info to block_lookup_symbol_primary Tom Tromey
` (8 subsequent siblings)
29 siblings, 0 replies; 50+ messages in thread
From: Tom Tromey @ 2025-04-02 23:45 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This simplifies block_lookup_symbol_primary by using
block_iterator_range.
---
gdb/block.c | 10 ++--------
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/gdb/block.c b/gdb/block.c
index cf7ca6785b0c9ef4f3d809a245261d18ffaf06d8..2c07b38f8f806b8a154eb42adbd3f53a3535c57f 100644
--- a/gdb/block.c
+++ b/gdb/block.c
@@ -721,20 +721,14 @@ struct symbol *
block_lookup_symbol_primary (const struct block *block, const char *name,
const domain_search_flags domain)
{
- struct symbol *sym, *other;
- struct mdict_iterator mdict_iter;
-
lookup_name_info lookup_name (name, symbol_name_match_type::FULL);
/* Verify BLOCK is STATIC_BLOCK or GLOBAL_BLOCK. */
gdb_assert (block->superblock () == NULL
|| block->superblock ()->superblock () == NULL);
- other = NULL;
- for (sym = mdict_iter_match_first (block->multidict (), lookup_name,
- &mdict_iter);
- sym != NULL;
- sym = mdict_iter_match_next (lookup_name, &mdict_iter))
+ symbol *other = nullptr;
+ for (symbol *sym : block_iterator_range (block, &lookup_name))
{
/* With the fix for PR gcc/debug/91507, we get for:
...
--
2.46.1
^ permalink raw reply [flat|nested] 50+ messages in thread* [PATCH v2 22/28] Pass lookup_name_info to block_lookup_symbol_primary
2025-04-02 23:44 [PATCH v2 00/28] Search symbols via quick API Tom Tromey
` (20 preceding siblings ...)
2025-04-02 23:45 ` [PATCH v2 21/28] Simplify block_lookup_symbol_primary Tom Tromey
@ 2025-04-02 23:45 ` Tom Tromey
2025-04-02 23:45 ` [PATCH v2 23/28] Simplify block_lookup_symbol Tom Tromey
` (7 subsequent siblings)
29 siblings, 0 replies; 50+ messages in thread
From: Tom Tromey @ 2025-04-02 23:45 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This changes block_lookup_symbol_primary to accept a lookup_name_info.
This follows the general trend of hoisting these objects to the
outermost layer where they make sense -- somewhat reducing the cost of
using them.
---
gdb/block.c | 7 +++----
gdb/block.h | 2 +-
gdb/symtab.c | 3 ++-
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/gdb/block.c b/gdb/block.c
index 2c07b38f8f806b8a154eb42adbd3f53a3535c57f..583c4ef1bf07ab942af71d0f1d1b38799849451d 100644
--- a/gdb/block.c
+++ b/gdb/block.c
@@ -718,17 +718,16 @@ block_lookup_symbol (const struct block *block, const lookup_name_info &name,
/* See block.h. */
struct symbol *
-block_lookup_symbol_primary (const struct block *block, const char *name,
+block_lookup_symbol_primary (const struct block *block,
+ const lookup_name_info &name,
const domain_search_flags domain)
{
- lookup_name_info lookup_name (name, symbol_name_match_type::FULL);
-
/* Verify BLOCK is STATIC_BLOCK or GLOBAL_BLOCK. */
gdb_assert (block->superblock () == NULL
|| block->superblock ()->superblock () == NULL);
symbol *other = nullptr;
- for (symbol *sym : block_iterator_range (block, &lookup_name))
+ for (symbol *sym : block_iterator_range (block, &name))
{
/* With the fix for PR gcc/debug/91507, we get for:
...
diff --git a/gdb/block.h b/gdb/block.h
index 75f5c8e93944342782343ebfb9b5b99afce1d4ff..c348c7fdcc4088e7f5c912e49c387e2c650eda45 100644
--- a/gdb/block.h
+++ b/gdb/block.h
@@ -636,7 +636,7 @@ extern struct symbol *block_lookup_symbol (const struct block *block,
extern struct symbol *block_lookup_symbol_primary
(const struct block *block,
- const char *name,
+ const lookup_name_info &name,
const domain_search_flags domain);
/* Find symbol NAME in BLOCK and in DOMAIN. This will return a
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 508374f1358e92108db7546246e2323c5d7b5905..4822b5681f8370394c9cb6ca6c38ac2214191a39 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -2365,6 +2365,7 @@ lookup_symbol_in_objfile_symtabs (struct objfile *objfile,
block_index == GLOBAL_BLOCK ? "GLOBAL_BLOCK" : "STATIC_BLOCK",
name, domain_name (domain).c_str ());
+ lookup_name_info lookup_name (name, symbol_name_match_type::FULL);
struct block_symbol other;
other.symbol = NULL;
for (compunit_symtab *cust : objfile->compunits ())
@@ -2375,7 +2376,7 @@ lookup_symbol_in_objfile_symtabs (struct objfile *objfile,
bv = cust->blockvector ();
block = bv->block (block_index);
- result.symbol = block_lookup_symbol_primary (block, name, domain);
+ result.symbol = block_lookup_symbol_primary (block, lookup_name, domain);
result.block = block;
if (result.symbol == NULL)
continue;
--
2.46.1
^ permalink raw reply [flat|nested] 50+ messages in thread* [PATCH v2 23/28] Simplify block_lookup_symbol
2025-04-02 23:44 [PATCH v2 00/28] Search symbols via quick API Tom Tromey
` (21 preceding siblings ...)
2025-04-02 23:45 ` [PATCH v2 22/28] Pass lookup_name_info to block_lookup_symbol_primary Tom Tromey
@ 2025-04-02 23:45 ` Tom Tromey
2025-04-02 23:45 ` [PATCH v2 24/28] Add best_symbol_tracker Tom Tromey
` (6 subsequent siblings)
29 siblings, 0 replies; 50+ messages in thread
From: Tom Tromey @ 2025-04-02 23:45 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
One loop in block_lookup_symbol is identical to the code in
block_lookup_symbol_primary. This patch simplifies the former by
having it call the latter.
This removes an assert. However, note that the assert is not needed
-- it does not check any invariant that must be maintained.
---
gdb/block.c | 23 +----------------------
1 file changed, 1 insertion(+), 22 deletions(-)
diff --git a/gdb/block.c b/gdb/block.c
index 583c4ef1bf07ab942af71d0f1d1b38799849451d..d8f8bf56de7b1498ae3f89650a3c86c4eafe3bfe 100644
--- a/gdb/block.c
+++ b/gdb/block.c
@@ -669,24 +669,7 @@ block_lookup_symbol (const struct block *block, const lookup_name_info &name,
const domain_search_flags domain)
{
if (!block->function ())
- {
- struct symbol *other = NULL;
-
- for (struct symbol *sym : block_iterator_range (block, &name))
- {
- /* See comment related to PR gcc/debug/91507 in
- block_lookup_symbol_primary. */
- if (best_symbol (sym, domain))
- return sym;
- /* This is a bit of a hack, but symbol_matches_domain might ignore
- STRUCT vs VAR domain symbols. So if a matching symbol is found,
- make sure there is no "better" matching symbol, i.e., one with
- exactly the same domain. PR 16253. */
- if (sym->matches (domain))
- other = better_symbol (other, sym, domain);
- }
- return other;
- }
+ return block_lookup_symbol_primary (block, name, domain);
else
{
/* Note that parameter symbols do not always show up last in the
@@ -722,10 +705,6 @@ block_lookup_symbol_primary (const struct block *block,
const lookup_name_info &name,
const domain_search_flags domain)
{
- /* Verify BLOCK is STATIC_BLOCK or GLOBAL_BLOCK. */
- gdb_assert (block->superblock () == NULL
- || block->superblock ()->superblock () == NULL);
-
symbol *other = nullptr;
for (symbol *sym : block_iterator_range (block, &name))
{
--
2.46.1
^ permalink raw reply [flat|nested] 50+ messages in thread* [PATCH v2 24/28] Add best_symbol_tracker
2025-04-02 23:44 [PATCH v2 00/28] Search symbols via quick API Tom Tromey
` (22 preceding siblings ...)
2025-04-02 23:45 ` [PATCH v2 23/28] Simplify block_lookup_symbol Tom Tromey
@ 2025-04-02 23:45 ` Tom Tromey
2025-04-02 23:45 ` [PATCH v2 25/28] Convert lookup_symbol_via_quick_fns Tom Tromey
` (5 subsequent siblings)
29 siblings, 0 replies; 50+ messages in thread
From: Tom Tromey @ 2025-04-02 23:45 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This adds a new best_symbol_tracker struct. This is used to implement
the "best symbol" logic that is used sometimes in symtab.c. This
approach makes it simpler and more efficient to track the "best"
symbol when searching across multiple blocks.
---
gdb/block.c | 29 ++++++++++++++++++++++-------
gdb/block.h | 27 ++++++++++++++++++++-------
gdb/symtab.c | 33 +++++++--------------------------
3 files changed, 49 insertions(+), 40 deletions(-)
diff --git a/gdb/block.c b/gdb/block.c
index d8f8bf56de7b1498ae3f89650a3c86c4eafe3bfe..27295168ba564334e5c50c98ad20c66bf2b21aa2 100644
--- a/gdb/block.c
+++ b/gdb/block.c
@@ -669,7 +669,11 @@ block_lookup_symbol (const struct block *block, const lookup_name_info &name,
const domain_search_flags domain)
{
if (!block->function ())
- return block_lookup_symbol_primary (block, name, domain);
+ {
+ best_symbol_tracker tracker;
+ tracker.search (nullptr, block, name, domain);
+ return tracker.currently_best.symbol;
+ }
else
{
/* Note that parameter symbols do not always show up last in the
@@ -700,12 +704,12 @@ block_lookup_symbol (const struct block *block, const lookup_name_info &name,
/* See block.h. */
-struct symbol *
-block_lookup_symbol_primary (const struct block *block,
+bool
+best_symbol_tracker::search (compunit_symtab *symtab,
+ const struct block *block,
const lookup_name_info &name,
const domain_search_flags domain)
{
- symbol *other = nullptr;
for (symbol *sym : block_iterator_range (block, &name))
{
/* With the fix for PR gcc/debug/91507, we get for:
@@ -736,17 +740,28 @@ block_lookup_symbol_primary (const struct block *block,
the only option to make this work is improve the fallback to use the
size of the minimal symbol. Filed as PR exp/24989. */
if (best_symbol (sym, domain))
- return sym;
+ {
+ best_symtab = symtab;
+ currently_best = { sym, block };
+ return true;
+ }
/* This is a bit of a hack, but 'matches' might ignore
STRUCT vs VAR domain symbols. So if a matching symbol is found,
make sure there is no "better" matching symbol, i.e., one with
exactly the same domain. PR 16253. */
if (sym->matches (domain))
- other = better_symbol (other, sym, domain);
+ {
+ symbol *better = better_symbol (sym, currently_best.symbol, domain);
+ if (better != currently_best.symbol)
+ {
+ best_symtab = symtab;
+ currently_best = { better, block };
+ }
+ }
}
- return other;
+ return false;
}
/* See block.h. */
diff --git a/gdb/block.h b/gdb/block.h
index c348c7fdcc4088e7f5c912e49c387e2c650eda45..b1daed216c956043b63ee99d8418607b417593ae 100644
--- a/gdb/block.h
+++ b/gdb/block.h
@@ -630,14 +630,27 @@ extern struct symbol *block_lookup_symbol (const struct block *block,
const lookup_name_info &name,
const domain_search_flags domain);
-/* Search BLOCK for symbol NAME in DOMAIN but only in primary symbol table of
- BLOCK. BLOCK must be STATIC_BLOCK or GLOBAL_BLOCK. Function is useful if
- one iterates all global/static blocks of an objfile. */
+/* When searching for a symbol, the "best" symbol is preferred over
+ one that is merely acceptable. See 'best_symbol'. This class
+ keeps track of this distinction while searching. */
-extern struct symbol *block_lookup_symbol_primary
- (const struct block *block,
- const lookup_name_info &name,
- const domain_search_flags domain);
+struct best_symbol_tracker
+{
+ /* The symtab in which the currently best symbol appears. */
+ compunit_symtab *best_symtab = nullptr;
+
+ /* The currently best (really "better") symbol. */
+ block_symbol currently_best {};
+
+ /* Search BLOCK (which must have come from SYMTAB) for a symbol
+ matching NAME and DOMAIN. When a symbol is found, update
+ 'currently_best'. If a best symbol is found, return true.
+ Otherwise, return false. SYMTAB can be nullptr if the caller
+ does not care about this tracking. */
+ bool search (compunit_symtab *symtab,
+ const block *block, const lookup_name_info &name,
+ domain_search_flags domain);
+};
/* Find symbol NAME in BLOCK and in DOMAIN. This will return a
matching symbol whose type is not a "opaque", see TYPE_IS_OPAQUE.
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 4822b5681f8370394c9cb6ca6c38ac2214191a39..8961e117348b8fe2b2ebf6b73ad1ca0fe9edbe35 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -2366,44 +2366,25 @@ lookup_symbol_in_objfile_symtabs (struct objfile *objfile,
name, domain_name (domain).c_str ());
lookup_name_info lookup_name (name, symbol_name_match_type::FULL);
- struct block_symbol other;
- other.symbol = NULL;
+ best_symbol_tracker accum;
for (compunit_symtab *cust : objfile->compunits ())
{
const struct blockvector *bv;
const struct block *block;
- struct block_symbol result;
bv = cust->blockvector ();
block = bv->block (block_index);
- result.symbol = block_lookup_symbol_primary (block, lookup_name, domain);
- result.block = block;
- if (result.symbol == NULL)
- continue;
- if (best_symbol (result.symbol, domain))
- {
- other = result;
- break;
- }
- if (result.symbol->matches (domain))
- {
- struct symbol *better
- = better_symbol (other.symbol, result.symbol, domain);
- if (better != other.symbol)
- {
- other.symbol = better;
- other.block = block;
- }
- }
+ if (accum.search (cust, block, lookup_name, domain))
+ break;
}
- if (other.symbol != NULL)
+ if (accum.currently_best.symbol != nullptr)
{
symbol_lookup_debug_printf_v
("lookup_symbol_in_objfile_symtabs (...) = %s (block %s)",
- host_address_to_string (other.symbol),
- host_address_to_string (other.block));
- return other;
+ host_address_to_string (accum.currently_best.symbol),
+ host_address_to_string (accum.currently_best.block));
+ return accum.currently_best;
}
symbol_lookup_debug_printf_v
--
2.46.1
^ permalink raw reply [flat|nested] 50+ messages in thread* [PATCH v2 25/28] Convert lookup_symbol_via_quick_fns
2025-04-02 23:44 [PATCH v2 00/28] Search symbols via quick API Tom Tromey
` (23 preceding siblings ...)
2025-04-02 23:45 ` [PATCH v2 24/28] Add best_symbol_tracker Tom Tromey
@ 2025-04-02 23:45 ` Tom Tromey
2025-04-02 23:45 ` [PATCH v2 26/28] Convert lookup_symbol_in_objfile Tom Tromey
` (4 subsequent siblings)
29 siblings, 0 replies; 50+ messages in thread
From: Tom Tromey @ 2025-04-02 23:45 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This converts lookup_symbol_via_quick_fns to the callback approach,
merging the search loop and the call to expand_symtabs_matching.
Note that this changes lookup_symbol_via_quick_fns to use a
best_symbol_tracker. Before this patch there was a discrepancy here
between the two search functions.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=16994
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=16998
---
gdb/symtab.c | 39 ++++++++++++++++++++++-----------------
1 file changed, 22 insertions(+), 17 deletions(-)
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 8961e117348b8fe2b2ebf6b73ad1ca0fe9edbe35..511dfd884bf41d61a475e953d0f68a2031bc2ca3 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -2458,11 +2458,6 @@ lookup_symbol_via_quick_fns (struct objfile *objfile,
enum block_enum block_index, const char *name,
const domain_search_flags domain)
{
- struct compunit_symtab *cust;
- const struct blockvector *bv;
- const struct block *block;
- struct block_symbol result;
-
symbol_lookup_debug_printf_v
("lookup_symbol_via_quick_fns (%s, %s, %s, %s)",
objfile_debug_name (objfile),
@@ -2470,27 +2465,37 @@ lookup_symbol_via_quick_fns (struct objfile *objfile,
name, domain_name (domain).c_str ());
lookup_name_info lookup_name (name, symbol_name_match_type::FULL);
- cust = objfile->lookup_symbol (block_index, lookup_name, domain);
- if (cust == NULL)
+ best_symbol_tracker accum;
+ auto searcher = [&] (compunit_symtab *symtab)
+ {
+ const struct blockvector *bv = symtab->blockvector ();
+ const struct block *block = bv->block (block_index);
+ /* If the accumulator finds a best symbol, end the search by
+ returning false; otherwise keep going by returning true. */
+ return !accum.search (symtab, block, lookup_name, domain);
+ };
+
+ objfile->expand_symtabs_matching (nullptr, &lookup_name, nullptr,
+ searcher,
+ block_index == GLOBAL_BLOCK
+ ? SEARCH_GLOBAL_BLOCK
+ : SEARCH_STATIC_BLOCK,
+ domain);
+ if (accum.best_symtab == nullptr)
{
symbol_lookup_debug_printf_v
("lookup_symbol_via_quick_fns (...) = NULL");
return {};
}
-
- bv = cust->blockvector ();
- block = bv->block (block_index);
- result.symbol = block_lookup_symbol (block, lookup_name, domain);
- if (result.symbol == NULL)
- error_in_psymtab_expansion (block_index, name, cust);
+ if (accum.currently_best.symbol == nullptr)
+ error_in_psymtab_expansion (block_index, name, accum.best_symtab);
symbol_lookup_debug_printf_v
("lookup_symbol_via_quick_fns (...) = %s (block %s)",
- host_address_to_string (result.symbol),
- host_address_to_string (block));
+ host_address_to_string (accum.currently_best.symbol),
+ host_address_to_string (accum.currently_best.block));
- result.block = block;
- return result;
+ return accum.currently_best;
}
/* See language.h. */
--
2.46.1
^ permalink raw reply [flat|nested] 50+ messages in thread* [PATCH v2 26/28] Convert lookup_symbol_in_objfile
2025-04-02 23:44 [PATCH v2 00/28] Search symbols via quick API Tom Tromey
` (24 preceding siblings ...)
2025-04-02 23:45 ` [PATCH v2 25/28] Convert lookup_symbol_via_quick_fns Tom Tromey
@ 2025-04-02 23:45 ` Tom Tromey
2025-04-02 23:45 ` [PATCH v2 27/28] Make dw_expand_symtabs_matching_file_matcher static Tom Tromey
` (3 subsequent siblings)
29 siblings, 0 replies; 50+ messages in thread
From: Tom Tromey @ 2025-04-02 23:45 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This converts lookup_symbol_in_objfile to the callback approach by
removing the call to lookup_symbol_in_objfile_symtabs. (The latter is
not removed as there are still other callers.)
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=16994
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=16998
---
gdb/symtab.c | 16 ++--------------
1 file changed, 2 insertions(+), 14 deletions(-)
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 511dfd884bf41d61a475e953d0f68a2031bc2ca3..16e0628b49bca17cd955f7be544128da47e69bc7 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -2595,24 +2595,12 @@ lookup_symbol_in_objfile (struct objfile *objfile, enum block_enum block_index,
? "GLOBAL_BLOCK" : "STATIC_BLOCK",
name, domain_name (domain).c_str ());
- result = lookup_symbol_in_objfile_symtabs (objfile, block_index,
- name, domain);
- if (result.symbol != NULL)
- {
- symbol_lookup_debug_printf
- ("lookup_symbol_in_objfile (...) = %s (in symtabs)",
- host_address_to_string (result.symbol));
- return result;
- }
-
result = lookup_symbol_via_quick_fns (objfile, block_index,
name, domain);
- symbol_lookup_debug_printf ("lookup_symbol_in_objfile (...) = %s%s",
+ symbol_lookup_debug_printf ("lookup_symbol_in_objfile (...) = %s",
result.symbol != NULL
? host_address_to_string (result.symbol)
- : "NULL",
- result.symbol != NULL ? " (via quick fns)"
- : "");
+ : "NULL");
return result;
}
--
2.46.1
^ permalink raw reply [flat|nested] 50+ messages in thread* [PATCH v2 27/28] Make dw_expand_symtabs_matching_file_matcher static
2025-04-02 23:44 [PATCH v2 00/28] Search symbols via quick API Tom Tromey
` (25 preceding siblings ...)
2025-04-02 23:45 ` [PATCH v2 26/28] Convert lookup_symbol_in_objfile Tom Tromey
@ 2025-04-02 23:45 ` Tom Tromey
2025-04-23 20:00 ` Simon Marchi
2025-04-02 23:45 ` [PATCH v2 28/28] Remove enter_symbol_lookup Tom Tromey
` (2 subsequent siblings)
29 siblings, 1 reply; 50+ messages in thread
From: Tom Tromey @ 2025-04-02 23:45 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
dw_expand_symtabs_matching_file_matcher is no longer needed outside of
read.c, so it can be made static.
---
gdb/dwarf2/read.c | 11 +++++++++--
gdb/dwarf2/read.h | 9 ---------
2 files changed, 9 insertions(+), 11 deletions(-)
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 6eee9631623435cbb39830116b790ba4b1f0c21d..efd17c27fcc10f078683ebfa9158ebfc2011f6b3 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -843,6 +843,11 @@ static struct dwarf2_section_info *cu_debug_loc_section (struct dwarf2_cu *cu);
static struct dwarf2_section_info *cu_debug_rnglists_section
(struct dwarf2_cu *cu, dwarf_tag tag);
+static void dw_expand_symtabs_matching_file_matcher
+ (dwarf2_per_objfile *per_objfile,
+ auto_bool_vector &marked,
+ expand_symtabs_file_matcher file_matcher);
+
static void get_scope_pc_bounds (struct die_info *,
unrelocated_addr *, unrelocated_addr *,
struct dwarf2_cu *);
@@ -1961,9 +1966,11 @@ dw2_expand_symtabs_matching_one
return true;
}
-/* See read.h. */
+/* If FILE_MATCHER is non-NULL, set all the
+ dwarf2_per_cu_quick_data::MARK of the current DWARF2_PER_OBJFILE
+ that match FILE_MATCHER. */
-void
+static void
dw_expand_symtabs_matching_file_matcher
(dwarf2_per_objfile *per_objfile,
auto_bool_vector &marked,
diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h
index 6cc08993a3ce7cdfa8b80350e8f0b965ed69b3e3..996afa6feeaf42a3a8750a5af5fd2aa5a0622952 100644
--- a/gdb/dwarf2/read.h
+++ b/gdb/dwarf2/read.h
@@ -1215,15 +1215,6 @@ extern bool dw2_expand_symtabs_matching_one
expand_symtabs_expansion_listener expansion_notify,
expand_symtabs_lang_matcher lang_matcher);
-/* If FILE_MATCHER is non-NULL, set all the
- dwarf2_per_cu_quick_data::MARK of the current DWARF2_PER_OBJFILE
- that match FILE_MATCHER. */
-
-extern void dw_expand_symtabs_matching_file_matcher
- (dwarf2_per_objfile *per_objfile,
- auto_bool_vector &marked,
- expand_symtabs_file_matcher file_matcher);
-
/* Return pointer to string at .debug_str offset STR_OFFSET. */
extern const char *read_indirect_string_at_offset
--
2.46.1
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH v2 27/28] Make dw_expand_symtabs_matching_file_matcher static
2025-04-02 23:45 ` [PATCH v2 27/28] Make dw_expand_symtabs_matching_file_matcher static Tom Tromey
@ 2025-04-23 20:00 ` Simon Marchi
2025-04-23 20:09 ` Tom Tromey
0 siblings, 1 reply; 50+ messages in thread
From: Simon Marchi @ 2025-04-23 20:00 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
> @@ -1961,9 +1966,11 @@ dw2_expand_symtabs_matching_one
> return true;
> }
>
> -/* See read.h. */
> +/* If FILE_MATCHER is non-NULL, set all the
> + dwarf2_per_cu_quick_data::MARK of the current DWARF2_PER_OBJFILE
> + that match FILE_MATCHER. */
The name dwarf2_per_cu_quick_data sounds outdated, the comment could
perhaps be refreshed, while at it.
Simon
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH v2 27/28] Make dw_expand_symtabs_matching_file_matcher static
2025-04-23 20:00 ` Simon Marchi
@ 2025-04-23 20:09 ` Tom Tromey
2025-04-23 20:44 ` Tom Tromey
0 siblings, 1 reply; 50+ messages in thread
From: Tom Tromey @ 2025-04-23 20:09 UTC (permalink / raw)
To: Simon Marchi; +Cc: Tom Tromey, gdb-patches
>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:
>> @@ -1961,9 +1966,11 @@ dw2_expand_symtabs_matching_one
>> return true;
>> }
>>
>> -/* See read.h. */
>> +/* If FILE_MATCHER is non-NULL, set all the
>> + dwarf2_per_cu_quick_data::MARK of the current DWARF2_PER_OBJFILE
>> + that match FILE_MATCHER. */
Simon> The name dwarf2_per_cu_quick_data sounds outdated, the comment could
Simon> perhaps be refreshed, while at it.
FWIW I updated the comment already when renaming the vectors in that
earlier patch.
Tom
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH v2 27/28] Make dw_expand_symtabs_matching_file_matcher static
2025-04-23 20:09 ` Tom Tromey
@ 2025-04-23 20:44 ` Tom Tromey
0 siblings, 0 replies; 50+ messages in thread
From: Tom Tromey @ 2025-04-23 20:44 UTC (permalink / raw)
To: Tom Tromey; +Cc: Simon Marchi, gdb-patches
>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:
>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:
>>> @@ -1961,9 +1966,11 @@ dw2_expand_symtabs_matching_one
>>> return true;
>>> }
>>>
>>> -/* See read.h. */
>>> +/* If FILE_MATCHER is non-NULL, set all the
>>> + dwarf2_per_cu_quick_data::MARK of the current DWARF2_PER_OBJFILE
>>> + that match FILE_MATCHER. */
Simon> The name dwarf2_per_cu_quick_data sounds outdated, the comment could
Simon> perhaps be refreshed, while at it.
Tom> FWIW I updated the comment already when renaming the vectors in that
Tom> earlier patch.
... but double-checking showed that I didn't update this patch, and also
that the rewritten comment had a typo...
Tom
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH v2 28/28] Remove enter_symbol_lookup
2025-04-02 23:44 [PATCH v2 00/28] Search symbols via quick API Tom Tromey
` (26 preceding siblings ...)
2025-04-02 23:45 ` [PATCH v2 27/28] Make dw_expand_symtabs_matching_file_matcher static Tom Tromey
@ 2025-04-02 23:45 ` Tom Tromey
2025-04-23 20:09 ` [PATCH v2 00/28] Search symbols via quick API Simon Marchi
2025-04-28 14:07 ` Guinevere Larsen
29 siblings, 0 replies; 50+ messages in thread
From: Tom Tromey @ 2025-04-02 23:45 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
The "enter_symbol_lookup" class was introduced to work around the lack
of reentrancy in symbol lookup. There were two problems here:
1. The DWARF reader kept a mark bit on the dwarf2_per_cu_data object.
This bit is gone now, replaced with a local mark vector.
2. Some spots in gdb first examined the expanded symbol tables, and
then on failure expanded some symtabs and searched the newly
expanded ones (skipping previousy-expanded ones). Fixing this has
been the main point of this series.
Now that both of these barriers are gone, I think enter_symbol_lookup
can be removed.
One proof of this idea is that, without the first fix mentioned above,
py-symbol.exp regressed because gdbpy_lookup_static_symbols did not
first ensure that the current language was set -- i.e., there was a
latent bug in the enter_symbol_lookup patch anyway.
---
gdb/symtab.c | 41 -----------------------------------------
1 file changed, 41 deletions(-)
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 16e0628b49bca17cd955f7be544128da47e69bc7..1bc54e8760c7075b3ec5a38c0c16dcbdbe16e25e 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -124,41 +124,6 @@ struct main_info
static const registry<program_space>::key<main_info> main_progspace_key;
-/* Symbol lookup is not reentrant (though this is not an intrinsic
- restriction). Keep track of whether a symbol lookup is active, to be able
- to detect reentrancy. */
-static bool in_symbol_lookup;
-
-/* Struct to mark that a symbol lookup is active for the duration of its
- lifetime. */
-
-struct enter_symbol_lookup
-{
- enter_symbol_lookup ()
- {
- /* Ensure that the current language has been set. Normally the
- language is set lazily. However, when performing a symbol lookup,
- this could result in a recursive call into the lookup code in some
- cases. Set it now to ensure that this does not happen. */
- get_current_language ();
-
- /* Detect symbol lookup reentrance. */
- gdb_assert (!in_symbol_lookup);
-
- in_symbol_lookup = true;
- }
-
- ~enter_symbol_lookup ()
- {
- /* Sanity check. */
- gdb_assert (in_symbol_lookup);
-
- in_symbol_lookup = false;
- }
-
- DISABLE_COPY_AND_ASSIGN (enter_symbol_lookup);
-};
-
/* The default symbol cache size.
There is no extra cpu cost for large N (except when flushing the cache,
which is rare). The value here is just a first attempt. A better default
@@ -2294,8 +2259,6 @@ lookup_symbol_in_block (const char *name, symbol_name_match_type match_type,
const struct block *block,
const domain_search_flags domain)
{
- enter_symbol_lookup tmp;
-
struct symbol *sym;
if (symbol_lookup_debug)
@@ -2331,8 +2294,6 @@ lookup_global_symbol_from_objfile (struct objfile *main_objfile,
const char *name,
const domain_search_flags domain)
{
- enter_symbol_lookup tmp;
-
gdb_assert (block_index == GLOBAL_BLOCK || block_index == STATIC_BLOCK);
for (objfile *objfile : main_objfile->separate_debug_objfiles ())
@@ -2633,8 +2594,6 @@ lookup_global_or_static_symbol (const char *name,
return result;
}
- enter_symbol_lookup tmp;
-
/* Do a global search (of global blocks, heh). */
if (result.symbol == NULL)
gdbarch_iterate_over_objfiles_in_search_order
--
2.46.1
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH v2 00/28] Search symbols via quick API
2025-04-02 23:44 [PATCH v2 00/28] Search symbols via quick API Tom Tromey
` (27 preceding siblings ...)
2025-04-02 23:45 ` [PATCH v2 28/28] Remove enter_symbol_lookup Tom Tromey
@ 2025-04-23 20:09 ` Simon Marchi
2025-04-24 21:09 ` Tom Tromey
2025-04-28 14:07 ` Guinevere Larsen
29 siblings, 1 reply; 50+ messages in thread
From: Simon Marchi @ 2025-04-23 20:09 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
On 2025-04-02 19:44, Tom Tromey wrote:
> This series changes how symbols are looked up in gdb.
>
> Currently, symbol lookup is done in two phases. In one phase, gdb
> searches all existing symtabs for a symbol. In another phase, gdb
> will call expand_symtabs_matching to expand some symtabs.
>
> Different spots in gdb may order these phases differently -- so some
> places will expand symtabs and then search the compunits, but other
> places will first search expanded symtabs and then use the
> expand_symtabs_matching callback to add further results.
>
> This approach has a few drawbacks.
>
> * The double search means that some discrepancies between the indexer
> and the full reader go unnoticed. This may arguably be a strength
> of the approach, though frequently a carefully crafted test case can
> show this as a bug. Essentially, though, some searches only work by
> accident.
>
> * Searching all expanded symtabs means that, as a debug session drags
> on, searches will examine many irrelevant symtabs.
>
> * In some places, the two phases use different code to perform the
> actual search, meaning that the results can depend on previous CU
> expansion decisions.
>
> * Similarly, if just a single symbol is needed, then both approaches
> are bad: expand-then-search will be unnecessarily inefficient, while
> search-then-expand approach means that the result depends on which
> CUs happen to have already been expanded.
>
> This series changes all of this. Now, all symbol lookups are done via
> the "quick" API, with the idea being that the final search of the
> expanded symtab is done via the expand_symtabs_matching callback.
>
> This fixes all the issues pointed out above:
>
> * Only relevant symtabs are searched, because the index only considers
> matching symbols.
>
> * Some discrepancies between the indexer and the full reader show up
> as symbol lookup failures. Others (if the indexer thinks a symbol
> exists but the full reader does not) will just be inefficient -- but
> we could add a verifier for this.
>
> * Lookups are no longer dependent on symtab expansion state, because
> again the indexer is pre-filtering the matches.
>
> Re-reading the series -- it's been in development for quite a while
> and I've already landed ~20 preliminary patches -- shows that there
> are still a few cleanups in here that could perhaps have gone in
> separately.
>
> Anyway, the essential change is to make the implementations of
> expand_symtabs_matching also call the callback when a symtab has
> already been expanded. After this, most of the work is cleaning up
> individual callers.
>
> This change lead to some surprising places. For example, I had to
> rewrite the .gdb_index reader, because the work done for
> "templates.exp" simply never worked in this mode -- and the test
> obscured the problems, a classic case of lookups working by accident.
>
> I regression tested each patch in this series. Furthermore I
> regression tested the series as a whole using the cc-with-debug-names
> and cc-with-gdb-index target boards. All of this was done on x86-64
> Fedora 40.
>
> I think maybe I've separately submitted "Ada import functions not in
> index" patch. Anyway it's part of this series now.
>
> Signed-off-by: Tom Tromey <tom@tromey.com>
> ---
I went over the series but not in depth. I like how it simplifies the
symbol lookup functions and how it makes things less error prone. You
can add:
Acked-By: Simon Marchi <simon.marchi@efficios.com>
Since it touches a lot of different code, I think you should merge it
soon, to avoid having to deal with more conflicts. I also have some
stuff that I will need rebase on top of this, so I would like to get to
it sooner than later.
Simon
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH v2 00/28] Search symbols via quick API
2025-04-23 20:09 ` [PATCH v2 00/28] Search symbols via quick API Simon Marchi
@ 2025-04-24 21:09 ` Tom Tromey
0 siblings, 0 replies; 50+ messages in thread
From: Tom Tromey @ 2025-04-24 21:09 UTC (permalink / raw)
To: Simon Marchi; +Cc: Tom Tromey, gdb-patches
>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:
Simon> Since it touches a lot of different code, I think you should merge it
Simon> soon, to avoid having to deal with more conflicts. I also have some
Simon> stuff that I will need rebase on top of this, so I would like to get to
Simon> it sooner than later.
I re-tested it yesterday and found some new regressions.
Landing it will have to wait until I fix those.
Tom
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH v2 00/28] Search symbols via quick API
2025-04-02 23:44 [PATCH v2 00/28] Search symbols via quick API Tom Tromey
` (28 preceding siblings ...)
2025-04-23 20:09 ` [PATCH v2 00/28] Search symbols via quick API Simon Marchi
@ 2025-04-28 14:07 ` Guinevere Larsen
2025-04-28 22:06 ` Tom Tromey
29 siblings, 1 reply; 50+ messages in thread
From: Guinevere Larsen @ 2025-04-28 14:07 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
Hi Tromey,
I am having an issue with the quick_fns interface, and while it isn't
affected by this series directly (as I originally thought it could be),
I would appreciate if you could share your expertise on this area. The
TL;DR of the issue is: the quick API seems to be caching something and
searching objfile B for a symbol can return a symbol with details from
objfile A (that was loaded first). I tried looking around for where the
cache could be hidden, but couldn't find it. Do you know if where the
cache is, if it really exists?
Some more context: Now that we have a way to get all solibs (and thus
objfiles) that belong to one namespace, so I'm trying to implement
search for a symbol inside a specific namespace. To do this, I iterate
through objfiles in the selected namespace and use
lookup_symbol_in_objfile on all of them. The first issue I ran into was
the gdb_bfd_cache making the second objfile's symbols not be read, which
I already managed to fix. I know we have correct reading of symbols and
addresses if I follow the dwarf reading, so if symbol lookup was
correct, everything should be working.
Yet, the symbols found through the quick_fns all seem to point to the
same inferior address (I'm unsure if they are different symbols, or the
same symbol returned through dynamic allocation so pointing to different
addresses), meaning that we still effectively only find the first one
and return it. I can verify it by either taking the address directly, or
setting the variable in one namespace and reading it in another.
If you'd like to play around with my changes, I pushed them to the
users/guinevere/symbol_in_linker_namespace branch, and you can use the
test gdb.base/dlmopen-ns-ids to search for the variable gdb_dlmopen_glob
in different namespaces. (gdb.base/dlmopen.exp may not work because I
haven't looked into LOC_UNRESOLVED variables yet, only LOC_STATIC).
--
Cheers,
Guinevere Larsen
She/Her/Hers
On 4/2/25 8:44 PM, Tom Tromey wrote:
> This series changes how symbols are looked up in gdb.
>
> Currently, symbol lookup is done in two phases. In one phase, gdb
> searches all existing symtabs for a symbol. In another phase, gdb
> will call expand_symtabs_matching to expand some symtabs.
>
> Different spots in gdb may order these phases differently -- so some
> places will expand symtabs and then search the compunits, but other
> places will first search expanded symtabs and then use the
> expand_symtabs_matching callback to add further results.
>
> This approach has a few drawbacks.
>
> * The double search means that some discrepancies between the indexer
> and the full reader go unnoticed. This may arguably be a strength
> of the approach, though frequently a carefully crafted test case can
> show this as a bug. Essentially, though, some searches only work by
> accident.
>
> * Searching all expanded symtabs means that, as a debug session drags
> on, searches will examine many irrelevant symtabs.
>
> * In some places, the two phases use different code to perform the
> actual search, meaning that the results can depend on previous CU
> expansion decisions.
>
> * Similarly, if just a single symbol is needed, then both approaches
> are bad: expand-then-search will be unnecessarily inefficient, while
> search-then-expand approach means that the result depends on which
> CUs happen to have already been expanded.
>
> This series changes all of this. Now, all symbol lookups are done via
> the "quick" API, with the idea being that the final search of the
> expanded symtab is done via the expand_symtabs_matching callback.
>
> This fixes all the issues pointed out above:
>
> * Only relevant symtabs are searched, because the index only considers
> matching symbols.
>
> * Some discrepancies between the indexer and the full reader show up
> as symbol lookup failures. Others (if the indexer thinks a symbol
> exists but the full reader does not) will just be inefficient -- but
> we could add a verifier for this.
>
> * Lookups are no longer dependent on symtab expansion state, because
> again the indexer is pre-filtering the matches.
>
> Re-reading the series -- it's been in development for quite a while
> and I've already landed ~20 preliminary patches -- shows that there
> are still a few cleanups in here that could perhaps have gone in
> separately.
>
> Anyway, the essential change is to make the implementations of
> expand_symtabs_matching also call the callback when a symtab has
> already been expanded. After this, most of the work is cleaning up
> individual callers.
>
> This change lead to some surprising places. For example, I had to
> rewrite the .gdb_index reader, because the work done for
> "templates.exp" simply never worked in this mode -- and the test
> obscured the problems, a classic case of lookups working by accident.
>
> I regression tested each patch in this series. Furthermore I
> regression tested the series as a whole using the cc-with-debug-names
> and cc-with-gdb-index target boards. All of this was done on x86-64
> Fedora 40.
>
> I think maybe I've separately submitted "Ada import functions not in
> index" patch. Anyway it's part of this series now.
>
> Signed-off-by: Tom Tromey <tom@tromey.com>
> ---
> Changes in v2:
> - Rebased for all the other recent DWARF changes
> - Link to v1: https://inbox.sourceware.org/gdb-patches/20250311-search-in-psyms-v1-0-d73d9be20983@tromey.com
>
> ---
> Tom Tromey (28):
> Add another minor hack to cooked_index_entry::full_name
> Change ada_decode to preserve upper-case in some situations
> Emit some type declarations in .gdb_index
> Ada import functions not in index
> Fix index's handling of DW_TAG_imported_declaration
> Put all CTF symbols in global scope
> Restore "ingestion" of .debug_str when writing .debug_names
> Entries from anon-struct.exp not in cooked index
> Remove dwarf2_per_cu_data::mark
> Have expand_symtabs_matching work for already-expanded CUs
> Rewrite the .gdb_index reader
> Convert default_collect_symbol_completion_matches_break_on
> Convert gdbpy_lookup_static_symbols
> Convert ada_add_global_exceptions
> Convert ada_language_defn::collect_symbol_completion_matches
> Convert ada-lang.c:map_matching_symbols
> Remove expand_symtabs_matching
> Simplify basic_lookup_transparent_type
> Remove objfile::expand_symtabs_for_function
> Convert linespec.c:iterate_over_all_matching_symtabs
> Simplify block_lookup_symbol_primary
> Pass lookup_name_info to block_lookup_symbol_primary
> Simplify block_lookup_symbol
> Add best_symbol_tracker
> Convert lookup_symbol_via_quick_fns
> Convert lookup_symbol_in_objfile
> Make dw_expand_symtabs_matching_file_matcher static
> Remove enter_symbol_lookup
>
> gdb/ada-lang.c | 221 ++---
> gdb/ada-lang.h | 15 +-
> gdb/block.c | 57 +-
> gdb/block.h | 27 +-
> gdb/cp-support.c | 28 +-
> gdb/ctfread.c | 6 +-
> gdb/dwarf2/abbrev.c | 7 +-
> gdb/dwarf2/abbrev.h | 8 +
> gdb/dwarf2/cooked-index-entry.c | 7 +
> gdb/dwarf2/cooked-index-shard.c | 13 +-
> gdb/dwarf2/cooked-index-shard.h | 7 +
> gdb/dwarf2/cooked-index-worker.h | 17 +-
> gdb/dwarf2/cooked-indexer.c | 37 +-
> gdb/dwarf2/index-write.c | 103 ++-
> gdb/dwarf2/read-gdb-index.c | 1268 ++++++-----------------------
> gdb/dwarf2/read-gdb-index.h | 14 +
> gdb/dwarf2/read.c | 176 ++--
> gdb/dwarf2/read.h | 48 +-
> gdb/dwarf2/tag.h | 12 +
> gdb/linespec.c | 17 +-
> gdb/objfiles.h | 4 -
> gdb/psymtab.c | 3 -
> gdb/python/py-symbol.c | 29 +-
> gdb/symfile-debug.c | 22 -
> gdb/symfile.c | 25 -
> gdb/symfile.h | 9 -
> gdb/symtab.c | 223 ++---
> gdb/symtab.h | 2 +-
> gdb/testsuite/gdb.ada/import.exp | 28 +-
> gdb/testsuite/gdb.ctf/cross-tu-cyclic.exp | 4 +-
> 30 files changed, 861 insertions(+), 1576 deletions(-)
> ---
> base-commit: 60ac9c60fe5b6dc5a59a30a971c3fad020fecf45
> change-id: 20250311-search-in-psyms-3eb1049177e0
>
> Best regards,
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH v2 00/28] Search symbols via quick API
2025-04-28 14:07 ` Guinevere Larsen
@ 2025-04-28 22:06 ` Tom Tromey
2025-04-29 19:31 ` Guinevere Larsen
0 siblings, 1 reply; 50+ messages in thread
From: Tom Tromey @ 2025-04-28 22:06 UTC (permalink / raw)
To: Guinevere Larsen; +Cc: Tom Tromey, gdb-patches
>>>>> "Guinevere" == Guinevere Larsen <guinevere@redhat.com> writes:
Guinevere> I am having an issue with the quick_fns interface, and while it isn't
Guinevere> affected by this series directly (as I originally thought it could
Guinevere> be), I would appreciate if you could share your expertise on this
Guinevere> area. The TL;DR of the issue is: the quick API seems to be caching
Guinevere> something and searching objfile B for a symbol can return a symbol
Guinevere> with details from objfile A (that was loaded first). I tried looking
Guinevere> around for where the cache could be hidden, but couldn't find it. Do
Guinevere> you know if where the cache is, if it really exists?
Look for symbol_cache in symtab.c.
Also there's a separate cache in ada-lang.c, but this one only affects
Ada.
It's been a while since I looked to see if either of these are really
helpful.
There are also some debug settings for symbol lookup, look in symtab.c
and symfile-debug.c.
Tom
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH v2 00/28] Search symbols via quick API
2025-04-28 22:06 ` Tom Tromey
@ 2025-04-29 19:31 ` Guinevere Larsen
0 siblings, 0 replies; 50+ messages in thread
From: Guinevere Larsen @ 2025-04-29 19:31 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
On 4/28/25 7:06 PM, Tom Tromey wrote:
>>>>>> "Guinevere" == Guinevere Larsen <guinevere@redhat.com> writes:
> Guinevere> I am having an issue with the quick_fns interface, and while it isn't
> Guinevere> affected by this series directly (as I originally thought it could
> Guinevere> be), I would appreciate if you could share your expertise on this
> Guinevere> area. The TL;DR of the issue is: the quick API seems to be caching
> Guinevere> something and searching objfile B for a symbol can return a symbol
> Guinevere> with details from objfile A (that was loaded first). I tried looking
> Guinevere> around for where the cache could be hidden, but couldn't find it. Do
> Guinevere> you know if where the cache is, if it really exists?
>
> Look for symbol_cache in symtab.c.
Drat, this isn't the culprit I was looking for.
My first test was using `maint flush symbol-cache` between printing
symbols from different namespaces. No change.
Then I tried stopping in the new lookup_symbol_in_linker_namespace, and
then setting a breakpoint in get_symbol_cache. No hits.
> Also there's a separate cache in ada-lang.c, but this one only affects
> Ada.
Yeah, I'm in C, so this shouldn't affect anything
> It's been a while since I looked to see if either of these are really
> helpful.
>
> There are also some debug settings for symbol lookup, look in symtab.c
> and symfile-debug.c.
I looked these up, but I have tried all of them, and nothing looked
suspicious... The most interesting one was symtab-create, but even then,
both of the namespaces had the exact same output, so I don't think the
second one was shortcutting by using the first one's symtab.
Thank you for the pointers, though, and if you have other ideas that may
help (even if you're not sure if they're useful), I'll be glad to hear them!
--
Cheers,
Guinevere Larsen
She/Her/Hers
^ permalink raw reply [flat|nested] 50+ messages in thread