[PATCH 00/13] [gdb/symtab, cc-with-dwz] Fix gdb.cp/breakpoint-locs.exp

[PATCH 00/13] [gdb/symtab, cc-with-dwz] Fix gdb.cp/breakpoint-locs.exp

[Tom de Vries]

[ Submitted an earlier RFC version at
https://sourceware.org/pipermail/gdb-patches/2023-September/202443.html. ]

When running test-case gdb.cp/breakpoint-locs.exp with target board
cc-with-dwz, we occasionally run into:
...
FAIL: gdb.cp/breakpoint-locs.exp: break N1::C1::baz
...
due to the breakpoint having one instead of two locations.

This is PR symtab/30728.  This series fixes it.

The fix is dependent on the fix for PR symtab/30846, which is about incorrect
parent entries in the cooked index, due to inter-cu (intra-shard and
inter-shard) references.

The series consists of three parts:
- patches that fix PR symtab/30846.
- patches that add two optimizations to the fix for PR symtab/30846
  (one preparation patch, and two optimization patches),
- the last patch, that fixes PR symtab/30728.

The fix for PR symtab/30728 consists of adding entries to the cooked index for
DW_TAG_inlined_subroutine DIEs, with correct parent entries.

Tested on x86_64-linux, with target boards unix, cc-with-dwz and
cc-with-dwz-m.

I've split out the two added test-cases into separate patches.  Unfortunately
the patch "[gdb/symtab] Resolve deferred entries, inter-shard case" is still
somewhat large due to moving code about.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30728
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30846

Tom de Vries (13):
  [gdb/symtab] Factor out m_die_range_map usage
  [gdb/symtab] Check effect in parent_map::set_parent
  [gdb/symtab] Handle nullptr parent in parent_map::set_parent
  [gdb/symtab] Add parent_map::dump
  [gdb/symtab] Factor out m_deferred_entries usage
  [gdb/symtab] Add debug_handle_deferred_entries
  [gdb/symtab] Resolve deferred entries, inter-shard case
  [gdb/testsuite] Add gdb.dwarf2/forward-spec-inter-cu.exp
  [gdb/symtab] Keep track of processed DIEs in shard
  [gdb/symtab] Resolve deferred entries, intra-shard case
  [gdb/symtab] Don't defer backward refs, inter-cu intra-shard case
  [gdb/testsuite] Add gdb.dwarf2/backward-spec-inter-cu.exp
  [gdb/symtab] Add DW_TAG_inlined_subroutine entries in the cooked index
    for c++

 gdb/addrmap.c                                 |  19 +-
 gdb/addrmap.h                                 |   7 +-
 gdb/dwarf2/cooked-index.c                     | 141 ++++++++++++++
 gdb/dwarf2/cooked-index.h                     | 145 +++++++++++++-
 gdb/dwarf2/read.c                             | 178 +++++++++++++-----
 .../gdb.dwarf2/backward-spec-inter-cu.exp     | 111 +++++++++++
 .../gdb.dwarf2/forward-spec-inter-cu.exp      | 107 +++++++++++
 7 files changed, 657 insertions(+), 51 deletions(-)
 create mode 100644 gdb/testsuite/gdb.dwarf2/backward-spec-inter-cu.exp
 create mode 100644 gdb/testsuite/gdb.dwarf2/forward-spec-inter-cu.exp


base-commit: 6a6117ab0ffe18ea984abca84869eae799c1b346
-- 
2.35.3

[PATCH 01/13] [gdb/symtab] Factor out m_die_range_map usage

[Tom de Vries]

Factor out usage of cooked_indexer::m_die_range_map into new class parent_map
with member functions find_parent and set_parent.

Tested on x86_64-linux.
---
 gdb/dwarf2/cooked-index.h | 22 ++++++++++++++++++++++
 gdb/dwarf2/read.c         | 23 +++++++++++++++++------
 2 files changed, 39 insertions(+), 6 deletions(-)

diff --git a/gdb/dwarf2/cooked-index.h b/gdb/dwarf2/cooked-index.h
index 5aacb321c91..a029fcf2cc9 100644
--- a/gdb/dwarf2/cooked-index.h
+++ b/gdb/dwarf2/cooked-index.h
@@ -239,6 +239,28 @@ struct cooked_index_entry : public allocate_on_obstack
 		    bool for_name) const;
 };
 
+class parent_map
+{
+public:
+  /* Find the parent of DIE LOOKUP.  */
+  const cooked_index_entry *find_parent (CORE_ADDR lookup) const
+  {
+    const void *obj = m_parent_map.find (lookup);
+    return static_cast<const cooked_index_entry *> (obj);
+  }
+
+  /* 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_parent_map.set_empty (start, end, (void *)parent_entry);
+  }
+
+private:
+  /* An addrmap that maps from section offsets to cooked_index_entry *.  */
+  addrmap_mutable m_parent_map;
+};
+
 class cooked_index;
 
 /* An index of interesting DIEs.  This is "cooked", in contrast to a
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 5bbc8e24cf9..17bc650a055 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -4797,7 +4797,20 @@ class cooked_indexer
   /* 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;
+  parent_map m_die_range_map;
+
+  /* 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.  */
   struct deferred_entry
@@ -16356,8 +16369,7 @@ cooked_indexer::scan_attributes (dwarf2_per_cu_data *scanning_per_cu,
 	  else 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);
+	      *parent_entry = find_parent (lookup);
 	    }
 
 	  unsigned int bytes_read;
@@ -16479,7 +16491,7 @@ cooked_indexer::recurse (cutu_reader *reader,
 				   reader->cu->per_cu->is_dwz);
       CORE_ADDR end = 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);
+      set_parent (start, end, parent_entry);
     }
 
   return info_ptr;
@@ -16652,8 +16664,7 @@ cooked_indexer::make_index (cutu_reader *reader)
 
   for (const auto &entry : m_deferred_entries)
     {
-      void *obj = m_die_range_map.find (entry.spec_offset);
-      cooked_index_entry *parent = static_cast<cooked_index_entry *> (obj);
+      const cooked_index_entry *parent = find_parent (entry.spec_offset);
       m_index_storage->add (entry.die_offset, entry.tag, entry.flags,
 			    entry.name, parent, m_per_cu);
     }
-- 
2.35.3

[PATCH 02/13] [gdb/symtab] Check effect in parent_map::set_parent

[Tom de Vries]

Set_parent uses m_die_range_map.set_empty to set the parent of a range.

If part of the range is already set, it remains the same.

If the entire range is already set, the set_empty has no effect, silently.

Fix this by verifying that calling set_empty has the desired effect on the
start and end points of the range.

Tested on x86_64-linux.
---
 gdb/dwarf2/cooked-index.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/gdb/dwarf2/cooked-index.h b/gdb/dwarf2/cooked-index.h
index a029fcf2cc9..9900dbb84a7 100644
--- a/gdb/dwarf2/cooked-index.h
+++ b/gdb/dwarf2/cooked-index.h
@@ -254,6 +254,13 @@ class parent_map
 		   const cooked_index_entry *parent_entry)
   {
     m_parent_map.set_empty (start, end, (void *)parent_entry);
+
+    /* Assert that set_parent has the desired effect.  This is not trivial due
+       to how set_empty works.  If the range already has been set before, it
+       has no effect.  */
+    gdb_assert (m_parent_map.find (start) == parent_entry);
+    if (end != start)
+      gdb_assert (m_parent_map.find (end) == parent_entry);
   }
 
 private:
-- 
2.35.3

[PATCH 03/13] [gdb/symtab] Handle nullptr parent in parent_map::set_parent

[Tom de Vries]

Set_parent uses m_die_range_map.set_empty, which doesn't allow
parent_entry == nullptr.

So it may be necessary to guard calls to set_parent with
"if (parent_entry != nullptr)".

Fix this by handling the parent_entry == nullptr case.

Tested on x86_64-linux.
---
 gdb/dwarf2/cooked-index.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/gdb/dwarf2/cooked-index.h b/gdb/dwarf2/cooked-index.h
index 9900dbb84a7..b51128d1f58 100644
--- a/gdb/dwarf2/cooked-index.h
+++ b/gdb/dwarf2/cooked-index.h
@@ -253,7 +253,10 @@ class parent_map
   void set_parent (CORE_ADDR start, CORE_ADDR end,
 		   const cooked_index_entry *parent_entry)
   {
-    m_parent_map.set_empty (start, end, (void *)parent_entry);
+    /* Calling set_empty with nullptr is currently not allowed.  Note that we
+       still check the desired effect.  */
+    if (parent_entry != nullptr)
+      m_parent_map.set_empty (start, end, (void *)parent_entry);
 
     /* Assert that set_parent has the desired effect.  This is not trivial due
        to how set_empty works.  If the range already has been set before, it
-- 
2.35.3

[PATCH 04/13] [gdb/symtab] Add parent_map::dump

[Tom de Vries]

When dumping m_die_range_map using:
...
    addrmap_dump (&m_die_range_map, gdb_stdlog, nullptr);
...
I get:
...
  0x00000000000000d4 0x355e600
  0x00000000000000de 0x355e630
  0x00000000000000f5 0x355e600
  0x0000000000000101 0x0
  0x00000000000001c2 0x355e7b0
  0x00000000000001cc 0x355e7e0
  0x00000000000001e3 0x355e7b0
  0x00000000000001ef 0x0
...
which doesn't make it clear which cooked_die_entries the values refer to.

Add a function parent_map::dump that passes an annotation function
to addrmap_dump such that we have instead:
...
  0x00000000000000d4 0x360d300 (0xd3)
  0x00000000000000de 0x360d330 (0xdd)
  0x00000000000000f5 0x360d300 (0xd3)
  0x0000000000000101 0x0
  0x00000000000001c2 0x360d4b0 (0x1c1)
  0x00000000000001cc 0x360d4e0 (0x1cb)
  0x00000000000001e3 0x360d4b0 (0x1c1)
  0x00000000000001ef 0x0
...

Tested on x86_64-linux.
---
 gdb/addrmap.c             | 19 +++++++++++++------
 gdb/addrmap.h             |  7 +++++--
 gdb/dwarf2/cooked-index.h | 18 ++++++++++++++++++
 3 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/gdb/addrmap.c b/gdb/addrmap.c
index d16775d49d4..4c58427903b 100644
--- a/gdb/addrmap.c
+++ b/gdb/addrmap.c
@@ -361,7 +361,9 @@ addrmap_mutable::~addrmap_mutable ()
 /* See addrmap.h.  */
 
 void
-addrmap_dump (struct addrmap *map, struct ui_file *outfile, void *payload)
+addrmap_dump (struct addrmap *map, struct ui_file *outfile, void *payload,
+	      void (*annotate_value)(struct ui_file *outfile,
+				     const void *value))
 {
   /* True if the previously printed addrmap entry was for PAYLOAD.
      If so, we want to print the next one as well (since the next
@@ -380,11 +382,16 @@ addrmap_dump (struct addrmap *map, struct ui_file *outfile, void *payload)
       addr_str = "<ends here>";
 
     if (matches || previous_matched)
-      gdb_printf (outfile, "  %s%s %s\n",
-		  payload != nullptr ? "  " : "",
-		  core_addr_to_string (start_addr),
-		  addr_str);
-
+      {
+	gdb_printf (outfile, "  %s%s %s",
+		    payload != nullptr ? "  " : "",
+		    core_addr_to_string (start_addr),
+		    addr_str);
+	/* Annotate value.  */
+	if (annotate_value != nullptr)
+	  annotate_value (outfile, obj);
+	gdb_printf (outfile, "\n");
+      }
     previous_matched = matches;
 
     return 0;
diff --git a/gdb/addrmap.h b/gdb/addrmap.h
index e00dda6e711..f67cd32df1d 100644
--- a/gdb/addrmap.h
+++ b/gdb/addrmap.h
@@ -205,8 +205,11 @@ struct addrmap_mutable : public addrmap
 
 /* Dump the addrmap to OUTFILE.  If PAYLOAD is non-NULL, only dump any
    components that map to PAYLOAD.  (If PAYLOAD is NULL, the entire
-   map is dumped.)  */
+   map is dumped.)  If ANNOTATE_VALUE is non-nullptr, call it for each
+   value.  */
 void addrmap_dump (struct addrmap *map, struct ui_file *outfile,
-		   void *payload);
+		   void *payload,
+		   void (*annotate_value)(struct ui_file *outfile,
+					  const void *value) = nullptr);
 
 #endif /* ADDRMAP_H */
diff --git a/gdb/dwarf2/cooked-index.h b/gdb/dwarf2/cooked-index.h
index b51128d1f58..396c25b0718 100644
--- a/gdb/dwarf2/cooked-index.h
+++ b/gdb/dwarf2/cooked-index.h
@@ -266,6 +266,24 @@ class parent_map
       gdb_assert (m_parent_map.find (end) == parent_entry);
   }
 
+  /* Dump the parent map.  */
+  void dump ()
+  {
+    auto annotate_cooked_index_entry
+      = [] (struct ui_file *outfile, const void *value)
+      {
+	const cooked_index_entry *parent_entry
+	  = (const cooked_index_entry *)value;
+	if (parent_entry == nullptr)
+	  return;
+	gdb_printf (outfile, " (0x%" PRIx64 ")",
+		    to_underlying (parent_entry->die_offset));
+      };
+
+    addrmap_dump (&m_parent_map, gdb_stdlog, nullptr,
+		  annotate_cooked_index_entry);
+  }
+
 private:
   /* An addrmap that maps from section offsets to cooked_index_entry *.  */
   addrmap_mutable m_parent_map;
-- 
2.35.3

[PATCH 05/13] [gdb/symtab] Factor out m_deferred_entries usage

[Tom de Vries]

Factor out usage of cooked_indexer::m_deferred_entries in new member
functions defer_entry, handle_deferred_entries and resolve_deferred_entry.

Tested on x86_64-linux.
---
 gdb/dwarf2/read.c | 35 ++++++++++++++++++++++++++++-------
 1 file changed, 28 insertions(+), 7 deletions(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 17bc650a055..54097dd4d21 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -4830,6 +4830,32 @@ class cooked_indexer
      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)
+  {
+    m_deferred_entries.push_back (de);
+  }
+
+  /* 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)
+  {
+    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 ()
+  {
+    for (const auto &entry : m_deferred_entries)
+      {
+	const cooked_index_entry *parent_entry
+	  = find_parent (entry.spec_offset);
+	resolve_deferred_entry (entry, parent_entry);
+      }
+  }
 };
 
 /* Subroutine of dwarf2_build_psymtabs_hard to simplify it.
@@ -16557,7 +16583,7 @@ cooked_indexer::index_dies (cutu_reader *reader,
       if (name != nullptr)
 	{
 	  if (defer != 0)
-	    m_deferred_entries.push_back ({
+	    defer_entry ({
 		this_die, name, defer, abbrev->tag, flags
 	      });
 	  else
@@ -16662,12 +16688,7 @@ cooked_indexer::make_index (cutu_reader *reader)
     return;
   index_dies (reader, reader->info_ptr, nullptr, false);
 
-  for (const auto &entry : m_deferred_entries)
-    {
-      const cooked_index_entry *parent = find_parent (entry.spec_offset);
-      m_index_storage->add (entry.die_offset, entry.tag, entry.flags,
-			    entry.name, parent, m_per_cu);
-    }
+  handle_deferred_entries ();
 }
 
 /* An implementation of quick_symbol_functions for the cooked DWARF
-- 
2.35.3

[PATCH 06/13] [gdb/symtab] Add debug_handle_deferred_entries

[Tom de Vries]

Add a const bool debug_handle_deferred_entries, defaulting to false, that
allows us to easily debug the handling of deferred entries.

Example output:
...
(gdb) file forward-spec^M
Reading symbols from forward-spec...^M
  0x00000000000000dd 0x7f0044002ed0 (0xdc)^M
  0x00000000000000e8 0x0^M
Resolve deferred: 0xca -> 0xe0: 0xdc^M
...

This tells us that to find the parent of DIE 0xca, we lookup the parent of DIE
0xe0 in the parent map, which is 0xdc.

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

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 54097dd4d21..23de6714268 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -4837,11 +4837,26 @@ class cooked_indexer
     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);
   }
@@ -4849,6 +4864,9 @@ class cooked_indexer
   /* 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
-- 
2.35.3

[PATCH 07/13] [gdb/symtab] Resolve deferred entries, inter-shard case

[Tom de Vries]

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(-)

diff --git a/gdb/dwarf2/cooked-index.c b/gdb/dwarf2/cooked-index.c
index 58ea541a5c9..68b5644cf23 100644
--- a/gdb/dwarf2/cooked-index.c
+++ b/gdb/dwarf2/cooked-index.c
@@ -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
diff --git a/gdb/dwarf2/cooked-index.h b/gdb/dwarf2/cooked-index.h
index 396c25b0718..3a582d187c2 100644
--- a/gdb/dwarf2/cooked-index.h
+++ b/gdb/dwarf2/cooked-index.h
@@ -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 &);
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 23de6714268..87955d1d2e1 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -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
-- 
2.35.3

[PATCH 08/13] [gdb/testsuite] Add gdb.dwarf2/forward-spec-inter-cu.exp

[Tom de Vries]

Add a regression test for PR symtab/30846.

Tested on x86_64-linux.
---
 .../gdb.dwarf2/forward-spec-inter-cu.exp      | 107 ++++++++++++++++++
 1 file changed, 107 insertions(+)
 create mode 100644 gdb/testsuite/gdb.dwarf2/forward-spec-inter-cu.exp

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..bb3b6b4b92f
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/forward-spec-inter-cu.exp
@@ -0,0 +1,107 @@
+# 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 {[prepare_for_testing "failed to prepare" ${testfile} \
+	 [list $srcfile $asm_file] {nodebug}]} {
+    return -1
+}
+
+set in_v 0
+gdb_test_multiple "maint print objfiles" "v has a parent" {
+    -re "^ *\\\[\[0-9\]\\\] *\\(\\(cooked_index_entry\[^\r\n\]*" {
+	set in_v 0
+	exp_continue
+    }
+    -re "^ *name: *v\[\r\n\]*" {
+	set in_v 1
+	exp_continue
+    }
+    -re "^ *parent: *\\(\\(cooked_index_entry \\*\\) (0|$hex)\\)" {
+	if {$in_v} {
+	    if {$expect_out(1,string) == "0"} {
+		fail $gdb_test_name
+	    } else {
+		pass $gdb_test_name
+	    }
+	    set in_v 0
+	}
+	exp_continue
+    }
+    -re "^\[^\r\n\]*\[\r\n\]+" {
+	exp_continue
+    }
+    -re "$gdb_prompt " {
+	# Done.
+    }
+}
-- 
2.35.3

[PATCH 09/13] [gdb/symtab] Keep track of processed DIEs in shard

[Tom de Vries]

For optimizations in two following patches, we keep track in each shard which
DIEs have been processed.

Tested on x86_64-linux.
---
 gdb/dwarf2/cooked-index.c |  2 ++
 gdb/dwarf2/cooked-index.h | 15 +++++++++++++++
 gdb/dwarf2/read.c         | 32 ++++++++++++++++++++++++++++++++
 3 files changed, 49 insertions(+)

diff --git a/gdb/dwarf2/cooked-index.c b/gdb/dwarf2/cooked-index.c
index 68b5644cf23..478c66a24c4 100644
--- a/gdb/dwarf2/cooked-index.c
+++ b/gdb/dwarf2/cooked-index.c
@@ -730,6 +730,8 @@ cooked_index::handle_deferred_entries ()
       shard->m_die_range_map = nullptr;
       delete shard->m_deferred_entries;
       shard->m_deferred_entries = nullptr;
+      delete shard->m_die_range_map_valid;
+      shard->m_die_range_map_valid = nullptr;
     }
 }
 
diff --git a/gdb/dwarf2/cooked-index.h b/gdb/dwarf2/cooked-index.h
index 3a582d187c2..064944aa09a 100644
--- a/gdb/dwarf2/cooked-index.h
+++ b/gdb/dwarf2/cooked-index.h
@@ -315,6 +315,7 @@ class cooked_index_shard
   cooked_index_shard () {
     m_die_range_map = new parent_map;
     m_deferred_entries = new std::vector<deferred_entry>;
+    m_die_range_map_valid = new addrmap_mutable;
   }
 
   DISABLE_COPY_AND_ASSIGN (cooked_index_shard);
@@ -395,6 +396,18 @@ class cooked_index_shard
   const cooked_index_entry *resolve_deferred_entry
     (const deferred_entry &entry, const cooked_index_entry *parent_entry);
 
+  /* Mark parents in range [START, END] as valid .  */
+  void set_parent_valid (CORE_ADDR start, CORE_ADDR end)
+  {
+    m_die_range_map_valid->set_empty (start, end, (void *) 1);
+  }
+
+  /* Return true if find_parents can be relied upon.  */
+  bool parent_valid (CORE_ADDR addr)
+  {
+    return m_die_range_map_valid->find (addr) != nullptr;
+  }
+
 private:
 
   /* Return the entry that is believed to represent the program's
@@ -458,6 +471,8 @@ class cooked_index_shard
      understand this.  */
   parent_map *m_die_range_map;
 
+  addrmap *m_die_range_map_valid;
+
   /* 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
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 87955d1d2e1..b5d9103e4b2 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -4689,6 +4689,12 @@ class cooked_index_storage
     m_index->defer_entry (de);
   }
 
+  /* Mark parents in range [START, END] as valid .  */
+  void set_parent_valid (CORE_ADDR start, CORE_ADDR end)
+  {
+    m_index->set_parent_valid (start, end);
+  }
+
 private:
 
   /* Hash function for a cutu_reader.  */
@@ -4838,6 +4844,11 @@ class cooked_indexer
   {
     m_index_storage->defer_entry (de);
   }
+
+  void set_parent_valid (CORE_ADDR start, CORE_ADDR end)
+  {
+    m_index_storage->set_parent_valid (start, end);
+  }
 };
 
 /* Subroutine of dwarf2_build_psymtabs_hard to simplify it.
@@ -16526,9 +16537,22 @@ cooked_indexer::index_dies (cutu_reader *reader,
 			     + to_underlying (reader->cu->header.sect_off)
 			     + reader->cu->header.get_length_with_initial ());
 
+  const CORE_ADDR start_cu
+    = form_addr (sect_offset (info_ptr - reader->buffer),
+		 reader->cu->per_cu->is_dwz,
+		 reader->cu->per_cu->is_debug_types);
+
   while (info_ptr < end_ptr)
     {
       sect_offset this_die = (sect_offset) (info_ptr - reader->buffer);
+
+      {
+	CORE_ADDR end_prev_die
+	  = form_addr (this_die - 1, reader->cu->per_cu->is_dwz,
+		       reader->cu->per_cu->is_debug_types);
+	set_parent_valid (start_cu, end_prev_die);
+      }
+
       unsigned int bytes_read;
       const abbrev_info *abbrev = peek_die_abbrev (*reader, info_ptr,
 						   &bytes_read);
@@ -16674,6 +16698,14 @@ cooked_indexer::index_dies (cutu_reader *reader,
 	}
     }
 
+  {
+    CORE_ADDR end_prev_die
+      = form_addr (sect_offset (info_ptr - reader->buffer - 1),
+		   reader->cu->per_cu->is_dwz,
+		   reader->cu->per_cu->is_debug_types);
+    set_parent_valid (start_cu, end_prev_die);
+  }
+
   return info_ptr;
 }
 
-- 
2.35.3

[PATCH 10/13] [gdb/symtab] Resolve deferred entries, intra-shard case

[Tom de Vries]

In a previous patch we've solved the generic case of handling deferred
entries.

Now add an optimization that handles deferred entries with an intra-shard
dependency in the parallel for.

By using "maint set worker-threads 0" we force a single shard, and the
resulting debug_handle_deferred_entries is:
...
$ gdb -q -batch -iex "maint set worker-threads 0" forward-spec
handle_deferred_entries, intra-shard case
parent map
  0x00000000000000ca 0x2934500 (deferred)
  0x00000000000000cb 0x0
  0x00000000000000dd 0x37d79d0 (0xdc)
  0x00000000000000e8 0x0
parent valid map
  0x000000000000002e 0x1
  0x0000000000000045 0x0
  0x0000000000000062 0x1
  0x0000000000000084 0x0
  0x00000000000000ca 0x1
  0x00000000000000e9 0x0
  0x0000000000000121 0x1
  0x00000000000002ac 0x0
Resolve deferred: 0xca -> 0xe0: 0xdc
handle_deferred_entries, inter-shard case
  0x00000000000000ca 0x2934500 (deferred)
  0x00000000000000cb 0x0
  0x00000000000000dd 0x37d79d0 (0xdc)
  0x00000000000000e8 0x0
...

Tested on x86_64-linux.
---
 gdb/dwarf2/cooked-index.c | 59 +++++++++++++++++++++++++++++++++++++--
 gdb/dwarf2/cooked-index.h |  7 +++++
 gdb/dwarf2/read.c         | 10 +++++++
 3 files changed, 74 insertions(+), 2 deletions(-)

diff --git a/gdb/dwarf2/cooked-index.c b/gdb/dwarf2/cooked-index.c
index 478c66a24c4..f09ab594d8f 100644
--- a/gdb/dwarf2/cooked-index.c
+++ b/gdb/dwarf2/cooked-index.c
@@ -452,6 +452,7 @@ cooked_index_shard::wait (bool allow_quit) const
 cooked_index::cooked_index (vec_type &&vec)
   : m_vector (std::move (vec))
 {
+  /* Handle deferred entries, inter-cu case.  */
   handle_deferred_entries ();
 
   for (auto &idx : m_vector)
@@ -662,6 +663,57 @@ const static bool debug_handle_deferred_entries = false;
 
 /* See cooked-index.h.  */
 
+const cooked_index_entry *
+cooked_index_shard::find_parent_deferred_entry
+  (const cooked_index_shard::deferred_entry &entry) const
+{
+  const cooked_index_entry *parent_entry = nullptr;
+
+  auto res = 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;
+    }
+
+  return parent_entry;
+}
+
+/* See cooked-index.h.  */
+
+void
+cooked_index_shard::handle_deferred_entries ()
+{
+
+  if (debug_handle_deferred_entries)
+    {
+      gdb_printf ("handle_deferred_entries, intra-shard case\n");
+      gdb_printf ("parent map\n");
+      m_die_range_map->dump ();
+      gdb_printf ("parent valid map\n");
+      addrmap_dump (m_die_range_map_valid, gdb_stdlog, nullptr, nullptr);
+    }
+
+  for (auto it = m_deferred_entries->begin (); it != m_deferred_entries->end (); )
+    {
+      const deferred_entry & deferred_entry = *it;
+      if (!parent_valid (deferred_entry.spec_offset))
+	{
+	  it++;
+	  continue;
+	}
+      const cooked_index_entry *parent_entry
+	= find_parent_deferred_entry (deferred_entry);
+      resolve_deferred_entry (deferred_entry, parent_entry);
+      it = m_deferred_entries->erase (it);
+    }
+}
+
+/* See cooked-index.h.  */
+
 const cooked_index_entry *
 cooked_index_shard::resolve_deferred_entry
   (const deferred_entry &de, const cooked_index_entry *parent_entry)
@@ -713,8 +765,11 @@ void
 cooked_index::handle_deferred_entries ()
 {
   if (debug_handle_deferred_entries)
-    for (auto &shard : m_vector)
-      shard->m_die_range_map->dump ();
+    {
+      gdb_printf ("handle_deferred_entries, inter-shard case\n");
+      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)
diff --git a/gdb/dwarf2/cooked-index.h b/gdb/dwarf2/cooked-index.h
index 064944aa09a..b36d5247653 100644
--- a/gdb/dwarf2/cooked-index.h
+++ b/gdb/dwarf2/cooked-index.h
@@ -396,6 +396,13 @@ class cooked_index_shard
   const cooked_index_entry *resolve_deferred_entry
     (const deferred_entry &entry, const cooked_index_entry *parent_entry);
 
+  /* Find the parent entry for 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 ();
+
   /* Mark parents in range [START, END] as valid .  */
   void set_parent_valid (CORE_ADDR start, CORE_ADDR end)
   {
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index b5d9103e4b2..fca83aed5f7 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -4689,6 +4689,12 @@ class cooked_index_storage
     m_index->defer_entry (de);
   }
 
+  /* Handle deferred entries, intra-cu case.  */
+  void handle_deferred_entries ()
+  {
+    m_index->handle_deferred_entries ();
+  }
+
   /* Mark parents in range [START, END] as valid .  */
   void set_parent_valid (CORE_ADDR start, CORE_ADDR end)
   {
@@ -5181,6 +5187,10 @@ dwarf2_build_psymtabs_hard (dwarf2_per_objfile *per_objfile)
 		errors.push_back (std::move (except));
 	      }
 	  }
+
+	/* Handle deferred entries, intra-cu case.  */
+	thread_storage.handle_deferred_entries ();
+
 	return result_type (thread_storage.release (), std::move (errors));
       }, task_size);
 
-- 
2.35.3

[PATCH 11/13] [gdb/symtab] Don't defer backward refs, inter-cu intra-shard case

[Tom de Vries]

Consider the test-case gdb.dwarf2/backward-spec-inter-cu.exp with
debug_handle_deferred_entries:
...
$ gdb -q -batch -iex "maint set worker-threads 0" backward-spec-inter-cu:
handle_deferred_entries, intra-shard case
parent map
  0x00000000000000c8 0x3254a80 (0xc7)
  0x00000000000000d3 0x0
  0x00000000000000e1 0x2935500 (deferred)
  0x00000000000000e2 0x0
parent valid map
  0x000000000000002e 0x1
  0x0000000000000045 0x0
  0x0000000000000062 0x1
  0x0000000000000084 0x0
  0x00000000000000be 0x1
  0x00000000000000d4 0x0
  0x00000000000000e1 0x1
  0x00000000000000eb 0x0
  0x0000000000000123 0x1
  0x00000000000002ae 0x0
Resolve deferred: 0xe1 -> 0xcb: 0xc7
handle_deferred_entries, inter-shard case
  0x00000000000000c8 0x3254a80 (0xc7)
  0x00000000000000d3 0x0
  0x00000000000000e1 0x2935500 (deferred)
  0x00000000000000e2 0x0
...

It has a backward inter-cu, intra-shard dependency, which is first deferred
and then resolved in the intra-shard case.

However, we can handle this more optimally: we can not defer and resolve
immediately, because by the time that we process 0xe1, 0xcb is already
processed by the same shard, and consequently present in the parent map.

Fix this by not deferring dependencies that are present in the shard's parent
map, such that we have instead:
...
handle_deferred_entries, intra-shard case
parent map
  0x00000000000000c8 0x3d36cb0 (0xc7)
  0x00000000000000d3 0x0
parent valid map
  0x000000000000002e 0x1
  0x0000000000000045 0x0
  0x0000000000000062 0x1
  0x0000000000000084 0x0
  0x00000000000000be 0x1
  0x00000000000000d4 0x0
  0x00000000000000e1 0x1
  0x00000000000000eb 0x0
  0x0000000000000123 0x1
  0x00000000000002ae 0x0
handle_deferred_entries, inter-shard case
  0x00000000000000c8 0x3d36cb0 (0xc7)
  0x00000000000000d3 0x0
...

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

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index fca83aed5f7..6f218f172a9 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -4701,6 +4701,12 @@ class cooked_index_storage
     m_index->set_parent_valid (start, end);
   }
 
+  /* Return true if find_parents can be relied upon.  */
+  bool parent_valid (CORE_ADDR addr)
+  {
+    return m_index->parent_valid (addr);
+  }
+
 private:
 
   /* Hash function for a cutu_reader.  */
@@ -4855,6 +4861,12 @@ class cooked_indexer
   {
     m_index_storage->set_parent_valid (start, end);
   }
+
+  /* Return true if find_parents can be relied upon.  */
+  bool parent_valid (CORE_ADDR addr)
+  {
+    return m_index_storage->parent_valid (addr);
+  }
 };
 
 /* Subroutine of dwarf2_build_psymtabs_hard to simplify it.
@@ -16391,7 +16403,11 @@ 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
+	  CORE_ADDR origin_addr = form_addr (origin_offset, origin_is_dwz,
+					     reader->cu->per_cu->is_debug_types);
+
+	  if (((new_reader->cu != reader->cu
+		&& !parent_valid (origin_addr))
 	       || (new_reader->cu == reader->cu
 		   && new_info_ptr > watermark_ptr))
 	      && *parent_entry == nullptr)
-- 
2.35.3

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

[Tom de Vries]

Add test-case gdb.dwarf2/backward-spec-inter-cu.exp, which excercises the code
added in the previous patch.

Tested on x86_64-linux.
---
 .../gdb.dwarf2/backward-spec-inter-cu.exp     | 111 ++++++++++++++++++
 1 file changed, 111 insertions(+)
 create mode 100644 gdb/testsuite/gdb.dwarf2/backward-spec-inter-cu.exp

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..acb6e2ce8b2
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/backward-spec-inter-cu.exp
@@ -0,0 +1,111 @@
+# 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}
+	} {
+	    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 prepare" ${testfile} \
+	 [list $srcfile $asm_file] {nodebug}]} {
+    return -1
+}
+
+clean_restart
+gdb_test_no_output "maint set worker-threads 0"
+gdb_load $binfile
+
+set in_v 0
+gdb_test_multiple "maint print objfiles" "v has a parent" {
+    -re "^ *\\\[\[0-9\]\\\] *\\(\\(cooked_index_entry\[^\r\n\]*" {
+	set in_v 0
+	exp_continue
+    }
+    -re "^ *name: *v\[\r\n\]*" {
+	set in_v 1
+	exp_continue
+    }
+    -re "^ *parent: *\\(\\(cooked_index_entry \\*\\) (0|$hex)\\)" {
+	if {$in_v} {
+	    if {$expect_out(1,string) == "0"} {
+		fail $gdb_test_name
+	    } else {
+		pass $gdb_test_name
+	    }
+	    set in_v 0
+	}
+	exp_continue
+    }
+    -re "^\[^\r\n\]*\[\r\n\]+" {
+	exp_continue
+    }
+    -re "$gdb_prompt " {
+	# Done.
+    }
+}
-- 
2.35.3

[PATCH 13/13] [gdb/symtab] Add DW_TAG_inlined_subroutine entries in the cooked index for c++

[Tom de Vries]

When running test-case gdb.cp/breakpoint-locs.exp with target board
cc-with-dwz, we occasionally run into:
...
FAIL: gdb.cp/breakpoint-locs.exp: break N1::C1::baz
...
due to the breakpoint having one instead of two locations.

By using "maint set worker-threads 0" we can make the test-case fail reliably.

In the failing case the inlined_subroutine DIE at 0x1cf has an incorrect
parent, and the one at 0x148 has the correct parent:
...
    [7] ((cooked_index_entry *) 0x7fdf800030b0)
    name:       baz
    canonical:  baz
    qualified:  N1::C1::baz
    DWARF tag:  DW_TAG_inlined_subroutine
    flags:      0x0 []
    DIE offset: 0x148
    parent:     ((cooked_index_entry *) 0x7fdf80002ed0) [C1]
  ...
    [20] ((cooked_index_entry *) 0x7fdf84005340)
    name:       baz
    canonical:  baz
    qualified:  baz
    DWARF tag:  DW_TAG_inlined_subroutine
    flags:      0x0 []
    DIE offset: 0x1cf
    parent:     ((cooked_index_entry *) 0)
...
and in the passing case it's the other way around.

The corresponding dwarf for 0x148 is this:
...
 <0><b>: Abbrev Number: 34 (DW_TAG_partial_unit)
 <1><14>: Abbrev Number: 37 (DW_TAG_namespace)
    <15>   DW_AT_name        : N1
 <2><19>: Abbrev Number: 36 (DW_TAG_class_type)
    <1a>   DW_AT_name        : C1
 <3><20>: Abbrev Number: 32 (DW_TAG_subprogram)
    <21>   DW_AT_external    : 1
    <21>   DW_AT_name        : baz
    <27>   DW_AT_linkage_name: _ZN2N12C13bazEv
    <2b>   DW_AT_accessibility: 1       (public)
    <2c>   DW_AT_declaration : 1
 <1><2e>: Abbrev Number: 35 (DW_TAG_subprogram)
    <2f>   DW_AT_specification: <0x20>
    <30>   DW_AT_inline      : 3        (declared as inline and inlined)
 <0><ee>: Abbrev Number: 16 (DW_TAG_compile_unit)
    <f3>   DW_AT_language    : 4        (C++)
    <f4>   DW_AT_name        : gdb.cp/breakpoint-locs.cc
 <1><13b>: Abbrev Number: 25 (DW_TAG_subprogram)
    <13c>   DW_AT_specification: <0x115>
    <13d>   DW_AT_low_pc      : 0x4004d7
    <145>   DW_AT_high_pc     : 16
    <146>   DW_AT_frame_base  : 1 byte block: 9c        (DW_OP_call_frame_cfa)
    <148>   DW_AT_GNU_all_call_sites: 1
 <2><148>: Abbrev Number: 24 (DW_TAG_inlined_subroutine)
    <149>   DW_AT_abstract_origin: <0x2e>
    <14d>   DW_AT_low_pc      : 0x4004db
    <155>   DW_AT_high_pc     : 9
    <156>   DW_AT_call_file   : 1
    <157>   DW_AT_call_line   : 22
...
and for 0x1cf it's similar but in a CU with name gdb.cp/breakpoint-locs-2.cc.

In the target board unix case, the test-case passes because both CUs are
expanded, because each CU contains the equivalent of 0x2e.

But in the dwz case, dwz factored out that DIE into the PU, so that mechanism
no longer works.

Note that when we used partial symtabs for dwarf we relied on the
DW_TAG_inlined_subroutine DIEs, see commit f9b5d5ea18a ("[gdb/symtab] Fix
missing breakpoint location for inlined function").  So lets fix this in the
same way, by adding DW_TAG_inlined_subroutine entries in the cooked index.

Currently the cooked index doesn't add an entry for it because in
cooked_indexer::index_dies we don't recurse into the children of a
DW_TAG_subprogram for c++:
...
	    case DW_TAG_subprogram:
	      if ((m_language == language_fortran
		   || m_language == language_ada)
		  && this_entry != nullptr)
		{
		  info_ptr = recurse (reader, info_ptr, this_entry, true);
...

Doing so yields entries with incorrect qualified names N1::foo::baz and
N1::bar::baz, due to an incorrect parent field.  Indeed in DIE terms the
parent is foo/bar, but we use the parent field to track encapsulating scopes,
so fix this by ignoring the actual DIE parent.

That gives us the qualified name baz in both cases.  Adding some debugging in
handle_deferred_dies shows us why:
...
  0x0000000000000015 0x7f9400002eb0 (0x14)^M
  0x000000000000001a 0x7f9400002ee0 (0x19)^M
  0x000000000000002d 0x7f9400002eb0 (0x14)^M
  0x000000000000002e 0x0^M
  0x000000000000002f 0x7f9400002f10 (0x2e)^M
  0x0000000000000039 0x0^M
  0x000000000000010f 0x7f9400002fd0 (0x10e)^M
  0x0000000000000121 0x0^M
  0x000000000000013c 0x7f9400003060 (0x13b)^M
  0x0000000000000148 0x2934520 (deferred)^M
  0x0000000000000149 0x7f9400003060 (0x13b)^M
  0x000000000000016d 0x0^M
  0x000000000000019a 0x7f94080052e0 (0x199)^M
  0x00000000000001ac 0x0^M
  0x00000000000001c3 0x7f9408005370 (0x1c2)^M
  0x00000000000001cf 0x2934520 (deferred)^M
  0x00000000000001d0 0x7f9408005370 (0x1c2)^M
  0x00000000000001f4 0x0^M
Resolve deferred: 0x148 -> 0x2e: no parent^M
Resolve deferred: 0x1cf -> 0x2e: no parent^M
...

We try to find the find the parent of 0x148, by looking up the parent of 0x2e
in m_die_range_map, which is nullptr, in other words no parent.

Fix this by recording more parent relations in m_die_range_map, such that we
have:
...
  0x0000000000000015 0x7fc980002eb0 (0x14)^M
  0x000000000000001a 0x7fc980002ee0 (0x19)^M
  0x000000000000002d 0x7fc980002eb0 (0x14)^M
  0x000000000000002e 0x7fc980002ee0 (0x19)^M
  0x000000000000002f 0x7fc980002f10 (0x2e)^M
  0x0000000000000039 0x0^M
  0x000000000000010f 0x7fc980002fd0 (0x10e)^M
  0x0000000000000121 0x0^M
  0x000000000000013b 0x7fc980002fd0 (0x10e)^M
  0x000000000000013c 0x7fc980003060 (0x13b)^M
  0x0000000000000148 0x2934520 (deferred)^M
  0x0000000000000149 0x7fc980003060 (0x13b)^M
  0x000000000000016d 0x0^M
  0x000000000000019a 0x7fc978002eb0 (0x199)^M
  0x00000000000001ac 0x0^M
  0x00000000000001c2 0x7fc978002eb0 (0x199)^M
  0x00000000000001c3 0x7fc978002f40 (0x1c2)^M
  0x00000000000001cf 0x2934520 (deferred)^M
  0x00000000000001d0 0x7fc978002f40 (0x1c2)^M
  0x00000000000001f4 0x0^M
Resolve deferred: 0x148 -> 0x2e: 0x19^M
Resolve deferred: 0x1cf -> 0x2e: 0x19^M
...

This gives us the desired outcome of N1::C1::baz for both DIEs, and makes the
test-case pass.

Tested on x86_64-linux.
---
 gdb/dwarf2/read.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 6f218f172a9..a770282c6ae 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -16606,6 +16606,8 @@ cooked_indexer::index_dies (cutu_reader *reader,
       cooked_index_flag flags = IS_STATIC;
       sect_offset sibling {};
       const cooked_index_entry *this_parent_entry = parent_entry;
+      if (abbrev->tag == DW_TAG_inlined_subroutine)
+	this_parent_entry = nullptr;
       info_ptr = scan_attributes (reader->cu->per_cu, reader, info_ptr,
 				  info_ptr, abbrev, &name, &linkage_name,
 				  &flags, &sibling, &this_parent_entry,
@@ -16635,9 +16637,14 @@ cooked_indexer::index_dies (cutu_reader *reader,
 		});
 	    }
 	  else
-	    this_entry = m_index_storage->add (this_die, abbrev->tag, flags,
-					       name, this_parent_entry,
-					       m_per_cu);
+	    {
+	      CORE_ADDR addr = form_addr (this_die, reader->cu->per_cu->is_dwz,
+					  reader->cu->per_cu->is_debug_types);
+	      set_parent (addr, addr, this_parent_entry);
+	      this_entry = m_index_storage->add (this_die, abbrev->tag, flags,
+						 name, this_parent_entry,
+						 m_per_cu);
+	    }
 	}
 
       if (linkage_name != nullptr)
@@ -16698,7 +16705,8 @@ cooked_indexer::index_dies (cutu_reader *reader,
 
 	    case DW_TAG_subprogram:
 	      if ((m_language == language_fortran
-		   || m_language == language_ada)
+		   || m_language == language_ada
+		   || m_language == language_cplus)
 		  && this_entry != nullptr)
 		{
 		  info_ptr = recurse (reader, info_ptr, this_entry, true);
-- 
2.35.3

Re: [PATCH 01/13] [gdb/symtab] Factor out m_die_range_map usage

[Alexandra Petlanova Hajkova]

[-- Attachment #1: Type: text/plain, Size: 1216 bytes --]

On Mon, Oct 2, 2023 at 2:53 PM Tom de Vries via Gdb-patches <
gdb-patches@sourceware.org> wrote:

> Factor out usage of cooked_indexer::m_die_range_map into new class
> parent_map
> with member functions find_parent and set_parent.
>
> Tested on x86_64-linux.
> ---
>  gdb/dwarf2/cooked-index.h | 22 ++++++++++++++++++++++
>  gdb/dwarf2/read.c         | 23 +++++++++++++++++------
>  2 files changed, 39 insertions(+), 6 deletions(-)
>
>
> It seems breakpoint-locs.exp test does not fail anymore with this patch
applied on ppc64le Fedora Rawhide. But I wouldn't expect that
from just a factoring out. Is that a coincidence?
< Breakpoint 1 at 0x10010208: file
/root/build/gdb/testsuite/../../../binutils-g
db/gdb/testsuite/gdb.cp/breakpoint-locs.h, line 23.
< (gdb) FAIL: gdb.cp/breakpoint-locs.exp: break N1::C1::baz
< testcase
/root/build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.cp/breakpoint-locs.exp
completed in 0 seconds
---
> Breakpoint 1 at 0x10010208: N1::C1::baz. (2 locations)
> (gdb) PASS: gdb.cp/breakpoint-locs.exp: break N1::C1::baz
> testcase
/root/build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.cp/breakpoint-locs.exp
completed in 1 seconds

Re: [PATCH 01/13] [gdb/symtab] Factor out m_die_range_map usage

[Tom de Vries]

On 10/9/23 22:33, Alexandra Petlanova Hajkova wrote:
> On Mon, Oct 2, 2023 at 2:53 PM Tom de Vries via Gdb-patches 
> <gdb-patches@sourceware.org <mailto:gdb-patches@sourceware.org>> wrote:
> 
>     Factor out usage of cooked_indexer::m_die_range_map into new class
>     parent_map
>     with member functions find_parent and set_parent.
> 
>     Tested on x86_64-linux.
>     ---
>       gdb/dwarf2/cooked-index.h | 22 ++++++++++++++++++++++
>       gdb/dwarf2/read.c         | 23 +++++++++++++++++------
>       2 files changed, 39 insertions(+), 6 deletions(-)
> 
> 
> It seems breakpoint-locs.exp test does not fail anymore with this patch 
> applied on ppc64le Fedora Rawhide. But I wouldn't expect that > from just a factoring out. Is that a coincidence?

Hi,

if you're talking about target board unix, then it's a failure I'm 
unaware of, and indeed I wouldn't expect this patch to change anything.

If you're talking about target board cc-with-dwz or cc-with-dwz-m, then 
you might be looking at nondeterministic behaviour of the cooked index, 
due to the race between 2 CUs to read a PU.  Using "maint set 
worker-threads 0" avoids that race.  For me on x86_64-linux that makes 
the failure reproducible.

Thanks,
- Tom


> < Breakpoint 1 at 0x10010208: file 
> /root/build/gdb/testsuite/../../../binutils-g
> db/gdb/testsuite/gdb.cp/breakpoint-locs.h, line 23.
> < (gdb) FAIL: gdb.cp/breakpoint-locs.exp: break N1::C1::baz
> < testcase 
> /root/build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.cp/breakpoint-locs.exp completed in 0 seconds
> ---
>  > Breakpoint 1 at 0x10010208: N1::C1::baz. (2 locations)
>  > (gdb) PASS: gdb.cp/breakpoint-locs.exp: break N1::C1::baz
>  > testcase 
> /root/build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.cp/breakpoint-locs.exp completed in 1 seconds

Re: [PATCH 04/13] [gdb/symtab] Add parent_map::dump

[Alexandra Petlanova Hajkova]

[-- Attachment #1: Type: text/plain, Size: 1164 bytes --]

On Mon, Oct 2, 2023 at 2:52 PM Tom de Vries via Gdb-patches <
gdb-patches@sourceware.org> wrote:

> When dumping m_die_range_map using:
> ...
>     addrmap_dump (&m_die_range_map, gdb_stdlog, nullptr);
> ...
> I get:
> ...
>   0x00000000000000d4 0x355e600
>   0x00000000000000de 0x355e630
>   0x00000000000000f5 0x355e600
>   0x0000000000000101 0x0
>   0x00000000000001c2 0x355e7b0
>   0x00000000000001cc 0x355e7e0
>   0x00000000000001e3 0x355e7b0
>   0x00000000000001ef 0x0
> ...
> which doesn't make it clear which cooked_die_entries the values refer to.
>
> Add a function parent_map::dump that passes an annotation function
> to addrmap_dump such that we have instead:
> ...
>   0x00000000000000d4 0x360d300 (0xd3)
>   0x00000000000000de 0x360d330 (0xdd)
>   0x00000000000000f5 0x360d300 (0xd3)
>   0x0000000000000101 0x0
>   0x00000000000001c2 0x360d4b0 (0x1c1)
>   0x00000000000001cc 0x360d4e0 (0x1cb)
>   0x00000000000001e3 0x360d4b0 (0x1c1)
>   0x00000000000001ef 0x0
> ...
>
> Tested on x86_64-linux.
> ---
>  I think is a very useful cahnge. I can confirm this causes no regrissions
> on ppc64le Fedora Rawhide.
>

[PING][PATCH 00/13] [gdb/symtab, cc-with-dwz] Fix gdb.cp/breakpoint-locs.exp

[Tom de Vries]

On 10/2/23 14:50, Tom de Vries via Gdb-patches wrote:
> [ Submitted an earlier RFC version at
> https://sourceware.org/pipermail/gdb-patches/2023-September/202443.html. ]
> 
> When running test-case gdb.cp/breakpoint-locs.exp with target board
> cc-with-dwz, we occasionally run into:
> ...
> FAIL: gdb.cp/breakpoint-locs.exp: break N1::C1::baz
> ...
> due to the breakpoint having one instead of two locations.
> 
> This is PR symtab/30728.  This series fixes it.
> 
> The fix is dependent on the fix for PR symtab/30846, which is about incorrect
> parent entries in the cooked index, due to inter-cu (intra-shard and
> inter-shard) references.
> 
> The series consists of three parts:
> - patches that fix PR symtab/30846.
> - patches that add two optimizations to the fix for PR symtab/30846
>    (one preparation patch, and two optimization patches),
> - the last patch, that fixes PR symtab/30728.
> 
> The fix for PR symtab/30728 consists of adding entries to the cooked index for
> DW_TAG_inlined_subroutine DIEs, with correct parent entries.
> 
> Tested on x86_64-linux, with target boards unix, cc-with-dwz and
> cc-with-dwz-m.
> 
> I've split out the two added test-cases into separate patches.  Unfortunately
> the patch "[gdb/symtab] Resolve deferred entries, inter-shard case" is still
> somewhat large due to moving code about.
> 

Ping.

Thanks,
- Tom

> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30728
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30846
> 
> Tom de Vries (13):
>    [gdb/symtab] Factor out m_die_range_map usage
>    [gdb/symtab] Check effect in parent_map::set_parent
>    [gdb/symtab] Handle nullptr parent in parent_map::set_parent
>    [gdb/symtab] Add parent_map::dump
>    [gdb/symtab] Factor out m_deferred_entries usage
>    [gdb/symtab] Add debug_handle_deferred_entries
>    [gdb/symtab] Resolve deferred entries, inter-shard case
>    [gdb/testsuite] Add gdb.dwarf2/forward-spec-inter-cu.exp
>    [gdb/symtab] Keep track of processed DIEs in shard
>    [gdb/symtab] Resolve deferred entries, intra-shard case
>    [gdb/symtab] Don't defer backward refs, inter-cu intra-shard case
>    [gdb/testsuite] Add gdb.dwarf2/backward-spec-inter-cu.exp
>    [gdb/symtab] Add DW_TAG_inlined_subroutine entries in the cooked index
>      for c++
> 
>   gdb/addrmap.c                                 |  19 +-
>   gdb/addrmap.h                                 |   7 +-
>   gdb/dwarf2/cooked-index.c                     | 141 ++++++++++++++
>   gdb/dwarf2/cooked-index.h                     | 145 +++++++++++++-
>   gdb/dwarf2/read.c                             | 178 +++++++++++++-----
>   .../gdb.dwarf2/backward-spec-inter-cu.exp     | 111 +++++++++++
>   .../gdb.dwarf2/forward-spec-inter-cu.exp      | 107 +++++++++++
>   7 files changed, 657 insertions(+), 51 deletions(-)
>   create mode 100644 gdb/testsuite/gdb.dwarf2/backward-spec-inter-cu.exp
>   create mode 100644 gdb/testsuite/gdb.dwarf2/forward-spec-inter-cu.exp
> 
> 
> base-commit: 6a6117ab0ffe18ea984abca84869eae799c1b346

Re: [PATCH 01/13] [gdb/symtab] Factor out m_die_range_map usage

[Tom Tromey]

>>>>> "Tom" == Tom de Vries via Gdb-patches <gdb-patches@sourceware.org> writes:

Tom> Factor out usage of cooked_indexer::m_die_range_map into new class parent_map
Tom> with member functions find_parent and set_parent.

Tom> +  /* Find the parent of DIE LOOKUP.  */
Tom> +  const cooked_index_entry *find_parent (CORE_ADDR lookup) const
Tom> +  {

I sort of regret doing this stuff using CORE_ADDR.  I think the API
would be better if 'form_addr' was handled privately in the wrapper
class.

Tom

Re: [PATCH 02/13] [gdb/symtab] Check effect in parent_map::set_parent

[Tom Tromey]

>>>>> "Tom" == Tom de Vries via Gdb-patches <gdb-patches@sourceware.org> writes:

Tom> Set_parent uses m_die_range_map.set_empty to set the parent of a range.
Tom> If part of the range is already set, it remains the same.

Tom> If the entire range is already set, the set_empty has no effect, silently.

Tom> Fix this by verifying that calling set_empty has the desired effect on the
Tom> start and end points of the range.

This seems like it might be better as a unit test.

Tom

Re: [PATCH 04/13] [gdb/symtab] Add parent_map::dump

[Tom Tromey]

>>>>> "Tom" == Tom de Vries via Gdb-patches <gdb-patches@sourceware.org> writes:

Tom>  void
Tom> -addrmap_dump (struct addrmap *map, struct ui_file *outfile, void *payload)
Tom> +addrmap_dump (struct addrmap *map, struct ui_file *outfile, void *payload,
Tom> +	      void (*annotate_value)(struct ui_file *outfile,
Tom> +				     const void *value))

I think we prefer function_view in new code.

Tom

Re: [PATCH 07/13] [gdb/symtab] Resolve deferred entries, inter-shard case

[Tom Tromey]

>>>>> "Tom" == Tom de Vries via Gdb-patches <gdb-patches@sourceware.org> writes:

Tom> We need to generically solve the case of inter-CU dependencies, and that
Tom> includes inter-shard dependencies as well.

I'm not sure we really do.  AFAIK compilers don't emit this, only dwz.
So maybe there's a way to make it so we only pay the cost for the dwz
case.

I may send more comments on this patch later.  I find it a bit hard to
review, mainly because this part of DWARF is my absolute pet peeve.
It's so needlessly bad.

Tom> @@ -446,6 +452,8 @@ cooked_index_shard::wait (bool allow_quit) const
Tom>  cooked_index::cooked_index (vec_type &&vec)
Tom>    : m_vector (std::move (vec))
Tom>  {
Tom> +  handle_deferred_entries ();
Tom> +

This is done in the main thread.  How much does it slow down startup?

Tom> +  for (auto &shard : m_vector)
Tom> +    {
Tom> +      delete shard->m_die_range_map;
Tom> +      shard->m_die_range_map = nullptr;
Tom> +      delete shard->m_deferred_entries;
Tom> +      shard->m_deferred_entries = nullptr;
Tom> +    }

I think we prefer unique_ptr rather than explicit deletes.

Tom

Re: [PATCH 09/13] [gdb/symtab] Keep track of processed DIEs in shard

[Alexandra Petlanova Hajkova]

[-- Attachment #1: Type: text/plain, Size: 387 bytes --]

>
> +  {
> +    CORE_ADDR end_prev_die
> +      = form_addr (sect_offset (info_ptr - reader->buffer - 1),
> +                  reader->cu->per_cu->is_dwz,
> +                  reader->cu->per_cu->is_debug_types);
> +    set_parent_valid (start_cu, end_prev_die);
> +  }
> +
>    return info_ptr;
>  }
>
> --
> 2.35.3
>

I can confirm this causes no regression on aarch64 Fedora Rawhide.

Re: [PATCH 01/13] [gdb/symtab] Factor out m_die_range_map usage

[Tom de Vries]

[-- Attachment #1: Type: text/plain, Size: 735 bytes --]

On 10/20/23 21:50, Tom Tromey wrote:
>>>>>> "Tom" == Tom de Vries via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> Tom> Factor out usage of cooked_indexer::m_die_range_map into new class parent_map
> Tom> with member functions find_parent and set_parent.
> 
> Tom> +  /* Find the parent of DIE LOOKUP.  */
> Tom> +  const cooked_index_entry *find_parent (CORE_ADDR lookup) const
> Tom> +  {
> 
> I sort of regret doing this stuff using CORE_ADDR.  I think the API
> would be better if 'form_addr' was handled privately in the wrapper
> class.

This updated patch moves form_addr into the wrapper class, but doesn't 
make it private because there's still one remaining reference from 
outside the class.

WDYT?

Thanks,
- Tom


[-- Attachment #2: 0001-gdb-symtab-Factor-out-m_die_range_map-usage.patch --]
[-- Type: text/x-patch, Size: 7329 bytes --]

From 946ead3f1f4d6a46c481a6dfe1578fa49afcf1a8 Mon Sep 17 00:00:00 2001
From: Tom de Vries <tdevries@suse.de>
Date: Tue, 22 Aug 2023 13:17:47 +0200
Subject: [PATCH] [gdb/symtab] Factor out m_die_range_map usage

Factor out usage of cooked_indexer::m_die_range_map into new class parent_map
with member functions find_parent and set_parent.

Tested on x86_64-linux.
---
 gdb/dwarf2/cooked-index.h | 51 +++++++++++++++++++++++++++++++++
 gdb/dwarf2/read.c         | 60 +++++++++++++++++++++------------------
 2 files changed, 83 insertions(+), 28 deletions(-)

diff --git a/gdb/dwarf2/cooked-index.h b/gdb/dwarf2/cooked-index.h
index 5675ea68bb8..47bc48ea459 100644
--- a/gdb/dwarf2/cooked-index.h
+++ b/gdb/dwarf2/cooked-index.h
@@ -239,6 +239,57 @@ struct cooked_index_entry : public allocate_on_obstack
 		    bool for_name) const;
 };
 
+class parent_map
+{
+public:
+
+  /* A DIE address as formed by form_addr, defining a tuple
+     <OFFSET, IS_DWZ>.  */
+  typedef CORE_ADDR die_addr;
+
+  /* Find the parent of DIE LOOKUP.  */
+  const cooked_index_entry *find_parent (die_addr lookup) const
+  {
+    const void *obj = m_parent_map.find (lookup);
+    return static_cast<const cooked_index_entry *> (obj);
+  }
+
+  /* Find the parent of DIE <OFFSET, IS_DWZ>.  */
+  const cooked_index_entry *find_parent (sect_offset offset, bool is_dwz) const
+  {
+    return find_parent (form_addr (offset, is_dwz));
+  }
+
+  /* Set the parent of DIES in range [START, END] to PARENT_ENTRY.  */
+  void set_parent (die_addr start, die_addr end,
+		   const cooked_index_entry *parent_entry)
+  {
+    m_parent_map.set_empty (start, end, (void *)parent_entry);
+  }
+
+  /* Set the parent of DIES in range <[START, END], IS_DWZ> to PARENT_ENTRY.  */
+  void set_parent (sect_offset start, sect_offset end, bool is_dwz,
+		   const cooked_index_entry *parent_entry)
+  {
+    set_parent (form_addr (start, is_dwz), form_addr (end, is_dwz),
+		parent_entry);
+  }
+
+  /* A helper function to turn a tuple <OFFSET, IS_DWZ> into a die_addr.  */
+  static die_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;
+  }
+
+private:
+
+  /* An addrmap that maps from section offsets to cooked_index_entry *.  */
+  addrmap_mutable m_parent_map;
+};
+
 class cooked_index;
 
 /* An index of interesting DIEs.  This is "cooked", in contrast to a
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 9311666a832..926feab8a7a 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -4552,16 +4552,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);
@@ -4599,7 +4589,7 @@ class cooked_indexer
 				   cooked_index_flag *flags,
 				   sect_offset *sibling_offset,
 				   const cooked_index_entry **parent_entry,
-				   CORE_ADDR *maybe_defer,
+				   parent_map::die_addr *maybe_defer,
 				   bool for_specification);
 
   /* Handle DW_TAG_imported_unit, by scanning the DIE to find
@@ -4629,14 +4619,34 @@ class cooked_indexer
   /* 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;
+  parent_map m_die_range_map;
+
+  /* Find the parent of DIE <OFFSET, IS_DWZ>.  */
+  const cooked_index_entry *find_parent (sect_offset offset, bool is_dwz) const
+  {
+    return m_die_range_map.find_parent (offset, is_dwz);
+  }
+
+  /* Find the parent of DIE LOOKUP.  */
+  const cooked_index_entry *find_parent (parent_map::die_addr lookup) const
+  {
+    return m_die_range_map.find_parent (lookup);
+  }
+
+  /* Set the parent of DIES in range <[START, END], IS_DWZ> to
+     PARENT_ENTRY.  */
+  void set_parent (sect_offset start, sect_offset end, bool is_dwz,
+		   const cooked_index_entry *parent_entry)
+  {
+    m_die_range_map.set_parent (start, end, is_dwz, parent_entry);
+  }
 
   /* A single deferred entry.  */
   struct deferred_entry
   {
     sect_offset die_offset;
     const char *name;
-    CORE_ADDR spec_offset;
+    parent_map::die_addr spec_offset;
     dwarf_tag tag;
     cooked_index_flag flags;
   };
@@ -16009,7 +16019,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::die_addr *maybe_defer,
 				 bool for_specification)
 {
   bool origin_is_dwz = false;
@@ -16182,13 +16192,9 @@ cooked_indexer::scan_attributes (dwarf2_per_cu_data *scanning_per_cu,
 	  if (new_reader->cu == reader->cu
 	      && new_info_ptr > watermark_ptr
 	      && *parent_entry == nullptr)
-	    *maybe_defer = form_addr (origin_offset, origin_is_dwz);
+	    *maybe_defer = parent_map::form_addr (origin_offset, origin_is_dwz);
 	  else 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);
-	    }
+	    *parent_entry = find_parent (origin_offset, origin_is_dwz);
 
 	  unsigned int bytes_read;
 	  const abbrev_info *new_abbrev = peek_die_abbrev (*new_reader,
@@ -16305,11 +16311,10 @@ 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),
-				 reader->cu->per_cu->is_dwz);
-      m_die_range_map.set_empty (start, end, (void *) parent_entry);
+      set_parent (parent_entry->die_offset + 1,
+		  sect_offset (info_ptr - 1 - reader->buffer),
+		  reader->cu->per_cu->is_dwz,
+		  parent_entry);
     }
 
   return info_ptr;
@@ -16351,7 +16356,7 @@ cooked_indexer::index_dies (cutu_reader *reader,
 
       const char *name = nullptr;
       const char *linkage_name = nullptr;
-      CORE_ADDR defer = 0;
+      parent_map::die_addr defer = 0;
       cooked_index_flag flags = IS_STATIC;
       sect_offset sibling {};
       const cooked_index_entry *this_parent_entry = parent_entry;
@@ -16482,8 +16487,7 @@ cooked_indexer::make_index (cutu_reader *reader)
 
   for (const auto &entry : m_deferred_entries)
     {
-      void *obj = m_die_range_map.find (entry.spec_offset);
-      cooked_index_entry *parent = static_cast<cooked_index_entry *> (obj);
+      const cooked_index_entry *parent = find_parent (entry.spec_offset);
       m_index_storage->add (entry.die_offset, entry.tag, entry.flags,
 			    entry.name, parent, m_per_cu);
     }

base-commit: 53302c2d33c26909cc8446819f6677076ba48ca9
-- 
2.35.3

Re: [PATCH 02/13] [gdb/symtab] Check effect in parent_map::set_parent

[Tom de Vries]

On 10/20/23 21:51, Tom Tromey wrote:
>>>>>> "Tom" == Tom de Vries via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> Tom> Set_parent uses m_die_range_map.set_empty to set the parent of a range.
> Tom> If part of the range is already set, it remains the same.
> 
> Tom> If the entire range is already set, the set_empty has no effect, silently.
> 
> Tom> Fix this by verifying that calling set_empty has the desired effect on the
> Tom> start and end points of the range.
> 
> This seems like it might be better as a unit test.

We're trying to check the uses of set_parent, not the implementation, so 
a unit test doesn't help there.

Consider the following sequence:
- set_parent (3, 6, b)
- set_parent (1, 10, a)

Now the parent map looks like:
- [1-2] : a
- [3-6] : b
- [7-10]: a

Now say we accidentally execute this in the opposite order:
- set_parent (1, 10, a)
- set_parent (3, 6, b)

Now the parent map looks like this instead:
- [1-10]: a

The second set_parent has no effect, and this happens silently.  The 
assert tries to make this mistake visible.

It could be fitting to add some kind of gdb_checking_assert scheme for 
this, which is off by default in releases, and can be switched on by 
some means, if this is considered too expensive.

Anyway, since it raises questions, I'll drop this patch for now, but I'd 
like to note that it was useful for me in development of this patch series.

Thanks,
- Tom

Re: [PATCH 09/13] [gdb/symtab] Keep track of processed DIEs in shard

[Tom de Vries]

On 10/2/23 14:50, Tom de Vries via Gdb-patches wrote:
>     while (info_ptr < end_ptr)
>       {
>         sect_offset this_die = (sect_offset) (info_ptr - reader->buffer);
> +
> +      {
> +	CORE_ADDR end_prev_die
> +	  = form_addr (this_die - 1, reader->cu->per_cu->is_dwz,
> +		       reader->cu->per_cu->is_debug_types);
> +	set_parent_valid (start_cu, end_prev_die);
> +      }
> +

This turned out to cause a performance degradation of 30%, and to be 
unnecessary, so I dropped this bit.

Thanks,
- Tom

Re: [PATCH 07/13] [gdb/symtab] Resolve deferred entries, inter-shard case

[Tom de Vries]

On 10/2/23 14:50, Tom de Vries via Gdb-patches wrote:
> +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);
> +

I was testing a to-be-submitted v2 of this patch series, and ran into 
this assert.  I was able to reproduce it using this series (v1) as well.

A minimal trigger looks like:
...
$ gdb -q -batch 
/usr/lib/debug/usr/lib64/libstdc++.so.6.0.32-13.2.1+git7813-150000.1.6.1.x86_64.debug
...
so I suppose when I first tested this series I didn't have debuginfo for 
/usr/lib64/libstdc++.so.6 installed.

Thanks,
- Tom