With the test-case included in this patch, we run into:
...
FAIL: gdb.dwarf2/forward-spec-inter-cu.exp: v has a parent
...
due to v having the parent nullptr:
...
[3] ((cooked_index_entry *) 0x7fc798005250)^M
name: v^M
canonical: v^M
qualified: v^M
DWARF tag: DW_TAG_variable^M
flags: 0x2 [IS_STATIC]^M
DIE offset: 0xbe^M
parent: ((cooked_index_entry *) 0)^M
...
while it should have the parent ns:
...
[18] ((cooked_index_entry *) 0x7fc794002ed0)^M
name: ns^M
canonical: ns^M
qualified: ns^M
DWARF tag: DW_TAG_namespace^M
flags: 0x0 []^M
DIE offset: 0xde^M
parent: ((cooked_index_entry *) 0)^M
...
The corresponding dwarf is:
...
Compilation Unit @ offset 0xb1:
...
<0><bc>: Abbrev Number: 2 (DW_TAG_compile_unit)
<bd> DW_AT_language : 4 (C++)
<1><be>: Abbrev Number: 3 (DW_TAG_variable)
<bf> DW_AT_specification: <0xe2>
<c3> DW_AT_location : (DW_OP_const1u: 23; DW_OP_stack_value)
...
Compilation Unit @ offset 0xc8:
...
<0><d3>: Abbrev Number: 2 (DW_TAG_compile_unit)
<d4> DW_AT_language : 4 (C++)
...
<1><de>: Abbrev Number: 4 (DW_TAG_namespace)
<df> DW_AT_name : ns
<2><e2>: Abbrev Number: 5 (DW_TAG_variable)
<e3> DW_AT_name : v
<e5> DW_AT_type : <0xd5>
<e9> DW_AT_declaration : 1
...
By switching on debug_handle_deferred_entries, we get:
...
0x00000000000000df 0x7f2b34007640 (0xde)^M
0x00000000000000ea 0x0^M
...
So, the DIE 0xbe is not marked as deferred. And even if it would be, the
incorrect value would be fetched, because m_die_range_map only has the scope
of a parsing a single CU (with the exception of imported PUs, but only if the
CU wins the race).
[ Then there's the problem that find_parent cannot distinguish between the
"no parent" and "don't know" cases, which means that getting the incorrect
parent happens silently. ]
We need to generically solve the case of inter-CU dependencies, and that
includes inter-shard dependencies as well. On my system, the test-case
exercises the inter-shard case, but that may be different on other systems,
depending on number and size of CUs in the dwarf, and the number of worker
threads.
So we:
- mark deferred entries in m_die_range_map using dummy cooked_index_entry
parent_map::defered, to prevent incorrect entries for deferred dies,
- defer all inter-CU dependencies (note that two subsequent patches implement
optimizations to deal with this more optimally),
- move m_die_range_map and m_deferred_dies to cooked_index_shard, and
- move handle_deferred_dies to the cooked_index, where it is called in the
constructor, and update it to handle the intra-shard case,
such that we get:
...
0x00000000000000be 0x2934520 (deferred)^M
0x00000000000000bf 0x0^M
0x00000000000000df 0x7f4260002ee0 (0xde)^M
0x00000000000000ea 0x0^M
Resolve deferred: 0xbe -> 0xe2: 0xde^M
...
and consequently:
...
[3] ((cooked_index_entry *) 0x7f4270002ee0)^M
name: v^M
canonical: v^M
qualified: ns::v^M
DWARF tag: DW_TAG_variable^M
flags: 0x2 [IS_STATIC]^M
DIE offset: 0xbe^M
parent: ((cooked_index_entry *) 0x7f4260002ee0) [ns]^M
...
Note that handling units from the .debug_info section alongside units from the
.debug_types section requires us to extend form_addr to take is_debug_types
into account.
Tested on x86_64-linux.
PR symtab/30846
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30846
---
gdb/dwarf2/cooked-index.c | 84 ++++++++++++++++++++++++++
gdb/dwarf2/cooked-index.h | 73 +++++++++++++++++++++-
gdb/dwarf2/read.c | 124 ++++++++++++++++----------------------
3 files changed, 207 insertions(+), 74 deletions(-)
@@ -228,6 +228,12 @@ cooked_index_entry::write_scope (struct obstack *storage,
/* See cooked-index.h. */
+cooked_index_entry parent_map::deferred((sect_offset)0, (dwarf_tag)0,
+ (cooked_index_flag)0, nullptr,
+ nullptr, nullptr);
+
+/* See cooked-index.h. */
+
const cooked_index_entry *
cooked_index_shard::add (sect_offset die_offset, enum dwarf_tag tag,
cooked_index_flag flags, const char *name,
@@ -446,6 +452,8 @@ cooked_index_shard::wait (bool allow_quit) const
cooked_index::cooked_index (vec_type &&vec)
: m_vector (std::move (vec))
{
+ handle_deferred_entries ();
+
for (auto &idx : m_vector)
idx->finalize ();
@@ -649,6 +657,82 @@ cooked_index::maybe_write_index (dwarf2_per_bfd *per_bfd,
global_index_cache.store (per_bfd, ctx);
}
+/* Set to true to debug handling of deferred entries. */
+const static bool debug_handle_deferred_entries = false;
+
+/* See cooked-index.h. */
+
+const cooked_index_entry *
+cooked_index_shard::resolve_deferred_entry
+ (const deferred_entry &de, const cooked_index_entry *parent_entry)
+{
+ if (debug_handle_deferred_entries)
+ {
+ gdb_printf (gdb_stdlog,
+ "Resolve deferred: 0x%" PRIx64 " -> 0x%lx: ",
+ to_underlying (de.die_offset),
+ de.spec_offset);
+ if (parent_entry == nullptr)
+ gdb_printf (gdb_stdlog, "no parent\n");
+ else
+ gdb_printf (gdb_stdlog, "0x%" PRIx64 "\n",
+ to_underlying (parent_entry->die_offset));
+ }
+
+ return add (de.die_offset, de.tag, de.flags, de.name,
+ parent_entry, de.per_cu);
+}
+
+/* See cooked-index.h. */
+
+const cooked_index_entry *
+cooked_index::find_parent_deferred_entry
+ (const cooked_index_shard::deferred_entry &entry) const
+{
+ const cooked_index_entry *parent_entry = nullptr;
+ for (auto &parent_map_shard : m_vector)
+ {
+ auto res = parent_map_shard->find_parent (entry.spec_offset);
+ if (res != nullptr)
+ {
+ /* We currently assume that no derrered entry is
+ dependent on another deferred entry. If that turns
+ out to be not the case, detect it here. */
+ gdb_assert (res != &parent_map::deferred);
+ parent_entry = res;
+ break;
+ }
+ }
+
+ return parent_entry;
+}
+
+/* See cooked-index.h. */
+
+void
+cooked_index::handle_deferred_entries ()
+{
+ if (debug_handle_deferred_entries)
+ for (auto &shard : m_vector)
+ shard->m_die_range_map->dump ();
+
+ for (auto &shard : m_vector)
+ for (const auto &deferred_entry : *shard->m_deferred_entries)
+ {
+ const cooked_index_entry *parent_entry
+ = find_parent_deferred_entry (deferred_entry);
+ shard->resolve_deferred_entry (deferred_entry, parent_entry);
+ }
+
+ for (auto &shard : m_vector)
+ {
+ delete shard->m_die_range_map;
+ shard->m_die_range_map = nullptr;
+ delete shard->m_deferred_entries;
+ shard->m_deferred_entries = nullptr;
+ }
+}
+
/* Wait for all the index cache entries to be written before gdb
exits. */
static void
@@ -242,6 +242,10 @@ struct cooked_index_entry : public allocate_on_obstack
class parent_map
{
public:
+ /* A dummy cooked_index_entry to mark that computing the parent has been
+ deferred. */
+ static cooked_index_entry deferred;
+
/* Find the parent of DIE LOOKUP. */
const cooked_index_entry *find_parent (CORE_ADDR lookup) const
{
@@ -276,6 +280,13 @@ class parent_map
= (const cooked_index_entry *)value;
if (parent_entry == nullptr)
return;
+
+ if (parent_entry == &parent_map::deferred)
+ {
+ gdb_printf (outfile, " (deferred)");
+ return;
+ }
+
gdb_printf (outfile, " (0x%" PRIx64 ")",
to_underlying (parent_entry->die_offset));
};
@@ -301,7 +312,11 @@ class cooked_index;
class cooked_index_shard
{
public:
- cooked_index_shard () = default;
+ cooked_index_shard () {
+ m_die_range_map = new parent_map;
+ m_deferred_entries = new std::vector<deferred_entry>;
+ }
+
DISABLE_COPY_AND_ASSIGN (cooked_index_shard);
/* Create a new cooked_index_entry and register it with this object.
@@ -345,6 +360,41 @@ class cooked_index_shard
for completion, will be returned. */
range find (const std::string &name, bool completing) const;
+ /* Find the parent of DIE LOOKUP. */
+ const cooked_index_entry *
+ find_parent (CORE_ADDR lookup) const
+ {
+ return m_die_range_map->find_parent (lookup);
+ }
+
+ /* Set the parent of DIES in range [START, END] to PARENT_ENTRY. */
+ void set_parent (CORE_ADDR start, CORE_ADDR end,
+ const cooked_index_entry *parent_entry)
+ {
+ m_die_range_map->set_parent (start, end, parent_entry);
+ }
+
+ /* A single deferred entry. See m_deferred_entries. */
+ struct deferred_entry
+ {
+ sect_offset die_offset;
+ const char *name;
+ CORE_ADDR spec_offset;
+ dwarf_tag tag;
+ cooked_index_flag flags;
+ dwarf2_per_cu_data *per_cu;
+ };
+
+ /* Defer creating a cooked_index_entry corresponding to DEFERRED. */
+ void defer_entry (deferred_entry de)
+ {
+ m_deferred_entries->push_back (de);
+ }
+
+ /* Variant of add that takes a deferred_entry as parameter. */
+ const cooked_index_entry *resolve_deferred_entry
+ (const deferred_entry &entry, const cooked_index_entry *parent_entry);
+
private:
/* Return the entry that is believed to represent the program's
@@ -402,6 +452,20 @@ class cooked_index_shard
that the 'get' method is never called on this future, only
'wait'. */
gdb::future<void> m_future;
+
+ /* An addrmap that maps from section offsets (see the form_addr
+ method) to newly-created entries. See m_deferred_entries to
+ understand this. */
+ parent_map *m_die_range_map;
+
+ /* The generated DWARF can sometimes have the declaration for a
+ method in a class (or perhaps namespace) scope, with the
+ definition appearing outside this scope... just one of the many
+ bad things about DWARF. In order to handle this situation, we
+ defer certain entries until the end of scanning, at which point
+ we'll know the containing context of all the DIEs that we might
+ have scanned. This vector stores these deferred entries. */
+ std::vector<deferred_entry> *m_deferred_entries;
};
/* The main index of DIEs. The parallel DIE indexers create
@@ -485,6 +549,13 @@ class cooked_index : public dwarf_scanner_base
private:
+ /* Find the parent corresponding to deferred entry ENTRY. */
+ const cooked_index_entry *find_parent_deferred_entry
+ (const cooked_index_shard::deferred_entry &entry) const;
+
+ /* Create cooked_index_entries for the deferred entries. */
+ void handle_deferred_entries ();
+
/* Maybe write the index to the index cache. */
void maybe_write_index (dwarf2_per_bfd *per_bfd,
const index_cache_store_context &);
@@ -4670,6 +4670,25 @@ class cooked_index_storage
return &m_addrmap;
}
+ /* Find the parent of DIE LOOKUP. */
+ const cooked_index_entry *find_parent (CORE_ADDR lookup)
+ {
+ return m_index->find_parent (lookup);
+ }
+
+ /* Set the parent of DIES in range [START, END] to PARENT_ENTRY. */
+ void set_parent (CORE_ADDR start, CORE_ADDR end,
+ const cooked_index_entry *parent_entry)
+ {
+ m_index->set_parent (start, end, parent_entry);
+ }
+
+ /* Defer creating a cooked_index_entry corresponding to deferred_entry DE. */
+ void defer_entry (cooked_index_shard::deferred_entry de)
+ {
+ m_index->defer_entry (de);
+ }
+
private:
/* Hash function for a cutu_reader. */
@@ -4722,11 +4741,13 @@ class cooked_indexer
/* A helper function to turn a section offset into an address that
can be used in an addrmap. */
- CORE_ADDR form_addr (sect_offset offset, bool is_dwz)
+ CORE_ADDR form_addr (sect_offset offset, bool is_dwz, bool is_debug_types)
{
CORE_ADDR value = to_underlying (offset);
if (is_dwz)
value |= ((CORE_ADDR) 1) << (8 * sizeof (CORE_ADDR) - 1);
+ if (is_debug_types)
+ value |= ((CORE_ADDR) 1) << (8 * sizeof (CORE_ADDR) - 2);
return value;
}
@@ -4802,77 +4823,20 @@ class cooked_indexer
/* Find the parent of DIE LOOKUP. */
const cooked_index_entry *find_parent (CORE_ADDR lookup) const
{
- return m_die_range_map.find_parent (lookup);
+ return m_index_storage->find_parent (lookup);
}
/* Set the parent of DIES in range [START, END] to PARENT_ENTRY. */
void set_parent (CORE_ADDR start, CORE_ADDR end,
const cooked_index_entry *parent_entry)
{
- m_die_range_map.set_parent (start, end, parent_entry);
+ m_index_storage->set_parent (start, end, parent_entry);
}
- /* A single deferred entry. */
- struct deferred_entry
- {
- sect_offset die_offset;
- const char *name;
- CORE_ADDR spec_offset;
- dwarf_tag tag;
- cooked_index_flag flags;
- };
-
- /* The generated DWARF can sometimes have the declaration for a
- method in a class (or perhaps namespace) scope, with the
- definition appearing outside this scope... just one of the many
- bad things about DWARF. In order to handle this situation, we
- defer certain entries until the end of scanning, at which point
- we'll know the containing context of all the DIEs that we might
- have scanned. This vector stores these deferred entries. */
- std::vector<deferred_entry> m_deferred_entries;
-
/* Defer creating a cooked_index_entry corresponding to deferred_entry DE. */
- void defer_entry (const deferred_entry &de)
+ void defer_entry (const cooked_index_shard::deferred_entry &de)
{
- m_deferred_entries.push_back (de);
- }
-
- /* Set to true to debug handling of deferred entries. */
- const bool debug_handle_deferred_entries = false;
-
- /* Create a cooked_index_entry corresponding to deferred_entry DE with
- parent PARENT_ENTRY. */
- const cooked_index_entry *resolve_deferred_entry
- (const deferred_entry &de, const cooked_index_entry *parent_entry)
- {
- if (debug_handle_deferred_entries)
- {
- gdb_printf (gdb_stdlog,
- "Resolve deferred: 0x%" PRIx64 " -> 0x%lx: ",
- to_underlying (de.die_offset),
- de.spec_offset);
- if (parent_entry == nullptr)
- gdb_printf (gdb_stdlog, "no parent\n");
- else
- gdb_printf (gdb_stdlog, "0x%" PRIx64 "\n",
- to_underlying (parent_entry->die_offset));
- }
- return m_index_storage->add (de.die_offset, de.tag, de.flags, de.name,
- parent_entry, m_per_cu);
- }
-
- /* Create cooked_index_entries for the deferred entries. */
- void handle_deferred_entries ()
- {
- if (debug_handle_deferred_entries)
- m_die_range_map.dump ();
-
- for (const auto &entry : m_deferred_entries)
- {
- const cooked_index_entry *parent_entry
- = find_parent (entry.spec_offset);
- resolve_deferred_entry (entry, parent_entry);
- }
+ m_index_storage->defer_entry (de);
}
};
@@ -16406,13 +16370,22 @@ cooked_indexer::scan_attributes (dwarf2_per_cu_data *scanning_per_cu,
const gdb_byte *new_info_ptr = (new_reader->buffer
+ to_underlying (origin_offset));
- if (new_reader->cu == reader->cu
- && new_info_ptr > watermark_ptr
+ if ((new_reader->cu != reader->cu
+ || (new_reader->cu == reader->cu
+ && new_info_ptr > watermark_ptr))
&& *parent_entry == nullptr)
- *maybe_defer = form_addr (origin_offset, origin_is_dwz);
+ {
+ gdb_assert (reader->cu->per_cu->is_debug_types
+ == new_reader->cu->per_cu->is_debug_types);
+ *maybe_defer = form_addr (origin_offset, origin_is_dwz,
+ reader->cu->per_cu->is_debug_types);
+ }
else if (*parent_entry == nullptr)
{
- CORE_ADDR lookup = form_addr (origin_offset, origin_is_dwz);
+ gdb_assert (reader->cu->per_cu->is_debug_types
+ == new_reader->cu->per_cu->is_debug_types);
+ CORE_ADDR lookup = form_addr (origin_offset, origin_is_dwz,
+ reader->cu->per_cu->is_debug_types);
*parent_entry = find_parent (lookup);
}
@@ -16532,9 +16505,11 @@ cooked_indexer::recurse (cutu_reader *reader,
/* Both start and end are inclusive, so use both "+ 1" and "- 1" to
limit the range to the children of parent_entry. */
CORE_ADDR start = form_addr (parent_entry->die_offset + 1,
- reader->cu->per_cu->is_dwz);
+ reader->cu->per_cu->is_dwz,
+ reader->cu->per_cu->is_debug_types);
CORE_ADDR end = form_addr (sect_offset (info_ptr - 1 - reader->buffer),
- reader->cu->per_cu->is_dwz);
+ reader->cu->per_cu->is_dwz,
+ reader->cu->per_cu->is_debug_types);
set_parent (start, end, parent_entry);
}
@@ -16601,9 +16576,14 @@ cooked_indexer::index_dies (cutu_reader *reader,
if (name != nullptr)
{
if (defer != 0)
- defer_entry ({
- this_die, name, defer, abbrev->tag, flags
- });
+ {
+ CORE_ADDR addr = form_addr (this_die, reader->cu->per_cu->is_dwz,
+ reader->cu->per_cu->is_debug_types);
+ set_parent (addr, addr, &parent_map::deferred);
+ defer_entry ({
+ this_die, name, defer, abbrev->tag, flags, m_per_cu
+ });
+ }
else
this_entry = m_index_storage->add (this_die, abbrev->tag, flags,
name, this_parent_entry,
@@ -16705,8 +16685,6 @@ cooked_indexer::make_index (cutu_reader *reader)
if (!reader->comp_unit_die->has_children)
return;
index_dies (reader, reader->info_ptr, nullptr, false);
-
- handle_deferred_entries ();
}
/* An implementation of quick_symbol_functions for the cooked DWARF