[PATCH gdb/symtab 0/7] Compute DWARF entry parents across CUs

[PATCH gdb/symtab 0/7] Compute DWARF entry parents across CUs

[Tom Tromey]

This is a different approach to a patch series that Tom de Vries sent.
The bug here is that gdb won't always correctly compute the parent of
an entry in some cross-CU reference cases.

I've included a few of Tom's patches in this series.  The main
difference overall is that this one incorporates the parent
computation into the finalization loop, so the work is parallelized.

Regression tested on x86-64 Fedora 38 with the cc-with-dwz board.
There is still one bug (the inlining one), but I believe that one
requires a different fix.

---
Tom Tromey (4):
      Change handling of DW_TAG_enumeration_type in DWARF scanner
      Add move operators for addrmap
      Introduce class parent_map for DIE range map
      Correctly handle DIE parent computations

Tom de Vries (3):
      [gdb/symtab] Refactor condition in scan_attributes
      [gdb/testsuite] Add gdb.dwarf2/forward-spec-inter-cu.exp
      [gdb/testsuite] Add gdb.dwarf2/backward-spec-inter-cu.exp

 gdb/addrmap.c                                      |  12 +-
 gdb/addrmap.h                                      |  19 ++-
 gdb/dwarf2/cooked-index.c                          |  13 +-
 gdb/dwarf2/cooked-index.h                          |  52 ++++++--
 gdb/dwarf2/parent-map.h                            | 132 +++++++++++++++++++++
 gdb/dwarf2/read.c                                  | 113 +++++++-----------
 .../gdb.dwarf2/backward-spec-inter-cu.exp          | 103 ++++++++++++++++
 gdb/testsuite/gdb.dwarf2/forward-spec-inter-cu.exp | 103 ++++++++++++++++
 8 files changed, 458 insertions(+), 89 deletions(-)
---
base-commit: 2002c0099f9fb4d737930acd66733cfad39f68f1
change-id: 20240117-die-map-madness-b84a75473c85

Best regards,
-- 
Tom Tromey <tom@tromey.com>

[PATCH gdb/symtab 1/7] Refactor condition in scan_attributes

[Tom Tromey]

From: Tom de Vries <tdevries@suse.de>

In scan_attributes there's code:
...
	  if (new_reader->cu == reader->cu
	      && new_info_ptr > watermark_ptr
	      && *parent_entry == nullptr)
	    ...
	  else if (*parent_entry == nullptr)
	    ...
...
that uses the "*parent_entry == nullptr" condition twice.

Make this somewhat more readable by factoring out the condition:
...
	  if (*parent_entry == nullptr)
	    {
	      if (new_reader->cu == reader->cu
		  && new_info_ptr > watermark_ptr)
		...
	      else
		...
	    }
...

This also allows us to factor out "form_addr (origin_offset, origin_is_dwz)".

Tested on x86_64-linux.
---
 gdb/dwarf2/read.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 8f2b7a35f27..22cd89122cd 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -16335,15 +16335,17 @@ 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
-	      && *parent_entry == nullptr)
-	    *maybe_defer = form_addr (origin_offset, origin_is_dwz);
-	  else if (*parent_entry == nullptr)
+	  if (*parent_entry == nullptr)
 	    {
-	      CORE_ADDR lookup = form_addr (origin_offset, origin_is_dwz);
-	      void *obj = m_die_range_map.find (lookup);
-	      *parent_entry = static_cast <cooked_index_entry *> (obj);
+	      CORE_ADDR addr = form_addr (origin_offset, origin_is_dwz);
+	      if (new_reader->cu == reader->cu
+		  && new_info_ptr > watermark_ptr)
+		*maybe_defer = addr;
+	      else
+		{
+		  void *obj = m_die_range_map.find (addr);
+		  *parent_entry = static_cast <cooked_index_entry *> (obj);
+		}
 	    }
 
 	  unsigned int bytes_read;

-- 
2.43.0

[PATCH 2/7] Change handling of DW_TAG_enumeration_type in DWARF scanner

[Tom Tromey]

Currently the DWARF scanner will enter enumeration constants into the
same namespace as the DW_TAG_enumeration_type itself.  This is the
right thing to do, but the implementation may result in strange
entries being added to the addrmap that maps DIE ranges to entries.

This came up when debugging an earlier version of this series; and
while I don't think this should impact the current series, it seems
better to clean this up anyway.

In the new code, rather than pass the "wrong" scope down through
recursive calls to the scanner, the correct scope is always passed,
and then the parent handling is done when creating the enumerator
entry.
---
 gdb/dwarf2/read.c | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 22cd89122cd..bf64d89ef3c 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -16522,6 +16522,12 @@ cooked_indexer::index_dies (cutu_reader *reader,
 				  info_ptr, abbrev, &name, &linkage_name,
 				  &flags, &sibling, &this_parent_entry,
 				  &defer, false);
+      /* If the parent is an enum, but not an enum class, then use the
+	 grandparent instead.  */
+      if (this_parent_entry != nullptr
+	  && this_parent_entry->tag == DW_TAG_enumeration_type
+	  && (this_parent_entry->flags & IS_ENUM_CLASS) == 0)
+	this_parent_entry = this_parent_entry->get_parent ();
 
       if (abbrev->tag == DW_TAG_namespace
 	  && m_language == language_cplus
@@ -16584,17 +16590,7 @@ cooked_indexer::index_dies (cutu_reader *reader,
 	      break;
 
 	    case DW_TAG_enumeration_type:
-	      /* We need to recurse even for an anonymous enumeration.
-		 Which scope we record as the parent scope depends on
-		 whether we're reading an "enum class".  If so, we use
-		 the enum itself as the parent, yielding names like
-		 "enum_class::enumerator"; otherwise we inject the
-		 names into our own parent scope.  */
-	      info_ptr = recurse (reader, info_ptr,
-				  ((flags & IS_ENUM_CLASS) == 0)
-				  ? parent_entry
-				  : this_entry,
-				  fully);
+	      info_ptr = recurse (reader, info_ptr, this_entry, fully);
 	      continue;
 
 	    case DW_TAG_module:

-- 
2.43.0

[PATCH 3/7] Add move operators for addrmap

[Tom Tromey]

A subsequent patch needs to move an addrmap.  This patch adds the
necessary support.  It also changes addrmap_fixed to take a 'const'
addrmap_mutable.  This is fine according to the contract of
addrmap_mutable; but it did require a compensating const_cast in the
implementation.
---
 gdb/addrmap.c | 12 +++++++-----
 gdb/addrmap.h | 19 ++++++++++++++++++-
 2 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/gdb/addrmap.c b/gdb/addrmap.c
index 162ab84763b..682ffa98573 100644
--- a/gdb/addrmap.c
+++ b/gdb/addrmap.c
@@ -251,12 +251,13 @@ addrmap_mutable::do_find (CORE_ADDR addr) const
 }
 
 
-addrmap_fixed::addrmap_fixed (struct obstack *obstack, addrmap_mutable *mut)
+addrmap_fixed::addrmap_fixed (struct obstack *obstack,
+			      const addrmap_mutable *mut)
 {
   size_t transition_count = 0;
 
   /* Count the number of transitions in the tree.  */
-  mut->foreach ([&] (CORE_ADDR start, void *obj)
+  mut->foreach ([&] (CORE_ADDR start, const void *obj)
     {
       ++transition_count;
       return 0;
@@ -274,10 +275,10 @@ addrmap_fixed::addrmap_fixed (struct obstack *obstack, addrmap_mutable *mut)
 
   /* Copy all entries from the splay tree to the array, in order 
      of increasing address.  */
-  mut->foreach ([&] (CORE_ADDR start, void *obj)
+  mut->foreach ([&] (CORE_ADDR start, const void *obj)
     {
       transitions[num_transitions].addr = start;
-      transitions[num_transitions].value = obj;
+      transitions[num_transitions].value = const_cast<void *> (obj);
       ++num_transitions;
       return 0;
     });
@@ -345,7 +346,8 @@ addrmap_mutable::addrmap_mutable ()
 
 addrmap_mutable::~addrmap_mutable ()
 {
-  splay_tree_delete (tree);
+  if (tree != nullptr)
+    splay_tree_delete (tree);
 }
 
 
diff --git a/gdb/addrmap.h b/gdb/addrmap.h
index ba83607ad8c..1dd1e35ec11 100644
--- a/gdb/addrmap.h
+++ b/gdb/addrmap.h
@@ -84,9 +84,14 @@ struct addrmap_fixed : public addrmap,
 {
 public:
 
-  addrmap_fixed (struct obstack *obstack, addrmap_mutable *mut);
+  addrmap_fixed (struct obstack *obstack, const addrmap_mutable *mut);
   DISABLE_COPY_AND_ASSIGN (addrmap_fixed);
 
+  /* It's fine to use the default move operators, because this addrmap
+     does not own the storage for the elements.  */
+  addrmap_fixed (addrmap_fixed &&other) = default;
+  addrmap_fixed &operator= (addrmap_fixed &&) = default;
+
   void relocate (CORE_ADDR offset) override;
 
 private:
@@ -123,6 +128,18 @@ struct addrmap_mutable : public addrmap
   ~addrmap_mutable ();
   DISABLE_COPY_AND_ASSIGN (addrmap_mutable);
 
+  addrmap_mutable (addrmap_mutable &&other)
+    : tree (other.tree)
+  {
+    other.tree = nullptr;
+  }
+
+  addrmap_mutable &operator= (addrmap_mutable &&other)
+  {
+    std::swap (tree, other.tree);
+    return *this;
+  }
+
   /* In the mutable address map MAP, associate the addresses from START
      to END_INCLUSIVE that are currently associated with NULL with OBJ
      instead.  Addresses mapped to an object other than NULL are left

-- 
2.43.0

[PATCH 4/7] Introduce class parent_map for DIE range map

[Tom Tromey]

This changes the DIE range map from a raw addrmap to a custom class.
A new type is used to represent the ranges, in an attempt to gain a
little type safety as well.

Note that the new code includes a map-of-maps type.  This is not used
yet, but will be used in the next patch.

Co-Authored-By: Tom de Vries <tdevries@suse.de>
---
 gdb/dwarf2/cooked-index.h |   7 +--
 gdb/dwarf2/parent-map.h   | 132 ++++++++++++++++++++++++++++++++++++++++++++++
 gdb/dwarf2/read.c         |  46 +++++++---------
 3 files changed, 154 insertions(+), 31 deletions(-)

diff --git a/gdb/dwarf2/cooked-index.h b/gdb/dwarf2/cooked-index.h
index ebaed04753c..442ee9e14fd 100644
--- a/gdb/dwarf2/cooked-index.h
+++ b/gdb/dwarf2/cooked-index.h
@@ -35,6 +35,7 @@
 #include "dwarf2/read.h"
 #include "dwarf2/tag.h"
 #include "dwarf2/abbrev-cache.h"
+#include "dwarf2/parent-map.h"
 #include "gdbsupport/range-chain.h"
 #include "gdbsupport/task-group.h"
 #include "complaints.h"
@@ -74,7 +75,7 @@ DEF_ENUM_FLAGS_TYPE (enum cooked_index_flag_enum, cooked_index_flag);
 
 union cooked_index_entry_ref
 {
-  cooked_index_entry_ref (CORE_ADDR deferred_)
+  cooked_index_entry_ref (parent_map::addr_type deferred_)
   {
     deferred = deferred_;
   }
@@ -85,7 +86,7 @@ union cooked_index_entry_ref
   }
 
   const cooked_index_entry *resolved;
-  CORE_ADDR deferred;
+  parent_map::addr_type deferred;
 };
 
 /* Return a string representation of FLAGS.  */
@@ -265,7 +266,7 @@ struct cooked_index_entry : public allocate_on_obstack
   }
 
   /* Return deferred parent entry.  */
-  CORE_ADDR get_deferred_parent () const
+  parent_map::addr_type get_deferred_parent () const
   {
     gdb_assert ((flags & IS_PARENT_DEFERRED) != 0);
     return m_parent_entry.deferred;
diff --git a/gdb/dwarf2/parent-map.h b/gdb/dwarf2/parent-map.h
new file mode 100644
index 00000000000..f070d505356
--- /dev/null
+++ b/gdb/dwarf2/parent-map.h
@@ -0,0 +1,132 @@
+/* DIE indexing 
+
+   Copyright (C) 2024 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef GDB_DWARF2_PARENT_MAP_H
+#define GDB_DWARF2_PARENT_MAP_H
+
+#include <algorithm>
+
+class cooked_index_entry;
+
+/* A class that handles mapping from a DIE range to a parent
+   entry.
+
+   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.  */
+class parent_map
+{
+public:
+
+  parent_map () = default;
+  ~parent_map () = default;
+
+  /* Move only.  */
+  DISABLE_COPY_AND_ASSIGN (parent_map);
+  parent_map (parent_map &&) = default;
+  parent_map &operator= (parent_map &&) = default;
+
+  /* A reasonably opaque type that is used here to combine a section
+     offset and the 'dwz' flag into a single value.  */
+  enum addr_type : CORE_ADDR { };
+
+  /* Turn a section offset into a value that can be used in a parent
+     map.  */
+  static addr_type form_addr (sect_offset offset, bool is_dwz)
+  {
+    CORE_ADDR value = to_underlying (offset);
+    if (is_dwz)
+      value |= ((CORE_ADDR) 1) << (8 * sizeof (CORE_ADDR) - 1);
+    return addr_type (value);
+  }
+
+  /* Add a new entry to this map.  DIEs from START to END, inclusive,
+     are mapped to PARENT.  */
+  void add_entry (addr_type start, addr_type end,
+		  const cooked_index_entry *parent)
+  {
+    gdb_assert (parent != nullptr);
+    m_map.set_empty (start, end, (void *) parent);
+  }
+
+  /* Look up an entry in this map.  */
+  const cooked_index_entry *find (addr_type search) const
+  {
+    return static_cast<const cooked_index_entry *> (m_map.find (search));
+  }
+
+  /* Return a fixed addrmap that is equivalent to this map.  */
+  addrmap_fixed *to_fixed (struct obstack *obstack) const
+  {
+    return new (obstack) addrmap_fixed (obstack, &m_map);
+  }
+
+private:
+
+  /* An addrmap that maps from section offsets to cooked_index_entry *.  */
+  addrmap_mutable m_map;
+};
+
+/* Keep a collection of parent_map objects, and allow for lookups
+   across all of them.  */
+class parent_map_map
+{
+public:
+
+  parent_map_map () = default;
+  ~parent_map_map () = default;
+
+  DISABLE_COPY_AND_ASSIGN (parent_map_map);
+
+  /* Add a parent_map to this map.  */
+  void add_map (const parent_map &map)
+  {
+    m_maps.push_back (map.to_fixed (&m_storage));
+  }
+
+  /* Look up an entry in this map.  */
+  const cooked_index_entry *find (parent_map::addr_type search) const
+  {
+    for (const auto &iter : m_maps)
+      {
+	const cooked_index_entry *result
+	  = static_cast<const cooked_index_entry *> (iter->find (search));
+	if (result != nullptr)
+	  return result;
+      }
+    return nullptr;
+  }
+
+private:
+
+  /* Storage for the convert maps.  */
+  auto_obstack m_storage;
+
+  /* While conceptually this class is a combination of parent_maps, in
+     practice it is just a number of fixed maps.  This is important
+     because we want to allow concurrent lookups, but a mutable
+     addrmap is based on a splay-tree, which is not thread-safe, even
+     for nominally read-only lookups.  */
+  std::vector<addrmap_fixed *> m_maps;
+};
+
+#endif /* GDB_DWARF2_PARENT_MAP_H */
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index bf64d89ef3c..585c15212f8 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -96,6 +96,7 @@
 #include "split-name.h"
 #include "gdbsupport/thread-pool.h"
 #include "run-on-main-thread.h"
+#include "dwarf2/parent-map.h"
 
 /* When == 1, print basic high level tracing messages.
    When > 1, be more verbose.
@@ -4473,16 +4474,6 @@ class cooked_indexer
 
 private:
 
-  /* 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 value = to_underlying (offset);
-    if (is_dwz)
-      value |= ((CORE_ADDR) 1) << (8 * sizeof (CORE_ADDR) - 1);
-    return value;
-  }
-
   /* A helper function to scan the PC bounds of READER and record them
      in the storage's addrmap.  */
   void check_bounds (cutu_reader *reader);
@@ -4520,7 +4511,7 @@ class cooked_indexer
 				   cooked_index_flag *flags,
 				   sect_offset *sibling_offset,
 				   const cooked_index_entry **parent_entry,
-				   CORE_ADDR *maybe_defer,
+				   parent_map::addr_type *maybe_defer,
 				   bool for_specification);
 
   /* Handle DW_TAG_imported_unit, by scanning the DIE to find
@@ -4547,10 +4538,9 @@ class cooked_indexer
   /* The language that we're assuming when reading.  */
   enum language m_language;
 
-  /* An addrmap that maps from section offsets (see the form_addr
-     method) to newly-created entries.  See m_deferred_entries to
-     understand this.  */
-  addrmap_mutable m_die_range_map;
+  /* Map from DIE ranges 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
@@ -16165,7 +16155,7 @@ cooked_indexer::scan_attributes (dwarf2_per_cu_data *scanning_per_cu,
 				 cooked_index_flag *flags,
 				 sect_offset *sibling_offset,
 				 const cooked_index_entry **parent_entry,
-				 CORE_ADDR *maybe_defer,
+				 parent_map::addr_type *maybe_defer,
 				 bool for_specification)
 {
   bool origin_is_dwz = false;
@@ -16337,15 +16327,13 @@ cooked_indexer::scan_attributes (dwarf2_per_cu_data *scanning_per_cu,
 
 	  if (*parent_entry == nullptr)
 	    {
-	      CORE_ADDR addr = form_addr (origin_offset, origin_is_dwz);
+	      parent_map::addr_type addr
+		= parent_map::form_addr (origin_offset, origin_is_dwz);
 	      if (new_reader->cu == reader->cu
 		  && new_info_ptr > watermark_ptr)
 		*maybe_defer = addr;
 	      else
-		{
-		  void *obj = m_die_range_map.find (addr);
-		  *parent_entry = static_cast <cooked_index_entry *> (obj);
-		}
+		*parent_entry = m_die_range_map.find (addr);
 	    }
 
 	  unsigned int bytes_read;
@@ -16463,11 +16451,13 @@ 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);
-      CORE_ADDR end = form_addr (sect_offset (info_ptr - 1 - reader->buffer),
+      parent_map::addr_type start
+	= parent_map::form_addr (parent_entry->die_offset + 1,
+				 reader->cu->per_cu->is_dwz);
+      parent_map::addr_type end
+	= parent_map::form_addr (sect_offset (info_ptr - 1 - reader->buffer),
 				 reader->cu->per_cu->is_dwz);
-      m_die_range_map.set_empty (start, end, (void *) parent_entry);
+      m_die_range_map.add_entry (start, end, parent_entry);
     }
 
   return info_ptr;
@@ -16509,7 +16499,7 @@ cooked_indexer::index_dies (cutu_reader *reader,
 
       const char *name = nullptr;
       const char *linkage_name = nullptr;
-      CORE_ADDR defer = 0;
+      parent_map::addr_type defer {};
       cooked_index_flag flags = IS_STATIC;
       sect_offset sibling {};
       const cooked_index_entry *this_parent_entry = parent_entry;
@@ -16645,8 +16635,8 @@ cooked_indexer::make_index (cutu_reader *reader)
 
   for (const auto &entry : m_deferred_entries)
     {
-      void *obj = m_die_range_map.find (entry->get_deferred_parent ());
-      cooked_index_entry *parent = static_cast<cooked_index_entry *> (obj);
+      const cooked_index_entry *parent
+	= m_die_range_map.find (entry->get_deferred_parent ());
       entry->resolve_parent (parent);
     }
 }

-- 
2.43.0

[PATCH 5/7] Correctly handle DIE parent computations

[Tom Tromey]

Tom de Vries pointed out that the combination of sharding,
multi-threading, and per-CU "racing" means that sometimes a cross-CU
DIE reference might not be correctly resolved.  However, it's
important to handle this correctly, due to some unfortunate aspects of
DWARF.

This patch implements this by arranging to preserve each worker's DIE
map through the end of index finalization.  The extra data is
discarded when finalization is done.  This approach also allows the
parent name resolution to be sharded, by integrating it into the
existing entry finalization loop.

In an earlier review, I remarked that addrmap couldn't be used here.
However, I was mistaken.  A *mutable* addrmap cannot be used, as those
are based on splay trees and restructure the tree even during lookups
(and thus aren't thread-safe).  A fixed addrmap, on the other hand, is
just a vector and is thread-safe.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30846
---
 gdb/dwarf2/cooked-index.c | 13 ++++++++---
 gdb/dwarf2/cooked-index.h | 45 ++++++++++++++++++++++++++++--------
 gdb/dwarf2/read.c         | 59 ++++++++++++++++++++---------------------------
 3 files changed, 70 insertions(+), 47 deletions(-)

diff --git a/gdb/dwarf2/cooked-index.c b/gdb/dwarf2/cooked-index.c
index e4f112b6ee0..93370f96bec 100644
--- a/gdb/dwarf2/cooked-index.c
+++ b/gdb/dwarf2/cooked-index.c
@@ -319,7 +319,7 @@ cooked_index_shard::handle_gnat_encoded_entry (cooked_index_entry *entry,
 /* See cooked-index.h.  */
 
 void
-cooked_index_shard::finalize ()
+cooked_index_shard::finalize (const parent_map_map *parent_maps)
 {
   auto hash_name_ptr = [] (const void *p)
     {
@@ -360,6 +360,13 @@ cooked_index_shard::finalize ()
 
   for (cooked_index_entry *entry : m_entries)
     {
+      if ((entry->flags & IS_PARENT_DEFERRED) != 0)
+	{
+	  const cooked_index_entry *new_parent
+	    = parent_maps->find (entry->get_deferred_parent ());
+	  entry->resolve_parent (new_parent);
+	}
+
       /* Note that this code must be kept in sync with
 	 language_requires_canonicalization.  */
       gdb_assert (entry->canonical == nullptr);
@@ -479,7 +486,7 @@ cooked_index::wait (cooked_state desired_state, bool allow_quit)
 }
 
 void
-cooked_index::set_contents (vec_type &&vec)
+cooked_index::set_contents (vec_type &&vec, const parent_map_map *parent_maps)
 {
   gdb_assert (m_vector.empty ());
   m_vector = std::move (vec);
@@ -502,7 +509,7 @@ cooked_index::set_contents (vec_type &&vec)
   for (auto &idx : m_vector)
     {
       auto this_index = idx.get ();
-      finalizers.add_task ([=] () { this_index->finalize (); });
+      finalizers.add_task ([=] () { this_index->finalize (parent_maps); });
     }
 
   finalizers.start ();
diff --git a/gdb/dwarf2/cooked-index.h b/gdb/dwarf2/cooked-index.h
index 442ee9e14fd..d8e6fe476a4 100644
--- a/gdb/dwarf2/cooked-index.h
+++ b/gdb/dwarf2/cooked-index.h
@@ -392,9 +392,9 @@ class cooked_index_shard
 
   /* Finalize the index.  This should be called a single time, when
      the index has been fully populated.  It enters all the entries
-     into the internal table.  This may be invoked in a worker
-     thread.  */
-  void finalize ();
+     into the internal table and fixes up all missing parent links.
+     This may be invoked in a worker thread.  */
+  void finalize (const parent_map_map *parent_maps);
 
   /* Storage for the entries.  */
   auto_obstack m_storage;
@@ -459,6 +459,19 @@ class cooked_index_storage
     return &m_addrmap;
   }
 
+  /* Return the parent_map that is currently being created.  */
+  parent_map *get_parent_map ()
+  {
+    return &m_parent_map;
+  }
+
+  /* Return the parent_map that is currently being created.  Ownership
+     is passed to the caller.  */
+  parent_map release_parent_map ()
+  {
+    return std::move (m_parent_map);
+  }
+
 private:
 
   /* Hash function for a cutu_reader.  */
@@ -474,6 +487,9 @@ class cooked_index_storage
   /* The index shard that is being constructed.  */
   std::unique_ptr<cooked_index_shard> m_index;
 
+  /* Parent map for each CU that is read.  */
+  parent_map m_parent_map;
+
   /* A writeable addrmap being constructed by this scanner.  */
   addrmap_mutable m_addrmap;
 };
@@ -544,13 +560,17 @@ class cooked_index_worker
  		    unit_iterator end);
 
   /* Each thread returns a tuple holding a cooked index, any collected
-     complaints, and a vector of errors that should be printed.  The
-     latter is done because GDB's I/O system is not thread-safe.
-     run_on_main_thread could be used, but that would mean the
-     messages are printed after the prompt, which looks weird.  */
+     complaints, a vector of errors that should be printed, and a
+     vector of parent maps.
+
+     The errors are retained because GDB's I/O system is not
+     thread-safe.  run_on_main_thread could be used, but that would
+     mean the messages are printed after the prompt, which looks
+     weird.  */
   using result_type = std::tuple<std::unique_ptr<cooked_index_shard>,
 				 complaint_collection,
-				 std::vector<gdb_exception>>;
+				 std::vector<gdb_exception>,
+				 parent_map>;
 
   /* The per-objfile object.  */
   dwarf2_per_objfile *m_per_objfile;
@@ -567,6 +587,10 @@ class cooked_index_worker
      This is enforced in the cooked_index_worker constructor.  */
   deferred_warnings m_warnings;
 
+  /* A map of all parent maps.  Used during finalization to fix up
+     parent relationships.  */
+  parent_map_map m_all_parents_map;
+
 #if CXX_STD_THREAD
   /* Current state of this object.  */
   cooked_state m_state = cooked_state::INITIAL;
@@ -654,8 +678,9 @@ class cooked_index : public dwarf_scanner_base
   void start_reading ();
 
   /* Called by cooked_index_worker to set the contents of this index
-     and transition to the MAIN_AVAILABLE state.  */
-  void set_contents (vec_type &&vec);
+     and transition to the MAIN_AVAILABLE state.  PARENT_MAPS is used
+     when resolving pending parent links.  */
+  void set_contents (vec_type &&vec, const parent_map_map *parent_maps);
 
   /* A range over a vector of subranges.  */
   using range = range_chain<cooked_index_shard::range>;
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 585c15212f8..494f2b5625c 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -4463,7 +4463,8 @@ class cooked_indexer
 		  enum language language)
     : m_index_storage (storage),
       m_per_cu (per_cu),
-      m_language (language)
+      m_language (language),
+      m_die_range_map (storage->get_parent_map ())
   {
   }
 
@@ -4538,18 +4539,8 @@ class cooked_indexer
   /* The language that we're assuming when reading.  */
   enum language m_language;
 
-  /* Map from DIE ranges 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<cooked_index_entry *> m_deferred_entries;
+  /* Map from DIE ranges to newly-created entries.  */
+  parent_map *m_die_range_map;
 };
 
 /* Subroutine of dwarf2_build_psymtabs_hard to simplify it.
@@ -4866,7 +4857,8 @@ cooked_index_worker::process_cus (size_t task_number, unit_iterator first,
 
   m_results[task_number] = result_type (thread_storage.release (),
 					complaint_handler.release (),
-					std::move (errors));
+					std::move (errors),
+					std::move (thread_storage.release_parent_map ()));
 }
 
 void
@@ -4876,7 +4868,10 @@ cooked_index_worker::done_reading ()
      can only be dealt with on the main thread.  */
   std::vector<std::unique_ptr<cooked_index_shard>> indexes;
   for (auto &one_result : m_results)
-    indexes.push_back (std::move (std::get<0> (one_result)));
+    {
+      indexes.push_back (std::move (std::get<0> (one_result)));
+      m_all_parents_map.add_map (std::get<3> (one_result));
+    }
 
   /* This has to wait until we read the CUs, we need the list of DWOs.  */
   process_skeletonless_type_units (m_per_objfile, &m_index_storage);
@@ -4884,11 +4879,13 @@ cooked_index_worker::done_reading ()
   indexes.push_back (m_index_storage.release ());
   indexes.shrink_to_fit ();
 
+  m_all_parents_map.add_map (m_index_storage.release_parent_map ());
+
   dwarf2_per_bfd *per_bfd = m_per_objfile->per_bfd;
   cooked_index *table
     = (gdb::checked_static_cast<cooked_index *>
        (per_bfd->index_table.get ()));
-  table->set_contents (std::move (indexes));
+  table->set_contents (std::move (indexes), &m_all_parents_map);
 }
 
 void
@@ -16327,13 +16324,17 @@ cooked_indexer::scan_attributes (dwarf2_per_cu_data *scanning_per_cu,
 
 	  if (*parent_entry == nullptr)
 	    {
+	      /* We only perform immediate lookups of parents for DIEs
+		 from earlier in this CU.  This avoids any problem
+		 with a NULL result when when we see a reference to a
+		 DIE in another CU that we may or may not have
+		 imported locally.  */
 	      parent_map::addr_type addr
 		= parent_map::form_addr (origin_offset, origin_is_dwz);
-	      if (new_reader->cu == reader->cu
-		  && new_info_ptr > watermark_ptr)
+	      if (new_reader->cu != reader->cu || new_info_ptr > watermark_ptr)
 		*maybe_defer = addr;
 	      else
-		*parent_entry = m_die_range_map.find (addr);
+		*parent_entry = m_die_range_map->find (addr);
 	    }
 
 	  unsigned int bytes_read;
@@ -16457,7 +16458,7 @@ cooked_indexer::recurse (cutu_reader *reader,
       parent_map::addr_type end
 	= parent_map::form_addr (sect_offset (info_ptr - 1 - reader->buffer),
 				 reader->cu->per_cu->is_dwz);
-      m_die_range_map.add_entry (start, end, parent_entry);
+      m_die_range_map->add_entry (start, end, parent_entry);
     }
 
   return info_ptr;
@@ -16534,13 +16535,10 @@ cooked_indexer::index_dies (cutu_reader *reader,
       if (name != nullptr)
 	{
 	  if (defer != 0)
-	    {
-	      this_entry
-		= m_index_storage->add (this_die, abbrev->tag,
-					flags | IS_PARENT_DEFERRED, name,
-					defer, m_per_cu);
-	      m_deferred_entries.push_back (this_entry);
-	    }
+	    this_entry
+	      = m_index_storage->add (this_die, abbrev->tag,
+				      flags | IS_PARENT_DEFERRED, name,
+				      defer, m_per_cu);
 	  else
 	    this_entry
 	      = m_index_storage->add (this_die, abbrev->tag, flags, name,
@@ -16632,13 +16630,6 @@ cooked_indexer::make_index (cutu_reader *reader)
   if (!reader->comp_unit_die->has_children)
     return;
   index_dies (reader, reader->info_ptr, nullptr, false);
-
-  for (const auto &entry : m_deferred_entries)
-    {
-      const cooked_index_entry *parent
-	= m_die_range_map.find (entry->get_deferred_parent ());
-      entry->resolve_parent (parent);
-    }
 }
 
 /* An implementation of quick_symbol_functions for the cooked DWARF

-- 
2.43.0

[PATCH gdb/testsuite 6/7] Add gdb.dwarf2/forward-spec-inter-cu.exp

[Tom Tromey]

From: Tom de Vries <tdevries@suse.de>

Add a regression test for PR symtab/30846.

Tested on x86_64-linux.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30846
---
 gdb/testsuite/gdb.dwarf2/forward-spec-inter-cu.exp | 103 +++++++++++++++++++++
 1 file changed, 103 insertions(+)

diff --git a/gdb/testsuite/gdb.dwarf2/forward-spec-inter-cu.exp b/gdb/testsuite/gdb.dwarf2/forward-spec-inter-cu.exp
new file mode 100644
index 00000000000..d8367b0a162
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/forward-spec-inter-cu.exp
@@ -0,0 +1,103 @@
+# Copyright 2023 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Check that the DWARF reader works with a a DW_AT_specification that
+# refers to a later DIE.  Inter-cu variant of forward-spec.exp.
+
+load_lib dwarf.exp
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+require dwarf2_support
+
+standard_testfile main.c -debug.S
+
+# Set up the DWARF for the test.
+set asm_file [standard_output_file $srcfile2]
+Dwarf::assemble $asm_file {
+    global srcfile
+
+    declare_labels spec
+
+    cu {} {
+	DW_TAG_compile_unit {
+	    {DW_AT_language @DW_LANG_C_plus_plus}
+	} {
+	    # The new indexer has special code to compute the full
+	    # name of an object that uses a specification that appears
+	    # later in the DWARF.
+	    DW_TAG_variable {
+		{DW_AT_specification %$spec}
+		{DW_AT_location {
+		    DW_OP_const1u 23
+		    DW_OP_stack_value
+		} SPECIAL_expr}
+	    }
+	}
+    }
+
+    cu {} {
+	DW_TAG_compile_unit {
+	    {DW_AT_language @DW_LANG_C_plus_plus}
+	} {
+	    declare_labels myint
+
+	    myint: DW_TAG_base_type {
+		{DW_AT_byte_size 4 DW_FORM_sdata}
+		{DW_AT_encoding  @DW_ATE_signed}
+		{DW_AT_name      myint}
+	    }
+
+	    DW_TAG_namespace {
+		{DW_AT_name ns}
+	    } {
+		spec: DW_TAG_variable {
+		    {DW_AT_name v}
+		    {DW_AT_type :$myint}
+		    {DW_AT_declaration 1 DW_FORM_flag_present}
+		}
+	    }
+	}
+    }
+}
+
+if {[build_executable "failed to build executable" ${testfile} \
+	 [list $srcfile $asm_file] {nodebug}]} {
+    return -1
+}
+
+set eol "\r\n"
+set ws "\[ \t\]"
+
+set worker_threads_list {}
+
+# Exercises the intra-shard case.
+lappend worker_threads_list 0
+
+# Might exercise the inter-shard case.
+lappend worker_threads_list default
+
+foreach_with_prefix worker_threads $worker_threads_list {
+
+    clean_restart
+
+    if { $worker_threads != "default" } {
+	gdb_test_no_output "maint set worker-threads $worker_threads"
+    }
+
+    gdb_load $binfile
+
+    gdb_test "maint print objfiles" "$eol$ws+qualified:$ws+ns::v$eol.*" \
+	"v has parent ns"
+}

-- 
2.43.0

[PATCH gdb/testsuite 7/7] Add gdb.dwarf2/backward-spec-inter-cu.exp

[Tom Tromey]

From: Tom de Vries <tdevries@suse.de>

Add another regression test for PR symtab/30846.

Tested on x86_64-linux.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30846
---
 .../gdb.dwarf2/backward-spec-inter-cu.exp          | 103 +++++++++++++++++++++
 1 file changed, 103 insertions(+)

diff --git a/gdb/testsuite/gdb.dwarf2/backward-spec-inter-cu.exp b/gdb/testsuite/gdb.dwarf2/backward-spec-inter-cu.exp
new file mode 100644
index 00000000000..59b3db50dbb
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/backward-spec-inter-cu.exp
@@ -0,0 +1,103 @@
+# Copyright 2023 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Check that the DWARF reader works with a a DW_AT_specification that
+# refers to an earlier DIE.  Inter-cu variant of forward-spec.exp.
+
+load_lib dwarf.exp
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+require dwarf2_support
+
+standard_testfile main.c -debug.S
+
+# Set up the DWARF for the test.
+set asm_file [standard_output_file $srcfile2]
+Dwarf::assemble $asm_file {
+    global srcfile
+
+    declare_labels spec
+
+    cu {} {
+	DW_TAG_compile_unit {
+	    {DW_AT_language @DW_LANG_C_plus_plus}
+	} {
+	    declare_labels myint
+
+	    myint: DW_TAG_base_type {
+		{DW_AT_byte_size 4 DW_FORM_sdata}
+		{DW_AT_encoding  @DW_ATE_signed}
+		{DW_AT_name      myint}
+	    }
+
+	    DW_TAG_namespace {
+		{DW_AT_name ns}
+	    } {
+		spec: DW_TAG_variable {
+		    {DW_AT_name v}
+		    {DW_AT_type :$myint}
+		    {DW_AT_declaration 1 DW_FORM_flag_present}
+		}
+	    }
+	}
+    }
+
+    cu {} {
+	DW_TAG_compile_unit {
+	    {DW_AT_language @DW_LANG_C_plus_plus}
+	} {
+	    # The new indexer has special code to compute the full
+	    # name of an object that uses a specification that appears
+	    # later in the DWARF.
+	    DW_TAG_variable {
+		{DW_AT_specification %$spec}
+		{DW_AT_location {
+		    DW_OP_const1u 23
+		    DW_OP_stack_value
+		} SPECIAL_expr}
+	    }
+	}
+    }
+}
+
+if {[build_executable "failed to build executable" ${testfile} \
+	 [list $srcfile $asm_file] {nodebug}]} {
+    return -1
+}
+
+set eol "\r\n"
+set ws "\[ \t\]"
+
+set worker_threads_list {}
+
+# Exercises the intra-shard case.
+lappend worker_threads_list 0
+
+# Might exercise the inter-shard case.
+lappend worker_threads_list default
+
+foreach_with_prefix worker_threads $worker_threads_list {
+
+    clean_restart
+
+    if { $worker_threads != "default" } {
+	gdb_test_no_output "maint set worker-threads $worker_threads"
+    }
+
+    gdb_load $binfile
+
+    gdb_test "maint print objfiles" "$eol$ws+qualified:$ws+ns::v$eol.*" \
+	"v has parent ns"
+}

-- 
2.43.0