From: Simon Marchi Date: Mon, 3 Mar 2025 21:18:55 +0000 (-0500) Subject: gdb/dwarf: save DWARF version in dwarf2_loclist_baton, remove it from dwarf2_per_cu X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=f62cf22157cf527f1cc5c91854b1be3373024d4a;p=thirdparty%2Fbinutils-gdb.git 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 Cc: Tom de Vries Cc: Tom Tromey Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32309 Change-Id: I30d4ede7d67da5d80ff65c6122f5868e1098ec52 Approved-By: Tom Tromey --- diff --git a/gdb/dwarf2/loc.c b/gdb/dwarf2/loc.c index 398c2b1b352..7c12c0d4d4d 100644 --- 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 5c7330e43a0..de9e14479ea 100644 --- 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 a58c83257c3..a25daa36d38 100644 --- 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 4376ee9fbe9..e178c60509e 100644 --- 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 ();