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>

diff --git a/gdb/dwarf2/loc.c b/gdb/dwarf2/loc.c
index 398c2b1b35223181b6c477b37eda5da1ed50a88f..7c12c0d4d4ddc1520c3fa1716de39470ece31406 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;
@@ -3976,12 +3976,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,
@@ -4031,7 +4031,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 5c7330e43a09cfd6a798d26f733fdffb2d7de36d..de9e14479eae28d6a074896173ef72030e74db1b 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 a58c83257c3750c4cc3f6ab63937e5695c8b7bd0..a25daa36d38caca8f5651ea7bc0a1b4d94b492a7 100644 (file)
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -3153,8 +3153,6 @@ cutu_reader::cutu_reader (dwarf2_per_cu *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
        {
@@ -3165,7 +3163,6 @@ cutu_reader::cutu_reader (dwarf2_per_cu *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);
        }
     }
 
@@ -3937,11 +3934,6 @@ read_comp_units_from_section (dwarf2_per_objfile *per_objfile,
                       hex_string (sig_ptr->signature));
        }
 
-      /* 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));
     }
@@ -7895,7 +7887,7 @@ open_and_init_dwo_file (dwarf2_cu *cu, const char *dwo_name,
 
   create_cus_hash_table (cu, *dwo_file);
 
-  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);
   else
@@ -13322,8 +13314,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
-         || cu->producer_is_gas_2_39 ()))
+      && (cu->header.version == 2 || cu->producer_is_gas_2_39 ()))
     {
       /* Work around PR gas/29517, pretend we have an DW_TAG_unspecified_type
         return type.  */
@@ -19516,6 +19507,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 4376ee9fbe9509896cc0939349c2f514945b031d..e178c60509eaa1ce58bd1da4d6b6845d9d840786 100644 (file)
--- a/gdb/dwarf2/read.h
+++ b/gdb/dwarf2/read.h
@@ -129,9 +129,6 @@ struct dwarf2_per_cu
 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 is contained in struct signatured_type iff
@@ -290,24 +287,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 ();