gdb/dwarf: move CU check up in cutu_reader::read_cutu_die_from_dwo

We have this pattern of check in multiple places:

    /* Skip dummy compilation units.  */
    if (m_info_ptr >= begin_info_ptr + this_cu->length ()
        || peek_abbrev_code (abfd, m_info_ptr) == 0)
      m_dummy_p = true;

In all places except one (read_cutu_die_from_dwo), this is done after
reading the unit header but before potentially reading the first DIE.
The effect is that we consider dummy units that have no DIE at all.
Either the "data" portion of the unit (the portion after the header) has
a size of zero, or the first abbrev code is 0, i.e. "end of list".

According to this old commit I found [1], dummy CUs were used as filler
for incremental LTO linking.  A comment reads:

   WARNING: If THIS_CU is a "dummy CU" (used as filler by the incremental
   linker) then DIE_READER_FUNC will not get called.

In read_cutu_die_from_dwo, however, this check is done after having read
the first DIE.  So at the time of the check, m_info_ptr has already been
advanced just past the first DIE.  As a result, compilations units with
a single DIE are considered (erroneously, IMO) as dummy.

In commit aab6de1613df ("gdb/dwarf: fix spurious error when encountering
dummy CU") [2], I mentioned a real world case where compilation units
with a single top-level DIE were being considered dummy.  I believe that
those units should not actually have been treated as dummy.  A CU with
just one DIE may not be very interesting, but I don't see any reason to
consider it dummy.

Move the dummy check above the read_toplevel_die call, and return early
if the CU is dummy.

I am 99% convinced that it's not even possible to encounter an empty
unit here, and considered turning it into an assert (it did pass the
testsuite).  This function is passed a dwo_unit, and functions that
create a dwo_unit are:

 - create_debug_type_hash_table (creates a dwo_unit for each type unit
   found in a dwo file)
 - create_cus_hash_table (creates a dwo_unit for each comp unit found in
   a dwo file)
 - create_dwo_unit_in_dwp_v1
 - create_dwo_unit_in_dwp_v2
 - create_dwo_unit_in_dwp_v5

In the first two, there are already dummy checks, so we wouldn't even
get to read_cutu_die_from_dwo for such an empty CU.  However, in the
last three, there is no such checks, we just trust the dwp file's index
and create dwo_units out of that.  So I guess it would be possible to
craft a broken dwp file with a CU that has no DIE.  Out of caution, I
didn't switch that to an assert, but I also don't really know what would
be the mode of failure if that were to happen.

Regtested using the various DWARF target boards on Debian 12.

[1] https://gitlab.com/gnutools/binutils-gdb/-/commit/dee91e82ae87f379c90fddff8db7c4b54a116609#dd409f60ba6f9c066432dafbda7093ac5eec76d1_3434_3419
[2] https://gitlab.com/gnutools/binutils-gdb/-/commit/aab6de1613df693059a6a2b505cc8f20d479d109

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

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index c5acd64dff94be12fba7e10faeb235f28b30993d..ff4f388daca9edf2d3cb1fd9238482aafb193009 100644 (file)
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -2833,6 +2833,14 @@ cutu_reader::read_cutu_die_from_dwo (dwarf2_cu *cu, dwo_unit *dwo_unit,
   this->init_cu_die_reader (cu, section, dwo_unit->dwo_file,
                            m_dwo_abbrev_table.get ());
 
+  /* Skip dummy compilation units.  */
+  if (m_info_ptr >= begin_info_ptr + dwo_unit->length
+      || peek_abbrev_code (abfd, m_info_ptr) == 0)
+    {
+      m_dummy_p = true;
+      return;
+    }
+
   /* Read in the die, filling in the attributes from the stub.  This
      has the benefit of simplifying the rest of the code - all the
      work to maintain the illusion of a single
@@ -2840,11 +2848,6 @@ cutu_reader::read_cutu_die_from_dwo (dwarf2_cu *cu, dwo_unit *dwo_unit,
   m_top_level_die
     = this->read_toplevel_die (gdb::make_array_view (attributes,
                                                     next_attr_idx));
-
-  /* Skip dummy compilation units.  */
-  if (m_info_ptr >= begin_info_ptr + dwo_unit->length
-      || peek_abbrev_code (abfd, m_info_ptr) == 0)
-    m_dummy_p = true;
 }
 
 /* Return the signature of the compile unit, if found. In DWARF 4 and before,