Fix two regressions caused by CU / TU merging

PR symtab/28160 and PR symtab/27893 concern GDB crashes in the test
suite when using the "fission" target board.  They are both caused by
the patches that merge the list of CUs with the list of TUs (and to a
lesser degree by the patches to share DWARF data across objfiles), and
the underlying issue is the same: it turns out that reading a DWO can
cause new type units to be created.  This means that the list of
dwarf2_per_cu_data objects depends on precisely which CUs have been
expanded.  However, because the type units can be created while
expanding a CU means that the vector of CUs can expand while it is
being iterated over -- a classic mistake.  Also, because a TU can be
added later, it means the resize_symtabs approach is incorrect.

This patch fixes resize_symtabs by removing it, and having set_symtab
resize the vector on demand.  It fixes the iteration problem by
introducing a safe (index-based) iterator and changing the relevant
spots to use it.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28160
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=27893

(cherry picked from commit d58e54bd277b90d847be09ae4b18bfdbc0dc2066)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index a9af00559c69f9c6e145d65f48031dd20e7aad07..195180df4184184ea2a1dac448b8645e9143bd40 100644 (file)
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -1639,6 +1639,73 @@ line_header_eq_voidp (const void *item_lhs, const void *item_rhs)
 
 \f
 
+/* An iterator for all_comp_units that is based on index.  This
+   approach makes it possible to iterate over all_comp_units safely,
+   when some caller in the loop may add new units.  */
+
+class all_comp_units_iterator
+{
+public:
+
+  all_comp_units_iterator (dwarf2_per_bfd *per_bfd, bool start)
+    : m_per_bfd (per_bfd),
+      m_index (start ? 0 : per_bfd->all_comp_units.size ())
+  {
+  }
+
+  all_comp_units_iterator &operator++ ()
+  {
+    ++m_index;
+    return *this;
+  }
+
+  dwarf2_per_cu_data *operator* () const
+  {
+    return m_per_bfd->get_cu (m_index);
+  }
+
+  bool operator== (const all_comp_units_iterator &other) const
+  {
+    return m_index == other.m_index;
+  }
+
+
+  bool operator!= (const all_comp_units_iterator &other) const
+  {
+    return m_index != other.m_index;
+  }
+
+private:
+
+  dwarf2_per_bfd *m_per_bfd;
+  size_t m_index;
+};
+
+/* A range adapter for the all_comp_units_iterator.  */
+class all_comp_units_range
+{
+public:
+
+  all_comp_units_range (dwarf2_per_bfd *per_bfd)
+    : m_per_bfd (per_bfd)
+  {
+  }
+
+  all_comp_units_iterator begin ()
+  {
+    return all_comp_units_iterator (m_per_bfd, true);
+  }
+
+  all_comp_units_iterator end ()
+  {
+    return all_comp_units_iterator (m_per_bfd, false);
+  }
+
+private:
+
+  dwarf2_per_bfd *m_per_bfd;
+};
+
 /* See declaration.  */
 
 dwarf2_per_bfd::dwarf2_per_bfd (bfd *obfd, const dwarf2_debug_sections *names,
@@ -1703,9 +1770,9 @@ private:
 bool
 dwarf2_per_objfile::symtab_set_p (const dwarf2_per_cu_data *per_cu) const
 {
-  gdb_assert (per_cu->index < this->m_symtabs.size ());
-
-  return this->m_symtabs[per_cu->index] != nullptr;
+  if (per_cu->index < this->m_symtabs.size ())
+    return this->m_symtabs[per_cu->index] != nullptr;
+  return false;
 }
 
 /* See read.h.  */
@@ -1713,9 +1780,9 @@ dwarf2_per_objfile::symtab_set_p (const dwarf2_per_cu_data *per_cu) const
 compunit_symtab *
 dwarf2_per_objfile::get_symtab (const dwarf2_per_cu_data *per_cu) const
 {
-  gdb_assert (per_cu->index < this->m_symtabs.size ());
-
-  return this->m_symtabs[per_cu->index];
+  if (per_cu->index < this->m_symtabs.size ())
+    return this->m_symtabs[per_cu->index];
+  return nullptr;
 }
 
 /* See read.h.  */
@@ -1724,9 +1791,9 @@ void
 dwarf2_per_objfile::set_symtab (const dwarf2_per_cu_data *per_cu,
                                compunit_symtab *symtab)
 {
-  gdb_assert (per_cu->index < this->m_symtabs.size ());
+  if (per_cu->index >= this->m_symtabs.size ())
+    this->m_symtabs.resize (per_cu->index + 1);
   gdb_assert (this->m_symtabs[per_cu->index] == nullptr);
-
   this->m_symtabs[per_cu->index] = symtab;
 }
 
@@ -4321,11 +4388,12 @@ dwarf2_gdb_index::expand_symtabs_matching
 
   if (symbol_matcher == NULL && lookup_name == NULL)
     {
-      for (const auto &per_cu : per_objfile->per_bfd->all_comp_units)
+      for (dwarf2_per_cu_data *per_cu
+            : all_comp_units_range (per_objfile->per_bfd))
        {
          QUIT;
 
-         if (!dw2_expand_symtabs_matching_one (per_cu.get (), per_objfile,
+         if (!dw2_expand_symtabs_matching_one (per_cu, per_objfile,
                                                file_matcher,
                                                expansion_notify))
            return false;
@@ -4437,14 +4505,14 @@ dwarf2_base_index_functions::map_symbol_filenames
        }
     }
 
-  for (const auto &per_cu : per_objfile->per_bfd->all_comp_units)
+  for (dwarf2_per_cu_data *per_cu
+        : all_comp_units_range (per_objfile->per_bfd))
     {
       /* We only need to look at symtabs not already expanded.  */
-      if (per_objfile->symtab_set_p (per_cu.get ()))
+      if (per_cu->is_debug_types || per_objfile->symtab_set_p (per_cu))
        continue;
 
-      quick_file_names *file_data = dw2_get_file_names (per_cu.get (),
-                                                       per_objfile);
+      quick_file_names *file_data = dw2_get_file_names (per_cu, per_objfile);
       if (file_data == nullptr
          || qfn_cache.find (file_data) != qfn_cache.end ())
        continue;
@@ -5309,11 +5377,12 @@ dwarf2_debug_names_index::expand_symtabs_matching
 
   if (symbol_matcher == NULL && lookup_name == NULL)
     {
-      for (const auto &per_cu : per_objfile->per_bfd->all_comp_units)
+      for (dwarf2_per_cu_data *per_cu
+            : all_comp_units_range (per_objfile->per_bfd))
        {
          QUIT;
 
-         if (!dw2_expand_symtabs_matching_one (per_cu.get (), per_objfile,
+         if (!dw2_expand_symtabs_matching_one (per_cu, per_objfile,
                                                file_matcher,
                                                expansion_notify))
            return false;
@@ -5421,7 +5490,6 @@ dwarf2_initialize_objfile (struct objfile *objfile)
       if (per_bfd->using_index)
        {
          dwarf_read_debug_printf ("using_index already set");
-         per_objfile->resize_symtabs ();
          objfile->qf.push_front (make_dwarf_gdb_index ());
          return;
        }
@@ -5430,7 +5498,6 @@ dwarf2_initialize_objfile (struct objfile *objfile)
       create_all_comp_units (per_objfile);
       per_bfd->quick_file_names_table
        = create_quick_file_names_table (per_bfd->all_comp_units.size ());
-      per_objfile->resize_symtabs ();
 
       for (int i = 0; i < per_bfd->all_comp_units.size (); ++i)
        {
@@ -5452,7 +5519,6 @@ dwarf2_initialize_objfile (struct objfile *objfile)
   if (per_bfd->debug_names_table != nullptr)
     {
       dwarf_read_debug_printf ("re-using shared debug names table");
-      per_objfile->resize_symtabs ();
       objfile->qf.push_front (make_dwarf_debug_names ());
       return;
     }
@@ -5462,7 +5528,6 @@ dwarf2_initialize_objfile (struct objfile *objfile)
   if (per_bfd->index_table != nullptr)
     {
       dwarf_read_debug_printf ("re-using shared index table");
-      per_objfile->resize_symtabs ();
       objfile->qf.push_front (make_dwarf_gdb_index ());
       return;
     }
@@ -5482,7 +5547,6 @@ dwarf2_initialize_objfile (struct objfile *objfile)
   if (dwarf2_read_debug_names (per_objfile))
     {
       dwarf_read_debug_printf ("found debug names");
-      per_objfile->resize_symtabs ();
       objfile->qf.push_front (make_dwarf_debug_names ());
       return;
     }
@@ -5492,7 +5556,6 @@ dwarf2_initialize_objfile (struct objfile *objfile)
                             get_gdb_index_contents_from_section<dwz_file>))
     {
       dwarf_read_debug_printf ("found gdb index from file");
-      per_objfile->resize_symtabs ();
       objfile->qf.push_front (make_dwarf_gdb_index ());
       return;
     }
@@ -5504,7 +5567,6 @@ dwarf2_initialize_objfile (struct objfile *objfile)
     {
       dwarf_read_debug_printf ("found gdb index from cache");
       global_index_cache.hit ();
-      per_objfile->resize_symtabs ();
       objfile->qf.push_front (make_dwarf_gdb_index ());
       return;
     }
@@ -5534,7 +5596,6 @@ dwarf2_build_psymtabs (struct objfile *objfile, psymbol_functions *psf)
        }
       else
        psf->set_partial_symtabs (per_bfd->partial_symtabs);
-      per_objfile->resize_symtabs ();
       return;
     }
 
@@ -5561,8 +5622,6 @@ dwarf2_build_psymtabs (struct objfile *objfile, psymbol_functions *psf)
       dwarf2_build_psymtabs_hard (per_objfile);
       psymtabs.keep ();
 
-      per_objfile->resize_symtabs ();
-
       /* (maybe) store an index in the cache.  */
       global_index_cache.store (per_objfile);
     }
@@ -5904,8 +5963,6 @@ add_type_unit (dwarf2_per_objfile *per_objfile, ULONGEST sig, void **slot)
     = per_objfile->per_bfd->allocate_signatured_type (sig);
   signatured_type *sig_type = sig_type_holder.get ();
 
-  per_objfile->resize_symtabs ();
-
   per_objfile->per_bfd->all_comp_units.emplace_back
     (sig_type_holder.release ());
   if (per_objfile->per_bfd->using_index)

diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h
index ee454ad300a12a74c1e90190241906bbafcbe7a9..1990849a81a43956872213821042a0e023a3679d 100644 (file)
--- a/gdb/dwarf2/read.h
+++ b/gdb/dwarf2/read.h
@@ -514,16 +514,6 @@ struct dwarf2_per_objfile
                                const struct comp_unit_head *cu_header,
                                unsigned int *bytes_read_ptr);
 
-  /* Resize the M_SYMTABS vector to the needed size (the number of partial
-     symtabs allocated by the per-bfd).  */
-  void resize_symtabs ()
-  {
-    /* The symtabs vector should only grow, not shrink.  */
-    gdb_assert (per_bfd->all_comp_units.size () >= m_symtabs.size ());
-
-    m_symtabs.resize (per_bfd->all_comp_units.size ());
-  }
-
   /* Return true if the symtab corresponding to PER_CU has been set,
      false otherwise.  */
   bool symtab_set_p (const dwarf2_per_cu_data *per_cu) const;