gdb/dwarf: save DWARF version in dwarf2_loclist_baton, remove it from dwarf2_per_cu

When running:

    $ make check TESTS="gdb.cp/cpexprs-debug-types.exp" RUNTESTFLAGS="--target_board=fission"

I get:

    (gdb) break -qualified main
    /home/smarchi/src/binutils-gdb/gdb/dwarf2/read.h:295: internal-error: version: Assertion `m_dwarf_version != 0' failed.

The problem is that dwarf2_per_cu objects created in the
read_cutu_die_from_dwo code path never have their DWARF version set.  A
seemingly obvious solution would be to add a call to
dwarf2_per_cu::set_version in there (there's a patch in the referenced
PR that does that).  However, this comment in
read_comp_units_from_section is a bit scary:

      /* Init this asap, to avoid a data race in the set_version in
	 cutu_reader::cutu_reader (which may be run in parallel for the cooked
	 index case).  */
      this_cu->set_version (cu_header.version);

I don't know if a DWO file can be read while the cooked indexer runs, so
if it would be a problem here, but I prefer to be safe than sorry.  This
patch side-steps the problem by deleting the DWARF version from
dwarf2_per_cu.

The only users of dwarf2_per_cu::version are the loclists callbacks in
`loc.c`.  Add the DWARF version to dwarf2_loclist_baton and modify those
callbacks to get the version from there instead.  Initialize that new
field in fill_in_loclist_baton.

I like this approach because there is no version field that is possibly
unset now.

I wasn't keen on doing this at first because I thought it would waste
space, but the dwarf2_loclist_baton has 7 bytes of padding at the end
anyway, so we might as well use that.

Cc: Ricky Zhou <ricky@rzhou.org>
Cc: Tom de Vries <tdevries@suse.de>
Cc: Tom Tromey <tom@tromey.com>
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32309
Change-Id: I30d4ede7d67da5d80ff65c6122f5868e1098ec52
Approved-By: Tom Tromey <tom@tromey.com>
(cherry picked from commit f62cf22157cf527f1cc5c91854b1be3373024d4a)

diff --git a/gdb/dwarf2/loc.c b/gdb/dwarf2/loc.c
index 989d33901e01070d05c1009cdd80db60af4f0b2f..9ea630f49b4c4c2b05a277f8b14de57475d01cc0 100644 (file)
--- a/gdb/dwarf2/loc.c
+++ b/gdb/dwarf2/loc.c
@@ -388,12 +388,12 @@ dwarf2_find_location_expression (const dwarf2_loclist_baton *baton,
       enum debug_loc_kind kind;
       const gdb_byte *new_ptr = NULL; /* init for gcc -Wall */
 
-      if (baton->per_cu->version () < 5 && baton->from_dwo)
+      if (baton->dwarf_version < 5 && baton->from_dwo)
        kind = decode_debug_loc_dwo_addresses (baton->per_cu,
                                               baton->per_objfile,
                                               loc_ptr, buf_end, &new_ptr,
                                               &low, &high, byte_order);
-      else if (baton->per_cu->version () < 5)
+      else if (baton->dwarf_version < 5)
        kind = decode_debug_loc_addresses (loc_ptr, buf_end, &new_ptr,
                                           &low, &high,
                                           byte_order, addr_size,
@@ -444,7 +444,7 @@ dwarf2_find_location_expression (const dwarf2_loclist_baton *baton,
          high = (unrelocated_addr) ((CORE_ADDR) high + (CORE_ADDR) base_address);
        }
 
-      if (baton->per_cu->version () < 5)
+      if (baton->dwarf_version < 5)
        {
          length = extract_unsigned_integer (loc_ptr, 2, byte_order);
          loc_ptr += 2;
@@ -3977,12 +3977,12 @@ loclist_describe_location (struct symbol *symbol, CORE_ADDR addr,
       enum debug_loc_kind kind;
       const gdb_byte *new_ptr = NULL; /* init for gcc -Wall */
 
-      if (dlbaton->per_cu->version () < 5 && dlbaton->from_dwo)
+      if (dlbaton->dwarf_version < 5 && dlbaton->from_dwo)
        kind = decode_debug_loc_dwo_addresses (dlbaton->per_cu,
                                               per_objfile,
                                               loc_ptr, buf_end, &new_ptr,
                                               &low, &high, byte_order);
-      else if (dlbaton->per_cu->version () < 5)
+      else if (dlbaton->dwarf_version < 5)
        kind = decode_debug_loc_addresses (loc_ptr, buf_end, &new_ptr,
                                           &low, &high,
                                           byte_order, addr_size,
@@ -4032,7 +4032,7 @@ loclist_describe_location (struct symbol *symbol, CORE_ADDR addr,
       CORE_ADDR low_reloc = per_objfile->relocate (low);
       CORE_ADDR high_reloc = per_objfile->relocate (high);
 
-      if (dlbaton->per_cu->version () < 5)
+      if (dlbaton->dwarf_version < 5)
         {
           length = extract_unsigned_integer (loc_ptr, 2, byte_order);
           loc_ptr += 2;

diff --git a/gdb/dwarf2/loc.h b/gdb/dwarf2/loc.h
index 78635682d9446b0220833fda6b96447080068653..8f7940657e53424bcb5a0f8a061c732d8ad918db 100644 (file)
--- a/gdb/dwarf2/loc.h
+++ b/gdb/dwarf2/loc.h
@@ -197,6 +197,9 @@ struct dwarf2_loclist_baton
   /* Non-zero if the location list lives in .debug_loc.dwo.
      The format of entries in this section are different.  */
   unsigned char from_dwo;
+
+  /* The version of DWARF this loclist comes from.  */
+  unsigned char dwarf_version;
 };
 
 /* The baton used when a dynamic property is an offset to a parent

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 096a20cb6276046fff72e67448cb5ebcb1605aaa..cc664a330024a11458a841556617c87f2515fa47 100644 (file)
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -4103,8 +4103,6 @@ cutu_reader::cutu_reader (dwarf2_per_cu_data *this_cu,
          /* Establish the type offset that can be used to lookup the type.  */
          sig_type->type_offset_in_section =
            this_cu->sect_off + to_underlying (sig_type->type_offset_in_tu);
-
-         this_cu->set_version (cu->header.version);
        }
       else
        {
@@ -4115,7 +4113,6 @@ cutu_reader::cutu_reader (dwarf2_per_cu_data *this_cu,
 
          gdb_assert (this_cu->sect_off == cu->header.sect_off);
          this_cu->set_length (cu->header.get_length_with_initial ());
-         this_cu->set_version (cu->header.version);
        }
     }
 
@@ -5129,10 +5126,6 @@ read_comp_units_from_section (dwarf2_per_objfile *per_objfile,
       this_cu->set_length (cu_header.get_length_with_initial ());
       this_cu->is_dwz = is_dwz;
       this_cu->section = section;
-      /* Init this asap, to avoid a data race in the set_version in
-        cutu_reader::cutu_reader (which may be run in parallel for the cooked
-        index case).  */
-      this_cu->set_version (cu_header.version);
 
       info_ptr = info_ptr + this_cu->length ();
       per_objfile->per_bfd->all_units.push_back (std::move (this_cu));
@@ -9237,7 +9230,7 @@ open_and_init_dwo_file (dwarf2_cu *cu, const char *dwo_name,
   create_cus_hash_table (per_objfile, cu, *dwo_file, dwo_file->sections.info,
                         dwo_file->cus);
 
-  if (cu->per_cu->version () < 5)
+  if (cu->header.version < 5)
     {
       create_debug_types_hash_table (per_objfile, dwo_file.get (),
                                     dwo_file->sections.types, dwo_file->tus);
@@ -14800,8 +14793,7 @@ read_subroutine_type (struct die_info *die, struct dwarf2_cu *cu)
   if (type->code () == TYPE_CODE_VOID
       && !type->is_stub ()
       && die->child == nullptr
-      && (cu->per_cu->version () == 2
-         || producer_is_gas_2_39 (cu)))
+      && (cu->header.version == 2 || producer_is_gas_2_39 (cu)))
     {
       /* Work around PR gas/29517, pretend we have an DW_TAG_unspecified_type
         return type.  */
@@ -21693,6 +21685,7 @@ fill_in_loclist_baton (struct dwarf2_cu *cu,
   else
     baton->base_address = {};
   baton->from_dwo = cu->dwo_unit != NULL;
+  baton->dwarf_version = cu->header.version;
 }
 
 static void

diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h
index e8735ddef6faa5eeb8c8077c73d2b5a61ef450b3..24a47ebd255a23e3445e62fc211c868e7b8aeab1 100644 (file)
--- a/gdb/dwarf2/read.h
+++ b/gdb/dwarf2/read.h
@@ -120,9 +120,6 @@ struct dwarf2_per_cu_data
 private:
   unsigned int m_length = 0;
 
-  /* DWARF standard version this data has been read from (such as 4 or 5).  */
-  unsigned char m_dwarf_version = 0;
-
 public:
   /* Non-zero if this CU is from .debug_types.
      Struct dwarf2_per_cu_data is contained in struct signatured_type iff
@@ -281,24 +278,6 @@ public:
       gdb_assert (m_length == length);
   }
 
-  /* Return DWARF version number of this CU.  */
-  short version () const
-  {
-    /* Make sure it's set already.  */
-    gdb_assert (m_dwarf_version != 0);
-    return m_dwarf_version;
-  }
-
-  void set_version (short version)
-  {
-    if (m_dwarf_version == 0)
-      /* Set if not set already.  */
-      m_dwarf_version = version;
-    else
-      /* If already set, verify that it's the same value.  */
-      gdb_assert (m_dwarf_version == version);
-  }
-
   dwarf_unit_type unit_type (bool strict_p = true) const
   {
     dwarf_unit_type ut = m_unit_type.load ();