gdb/dwarf: create and destroy dwarf2_per_bfd's CUs-to-expand queue

As described in the log of patch "gdb/dwarf: add assertion in
maybe_queue_comp_unit", it would happen that a call to
maybe_queue_comp_unit would enqueue a CU in the to-expand queue while
nothing up the stack was processing the queue.  This is not desirable,
as items are then left lingering in the queue when we exit the
dwarf2/read code.  This is an inconsistent state.

The normal case of using the queue is when we go through
dw2_do_instantiate_symtab and process_queue.  As depended-on CUs are
found, they get added to the queue.  process_queue expands CUs until the
queue is empty.

To catch these cases where things are enqueued while nothing up the
stack is processing the queue, change dwarf2_per_bfd::queue to be an
optional.  The optional is instantiated in dwarf2_queue_guard, just
before where we call process_queue.  In the dwarf2_queue_guard
destructor, the optional gets reset.  Therefore, the queue object is
instantiated only when something up the stack is handling it.  If
another entry point tries to enqueue a CU for expansion, an assertion
will fail and we know we have something to fix.

dwarf2_queue_guard sounds like the good place for this, as it's
currently responsible for making sure the queue gets cleared if we exit
due to an error.

This also allows asserting that when age_comp_units or remove_all_cus
run, the queue is not instantiated, and gives us one more level of
assurance that we won't free the DIEs of a CU that is in the
CUs-to-expand queue.

gdb/ChangeLog:

	PR gdb/26828
	* dwarf2/read.c (dwarf2_queue_guard) <dwarf2_queue_guard>:
	Instantiate queue.
	(~dwarf2_queue_guard): Clear queue.
	(queue_comp_unit): Assert that queue is
	instantiated.
	(process_queue): Adjust.
	* dwarf2/read.h (struct dwarf2_per_bfd) <queue>: Make optional.

Change-Id: I8fe3d77845bb4ad3d309eac906acebe79d9f0a9d

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 6aa71ea4e23ed8ea208fbbb6ed72f6821b17888f..d189a0919b277a8d96cfc3ce4975f79ebf116630 100644 (file)
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,14 @@
+2021-02-23  Simon Marchi  <simon.marchi@polymtl.ca>
+
+       PR gdb/26828
+       * dwarf2/read.c (dwarf2_queue_guard) <dwarf2_queue_guard>:
+       Instantiate queue.
+       (~dwarf2_queue_guard): Clear queue.
+       (queue_comp_unit): Assert that queue is
+       instantiated.
+       (process_queue): Adjust.
+       * dwarf2/read.h (struct dwarf2_per_bfd) <queue>: Make optional.
+
 2021-02-23  Simon Marchi  <simon.marchi@polymtl.ca>
 
        PR gdb/26828

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index b36694e54fe1bcff3b7a1f73c6a8c6091ae33e11..0fccce869fc09aac5d0fd1bc4b1dc7a3db59bb55 100644 (file)
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -1684,15 +1684,18 @@ public:
   explicit dwarf2_queue_guard (dwarf2_per_objfile *per_objfile)
     : m_per_objfile (per_objfile)
   {
+    gdb_assert (!m_per_objfile->per_bfd->queue.has_value ());
+
+    m_per_objfile->per_bfd->queue.emplace ();
   }
 
   /* Free any entries remaining on the queue.  There should only be
      entries left if we hit an error while processing the dwarf.  */
   ~dwarf2_queue_guard ()
   {
-    /* Ensure that no memory is allocated by the queue.  */
-    std::queue<dwarf2_queue_item> empty;
-    std::swap (m_per_objfile->per_bfd->queue, empty);
+    gdb_assert (m_per_objfile->per_bfd->queue.has_value ());
+
+    m_per_objfile->per_bfd->queue.reset ();
   }
 
   DISABLE_COPY_AND_ASSIGN (dwarf2_queue_guard);
@@ -1861,6 +1864,8 @@ dwarf2_per_bfd::~dwarf2_per_bfd ()
 void
 dwarf2_per_objfile::remove_all_cus ()
 {
+  gdb_assert (!this->per_bfd->queue.has_value ());
+
   for (auto pair : m_dwarf2_cus)
     delete pair.second;
 
@@ -2542,30 +2547,32 @@ dw2_do_instantiate_symtab (dwarf2_per_cu_data *per_cu,
   if (per_cu->type_unit_group_p ())
     return;
 
-  /* The destructor of dwarf2_queue_guard frees any entries left on
-     the queue.  After this point we're guaranteed to leave this function
-     with the dwarf queue empty.  */
-  dwarf2_queue_guard q_guard (per_objfile);
-
-  if (!per_objfile->symtab_set_p (per_cu))
-    {
-      queue_comp_unit (per_cu, per_objfile, language_minimal);
-      dwarf2_cu *cu = load_cu (per_cu, per_objfile, skip_partial);
+  {
+    /* The destructor of dwarf2_queue_guard frees any entries left on
+       the queue.  After this point we're guaranteed to leave this function
+       with the dwarf queue empty.  */
+    dwarf2_queue_guard q_guard (per_objfile);
 
-      /* If we just loaded a CU from a DWO, and we're working with an index
-        that may badly handle TUs, load all the TUs in that DWO as well.
-        http://sourceware.org/bugzilla/show_bug.cgi?id=15021  */
-      if (!per_cu->is_debug_types
-         && cu != NULL
-         && cu->dwo_unit != NULL
-         && per_objfile->per_bfd->index_table != NULL
-         && per_objfile->per_bfd->index_table->version <= 7
-         /* DWP files aren't supported yet.  */
-         && get_dwp_file (per_objfile) == NULL)
-       queue_and_load_all_dwo_tus (cu);
-    }
+    if (!per_objfile->symtab_set_p (per_cu))
+      {
+       queue_comp_unit (per_cu, per_objfile, language_minimal);
+       dwarf2_cu *cu = load_cu (per_cu, per_objfile, skip_partial);
+
+       /* If we just loaded a CU from a DWO, and we're working with an index
+          that may badly handle TUs, load all the TUs in that DWO as well.
+          http://sourceware.org/bugzilla/show_bug.cgi?id=15021  */
+       if (!per_cu->is_debug_types
+           && cu != NULL
+           && cu->dwo_unit != NULL
+           && per_objfile->per_bfd->index_table != NULL
+           && per_objfile->per_bfd->index_table->version <= 7
+           /* DWP files aren't supported yet.  */
+           && get_dwp_file (per_objfile) == NULL)
+         queue_and_load_all_dwo_tus (cu);
+      }
 
-  process_queue (per_objfile);
+    process_queue (per_objfile);
+  }
 
   /* Age the cache, releasing compilation units that have not
      been used recently.  */
@@ -9180,7 +9187,9 @@ queue_comp_unit (dwarf2_per_cu_data *per_cu,
                 enum language pretend_language)
 {
   per_cu->queued = 1;
-  per_cu->per_bfd->queue.emplace (per_cu, per_objfile, pretend_language);
+
+  gdb_assert (per_objfile->per_bfd->queue.has_value ());
+  per_cu->per_bfd->queue->emplace (per_cu, per_objfile, pretend_language);
 }
 
 /* If PER_CU is not yet expanded of queued for expansion, add it to the queue.
@@ -9275,9 +9284,9 @@ process_queue (dwarf2_per_objfile *per_objfile)
 
   /* The queue starts out with one item, but following a DIE reference
      may load a new CU, adding it to the end of the queue.  */
-  while (!per_objfile->per_bfd->queue.empty ())
+  while (!per_objfile->per_bfd->queue->empty ())
     {
-      dwarf2_queue_item &item = per_objfile->per_bfd->queue.front ();
+      dwarf2_queue_item &item = per_objfile->per_bfd->queue->front ();
       dwarf2_per_cu_data *per_cu = item.per_cu;
 
       if (!per_objfile->symtab_set_p (per_cu))
@@ -9323,7 +9332,7 @@ process_queue (dwarf2_per_objfile *per_objfile)
        }
 
       per_cu->queued = 0;
-      per_objfile->per_bfd->queue.pop ();
+      per_objfile->per_bfd->queue->pop ();
     }
 
   dwarf_read_debug_printf ("Done expanding symtabs of %s.",
@@ -25122,6 +25131,13 @@ dwarf2_per_objfile::age_comp_units ()
 {
   dwarf_read_debug_printf_v ("running");
 
+  /* This is not expected to be called in the middle of CU expansion.  There is
+     an invariant that if a CU is in the CUs-to-expand queue, its DIEs are
+     loaded in memory.  Calling age_comp_units while the queue is in use could
+     make us free the DIEs for a CU that is in the queue and therefore break
+     that invariant.  */
+  gdb_assert (!this->per_bfd->queue.has_value ());
+
   /* Start by clearing all marks.  */
   for (auto pair : m_dwarf2_cus)
     pair.second->mark = false;

diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h
index d2bae5a7ee878fd5d241205c10ef85e6c075d7e2..c7f6a111b8c4c321568318acb454fb1e893dd510 100644 (file)
--- a/gdb/dwarf2/read.h
+++ b/gdb/dwarf2/read.h
@@ -250,7 +250,7 @@ public:
     abstract_to_concrete;
 
   /* CUs that are queued to be read.  */
-  std::queue<dwarf2_queue_item> queue;
+  gdb::optional<std::queue<dwarf2_queue_item>> queue;
 
   /* We keep a separate reference to the partial symtabs, in case we
      are sharing them between objfiles.  This is only set after