public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 00/17] Rewrite .debug_names reader and writer
@ 2023-12-10 16:44 Tom Tromey
  2023-12-10 16:44 ` [PATCH 01/17] Refactor 'maint set dwarf synchronous' handling Tom Tromey
                   ` (16 more replies)
  0 siblings, 17 replies; 22+ messages in thread
From: Tom Tromey @ 2023-12-10 16:44 UTC (permalink / raw)
  To: gdb-patches

This series rewrites gdb's .debug_names reader and writer.

Currently, gdb can generate a .debug_names index, but the contents are
very wrong -- they use fully-qualified names, which is not at all what
is envisioned in the DWARF spec.

After this series, the section contents will be much closer to DWARF.
However, it's not really useful for gdb to follow DWARF exactly.

First, the DWARF spec has a few bugs affecting the generation of the
index.  GDB fixes these -- you won't see these fixes in this series,
though, as they were all handled by the new DWARF scanner.

Second, gdb likes to have some information not readily available in
the DWARF-specified contents.  You can see the new DW_IDX_* constants
patch for a list.

I've regression tested this using cc-with-debug-names on x86-64
Fedora 38.

It's possible that there are regressions with cc-with-debug-names at
some intermediate points in the series -- in particular I think it
would happen after the reader is rewritten but before the writer patch
is applied.  I didn't test this situation, since I don't think it is
very important.  It could be worked around by combining the two
patches, but I thought that would make review much more difficult.

This is based on my earlier series to read DWARF in the background.
This was done because (1) that's going to land eventually anyway and
(2) that series generalizes the cooked index code somewhat, making it
a nicer starting point.

---
Tom Tromey (17):
      Refactor 'maint set dwarf synchronous' handling
      Refactor quick-function installation in DWARF reader
      Remove IS_ENUM_CLASS from cooked_index_flag
      Add some new DW_IDX_* values
      Document GDB extensions to DWARF .debug_names
      Add language to cooked_index_entry
      Move cooked_index_functions to cooked-index.h
      Do not write the index cache from an index
      Change cooked_index_worker to abstract base class
      Remove cooked_index_worker::start_reading
      Empty hash table fix in .debug_names reader
      Fix dw2-zero-range.exp when an index is in use
      Explicitly expand CUs in dw2-inline-with-lexical-scope.exp
      Remove some .debug_names tests
      Rewrite .debug_names reader
      Export dwarf5_augmentation
      Rewrite .debug_names writer

 gdb/doc/gdb.texinfo                                |  39 +
 gdb/dwarf2/cooked-index.c                          | 162 +++-
 gdb/dwarf2/cooked-index.h                          | 184 +++-
 gdb/dwarf2/index-write.c                           | 401 ++++-----
 gdb/dwarf2/mapped-index.h                          |   4 +-
 gdb/dwarf2/read-debug-names.c                      | 949 ++++++++-------------
 gdb/dwarf2/read-debug-names.h                      |   2 +
 gdb/dwarf2/read.c                                  | 337 ++------
 gdb/testsuite/gdb.dwarf2/clang-debug-names-2-foo.c |  22 -
 gdb/testsuite/gdb.dwarf2/clang-debug-names-2.c     |  27 -
 gdb/testsuite/gdb.dwarf2/clang-debug-names-2.exp   |  42 -
 gdb/testsuite/gdb.dwarf2/clang-debug-names.c       |  25 -
 gdb/testsuite/gdb.dwarf2/clang-debug-names.exp     |  42 -
 gdb/testsuite/gdb.dwarf2/clang-debug-names.exp.tcl | 121 ---
 .../gdb.dwarf2/dw2-inline-with-lexical-scope.exp   |   4 +
 gdb/testsuite/gdb.dwarf2/dw2-zero-range.exp        |  10 +-
 include/dwarf2.def                                 |   9 +
 17 files changed, 948 insertions(+), 1432 deletions(-)
---
base-commit: 98eea77e9ce69ba00d5f93dd2264f1d1971fd86f
change-id: 20231210-debug-names-fix-cfbaf8761ecc

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


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 01/17] Refactor 'maint set dwarf synchronous' handling
  2023-12-10 16:44 [PATCH 00/17] Rewrite .debug_names reader and writer Tom Tromey
@ 2023-12-10 16:44 ` Tom Tromey
  2023-12-10 16:44 ` [PATCH 02/17] Refactor quick-function installation in DWARF reader Tom Tromey
                   ` (15 subsequent siblings)
  16 siblings, 0 replies; 22+ messages in thread
From: Tom Tromey @ 2023-12-10 16:44 UTC (permalink / raw)
  To: gdb-patches

The new .debug_names reader will reuse the background reading
infrastructure of the cooked index code.  In order to share the
handling of 'maint set dwarf synchronous' -- and to avoid having to
export this global -- this patch refactors this to be handled directly
in dwarf2_initialize_objfile.
---
 gdb/dwarf2/read.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index cc940d02cea..dc57af96275 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -3267,6 +3267,10 @@ dwarf2_initialize_objfile (struct objfile *objfile,
       global_index_cache.miss ();
       objfile->qf.push_front (make_cooked_index_funcs (per_objfile));
     }
+
+  if (dwarf_synchronous && per_bfd->index_table != nullptr)
+    per_bfd->index_table->wait_completely ();
+
   return true;
 }
 
@@ -16828,9 +16832,6 @@ make_cooked_index_funcs (dwarf2_per_objfile *per_objfile)
      avoids races.  */
   idx->start_reading ();
 
-  if (dwarf_synchronous)
-    idx->wait_completely ();
-
   return quick_symbol_functions_up (new cooked_index_functions);
 }
 

-- 
2.43.0


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 02/17] Refactor quick-function installation in DWARF reader
  2023-12-10 16:44 [PATCH 00/17] Rewrite .debug_names reader and writer Tom Tromey
  2023-12-10 16:44 ` [PATCH 01/17] Refactor 'maint set dwarf synchronous' handling Tom Tromey
@ 2023-12-10 16:44 ` Tom Tromey
  2023-12-10 16:44 ` [PATCH 03/17] Remove IS_ENUM_CLASS from cooked_index_flag Tom Tromey
                   ` (14 subsequent siblings)
  16 siblings, 0 replies; 22+ messages in thread
From: Tom Tromey @ 2023-12-10 16:44 UTC (permalink / raw)
  To: gdb-patches

While working on the previous patch, I saw that the handling of
quick-function installation could be unified
dwarf2_initialize_objfile.  In particular, at the end of the function,
if there is an index table, then it can be used to create the quick
function object.

This cleanup will be useful when rewriting the .debug_names reader.
---
 gdb/dwarf2/read.c | 38 ++++++++++++++------------------------
 1 file changed, 14 insertions(+), 24 deletions(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index dc57af96275..7262de4cc19 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -3190,8 +3190,7 @@ get_gdb_index_contents_from_cache_dwz (objfile *obj, dwz_file *dwz)
   return global_index_cache.lookup_gdb_index (build_id, &dwz->index_cache_res);
 }
 
-static quick_symbol_functions_up make_cooked_index_funcs
-     (dwarf2_per_objfile *);
+static void start_debug_info_reader (dwarf2_per_objfile *);
 
 /* See dwarf2/public.h.  */
 
@@ -3236,23 +3235,13 @@ dwarf2_initialize_objfile (struct objfile *objfile,
   /* Was a GDB index already read when we processed an objfile sharing
      PER_BFD?  */
   else if (per_bfd->index_table != nullptr)
-    {
-      dwarf_read_debug_printf ("re-using symbols");
-      objfile->qf.push_front (per_bfd->index_table->make_quick_functions ());
-    }
+    dwarf_read_debug_printf ("re-using symbols");
   else if (dwarf2_read_debug_names (per_objfile))
-    {
-      dwarf_read_debug_printf ("found debug names");
-      objfile->qf.push_front
-	(per_bfd->index_table->make_quick_functions ());
-    }
+    dwarf_read_debug_printf ("found debug names");
   else if (dwarf2_read_gdb_index (per_objfile,
 				  get_gdb_index_contents_from_section<struct dwarf2_per_bfd>,
 				  get_gdb_index_contents_from_section<dwz_file>))
-    {
-      dwarf_read_debug_printf ("found gdb index from file");
-      objfile->qf.push_front (per_bfd->index_table->make_quick_functions ());
-    }
+    dwarf_read_debug_printf ("found gdb index from file");
   /* ... otherwise, try to find the index in the index cache.  */
   else if (dwarf2_read_gdb_index (per_objfile,
 			     get_gdb_index_contents_from_cache,
@@ -3260,16 +3249,19 @@ dwarf2_initialize_objfile (struct objfile *objfile,
     {
       dwarf_read_debug_printf ("found gdb index from cache");
       global_index_cache.hit ();
-      objfile->qf.push_front (per_bfd->index_table->make_quick_functions ());
     }
   else
     {
       global_index_cache.miss ();
-      objfile->qf.push_front (make_cooked_index_funcs (per_objfile));
+      start_debug_info_reader (per_objfile);
     }
 
-  if (dwarf_synchronous && per_bfd->index_table != nullptr)
-    per_bfd->index_table->wait_completely ();
+  if (per_bfd->index_table != nullptr)
+    {
+      if (dwarf_synchronous)
+	per_bfd->index_table->wait_completely ();
+      objfile->qf.push_front (per_bfd->index_table->make_quick_functions ());
+    }
 
   return true;
 }
@@ -16818,10 +16810,10 @@ cooked_index_functions::expand_symtabs_matching
   return true;
 }
 
-/* Return a new cooked_index_functions object.  */
+/* Start reading .debug_info using the indexer.  */
 
-static quick_symbol_functions_up
-make_cooked_index_funcs (dwarf2_per_objfile *per_objfile)
+static void
+start_debug_info_reader (dwarf2_per_objfile *per_objfile)
 {
   /* Set the index table early so that sharing works even while
      scanning; and then start the scanning.  */
@@ -16831,8 +16823,6 @@ make_cooked_index_funcs (dwarf2_per_objfile *per_objfile)
   /* Don't start reading until after 'index_table' is set.  This
      avoids races.  */
   idx->start_reading ();
-
-  return quick_symbol_functions_up (new cooked_index_functions);
 }
 
 quick_symbol_functions_up

-- 
2.43.0


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 03/17] Remove IS_ENUM_CLASS from cooked_index_flag
  2023-12-10 16:44 [PATCH 00/17] Rewrite .debug_names reader and writer Tom Tromey
  2023-12-10 16:44 ` [PATCH 01/17] Refactor 'maint set dwarf synchronous' handling Tom Tromey
  2023-12-10 16:44 ` [PATCH 02/17] Refactor quick-function installation in DWARF reader Tom Tromey
@ 2023-12-10 16:44 ` Tom Tromey
  2023-12-10 16:44 ` [PATCH 04/17] Add some new DW_IDX_* values Tom Tromey
                   ` (13 subsequent siblings)
  16 siblings, 0 replies; 22+ messages in thread
From: Tom Tromey @ 2023-12-10 16:44 UTC (permalink / raw)
  To: gdb-patches

I noticed that cooked_index_flag::IS_ENUM_CLASS is not needed.  This
patch removes it.
---
 gdb/dwarf2/cooked-index.c |  1 -
 gdb/dwarf2/cooked-index.h |  6 ++----
 gdb/dwarf2/read.c         | 14 ++++++++------
 3 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/gdb/dwarf2/cooked-index.c b/gdb/dwarf2/cooked-index.c
index fabffc75d92..e59835ea11f 100644
--- a/gdb/dwarf2/cooked-index.c
+++ b/gdb/dwarf2/cooked-index.c
@@ -48,7 +48,6 @@ to_string (cooked_index_flag flags)
   static constexpr cooked_index_flag::string_mapping mapping[] = {
     MAP_ENUM_FLAG (IS_MAIN),
     MAP_ENUM_FLAG (IS_STATIC),
-    MAP_ENUM_FLAG (IS_ENUM_CLASS),
     MAP_ENUM_FLAG (IS_LINKAGE),
     MAP_ENUM_FLAG (IS_TYPE_DECLARATION),
   };
diff --git a/gdb/dwarf2/cooked-index.h b/gdb/dwarf2/cooked-index.h
index 5fccb2bffc0..323c335677b 100644
--- a/gdb/dwarf2/cooked-index.h
+++ b/gdb/dwarf2/cooked-index.h
@@ -56,13 +56,11 @@ enum cooked_index_flag_enum : unsigned char
   IS_MAIN = 1,
   /* True if this entry represents a "static" object.  */
   IS_STATIC = 2,
-  /* True if this entry is an "enum class".  */
-  IS_ENUM_CLASS = 4,
   /* True if this entry uses the linkage name.  */
-  IS_LINKAGE = 8,
+  IS_LINKAGE = 4,
   /* True if this entry is just for the declaration of a type, not the
      definition.  */
-  IS_TYPE_DECLARATION = 16,
+  IS_TYPE_DECLARATION = 8,
 };
 DEF_ENUM_FLAGS_TYPE (enum cooked_index_flag_enum, cooked_index_flag);
 
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 7262de4cc19..e7da647fec2 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -4517,6 +4517,7 @@ class cooked_indexer
 				   sect_offset *sibling_offset,
 				   const cooked_index_entry **parent_entry,
 				   CORE_ADDR *maybe_defer,
+				   bool *is_enum_class,
 				   bool for_specification);
 
   /* Handle DW_TAG_imported_unit, by scanning the DIE to find
@@ -16079,6 +16080,7 @@ cooked_indexer::scan_attributes (dwarf2_per_cu_data *scanning_per_cu,
 				 sect_offset *sibling_offset,
 				 const cooked_index_entry **parent_entry,
 				 CORE_ADDR *maybe_defer,
+				 bool *is_enum_class,
 				 bool for_specification)
 {
   bool origin_is_dwz = false;
@@ -16162,7 +16164,7 @@ cooked_indexer::scan_attributes (dwarf2_per_cu_data *scanning_per_cu,
 
 	case DW_AT_enum_class:
 	  if (attr.as_boolean ())
-	    *flags |= IS_ENUM_CLASS;
+	    *is_enum_class = true;
 	  break;
 
 	case DW_AT_low_pc:
@@ -16272,7 +16274,8 @@ cooked_indexer::scan_attributes (dwarf2_per_cu_data *scanning_per_cu,
 	  else
 	    scan_attributes (scanning_per_cu, new_reader, new_info_ptr,
 			     new_info_ptr, new_abbrev, name, linkage_name,
-			     flags, nullptr, parent_entry, maybe_defer, true);
+			     flags, nullptr, parent_entry, maybe_defer,
+			     is_enum_class, true);
 	}
     }
 
@@ -16424,10 +16427,11 @@ 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;
+      bool is_enum_class = false;
       info_ptr = scan_attributes (reader->cu->per_cu, reader, info_ptr,
 				  info_ptr, abbrev, &name, &linkage_name,
 				  &flags, &sibling, &this_parent_entry,
-				  &defer, false);
+				  &defer, &is_enum_class, false);
 
       if (abbrev->tag == DW_TAG_namespace
 	  && m_language == language_cplus
@@ -16493,9 +16497,7 @@ cooked_indexer::index_dies (cutu_reader *reader,
 		 "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,
+				  is_enum_class ? this_entry : parent_entry,
 				  fully);
 	      continue;
 

-- 
2.43.0


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 04/17] Add some new DW_IDX_* values
  2023-12-10 16:44 [PATCH 00/17] Rewrite .debug_names reader and writer Tom Tromey
                   ` (2 preceding siblings ...)
  2023-12-10 16:44 ` [PATCH 03/17] Remove IS_ENUM_CLASS from cooked_index_flag Tom Tromey
@ 2023-12-10 16:44 ` Tom Tromey
  2024-01-09 15:08   ` Tom Tromey
  2023-12-10 16:44 ` [PATCH 05/17] Document GDB extensions to DWARF .debug_names Tom Tromey
                   ` (12 subsequent siblings)
  16 siblings, 1 reply; 22+ messages in thread
From: Tom Tromey @ 2023-12-10 16:44 UTC (permalink / raw)
  To: gdb-patches

The new .debug_names writer in GDB needs to emit some symbol
properties, so that the reader can be fully functional.  This patch
adds a few new DW_IDX_* constants, and tries to document the existing
extensions as well.  (I will add more documentation of these to the
GDB manual as well.)
---
 include/dwarf2.def | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/include/dwarf2.def b/include/dwarf2.def
index 7ab3ee611fd..75b75d90188 100644
--- a/include/dwarf2.def
+++ b/include/dwarf2.def
@@ -802,8 +802,17 @@ DW_IDX (DW_IDX_parent, 4)
 DW_IDX (DW_IDX_type_hash, 5)
 DW_IDX_DUP (DW_IDX_lo_user, 0x2000)
 DW_IDX (DW_IDX_hi_user, 0x3fff)
+/* Internal linkage.  A flag.  */
 DW_IDX (DW_IDX_GNU_internal, 0x2000)
+/* External linkage.  A flag.  Note that gdb no longer generates this;
+   the default is to assume external linkage.  */
 DW_IDX (DW_IDX_GNU_external, 0x2001)
+/* This entry is the program's entry point.  A flag.  */
+DW_IDX (DW_IDX_GNU_main, 0x2002)
+/* Language for this entry.  A DW_LANG_* value.  */
+DW_IDX (DW_IDX_GNU_language, 0x2003)
+/* This entry is a linkage name.  A flag.  */
+DW_IDX (DW_IDX_GNU_linkage_name, 0x2004)
 DW_END_IDX
 
 /* DWARF5 Unit type header encodings  */

-- 
2.43.0


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 05/17] Document GDB extensions to DWARF .debug_names
  2023-12-10 16:44 [PATCH 00/17] Rewrite .debug_names reader and writer Tom Tromey
                   ` (3 preceding siblings ...)
  2023-12-10 16:44 ` [PATCH 04/17] Add some new DW_IDX_* values Tom Tromey
@ 2023-12-10 16:44 ` Tom Tromey
  2023-12-10 17:37   ` Eli Zaretskii
  2023-12-10 16:44 ` [PATCH 06/17] Add language to cooked_index_entry Tom Tromey
                   ` (11 subsequent siblings)
  16 siblings, 1 reply; 22+ messages in thread
From: Tom Tromey @ 2023-12-10 16:44 UTC (permalink / raw)
  To: gdb-patches

GDB's new .debug_names implementation uses some extensions.  This
patch adds some documentation on this to the manual.
---
 gdb/doc/gdb.texinfo | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 139484460c8..755f48ed1de 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -21477,6 +21477,7 @@ program.  To debug a core dump of a previous run, you must also tell
 * Separate Debug Files::        Debugging information in separate files
 * MiniDebugInfo::               Debugging information in a special section
 * Index Files::                 Index files speed up GDB
+* Debug Names::                 Extensions to .debug_names
 * Symbol Errors::               Errors reading symbol files
 * Data Files::                  GDB data files
 @end menu
@@ -22680,6 +22681,44 @@ Print the number of cache hits and misses since the launch of @value{GDBN}.
 
 @end table
 
+@node Debug Names
+@section Extensions to @samp{.debug_names}
+@cindex index files
+@cindex @samp{.debug_names} section
+
+The DWARF specification documents an optional index section called
+@samp{.debug_names}.  @value{GDBN} can both read and create this
+section.  However, in order to work with @value{GDBN}, some extensions
+were necessary.
+
+@value{GDBN} uses the augmentation string @samp{GDB2}.  Earlier
+versions used the string @samp{GDB}, but these versions of the index
+are no longer supported.
+
+@value{GDBN} does not use the specified hash table.  Therefore,
+because this hash table is optional, @value{GDBN} also does not write
+it.
+
+@value{GDBN} also generates and uses some extra index attributes:
+@table @code
+@item DW_IDX_GNU_internal
+This has the value @samp{0x2000}.  It is a flag that, when set,
+indicates that the associated entry has @code{static} linkage.
+
+@item DW_IDX_GNU_main
+This has the value @samp{0x2002}.  It is a flag that, when set,
+indicates that the associated entry is the program's @code{main}.
+
+@item DW_IDX_GNU_language
+This has the value @samp{0x2003}.  It is @samp{DW_LANG_} constant,
+indicating the language of the associated entry.
+
+@item DW_IDX_GNU_linkage_name
+This has the value @samp{0x2004}.  It is a flag that, when set,
+indicates that the associated entry is a linkage name, and not a
+source name.
+@end table
+
 @node Symbol Errors
 @section Errors Reading Symbol Files
 

-- 
2.43.0


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 06/17] Add language to cooked_index_entry
  2023-12-10 16:44 [PATCH 00/17] Rewrite .debug_names reader and writer Tom Tromey
                   ` (4 preceding siblings ...)
  2023-12-10 16:44 ` [PATCH 05/17] Document GDB extensions to DWARF .debug_names Tom Tromey
@ 2023-12-10 16:44 ` Tom Tromey
  2023-12-10 16:44 ` [PATCH 07/17] Move cooked_index_functions to cooked-index.h Tom Tromey
                   ` (10 subsequent siblings)
  16 siblings, 0 replies; 22+ messages in thread
From: Tom Tromey @ 2023-12-10 16:44 UTC (permalink / raw)
  To: gdb-patches

This adds a new 'lang' member to cooked_index_entry.  This holds the
language of the symbol.  This is primarily useful for the new
.debug_names reader, which will not scan the CUs for languages up
front.

This also changes cooked_index_shard::add to return a non-const
pointer.  This doesn't impact the current code, but is needed for the
new reader.
---
 gdb/dwarf2/cooked-index.c | 24 ++++++++++++------------
 gdb/dwarf2/cooked-index.h | 32 +++++++++++++++++++-------------
 gdb/dwarf2/index-write.c  |  4 ++--
 3 files changed, 33 insertions(+), 27 deletions(-)

diff --git a/gdb/dwarf2/cooked-index.c b/gdb/dwarf2/cooked-index.c
index e59835ea11f..d32fb360a4b 100644
--- a/gdb/dwarf2/cooked-index.c
+++ b/gdb/dwarf2/cooked-index.c
@@ -206,7 +206,7 @@ cooked_index_entry::full_name (struct obstack *storage, bool for_main) const
     return local_name;
 
   const char *sep = nullptr;
-  switch (per_cu->lang ())
+  switch (lang)
     {
     case language_cplus:
     case language_rust:
@@ -244,13 +244,14 @@ cooked_index_entry::write_scope (struct obstack *storage,
 
 /* See cooked-index.h.  */
 
-const cooked_index_entry *
+cooked_index_entry *
 cooked_index_shard::add (sect_offset die_offset, enum dwarf_tag tag,
-			 cooked_index_flag flags, const char *name,
+			 cooked_index_flag flags, enum language lang,
+			 const char *name,
 			 const cooked_index_entry *parent_entry,
 			 dwarf2_per_cu_data *per_cu)
 {
-  cooked_index_entry *result = create (die_offset, tag, flags, name,
+  cooked_index_entry *result = create (die_offset, tag, flags, lang, name,
 				       parent_entry, per_cu);
   m_entries.push_back (result);
 
@@ -260,7 +261,7 @@ cooked_index_shard::add (sect_offset die_offset, enum dwarf_tag tag,
     m_main = result;
   else if (parent_entry == nullptr
 	   && m_main == nullptr
-	   && language_may_use_plain_main (per_cu->lang ())
+	   && language_may_use_plain_main (lang)
 	   && strcmp (name, "main") == 0)
     m_main = result;
 
@@ -299,7 +300,7 @@ cooked_index_shard::handle_gnat_encoded_entry (cooked_index_entry *entry,
 	  gdb::unique_xmalloc_ptr<char> new_name
 	    = make_unique_xstrndup (name.data (), name.length ());
 	  last = create (entry->die_offset, DW_TAG_namespace,
-			 0, new_name.get (), parent,
+			 0, language_ada, new_name.get (), parent,
 			 entry->per_cu);
 	  last->canonical = last->name;
 	  m_names.push_back (std::move (new_name));
@@ -362,7 +363,7 @@ cooked_index_shard::finalize ()
       gdb_assert (entry->canonical == nullptr);
       if ((entry->flags & IS_LINKAGE) != 0)
 	entry->canonical = entry->name;
-      else if (entry->per_cu->lang () == language_ada)
+      else if (entry->lang == language_ada)
 	{
 	  gdb::unique_xmalloc_ptr<char> canon_name
 	    = handle_gnat_encoded_entry (entry, gnat_entries.get ());
@@ -374,15 +375,14 @@ cooked_index_shard::finalize ()
 	      m_names.push_back (std::move (canon_name));
 	    }
 	}
-      else if (entry->per_cu->lang () == language_cplus
-	       || entry->per_cu->lang () == language_c)
+      else if (entry->lang == language_cplus || entry->lang == language_c)
 	{
 	  void **slot = htab_find_slot (seen_names.get (), entry,
 					INSERT);
 	  if (*slot == nullptr)
 	    {
 	      gdb::unique_xmalloc_ptr<char> canon_name
-		= (entry->per_cu->lang () == language_cplus
+		= (entry->lang == language_cplus
 		   ? cp_canonicalize_string (entry->name)
 		   : c_canonicalize_name (entry->name));
 	      if (canon_name == nullptr)
@@ -570,7 +570,7 @@ cooked_index::get_main_name (struct obstack *obstack, enum language *lang)
   if (entry == nullptr)
     return nullptr;
 
-  *lang = entry->per_cu->lang ();
+  *lang = entry->lang;
   return entry->full_name (obstack, true);
 }
 
@@ -593,7 +593,7 @@ cooked_index::get_main () const
 	{
 	  if ((entry->flags & IS_MAIN) != 0)
 	    {
-	      if (!language_requires_canonicalization (entry->per_cu->lang ()))
+	      if (!language_requires_canonicalization (entry->lang))
 		{
 		  /* There won't be one better than this.  */
 		  return entry;
diff --git a/gdb/dwarf2/cooked-index.h b/gdb/dwarf2/cooked-index.h
index 323c335677b..4e7882204a9 100644
--- a/gdb/dwarf2/cooked-index.h
+++ b/gdb/dwarf2/cooked-index.h
@@ -85,12 +85,14 @@ extern bool language_requires_canonicalization (enum language lang);
 struct cooked_index_entry : public allocate_on_obstack
 {
   cooked_index_entry (sect_offset die_offset_, enum dwarf_tag tag_,
-		      cooked_index_flag flags_, const char *name_,
+		      cooked_index_flag flags_,
+		      enum language lang_, const char *name_,
 		      const cooked_index_entry *parent_entry_,
 		      dwarf2_per_cu_data *per_cu_)
     : name (name_),
       tag (tag_),
       flags (flags_),
+      lang (lang_),
       die_offset (die_offset_),
       parent_entry (parent_entry_),
       per_cu (per_cu_)
@@ -229,6 +231,8 @@ struct cooked_index_entry : public allocate_on_obstack
   enum dwarf_tag tag;
   /* Any flags attached to this entry.  */
   cooked_index_flag flags;
+  /* The language of this symbol.  */
+  ENUM_BITFIELD (language) lang : LANGUAGE_BITS;
   /* The offset of this DIE.  */
   sect_offset die_offset;
   /* The parent entry.  This is NULL for top-level entries.
@@ -264,11 +268,11 @@ class cooked_index_shard
 
   /* Create a new cooked_index_entry and register it with this object.
      Entries are owned by this object.  The new item is returned.  */
-  const cooked_index_entry *add (sect_offset die_offset, enum dwarf_tag tag,
-				 cooked_index_flag flags,
-				 const char *name,
-				 const cooked_index_entry *parent_entry,
-				 dwarf2_per_cu_data *per_cu);
+  cooked_index_entry *add (sect_offset die_offset, enum dwarf_tag tag,
+			   cooked_index_flag flags, enum language lang,
+			   const char *name,
+			   const cooked_index_entry *parent_entry,
+			   dwarf2_per_cu_data *per_cu);
 
   /* Install a new fixed addrmap from the given mutable addrmap.  */
   void install_addrmap (addrmap_mutable *map)
@@ -316,12 +320,13 @@ class cooked_index_shard
   cooked_index_entry *create (sect_offset die_offset,
 			      enum dwarf_tag tag,
 			      cooked_index_flag flags,
+			      enum language lang,
 			      const char *name,
 			      const cooked_index_entry *parent_entry,
 			      dwarf2_per_cu_data *per_cu)
   {
     return new (&m_storage) cooked_index_entry (die_offset, tag, flags,
-						name, parent_entry,
+						lang, name, parent_entry,
 						per_cu);
   }
 
@@ -379,13 +384,14 @@ class cooked_index_storage
 
   /* Add an entry to the index.  The arguments describe the entry; see
      cooked-index.h.  The new entry is returned.  */
-  const cooked_index_entry *add (sect_offset die_offset, enum dwarf_tag tag,
-				 cooked_index_flag flags,
-				 const char *name,
-				 const cooked_index_entry *parent_entry,
-				 dwarf2_per_cu_data *per_cu)
+  cooked_index_entry *add (sect_offset die_offset, enum dwarf_tag tag,
+			   cooked_index_flag flags,
+			   const char *name,
+			   const cooked_index_entry *parent_entry,
+			   dwarf2_per_cu_data *per_cu)
   {
-    return m_index->add (die_offset, tag, flags, name, parent_entry, per_cu);
+    return m_index->add (die_offset, tag, flags, per_cu->lang (),
+			 name, parent_entry, per_cu);
   }
 
   /* Install the current addrmap into the shard being constructed,
diff --git a/gdb/dwarf2/index-write.c b/gdb/dwarf2/index-write.c
index b4a0117330e..44728e4216d 100644
--- a/gdb/dwarf2/index-write.c
+++ b/gdb/dwarf2/index-write.c
@@ -1282,7 +1282,7 @@ write_cooked_index (cooked_index *table,
 
       const char *name = entry->full_name (symtab->obstack ());
 
-      if (entry->per_cu->lang () == language_ada)
+      if (entry->lang == language_ada)
 	{
 	  /* In order for the index to work when read back into
 	     gdb, it has to use the encoded name, with any
@@ -1290,7 +1290,7 @@ write_cooked_index (cooked_index *table,
 	  std::string encoded = ada_encode (name, false);
 	  name = obstack_strdup (symtab->obstack (), encoded.c_str ());
 	}
-      else if (entry->per_cu->lang () == language_cplus
+      else if (entry->lang == language_cplus
 	       && (entry->flags & IS_LINKAGE) != 0)
 	{
 	  /* GDB never put C++ linkage names into .gdb_index.  The

-- 
2.43.0


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 07/17] Move cooked_index_functions to cooked-index.h
  2023-12-10 16:44 [PATCH 00/17] Rewrite .debug_names reader and writer Tom Tromey
                   ` (5 preceding siblings ...)
  2023-12-10 16:44 ` [PATCH 06/17] Add language to cooked_index_entry Tom Tromey
@ 2023-12-10 16:44 ` Tom Tromey
  2023-12-10 16:44 ` [PATCH 08/17] Do not write the index cache from an index Tom Tromey
                   ` (9 subsequent siblings)
  16 siblings, 0 replies; 22+ messages in thread
From: Tom Tromey @ 2023-12-10 16:44 UTC (permalink / raw)
  To: gdb-patches

This moves the declaration of cooked_index_functions to
cooked-index.h.  This makes it visible for use by the rewritten
.debug_names reader, and it also lets us move the implementation of
make_quick_functions into cooked-index.c, where it really belongs.
---
 gdb/dwarf2/cooked-index.c |   6 +++
 gdb/dwarf2/cooked-index.h |  94 +++++++++++++++++++++++++++++++++++++++++++
 gdb/dwarf2/read.c         | 100 ----------------------------------------------
 3 files changed, 100 insertions(+), 100 deletions(-)

diff --git a/gdb/dwarf2/cooked-index.c b/gdb/dwarf2/cooked-index.c
index d32fb360a4b..684773cb192 100644
--- a/gdb/dwarf2/cooked-index.c
+++ b/gdb/dwarf2/cooked-index.c
@@ -614,6 +614,12 @@ cooked_index::get_main () const
   return best_entry;
 }
 
+quick_symbol_functions_up
+cooked_index::make_quick_functions () const
+{
+  return quick_symbol_functions_up (new cooked_index_functions);
+}
+
 /* See cooked-index.h.  */
 
 void
diff --git a/gdb/dwarf2/cooked-index.h b/gdb/dwarf2/cooked-index.h
index 4e7882204a9..62230cf429b 100644
--- a/gdb/dwarf2/cooked-index.h
+++ b/gdb/dwarf2/cooked-index.h
@@ -677,4 +677,98 @@ class cooked_index : public dwarf_scanner_base
   dwarf2_per_bfd *m_per_bfd;
 };
 
+/* An implementation of quick_symbol_functions for the cooked DWARF
+   index.  */
+
+struct cooked_index_functions : public dwarf2_base_index_functions
+{
+  cooked_index *wait (struct objfile *objfile, bool allow_quit)
+  {
+    dwarf2_per_objfile *per_objfile = get_dwarf2_per_objfile (objfile);
+    cooked_index *table
+      = (gdb::checked_static_cast<cooked_index *>
+	 (per_objfile->per_bfd->index_table.get ()));
+    table->wait (cooked_state::MAIN_AVAILABLE, allow_quit);
+    return table;
+  }
+
+  dwarf2_per_cu_data *find_per_cu (dwarf2_per_bfd *per_bfd,
+				   CORE_ADDR adjusted_pc) override;
+
+  struct compunit_symtab *find_compunit_symtab_by_address
+    (struct objfile *objfile, CORE_ADDR address) override;
+
+  bool has_unexpanded_symtabs (struct objfile *objfile) override
+  {
+    wait (objfile, true);
+    return dwarf2_base_index_functions::has_unexpanded_symtabs (objfile);
+  }
+
+  struct symtab *find_last_source_symtab (struct objfile *objfile) override
+  {
+    wait (objfile, true);
+    return dwarf2_base_index_functions::find_last_source_symtab (objfile);
+  }
+
+  void forget_cached_source_info (struct objfile *objfile) override
+  {
+    wait (objfile, true);
+    dwarf2_base_index_functions::forget_cached_source_info (objfile);
+  }
+
+  void print_stats (struct objfile *objfile, bool print_bcache) override
+  {
+    wait (objfile, true);
+    dwarf2_base_index_functions::print_stats (objfile, print_bcache);
+  }
+
+  void dump (struct objfile *objfile) override
+  {
+    cooked_index *index = wait (objfile, true);
+    gdb_printf ("Cooked index in use:\n");
+    gdb_printf ("\n");
+    index->dump (objfile->arch ());
+  }
+
+  void expand_all_symtabs (struct objfile *objfile) override
+  {
+    wait (objfile, true);
+    dwarf2_base_index_functions::expand_all_symtabs (objfile);
+  }
+
+  bool expand_symtabs_matching
+    (struct objfile *objfile,
+     gdb::function_view<expand_symtabs_file_matcher_ftype> file_matcher,
+     const lookup_name_info *lookup_name,
+     gdb::function_view<expand_symtabs_symbol_matcher_ftype> symbol_matcher,
+     gdb::function_view<expand_symtabs_exp_notify_ftype> expansion_notify,
+     block_search_flags search_flags,
+     domain_enum domain,
+     enum search_domain kind) override;
+
+  struct compunit_symtab *find_pc_sect_compunit_symtab
+    (struct objfile *objfile, struct bound_minimal_symbol msymbol,
+     CORE_ADDR pc, struct obj_section *section, int warn_if_readin) override
+  {
+    wait (objfile, true);
+    return (dwarf2_base_index_functions::find_pc_sect_compunit_symtab
+	    (objfile, msymbol, pc, section, warn_if_readin));
+  }
+
+  void map_symbol_filenames
+       (struct objfile *objfile,
+	gdb::function_view<symbol_filename_ftype> fun,
+	bool need_fullname) override
+  {
+    wait (objfile, true);
+    return (dwarf2_base_index_functions::map_symbol_filenames
+	    (objfile, fun, need_fullname));
+  }
+
+  void compute_main_name (struct objfile *objfile) override
+  {
+    wait (objfile, false);
+  }
+};
+
 #endif /* GDB_DWARF2_COOKED_INDEX_H */
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index e7da647fec2..acdb1969915 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -16560,100 +16560,6 @@ cooked_indexer::make_index (cutu_reader *reader)
     }
 }
 
-/* An implementation of quick_symbol_functions for the cooked DWARF
-   index.  */
-
-struct cooked_index_functions : public dwarf2_base_index_functions
-{
-  cooked_index *wait (struct objfile *objfile, bool allow_quit)
-  {
-    dwarf2_per_objfile *per_objfile = get_dwarf2_per_objfile (objfile);
-    cooked_index *table
-      = (gdb::checked_static_cast<cooked_index *>
-	 (per_objfile->per_bfd->index_table.get ()));
-    table->wait (cooked_state::MAIN_AVAILABLE, allow_quit);
-    return table;
-  }
-
-  dwarf2_per_cu_data *find_per_cu (dwarf2_per_bfd *per_bfd,
-				   CORE_ADDR adjusted_pc) override;
-
-  struct compunit_symtab *find_compunit_symtab_by_address
-    (struct objfile *objfile, CORE_ADDR address) override;
-
-  bool has_unexpanded_symtabs (struct objfile *objfile) override
-  {
-    wait (objfile, true);
-    return dwarf2_base_index_functions::has_unexpanded_symtabs (objfile);
-  }
-
-  struct symtab *find_last_source_symtab (struct objfile *objfile) override
-  {
-    wait (objfile, true);
-    return dwarf2_base_index_functions::find_last_source_symtab (objfile);
-  }
-
-  void forget_cached_source_info (struct objfile *objfile) override
-  {
-    wait (objfile, true);
-    dwarf2_base_index_functions::forget_cached_source_info (objfile);
-  }
-
-  void print_stats (struct objfile *objfile, bool print_bcache) override
-  {
-    wait (objfile, true);
-    dwarf2_base_index_functions::print_stats (objfile, print_bcache);
-  }
-
-  void dump (struct objfile *objfile) override
-  {
-    cooked_index *index = wait (objfile, true);
-    gdb_printf ("Cooked index in use:\n");
-    gdb_printf ("\n");
-    index->dump (objfile->arch ());
-  }
-
-  void expand_all_symtabs (struct objfile *objfile) override
-  {
-    wait (objfile, true);
-    dwarf2_base_index_functions::expand_all_symtabs (objfile);
-  }
-
-  bool expand_symtabs_matching
-    (struct objfile *objfile,
-     gdb::function_view<expand_symtabs_file_matcher_ftype> file_matcher,
-     const lookup_name_info *lookup_name,
-     gdb::function_view<expand_symtabs_symbol_matcher_ftype> symbol_matcher,
-     gdb::function_view<expand_symtabs_exp_notify_ftype> expansion_notify,
-     block_search_flags search_flags,
-     domain_enum domain,
-     enum search_domain kind) override;
-
-  struct compunit_symtab *find_pc_sect_compunit_symtab
-    (struct objfile *objfile, struct bound_minimal_symbol msymbol,
-     CORE_ADDR pc, struct obj_section *section, int warn_if_readin) override
-  {
-    wait (objfile, true);
-    return (dwarf2_base_index_functions::find_pc_sect_compunit_symtab
-	    (objfile, msymbol, pc, section, warn_if_readin));
-  }
-
-  void map_symbol_filenames
-       (struct objfile *objfile,
-	gdb::function_view<symbol_filename_ftype> fun,
-	bool need_fullname) override
-  {
-    wait (objfile, true);
-    return (dwarf2_base_index_functions::map_symbol_filenames
-	    (objfile, fun, need_fullname));
-  }
-
-  void compute_main_name (struct objfile *objfile) override
-  {
-    wait (objfile, false);
-  }
-};
-
 dwarf2_per_cu_data *
 cooked_index_functions::find_per_cu (dwarf2_per_bfd *per_bfd,
 				     CORE_ADDR adjusted_pc)
@@ -16827,12 +16733,6 @@ start_debug_info_reader (dwarf2_per_objfile *per_objfile)
   idx->start_reading ();
 }
 
-quick_symbol_functions_up
-cooked_index::make_quick_functions () const
-{
-  return quick_symbol_functions_up (new cooked_index_functions);
-}
-
 \f
 
 /* Read the .debug_loclists or .debug_rnglists header (they are the same format)

-- 
2.43.0


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 08/17] Do not write the index cache from an index
  2023-12-10 16:44 [PATCH 00/17] Rewrite .debug_names reader and writer Tom Tromey
                   ` (6 preceding siblings ...)
  2023-12-10 16:44 ` [PATCH 07/17] Move cooked_index_functions to cooked-index.h Tom Tromey
@ 2023-12-10 16:44 ` Tom Tromey
  2023-12-10 16:44 ` [PATCH 09/17] Change cooked_index_worker to abstract base class Tom Tromey
                   ` (8 subsequent siblings)
  16 siblings, 0 replies; 22+ messages in thread
From: Tom Tromey @ 2023-12-10 16:44 UTC (permalink / raw)
  To: gdb-patches

The new .debug_names reader will work by creating a cooked index from
.debug_names.  This patch updates cooked_index::maybe_write_index to
avoid writing the index in this case.

However, in order to do this in a clean way, the readers are changed
so that a nullptr result from index_for_writing means "cannot be
done", and then the error message is moved into write_dwarf_index
(where it historically lived).
---
 gdb/dwarf2/cooked-index.c | 7 +++++--
 gdb/dwarf2/index-write.c  | 2 ++
 gdb/dwarf2/mapped-index.h | 4 +---
 3 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/gdb/dwarf2/cooked-index.c b/gdb/dwarf2/cooked-index.c
index 684773cb192..a2bfe8e7b88 100644
--- a/gdb/dwarf2/cooked-index.c
+++ b/gdb/dwarf2/cooked-index.c
@@ -699,8 +699,11 @@ void
 cooked_index::maybe_write_index (dwarf2_per_bfd *per_bfd,
 				 const index_cache_store_context &ctx)
 {
-  /* (maybe) store an index in the cache.  */
-  global_index_cache.store (m_per_bfd, ctx);
+  if (index_for_writing () != nullptr)
+    {
+      /* (maybe) store an index in the cache.  */
+      global_index_cache.store (m_per_bfd, ctx);
+    }
   m_state->set (cooked_state::CACHE_DONE);
 }
 
diff --git a/gdb/dwarf2/index-write.c b/gdb/dwarf2/index-write.c
index 44728e4216d..c23fa25e021 100644
--- a/gdb/dwarf2/index-write.c
+++ b/gdb/dwarf2/index-write.c
@@ -1620,6 +1620,8 @@ write_dwarf_index (dwarf2_per_bfd *per_bfd, const char *dir,
   if (per_bfd->index_table == nullptr)
     error (_("No debugging symbols"));
   cooked_index *table = per_bfd->index_table->index_for_writing ();
+  if (table == nullptr)
+    error (_("Cannot use an index to create the index"));
 
   if (per_bfd->types.size () > 1)
     error (_("Cannot make an index when the file has multiple .debug_types sections"));
diff --git a/gdb/dwarf2/mapped-index.h b/gdb/dwarf2/mapped-index.h
index fb15da9f8d5..d01f4df4622 100644
--- a/gdb/dwarf2/mapped-index.h
+++ b/gdb/dwarf2/mapped-index.h
@@ -127,9 +127,7 @@ struct mapped_index_base : public dwarf_scanner_base
 				 dwarf2_per_objfile *per_objfile) const;
 
   cooked_index *index_for_writing () override
-  {
-    error (_("Cannot use an index to create the index"));
-  }
+  { return nullptr; }
 };
 
 #endif /* GDB_DWARF2_MAPPED_INDEX_H */

-- 
2.43.0


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 09/17] Change cooked_index_worker to abstract base class
  2023-12-10 16:44 [PATCH 00/17] Rewrite .debug_names reader and writer Tom Tromey
                   ` (7 preceding siblings ...)
  2023-12-10 16:44 ` [PATCH 08/17] Do not write the index cache from an index Tom Tromey
@ 2023-12-10 16:44 ` Tom Tromey
  2023-12-10 16:44 ` [PATCH 10/17] Remove cooked_index_worker::start_reading Tom Tromey
                   ` (7 subsequent siblings)
  16 siblings, 0 replies; 22+ messages in thread
From: Tom Tromey @ 2023-12-10 16:44 UTC (permalink / raw)
  To: gdb-patches

This changes cooked_index_worker to be an abstract base class.  The
base class implementation is moved to cooked-index.c, and a concrete
subclass is added to read.c.

This change is preparation for the new .debug_names reader, which will
supply its own concrete implementation of the worker.
---
 gdb/dwarf2/cooked-index.c | 132 ++++++++++++++++++++++++++++++++-
 gdb/dwarf2/cooked-index.h |  43 ++++++-----
 gdb/dwarf2/read.c         | 182 +++++++++++++---------------------------------
 3 files changed, 200 insertions(+), 157 deletions(-)

diff --git a/gdb/dwarf2/cooked-index.c b/gdb/dwarf2/cooked-index.c
index a2bfe8e7b88..604e7526c0f 100644
--- a/gdb/dwarf2/cooked-index.c
+++ b/gdb/dwarf2/cooked-index.c
@@ -440,9 +440,137 @@ cooked_index_shard::find (const std::string &name, bool completing) const
   return range (lower, upper);
 }
 
+/* See cooked-index.h.  */
+
+void
+cooked_index_worker::start ()
+{
+  gdb::thread_pool::g_thread_pool->post_task ([=] ()
+  {
+    this->start_reading ();
+  });
+}
+
+/* See cooked-index.h.  */
+
+void
+cooked_index_worker::start_reading ()
+{
+  SCOPE_EXIT { bfd_thread_cleanup (); };
+
+  try
+    {
+      do_reading ();
+    }
+  catch (const gdb_exception &exc)
+    {
+      m_failed = exc;
+      set (cooked_state::CACHE_DONE);
+    }
+}
+
+/* See cooked-index.h.  */
+
+bool
+cooked_index_worker::wait (cooked_state desired_state, bool allow_quit)
+{
+  bool done;
+#if CXX_STD_THREAD
+  {
+    std::unique_lock<std::mutex> lock (m_mutex);
+
+    /* This may be called from a non-main thread -- this functionality
+       is needed for the index cache -- but in this case we require
+       that the desired state already have been attained.  */
+    gdb_assert (is_main_thread () || desired_state <= m_state);
+
+    while (desired_state > m_state)
+      {
+	if (allow_quit)
+	  {
+	    std::chrono::milliseconds duration { 15 };
+	    if (m_cond.wait_for (lock, duration) == std::cv_status::timeout)
+	      QUIT;
+	  }
+	else
+	  m_cond.wait (lock);
+      }
+    done = m_state == cooked_state::CACHE_DONE;
+  }
+#else
+  /* Without threads, all the work is done immediately on the main
+     thread, and there is never anything to wait for.  */
+  done = true;
+#endif /* CXX_STD_THREAD */
+
+  /* Only the main thread is allowed to report complaints and the
+     like.  */
+  if (!is_main_thread ())
+    return false;
+
+  if (m_reported)
+    return done;
+  m_reported = true;
+
+  if (m_failed.has_value ())
+    {
+      /* start_reading failed -- report it.  */
+      exception_print (gdb_stderr, *m_failed);
+      m_failed.reset ();
+      return done;
+    }
+
+  /* Only show a given exception a single time.  */
+  std::unordered_set<gdb_exception> seen_exceptions;
+  for (auto &one_result : m_results)
+    {
+      re_emit_complaints (std::get<1> (one_result));
+      for (auto &one_exc : std::get<2> (one_result))
+	if (seen_exceptions.insert (one_exc).second)
+	  exception_print (gdb_stderr, one_exc);
+    }
+
+  print_stats ();
+
+  struct objfile *objfile = m_per_objfile->objfile;
+  dwarf2_per_bfd *per_bfd = m_per_objfile->per_bfd;
+  cooked_index *table
+    = (gdb::checked_static_cast<cooked_index *>
+       (per_bfd->index_table.get ()));
+
+  auto_obstack temp_storage;
+  enum language lang = language_unknown;
+  const char *main_name = table->get_main_name (&temp_storage, &lang);
+  if (main_name != nullptr)
+    set_objfile_main_name (objfile, main_name, lang);
+
+  /* dwarf_read_debug_printf ("Done building psymtabs of %s", */
+  /* 			   objfile_name (objfile)); */
+
+  return done;
+}
+
+/* See cooked-index.h.  */
+
+void
+cooked_index_worker::set (cooked_state desired_state)
+{
+  gdb_assert (desired_state != cooked_state::INITIAL);
+
+#if CXX_STD_THREAD
+  std::lock_guard<std::mutex> guard (m_mutex);
+  gdb_assert (desired_state > m_state);
+  m_state = desired_state;
+  m_cond.notify_one ();
+#else
+  /* Without threads, all the work is done immediately on the main
+     thread, and there is never anything to do.  */
+#endif /* CXX_STD_THREAD */
+}
 
-cooked_index::cooked_index (dwarf2_per_objfile *per_objfile)
-  : m_state (std::make_unique<cooked_index_worker> (per_objfile)),
+cooked_index::cooked_index (dwarf2_per_objfile *per_objfile,
+			    std::unique_ptr<cooked_index_worker> &&worker)
+  : m_state (std::move (worker)),
     m_per_bfd (per_objfile->per_bfd)
 {
   /* ACTIVE_VECTORS is not locked, and this assert ensures that this
diff --git a/gdb/dwarf2/cooked-index.h b/gdb/dwarf2/cooked-index.h
index 62230cf429b..ffb7bf71e81 100644
--- a/gdb/dwarf2/cooked-index.h
+++ b/gdb/dwarf2/cooked-index.h
@@ -446,13 +446,21 @@ enum class cooked_state
 
 /* An object of this type controls the scanning of the DWARF.  It
    schedules the worker tasks and tracks the current state.  Once
-   scanning is done, this object is discarded.  */
+   scanning is done, this object is discarded.
+   
+   This is an abstract base class that defines the basic behavior of
+   scanners.  Separate concrete implementations exist for scanning
+   .debug_names and .debug_info.  */
 
 class cooked_index_worker
 {
 public:
 
-  explicit cooked_index_worker (dwarf2_per_objfile *per_objfile);
+  explicit cooked_index_worker (dwarf2_per_objfile *per_objfile)
+    : m_per_objfile (per_objfile)
+  { }
+  virtual ~cooked_index_worker ()
+  { }
   DISABLE_COPY_AND_ASSIGN (cooked_index_worker);
 
   /* Start reading.  */
@@ -466,7 +474,7 @@ class cooked_index_worker
      cache writer.)  */
   bool wait (cooked_state desired_state, bool allow_quit);
 
-private:
+protected:
 
   /* Let cooked_index call the 'set' method.  */
   friend class cooked_index;
@@ -476,21 +484,15 @@ class cooked_index_worker
      problems.  */
   void start_reading ();
 
-  /* Helper function that does most of the work for start_reading.  */
-  void do_reading ();
-
-  /* After the last DWARF-reading task has finished, this function
-     does the remaining work to finish the scan.  */
-  void done_reading ();
-
-  /* An iterator for the comp units.  */
-  typedef std::vector<dwarf2_per_cu_data_up>::iterator unit_iterator;
+  /* Helper function that does most of the work for start_reading.
+     This must be able to be run in a worker thread without
+     problems.  */
+  virtual void do_reading () = 0;
 
-  /* Process a batch of CUs.  This may be called multiple times in
-     separate threads.  TASK_NUMBER indicates which task this is --
-     the result is stored in that slot of M_RESULTS.  */
-  void process_cus (size_t task_number, unit_iterator first,
- 		    unit_iterator end);
+  /* A callback that can print stats, if needed.  This is called when
+     transitioning to the 'MAIN_AVAILABLE' state.  */
+  virtual void print_stats ()
+  { }
 
   /* Each thread returns a tuple holding a cooked index, any collected
      complaints, and a vector of errors that should be printed.  The
@@ -503,10 +505,6 @@ class cooked_index_worker
 
   /* The per-objfile object.  */
   dwarf2_per_objfile *m_per_objfile;
-  /* A storage object for "leftovers" -- see the 'start' method, but
-     essentially things not parsed during the normal CU parsing
-     passes.  */
-  cooked_index_storage m_index_storage;
   /* Result of each worker task.  */
   std::vector<result_type> m_results;
 
@@ -588,7 +586,8 @@ class cooked_index : public dwarf_scanner_base
      object.  */
   using vec_type = std::vector<std::unique_ptr<cooked_index_shard>>;
 
-  explicit cooked_index (dwarf2_per_objfile *per_objfile);
+  cooked_index (dwarf2_per_objfile *per_objfile,
+		std::unique_ptr<cooked_index_worker> &&worker);
   ~cooked_index () override;
 
   DISABLE_COPY_AND_ASSIGN (cooked_index);
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index acdb1969915..2b553bc5ac5 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -4834,32 +4834,58 @@ process_skeletonless_type_units (dwarf2_per_objfile *per_objfile,
     }
 }
 
-cooked_index_worker::cooked_index_worker (dwarf2_per_objfile *per_objfile)
-  : m_per_objfile (per_objfile)
+/* A subclass of cooked_index_worker that handles scanning
+   .debug_info.  */
+
+class cooked_index_debug_info : public cooked_index_worker
 {
-  gdb_assert (is_main_thread ());
+public:
+  cooked_index_debug_info (dwarf2_per_objfile *per_objfile)
+    : cooked_index_worker (per_objfile)
+  {
+    gdb_assert (is_main_thread ());
 
-  struct objfile *objfile = per_objfile->objfile;
-  dwarf2_per_bfd *per_bfd = per_objfile->per_bfd;
+    struct objfile *objfile = per_objfile->objfile;
+    dwarf2_per_bfd *per_bfd = per_objfile->per_bfd;
 
-  dwarf_read_debug_printf ("Building psymtabs of objfile %s ...",
-			   objfile_name (objfile));
+    dwarf_read_debug_printf ("Building psymtabs of objfile %s ...",
+			     objfile_name (objfile));
 
-  per_bfd->map_info_sections (objfile);
-}
+    per_bfd->map_info_sections (objfile);
+  }
 
-void
-cooked_index_worker::start ()
-{
-  gdb::thread_pool::g_thread_pool->post_task ([=] ()
+private:
+
+  void do_reading () override;
+
+  void print_stats () override
   {
-    this->start_reading ();
-  });
-}
+    if (dwarf_read_debug > 0)
+      print_tu_stats (m_per_objfile);
+  }
+
+  /* After the last DWARF-reading task has finished, this function
+     does the remaining work to finish the scan.  */
+  void done_reading ();
+
+  /* An iterator for the comp units.  */
+  typedef std::vector<dwarf2_per_cu_data_up>::iterator unit_iterator;
+
+  /* Process a batch of CUs.  This may be called multiple times in
+     separate threads.  TASK_NUMBER indicates which task this is --
+     the result is stored in that slot of M_RESULTS.  */
+  void process_cus (size_t task_number, unit_iterator first,
+ 		    unit_iterator end);
+
+  /* A storage object for "leftovers" -- see the 'start' method, but
+     essentially things not parsed during the normal CU parsing
+     passes.  */
+  cooked_index_storage m_index_storage;
+};
 
 void
-cooked_index_worker::process_cus (size_t task_number, unit_iterator first,
-				  unit_iterator end)
+cooked_index_debug_info::process_cus (size_t task_number, unit_iterator first,
+				      unit_iterator end)
 {
   SCOPE_EXIT { bfd_thread_cleanup (); };
 
@@ -4887,7 +4913,7 @@ cooked_index_worker::process_cus (size_t task_number, unit_iterator first,
 }
 
 void
-cooked_index_worker::done_reading ()
+cooked_index_debug_info::done_reading ()
 {
   /* Only handle the scanning results here.  Complaints and exceptions
      can only be dealt with on the main thread.  */
@@ -4909,23 +4935,7 @@ cooked_index_worker::done_reading ()
 }
 
 void
-cooked_index_worker::start_reading ()
-{
-  SCOPE_EXIT { bfd_thread_cleanup (); };
-
-  try
-    {
-      do_reading ();
-    }
-  catch (const gdb_exception &exc)
-    {
-      m_failed = exc;
-      set (cooked_state::CACHE_DONE);
-    }
-}
-
-void
-cooked_index_worker::do_reading ()
+cooked_index_debug_info::do_reading ()
 {
   dwarf2_per_bfd *per_bfd = m_per_objfile->per_bfd;
 
@@ -4995,102 +5005,6 @@ cooked_index_worker::do_reading ()
   workers.start ();
 }
 
-bool
-cooked_index_worker::wait (cooked_state desired_state, bool allow_quit)
-{
-  bool done;
-#if CXX_STD_THREAD
-  {
-    std::unique_lock<std::mutex> lock (m_mutex);
-
-    /* This may be called from a non-main thread -- this functionality
-       is needed for the index cache -- but in this case we require
-       that the desired state already have been attained.  */
-    gdb_assert (is_main_thread () || desired_state <= m_state);
-
-    while (desired_state > m_state)
-      {
-	if (allow_quit)
-	  {
-	    std::chrono::milliseconds duration { 15 };
-	    if (m_cond.wait_for (lock, duration) == std::cv_status::timeout)
-	      QUIT;
-	  }
-	else
-	  m_cond.wait (lock);
-      }
-    done = m_state == cooked_state::CACHE_DONE;
-  }
-#else
-  /* Without threads, all the work is done immediately on the main
-     thread, and there is never anything to wait for.  */
-  done = true;
-#endif /* CXX_STD_THREAD */
-
-  /* Only the main thread is allowed to report complaints and the
-     like.  */
-  if (!is_main_thread ())
-    return false;
-
-  if (m_reported)
-    return done;
-  m_reported = true;
-
-  if (m_failed.has_value ())
-    {
-      /* start_reading failed -- report it.  */
-      exception_print (gdb_stderr, *m_failed);
-      m_failed.reset ();
-      return done;
-    }
-
-  /* Only show a given exception a single time.  */
-  std::unordered_set<gdb_exception> seen_exceptions;
-  for (auto &one_result : m_results)
-    {
-      re_emit_complaints (std::get<1> (one_result));
-      for (auto &one_exc : std::get<2> (one_result))
-	if (seen_exceptions.insert (one_exc).second)
-	  exception_print (gdb_stderr, one_exc);
-    }
-
-  if (dwarf_read_debug > 0)
-    print_tu_stats (m_per_objfile);
-
-  struct objfile *objfile = m_per_objfile->objfile;
-  dwarf2_per_bfd *per_bfd = m_per_objfile->per_bfd;
-  cooked_index *table
-    = (gdb::checked_static_cast<cooked_index *>
-       (per_bfd->index_table.get ()));
-
-  auto_obstack temp_storage;
-  enum language lang = language_unknown;
-  const char *main_name = table->get_main_name (&temp_storage, &lang);
-  if (main_name != nullptr)
-    set_objfile_main_name (objfile, main_name, lang);
-
-  dwarf_read_debug_printf ("Done building psymtabs of %s",
-			   objfile_name (objfile));
-
-  return done;
-}
-
-void
-cooked_index_worker::set (cooked_state desired_state)
-{
-  gdb_assert (desired_state != cooked_state::INITIAL);
-
-#if CXX_STD_THREAD
-  std::lock_guard<std::mutex> guard (m_mutex);
-  gdb_assert (desired_state > m_state);
-  m_state = desired_state;
-  m_cond.notify_one ();
-#else
-  /* Without threads, all the work is done immediately on the main
-     thread, and there is never anything to do.  */
-#endif /* CXX_STD_THREAD */
-}
-
 static void
 read_comp_units_from_section (dwarf2_per_objfile *per_objfile,
 			      struct dwarf2_section_info *section,
@@ -16726,7 +16640,9 @@ start_debug_info_reader (dwarf2_per_objfile *per_objfile)
   /* Set the index table early so that sharing works even while
      scanning; and then start the scanning.  */
   dwarf2_per_bfd *per_bfd = per_objfile->per_bfd;
-  cooked_index *idx = new cooked_index (per_objfile);
+  std::unique_ptr<cooked_index_worker> worker
+    (new cooked_index_debug_info (per_objfile));
+  cooked_index *idx = new cooked_index (per_objfile, std::move (worker));
   per_bfd->index_table.reset (idx);
   /* Don't start reading until after 'index_table' is set.  This
      avoids races.  */

-- 
2.43.0


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 10/17] Remove cooked_index_worker::start_reading
  2023-12-10 16:44 [PATCH 00/17] Rewrite .debug_names reader and writer Tom Tromey
                   ` (8 preceding siblings ...)
  2023-12-10 16:44 ` [PATCH 09/17] Change cooked_index_worker to abstract base class Tom Tromey
@ 2023-12-10 16:44 ` Tom Tromey
  2023-12-10 16:45 ` [PATCH 11/17] Empty hash table fix in .debug_names reader Tom Tromey
                   ` (6 subsequent siblings)
  16 siblings, 0 replies; 22+ messages in thread
From: Tom Tromey @ 2023-12-10 16:44 UTC (permalink / raw)
  To: gdb-patches

I noticed that cooked_index_worker::start_reading isn't really needed.
This patch removes it, and also removes the SCOPED_EXIT, in favor of a
direct call.
---
 gdb/dwarf2/cooked-index.c | 32 ++++++++++++--------------------
 gdb/dwarf2/cooked-index.h | 15 +++++----------
 2 files changed, 17 insertions(+), 30 deletions(-)

diff --git a/gdb/dwarf2/cooked-index.c b/gdb/dwarf2/cooked-index.c
index 604e7526c0f..ef4b3bf21fd 100644
--- a/gdb/dwarf2/cooked-index.c
+++ b/gdb/dwarf2/cooked-index.c
@@ -447,26 +447,18 @@ cooked_index_worker::start ()
 {
   gdb::thread_pool::g_thread_pool->post_task ([=] ()
   {
-    this->start_reading ();
-  });
-}
-
-/* See cooked-index.h.  */
-
-void
-cooked_index_worker::start_reading ()
-{
-  SCOPE_EXIT { bfd_thread_cleanup (); };
+    try
+      {
+	do_reading ();
+      }
+    catch (const gdb_exception &exc)
+      {
+	m_failed = exc;
+	set (cooked_state::CACHE_DONE);
+      }
 
-  try
-    {
-      do_reading ();
-    }
-  catch (const gdb_exception &exc)
-    {
-      m_failed = exc;
-      set (cooked_state::CACHE_DONE);
-    }
+    bfd_thread_cleanup ();
+  });
 }
 
 /* See cooked-index.h.  */
@@ -514,7 +506,7 @@ cooked_index_worker::wait (cooked_state desired_state, bool allow_quit)
 
   if (m_failed.has_value ())
     {
-      /* start_reading failed -- report it.  */
+      /* do_reading failed -- report it.  */
       exception_print (gdb_stderr, *m_failed);
       m_failed.reset ();
       return done;
diff --git a/gdb/dwarf2/cooked-index.h b/gdb/dwarf2/cooked-index.h
index ffb7bf71e81..b76af00af29 100644
--- a/gdb/dwarf2/cooked-index.h
+++ b/gdb/dwarf2/cooked-index.h
@@ -480,13 +480,8 @@ class cooked_index_worker
   friend class cooked_index;
   void set (cooked_state desired_state);
 
-  /* Start reading DWARF.  This can be run in a worker thread without
-     problems.  */
-  void start_reading ();
-
-  /* Helper function that does most of the work for start_reading.
-     This must be able to be run in a worker thread without
-     problems.  */
+  /* Helper function that does the work of reading.  This must be able
+     to be run in a worker thread without problems.  */
   virtual void do_reading () = 0;
 
   /* A callback that can print stats, if needed.  This is called when
@@ -518,9 +513,9 @@ class cooked_index_worker
   /* Mutex and condition variable used to synchronize.  */
   std::mutex m_mutex;
   std::condition_variable m_cond;
-  /* If set, an exception occurred during start_reading; in this case
-     the scanning is stopped and this exception will later be reported
-     by the 'wait' method.  */
+  /* If set, an exception occurred during reading; in this case the
+     scanning is stopped and this exception will later be reported by
+     the 'wait' method.  */
   std::optional<gdb_exception> m_failed;
 #endif /* CXX_STD_THREAD */
 };

-- 
2.43.0


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 11/17] Empty hash table fix in .debug_names reader
  2023-12-10 16:44 [PATCH 00/17] Rewrite .debug_names reader and writer Tom Tromey
                   ` (9 preceding siblings ...)
  2023-12-10 16:44 ` [PATCH 10/17] Remove cooked_index_worker::start_reading Tom Tromey
@ 2023-12-10 16:45 ` Tom Tromey
  2023-12-10 16:45 ` [PATCH 12/17] Fix dw2-zero-range.exp when an index is in use Tom Tromey
                   ` (5 subsequent siblings)
  16 siblings, 0 replies; 22+ messages in thread
From: Tom Tromey @ 2023-12-10 16:45 UTC (permalink / raw)
  To: gdb-patches

The handling of an empty hash table in the .debug_names reader is
slightly wrong.

Currently the code assumes there is always an array of hashes.
However, section 6.1.1.4.5 Hash Lookup Table says:

    The optional hash lookup table immediately follows the list of
    type signatures.

and then:

    The hash lookup table is actually two separate arrays: an array of
    buckets, followed immediately by an array of hashes.

My reading of this is that the hash table as a whole is optional, and
so the hashes will not exist in this case.  (This also makes sense
because the hashes are not useful without the buckets anyway.)

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=25950
---
 gdb/dwarf2/read-debug-names.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/gdb/dwarf2/read-debug-names.c b/gdb/dwarf2/read-debug-names.c
index e912c2a1114..c6b70550f81 100644
--- a/gdb/dwarf2/read-debug-names.c
+++ b/gdb/dwarf2/read-debug-names.c
@@ -290,7 +290,8 @@ read_debug_names_from_section (struct objfile *objfile,
   map.bucket_table_reordered = reinterpret_cast<const uint32_t *> (addr);
   addr += map.bucket_count * 4;
   map.hash_table_reordered = reinterpret_cast<const uint32_t *> (addr);
-  addr += map.name_count * 4;
+  if (map.bucket_count != 0)
+    addr += map.name_count * 4;
 
   /* Name Table */
   map.name_table_string_offs_reordered = addr;

-- 
2.43.0


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 12/17] Fix dw2-zero-range.exp when an index is in use
  2023-12-10 16:44 [PATCH 00/17] Rewrite .debug_names reader and writer Tom Tromey
                   ` (10 preceding siblings ...)
  2023-12-10 16:45 ` [PATCH 11/17] Empty hash table fix in .debug_names reader Tom Tromey
@ 2023-12-10 16:45 ` Tom Tromey
  2023-12-10 16:45 ` [PATCH 13/17] Explicitly expand CUs in dw2-inline-with-lexical-scope.exp Tom Tromey
                   ` (4 subsequent siblings)
  16 siblings, 0 replies; 22+ messages in thread
From: Tom Tromey @ 2023-12-10 16:45 UTC (permalink / raw)
  To: gdb-patches

dw2-zero-range.exp looks for a certain complaint, but this won't be
issued when an index is in use.  This patch changes the test to not
fail in this case.
---
 gdb/testsuite/gdb.dwarf2/dw2-zero-range.exp | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/gdb/testsuite/gdb.dwarf2/dw2-zero-range.exp b/gdb/testsuite/gdb.dwarf2/dw2-zero-range.exp
index 38c85a47b80..c6a1498e967 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-zero-range.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-zero-range.exp
@@ -132,7 +132,10 @@ foreach_with_prefix ranges_sect {ranges rnglists} {
 	    }
 	}
 
-	gdb_assert { $have_complaint } $test
+	# The complaint won't be seen if an index is in use.
+	if {[have_index $lib1] == ""} {
+	    gdb_assert { $have_complaint } $test
+	}
     }
 
     if { ! $readnow_p } {
@@ -149,7 +152,10 @@ foreach_with_prefix ranges_sect {ranges rnglists} {
 	gdb_load $lib1
 	set test "Zero address complaint - unrelocated - psymtab"
 	set have_complaint [regexp $re.* $gdb_file_cmd_msg]
-	gdb_assert { $have_complaint } $test
+	# The complaint won't be seen if an index is in use.
+	if {[have_index $lib1] == ""} {
+	    gdb_assert { $have_complaint } $test
+	}
     }
     gdb_test_no_output "maint set dwarf synchronous off"
 

-- 
2.43.0


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 13/17] Explicitly expand CUs in dw2-inline-with-lexical-scope.exp
  2023-12-10 16:44 [PATCH 00/17] Rewrite .debug_names reader and writer Tom Tromey
                   ` (11 preceding siblings ...)
  2023-12-10 16:45 ` [PATCH 12/17] Fix dw2-zero-range.exp when an index is in use Tom Tromey
@ 2023-12-10 16:45 ` Tom Tromey
  2023-12-10 16:45 ` [PATCH 14/17] Remove some .debug_names tests Tom Tromey
                   ` (3 subsequent siblings)
  16 siblings, 0 replies; 22+ messages in thread
From: Tom Tromey @ 2023-12-10 16:45 UTC (permalink / raw)
  To: gdb-patches

dw2-inline-with-lexical-scope.exp relies on the main CU being
expanded.  However, it doesn't guarantee that this actually happens,
and with the new .debug_names reader, it won't, because the "main"
program will be found in the index without requiring CU expansion.

This patch fixes the problem by explicitly expanding the CU in
question.

Note that this is an artificial bug -- it occurs because the generated
.debug_aranges isn't correct.
---
 gdb/testsuite/gdb.dwarf2/dw2-inline-with-lexical-scope.exp | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/gdb/testsuite/gdb.dwarf2/dw2-inline-with-lexical-scope.exp b/gdb/testsuite/gdb.dwarf2/dw2-inline-with-lexical-scope.exp
index fb1c5605e9b..d62d06b43c5 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-inline-with-lexical-scope.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-inline-with-lexical-scope.exp
@@ -133,5 +133,9 @@ if {[prepare_for_testing "failed to prepare" ${testfile} \
 
 runto breakpoint_label
 
+# When using cc-with-debug-names, nothing will force the CU to be
+# expanded.  Do it manually.
+gdb_test_no_output "maint expand-symtabs [file tail $srcfile]"
+
 # Bad GDB was printing an additional "value = <optimized out>".
 gdb_test "info locals" "value = 42\r\nnum = 42"

-- 
2.43.0


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 14/17] Remove some .debug_names tests
  2023-12-10 16:44 [PATCH 00/17] Rewrite .debug_names reader and writer Tom Tromey
                   ` (12 preceding siblings ...)
  2023-12-10 16:45 ` [PATCH 13/17] Explicitly expand CUs in dw2-inline-with-lexical-scope.exp Tom Tromey
@ 2023-12-10 16:45 ` Tom Tromey
  2023-12-10 16:45 ` [PATCH 15/17] Rewrite .debug_names reader Tom Tromey
                   ` (2 subsequent siblings)
  16 siblings, 0 replies; 22+ messages in thread
From: Tom Tromey @ 2023-12-10 16:45 UTC (permalink / raw)
  To: gdb-patches

These .debug_names tests were hand-written to mimic clang.  However,
they are difficult to update, and in any case the new reader won't
accept clang-generated indices.  Therefore this patch removes these
tests.
---
 gdb/testsuite/gdb.dwarf2/clang-debug-names-2-foo.c |  22 ----
 gdb/testsuite/gdb.dwarf2/clang-debug-names-2.c     |  27 -----
 gdb/testsuite/gdb.dwarf2/clang-debug-names-2.exp   |  42 -------
 gdb/testsuite/gdb.dwarf2/clang-debug-names.c       |  25 -----
 gdb/testsuite/gdb.dwarf2/clang-debug-names.exp     |  42 -------
 gdb/testsuite/gdb.dwarf2/clang-debug-names.exp.tcl | 121 ---------------------
 6 files changed, 279 deletions(-)

diff --git a/gdb/testsuite/gdb.dwarf2/clang-debug-names-2-foo.c b/gdb/testsuite/gdb.dwarf2/clang-debug-names-2-foo.c
deleted file mode 100644
index aff6d60d861..00000000000
--- a/gdb/testsuite/gdb.dwarf2/clang-debug-names-2-foo.c
+++ /dev/null
@@ -1,22 +0,0 @@
-/* This testcase is part of GDB, the GNU debugger.
-
-   Copyright 2020-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/>.  */
-
-int
-foo (void)
-{
-  return 3;
-}
diff --git a/gdb/testsuite/gdb.dwarf2/clang-debug-names-2.c b/gdb/testsuite/gdb.dwarf2/clang-debug-names-2.c
deleted file mode 100644
index 4887f95ed60..00000000000
--- a/gdb/testsuite/gdb.dwarf2/clang-debug-names-2.c
+++ /dev/null
@@ -1,27 +0,0 @@
-/* This testcase is part of GDB, the GNU debugger.
-
-   Copyright 2020-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/>.  */
-
-extern int foo (void);
-
-int
-main (void)
-{
-  asm ("main_label: .globl main_label");
-  int sum, a, b;
-  sum = a + b + foo ();
-  return sum;
-}
diff --git a/gdb/testsuite/gdb.dwarf2/clang-debug-names-2.exp b/gdb/testsuite/gdb.dwarf2/clang-debug-names-2.exp
deleted file mode 100644
index 7bc46ce6531..00000000000
--- a/gdb/testsuite/gdb.dwarf2/clang-debug-names-2.exp
+++ /dev/null
@@ -1,42 +0,0 @@
-# Copyright 2020-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/>.
-
-load_lib dwarf.exp
-
-# This test can only be run on targets which support DWARF-2 and use gas.
-require dwarf2_support
-
-standard_testfile .c -debug.S -foo.c
-
-lassign \
-    [function_range main \
-	 "${srcdir}/${subdir}/${srcfile} ${srcdir}/${subdir}/${srcfile3}"] \
-    main_start main_length
-
-set asm_file [standard_output_file $srcfile2]
-source $srcdir/$subdir/clang-debug-names.exp.tcl
-
-if { [build_executable_from_specs "failed to prepare" ${testfile} "" \
-	  $srcfile "nodebug" $asm_file "nodebug" $srcfile3 "debug"] } {
-    return -1
-}
-clean_restart $binfile
-
-set cmd "ptype main"
-set pass_re \
-    [multi_line \
-	 $cmd \
-	 "type = int \\(\\)"]
-gdb_test $cmd $pass_re
diff --git a/gdb/testsuite/gdb.dwarf2/clang-debug-names.c b/gdb/testsuite/gdb.dwarf2/clang-debug-names.c
deleted file mode 100644
index 12f184b0f85..00000000000
--- a/gdb/testsuite/gdb.dwarf2/clang-debug-names.c
+++ /dev/null
@@ -1,25 +0,0 @@
-/* This testcase is part of GDB, the GNU debugger.
-
-   Copyright 2020-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/>.  */
-
-int
-main (void)
-{
-  asm ("main_label: .globl main_label");
-  int sum, a, b;
-  sum = a + b;
-  return sum;
-}
diff --git a/gdb/testsuite/gdb.dwarf2/clang-debug-names.exp b/gdb/testsuite/gdb.dwarf2/clang-debug-names.exp
deleted file mode 100644
index 138bd86daee..00000000000
--- a/gdb/testsuite/gdb.dwarf2/clang-debug-names.exp
+++ /dev/null
@@ -1,42 +0,0 @@
-# Copyright 2020-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/>.
-
-load_lib dwarf.exp
-
-# This test can only be run on targets which support DWARF-2 and use gas.
-require dwarf2_support
-
-standard_testfile .c -debug.S
-
-lassign [function_range main ${srcdir}/${subdir}/${srcfile}] \
-    main_start main_length
-
-set asm_file [standard_output_file $srcfile2]
-source $srcdir/$subdir/clang-debug-names.exp.tcl
-
-if { [prepare_for_testing "failed to prepare" ${testfile} \
-	  [list $srcfile $asm_file] {nodebug}] } {
-    return -1
-}
-
-set test "no file command warnings"
-gdb_assert { [regexp "warning: " $gdb_file_cmd_msg] == 0 } $test
-
-set cmd "ptype main"
-set pass_re \
-    [multi_line \
-	 $cmd \
-	 "type = int \\(\\)"]
-gdb_test $cmd $pass_re
diff --git a/gdb/testsuite/gdb.dwarf2/clang-debug-names.exp.tcl b/gdb/testsuite/gdb.dwarf2/clang-debug-names.exp.tcl
deleted file mode 100644
index dca17fb86b7..00000000000
--- a/gdb/testsuite/gdb.dwarf2/clang-debug-names.exp.tcl
+++ /dev/null
@@ -1,121 +0,0 @@
-# Copyright 2020-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/>.
-
-# Set up the DWARF for the test.
-
-set main_str_label [Dwarf::_compute_label info_string3]
-set int_str_label [Dwarf::_compute_label info_string4]
-set main_die_label [Dwarf::_compute_label main_die_label]
-set int_die_label [Dwarf::_compute_label int_die_label]
-
-set debug_str \
-    [list \
-         "$main_str_label:" \
-         "  .asciz \"main\"" \
-         "$int_str_label:" \
-         "  .asciz \"int\""]
-
-set debug_names \
-    [list \
-         "  .4byte  .Ldebug_names_end - .Ldebug_names_start" \
-         ".Ldebug_names_start:" \
-         "  .short 5                     /* Header: version */" \
-         "  .short 0                     /* Header: padding */" \
-         "  .long 1                      /* Header: compilation unit count */" \
-         "  .long 0                      /* Header: local type unit count */" \
-         "  .long 0                      /* Header: foreign type unit count */" \
-         "  .long 2                      /* Header: bucket count */" \
-         "  .long 2                      /* Header: name count */" \
-         "  .long .Lnames_abbrev_end0-.Lnames_abbrev_start0 " \
-         "                               /* Header: abbreviation table size */" \
-         "  .long 8                      /* Header: augmentation string size */" \
-         "  .ascii \"LLVM0700\"   /* Header: augmentation string */" \
-         "  .long .Lcu1_begin            /* Compilation unit 0 */" \
-         "  .long 1                      /* Bucket 0 */" \
-         "  .long 0                      /* Bucket 1 */" \
-         "  .long 193495088              /* Hash in Bucket 0 */" \
-         "  .long 2090499946             /* Hash in Bucket 0 */" \
-         "  .long $int_str_label         /* String in Bucket 0: int */" \
-         "  .long $main_str_label        /* String in Bucket 0: main */" \
-         "  .long .Lnames1-.Lnames_entries0/* Offset in Bucket 0 */" \
-         "  .long .Lnames0-.Lnames_entries0/* Offset in Bucket 0 */" \
-         ".Lnames_abbrev_start0:" \
-         "  .byte 46                     /* Abbrev code */" \
-         "  .byte 46                     /* DW_TAG_subprogram */" \
-         "  .byte 3                      /* DW_IDX_die_offset */" \
-         "  .byte 19                     /* DW_FORM_ref4 */" \
-         "  .byte 0                      /* End of abbrev */" \
-         "  .byte 0                      /* End of abbrev */" \
-         "  .byte 36                     /* Abbrev code */" \
-         "  .byte 36                     /* DW_TAG_base_type */" \
-         "  .byte 3                      /* DW_IDX_die_offset */" \
-         "  .byte 19                     /* DW_FORM_ref4 */" \
-         "  .byte 0                      /* End of abbrev */" \
-         "  .byte 0                      /* End of abbrev */" \
-         "  .byte 0                      /* End of abbrev list */" \
-         ".Lnames_abbrev_end0:" \
-         ".Lnames_entries0:" \
-         ".Lnames1:" \
-         "  .byte 36                     /* Abbreviation code */" \
-         "  .long $int_die_label - .Lcu1_begin/* DW_IDX_die_offset */" \
-         "  .long 0                      /* End of list: int */" \
-         ".Lnames0:" \
-         "  .byte 46                     /* Abbreviation code */" \
-         "  .long $main_die_label - .Lcu1_begin/* DW_IDX_die_offset */" \
-         "  .long 0                      /* End of list: main */" \
-         "  .p2align 2" \
-         ".Ldebug_names_end:"]
-
-Dwarf::assemble $asm_file {
-    global srcdir subdir srcfile
-    global main_start main_length
-
-    cu {} {
-	DW_TAG_compile_unit {
-                {DW_AT_language @DW_LANG_C}
-                {DW_AT_name     clang-debug-names.c}
-                {DW_AT_comp_dir /tmp}
-
-        } {
-	    global int_die_label
-	    global main_die_label
-
-	    define_label $int_die_label
-	    base_type {
-		{name "int"}
-		{encoding @DW_ATE_signed}
-		{byte_size 4 DW_FORM_sdata}
-	    }
-
-	    define_label $main_die_label
-	    subprogram {
-		{name main}
-		{type :$int_die_label}
-		{low_pc $main_start addr}
-		{high_pc "$main_start + $main_length" addr}
-	    }
-	}
-    }
-
-    _defer_output .debug_str {
-	global debug_str
-	_emit [join $debug_str "\n"]
-    }
-
-    _defer_output .debug_names {
-	global debug_names
-	_emit [join $debug_names "\n"]
-    }
-}

-- 
2.43.0


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 15/17] Rewrite .debug_names reader
  2023-12-10 16:44 [PATCH 00/17] Rewrite .debug_names reader and writer Tom Tromey
                   ` (13 preceding siblings ...)
  2023-12-10 16:45 ` [PATCH 14/17] Remove some .debug_names tests Tom Tromey
@ 2023-12-10 16:45 ` Tom Tromey
  2023-12-10 16:45 ` [PATCH 16/17] Export dwarf5_augmentation Tom Tromey
  2023-12-10 16:45 ` [PATCH 17/17] Rewrite .debug_names writer Tom Tromey
  16 siblings, 0 replies; 22+ messages in thread
From: Tom Tromey @ 2023-12-10 16:45 UTC (permalink / raw)
  To: gdb-patches

This rewrites the .debug_names reader to follow the spec.

Since it was first written, gdb's .debug_names writer has been
incorrect -- while the form of the section has been ok, the contents
have been very gdb-specific.

This patch fixes the reader side of this equation, rewriting the
reader to create a cooked index internally -- an important detail
because it allows for the deletion of a lot of code, and it means the
various readers will agree more often.

This reader checks for a new augmentation string.  For the time being,
all other producers are ignored -- the old GDB ones because they are
wrong, and clang because it does not emit DW_IDX_parent.  (If there
are any other producers, I'm unaware of them.)

While the new reader mostly runs in a worker thread, it does not try
to distribute its work.  This could be done by partitioning the name
table.  The parent computations could also be done in parallel after
all names have been read.  I haven't attempted this.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=25950
---
 gdb/dwarf2/read-debug-names.c | 947 ++++++++++++++++--------------------------
 1 file changed, 368 insertions(+), 579 deletions(-)

diff --git a/gdb/dwarf2/read-debug-names.c b/gdb/dwarf2/read-debug-names.c
index c6b70550f81..1e53c246b91 100644
--- a/gdb/dwarf2/read-debug-names.c
+++ b/gdb/dwarf2/read-debug-names.c
@@ -20,6 +20,7 @@
 #include "defs.h"
 #include "read-debug-names.h"
 #include "dwarf2/aranges.h"
+#include "dwarf2/cooked-index.h"
 
 #include "complaints.h"
 #include "cp-support.h"
@@ -28,22 +29,62 @@
 #include "read.h"
 #include "stringify.h"
 
-/* A description of the mapped .debug_names.
-   Uninitialized map has CU_COUNT 0.  */
+/* This is just like cooked_index_functions, but overrides a single
+   method so the test suite can distinguish the .debug_names case from
+   the ordinary case.  */
+struct dwarf2_debug_names_index : public cooked_index_functions
+{
+  /* This dumps minimal information about .debug_names.  It is called
+     via "mt print objfiles".  The gdb.dwarf2/gdb-index.exp testcase
+     uses this to verify that .debug_names has been loaded.  */
+  void dump (struct objfile *objfile) override
+  {
+    gdb_printf (".debug_names: exists\n");
+    /* This could call the superclass method if that's useful.  */
+  }
+};
+
+/* This is like a cooked index, but as it has been ingested from
+   .debug_names, it can't be used to write out an index.  */
+class debug_names_index : public cooked_index
+{
+public:
+
+  using cooked_index::cooked_index;
+
+  cooked_index *index_for_writing () override
+  { return nullptr; }
+
+  quick_symbol_functions_up make_quick_functions () const override
+  { return quick_symbol_functions_up (new dwarf2_debug_names_index); }
+};
+
+/* A description of the mapped .debug_names.  */
 
-struct mapped_debug_names final : public mapped_index_base
+struct mapped_debug_names_reader
 {
-  bfd_endian dwarf5_byte_order;
-  bool dwarf5_is_dwarf64;
-  bool augmentation_is_gdb;
-  uint8_t offset_size;
+  const gdb_byte *scan_one_entry (const char *name,
+				  const gdb_byte *entry,
+				  cooked_index_entry **result,
+				  std::optional<ULONGEST> &parent);
+  void scan_entries (uint32_t index, const char *name, const gdb_byte *entry);
+  void scan_all_names ();
+
+  dwarf2_per_objfile *per_objfile = nullptr;
+  bfd *abfd = nullptr;
+  bfd_endian dwarf5_byte_order {};
+  bool dwarf5_is_dwarf64 = false;
+  bool augmentation_is_gdb = false;
+  uint8_t offset_size = 0;
   uint32_t cu_count = 0;
-  uint32_t tu_count, bucket_count, name_count;
-  const gdb_byte *cu_table_reordered, *tu_table_reordered;
-  const uint32_t *bucket_table_reordered, *hash_table_reordered;
-  const gdb_byte *name_table_string_offs_reordered;
-  const gdb_byte *name_table_entry_offs_reordered;
-  const gdb_byte *entry_pool;
+  uint32_t tu_count = 0, bucket_count = 0, name_count = 0;
+  const gdb_byte *cu_table_reordered = nullptr;
+  const gdb_byte *tu_table_reordered = nullptr;
+  const uint32_t *bucket_table_reordered = nullptr;
+  const uint32_t *hash_table_reordered = nullptr;
+  const gdb_byte *name_table_string_offs_reordered = nullptr;
+  const gdb_byte *name_table_entry_offs_reordered = nullptr;
+  const gdb_byte *entry_pool = nullptr;
 
   struct index_val
   {
@@ -64,41 +105,250 @@ struct mapped_debug_names final : public mapped_index_base
 
   std::unordered_map<ULONGEST, index_val> abbrev_map;
 
-  const char *namei_to_name
-    (uint32_t namei, dwarf2_per_objfile *per_objfile) const;
+  std::unique_ptr<cooked_index_shard> shard;
+  std::vector<std::pair<cooked_index_entry *, ULONGEST>> needs_parent;
+  std::vector<std::vector<cooked_index_entry *>> all_entries;
+};
 
-  /* Implementation of the mapped_index_base virtual interface, for
-     the name_components cache.  */
+/* Scan a single entry from the entries table.  Set *RESULT and PARENT
+   (if needed) and return the updated pointer on success, or return
+   nullptr on error, or at the end of the table.  */
 
-  const char *symbol_name_at
-    (offset_type idx, dwarf2_per_objfile *per_objfile) const override
-  { return namei_to_name (idx, per_objfile); }
+const gdb_byte *
+mapped_debug_names_reader::scan_one_entry (const char *name,
+					   const gdb_byte *entry,
+					   cooked_index_entry **result,
+					   std::optional<ULONGEST> &parent)
+{
+  unsigned int bytes_read;
+  const ULONGEST abbrev = read_unsigned_leb128 (abfd, entry, &bytes_read);
+  entry += bytes_read;
+  if (abbrev == 0)
+    return nullptr;
 
-  size_t symbol_name_count () const override
-  { return this->name_count; }
+  const auto indexval_it = abbrev_map.find (abbrev);
+  if (indexval_it == abbrev_map.cend ())
+    {
+      complaint (_("Wrong .debug_names undefined abbrev code %s "
+		   "[in module %s]"),
+		 pulongest (abbrev), bfd_get_filename (abfd));
+      return nullptr;
+    }
 
-  quick_symbol_functions_up make_quick_functions () const override;
-};
+  const auto &indexval = indexval_it->second;
+  cooked_index_flag flags = 0;
+  sect_offset die_offset {};
+  enum language lang = language_unknown;
+  dwarf2_per_cu_data *per_cu = nullptr;
+  for (const auto &attr : indexval.attr_vec)
+    {
+      ULONGEST ull;
+      switch (attr.form)
+	{
+	case DW_FORM_implicit_const:
+	  ull = attr.implicit_const;
+	  break;
+	case DW_FORM_flag_present:
+	  ull = 1;
+	  break;
+	case DW_FORM_udata:
+	  ull = read_unsigned_leb128 (abfd, entry, &bytes_read);
+	  entry += bytes_read;
+	  break;
+	case DW_FORM_ref4:
+	  ull = read_4_bytes (abfd, entry);
+	  entry += 4;
+	  break;
+	case DW_FORM_ref8:
+	  ull = read_8_bytes (abfd, entry);
+	  entry += 8;
+	  break;
+	case DW_FORM_ref_sig8:
+	  ull = read_8_bytes (abfd, entry);
+	  entry += 8;
+	  break;
+	default:
+	  complaint (_("Unsupported .debug_names form %s [in module %s]"),
+		     dwarf_form_name (attr.form),
+		     bfd_get_filename (abfd));
+	  return nullptr;
+	}
+      switch (attr.dw_idx)
+	{
+	case DW_IDX_compile_unit:
+	  {
+	    /* Don't crash on bad data.  */
+	    if (ull >= per_objfile->per_bfd->all_comp_units.size ())
+	      {
+		complaint (_(".debug_names entry has bad CU index %s"
+			     " [in module %s]"),
+			   pulongest (ull),
+			   bfd_get_filename (abfd));
+		continue;
+	      }
+	  }
+	  per_cu = per_objfile->per_bfd->get_cu (ull);
+	  break;
+	case DW_IDX_type_unit:
+	  /* Don't crash on bad data.  */
+	  if (ull >= per_objfile->per_bfd->all_type_units.size ())
+	    {
+	      complaint (_(".debug_names entry has bad TU index %s"
+			   " [in module %s]"),
+			 pulongest (ull),
+			 bfd_get_filename (abfd));
+	      continue;
+	    }
+	  {
+	    int nr_cus = per_objfile->per_bfd->all_comp_units.size ();
+	    per_cu = per_objfile->per_bfd->get_cu (nr_cus + ull);
+	  }
+	  break;
+	case DW_IDX_die_offset:
+	  die_offset = sect_offset (ull);
+	  /* In a per-CU index (as opposed to a per-module index), index
+	     entries without CU attribute implicitly refer to the single CU.  */
+	  if (per_cu == NULL)
+	    per_cu = per_objfile->per_bfd->get_cu (0);
+	  break;
+	case DW_IDX_parent:
+	  parent = ull;
+	  break;
+	case DW_IDX_GNU_internal:
+	  if (augmentation_is_gdb && ull != 0)
+	    flags |= IS_STATIC;
+	  break;
+	case DW_IDX_GNU_main:
+	  if (augmentation_is_gdb && ull != 0)
+	    flags |= IS_MAIN;
+	  break;
+	case DW_IDX_GNU_language:
+	  if (augmentation_is_gdb)
+	    lang = dwarf_lang_to_enum_language (ull);
+	  break;
+	case DW_IDX_GNU_linkage_name:
+	  if (augmentation_is_gdb && ull != 0)
+	    flags |= IS_LINKAGE;
+	  break;
+	}
+    }
+
+  *result = shard->add (die_offset, (dwarf_tag) indexval.dwarf_tag, flags,
+			lang, name, nullptr, per_cu);
+  return entry;
+}
+
+/* Scan all the entries for NAME, at name slot INDEX.  */
+
+void
+mapped_debug_names_reader::scan_entries (uint32_t index,
+					 const char *name,
+					 const gdb_byte *entry)
+{
+  std::vector<cooked_index_entry *> these_entries;
+  
+  while (true)
+    {
+      std::optional<ULONGEST> parent;
+      cooked_index_entry *this_entry;
+      entry = scan_one_entry (name, entry, &this_entry, parent);
+
+      if (entry == nullptr)
+	break;
+
+      these_entries.push_back (this_entry);
+      if (parent.has_value ())
+	needs_parent.emplace_back (this_entry, *parent);
+    }
 
-struct dwarf2_debug_names_index : public dwarf2_base_index_functions
+  all_entries[index] = std::move (these_entries);
+}
+
+/* Scan the name table and create all the entries.  */
+
+void
+mapped_debug_names_reader::scan_all_names ()
 {
-  void dump (struct objfile *objfile) override;
-
-  bool expand_symtabs_matching
-    (struct objfile *objfile,
-     gdb::function_view<expand_symtabs_file_matcher_ftype> file_matcher,
-     const lookup_name_info *lookup_name,
-     gdb::function_view<expand_symtabs_symbol_matcher_ftype> symbol_matcher,
-     gdb::function_view<expand_symtabs_exp_notify_ftype> expansion_notify,
-     block_search_flags search_flags,
-     domain_enum domain,
-     enum search_domain kind) override;
+  all_entries.resize (name_count);
+
+  /* In the first pass, create all the entries.  */
+  for (uint32_t i = 0; i < name_count; ++i)
+    {
+      const ULONGEST namei_string_offs
+	= extract_unsigned_integer ((name_table_string_offs_reordered
+				     + i * offset_size),
+				    offset_size, dwarf5_byte_order);
+      const char *name = read_indirect_string_at_offset (per_objfile,
+							 namei_string_offs);
+
+      const ULONGEST namei_entry_offs
+	= extract_unsigned_integer ((name_table_entry_offs_reordered
+				     + i * offset_size),
+				    offset_size, dwarf5_byte_order);
+      const gdb_byte *entry = entry_pool + namei_entry_offs;
+
+      scan_entries (i, name, entry);
+    }
+
+  /* Now update the parent pointers for all entries.  This has to be
+     done in a funny way because DWARF specifies the parent entry to
+     point to a name -- but we don't know which specific one.  */
+  for (auto [entry, parent_idx] : needs_parent)
+    {
+      /* Name entries are indexed from 1 in DWARF.  */
+      std::vector<cooked_index_entry *> &entries = all_entries[parent_idx - 1];
+      for (const auto &parent : entries)
+	if (parent->lang == entry->lang)
+	  {
+	    entry->parent_entry = parent;
+	    break;
+	  }
+    }
+}
+
+/* A reader for .debug_names.  */
+
+struct cooked_index_debug_names : public cooked_index_worker
+{
+  cooked_index_debug_names (dwarf2_per_objfile *per_objfile,
+			    mapped_debug_names_reader &&map)
+    : cooked_index_worker (per_objfile),
+      m_map (std::move (map))
+  { }
+
+  void do_reading () override;
+
+  mapped_debug_names_reader m_map;
 };
 
-quick_symbol_functions_up
-mapped_debug_names::make_quick_functions () const
+void
+cooked_index_debug_names::do_reading ()
 {
-  return quick_symbol_functions_up (new dwarf2_debug_names_index);
+  complaint_interceptor complaint_handler;
+  std::vector<gdb_exception> exceptions;
+  try
+    {
+      m_map.scan_all_names ();
+    }
+  catch (const gdb_exception &exc)
+    {
+      exceptions.push_back (std::move (exc));
+    }
+
+  dwarf2_per_bfd *per_bfd = m_per_objfile->per_bfd;
+  per_bfd->quick_file_names_table
+    = create_quick_file_names_table (per_bfd->all_units.size ());
+  m_results.emplace_back (nullptr,
+			  complaint_handler.release (),
+			  std::move (exceptions));
+  std::vector<std::unique_ptr<cooked_index_shard>> indexes;
+  indexes.push_back (std::move (m_map.shard));
+  cooked_index *table
+    = (gdb::checked_static_cast<cooked_index *>
+       (per_bfd->index_table.get ()));
+  table->set_contents (std::move (indexes));
+
+  bfd_thread_cleanup ();
 }
 
 /* Check the signatured type hash table from .debug_names.  */
@@ -106,7 +356,7 @@ mapped_debug_names::make_quick_functions () const
 static bool
 check_signatured_type_table_from_debug_names
   (dwarf2_per_objfile *per_objfile,
-   const mapped_debug_names &map,
+   const mapped_debug_names_reader &map,
    struct dwarf2_section_info *section)
 {
   struct objfile *objfile = per_objfile->objfile;
@@ -143,28 +393,18 @@ check_signatured_type_table_from_debug_names
   return true;
 }
 
-/* Read the address map data from DWARF-5 .debug_aranges, and use it to
-   populate the index_addrmap.  */
-
-static void
-create_addrmap_from_aranges (dwarf2_per_objfile *per_objfile,
-			     struct dwarf2_section_info *section)
-{
-  dwarf2_per_bfd *per_bfd = per_objfile->per_bfd;
-
-  addrmap_mutable mutable_map;
-
-  section->read (per_objfile->objfile);
-  if (read_addrmap_from_aranges (per_objfile, section, &mutable_map))
-    per_bfd->index_addrmap
-      = new (&per_bfd->obstack) addrmap_fixed (&per_bfd->obstack,
-					       &mutable_map);
-}
-
 /* DWARF-5 debug_names reader.  */
 
-/* DWARF-5 augmentation string for GDB's DW_IDX_GNU_* extension.  */
-static const gdb_byte dwarf5_augmentation[] = { 'G', 'D', 'B', 0 };
+/* The old, no-longer-supported GDB augmentation.  */
+static const gdb_byte old_gdb_augmentation[]
+     = { 'G', 'D', 'B', 0 };
+static_assert (sizeof (old_gdb_augmentation) % 4 == 0);
+
+/* DWARF-5 augmentation string for GDB's DW_IDX_GNU_* extension.  This
+   must have a size that is a multiple of 4.  */
+static const gdb_byte dwarf5_augmentation[]
+     = { 'G', 'D', 'B', '2', 0, 0, 0, 0 };
+static_assert (sizeof (dwarf5_augmentation) % 4 == 0);
 
 /* A helper function that reads the .debug_names section in SECTION
    and fills in MAP.  FILENAME is the name of the file containing the
@@ -173,11 +413,13 @@ static const gdb_byte dwarf5_augmentation[] = { 'G', 'D', 'B', 0 };
    Returns true if all went well, false otherwise.  */
 
 static bool
-read_debug_names_from_section (struct objfile *objfile,
+read_debug_names_from_section (dwarf2_per_objfile *per_objfile,
 			       const char *filename,
 			       struct dwarf2_section_info *section,
-			       mapped_debug_names &map)
+			       mapped_debug_names_reader &map)
 {
+  struct objfile *objfile = per_objfile->objfile;
+
   if (section->empty ())
     return false;
 
@@ -188,11 +430,13 @@ read_debug_names_from_section (struct objfile *objfile,
 
   section->read (objfile);
 
+  map.per_objfile = per_objfile;
   map.dwarf5_byte_order = gdbarch_byte_order (objfile->arch ());
 
   const gdb_byte *addr = section->buffer;
 
-  bfd *const abfd = section->get_bfd_owner ();
+  bfd *abfd = section->get_bfd_owner ();
+  map.abfd = abfd;
 
   unsigned int bytes_read;
   LONGEST length = read_initial_length (abfd, addr, &bytes_read);
@@ -271,11 +515,27 @@ read_debug_names_from_section (struct objfile *objfile,
      string.  This value is rounded up to a multiple of 4.  */
   uint32_t augmentation_string_size = read_4_bytes (abfd, addr);
   addr += 4;
+  augmentation_string_size += (-augmentation_string_size) & 3;
+
+  if (augmentation_string_size == sizeof (old_gdb_augmentation)
+      && memcmp (addr, old_gdb_augmentation,
+		 sizeof (old_gdb_augmentation)) == 0)
+    {
+      warning (_(".debug_names created by an old version of gdb; ignoring"));
+      return false;
+    }
+
   map.augmentation_is_gdb = ((augmentation_string_size
 			      == sizeof (dwarf5_augmentation))
 			     && memcmp (addr, dwarf5_augmentation,
 					sizeof (dwarf5_augmentation)) == 0);
-  augmentation_string_size += (-augmentation_string_size) & 3;
+
+  if (!map.augmentation_is_gdb)
+    {
+      warning (_(".debug_names not created by gdb; ignoring"));
+      return false;
+    }
+
   addr += augmentation_string_size;
 
   /* List of CUs */
@@ -308,7 +568,7 @@ read_debug_names_from_section (struct objfile *objfile,
 	break;
 
       const auto insertpair
-	= map.abbrev_map.emplace (index_num, mapped_debug_names::index_val ());
+	= map.abbrev_map.emplace (index_num, mapped_debug_names_reader::index_val ());
       if (!insertpair.second)
 	{
 	  warning (_("Section .debug_names in %s has duplicate index %s, "
@@ -316,13 +576,13 @@ read_debug_names_from_section (struct objfile *objfile,
 		   filename, pulongest (index_num));
 	  return false;
 	}
-      mapped_debug_names::index_val &indexval = insertpair.first->second;
+      mapped_debug_names_reader::index_val &indexval = insertpair.first->second;
       indexval.dwarf_tag = read_unsigned_leb128 (abfd, addr, &bytes_read);
       addr += bytes_read;
 
       for (;;)
 	{
-	  mapped_debug_names::index_val::attr attr;
+	  mapped_debug_names_reader::index_val::attr attr;
 	  attr.dw_idx = read_unsigned_leb128 (abfd, addr, &bytes_read);
 	  addr += bytes_read;
 	  attr.form = read_unsigned_leb128 (abfd, addr, &bytes_read);
@@ -356,9 +616,9 @@ read_debug_names_from_section (struct objfile *objfile,
 
 static bool
 check_cus_from_debug_names_list (dwarf2_per_bfd *per_bfd,
-				 const mapped_debug_names &map,
-				 dwarf2_section_info &section,
-				 bool is_dwz)
+				  const mapped_debug_names_reader &map,
+				  dwarf2_section_info &section,
+				  bool is_dwz)
 {
   int nr_cus = per_bfd->all_comp_units.size ();
 
@@ -420,8 +680,8 @@ check_cus_from_debug_names_list (dwarf2_per_bfd *per_bfd,
 
 static bool
 check_cus_from_debug_names (dwarf2_per_bfd *per_bfd,
-			    const mapped_debug_names &map,
-			    const mapped_debug_names &dwz_map)
+			     const mapped_debug_names_reader &map,
+			     const mapped_debug_names_reader &dwz_map)
 {
   if (!check_cus_from_debug_names_list (per_bfd, map, per_bfd->info,
 					false /* is_dwz */))
@@ -435,22 +695,23 @@ check_cus_from_debug_names (dwarf2_per_bfd *per_bfd,
 					  true /* is_dwz */);
 }
 
-/* See read-debug-names.h.  */
+/* This does all the work for dwarf2_read_debug_names, but putting it
+   into a separate function makes some cleanup a bit simpler.  */
 
-bool
-dwarf2_read_debug_names (dwarf2_per_objfile *per_objfile)
+static bool
+do_dwarf2_read_debug_names (dwarf2_per_objfile *per_objfile)
 {
-  auto map = std::make_unique<mapped_debug_names> ();
-  mapped_debug_names dwz_map;
+  mapped_debug_names_reader map;
+  mapped_debug_names_reader dwz_map;
   struct objfile *objfile = per_objfile->objfile;
   dwarf2_per_bfd *per_bfd = per_objfile->per_bfd;
 
-  if (!read_debug_names_from_section (objfile, objfile_name (objfile),
-				      &per_bfd->debug_names, *map))
+  if (!read_debug_names_from_section (per_objfile, objfile_name (objfile),
+				      &per_bfd->debug_names, map))
     return false;
 
   /* Don't use the index if it's empty.  */
-  if (map->name_count == 0)
+  if (map.name_count == 0)
     return false;
 
   /* If there is a .dwz file, read it so we can get its CU list as
@@ -458,7 +719,7 @@ dwarf2_read_debug_names (dwarf2_per_objfile *per_objfile)
   dwz_file *dwz = dwarf2_get_dwz_file (per_bfd);
   if (dwz != NULL)
     {
-      if (!read_debug_names_from_section (objfile,
+      if (!read_debug_names_from_section (per_objfile,
 					  bfd_get_filename (dwz->dwz_bfd.get ()),
 					  &dwz->debug_names, dwz_map))
 	{
@@ -469,525 +730,53 @@ dwarf2_read_debug_names (dwarf2_per_objfile *per_objfile)
     }
 
   create_all_units (per_objfile);
-  if (!check_cus_from_debug_names (per_bfd, *map, dwz_map))
-    {
-      per_bfd->all_units.clear ();
-      return false;
-    }
+  if (!check_cus_from_debug_names (per_bfd, map, dwz_map))
+    return false;
 
-  if (map->tu_count != 0)
+  if (map.tu_count != 0)
     {
       /* We can only handle a single .debug_types when we have an
 	 index.  */
       if (per_bfd->types.size () > 1)
-	{
-	  per_bfd->all_units.clear ();
-	  return false;
-	}
+	return false;
 
       dwarf2_section_info *section
 	= (per_bfd->types.size () == 1
 	   ? &per_bfd->types[0]
 	   : &per_bfd->info);
 
-      if (!check_signatured_type_table_from_debug_names
-	     (per_objfile, *map, section))
-	{
-	  per_bfd->all_units.clear ();
-	  return false;
-	}
-    }
-
-  create_addrmap_from_aranges (per_objfile, &per_bfd->debug_aranges);
-
-  per_bfd->index_table = std::move (map);
-  per_bfd->quick_file_names_table =
-    create_quick_file_names_table (per_bfd->all_units.size ());
-
-  return true;
-}
-
-/* Type used to manage iterating over all CUs looking for a symbol for
-   .debug_names.  */
-
-class dw2_debug_names_iterator
-{
-public:
-  dw2_debug_names_iterator (const mapped_debug_names &map,
-			    block_search_flags block_index,
-			    domain_enum domain,
-			    const char *name, dwarf2_per_objfile *per_objfile)
-    : m_map (map), m_block_index (block_index), m_domain (domain),
-      m_addr (find_vec_in_debug_names (map, name, per_objfile)),
-      m_per_objfile (per_objfile)
-  {}
-
-  dw2_debug_names_iterator (const mapped_debug_names &map,
-			    search_domain search, uint32_t namei,
-			    dwarf2_per_objfile *per_objfile,
-			    domain_enum domain = UNDEF_DOMAIN)
-    : m_map (map),
-      m_domain (domain),
-      m_search (search),
-      m_addr (find_vec_in_debug_names (map, namei, per_objfile)),
-      m_per_objfile (per_objfile)
-  {}
-
-  dw2_debug_names_iterator (const mapped_debug_names &map,
-			    block_search_flags block_index, domain_enum domain,
-			    uint32_t namei, dwarf2_per_objfile *per_objfile)
-    : m_map (map), m_block_index (block_index), m_domain (domain),
-      m_addr (find_vec_in_debug_names (map, namei, per_objfile)),
-      m_per_objfile (per_objfile)
-  {}
-
-  /* Return the next matching CU or NULL if there are no more.  */
-  dwarf2_per_cu_data *next ();
-
-private:
-  static const gdb_byte *find_vec_in_debug_names (const mapped_debug_names &map,
-						  const char *name,
-						  dwarf2_per_objfile *per_objfile);
-  static const gdb_byte *find_vec_in_debug_names (const mapped_debug_names &map,
-						  uint32_t namei,
-						  dwarf2_per_objfile *per_objfile);
-
-  /* The internalized form of .debug_names.  */
-  const mapped_debug_names &m_map;
-
-  /* Restrict the search to these blocks.  */
-  block_search_flags m_block_index = (SEARCH_GLOBAL_BLOCK
-				      | SEARCH_STATIC_BLOCK);
-
-  /* The kind of symbol we're looking for.  */
-  const domain_enum m_domain = UNDEF_DOMAIN;
-  const search_domain m_search = ALL_DOMAIN;
-
-  /* The list of CUs from the index entry of the symbol, or NULL if
-     not found.  */
-  const gdb_byte *m_addr;
-
-  dwarf2_per_objfile *m_per_objfile;
-};
-
-const char *
-mapped_debug_names::namei_to_name
-  (uint32_t namei, dwarf2_per_objfile *per_objfile) const
-{
-  const ULONGEST namei_string_offs
-    = extract_unsigned_integer ((name_table_string_offs_reordered
-				 + namei * offset_size),
-				offset_size,
-				dwarf5_byte_order);
-  return read_indirect_string_at_offset (per_objfile, namei_string_offs);
-}
-
-/* Find a slot in .debug_names for the object named NAME.  If NAME is
-   found, return pointer to its pool data.  If NAME cannot be found,
-   return NULL.  */
-
-const gdb_byte *
-dw2_debug_names_iterator::find_vec_in_debug_names
-  (const mapped_debug_names &map, const char *name,
-   dwarf2_per_objfile *per_objfile)
-{
-  int (*cmp) (const char *, const char *);
-
-  gdb::unique_xmalloc_ptr<char> without_params;
-  if (current_language->la_language == language_cplus
-      || current_language->la_language == language_fortran
-      || current_language->la_language == language_d)
-    {
-      /* NAME is already canonical.  Drop any qualifiers as
-	 .debug_names does not contain any.  */
-
-      if (strchr (name, '(') != NULL)
-	{
-	  without_params = cp_remove_params (name);
-	  if (without_params != NULL)
-	    name = without_params.get ();
-	}
-    }
-
-  cmp = (case_sensitivity == case_sensitive_on ? strcmp : strcasecmp);
-
-  const uint32_t full_hash = dwarf5_djb_hash (name);
-  uint32_t namei
-    = extract_unsigned_integer (reinterpret_cast<const gdb_byte *>
-				(map.bucket_table_reordered
-				 + (full_hash % map.bucket_count)), 4,
-				map.dwarf5_byte_order);
-  if (namei == 0)
-    return NULL;
-  --namei;
-  if (namei >= map.name_count)
-    {
-      complaint (_("Wrong .debug_names with name index %u but name_count=%u "
-		   "[in module %s]"),
-		 namei, map.name_count,
-		 objfile_name (per_objfile->objfile));
-      return NULL;
-    }
-
-  for (;;)
-    {
-      const uint32_t namei_full_hash
-	= extract_unsigned_integer (reinterpret_cast<const gdb_byte *>
-				    (map.hash_table_reordered + namei), 4,
-				    map.dwarf5_byte_order);
-      if (full_hash % map.bucket_count != namei_full_hash % map.bucket_count)
-	return NULL;
-
-      if (full_hash == namei_full_hash)
-	{
-	  const char *const namei_string = map.namei_to_name (namei, per_objfile);
-
-#if 0 /* An expensive sanity check.  */
-	  if (namei_full_hash != dwarf5_djb_hash (namei_string))
-	    {
-	      complaint (_("Wrong .debug_names hash for string at index %u "
-			   "[in module %s]"),
-			 namei, objfile_name (dwarf2_per_objfile->objfile));
-	      return NULL;
-	    }
-#endif
-
-	  if (cmp (namei_string, name) == 0)
-	    {
-	      const ULONGEST namei_entry_offs
-		= extract_unsigned_integer ((map.name_table_entry_offs_reordered
-					     + namei * map.offset_size),
-					    map.offset_size, map.dwarf5_byte_order);
-	      return map.entry_pool + namei_entry_offs;
-	    }
-	}
-
-      ++namei;
-      if (namei >= map.name_count)
-	return NULL;
-    }
-}
-
-const gdb_byte *
-dw2_debug_names_iterator::find_vec_in_debug_names
-  (const mapped_debug_names &map, uint32_t namei, dwarf2_per_objfile *per_objfile)
-{
-  if (namei >= map.name_count)
-    {
-      complaint (_("Wrong .debug_names with name index %u but name_count=%u "
-		   "[in module %s]"),
-		 namei, map.name_count,
-		 objfile_name (per_objfile->objfile));
-      return NULL;
-    }
-
-  const ULONGEST namei_entry_offs
-    = extract_unsigned_integer ((map.name_table_entry_offs_reordered
-				 + namei * map.offset_size),
-				map.offset_size, map.dwarf5_byte_order);
-  return map.entry_pool + namei_entry_offs;
-}
-
-/* See dw2_debug_names_iterator.  */
-
-dwarf2_per_cu_data *
-dw2_debug_names_iterator::next ()
-{
-  if (m_addr == NULL)
-    return NULL;
-
-  dwarf2_per_bfd *per_bfd = m_per_objfile->per_bfd;
-  struct objfile *objfile = m_per_objfile->objfile;
-  bfd *const abfd = objfile->obfd.get ();
-
- again:
-
-  unsigned int bytes_read;
-  const ULONGEST abbrev = read_unsigned_leb128 (abfd, m_addr, &bytes_read);
-  m_addr += bytes_read;
-  if (abbrev == 0)
-    return NULL;
-
-  const auto indexval_it = m_map.abbrev_map.find (abbrev);
-  if (indexval_it == m_map.abbrev_map.cend ())
-    {
-      complaint (_("Wrong .debug_names undefined abbrev code %s "
-		   "[in module %s]"),
-		 pulongest (abbrev), objfile_name (objfile));
-      return NULL;
-    }
-  const mapped_debug_names::index_val &indexval = indexval_it->second;
-  enum class symbol_linkage {
-    unknown,
-    static_,
-    extern_,
-  } symbol_linkage_ = symbol_linkage::unknown;
-  dwarf2_per_cu_data *per_cu = NULL;
-  for (const mapped_debug_names::index_val::attr &attr : indexval.attr_vec)
-    {
-      ULONGEST ull;
-      switch (attr.form)
-	{
-	case DW_FORM_implicit_const:
-	  ull = attr.implicit_const;
-	  break;
-	case DW_FORM_flag_present:
-	  ull = 1;
-	  break;
-	case DW_FORM_udata:
-	  ull = read_unsigned_leb128 (abfd, m_addr, &bytes_read);
-	  m_addr += bytes_read;
-	  break;
-	case DW_FORM_ref4:
-	  ull = read_4_bytes (abfd, m_addr);
-	  m_addr += 4;
-	  break;
-	case DW_FORM_ref8:
-	  ull = read_8_bytes (abfd, m_addr);
-	  m_addr += 8;
-	  break;
-	case DW_FORM_ref_sig8:
-	  ull = read_8_bytes (abfd, m_addr);
-	  m_addr += 8;
-	  break;
-	default:
-	  complaint (_("Unsupported .debug_names form %s [in module %s]"),
-		     dwarf_form_name (attr.form),
-		     objfile_name (objfile));
-	  return NULL;
-	}
-      switch (attr.dw_idx)
-	{
-	case DW_IDX_compile_unit:
-	  {
-	    /* Don't crash on bad data.  */
-	    if (ull >= per_bfd->all_comp_units.size ())
-	      {
-		complaint (_(".debug_names entry has bad CU index %s"
-			     " [in module %s]"),
-			   pulongest (ull),
-			   objfile_name (objfile));
-		continue;
-	      }
-	  }
-	  per_cu = per_bfd->get_index_cu (ull);
-	  break;
-	case DW_IDX_type_unit:
-	  /* Don't crash on bad data.  */
-	  if (ull >= per_bfd->all_type_units.size ())
-	    {
-	      complaint (_(".debug_names entry has bad TU index %s"
-			   " [in module %s]"),
-			 pulongest (ull),
-			 objfile_name (objfile));
-	      continue;
-	    }
-	  {
-	    per_cu = per_bfd->get_index_tu (ull);
-	  }
-	  break;
-	case DW_IDX_die_offset:
-	  /* In a per-CU index (as opposed to a per-module index), index
-	     entries without CU attribute implicitly refer to the single CU.  */
-	  if (per_cu == NULL)
-	    per_cu = per_bfd->get_index_cu (0);
-	  break;
-	case DW_IDX_GNU_internal:
-	  if (!m_map.augmentation_is_gdb)
-	    break;
-	  symbol_linkage_ = symbol_linkage::static_;
-	  break;
-	case DW_IDX_GNU_external:
-	  if (!m_map.augmentation_is_gdb)
-	    break;
-	  symbol_linkage_ = symbol_linkage::extern_;
-	  break;
-	}
+      if (!check_signatured_type_table_from_debug_names (per_objfile,
+							 map, section))
+	return false;
     }
 
-  /* Skip if we couldn't find a valid CU/TU index.  */
-  if (per_cu == nullptr)
-    goto again;
+  per_bfd->debug_aranges.read (per_objfile->objfile);
+  addrmap_mutable addrmap;
+  if (!read_addrmap_from_aranges (per_objfile, &per_bfd->debug_aranges,
+				  &addrmap))
+    return false;
 
-  /* Skip if already read in.  */
-  if (m_per_objfile->symtab_set_p (per_cu))
-    goto again;
+  map.shard = std::make_unique<cooked_index_shard> ();
+  map.shard->install_addrmap (&addrmap);
 
-  /* Check static vs global.  */
-  if (symbol_linkage_ != symbol_linkage::unknown)
-    {
-      if (symbol_linkage_ == symbol_linkage::static_)
-	{
-	  if ((m_block_index & SEARCH_STATIC_BLOCK) == 0)
-	    goto again;
-	}
-      else
-	{
-	  if ((m_block_index & SEARCH_GLOBAL_BLOCK) == 0)
-	    goto again;
-	}
-    }
+  cooked_index *idx
+    = new debug_names_index (per_objfile,
+			     (std::make_unique<cooked_index_debug_names>
+			      (per_objfile, std::move (map))));
+  per_bfd->index_table.reset (idx);
 
-  /* Match dw2_symtab_iter_next, symbol_kind
-     and debug_names::psymbol_tag.  */
-  switch (m_domain)
-    {
-    case VAR_DOMAIN:
-      switch (indexval.dwarf_tag)
-	{
-	case DW_TAG_variable:
-	case DW_TAG_subprogram:
-	/* Some types are also in VAR_DOMAIN.  */
-	case DW_TAG_typedef:
-	case DW_TAG_structure_type:
-	  break;
-	default:
-	  goto again;
-	}
-      break;
-    case STRUCT_DOMAIN:
-      switch (indexval.dwarf_tag)
-	{
-	case DW_TAG_typedef:
-	case DW_TAG_structure_type:
-	  break;
-	default:
-	  goto again;
-	}
-      break;
-    case LABEL_DOMAIN:
-      switch (indexval.dwarf_tag)
-	{
-	case 0:
-	case DW_TAG_variable:
-	  break;
-	default:
-	  goto again;
-	}
-      break;
-    case MODULE_DOMAIN:
-      switch (indexval.dwarf_tag)
-	{
-	case DW_TAG_module:
-	  break;
-	default:
-	  goto again;
-	}
-      break;
-    default:
-      break;
-    }
-
-  /* Match dw2_expand_symtabs_matching, symbol_kind and
-     debug_names::psymbol_tag.  */
-  switch (m_search)
-    {
-    case VARIABLES_DOMAIN:
-      switch (indexval.dwarf_tag)
-	{
-	case DW_TAG_variable:
-	  break;
-	default:
-	  goto again;
-	}
-      break;
-    case FUNCTIONS_DOMAIN:
-      switch (indexval.dwarf_tag)
-	{
-	case DW_TAG_subprogram:
-	  break;
-	default:
-	  goto again;
-	}
-      break;
-    case TYPES_DOMAIN:
-      switch (indexval.dwarf_tag)
-	{
-	case DW_TAG_typedef:
-	case DW_TAG_structure_type:
-	  break;
-	default:
-	  goto again;
-	}
-      break;
-    case MODULES_DOMAIN:
-      switch (indexval.dwarf_tag)
-	{
-	case DW_TAG_module:
-	  break;
-	default:
-	  goto again;
-	}
-    default:
-      break;
-    }
+  idx->start_reading ();
 
-  return per_cu;
+  return true;
 }
 
-/* This dumps minimal information about .debug_names.  It is called
-   via "mt print objfiles".  The gdb.dwarf2/gdb-index.exp testcase
-   uses this to verify that .debug_names has been loaded.  */
-
-void
-dwarf2_debug_names_index::dump (struct objfile *objfile)
-{
-  gdb_printf (".debug_names: exists\n");
-}
+/* See read-debug-names.h.  */
 
 bool
-dwarf2_debug_names_index::expand_symtabs_matching
-  (struct objfile *objfile,
-   gdb::function_view<expand_symtabs_file_matcher_ftype> file_matcher,
-   const lookup_name_info *lookup_name,
-   gdb::function_view<expand_symtabs_symbol_matcher_ftype> symbol_matcher,
-   gdb::function_view<expand_symtabs_exp_notify_ftype> expansion_notify,
-   block_search_flags search_flags,
-   domain_enum domain,
-   enum search_domain kind)
+dwarf2_read_debug_names (dwarf2_per_objfile *per_objfile)
 {
-  dwarf2_per_objfile *per_objfile = get_dwarf2_per_objfile (objfile);
-
-  dw_expand_symtabs_matching_file_matcher (per_objfile, file_matcher);
-
-  /* This invariant is documented in quick-functions.h.  */
-  gdb_assert (lookup_name != nullptr || symbol_matcher == nullptr);
-  if (lookup_name == nullptr)
-    {
-      for (dwarf2_per_cu_data *per_cu
-	     : all_units_range (per_objfile->per_bfd))
-	{
-	  QUIT;
-
-	  if (!dw2_expand_symtabs_matching_one (per_cu, per_objfile,
-						file_matcher,
-						expansion_notify))
-	    return false;
-	}
-      return true;
-    }
-
-  mapped_debug_names &map
-    = *(gdb::checked_static_cast<mapped_debug_names *>
-	(per_objfile->per_bfd->index_table.get ()));
-
-  bool result
-    = dw2_expand_symtabs_matching_symbol (map, *lookup_name,
-					  symbol_matcher,
-					  [&] (offset_type namei)
-    {
-      /* The name was matched, now expand corresponding CUs that were
-	 marked.  */
-      dw2_debug_names_iterator iter (map, kind, namei, per_objfile, domain);
-
-      struct dwarf2_per_cu_data *per_cu;
-      while ((per_cu = iter.next ()) != NULL)
-	if (!dw2_expand_symtabs_matching_one (per_cu, per_objfile,
-					      file_matcher,
-					      expansion_notify))
-	  return false;
-      return true;
-    }, per_objfile);
-
+  bool result = do_dwarf2_read_debug_names (per_objfile);
+  if (!result)
+    per_objfile->per_bfd->all_units.clear ();
   return result;
 }

-- 
2.43.0


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 16/17] Export dwarf5_augmentation
  2023-12-10 16:44 [PATCH 00/17] Rewrite .debug_names reader and writer Tom Tromey
                   ` (14 preceding siblings ...)
  2023-12-10 16:45 ` [PATCH 15/17] Rewrite .debug_names reader Tom Tromey
@ 2023-12-10 16:45 ` Tom Tromey
  2023-12-10 16:45 ` [PATCH 17/17] Rewrite .debug_names writer Tom Tromey
  16 siblings, 0 replies; 22+ messages in thread
From: Tom Tromey @ 2023-12-10 16:45 UTC (permalink / raw)
  To: gdb-patches

I don't know why gdb had the .debug_names augmentation string in two
separate places; this patch exports it in one spot, to be used in
another.
---
 gdb/dwarf2/read-debug-names.c | 3 +--
 gdb/dwarf2/read-debug-names.h | 2 ++
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/gdb/dwarf2/read-debug-names.c b/gdb/dwarf2/read-debug-names.c
index 1e53c246b91..4325a5ddcc6 100644
--- a/gdb/dwarf2/read-debug-names.c
+++ b/gdb/dwarf2/read-debug-names.c
@@ -402,8 +402,7 @@ static_assert (sizeof (old_gdb_augmentation) % 4 == 0);
 
 /* DWARF-5 augmentation string for GDB's DW_IDX_GNU_* extension.  This
    must have a size that is a multiple of 4.  */
-static const gdb_byte dwarf5_augmentation[]
-     = { 'G', 'D', 'B', '2', 0, 0, 0, 0 };
+const gdb_byte dwarf5_augmentation[8] = { 'G', 'D', 'B', '2', 0, 0, 0, 0 };
 static_assert (sizeof (dwarf5_augmentation) % 4 == 0);
 
 /* A helper function that reads the .debug_names section in SECTION
diff --git a/gdb/dwarf2/read-debug-names.h b/gdb/dwarf2/read-debug-names.h
index 0c20e8cfb8d..9240d3c08c1 100644
--- a/gdb/dwarf2/read-debug-names.h
+++ b/gdb/dwarf2/read-debug-names.h
@@ -22,6 +22,8 @@
 
 struct dwarf2_per_objfile;
 
+extern const gdb_byte dwarf5_augmentation[8];
+
 /* Read .debug_names.  If everything went ok, initialize the "quick"
    elements of all the CUs and return true.  Otherwise, return false.  */
 

-- 
2.43.0


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 17/17] Rewrite .debug_names writer
  2023-12-10 16:44 [PATCH 00/17] Rewrite .debug_names reader and writer Tom Tromey
                   ` (15 preceding siblings ...)
  2023-12-10 16:45 ` [PATCH 16/17] Export dwarf5_augmentation Tom Tromey
@ 2023-12-10 16:45 ` Tom Tromey
  16 siblings, 0 replies; 22+ messages in thread
From: Tom Tromey @ 2023-12-10 16:45 UTC (permalink / raw)
  To: gdb-patches

This rewrites GDB's .debug_names writer.  It is now closer to the form
imagined in the DWARF spec.  In particular, names are emitted exactly
as they appear in the original DWARF.

In order to make the reader work nicely, some extensions were needed.
These were all documented in an earlier patch.  Note that in
particular this writer solves the "main name" problem by putting a
flag into the table.

GDB does not use the .debug_names hash table, so it also does not
write one.  I consider this hash table to be essentially useless in
general, due to the name canonicalization problem -- while DWARF says
that writers should use the system demangling style, (1) this style
varies across systems, so it can't truly be relied on; and (2) at
least GCC and one other compiler don't actually follow this part of
the spec anyway.

It's important to note, though, that even if the hash was somehow
useful, GDB probably still would not use it -- a sorted list of names
is needed for completion and performs reasonably well for other
lookups, so a hash table is just overhead, IMO.

String emission is also simplified.  There's no need in this writer to
ingest the contents of .debug_str.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=24820
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=24549
---
 gdb/dwarf2/index-write.c | 395 +++++++++++++++++++----------------------------
 1 file changed, 156 insertions(+), 239 deletions(-)

diff --git a/gdb/dwarf2/index-write.c b/gdb/dwarf2/index-write.c
index c23fa25e021..940c58e59bd 100644
--- a/gdb/dwarf2/index-write.c
+++ b/gdb/dwarf2/index-write.c
@@ -40,10 +40,12 @@
 #include "ada-lang.h"
 #include "dwarf2/tag.h"
 #include "gdbsupport/gdb_tilde_expand.h"
+#include "dwarf2/read-debug-names.h"
 
 #include <algorithm>
 #include <cmath>
 #include <forward_list>
+#include <map>
 #include <set>
 #include <unordered_map>
 #include <unordered_set>
@@ -672,66 +674,15 @@ class debug_names
   /* Insert one symbol.  */
   void insert (const cooked_index_entry *entry)
   {
-    const auto it = m_cu_index_htab.find (entry->per_cu);
-    gdb_assert (it != m_cu_index_htab.cend ());
-    const char *name = entry->full_name (&m_string_obstack);
-
-    /* This is incorrect but it mirrors gdb's historical behavior; and
-       because the current .debug_names generation is also incorrect,
-       it seems better to follow what was done before, rather than
-       introduce a mismatch between the newer and older gdb.  */
-    dwarf_tag tag = entry->tag;
-    if (tag != DW_TAG_typedef && tag_is_type (tag))
-      tag = DW_TAG_structure_type;
-    else if (tag == DW_TAG_enumerator || tag == DW_TAG_constant)
-      tag = DW_TAG_variable;
-
-    int cu_index = it->second;
-    bool is_static = (entry->flags & IS_STATIC) != 0;
-    unit_kind kind = (entry->per_cu->is_debug_types
-		      ? unit_kind::tu
-		      : unit_kind::cu);
-
-    if (entry->per_cu->lang () == language_ada)
-      {
-	/* We want to ensure that the Ada main function's name appears
-	   verbatim in the index.  However, this name will be of the
-	   form "_ada_mumble", and will be rewritten by ada_decode.
-	   So, recognize it specially here and add it to the index by
-	   hand.  */
-	if (strcmp (main_name (), name) == 0)
-	  {
-	    const auto insertpair
-	      = m_name_to_value_set.emplace (c_str_view (name),
-					     std::set<symbol_value> ());
-	    std::set<symbol_value> &value_set = insertpair.first->second;
-	    value_set.emplace (symbol_value (tag, cu_index, is_static, kind));
-	  }
-
-	/* In order for the index to work when read back into gdb, it
-	   has to supply a funny form of the name: it should be the
-	   encoded name, with any suffixes stripped.  Using the
-	   ordinary encoded name will not work properly with the
-	   searching logic in find_name_components_bounds; nor will
-	   using the decoded name.  Furthermore, an Ada "verbatim"
-	   name (of the form "<MumBle>") must be entered without the
-	   angle brackets.  Note that the current index is unusual,
-	   see PR symtab/24820 for details.  */
-	std::string decoded = ada_decode (name);
-	if (decoded[0] == '<')
-	  name = (char *) obstack_copy0 (&m_string_obstack,
-					 decoded.c_str () + 1,
-					 decoded.length () - 2);
-	else
-	  name = obstack_strdup (&m_string_obstack,
-				 ada_encode (decoded.c_str ()));
-      }
+    /* These entries are synthesized by the reader, and so should not
+       be written.  */
+    if (entry->lang == language_ada && entry->tag == DW_TAG_namespace)
+      return;
 
     const auto insertpair
-      = m_name_to_value_set.emplace (c_str_view (name),
-				     std::set<symbol_value> ());
-    std::set<symbol_value> &value_set = insertpair.first->second;
-    value_set.emplace (symbol_value (tag, cu_index, is_static, kind));
+      = m_name_to_value_set.try_emplace (c_str_view (entry->name));
+    entry_list &elist = insertpair.first->second;
+    elist.entries.push_back (entry);
   }
 
   /* Build all the tables.  All symbols must be already inserted.
@@ -742,127 +693,137 @@ class debug_names
     /* Verify the build method has not be called twice.  */
     gdb_assert (m_abbrev_table.empty ());
     const size_t name_count = m_name_to_value_set.size ();
-    m_bucket_table.resize
-      (std::pow (2, std::ceil (std::log2 (name_count * 4 / 3))));
-    m_hash_table.reserve (name_count);
     m_name_table_string_offs.reserve (name_count);
     m_name_table_entry_offs.reserve (name_count);
 
-    /* Map each hash of symbol to its name and value.  */
-    struct hash_it_pair
-    {
-      uint32_t hash;
-      decltype (m_name_to_value_set)::const_iterator it;
-    };
-    std::vector<std::forward_list<hash_it_pair>> bucket_hash;
-    bucket_hash.resize (m_bucket_table.size ());
-    for (decltype (m_name_to_value_set)::const_iterator it
-	   = m_name_to_value_set.cbegin ();
-	 it != m_name_to_value_set.cend ();
-	 ++it)
-      {
-	const char *const name = it->first.c_str ();
-	const uint32_t hash = dwarf5_djb_hash (name);
-	hash_it_pair hashitpair;
-	hashitpair.hash = hash;
-	hashitpair.it = it;
-	auto &slot = bucket_hash[hash % bucket_hash.size()];
-	slot.push_front (std::move (hashitpair));
-      }
-    for (size_t bucket_ix = 0; bucket_ix < bucket_hash.size (); ++bucket_ix)
+    /* The name table is indexed from 1.  The numbers are needed here
+       so that parent entries can be handled correctly.  */
+    int next_name = 1;
+    for (auto &item : m_name_to_value_set)
+      item.second.index = next_name++;
+
+    /* The next available abbrev number.  */
+    int next_abbrev = 1;
+
+    for (auto &item : m_name_to_value_set)
       {
-	std::forward_list<hash_it_pair> &hashitlist = bucket_hash[bucket_ix];
-	if (hashitlist.empty ())
-	  continue;
+	const c_str_view &name = item.first;
+	entry_list &these_entries = item.second;
 
 	/* Sort the items within each bucket.  This ensures that the
 	   generated index files will be the same no matter the order in
 	   which symbols were added into the index.  */
-	hashitlist.sort ([] (const hash_it_pair &a, const hash_it_pair &b)
-	{
-	  return a.it->first < b.it->first;
-	});
-
-	uint32_t &bucket_slot = m_bucket_table[bucket_ix];
-	/* The hashes array is indexed starting at 1.  */
-	store_unsigned_integer (reinterpret_cast<gdb_byte *> (&bucket_slot),
-				sizeof (bucket_slot), m_dwarf5_byte_order,
-				m_hash_table.size () + 1);
-	for (const hash_it_pair &hashitpair : hashitlist)
+	std::sort (these_entries.entries.begin (),
+		   these_entries.entries.end (),
+		   [] (const cooked_index_entry *a,
+		       const cooked_index_entry *b)
+		   {
+		     /* Sort first by CU.  */
+		     if (a->per_cu->index != b->per_cu->index)
+		       return a->per_cu->index < b->per_cu->index;
+		     /* Then by DIE in the CU.  */
+		     if (a->die_offset != b->die_offset)
+		       return a->die_offset < b->die_offset;
+		     /* We might have two entries for a DIE because
+			the linkage name is entered separately.  So,
+			sort by flags.  */
+		     return a->flags < b->flags;
+		   });
+
+	m_name_table_string_offs.push_back_reorder
+	  (m_debugstrlookup.lookup (name.c_str ())); /* ??? */
+	m_name_table_entry_offs.push_back_reorder (m_entry_pool.size ());
+
+	for (const cooked_index_entry *entry : these_entries.entries)
 	  {
-	    m_hash_table.push_back (0);
-	    store_unsigned_integer (reinterpret_cast<gdb_byte *>
-							(&m_hash_table.back ()),
-				    sizeof (m_hash_table.back ()),
-				    m_dwarf5_byte_order, hashitpair.hash);
-	    const c_str_view &name = hashitpair.it->first;
-	    const std::set<symbol_value> &value_set = hashitpair.it->second;
-	    m_name_table_string_offs.push_back_reorder
-	      (m_debugstrlookup.lookup (name.c_str ()));
-	    m_name_table_entry_offs.push_back_reorder (m_entry_pool.size ());
-	    gdb_assert (!value_set.empty ());
-	    for (const symbol_value &value : value_set)
+	    unit_kind kind = (entry->per_cu->is_debug_types
+			      ? unit_kind::tu
+			      : unit_kind::cu);
+	    /* Currently Ada parentage is synthesized by the
+	       reader and so must be ignored here.  */
+	    const cooked_index_entry *parent = (entry->lang == language_ada
+						? nullptr
+						: entry->parent_entry);
+
+	    int &idx = m_indexkey_to_idx[index_key (entry->tag,
+						    kind,
+						    entry->flags,
+						    entry->lang,
+						    parent != nullptr)];
+	    if (idx == 0)
 	      {
-		int &idx = m_indexkey_to_idx[index_key (value.dwarf_tag,
-							value.is_static,
-							value.kind)];
-		if (idx == 0)
+		idx = next_abbrev++;
+		m_abbrev_table.append_unsigned_leb128 (idx);
+		m_abbrev_table.append_unsigned_leb128 (entry->tag);
+		m_abbrev_table.append_unsigned_leb128
+		  (kind == unit_kind::cu
+		   ? DW_IDX_compile_unit
+		   : DW_IDX_type_unit);
+		m_abbrev_table.append_unsigned_leb128 (DW_FORM_udata);
+		m_abbrev_table.append_unsigned_leb128 (DW_IDX_die_offset);
+		m_abbrev_table.append_unsigned_leb128 (DW_FORM_udata);
+		m_abbrev_table.append_unsigned_leb128 (DW_IDX_GNU_language);
+		m_abbrev_table.append_unsigned_leb128 (DW_FORM_udata);
+		if ((entry->flags & IS_STATIC) != 0)
 		  {
-		    idx = m_idx_next++;
-		    m_abbrev_table.append_unsigned_leb128 (idx);
-		    m_abbrev_table.append_unsigned_leb128 (value.dwarf_tag);
-		    m_abbrev_table.append_unsigned_leb128
-			      (value.kind == unit_kind::cu ? DW_IDX_compile_unit
-							   : DW_IDX_type_unit);
-		    m_abbrev_table.append_unsigned_leb128 (DW_FORM_udata);
-		    m_abbrev_table.append_unsigned_leb128 (value.is_static
-							   ? DW_IDX_GNU_internal
-							   : DW_IDX_GNU_external);
+		    m_abbrev_table.append_unsigned_leb128 (DW_IDX_GNU_internal);
 		    m_abbrev_table.append_unsigned_leb128 (DW_FORM_flag_present);
-
-		    /* Terminate attributes list.  */
-		    m_abbrev_table.append_unsigned_leb128 (0);
-		    m_abbrev_table.append_unsigned_leb128 (0);
+		  }
+		if ((entry->flags & IS_MAIN) != 0)
+		  {
+		    m_abbrev_table.append_unsigned_leb128 (DW_IDX_GNU_main);
+		    m_abbrev_table.append_unsigned_leb128 (DW_FORM_flag_present);
+		  }
+		if ((entry->flags & IS_LINKAGE) != 0)
+		  {
+		    m_abbrev_table.append_unsigned_leb128 (DW_IDX_GNU_linkage_name);
+		    m_abbrev_table.append_unsigned_leb128 (DW_FORM_flag_present);
+		  }
+		if (parent != nullptr)
+		  {
+		    m_abbrev_table.append_unsigned_leb128 (DW_IDX_parent);
+		    m_abbrev_table.append_unsigned_leb128 (DW_FORM_udata);
 		  }
 
-		m_entry_pool.append_unsigned_leb128 (idx);
-		m_entry_pool.append_unsigned_leb128 (value.cu_index);
+		/* Terminate attributes list.  */
+		m_abbrev_table.append_unsigned_leb128 (0);
+		m_abbrev_table.append_unsigned_leb128 (0);
 	      }
 
-	    /* Terminate the list of CUs.  */
-	    m_entry_pool.append_unsigned_leb128 (0);
+	    m_entry_pool.append_unsigned_leb128 (idx);
+
+	    const auto it = m_cu_index_htab.find (entry->per_cu);
+	    gdb_assert (it != m_cu_index_htab.cend ());
+	    m_entry_pool.append_unsigned_leb128 (it->second);
+
+	    m_entry_pool.append_unsigned_leb128 (to_underlying (entry->die_offset));
+	    m_entry_pool.append_unsigned_leb128 (entry->per_cu->dw_lang ());
+
+	    if (parent != nullptr)
+	      {
+		c_str_view par_name (parent->name);
+		auto name_iter = m_name_to_value_set.find (par_name);
+		gdb_assert (name_iter != m_name_to_value_set.end ());
+		gdb_assert (name_iter->second.index != 0);
+		m_entry_pool.append_unsigned_leb128 (name_iter->second.index);
+	      }
 	  }
+
+	/* Terminate the list of entries.  */
+	m_entry_pool.append_unsigned_leb128 (0);
       }
-    gdb_assert (m_hash_table.size () == name_count);
 
     /* Terminate tags list.  */
     m_abbrev_table.append_unsigned_leb128 (0);
   }
 
-  /* Return .debug_names bucket count.  This must be called only after
-     calling the build method.  */
-  uint32_t bucket_count () const
-  {
-    /* Verify the build method has been already called.  */
-    gdb_assert (!m_abbrev_table.empty ());
-    const uint32_t retval = m_bucket_table.size ();
-
-    /* Check for overflow.  */
-    gdb_assert (retval == m_bucket_table.size ());
-    return retval;
-  }
-
   /* Return .debug_names names count.  This must be called only after
      calling the build method.  */
   uint32_t name_count () const
   {
     /* Verify the build method has been already called.  */
     gdb_assert (!m_abbrev_table.empty ());
-    const uint32_t retval = m_hash_table.size ();
-
-    /* Check for overflow.  */
-    gdb_assert (retval == m_hash_table.size ());
-    return retval;
+    return m_name_to_value_set.size ();
   }
 
   /* Return number of bytes of .debug_names abbreviation table.  This
@@ -880,8 +841,6 @@ class debug_names
     /* Verify the build method has been already called.  */
     gdb_assert (!m_abbrev_table.empty ());
     size_t expected_bytes = 0;
-    expected_bytes += m_bucket_table.size () * sizeof (m_bucket_table[0]);
-    expected_bytes += m_hash_table.size () * sizeof (m_hash_table[0]);
     expected_bytes += m_name_table_string_offs.bytes ();
     expected_bytes += m_name_table_entry_offs.bytes ();
     expected_bytes += m_abbrev_table.size ();
@@ -896,8 +855,6 @@ class debug_names
   {
     /* Verify the build method has been already called.  */
     gdb_assert (!m_abbrev_table.empty ());
-    ::file_write (file_names, m_bucket_table);
-    ::file_write (file_names, m_hash_table);
     m_name_table_string_offs.file_write (file_names);
     m_name_table_entry_offs.file_write (file_names);
     m_abbrev_table.file_write (file_names);
@@ -918,29 +875,11 @@ class debug_names
   {
   public:
 
-    /* Object constructor to be called for current DWARF2_PER_BFD.
-       All .debug_str section strings are automatically stored.  */
+    /* Object constructor to be called for current DWARF2_PER_BFD.  */
     debug_str_lookup (dwarf2_per_bfd *per_bfd)
       : m_abfd (per_bfd->obfd),
 	m_per_bfd (per_bfd)
     {
-      gdb_assert (per_bfd->str.readin);
-      if (per_bfd->str.buffer == NULL)
-	return;
-      for (const gdb_byte *data = per_bfd->str.buffer;
-	   data < (per_bfd->str.buffer
-		   + per_bfd->str.size);)
-	{
-	  const char *const s = reinterpret_cast<const char *> (data);
-	  const auto insertpair
-	    = m_str_table.emplace (c_str_view (s),
-				   data - per_bfd->str.buffer);
-	  if (!insertpair.second)
-	    complaint (_("Duplicate string \"%s\" in "
-			 ".debug_str section [in module %s]"),
-		       s, bfd_get_filename (m_abfd));
-	  data += strlen (s) + 1;
-	}
     }
 
     /* Return offset of symbol name S in the .debug_str section.  Add
@@ -948,6 +887,13 @@ class debug_names
        yet.  */
     size_t lookup (const char *s)
     {
+      /* Most strings will have come from the string table
+	 already.  */
+      const gdb_byte *b = (const gdb_byte *) s;
+      if (b >= m_per_bfd->str.buffer
+	  && b < m_per_bfd->str.buffer + m_per_bfd->str.size)
+	return b - m_per_bfd->str.buffer;
+
       const auto it = m_str_table.find (c_str_view (s));
       if (it != m_str_table.end ())
 	return it->second;
@@ -978,66 +924,41 @@ class debug_names
   class index_key
   {
   public:
-    index_key (int dwarf_tag_, bool is_static_, unit_kind kind_)
-      : dwarf_tag (dwarf_tag_), is_static (is_static_), kind (kind_)
+    index_key (dwarf_tag tag_, unit_kind kind_, cooked_index_flag flags_,
+	       enum language lang_, bool has_parent_)
+      : tag (tag_),
+	kind (kind_),
+	flags (flags_ & ~IS_TYPE_DECLARATION),
+	lang (lang_),
+	has_parent (has_parent_)
     {
     }
 
-    bool
-    operator== (const index_key &other) const
+    bool operator== (const index_key &other) const
     {
-      return (dwarf_tag == other.dwarf_tag && is_static == other.is_static
-	      && kind == other.kind);
+      return (tag == other.tag
+	      && kind == other.kind
+	      && flags == other.flags
+	      && lang == other.lang
+	      && has_parent == other.has_parent);
     }
 
-    const int dwarf_tag;
-    const bool is_static;
+    const dwarf_tag tag;
     const unit_kind kind;
+    const cooked_index_flag flags;
+    const enum language lang;
+    const bool has_parent;
   };
 
   /* Provide std::unordered_map::hasher for index_key.  */
   class index_key_hasher
   {
   public:
-    size_t
-    operator () (const index_key &key) const
-    {
-      return (std::hash<int>() (key.dwarf_tag) << 1) | key.is_static;
-    }
-  };
-
-  /* Parameters of one symbol entry.  */
-  class symbol_value
-  {
-  public:
-    const int dwarf_tag, cu_index;
-    const bool is_static;
-    const unit_kind kind;
-
-    symbol_value (int dwarf_tag_, int cu_index_, bool is_static_,
-		  unit_kind kind_)
-      : dwarf_tag (dwarf_tag_), cu_index (cu_index_), is_static (is_static_),
-	kind (kind_)
-    {}
-
-    bool
-    operator< (const symbol_value &other) const
+    size_t operator () (const index_key &key) const
     {
-#define X(n) \
-  do \
-    { \
-      if (n < other.n) \
-	return true; \
-      if (n > other.n) \
-	return false; \
-    } \
-  while (0)
-      X (dwarf_tag);
-      X (is_static);
-      X (kind);
-      X (cu_index);
-#undef X
-      return false;
+      return (std::hash<int>() (key.tag)
+	      ^ std::hash<int>() (key.flags)
+	      ^ std::hash<int>() (key.lang));
     }
   };
 
@@ -1139,14 +1060,15 @@ class debug_names
     offset_vec_tmpl<OffsetSize> m_name_table_entry_offs;
   };
 
-  /* Store value of each symbol.  */
-  std::unordered_map<c_str_view, std::set<symbol_value>, c_str_view_hasher>
-    m_name_to_value_set;
+  struct entry_list
+  {
+    unsigned index = 0;
+    std::vector<const cooked_index_entry *> entries;
+  };
 
-  /* Tables of DWARF-5 .debug_names.  They are in object file byte
-     order.  */
-  std::vector<uint32_t> m_bucket_table;
-  std::vector<uint32_t> m_hash_table;
+  /* Store value of each symbol.  Note that we rely on the sorting
+     behavior of map to make the output stable.  */
+  std::map<c_str_view, entry_list> m_name_to_value_set;
 
   const bfd_endian m_dwarf5_byte_order;
   dwarf_tmpl<uint32_t> m_dwarf32;
@@ -1159,10 +1081,6 @@ class debug_names
      index value.  */
   std::unordered_map<index_key, int, index_key_hasher> m_indexkey_to_idx;
 
-  /* Next unused .debug_names abbreviation tag for
-     m_indexkey_to_idx.  */
-  int m_idx_next = 1;
-
   /* .debug_names abbreviation table.  */
   data_buf m_abbrev_table;
 
@@ -1441,9 +1359,6 @@ write_gdbindex (dwarf2_per_bfd *per_bfd, cooked_index *table,
     gdb_assert (dwz_cu_list.empty ());
 }
 
-/* DWARF-5 augmentation string for GDB's DW_IDX_GNU_* extension.  */
-static const gdb_byte dwarf5_gdb_augmentation[] = { 'G', 'D', 'B', 0 };
-
 /* Write a new .debug_names section for OBJFILE into OUT_FILE, write
    needed addition to .debug_str section to OUT_FILE_STR.  Return how
    many bytes were expected to be written into OUT_FILE.  */
@@ -1492,7 +1407,7 @@ write_debug_names (dwarf2_per_bfd *per_bfd, cooked_index *table,
   const offset_type bytes_of_header
     = ((dwarf5_is_dwarf64 ? 12 : 4)
        + 2 + 2 + 7 * 4
-       + sizeof (dwarf5_gdb_augmentation));
+       + sizeof (dwarf5_augmentation));
   size_t expected_bytes = 0;
   expected_bytes += bytes_of_header;
   expected_bytes += cu_list.size ();
@@ -1530,8 +1445,10 @@ write_debug_names (dwarf2_per_bfd *per_bfd, cooked_index *table,
   header.append_uint (4, dwarf5_byte_order, 0);
 
   /* bucket_count - The number of hash buckets in the hash lookup
-     table.  */
-  header.append_uint (4, dwarf5_byte_order, nametable.bucket_count ());
+     table.  GDB does not use the hash table, so there's also no need
+     to write it -- plus, the hash table is broken as defined due to
+     the lack of name canonicalization.  */
+  header.append_uint (4, dwarf5_byte_order, 0);
 
   /* name_count - The number of unique names in the index.  */
   header.append_uint (4, dwarf5_byte_order, nametable.name_count ());
@@ -1542,9 +1459,9 @@ write_debug_names (dwarf2_per_bfd *per_bfd, cooked_index *table,
 
   /* augmentation_string_size - The size in bytes of the augmentation
      string.  This value is rounded up to a multiple of 4.  */
-  static_assert (sizeof (dwarf5_gdb_augmentation) % 4 == 0, "");
-  header.append_uint (4, dwarf5_byte_order, sizeof (dwarf5_gdb_augmentation));
-  header.append_array (dwarf5_gdb_augmentation);
+  static_assert (sizeof (dwarf5_augmentation) % 4 == 0);
+  header.append_uint (4, dwarf5_byte_order, sizeof (dwarf5_augmentation));
+  header.append_array (dwarf5_augmentation);
 
   gdb_assert (header.size () == bytes_of_header);
 

-- 
2.43.0


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 05/17] Document GDB extensions to DWARF .debug_names
  2023-12-10 16:44 ` [PATCH 05/17] Document GDB extensions to DWARF .debug_names Tom Tromey
@ 2023-12-10 17:37   ` Eli Zaretskii
  2024-01-17 16:24     ` Tom Tromey
  0 siblings, 1 reply; 22+ messages in thread
From: Eli Zaretskii @ 2023-12-10 17:37 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

> From: Tom Tromey <tom@tromey.com>
> Date: Sun, 10 Dec 2023 09:44:54 -0700
> 
> GDB's new .debug_names implementation uses some extensions.  This
> patch adds some documentation on this to the manual.
> ---
>  gdb/doc/gdb.texinfo | 39 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 39 insertions(+)

Thanks.

> +@node Debug Names
> +@section Extensions to @samp{.debug_names}
> +@cindex index files
> +@cindex @samp{.debug_names} section
> +
> +The DWARF specification documents an optional index section called
> +@samp{.debug_names}.  @value{GDBN} can both read and create this
> +section.  However, in order to work with @value{GDBN}, some extensions
> +were necessary.
> +
> +@value{GDBN} uses the augmentation string @samp{GDB2}.  Earlier
> +versions used the string @samp{GDB}, but these versions of the index
> +are no longer supported.
> +
> +@value{GDBN} does not use the specified hash table.  Therefore,
> +because this hash table is optional, @value{GDBN} also does not write
> +it.
> +
> +@value{GDBN} also generates and uses some extra index attributes:
> +@table @code
> +@item DW_IDX_GNU_internal
> +This has the value @samp{0x2000}.  It is a flag that, when set,
> +indicates that the associated entry has @code{static} linkage.
> +
> +@item DW_IDX_GNU_main
> +This has the value @samp{0x2002}.  It is a flag that, when set,
> +indicates that the associated entry is the program's @code{main}.
> +
> +@item DW_IDX_GNU_language
> +This has the value @samp{0x2003}.  It is @samp{DW_LANG_} constant,
> +indicating the language of the associated entry.
> +
> +@item DW_IDX_GNU_linkage_name
> +This has the value @samp{0x2004}.  It is a flag that, when set,
> +indicates that the associated entry is a linkage name, and not a
> +source name.
> +@end table
> +

After reading this, I wonder what is the purpose of having this i the
manual.  This section seems to assume the reader is familiar with the
DWARF spec, including some of its very intimate details.  Is that
really the case?  For example, the part about the "augmentation
string", or the description of the extra index attributes -- what kind
of readers of the manual will need this information, and for what
purposes?

But if this information is deemed useful, I have no objections for it
being included, and it's fine by me, both English-wise and
markup-wise.

Reviewed-By: Eli Zaretskii <eliz@gnu.org>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 04/17] Add some new DW_IDX_* values
  2023-12-10 16:44 ` [PATCH 04/17] Add some new DW_IDX_* values Tom Tromey
@ 2024-01-09 15:08   ` Tom Tromey
  2024-01-09 19:02     ` Tom Tromey
  0 siblings, 1 reply; 22+ messages in thread
From: Tom Tromey @ 2024-01-09 15:08 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:

Tom> The new .debug_names writer in GDB needs to emit some symbol
Tom> properties, so that the reader can be fully functional.  This patch
Tom> adds a few new DW_IDX_* constants, and tries to document the existing
Tom> extensions as well.  (I will add more documentation of these to the
Tom> GDB manual as well.)

I sent this particular patch to gcc as well.  It was approved there and
I applied it, so I'm going to apply it to gdb now as well.

This doesn't materially impact the rest of this series.

Tom

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 04/17] Add some new DW_IDX_* values
  2024-01-09 15:08   ` Tom Tromey
@ 2024-01-09 19:02     ` Tom Tromey
  0 siblings, 0 replies; 22+ messages in thread
From: Tom Tromey @ 2024-01-09 19:02 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

Tom> The new .debug_names writer in GDB needs to emit some symbol
Tom> properties, so that the reader can be fully functional.  This patch
Tom> adds a few new DW_IDX_* constants, and tries to document the existing
Tom> extensions as well.  (I will add more documentation of these to the
Tom> GDB manual as well.)

> I sent this particular patch to gcc as well.  It was approved there and
> I applied it, so I'm going to apply it to gdb now as well.

It turns out I did this in December and then forgot about it.

Tom

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 05/17] Document GDB extensions to DWARF .debug_names
  2023-12-10 17:37   ` Eli Zaretskii
@ 2024-01-17 16:24     ` Tom Tromey
  0 siblings, 0 replies; 22+ messages in thread
From: Tom Tromey @ 2024-01-17 16:24 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Tom Tromey, gdb-patches

>>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:

Eli> After reading this, I wonder what is the purpose of having this i the
Eli> manual.  This section seems to assume the reader is familiar with the
Eli> DWARF spec, including some of its very intimate details.  Is that
Eli> really the case?  For example, the part about the "augmentation
Eli> string", or the description of the extra index attributes -- what kind
Eli> of readers of the manual will need this information, and for what
Eli> purposes?

Eli> But if this information is deemed useful, I have no objections for it
Eli> being included, and it's fine by me, both English-wise and
Eli> markup-wise.

gdb has expectations for this section that go beyond what DWARF
specifies.  So, this text is technical documentation about a format used
by gdb, for other people working on debuginfo tools; sort of like how
.gdb_index is also documented.

Tom

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2024-01-17 16:24 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-10 16:44 [PATCH 00/17] Rewrite .debug_names reader and writer Tom Tromey
2023-12-10 16:44 ` [PATCH 01/17] Refactor 'maint set dwarf synchronous' handling Tom Tromey
2023-12-10 16:44 ` [PATCH 02/17] Refactor quick-function installation in DWARF reader Tom Tromey
2023-12-10 16:44 ` [PATCH 03/17] Remove IS_ENUM_CLASS from cooked_index_flag Tom Tromey
2023-12-10 16:44 ` [PATCH 04/17] Add some new DW_IDX_* values Tom Tromey
2024-01-09 15:08   ` Tom Tromey
2024-01-09 19:02     ` Tom Tromey
2023-12-10 16:44 ` [PATCH 05/17] Document GDB extensions to DWARF .debug_names Tom Tromey
2023-12-10 17:37   ` Eli Zaretskii
2024-01-17 16:24     ` Tom Tromey
2023-12-10 16:44 ` [PATCH 06/17] Add language to cooked_index_entry Tom Tromey
2023-12-10 16:44 ` [PATCH 07/17] Move cooked_index_functions to cooked-index.h Tom Tromey
2023-12-10 16:44 ` [PATCH 08/17] Do not write the index cache from an index Tom Tromey
2023-12-10 16:44 ` [PATCH 09/17] Change cooked_index_worker to abstract base class Tom Tromey
2023-12-10 16:44 ` [PATCH 10/17] Remove cooked_index_worker::start_reading Tom Tromey
2023-12-10 16:45 ` [PATCH 11/17] Empty hash table fix in .debug_names reader Tom Tromey
2023-12-10 16:45 ` [PATCH 12/17] Fix dw2-zero-range.exp when an index is in use Tom Tromey
2023-12-10 16:45 ` [PATCH 13/17] Explicitly expand CUs in dw2-inline-with-lexical-scope.exp Tom Tromey
2023-12-10 16:45 ` [PATCH 14/17] Remove some .debug_names tests Tom Tromey
2023-12-10 16:45 ` [PATCH 15/17] Rewrite .debug_names reader Tom Tromey
2023-12-10 16:45 ` [PATCH 16/17] Export dwarf5_augmentation Tom Tromey
2023-12-10 16:45 ` [PATCH 17/17] Rewrite .debug_names writer Tom Tromey

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).