gdb/dwarf: assume that no dwarf2_cu exist when calling load_full_comp_unit

After staring at the code, I got convinced that it was not possible for
load_full_comp_unit to be called while a dwarf2_cu object exists in
per_objfile for this_cu.  If you follow all callers of
load_full_comp_unit, you can see that all calls to load_full_comp_unit
(except one, see below) are gated one way or another by the fact that:

  per_objfile->get_cu (per_cu) == nullptr

Some calls are gated by maybe_queue_comp_unit returning true.  If it
returns true, then necessarily the dwarf2_cu is unset for that per_cu.

The spot that didn't seem to check for whether the dwarf2_cu is already
set before calling load_full_comp_unit is dw2_do_instantiate_symtab.  It
didn't trigger when running the testsuite, but I could imagine a made up
case where the dwarf2_cu would already be set because we looked up a DIE
reference to it (follow_die_ref) for whatever reason.  Then, something
would cause the symtab for that CU to be expanded and
dw2_do_instantiate_symtab to be called.

I added a check in that function, because it seemed prudent to do so.
All other load_cu calls are gated by this check, so it makes this call
look just like the others.

Finally, because all call sites that use cutu_reader::release_cu pass
nullptr for `existing_cu` (and therefore cutu_reader creates a new
dwarf2_cu), we know that cutu_reader::release_cu will always return a
non-nullptr value.  Add an assert in it and remove checks in
load_full_comp_unit and read_signatured_type.

Change-Id: I496be34bd4bf7edfa38d5135cf4bc4ccd960abe2
Approved-By: Tom Tromey <tom@tromey.com>

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 41e38c9297ac0a4ca2fe4ca8b16352f009950610..85e4d59a9ab38a9c47d525df65b29658fe78b217 100644 (file)
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -1676,7 +1676,10 @@ dw2_do_instantiate_symtab (dwarf2_per_cu *per_cu,
     if (!per_objfile->symtab_set_p (per_cu))
       {
        queue_comp_unit (per_cu, per_objfile);
-       dwarf2_cu *cu = load_cu (per_cu, per_objfile, skip_partial);
+       dwarf2_cu *cu = per_objfile->get_cu (per_cu);
+
+       if (cu == nullptr)
+         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.
@@ -3250,6 +3253,7 @@ dwarf2_cu_up
 cutu_reader::release_cu ()
 {
   gdb_assert (!m_dummy_p);
+  gdb_assert (m_new_cu != nullptr);
 
   return std::move (m_new_cu);
 }
@@ -4380,21 +4384,18 @@ load_full_comp_unit (dwarf2_per_cu *this_cu, dwarf2_per_objfile *per_objfile,
                     bool skip_partial, enum language pretend_language)
 {
   gdb_assert (! this_cu->is_debug_types);
+  gdb_assert (per_objfile->get_cu (this_cu) == nullptr);
 
-  cutu_reader reader (this_cu, per_objfile, NULL, per_objfile->get_cu (this_cu),
-                     skip_partial, pretend_language);
+  cutu_reader reader (this_cu, per_objfile, nullptr, nullptr, skip_partial,
+                     pretend_language);
   if (reader.is_dummy ())
     return;
 
   reader.read_all_dies ();
 
-  if (auto new_cu = reader.release_cu ();
-      new_cu != nullptr)
-    {
-      /* Save this dwarf2_cu in the per_objfile.  The per_objfile owns it
-        now.  */
-      per_objfile->set_cu (this_cu, std::move (new_cu));
-    }
+  /* Save this dwarf2_cu in the per_objfile.  The per_objfile owns it
+     now.  */
+  per_objfile->set_cu (this_cu, reader.release_cu ());
 }
 
 /* Add a DIE to the delayed physname list.  */
@@ -19091,9 +19092,7 @@ read_signatured_type (signatured_type *sig_type,
 
       /* Save this dwarf2_cu in the per_objfile.  The per_objfile owns it
         now.  */
-      dwarf2_cu_up new_cu = reader.release_cu ();
-      gdb_assert (new_cu != nullptr);
-      per_objfile->set_cu (sig_type, std::move (new_cu));
+      per_objfile->set_cu (sig_type, reader.release_cu ());
     }
 
   sig_type->tu_read = 1;