[gdb/symtab] Redo "Fix assert in set_length"

This reverts commit 1c04f72368c ("[gdb/symtab] Fix assert in set_length"), due
to a regression reported in PR29572, and implements a different fix for PR29453.

The fix is to not use the CU table in a .debug_names section to construct
all_units, but instead use create_all_units, and then verify the CU
table from .debug_names.  This also fixes PR25969, so remove the KFAIL.

Approved-By: Tom Tromey <tom@tromey.com>
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29572
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=25969

diff --git a/gdb/dwarf2/read-debug-names.c b/gdb/dwarf2/read-debug-names.c
index 78e0df27314024e9216008b1284619cf5b5db634..67e2bf24eb872dedd1795fb5c2749ab3ceb16573 100644 (file)
--- a/gdb/dwarf2/read-debug-names.c
+++ b/gdb/dwarf2/read-debug-names.c
@@ -108,52 +108,46 @@ mapped_debug_names::make_quick_functions () const
   return quick_symbol_functions_up (new dwarf2_debug_names_index);
 }
 
-/* Create the signatured type hash table from .debug_names.  */
+/* Check the signatured type hash table from .debug_names.  */
 
-static void
-create_signatured_type_table_from_debug_names
+static bool
+check_signatured_type_table_from_debug_names
   (dwarf2_per_objfile *per_objfile,
    const mapped_debug_names &map,
-   struct dwarf2_section_info *section,
-   struct dwarf2_section_info *abbrev_section)
+   struct dwarf2_section_info *section)
 {
   struct objfile *objfile = per_objfile->objfile;
+  dwarf2_per_bfd *per_bfd = per_objfile->per_bfd;
+  int nr_cus = per_bfd->all_comp_units.size ();
+  int nr_cus_tus = per_bfd->all_units.size ();
 
   section->read (objfile);
-  abbrev_section->read (objfile);
-
-  htab_up sig_types_hash = allocate_signatured_type_table ();
 
+  uint32_t j = nr_cus;
   for (uint32_t i = 0; i < map.tu_count; ++i)
     {
-      signatured_type_up sig_type;
-      void **slot;
-
       sect_offset sect_off
        = (sect_offset) (extract_unsigned_integer
                         (map.tu_table_reordered + i * map.offset_size,
                          map.offset_size,
                          map.dwarf5_byte_order));
 
-      comp_unit_head cu_header;
-      read_and_check_comp_unit_head (per_objfile, &cu_header, section,
-                                    abbrev_section,
-                                    section->buffer + to_underlying (sect_off),
-                                    rcuh_kind::TYPE);
-
-      sig_type = per_objfile->per_bfd->allocate_signatured_type
-       (cu_header.signature);
-      sig_type->type_offset_in_tu = cu_header.type_cu_offset_in_tu;
-      sig_type->section = section;
-      sig_type->sect_off = sect_off;
-
-      slot = htab_find_slot (sig_types_hash.get (), sig_type.get (), INSERT);
-      *slot = sig_type.get ();
-
-      per_objfile->per_bfd->all_units.emplace_back (sig_type.release ());
+      bool found = false;
+      for (; j < nr_cus_tus; j++)
+       if (per_bfd->get_cu (j)->sect_off == sect_off)
+         {
+           found = true;
+           break;
+         }
+      if (!found)
+       {
+         warning (_("Section .debug_names has incorrect entry in TU table,"
+                    " ignoring .debug_names."));
+         return false;
+       }
+      per_bfd->all_comp_units_index_tus.push_back (per_bfd->get_cu (j));
     }
-
-  per_objfile->per_bfd->signatured_types = std::move (sig_types_hash);
+  return true;
 }
 
 /* Read the address map data from DWARF-5 .debug_aranges, and use it to
@@ -363,17 +357,20 @@ read_debug_names_from_section (struct objfile *objfile,
   return true;
 }
 
-/* A helper for create_cus_from_debug_names that handles the MAP's CU
+/* A helper for check_cus_from_debug_names that handles the MAP's CU
    list.  */
 
 static bool
-create_cus_from_debug_names_list (dwarf2_per_bfd *per_bfd,
-                                 const mapped_debug_names &map,
-                                 dwarf2_section_info &section,
-                                 bool is_dwz)
+check_cus_from_debug_names_list (dwarf2_per_bfd *per_bfd,
+                                const mapped_debug_names &map,
+                                dwarf2_section_info &section,
+                                bool is_dwz)
 {
+  int nr_cus = per_bfd->all_comp_units.size ();
+
   if (!map.augmentation_is_gdb)
     {
+      uint32_t j = 0;
       for (uint32_t i = 0; i < map.cu_count; ++i)
        {
          sect_offset sect_off
@@ -381,56 +378,44 @@ create_cus_from_debug_names_list (dwarf2_per_bfd *per_bfd,
                             (map.cu_table_reordered + i * map.offset_size,
                              map.offset_size,
                              map.dwarf5_byte_order));
-         /* We don't know the length of the CU, because the CU list in a
-            .debug_names index can be incomplete, so we can't use the start
-            of the next CU as end of this CU.  We create the CUs here with
-            length 0, and in cutu_reader::cutu_reader we'll fill in the
-            actual length.  */
-         dwarf2_per_cu_data_up per_cu
-           = create_cu_from_index_list (per_bfd, &section, is_dwz,
-                                        sect_off, 0);
-         per_bfd->all_units.push_back (std::move (per_cu));
+         bool found = false;
+         for (; j < nr_cus; j++)
+           if (per_bfd->get_cu (j)->sect_off == sect_off)
+             {
+               found = true;
+               break;
+             }
+         if (!found)
+           {
+             warning (_("Section .debug_names has incorrect entry in CU table,"
+                        " ignoring .debug_names."));
+             return false;
+           }
+         per_bfd->all_comp_units_index_cus.push_back (per_bfd->get_cu (j));
        }
       return true;
     }
 
-  sect_offset sect_off_prev;
-  for (uint32_t i = 0; i <= map.cu_count; ++i)
+  if (map.cu_count != nr_cus)
     {
-      sect_offset sect_off_next;
-      if (i < map.cu_count)
-       {
-         sect_off_next
-           = (sect_offset) (extract_unsigned_integer
-                            (map.cu_table_reordered + i * map.offset_size,
-                             map.offset_size,
-                             map.dwarf5_byte_order));
-       }
-      else
-       sect_off_next = (sect_offset) section.size;
-      if (i >= 1)
+      warning (_("Section .debug_names has incorrect number of CUs in CU table,"
+                " ignoring .debug_names."));
+      return false;
+    }
+
+  for (uint32_t i = 0; i < map.cu_count; ++i)
+    {
+      sect_offset sect_off
+       = (sect_offset) (extract_unsigned_integer
+                        (map.cu_table_reordered + i * map.offset_size,
+                         map.offset_size,
+                         map.dwarf5_byte_order));
+      if (sect_off != per_bfd->get_cu (i)->sect_off)
        {
-         if (sect_off_next == sect_off_prev)
-           {
-             warning (_("Section .debug_names has duplicate entry in CU table,"
-                        " ignoring .debug_names."));
-             return false;
-           }
-         if (sect_off_next < sect_off_prev)
-           {
-             warning (_("Section .debug_names has non-ascending CU table,"
-                        " ignoring .debug_names."));
-             return false;
-           }
-         /* Note: we're not using length = sect_off_next - sect_off_prev,
-            to gracefully handle an incomplete CU list.  */
-         const ULONGEST length = 0;
-         dwarf2_per_cu_data_up per_cu
-           = create_cu_from_index_list (per_bfd, &section, is_dwz,
-                                        sect_off_prev, length);
-         per_bfd->all_units.push_back (std::move (per_cu));
+         warning (_("Section .debug_names has incorrect entry in CU table,"
+                    " ignoring .debug_names."));
+         return false;
        }
-      sect_off_prev = sect_off_next;
     }
 
   return true;
@@ -440,23 +425,20 @@ create_cus_from_debug_names_list (dwarf2_per_bfd *per_bfd,
    the CU objects for this dwarf2_per_objfile.  */
 
 static bool
-create_cus_from_debug_names (dwarf2_per_bfd *per_bfd,
-                            const mapped_debug_names &map,
-                            const mapped_debug_names &dwz_map)
+check_cus_from_debug_names (dwarf2_per_bfd *per_bfd,
+                           const mapped_debug_names &map,
+                           const mapped_debug_names &dwz_map)
 {
-  gdb_assert (per_bfd->all_units.empty ());
-  per_bfd->all_units.reserve (map.cu_count + dwz_map.cu_count);
-
-  if (!create_cus_from_debug_names_list (per_bfd, map, per_bfd->info,
-                                        false /* is_dwz */))
+  if (!check_cus_from_debug_names_list (per_bfd, map, per_bfd->info,
+                                       false /* is_dwz */))
     return false;
 
   if (dwz_map.cu_count == 0)
     return true;
 
   dwz_file *dwz = dwarf2_get_dwz_file (per_bfd);
-  return create_cus_from_debug_names_list (per_bfd, dwz_map, dwz->info,
-                                          true /* is_dwz */);
+  return check_cus_from_debug_names_list (per_bfd, dwz_map, dwz->info,
+                                         true /* is_dwz */);
 }
 
 /* See read-debug-names.h.  */
@@ -492,7 +474,8 @@ dwarf2_read_debug_names (dwarf2_per_objfile *per_objfile)
        }
     }
 
-  if (!create_cus_from_debug_names (per_bfd, *map, dwz_map))
+  create_all_units (per_objfile, false);
+  if (!check_cus_from_debug_names (per_bfd, *map, dwz_map))
     {
       per_bfd->all_units.clear ();
       return false;
@@ -513,12 +496,14 @@ dwarf2_read_debug_names (dwarf2_per_objfile *per_objfile)
           ? &per_bfd->types[0]
           : &per_bfd->info);
 
-      create_signatured_type_table_from_debug_names
-       (per_objfile, *map, section, &per_bfd->abbrev);
+      if (!check_signatured_type_table_from_debug_names
+            (per_objfile, *map, section))
+       {
+         per_bfd->all_units.clear ();
+         return false;
+       }
     }
 
-  finalize_all_units (per_bfd);
-
   create_addrmap_from_aranges (per_objfile, &per_bfd->debug_aranges);
 
   per_bfd->index_table = std::move (map);
@@ -791,7 +776,7 @@ dw2_debug_names_iterator::next ()
                continue;
              }
          }
-         per_cu = per_bfd->get_cu (ull);
+         per_cu = per_bfd->get_index_cu (ull);
          break;
        case DW_IDX_type_unit:
          /* Don't crash on bad data.  */
@@ -804,15 +789,14 @@ dw2_debug_names_iterator::next ()
              continue;
            }
          {
-           int nr_cus = per_bfd->all_comp_units.size ();
-           per_cu = per_bfd->get_cu (nr_cus + ull);
+           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_cu (0);
+           per_cu = per_bfd->get_index_cu (0);
          break;
        case DW_IDX_GNU_internal:
          if (!m_map.augmentation_is_gdb)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index d83487007243e34f3937fb2869fce7f78aa0ce63..dd25e34a2c1e8018c6eb0654230cd69a43703d04 100644 (file)
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -1048,8 +1048,6 @@ static void prepare_one_comp_unit (struct dwarf2_cu *cu,
 static struct type *set_die_type (struct die_info *, struct type *,
                                  struct dwarf2_cu *, bool = false);
 
-static void create_all_units (dwarf2_per_objfile *per_objfile);
-
 static void load_full_comp_unit (dwarf2_per_cu_data *per_cu,
                                 dwarf2_per_objfile *per_objfile,
                                 dwarf2_cu *existing_cu,
@@ -5109,11 +5107,10 @@ finalize_all_units (dwarf2_per_bfd *per_bfd)
   per_bfd->all_type_units = tmp.slice (nr_cus, nr_tus);
 }
 
-/* Create a list of all compilation units in OBJFILE.
-   This is only done for -readnow and building partial symtabs.  */
+/* See read.h.  */
 
-static void
-create_all_units (dwarf2_per_objfile *per_objfile)
+void
+create_all_units (dwarf2_per_objfile *per_objfile, bool pre_read_p)
 {
   htab_up types_htab;
   gdb_assert (per_objfile->per_bfd->all_units.empty ());
@@ -5129,12 +5126,15 @@ create_all_units (dwarf2_per_objfile *per_objfile)
   dwz_file *dwz = dwarf2_get_dwz_file (per_objfile->per_bfd);
   if (dwz != NULL)
     {
-      /* Pre-read the sections we'll need to construct an index.  */
-      struct objfile *objfile = per_objfile->objfile;
-      dwz->abbrev.read (objfile);
-      dwz->info.read (objfile);
-      dwz->str.read (objfile);
-      dwz->line.read (objfile);
+      if (pre_read_p)
+       {
+         /* Pre-read the sections we'll need to construct an index.  */
+         struct objfile *objfile = per_objfile->objfile;
+         dwz->abbrev.read (objfile);
+         dwz->info.read (objfile);
+         dwz->str.read (objfile);
+         dwz->line.read (objfile);
+       }
       read_comp_units_from_section (per_objfile, &dwz->info, &dwz->abbrev, 1,
                                    types_htab, rcuh_kind::COMPILE);
 

diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h
index 7bb6d4c4d4b8f90676139072e2bfe7685a5c686c..6f97f641c8e9cb315d5935b91c879df5e8617fac 100644 (file)
--- a/gdb/dwarf2/read.h
+++ b/gdb/dwarf2/read.h
@@ -440,6 +440,20 @@ struct dwarf2_per_bfd
     return this->all_units[index].get ();
   }
 
+  /* Return the CU given its index in the CU table in the index.  */
+  dwarf2_per_cu_data *get_index_cu (int index) const
+  {
+    if (this->all_comp_units_index_cus.empty ())
+      return get_cu (index);
+
+    return this->all_comp_units_index_cus[index];
+  }
+
+  dwarf2_per_cu_data *get_index_tu (int index) const
+  {
+    return this->all_comp_units_index_tus[index];
+  }
+
   /* A convenience function to allocate a dwarf2_per_cu_data.  The
      returned object has its "index" field set properly.  The object
      is allocated on the dwarf2_per_bfd obstack.  */
@@ -500,6 +514,9 @@ public:
   gdb::array_view<dwarf2_per_cu_data_up> all_comp_units;
   gdb::array_view<dwarf2_per_cu_data_up> all_type_units;
 
+  std::vector<dwarf2_per_cu_data*> all_comp_units_index_cus;
+  std::vector<dwarf2_per_cu_data*> all_comp_units_index_tus;
+
   /* Table of struct type_unit_group objects.
      The hash key is the DW_AT_stmt_list value.  */
   htab_up type_unit_groups;
@@ -947,6 +964,11 @@ extern dwarf2_per_cu_data_up create_cu_from_index_list
 
 extern void finalize_all_units (dwarf2_per_bfd *per_bfd);
 
+/* Create a list of all compilation units in OBJFILE.  */
+
+extern void create_all_units (dwarf2_per_objfile *per_objfile,
+                             bool pre_read_p = true);
+
 /* Create a quick_file_names hash table.  */
 
 extern htab_up create_quick_file_names_table (unsigned int nr_initial_entries);

diff --git a/gdb/testsuite/gdb.dwarf2/clang-debug-names.exp b/gdb/testsuite/gdb.dwarf2/clang-debug-names.exp
index d6ee18b08986f0c49bf00bf1b7bdec06e74890c9..138bd86daee08787ac8268eed1df6937b8d90a40 100644 (file)
--- a/gdb/testsuite/gdb.dwarf2/clang-debug-names.exp
+++ b/gdb/testsuite/gdb.dwarf2/clang-debug-names.exp
@@ -32,20 +32,7 @@ if { [prepare_for_testing "failed to prepare" ${testfile} \
 }
 
 set test "no file command warnings"
-if { [regexp "warning: " $gdb_file_cmd_msg] } {
-    set kfail_re \
-       [concat \
-            "warning: Section .debug_aranges in \[^\r\n\]* entry" \
-            "at offset 0 debug_info_offset 0 does not exists," \
-            "ignoring \\.debug_aranges\\."]
-    if { [regexp $kfail_re $gdb_file_cmd_msg] } {
-       kfail symtab/25969 $test
-    } else {
-       fail $test
-    }
-} else {
-    pass $test
-}
+gdb_assert { [regexp "warning: " $gdb_file_cmd_msg] == 0 } $test
 
 set cmd "ptype main"
 set pass_re \

diff --git a/gdb/testsuite/gdb.dwarf2/debug-names-duplicate-cu.exp b/gdb/testsuite/gdb.dwarf2/debug-names-duplicate-cu.exp
index ca57ecbfc7021eedee69034add2512f08dde19a4..c6a0d01972855e294dca98af0441d35c3ebe2eea 100644 (file)
--- a/gdb/testsuite/gdb.dwarf2/debug-names-duplicate-cu.exp
+++ b/gdb/testsuite/gdb.dwarf2/debug-names-duplicate-cu.exp
@@ -66,7 +66,7 @@ if [prepare_for_testing "failed to prepare" $testfile "${asm_file} ${srcfile}" \
 set re \
     [list \
         "warning:" \
-        "Section .debug_names has duplicate entry in CU table," \
+        "Section .debug_names has incorrect number of CUs in CU table," \
         "ignoring .debug_names."]
 set re [join $re]
 gdb_assert {[regexp $re $gdb_file_cmd_msg]} "warning"

diff --git a/gdb/testsuite/gdb.dwarf2/debug-names-missing-cu.exp b/gdb/testsuite/gdb.dwarf2/debug-names-missing-cu.exp
index 5af32c8d6f2255bb706a7ce2b77e29d1f6cbadf6..d4e234cca53df407514d4d9cdae6fe39c1819adc 100644 (file)
--- a/gdb/testsuite/gdb.dwarf2/debug-names-missing-cu.exp
+++ b/gdb/testsuite/gdb.dwarf2/debug-names-missing-cu.exp
@@ -67,7 +67,7 @@ if [prepare_for_testing "failed to prepare" $testfile "${asm_file} ${srcfile}" \
 
 # Verify that .debug_names section is not ignored.
 set index [have_index $binfile]
-gdb_assert { [string equal $index "debug_names"] } ".debug_names used"
+gdb_assert { [string equal $index ""] } ".debug_names not used"
 
 # Verify that initially no symtab is expanded.
 gdb_test_no_output "maint info symtabs"

diff --git a/gdb/testsuite/gdb.dwarf2/debug-names-non-ascending-cu.exp b/gdb/testsuite/gdb.dwarf2/debug-names-non-ascending-cu.exp
index d866a3c502b503e3327ce8ab6a47a8f08a47fb0c..6352e8ddd5ec0a1a4c85bde8201cb1aa0025de0a 100644 (file)
--- a/gdb/testsuite/gdb.dwarf2/debug-names-non-ascending-cu.exp
+++ b/gdb/testsuite/gdb.dwarf2/debug-names-non-ascending-cu.exp
@@ -71,7 +71,7 @@ if [prepare_for_testing "failed to prepare" $testfile "${asm_file} ${srcfile}" \
 set re \
     [list \
         "warning:" \
-        "Section .debug_names has non-ascending CU table," \
+        "Section .debug_names has incorrect entry in CU table," \
         "ignoring .debug_names."]
 set re [join $re]
 gdb_assert {[regexp $re $gdb_file_cmd_msg]} "warning"