public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 0/9] Unify the DWARF index fields
@ 2022-04-20  4:28 Tom Tromey
  2022-04-20  4:28 ` [PATCH 1/9] Move mapped_index_base to new header file Tom Tromey
                   ` (8 more replies)
  0 siblings, 9 replies; 17+ messages in thread
From: Tom Tromey @ 2022-04-20  4:28 UTC (permalink / raw)
  To: gdb-patches

Currently the DWARF reader uses three fields to represent the various
indexes -- but only one can be non-null at a time:

-  /* The mapped index, or NULL if .gdb_index is missing or not being used.  */
-  std::unique_ptr<mapped_index> index_table;
-
-  /* The mapped index, or NULL if .debug_names is missing or not being used.  */
-  std::unique_ptr<mapped_debug_names> debug_names_table;
-
-  /* The cooked index, or NULL if not using one.  */
-  std::unique_ptr<cooked_index_vector> cooked_index_table;

There are some other quirks in here as well, for example there are
special cases for -readnow (that are implemented in multiple indices
but only ever needed in one), and two of these classes share a base
class but intentionally do not have a virtual destructor.

This series cleans up all the weirdness here.

Regression tested on x86-64 Fedora 34.

Tom



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

* [PATCH 1/9] Move mapped_index_base to new header file
  2022-04-20  4:28 [PATCH 0/9] Unify the DWARF index fields Tom Tromey
@ 2022-04-20  4:28 ` Tom Tromey
  2022-04-20 20:38   ` Lancelot SIX
  2022-04-20  4:28 ` [PATCH 2/9] Give mapped_index_base a virtual destructor Tom Tromey
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 17+ messages in thread
From: Tom Tromey @ 2022-04-20  4:28 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This moves mapped_index_base and the helper struct name_component to a
new header file in gdb/dwarf2/.
---
 gdb/dwarf2/mapped-index.h | 97 +++++++++++++++++++++++++++++++++++++++
 gdb/dwarf2/read.c         | 72 -----------------------------
 gdb/dwarf2/read.h         |  1 +
 3 files changed, 98 insertions(+), 72 deletions(-)
 create mode 100644 gdb/dwarf2/mapped-index.h

diff --git a/gdb/dwarf2/mapped-index.h b/gdb/dwarf2/mapped-index.h
new file mode 100644
index 00000000000..c2ff27ac4f5
--- /dev/null
+++ b/gdb/dwarf2/mapped-index.h
@@ -0,0 +1,97 @@
+/* Base class for mapped indices
+
+   Copyright (C) 2021 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef GDB_DWARF2_MAPPED_INDEX_H
+#define GDB_DWARF2_MAPPED_INDEX_H
+
+#include "language.h"
+
+/* An index into a (C++) symbol name component in a symbol name as
+   recorded in the mapped_index's symbol table.  For each C++ symbol
+   in the symbol table, we record one entry for the start of each
+   component in the symbol in a table of name components, and then
+   sort the table, in order to be able to binary search symbol names,
+   ignoring leading namespaces, both completion and regular look up.
+   For example, for symbol "A::B::C", we'll have an entry that points
+   to "A::B::C", another that points to "B::C", and another for "C".
+   Note that function symbols in GDB index have no parameter
+   information, just the function/method names.  You can convert a
+   name_component to a "const char *" using the
+   'mapped_index::symbol_name_at(offset_type)' method.  */
+
+struct name_component
+{
+  /* Offset in the symbol name where the component starts.  Stored as
+     a (32-bit) offset instead of a pointer to save memory and improve
+     locality on 64-bit architectures.  */
+  offset_type name_offset;
+
+  /* The symbol's index in the symbol and constant pool tables of a
+     mapped_index.  */
+  offset_type idx;
+};
+
+/* Base class containing bits shared by both .gdb_index and
+   .debug_name indexes.  */
+
+struct mapped_index_base
+{
+  mapped_index_base () = default;
+  DISABLE_COPY_AND_ASSIGN (mapped_index_base);
+
+  /* The name_component table (a sorted vector).  See name_component's
+     description above.  */
+  std::vector<name_component> name_components;
+
+  /* How NAME_COMPONENTS is sorted.  */
+  enum case_sensitivity name_components_casing;
+
+  /* Return the number of names in the symbol table.  */
+  virtual size_t symbol_name_count () const = 0;
+
+  /* Get the name of the symbol at IDX in the symbol table.  */
+  virtual const char *symbol_name_at
+    (offset_type idx, dwarf2_per_objfile *per_objfile) const = 0;
+
+  /* Return whether the name at IDX in the symbol table should be
+     ignored.  */
+  virtual bool symbol_name_slot_invalid (offset_type idx) const
+  {
+    return false;
+  }
+
+  /* Build the symbol name component sorted vector, if we haven't
+     yet.  */
+  void build_name_components (dwarf2_per_objfile *per_objfile);
+
+  /* Returns the lower (inclusive) and upper (exclusive) bounds of the
+     possible matches for LN_NO_PARAMS in the name component
+     vector.  */
+  std::pair<std::vector<name_component>::const_iterator,
+	    std::vector<name_component>::const_iterator>
+    find_name_components_bounds (const lookup_name_info &ln_no_params,
+				 enum language lang,
+				 dwarf2_per_objfile *per_objfile) const;
+
+  /* Prevent deleting/destroying via a base class pointer.  */
+protected:
+  ~mapped_index_base() = default;
+};
+
+#endif /* GDB_DWARF2_MAPPED_INDEX_H */
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 6dcd446e5f4..3f44c72565b 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -156,78 +156,6 @@ static int dwarf2_loclist_block_index;
 /* Size of .debug_rnglists section header for 64-bit DWARF format.  */
 #define RNGLIST_HEADER_SIZE64 20
 
-/* An index into a (C++) symbol name component in a symbol name as
-   recorded in the mapped_index's symbol table.  For each C++ symbol
-   in the symbol table, we record one entry for the start of each
-   component in the symbol in a table of name components, and then
-   sort the table, in order to be able to binary search symbol names,
-   ignoring leading namespaces, both completion and regular look up.
-   For example, for symbol "A::B::C", we'll have an entry that points
-   to "A::B::C", another that points to "B::C", and another for "C".
-   Note that function symbols in GDB index have no parameter
-   information, just the function/method names.  You can convert a
-   name_component to a "const char *" using the
-   'mapped_index::symbol_name_at(offset_type)' method.  */
-
-struct name_component
-{
-  /* Offset in the symbol name where the component starts.  Stored as
-     a (32-bit) offset instead of a pointer to save memory and improve
-     locality on 64-bit architectures.  */
-  offset_type name_offset;
-
-  /* The symbol's index in the symbol and constant pool tables of a
-     mapped_index.  */
-  offset_type idx;
-};
-
-/* Base class containing bits shared by both .gdb_index and
-   .debug_name indexes.  */
-
-struct mapped_index_base
-{
-  mapped_index_base () = default;
-  DISABLE_COPY_AND_ASSIGN (mapped_index_base);
-
-  /* The name_component table (a sorted vector).  See name_component's
-     description above.  */
-  std::vector<name_component> name_components;
-
-  /* How NAME_COMPONENTS is sorted.  */
-  enum case_sensitivity name_components_casing;
-
-  /* Return the number of names in the symbol table.  */
-  virtual size_t symbol_name_count () const = 0;
-
-  /* Get the name of the symbol at IDX in the symbol table.  */
-  virtual const char *symbol_name_at
-    (offset_type idx, dwarf2_per_objfile *per_objfile) const = 0;
-
-  /* Return whether the name at IDX in the symbol table should be
-     ignored.  */
-  virtual bool symbol_name_slot_invalid (offset_type idx) const
-  {
-    return false;
-  }
-
-  /* Build the symbol name component sorted vector, if we haven't
-     yet.  */
-  void build_name_components (dwarf2_per_objfile *per_objfile);
-
-  /* Returns the lower (inclusive) and upper (exclusive) bounds of the
-     possible matches for LN_NO_PARAMS in the name component
-     vector.  */
-  std::pair<std::vector<name_component>::const_iterator,
-	    std::vector<name_component>::const_iterator>
-    find_name_components_bounds (const lookup_name_info &ln_no_params,
-				 enum language lang,
-				 dwarf2_per_objfile *per_objfile) const;
-
-  /* Prevent deleting/destroying via a base class pointer.  */
-protected:
-  ~mapped_index_base() = default;
-};
-
 /* This is a view into the index that converts from bytes to an
    offset_type, and allows indexing.  Unaligned bytes are specifically
    allowed here, and handled via unpacking.  */
diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h
index be3b9d19601..f3b09c63b64 100644
--- a/gdb/dwarf2/read.h
+++ b/gdb/dwarf2/read.h
@@ -26,6 +26,7 @@
 #include "dwarf2/cooked-index.h"
 #include "dwarf2/file-and-dir.h"
 #include "dwarf2/index-cache.h"
+#include "dwarf2/mapped-index.h"
 #include "dwarf2/section.h"
 #include "filename-seen-cache.h"
 #include "gdbsupport/gdb_obstack.h"
-- 
2.34.1


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

* [PATCH 2/9] Give mapped_index_base a virtual destructor
  2022-04-20  4:28 [PATCH 0/9] Unify the DWARF index fields Tom Tromey
  2022-04-20  4:28 ` [PATCH 1/9] Move mapped_index_base to new header file Tom Tromey
@ 2022-04-20  4:28 ` Tom Tromey
  2022-04-20  4:29 ` [PATCH 3/9] Let mapped index classes create the quick_symbol_functions object Tom Tromey
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: Tom Tromey @ 2022-04-20  4:28 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This changes mapped_index_base to have a virtual destructor, so it can
be destroyed via its base class.
---
 gdb/dwarf2/mapped-index.h | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/gdb/dwarf2/mapped-index.h b/gdb/dwarf2/mapped-index.h
index c2ff27ac4f5..fb6828e43df 100644
--- a/gdb/dwarf2/mapped-index.h
+++ b/gdb/dwarf2/mapped-index.h
@@ -53,6 +53,7 @@ struct name_component
 struct mapped_index_base
 {
   mapped_index_base () = default;
+  virtual ~mapped_index_base() = default;
   DISABLE_COPY_AND_ASSIGN (mapped_index_base);
 
   /* The name_component table (a sorted vector).  See name_component's
@@ -88,10 +89,6 @@ struct mapped_index_base
     find_name_components_bounds (const lookup_name_info &ln_no_params,
 				 enum language lang,
 				 dwarf2_per_objfile *per_objfile) const;
-
-  /* Prevent deleting/destroying via a base class pointer.  */
-protected:
-  ~mapped_index_base() = default;
 };
 
 #endif /* GDB_DWARF2_MAPPED_INDEX_H */
-- 
2.34.1


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

* [PATCH 3/9] Let mapped index classes create the quick_symbol_functions object
  2022-04-20  4:28 [PATCH 0/9] Unify the DWARF index fields Tom Tromey
  2022-04-20  4:28 ` [PATCH 1/9] Move mapped_index_base to new header file Tom Tromey
  2022-04-20  4:28 ` [PATCH 2/9] Give mapped_index_base a virtual destructor Tom Tromey
@ 2022-04-20  4:29 ` Tom Tromey
  2022-04-20  4:29 ` [PATCH 4/9] Remove some "OBJF_READNOW" code from dwarf2_debug_names_index Tom Tromey
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: Tom Tromey @ 2022-04-20  4:29 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This changes the mapped index classes to create the
quick_symbol_functions objects.  This is a step toward having a more
abstract interface to mapped indices.
---
 gdb/dwarf2/mapped-index.h |  4 ++++
 gdb/dwarf2/read.c         | 31 ++++++++++++++++++++++++-------
 2 files changed, 28 insertions(+), 7 deletions(-)

diff --git a/gdb/dwarf2/mapped-index.h b/gdb/dwarf2/mapped-index.h
index fb6828e43df..9f51a9e6578 100644
--- a/gdb/dwarf2/mapped-index.h
+++ b/gdb/dwarf2/mapped-index.h
@@ -77,6 +77,10 @@ struct mapped_index_base
     return false;
   }
 
+  /* Return a quick_symbol_functions instance that refers back to this
+     mapped_index_base.  */
+  virtual quick_symbol_functions_up make_quick_functions () const = 0;
+
   /* Build the symbol name component sorted vector, if we haven't
      yet.  */
   void build_name_components (dwarf2_per_objfile *per_objfile);
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 3f44c72565b..c0bb18a26f3 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -243,6 +243,8 @@ struct mapped_index final : public mapped_index_base
 
   size_t symbol_name_count () const override
   { return this->symbol_table.size () / 2; }
+
+  quick_symbol_functions_up make_quick_functions () const override;
 };
 
 /* A description of the mapped .debug_names.
@@ -292,6 +294,8 @@ struct mapped_debug_names final : public mapped_index_base
 
   size_t symbol_name_count () const override
   { return this->name_count; }
+
+  quick_symbol_functions_up make_quick_functions () const override;
 };
 
 /* See dwarf2/read.h.  */
@@ -1887,8 +1891,14 @@ make_dwarf_gdb_index ()
   return quick_symbol_functions_up (new dwarf2_gdb_index);
 }
 
-static quick_symbol_functions_up
-make_dwarf_debug_names ()
+quick_symbol_functions_up
+mapped_index::make_quick_functions () const
+{
+  return quick_symbol_functions_up (new dwarf2_gdb_index);
+}
+
+quick_symbol_functions_up
+mapped_debug_names::make_quick_functions () const
 {
   return quick_symbol_functions_up (new dwarf2_debug_names_index);
 }
@@ -3494,6 +3504,11 @@ class mock_mapped_index : public mapped_index_base
     return m_symbol_table[idx];
   }
 
+  quick_symbol_functions_up make_quick_functions () const override
+  {
+    return nullptr;
+  }
+
 private:
   gdb::array_view<const char *> m_symbol_table;
 };
@@ -5281,7 +5296,8 @@ dwarf2_initialize_objfile (struct objfile *objfile)
   if (per_bfd->debug_names_table != nullptr)
     {
       dwarf_read_debug_printf ("re-using shared debug names table");
-      objfile->qf.push_front (make_dwarf_debug_names ());
+      objfile->qf.push_front
+	(per_bfd->debug_names_table->make_quick_functions ());
       return;
     }
 
@@ -5290,7 +5306,7 @@ dwarf2_initialize_objfile (struct objfile *objfile)
   if (per_bfd->index_table != nullptr)
     {
       dwarf_read_debug_printf ("re-using shared index table");
-      objfile->qf.push_front (make_dwarf_gdb_index ());
+      objfile->qf.push_front (per_bfd->index_table->make_quick_functions ());
       return;
     }
 
@@ -5304,7 +5320,8 @@ dwarf2_initialize_objfile (struct objfile *objfile)
   if (dwarf2_read_debug_names (per_objfile))
     {
       dwarf_read_debug_printf ("found debug names");
-      objfile->qf.push_front (make_dwarf_debug_names ());
+      objfile->qf.push_front
+	(per_bfd->debug_names_table->make_quick_functions ());
       return;
     }
 
@@ -5313,7 +5330,7 @@ dwarf2_initialize_objfile (struct objfile *objfile)
 			     get_gdb_index_contents_from_section<dwz_file>))
     {
       dwarf_read_debug_printf ("found gdb index from file");
-      objfile->qf.push_front (make_dwarf_gdb_index ());
+      objfile->qf.push_front (per_bfd->index_table->make_quick_functions ());
       return;
     }
 
@@ -5324,7 +5341,7 @@ dwarf2_initialize_objfile (struct objfile *objfile)
     {
       dwarf_read_debug_printf ("found gdb index from cache");
       global_index_cache.hit ();
-      objfile->qf.push_front (make_dwarf_gdb_index ());
+      objfile->qf.push_front (per_bfd->index_table->make_quick_functions ());
       return;
     }
 
-- 
2.34.1


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

* [PATCH 4/9] Remove some "OBJF_READNOW" code from dwarf2_debug_names_index
  2022-04-20  4:28 [PATCH 0/9] Unify the DWARF index fields Tom Tromey
                   ` (2 preceding siblings ...)
  2022-04-20  4:29 ` [PATCH 3/9] Let mapped index classes create the quick_symbol_functions object Tom Tromey
@ 2022-04-20  4:29 ` Tom Tromey
  2022-04-20  4:29 ` [PATCH 5/9] Introduce readnow_functions Tom Tromey
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: Tom Tromey @ 2022-04-20  4:29 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

The dwarf2_debug_names_index code treats a NULL debug_names_table as
if it were from OBJF_READNOW.  However, this trick is only done for
gdb_index, never for debug_names -- see dwarf2_initialize_objfile.
---
 gdb/dwarf2/read.c | 17 +----------------
 1 file changed, 1 insertion(+), 16 deletions(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index c0bb18a26f3..165bea9c5a8 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -5096,14 +5096,7 @@ dw2_debug_names_iterator::next ()
 void
 dwarf2_debug_names_index::dump (struct objfile *objfile)
 {
-  dwarf2_per_objfile *per_objfile = get_dwarf2_per_objfile (objfile);
-
-  gdb_printf (".debug_names:");
-  if (per_objfile->per_bfd->debug_names_table)
-    gdb_printf (" exists\n");
-  else
-    gdb_printf (" faked for \"readnow\"\n");
-  gdb_printf ("\n");
+  gdb_printf (".debug_names: exists\n");
 }
 
 void
@@ -5115,10 +5108,6 @@ dwarf2_debug_names_index::expand_matching_symbols
 {
   dwarf2_per_objfile *per_objfile = get_dwarf2_per_objfile (objfile);
 
-  /* debug_names_table is NULL if OBJF_READNOW.  */
-  if (!per_objfile->per_bfd->debug_names_table)
-    return;
-
   mapped_debug_names &map = *per_objfile->per_bfd->debug_names_table;
   const block_search_flags block_flags
     = global ? SEARCH_GLOBAL_BLOCK : SEARCH_STATIC_BLOCK;
@@ -5160,10 +5149,6 @@ dwarf2_debug_names_index::expand_symtabs_matching
 {
   dwarf2_per_objfile *per_objfile = get_dwarf2_per_objfile (objfile);
 
-  /* debug_names_table is NULL if OBJF_READNOW.  */
-  if (!per_objfile->per_bfd->debug_names_table)
-    return true;
-
   dw_expand_symtabs_matching_file_matcher (per_objfile, file_matcher);
 
   /* This invariant is documented in quick-functions.h.  */
-- 
2.34.1


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

* [PATCH 5/9] Introduce readnow_functions
  2022-04-20  4:28 [PATCH 0/9] Unify the DWARF index fields Tom Tromey
                   ` (3 preceding siblings ...)
  2022-04-20  4:29 ` [PATCH 4/9] Remove some "OBJF_READNOW" code from dwarf2_debug_names_index Tom Tromey
@ 2022-04-20  4:29 ` Tom Tromey
  2022-04-20  4:29 ` [PATCH 6/9] Introduce and use dwarf_scanner_base Tom Tromey
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: Tom Tromey @ 2022-04-20  4:29 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This introduces readnow_functions, a new subclass of
dwarf2_base_index_functions, and changes the DWARF reader to use it.
This lets us drop the "index is NULL" hack from the gdb index code.
---
 gdb/dwarf2/read.c | 109 ++++++++++++++++++++++++----------------------
 1 file changed, 56 insertions(+), 53 deletions(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 165bea9c5a8..83651f21d31 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -1841,6 +1841,40 @@ struct dwarf2_base_index_functions : public quick_symbol_functions
 			     bool need_fullname) override;
 };
 
+/* With OBJF_READNOW, the DWARF reader expands all CUs immediately.
+   It's handy in this case to have an empty implementation of the
+   quick symbol functions, to avoid special cases in the rest of the
+   code.  */
+
+struct readnow_functions : public dwarf2_base_index_functions
+{
+  void dump (struct objfile *objfile) override
+  {
+  }
+
+  void expand_matching_symbols
+    (struct objfile *,
+     const lookup_name_info &lookup_name,
+     domain_enum domain,
+     int global,
+     symbol_compare_ftype *ordered_compare) 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
+  {
+    return true;
+  }
+};
+
 struct dwarf2_gdb_index : public dwarf2_base_index_functions
 {
   void dump (struct objfile *objfile) override;
@@ -1885,12 +1919,6 @@ struct dwarf2_debug_names_index : public dwarf2_base_index_functions
      enum search_domain kind) override;
 };
 
-static quick_symbol_functions_up
-make_dwarf_gdb_index ()
-{
-  return quick_symbol_functions_up (new dwarf2_gdb_index);
-}
-
 quick_symbol_functions_up
 mapped_index::make_quick_functions () const
 {
@@ -2910,9 +2938,6 @@ dw2_symtab_iter_init (struct dw2_symtab_iterator *iter,
   iter->length = 0;
 
   mapped_index *index = per_objfile->per_bfd->index_table.get ();
-  /* index is NULL if OBJF_READNOW.  */
-  if (index == NULL)
-    return;
 
   gdb_assert (!index->symbol_name_slot_invalid (namei));
   offset_type vec_idx = index->symbol_vec_index (namei);
@@ -3048,14 +3073,8 @@ dwarf2_gdb_index::dump (struct objfile *objfile)
 {
   dwarf2_per_objfile *per_objfile = get_dwarf2_per_objfile (objfile);
 
-  gdb_printf (".gdb_index:");
-  if (per_objfile->per_bfd->index_table != NULL)
-    {
-      gdb_printf (" version %d\n",
-		  per_objfile->per_bfd->index_table->version);
-    }
-  else
-    gdb_printf (" faked for \"readnow\"\n");
+  gdb_printf (".gdb_index: version %d\n",
+	      per_objfile->per_bfd->index_table->version);
   gdb_printf ("\n");
 }
 
@@ -3105,37 +3124,28 @@ dwarf2_gdb_index::expand_matching_symbols
 
   const block_enum block_kind = global ? GLOBAL_BLOCK : STATIC_BLOCK;
 
-  if (per_objfile->per_bfd->index_table != nullptr)
-    {
-      mapped_index &index = *per_objfile->per_bfd->index_table;
+  mapped_index &index = *per_objfile->per_bfd->index_table;
 
-      const char *match_name = name.ada ().lookup_name ().c_str ();
-      auto matcher = [&] (const char *symname)
-	{
-	  if (ordered_compare == nullptr)
-	    return true;
-	  return ordered_compare (symname, match_name) == 0;
-	};
+  const char *match_name = name.ada ().lookup_name ().c_str ();
+  auto matcher = [&] (const char *symname)
+  {
+    if (ordered_compare == nullptr)
+      return true;
+    return ordered_compare (symname, match_name) == 0;
+  };
 
-      dw2_expand_symtabs_matching_symbol (index, name, matcher,
-					  [&] (offset_type namei)
-      {
-	struct dw2_symtab_iterator iter;
-	struct dwarf2_per_cu_data *per_cu;
-
-	dw2_symtab_iter_init (&iter, per_objfile, block_kind, domain,
-			      namei);
-	while ((per_cu = dw2_symtab_iter_next (&iter)) != NULL)
-	  dw2_expand_symtabs_matching_one (per_cu, per_objfile, nullptr,
-					   nullptr);
-	return true;
-      }, per_objfile);
-    }
-  else
+  dw2_expand_symtabs_matching_symbol (index, name, matcher,
+				      [&] (offset_type namei)
     {
-      /* We have -readnow: no .gdb_index, but no partial symtabs either.  So,
-	 proceed assuming all symtabs have been read in.  */
-    }
+      struct dw2_symtab_iterator iter;
+      struct dwarf2_per_cu_data *per_cu;
+
+      dw2_symtab_iter_init (&iter, per_objfile, block_kind, domain, namei);
+      while ((per_cu = dw2_symtab_iter_next (&iter)) != NULL)
+	dw2_expand_symtabs_matching_one (per_cu, per_objfile, nullptr,
+					 nullptr);
+      return true;
+    }, per_objfile);
 }
 
 /* Starting from a search name, return the string that finds the upper
@@ -4151,10 +4161,6 @@ dwarf2_gdb_index::expand_symtabs_matching
 {
   dwarf2_per_objfile *per_objfile = get_dwarf2_per_objfile (objfile);
 
-  /* index_table is NULL if OBJF_READNOW.  */
-  if (!per_objfile->per_bfd->index_table)
-    return true;
-
   dw_expand_symtabs_matching_file_matcher (per_objfile, file_matcher);
 
   /* This invariant is documented in quick-functions.h.  */
@@ -5269,10 +5275,7 @@ dwarf2_initialize_objfile (struct objfile *objfile)
       per_bfd->quick_file_names_table
 	= create_quick_file_names_table (per_bfd->all_comp_units.size ());
 
-      /* Arrange for gdb to see the "quick" functions.  However, these
-	 functions will be no-ops because we will have expanded all
-	 symtabs.  */
-      objfile->qf.push_front (make_dwarf_gdb_index ());
+      objfile->qf.emplace_front (new readnow_functions);
       return;
     }
 
-- 
2.34.1


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

* [PATCH 6/9] Introduce and use dwarf_scanner_base
  2022-04-20  4:28 [PATCH 0/9] Unify the DWARF index fields Tom Tromey
                   ` (4 preceding siblings ...)
  2022-04-20  4:29 ` [PATCH 5/9] Introduce readnow_functions Tom Tromey
@ 2022-04-20  4:29 ` Tom Tromey
  2022-04-20  4:29 ` [PATCH 7/9] Simplify version check in dw2_symtab_iter_next Tom Tromey
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: Tom Tromey @ 2022-04-20  4:29 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This introduces dwarf_scanner_base, a base class for all the index
readers in the DWARF code.  Then, it changes both mapped_index_base
and cooked_index_vector to derive from this new base class.
---
 gdb/dwarf2/cooked-index.h |  5 ++++-
 gdb/dwarf2/mapped-index.h | 20 ++++++++++++++------
 gdb/dwarf2/read.c         |  9 ++++++++-
 3 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/gdb/dwarf2/cooked-index.h b/gdb/dwarf2/cooked-index.h
index 4b52eaf93d0..e1ff05645c5 100644
--- a/gdb/dwarf2/cooked-index.h
+++ b/gdb/dwarf2/cooked-index.h
@@ -31,6 +31,7 @@
 #include "addrmap.h"
 #include "gdbsupport/iterator-range.h"
 #include "gdbsupport/thread-pool.h"
+#include "dwarf2/mapped-index.h"
 
 struct dwarf2_per_cu_data;
 
@@ -241,7 +242,7 @@ class cooked_index
    cooked_index_vector for storage and final indexing.  The index is
    made by iterating over the entries previously created.  */
 
-class cooked_index_vector
+class cooked_index_vector : public dwarf_scanner_base
 {
 public:
 
@@ -291,6 +292,8 @@ class cooked_index_vector
      "main".  This will return NULL if no such entry is available.  */
   const cooked_index_entry *get_main () const;
 
+  quick_symbol_functions_up make_quick_functions () const override;
+
 private:
 
   /* GNAT only emits mangled ("encoded") names in the DWARF, and does
diff --git a/gdb/dwarf2/mapped-index.h b/gdb/dwarf2/mapped-index.h
index 9f51a9e6578..39a9ede4896 100644
--- a/gdb/dwarf2/mapped-index.h
+++ b/gdb/dwarf2/mapped-index.h
@@ -47,13 +47,25 @@ struct name_component
   offset_type idx;
 };
 
+/* Base class of all DWARF scanner types.  */
+
+struct dwarf_scanner_base
+{
+  dwarf_scanner_base () = default;
+  virtual ~dwarf_scanner_base () = default;
+  DISABLE_COPY_AND_ASSIGN (dwarf_scanner_base);
+
+  /* Return a quick_symbol_functions instance that refers back to this
+     dwarf_scanner_base.  */
+  virtual quick_symbol_functions_up make_quick_functions () const = 0;
+};
+
 /* Base class containing bits shared by both .gdb_index and
    .debug_name indexes.  */
 
-struct mapped_index_base
+struct mapped_index_base : public dwarf_scanner_base
 {
   mapped_index_base () = default;
-  virtual ~mapped_index_base() = default;
   DISABLE_COPY_AND_ASSIGN (mapped_index_base);
 
   /* The name_component table (a sorted vector).  See name_component's
@@ -77,10 +89,6 @@ struct mapped_index_base
     return false;
   }
 
-  /* Return a quick_symbol_functions instance that refers back to this
-     mapped_index_base.  */
-  virtual quick_symbol_functions_up make_quick_functions () const = 0;
-
   /* Build the symbol name component sorted vector, if we haven't
      yet.  */
   void build_name_components (dwarf2_per_objfile *per_objfile);
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 83651f21d31..3053ec9b303 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -5301,7 +5301,8 @@ dwarf2_initialize_objfile (struct objfile *objfile)
   if (per_bfd->cooked_index_table != nullptr)
     {
       dwarf_read_debug_printf ("re-using cooked index table");
-      objfile->qf.push_front (make_cooked_index_funcs ());
+      objfile->qf.push_front
+	(per_bfd->cooked_index_table->make_quick_functions ());
       return;
     }
 
@@ -18682,6 +18683,12 @@ make_cooked_index_funcs ()
   return quick_symbol_functions_up (new cooked_index_functions);
 }
 
+quick_symbol_functions_up
+cooked_index_vector::make_quick_functions () const
+{
+  return make_cooked_index_funcs ();
+}
+
 \f
 
 /* Read the .debug_loclists or .debug_rnglists header (they are the same format)
-- 
2.34.1


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

* [PATCH 7/9] Simplify version check in dw2_symtab_iter_next
  2022-04-20  4:28 [PATCH 0/9] Unify the DWARF index fields Tom Tromey
                   ` (5 preceding siblings ...)
  2022-04-20  4:29 ` [PATCH 6/9] Introduce and use dwarf_scanner_base Tom Tromey
@ 2022-04-20  4:29 ` Tom Tromey
  2022-04-20 20:49   ` Lancelot SIX
  2022-04-20  4:29 ` [PATCH 8/9] Add an ad hoc version check to dwarf_scanner_base Tom Tromey
  2022-04-20  4:29 ` [PATCH 9/9] Unify the DWARF index holders Tom Tromey
  8 siblings, 1 reply; 17+ messages in thread
From: Tom Tromey @ 2022-04-20  4:29 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This simplifies the index versio check in dw2_symtab_iter_next, by
passing a reference to the index object to this function.  This avoids
an indirection via the per_bfd object.
---
 gdb/dwarf2/read.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 3053ec9b303..f340cd68e67 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -2949,7 +2949,8 @@ dw2_symtab_iter_init (struct dw2_symtab_iterator *iter,
 /* Return the next matching CU or NULL if there are no more.  */
 
 static struct dwarf2_per_cu_data *
-dw2_symtab_iter_next (struct dw2_symtab_iterator *iter)
+dw2_symtab_iter_next (struct dw2_symtab_iterator *iter,
+		      mapped_index &index)
 {
   dwarf2_per_objfile *per_objfile = iter->per_objfile;
 
@@ -2963,9 +2964,8 @@ dw2_symtab_iter_next (struct dw2_symtab_iterator *iter)
 	 Indices prior to version 7 don't record them,
 	 and indices >= 7 may elide them for certain symbols
 	 (gold does this).  */
-      int attrs_valid =
-	(per_objfile->per_bfd->index_table->version >= 7
-	 && symbol_kind != GDB_INDEX_SYMBOL_KIND_NONE);
+      int attrs_valid = (index.version >= 7
+			 && symbol_kind != GDB_INDEX_SYMBOL_KIND_NONE);
 
       /* Don't crash on bad data.  */
       if (cu_index >= per_objfile->per_bfd->all_comp_units.size ())
@@ -3141,7 +3141,7 @@ dwarf2_gdb_index::expand_matching_symbols
       struct dwarf2_per_cu_data *per_cu;
 
       dw2_symtab_iter_init (&iter, per_objfile, block_kind, domain, namei);
-      while ((per_cu = dw2_symtab_iter_next (&iter)) != NULL)
+      while ((per_cu = dw2_symtab_iter_next (&iter, index)) != NULL)
 	dw2_expand_symtabs_matching_one (per_cu, per_objfile, nullptr,
 					 nullptr);
       return true;
-- 
2.34.1


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

* [PATCH 8/9] Add an ad hoc version check to dwarf_scanner_base
  2022-04-20  4:28 [PATCH 0/9] Unify the DWARF index fields Tom Tromey
                   ` (6 preceding siblings ...)
  2022-04-20  4:29 ` [PATCH 7/9] Simplify version check in dw2_symtab_iter_next Tom Tromey
@ 2022-04-20  4:29 ` Tom Tromey
  2022-04-20  4:29 ` [PATCH 9/9] Unify the DWARF index holders Tom Tromey
  8 siblings, 0 replies; 17+ messages in thread
From: Tom Tromey @ 2022-04-20  4:29 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

Some generic code in the DWARF reader has a special case for older
versions of .gdb_index.  This patch adds an ad hoc version check
method so that these spots can work without specific knowledge of
which index is in use.
---
 gdb/dwarf2/mapped-index.h | 8 ++++++++
 gdb/dwarf2/read.c         | 9 +++++++--
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/gdb/dwarf2/mapped-index.h b/gdb/dwarf2/mapped-index.h
index 39a9ede4896..e5d77469925 100644
--- a/gdb/dwarf2/mapped-index.h
+++ b/gdb/dwarf2/mapped-index.h
@@ -58,6 +58,14 @@ struct dwarf_scanner_base
   /* Return a quick_symbol_functions instance that refers back to this
      dwarf_scanner_base.  */
   virtual quick_symbol_functions_up make_quick_functions () const = 0;
+
+  /* An ad hoc version check.  This is needed for .gdb_index to check
+     whether a version 8 or above index is in use.  Returns true if
+     the index is usable, false otherwise.  */
+  virtual bool version_check () const
+  {
+    return true;
+  }
 };
 
 /* Base class containing bits shared by both .gdb_index and
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index f340cd68e67..aa3bf6e2f25 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -245,6 +245,11 @@ struct mapped_index final : public mapped_index_base
   { return this->symbol_table.size () / 2; }
 
   quick_symbol_functions_up make_quick_functions () const override;
+
+  bool version_check () const override
+  {
+    return version >= 8;
+  }
 };
 
 /* A description of the mapped .debug_names.
@@ -2038,7 +2043,7 @@ dw2_do_instantiate_symtab (dwarf2_per_cu_data *per_cu,
 	    && cu != NULL
 	    && cu->dwo_unit != NULL
 	    && per_objfile->per_bfd->index_table != NULL
-	    && per_objfile->per_bfd->index_table->version <= 7
+	    && !per_objfile->per_bfd->index_table->version_check ()
 	    /* DWP files aren't supported yet.  */
 	    && get_dwp_file (per_objfile) == NULL)
 	  queue_and_load_all_dwo_tus (cu);
@@ -22576,7 +22581,7 @@ follow_die_sig_1 (struct die_info *src_die, struct signatured_type *sig_type,
       /* For .gdb_index version 7 keep track of included TUs.
 	 http://sourceware.org/bugzilla/show_bug.cgi?id=15021.  */
       if (per_objfile->per_bfd->index_table != NULL
-	  && per_objfile->per_bfd->index_table->version <= 7)
+	  && !per_objfile->per_bfd->index_table->version_check ())
 	{
 	  (*ref_cu)->per_cu->imported_symtabs_push (sig_cu->per_cu);
 	}
-- 
2.34.1


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

* [PATCH 9/9] Unify the DWARF index holders
  2022-04-20  4:28 [PATCH 0/9] Unify the DWARF index fields Tom Tromey
                   ` (7 preceding siblings ...)
  2022-04-20  4:29 ` [PATCH 8/9] Add an ad hoc version check to dwarf_scanner_base Tom Tromey
@ 2022-04-20  4:29 ` Tom Tromey
  2022-04-20 12:15   ` Pedro Alves
  8 siblings, 1 reply; 17+ messages in thread
From: Tom Tromey @ 2022-04-20  4:29 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

The dwarf2_per_bfd object has a separate field for each possible kind
of index.  Until an earlier patch in this series, two of these were
even derived from a common base class, but still had separate slots.
This patch unifies all the index fields using the common base class
that was introduced earlier in this series.  This makes it more
obvious that only a single index can be active at a time, and also
removes some code from dwarf2_initialize_objfile.
---
 gdb/dwarf2/index-write.c |  28 +++++-----
 gdb/dwarf2/read.c        | 107 ++++++++++++++++++++-------------------
 gdb/dwarf2/read.h        |  10 +---
 3 files changed, 72 insertions(+), 73 deletions(-)

diff --git a/gdb/dwarf2/index-write.c b/gdb/dwarf2/index-write.c
index 58b0f0b98e3..a69201982b8 100644
--- a/gdb/dwarf2/index-write.c
+++ b/gdb/dwarf2/index-write.c
@@ -1087,12 +1087,11 @@ write_gdbindex_1 (FILE *out_file,
 /* Write the contents of the internal "cooked" index.  */
 
 static void
-write_cooked_index (dwarf2_per_objfile *per_objfile,
+write_cooked_index (cooked_index_vector *table,
 		    const cu_index_map &cu_index_htab,
 		    struct mapped_symtab *symtab)
 {
-  for (const cooked_index_entry *entry
-	 : per_objfile->per_bfd->cooked_index_table->all_entries ())
+  for (const cooked_index_entry *entry : table->all_entries ())
     {
       const auto it = cu_index_htab.find (entry->per_cu);
       gdb_assert (it != cu_index_htab.cend ());
@@ -1178,13 +1177,14 @@ write_gdbindex (dwarf2_per_objfile *per_objfile, FILE *out_file,
       ++this_counter;
     }
 
-  write_cooked_index (per_objfile, cu_index_htab, &symtab);
+  cooked_index_vector *table
+    = (dynamic_cast<cooked_index_vector *>
+       (per_objfile->per_bfd->index_table.get ()));
+  write_cooked_index (table, cu_index_htab, &symtab);
 
   /* Dump the address map.  */
   data_buf addr_vec;
-  std::vector<addrmap *> addrmaps
-    = per_objfile->per_bfd->cooked_index_table->get_addrmaps ();
-  for (auto map : addrmaps)
+  for (auto map : table->get_addrmaps ())
     write_address_map (map, addr_vec, cu_index_htab);
 
   /* Now that we've processed all symbols we can shrink their cu_indices
@@ -1250,8 +1250,10 @@ write_debug_names (dwarf2_per_objfile *per_objfile,
 			  - per_objfile->per_bfd->tu_stats.nr_tus));
   gdb_assert (types_counter == per_objfile->per_bfd->tu_stats.nr_tus);
 
-  for (const cooked_index_entry *entry
-	 : per_objfile->per_bfd->cooked_index_table->all_entries ())
+  cooked_index_vector *table
+    = (dynamic_cast<cooked_index_vector *>
+       (per_objfile->per_bfd->index_table.get ()));
+  for (const cooked_index_entry *entry : table->all_entries ())
     nametable.insert (entry);
 
   nametable.build ();
@@ -1388,10 +1390,12 @@ write_dwarf_index (dwarf2_per_objfile *per_objfile, const char *dir,
 {
   struct objfile *objfile = per_objfile->objfile;
 
-  if (per_objfile->per_bfd->cooked_index_table == nullptr)
+  cooked_index_vector *table
+    = (dynamic_cast<cooked_index_vector *>
+       (per_objfile->per_bfd->index_table.get ()));
+  if (table == nullptr)
     {
-      if (per_objfile->per_bfd->index_table != nullptr
-	  || per_objfile->per_bfd->debug_names_table != nullptr)
+      if (per_objfile->per_bfd->index_table != nullptr)
 	error (_("Cannot use an index to create the index"));
       error (_("No debugging symbols"));
     }
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index aa3bf6e2f25..36d0f9ded9e 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -2932,7 +2932,8 @@ static void
 dw2_symtab_iter_init (struct dw2_symtab_iterator *iter,
 		      dwarf2_per_objfile *per_objfile,
 		      gdb::optional<block_enum> block_index,
-		      domain_enum domain, offset_type namei)
+		      domain_enum domain, offset_type namei,
+		      mapped_index &index)
 {
   iter->per_objfile = per_objfile;
   iter->block_index = block_index;
@@ -2942,12 +2943,10 @@ dw2_symtab_iter_init (struct dw2_symtab_iterator *iter,
   iter->vec = {};
   iter->length = 0;
 
-  mapped_index *index = per_objfile->per_bfd->index_table.get ();
-
-  gdb_assert (!index->symbol_name_slot_invalid (namei));
-  offset_type vec_idx = index->symbol_vec_index (namei);
+  gdb_assert (!index.symbol_name_slot_invalid (namei));
+  offset_type vec_idx = index.symbol_vec_index (namei);
 
-  iter->vec = offset_view (index->constant_pool.slice (vec_idx));
+  iter->vec = offset_view (index.constant_pool.slice (vec_idx));
   iter->length = iter->vec[0];
 }
 
@@ -3078,8 +3077,9 @@ dwarf2_gdb_index::dump (struct objfile *objfile)
 {
   dwarf2_per_objfile *per_objfile = get_dwarf2_per_objfile (objfile);
 
-  gdb_printf (".gdb_index: version %d\n",
-	      per_objfile->per_bfd->index_table->version);
+  mapped_index *index = (dynamic_cast<mapped_index *>
+			 (per_objfile->per_bfd->index_table.get ()));
+  gdb_printf (".gdb_index: version %d\n", index->version);
   gdb_printf ("\n");
 }
 
@@ -3129,7 +3129,9 @@ dwarf2_gdb_index::expand_matching_symbols
 
   const block_enum block_kind = global ? GLOBAL_BLOCK : STATIC_BLOCK;
 
-  mapped_index &index = *per_objfile->per_bfd->index_table;
+  mapped_index &index
+    = (dynamic_cast<mapped_index &>
+       (*per_objfile->per_bfd->index_table.get ()));
 
   const char *match_name = name.ada ().lookup_name ().c_str ();
   auto matcher = [&] (const char *symname)
@@ -3145,7 +3147,8 @@ dwarf2_gdb_index::expand_matching_symbols
       struct dw2_symtab_iterator iter;
       struct dwarf2_per_cu_data *per_cu;
 
-      dw2_symtab_iter_init (&iter, per_objfile, block_kind, domain, namei);
+      dw2_symtab_iter_init (&iter, per_objfile, block_kind, domain, namei,
+			    index);
       while ((per_cu = dw2_symtab_iter_next (&iter, index)) != NULL)
 	dw2_expand_symtabs_matching_one (per_cu, per_objfile, nullptr,
 					 nullptr);
@@ -3967,7 +3970,9 @@ dw2_expand_marked_cus
 {
   offset_type vec_len, vec_idx;
   bool global_seen = false;
-  mapped_index &index = *per_objfile->per_bfd->index_table;
+  mapped_index &index
+    = (dynamic_cast<mapped_index &>
+       (*per_objfile->per_bfd->index_table.get ()));
 
   offset_view vec (index.constant_pool.slice (index.symbol_vec_index (idx)));
   vec_len = vec[0];
@@ -4185,7 +4190,9 @@ dwarf2_gdb_index::expand_symtabs_matching
       return true;
     }
 
-  mapped_index &index = *per_objfile->per_bfd->index_table;
+  mapped_index &index
+    = (dynamic_cast<mapped_index &>
+       (*per_objfile->per_bfd->index_table.get ()));
 
   bool result
     = dw2_expand_symtabs_matching_symbol (index, *lookup_name,
@@ -4683,7 +4690,7 @@ dwarf2_read_debug_names (dwarf2_per_objfile *per_objfile)
 
   create_addrmap_from_aranges (per_objfile, &per_bfd->debug_aranges);
 
-  per_bfd->debug_names_table = std::move (map);
+  per_bfd->index_table = std::move (map);
   per_bfd->quick_file_names_table =
     create_quick_file_names_table (per_bfd->all_comp_units.size ());
 
@@ -5119,7 +5126,9 @@ dwarf2_debug_names_index::expand_matching_symbols
 {
   dwarf2_per_objfile *per_objfile = get_dwarf2_per_objfile (objfile);
 
-  mapped_debug_names &map = *per_objfile->per_bfd->debug_names_table;
+  mapped_debug_names &map
+    = (dynamic_cast<mapped_debug_names &>
+       (*per_objfile->per_bfd->index_table.get ()));
   const block_search_flags block_flags
     = global ? SEARCH_GLOBAL_BLOCK : SEARCH_STATIC_BLOCK;
 
@@ -5179,7 +5188,9 @@ dwarf2_debug_names_index::expand_symtabs_matching
       return true;
     }
 
-  mapped_debug_names &map = *per_objfile->per_bfd->debug_names_table;
+  mapped_debug_names &map
+    = (dynamic_cast<mapped_debug_names &>
+       (*per_objfile->per_bfd->index_table.get ()));
 
   bool result
     = dw2_expand_symtabs_matching_symbol (map, *lookup_name,
@@ -5284,38 +5295,20 @@ dwarf2_initialize_objfile (struct objfile *objfile)
       return;
     }
 
-  /* Was a debug names index already read when we processed an objfile sharing
-     PER_BFD?  */
-  if (per_bfd->debug_names_table != nullptr)
-    {
-      dwarf_read_debug_printf ("re-using shared debug names table");
-      objfile->qf.push_front
-	(per_bfd->debug_names_table->make_quick_functions ());
-      return;
-    }
-
   /* Was a GDB index already read when we processed an objfile sharing
      PER_BFD?  */
   if (per_bfd->index_table != nullptr)
     {
-      dwarf_read_debug_printf ("re-using shared index table");
+      dwarf_read_debug_printf ("re-using symbols");
       objfile->qf.push_front (per_bfd->index_table->make_quick_functions ());
       return;
     }
 
-  if (per_bfd->cooked_index_table != nullptr)
-    {
-      dwarf_read_debug_printf ("re-using cooked index table");
-      objfile->qf.push_front
-	(per_bfd->cooked_index_table->make_quick_functions ());
-      return;
-    }
-
   if (dwarf2_read_debug_names (per_objfile))
     {
       dwarf_read_debug_printf ("found debug names");
       objfile->qf.push_front
-	(per_bfd->debug_names_table->make_quick_functions ());
+	(per_bfd->index_table->make_quick_functions ());
       return;
     }
 
@@ -5354,7 +5347,7 @@ dwarf2_build_psymtabs (struct objfile *objfile, bool already_attached)
 
   if (already_attached)
     {
-      if (per_objfile->per_bfd->cooked_index_table != nullptr)
+      if (per_objfile->per_bfd->index_table != nullptr)
 	return;
     }
   else
@@ -7134,11 +7127,11 @@ dwarf2_build_psymtabs_hard (dwarf2_per_objfile *per_objfile)
 		     }),
      indexes.end ());
   indexes.shrink_to_fit ();
-  per_bfd->cooked_index_table.reset
-    (new cooked_index_vector (std::move (indexes)));
 
-  const cooked_index_entry *main_entry
-    = per_bfd->cooked_index_table->get_main ();
+  cooked_index_vector *vec = new cooked_index_vector (std::move (indexes));
+  per_bfd->index_table.reset (vec);
+
+  const cooked_index_entry *main_entry = vec->get_main ();
   if (main_entry != nullptr)
     set_objfile_main_name (objfile, main_entry->name,
 			   main_entry->per_cu->lang);
@@ -18478,12 +18471,14 @@ cooked_index_functions::find_pc_sect_compunit_symtab
       int warn_if_readin)
 {
   dwarf2_per_objfile *per_objfile = get_dwarf2_per_objfile (objfile);
-  if (per_objfile->per_bfd->cooked_index_table == nullptr)
+  if (per_objfile->per_bfd->index_table == nullptr)
     return nullptr;
 
   CORE_ADDR baseaddr = objfile->text_section_offset ();
-  dwarf2_per_cu_data *per_cu
-    = per_objfile->per_bfd->cooked_index_table->lookup (pc - baseaddr);
+  cooked_index_vector *table
+    = (dynamic_cast<cooked_index_vector *>
+       (per_objfile->per_bfd->index_table.get ()));
+  dwarf2_per_cu_data *per_cu = table->lookup (pc - baseaddr);
   if (per_cu == nullptr)
     return nullptr;
 
@@ -18507,12 +18502,14 @@ cooked_index_functions::find_compunit_symtab_by_address
     return nullptr;
 
   dwarf2_per_objfile *per_objfile = get_dwarf2_per_objfile (objfile);
-  if (per_objfile->per_bfd->cooked_index_table == nullptr)
+  if (per_objfile->per_bfd->index_table == nullptr)
     return nullptr;
 
   CORE_ADDR baseaddr = objfile->data_section_offset ();
-  dwarf2_per_cu_data *per_cu
-    = per_objfile->per_bfd->cooked_index_table->lookup (address - baseaddr);
+  cooked_index_vector *table
+    = (dynamic_cast<cooked_index_vector *>
+       (per_objfile->per_bfd->index_table.get ()));
+  dwarf2_per_cu_data *per_cu = table->lookup (address - baseaddr);
   if (per_cu == nullptr)
     return nullptr;
 
@@ -18528,7 +18525,7 @@ cooked_index_functions::expand_matching_symbols
       symbol_compare_ftype *ordered_compare)
 {
   dwarf2_per_objfile *per_objfile = get_dwarf2_per_objfile (objfile);
-  if (per_objfile->per_bfd->cooked_index_table == nullptr)
+  if (per_objfile->per_bfd->index_table == nullptr)
     return;
   const block_search_flags search_flags = (global
 					   ? SEARCH_GLOBAL_BLOCK
@@ -18537,8 +18534,10 @@ cooked_index_functions::expand_matching_symbols
   symbol_name_matcher_ftype *name_match
     = lang->get_symbol_name_matcher (lookup_name);
 
-  for (const cooked_index_entry *entry
-	 : per_objfile->per_bfd->cooked_index_table->all_entries ())
+  cooked_index_vector *table
+    = (dynamic_cast<cooked_index_vector *>
+       (per_objfile->per_bfd->index_table.get ()));
+  for (const cooked_index_entry *entry : table->all_entries ())
     {
       if (entry->parent_entry != nullptr)
 	continue;
@@ -18564,7 +18563,7 @@ cooked_index_functions::expand_symtabs_matching
       enum search_domain kind)
 {
   dwarf2_per_objfile *per_objfile = get_dwarf2_per_objfile (objfile);
-  if (per_objfile->per_bfd->cooked_index_table == nullptr)
+  if (per_objfile->per_bfd->index_table == nullptr)
     return true;
 
   dw_expand_symtabs_matching_file_matcher (per_objfile, file_matcher);
@@ -18602,14 +18601,16 @@ cooked_index_functions::expand_symtabs_matching
     language_ada
   };
 
+  cooked_index_vector *table
+    = (dynamic_cast<cooked_index_vector *>
+       (per_objfile->per_bfd->index_table.get ()));
   for (enum language lang : unique_styles)
     {
       std::vector<gdb::string_view> name_vec
 	= lookup_name_without_params.split_name (lang);
 
-      for (const cooked_index_entry *entry
-	   : per_objfile->per_bfd->cooked_index_table->find (name_vec.back (),
-							     completing))
+      for (const cooked_index_entry *entry : table->find (name_vec.back (),
+							  completing))
 	{
 	  /* No need to consider symbols from expanded CUs.  */
 	  if (per_objfile->symtab_set_p (entry->per_cu))
diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h
index f3b09c63b64..b58c574c2be 100644
--- a/gdb/dwarf2/read.h
+++ b/gdb/dwarf2/read.h
@@ -441,14 +441,8 @@ struct dwarf2_per_bfd
      VMA of 0.  */
   bool has_section_at_zero = false;
 
-  /* The mapped index, or NULL if .gdb_index is missing or not being used.  */
-  std::unique_ptr<mapped_index> index_table;
-
-  /* The mapped index, or NULL if .debug_names is missing or not being used.  */
-  std::unique_ptr<mapped_debug_names> debug_names_table;
-
-  /* The cooked index, or NULL if not using one.  */
-  std::unique_ptr<cooked_index_vector> cooked_index_table;
+  /* The mapped index, or NULL in the readnow case.  */
+  std::unique_ptr<dwarf_scanner_base> index_table;
 
   /* When using index_table, this keeps track of all quick_file_names entries.
      TUs typically share line table entries with a CU, so we maintain a
-- 
2.34.1


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

* Re: [PATCH 9/9] Unify the DWARF index holders
  2022-04-20  4:29 ` [PATCH 9/9] Unify the DWARF index holders Tom Tromey
@ 2022-04-20 12:15   ` Pedro Alves
  2022-04-20 12:53     ` Tom Tromey
  0 siblings, 1 reply; 17+ messages in thread
From: Pedro Alves @ 2022-04-20 12:15 UTC (permalink / raw)
  To: Tom Tromey, gdb-patches

I read the series and it LGTM.  There was just one detail that I didn't get.

On 2022-04-20 05:29, Tom Tromey wrote:
> @@ -3078,8 +3077,9 @@ dwarf2_gdb_index::dump (struct objfile *objfile)
>  {
>    dwarf2_per_objfile *per_objfile = get_dwarf2_per_objfile (objfile);
>  
> -  gdb_printf (".gdb_index: version %d\n",
> -	      per_objfile->per_bfd->index_table->version);
> +  mapped_index *index = (dynamic_cast<mapped_index *>
> +			 (per_objfile->per_bfd->index_table.get ()));
> +  gdb_printf (".gdb_index: version %d\n", index->version);
>    gdb_printf ("\n");
>  }

The patch uses dynamic_cast throughout, when you know the cast must succeed,
which seems strange.

When you cast to a reference type, like this:

> -  mapped_index &index = *per_objfile->per_bfd->index_table;
> +  mapped_index &index
> +    = (dynamic_cast<mapped_index &>
> +       (*per_objfile->per_bfd->index_table.get ()));
>  

.. it looks even more strange, because if the cast failed, this would
throw std::bad_cast, which nothing catches and it would kill gdb.

I think it would be better to use static_cast instead.

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

* Re: [PATCH 9/9] Unify the DWARF index holders
  2022-04-20 12:15   ` Pedro Alves
@ 2022-04-20 12:53     ` Tom Tromey
  2022-04-20 13:52       ` Pedro Alves
  0 siblings, 1 reply; 17+ messages in thread
From: Tom Tromey @ 2022-04-20 12:53 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Tom Tromey, gdb-patches

>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:

>> +  mapped_index *index = (dynamic_cast<mapped_index *>
>> +			 (per_objfile->per_bfd->index_table.get ()));

Pedro> The patch uses dynamic_cast throughout, when you know the cast must succeed,
Pedro> which seems strange.

I tend to do that as a form of error checking.

In an older project of mine I had something like:

    template<typename T, V>
    T *
    assert_cast (V *v)
    {
      T *result = dynamic_cast<V *> (v);
      gdb_assert (result != nullptr);
      return result;
    }

... ensuring that these "must succeed" casts really do succeed.

Pedro> I think it would be better to use static_cast instead.

I can do that, though what do you think of the assert_cast thing?

Tom

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

* Re: [PATCH 9/9] Unify the DWARF index holders
  2022-04-20 12:53     ` Tom Tromey
@ 2022-04-20 13:52       ` Pedro Alves
  2022-04-20 14:53         ` Tom Tromey
  0 siblings, 1 reply; 17+ messages in thread
From: Pedro Alves @ 2022-04-20 13:52 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On 2022-04-20 13:53, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:
> 
>>> +  mapped_index *index = (dynamic_cast<mapped_index *>
>>> +			 (per_objfile->per_bfd->index_table.get ()));
> 
> Pedro> The patch uses dynamic_cast throughout, when you know the cast must succeed,
> Pedro> which seems strange.
> 
> I tend to do that as a form of error checking.
> 
> In an older project of mine I had something like:
> 
>     template<typename T, V>
>     T *
>     assert_cast (V *v)
>     {
>       T *result = dynamic_cast<V *> (v);
>       gdb_assert (result != nullptr);
>       return result;
>     }
> 
> ... ensuring that these "must succeed" casts really do succeed.
> 
> Pedro> I think it would be better to use static_cast instead.
> 
> I can do that, though what do you think of the assert_cast thing?

I'm not sure.  dynamic_cast is known to be costly/slow, not sure we should
encourage using it pervasively, it scares me a bit to have something like that
used in code that might be performance critical, like the DWARF stuff.

See this benchmark I found somewhere, and then tweaked to run against GCC:
https://www.quick-bench.com/q/taWz4dfQqvtzeoU4z12WkeDyNwg

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

* Re: [PATCH 9/9] Unify the DWARF index holders
  2022-04-20 13:52       ` Pedro Alves
@ 2022-04-20 14:53         ` Tom Tromey
  0 siblings, 0 replies; 17+ messages in thread
From: Tom Tromey @ 2022-04-20 14:53 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Tom Tromey, gdb-patches

Pedro> I'm not sure.  dynamic_cast is known to be costly/slow, not sure we should
Pedro> encourage using it pervasively, it scares me a bit to have something like that
Pedro> used in code that might be performance critical, like the DWARF stuff.

Ok.  I suppose if we need it we can add virtual methods to do the casting.

Tom

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

* Re: [PATCH 1/9] Move mapped_index_base to new header file
  2022-04-20  4:28 ` [PATCH 1/9] Move mapped_index_base to new header file Tom Tromey
@ 2022-04-20 20:38   ` Lancelot SIX
  2022-04-21 15:15     ` Pedro Alves
  0 siblings, 1 reply; 17+ messages in thread
From: Lancelot SIX @ 2022-04-20 20:38 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

> +/* Base class for mapped indices
> +
> +   Copyright (C) 2021 Free Software Foundation, Inc.
> +

Hi,

just a nit, but the date should probably be 2022 here I guess (or
something like 2017-2022 at least if it should track where the code
comes from, I am not sure).

Best,
Lancelot.

> +   This file is part of GDB.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +

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

* Re: [PATCH 7/9] Simplify version check in dw2_symtab_iter_next
  2022-04-20  4:29 ` [PATCH 7/9] Simplify version check in dw2_symtab_iter_next Tom Tromey
@ 2022-04-20 20:49   ` Lancelot SIX
  0 siblings, 0 replies; 17+ messages in thread
From: Lancelot SIX @ 2022-04-20 20:49 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

Hi,

On Tue, Apr 19, 2022 at 10:29:04PM -0600, Tom Tromey wrote:
> This simplifies the index versio check in dw2_symtab_iter_next, by

s/versio/version/ ?

> passing a reference to the index object to this function.  This avoids
> an indirection via the per_bfd object.
> ---
>  gdb/dwarf2/read.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
> index 3053ec9b303..f340cd68e67 100644
> --- a/gdb/dwarf2/read.c
> +++ b/gdb/dwarf2/read.c
> @@ -2949,7 +2949,8 @@ dw2_symtab_iter_init (struct dw2_symtab_iterator *iter,
>  /* Return the next matching CU or NULL if there are no more.  */
>  
>  static struct dwarf2_per_cu_data *
> -dw2_symtab_iter_next (struct dw2_symtab_iterator *iter)
> +dw2_symtab_iter_next (struct dw2_symtab_iterator *iter,
> +		      mapped_index &index)
>  {
>    dwarf2_per_objfile *per_objfile = iter->per_objfile;
>  
> @@ -2963,9 +2964,8 @@ dw2_symtab_iter_next (struct dw2_symtab_iterator *iter)
>  	 Indices prior to version 7 don't record them,
>  	 and indices >= 7 may elide them for certain symbols
>  	 (gold does this).  */
> -      int attrs_valid =
> -	(per_objfile->per_bfd->index_table->version >= 7
> -	 && symbol_kind != GDB_INDEX_SYMBOL_KIND_NONE);
> +      int attrs_valid = (index.version >= 7
> +			 && symbol_kind != GDB_INDEX_SYMBOL_KIND_NONE);
>  
>        /* Don't crash on bad data.  */
>        if (cu_index >= per_objfile->per_bfd->all_comp_units.size ())
> @@ -3141,7 +3141,7 @@ dwarf2_gdb_index::expand_matching_symbols
>        struct dwarf2_per_cu_data *per_cu;
>  
>        dw2_symtab_iter_init (&iter, per_objfile, block_kind, domain, namei);
> -      while ((per_cu = dw2_symtab_iter_next (&iter)) != NULL)
> +      while ((per_cu = dw2_symtab_iter_next (&iter, index)) != NULL)

While at editing this line, NULL could become nullptr.  But this is up
to you really.

Best,
Lancelot.

>  	dw2_expand_symtabs_matching_one (per_cu, per_objfile, nullptr,
>  					 nullptr);
>        return true;
> -- 
> 2.34.1
> 

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

* Re: [PATCH 1/9] Move mapped_index_base to new header file
  2022-04-20 20:38   ` Lancelot SIX
@ 2022-04-21 15:15     ` Pedro Alves
  0 siblings, 0 replies; 17+ messages in thread
From: Pedro Alves @ 2022-04-21 15:15 UTC (permalink / raw)
  To: Lancelot SIX, Tom Tromey; +Cc: gdb-patches

On 2022-04-20 21:38, Lancelot SIX via Gdb-patches wrote:
>> +/* Base class for mapped indices
>> +
>> +   Copyright (C) 2021 Free Software Foundation, Inc.
>> +
> 
> Hi,
> 
> just a nit, but the date should probably be 2022 here I guess (or
> something like 2017-2022 at least if it should track where the code
> comes from, I am not sure).

The latter.  What matters for copyright is the contents, not the file.

A thought experiment I like to see how date-of-file can't be the right one is this:
If date-of-file is what mattered, then you could do this:

 - old.c has copyright 1990
 - cp old.c new.c
 - replace copyright year in new.c with just 2022, as it's a new file
 - mv new.c old.c

yay, I just extended copyright protection of old.c for a few more years!

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

end of thread, other threads:[~2022-04-21 15:15 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-20  4:28 [PATCH 0/9] Unify the DWARF index fields Tom Tromey
2022-04-20  4:28 ` [PATCH 1/9] Move mapped_index_base to new header file Tom Tromey
2022-04-20 20:38   ` Lancelot SIX
2022-04-21 15:15     ` Pedro Alves
2022-04-20  4:28 ` [PATCH 2/9] Give mapped_index_base a virtual destructor Tom Tromey
2022-04-20  4:29 ` [PATCH 3/9] Let mapped index classes create the quick_symbol_functions object Tom Tromey
2022-04-20  4:29 ` [PATCH 4/9] Remove some "OBJF_READNOW" code from dwarf2_debug_names_index Tom Tromey
2022-04-20  4:29 ` [PATCH 5/9] Introduce readnow_functions Tom Tromey
2022-04-20  4:29 ` [PATCH 6/9] Introduce and use dwarf_scanner_base Tom Tromey
2022-04-20  4:29 ` [PATCH 7/9] Simplify version check in dw2_symtab_iter_next Tom Tromey
2022-04-20 20:49   ` Lancelot SIX
2022-04-20  4:29 ` [PATCH 8/9] Add an ad hoc version check to dwarf_scanner_base Tom Tromey
2022-04-20  4:29 ` [PATCH 9/9] Unify the DWARF index holders Tom Tromey
2022-04-20 12:15   ` Pedro Alves
2022-04-20 12:53     ` Tom Tromey
2022-04-20 13:52       ` Pedro Alves
2022-04-20 14:53         ` 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).