From c20beece0854fa2fe2541b4a121d9b73188387f1 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Fri, 21 Nov 2025 15:14:03 -0500 Subject: [PATCH] gdb/dwarf: store addr/offset/ref_addr sizes in dwarf2_per_cu New in v2: - make the test try with indexes by default - using uint8_t instead of unsigned char In some specific circumstances, it is possible for GDB to read a type unit from a .dwo file without ever reading in the section of the stub in the main file. In that case, calling any of these methods: - dwarf2_per_cu::addr_size() - dwarf2_per_cu::offset_size() - dwarf2_per_cu::ref_addr_size() will cause a crash, because they will try to read the unit header from the not-read-in section buffer. See the previous patch for more details. The remaining calls to these methods are in the loc.c and expr.c files. That is, in the location and expression machinery. It is possible to set things up to cause them to trigger a crash, as shown by the new test, when running it with the cc-with-gdb-index board: $ make check TESTS="gdb.dwarf2/fission-type-unit-locexpr.exp" RUNTESTFLAGS="--target_board=cc-with-gdb-index" Running /home/simark/src/binutils-gdb/gdb/testsuite/gdb.dwarf2/fission-type-unit-locexpr.exp ... ERROR: GDB process no longer exists The backtrace at the moment of the crash is: #0 0x0000555566968b1f in bfd_getl32 (p=0x78) at /home/simark/src/binutils-gdb/bfd/libbfd.c:846 #1 0x00005555642e51b7 in read_initial_length (abfd=0x7d1ff1eb0e40, buf=0x78 , bytes_read=0x7bfff09daca0, handle_nonstd=true) at /home/simark/src/binutils-gdb/gdb/dwarf2/leb.c:92 #2 0x00005555647ca584 in read_unit_head (header=0x7d0ff1e06c70, info_ptr=0x78 , section=0x7c3ff1dea7d0, section_kind=ruh_kind::COMPILE) at /home/simark/src/binutils-gdb/gdb/dwarf2/unit-head.c:44 #3 0x000055556452df18 in dwarf2_per_cu::get_header (this=0x7d0ff1e06c40) at /home/simark/src/binutils-gdb/gdb/dwarf2/read.c:18531 #4 0x000055556452e10e in dwarf2_per_cu::addr_size (this=0x7d0ff1e06c40) at /home/simark/src/binutils-gdb/gdb/dwarf2/read.c:18544 #5 0x0000555564314ac3 in dwarf2_locexpr_baton_eval (dlbaton=0x7bfff0c9a508, frame=..., addr_stack=0x7bfff0b59150, valp=0x7bfff0c9a430, push_values=..., is_reference=0x7bfff0d33030) at /home/simark/src/binutils-gdb/gdb/dwarf2/loc.c:1593 #6 0x0000555564315bd2 in dwarf2_evaluate_property (prop=0x7bfff0c9a450, initial_frame=..., addr_stack=0x7bfff0b59150, value=0x7bfff0c9a430, push_values=...) at /home/simark/src/binutils-gdb/gdb/dwarf2/loc.c:1668 #7 0x0000555564a14ee1 in resolve_dynamic_field (field=..., addr_stack=0x7bfff0b59150, frame=...) at /home/simark/src/binutils-gdb/gdb/gdbtypes.c:2758 #8 0x0000555564a15e24 in resolve_dynamic_struct (type=0x7e0ff1f02550, addr_stack=0x7bfff0b59150, frame=...) at /home/simark/src/binutils-gdb/gdb/gdbtypes.c:2839 #9 0x0000555564a17061 in resolve_dynamic_type_internal (type=0x7e0ff1f02550, addr_stack=0x7bfff0b59150, frame=..., top_level=true) at /home/simark/src/binutils-gdb/gdb/gdbtypes.c:2972 #10 0x0000555564a17899 in resolve_dynamic_type (type=0x7e0ff1f02550, valaddr=..., addr=0x4010, in_frame=0x7bfff0d32e60) at /home/simark/src/binutils-gdb/gdb/gdbtypes.c:3019 #11 0x000055556675fb34 in value_from_contents_and_address (type=0x7e0ff1f02550, valaddr=0x0, address=0x4010, frame=...) at /home/simark/src/binutils-gdb/gdb/value.c:3674 #12 0x00005555666ce911 in get_value_at (type=0x7e0ff1f02550, addr=0x4010, frame=..., lazy=1) at /home/simark/src/binutils-gdb/gdb/valops.c:992 #13 0x00005555666ceb89 in value_at_lazy (type=0x7e0ff1f02550, addr=0x4010, frame=...) at /home/simark/src/binutils-gdb/gdb/valops.c:1039 #14 0x000055556491909f in language_defn::read_var_value (this=0x5555725fce40 , var=0x7e0ff1f02500, var_block=0x7e0ff1f025d0, frame_param=...) at /home/simark/src/binutils-gdb/gdb/findvar.c:504 #15 0x000055556491961b in read_var_value (var=0x7e0ff1f02500, var_block=0x7e0ff1f025d0, frame=...) at /home/simark/src/binutils-gdb/gdb/findvar.c:518 #16 0x00005555666d1861 in value_of_variable (var=0x7e0ff1f02500, b=0x7e0ff1f025d0) at /home/simark/src/binutils-gdb/gdb/valops.c:1384 #17 0x00005555647f7099 in evaluate_var_value (noside=EVAL_NORMAL, blk=0x7e0ff1f025d0, var=0x7e0ff1f02500) at /home/simark/src/binutils-gdb/gdb/eval.c:533 #18 0x00005555647f740c in expr::var_value_operation::evaluate (this=0x7c2ff1e3b690, expect_type=0x0, exp=0x7c2ff1e3aa00, noside=EVAL_NORMAL) at /home/simark/src/binutils-gdb/gdb/eval.c:559 #19 0x00005555647f3347 in expression::evaluate (this=0x7c2ff1e3aa00, expect_type=0x0, noside=EVAL_NORMAL) at /home/simark/src/binutils-gdb/gdb/eval.c:109 #20 0x000055556543ac2f in process_print_command_args (args=0x7fffffffe728 "global_var", print_opts=0x7bfff0be4a30, voidprint=true) at /home/simark/src/binutils-gdb/gdb/printcmd.c:1328 #21 0x000055556543ae65 in print_command_1 (args=0x7fffffffe728 "global_var", voidprint=1) at /home/simark/src/binutils-gdb/gdb/printcmd.c:1341 #22 0x000055556543b707 in print_command (exp=0x7fffffffe728 "global_var", from_tty=1) at /home/simark/src/binutils-gdb/gdb/printcmd.c:1408 The problem to solve is: in order to evaluate a location expression, we need to know some information (the various sizes) found in the unit header. In that context, it's not possible to get it from dwarf2_cu::header, like the previous patch did: at the time the expression is evaluated, the corresponding dwarf2_cu might have been freed. We don't want to re-build a dwarf2_cu just for that, it would be very inefficient. We could force reading in the dwarf2_per_cu::section section (in the main file), but we never needed to read that section before, so it would be better to avoid reading it unnecessarily. My initial attempt was to store this information in baton objects (dwarf2_locexpr_baton & co), so that it can be retrieved when the time comes to evaluate the expressions. However, it quickly became obvious that storing it there would be redundant and wasteful. I instead opted to store this information directly inside dwarf2_per_cu, making it easily available when evaluating expressions. These fields initially have the value 0, and are set by cutu_reader whenever the unit is parsed. The various getters (dwarf2_per_cu::addr_size & al) now just return these fields. Doing so allows removing anything related to reading the header from dwarf2_per_cu, which I think is a nice simplification. This means that nothing ever needs to read the header from just a dwarf2_per_cu. It also happens to shrink the dwarf2_per_cu object size a bit, going from: (top-gdb) p sizeof(dwarf2_per_cu) $1 = 176 to (top-gdb) p sizeof(dwarf2_per_cu) $1 = 120 I placed the new fields at this strange location in dwarf2_per_cu because there happened to be a nice 3 bytes hole there (on Linux amd64 at least). The new test set things up as described previously. Note that the crash only occurs if using the cc-with-gdb-index board. Change-Id: I50807a1bbb605f0f92606a9e61c026e3376a4fcf Approved-By: Andrew Burgess --- gdb/dwarf2/read.c | 64 ++---- gdb/dwarf2/read.h | 56 ++++-- .../gdb.dwarf2/fission-type-unit-locexpr.c | 25 +++ .../gdb.dwarf2/fission-type-unit-locexpr.exp | 183 ++++++++++++++++++ 4 files changed, 263 insertions(+), 65 deletions(-) create mode 100644 gdb/testsuite/gdb.dwarf2/fission-type-unit-locexpr.c create mode 100644 gdb/testsuite/gdb.dwarf2/fission-type-unit-locexpr.exp diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c index b34c3463774..49cbcdd96f4 100644 --- a/gdb/dwarf2/read.c +++ b/gdb/dwarf2/read.c @@ -2856,6 +2856,14 @@ cutu_reader::read_cutu_die_from_dwo (dwarf2_cu *cu, dwo_unit *dwo_unit, dwo_unit->length = cu->header.get_length_with_initial (); } + /* Record some information found in the header. This will be needed when + evaluating DWARF expressions in the context of this unit, for instance. */ + per_cu->set_addr_size (cu->header.addr_size); + per_cu->set_offset_size (cu->header.offset_size); + per_cu->set_ref_addr_size (cu->header.version == 2 + ? cu->header.addr_size + : cu->header.offset_size); + dwo_abbrev_section->read (objfile); m_dwo_abbrev_table = abbrev_table::read (dwo_abbrev_section, cu->header.abbrev_sect_off); @@ -3099,6 +3107,15 @@ 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 ()); } + + /* Record some information found in the header. This will be needed + when evaluating DWARF expressions in the context of this unit, for + instance. */ + this_cu.set_addr_size (cu->header.addr_size); + this_cu.set_offset_size (cu->header.offset_size); + this_cu.set_ref_addr_size (cu->header.version == 2 + ? cu->header.addr_size + : cu->header.offset_size); } /* Skip dummy compilation units. */ @@ -18642,53 +18659,6 @@ dwarf2_symbol_mark_computed (const struct attribute *attr, struct symbol *sym, /* See read.h. */ -const unit_head * -dwarf2_per_cu::get_header () const -{ - if (!m_header_read_in) - { - const gdb_byte *info_ptr - = this->section ()->buffer + to_underlying (this->sect_off ()); - - read_unit_head (&m_header, info_ptr, this->section (), ruh_kind::COMPILE); - - m_header_read_in = true; - } - - return &m_header; -} - -/* See read.h. */ - -int -dwarf2_per_cu::addr_size () const -{ - return this->get_header ()->addr_size; -} - -/* See read.h. */ - -int -dwarf2_per_cu::offset_size () const -{ - return this->get_header ()->offset_size; -} - -/* See read.h. */ - -int -dwarf2_per_cu::ref_addr_size () const -{ - const unit_head *header = this->get_header (); - - if (header->version == 2) - return header->addr_size; - else - return header->offset_size; -} - -/* See read.h. */ - void dwarf2_per_cu::set_lang (enum language lang, dwarf_source_language dw_lang) { diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h index 5d8660e9c42..bcbd18296ec 100644 --- a/gdb/dwarf2/read.h +++ b/gdb/dwarf2/read.h @@ -126,7 +126,6 @@ struct dwarf2_per_cu tu_read (false), lto_artificial (false), queued (false), - m_header_read_in (false), files_read (false), scanned (false), m_section (section), @@ -189,11 +188,6 @@ public: any of the current compilation units are processed. */ packed queued; -private: - /* True if HEADER has been read in. */ - mutable packed m_header_read_in; - -public: /* True if we've tried to read the file table. There will be no point in trying to read it again next time. */ packed files_read; @@ -219,6 +213,14 @@ public: not. */ std::atomic scanned; +private: + /* Sizes for an address, an offset, and a section offset. These fields are + set by cutu_reader when the unit is read. */ + std::uint8_t m_addr_size = 0; + std::uint8_t m_offset_size = 0; + std::uint8_t m_ref_addr_size = 0; + +public: /* Our index in the unshared "symtabs" vector. */ unsigned index = 0; @@ -231,12 +233,6 @@ private: /* Backlink to the owner of this. */ dwarf2_per_bfd *m_per_bfd; - /* DWARF header of this unit. Note that dwarf2_cu reads its own version of - the header, which may differ from this one, since it may pass - rch_kind::TYPE to read_unit_head, whereas for dwarf2_per_cu we always pass - ruh_kind::COMPILE. */ - mutable unit_head m_header; - public: /* The file and directory for this CU. This is cached so that we don't need to re-examine the DWO in some situations. This may be @@ -281,20 +277,44 @@ public: bool is_dwz () const { return m_is_dwz; } - /* Get the header of this per_cu, reading it if necessary. */ - const unit_head *get_header () const; - /* Return the address size given in the compilation unit header for this CU. */ - int addr_size () const; + std::uint8_t addr_size () const + { + gdb_assert (m_addr_size != 0); + return m_addr_size; + } + + /* Set the address size given in the compilation unit header for + this CU. */ + void set_addr_size (std::uint8_t addr_size) + { m_addr_size = addr_size; } /* Return the offset size given in the compilation unit header for this CU. */ - int offset_size () const; + std::uint8_t offset_size () const + { + gdb_assert (m_offset_size != 0); + return m_offset_size; + } + + /* Set the offset size given in the compilation unit header for + this CU. */ + void set_offset_size (std::uint8_t offset_size) + { m_offset_size = offset_size; } + + /* Return the DW_FORM_ref_addr size given in the compilation unit + header for this CU. */ + std::uint8_t ref_addr_size () const + { + gdb_assert (m_ref_addr_size != 0); + return m_ref_addr_size; + } /* Return the DW_FORM_ref_addr size given in the compilation unit header for this CU. */ - int ref_addr_size () const; + void set_ref_addr_size (std::uint8_t ref_addr_size) + { m_ref_addr_size = ref_addr_size; } /* Return length of this CU. */ unsigned int length () const diff --git a/gdb/testsuite/gdb.dwarf2/fission-type-unit-locexpr.c b/gdb/testsuite/gdb.dwarf2/fission-type-unit-locexpr.c new file mode 100644 index 00000000000..1a84f509633 --- /dev/null +++ b/gdb/testsuite/gdb.dwarf2/fission-type-unit-locexpr.c @@ -0,0 +1,25 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2025 Free Software Foundation, Inc. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see + . */ + +unsigned char buf[] = { 0x11, 0x22, 0x22, 0x11, }; + +int +main (void) +{ + return 0; +} diff --git a/gdb/testsuite/gdb.dwarf2/fission-type-unit-locexpr.exp b/gdb/testsuite/gdb.dwarf2/fission-type-unit-locexpr.exp new file mode 100644 index 00000000000..e6933a40edb --- /dev/null +++ b/gdb/testsuite/gdb.dwarf2/fission-type-unit-locexpr.exp @@ -0,0 +1,183 @@ +# Copyright 2025 Free Software Foundation, Inc. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +# This test is meant to reproduce a bug occurring in those specific +# circumstances: +# +# - Split DWARF (debug info in a .dwo file) +# - A type unit in the .dwo file, with a corresponding stub in the main file +# - A GDB index +# - We evaluate a DWARF expression in the context of that type unit +# +# The test doesn't create a GDB index, but that can be done by running it +# with the cc-with-gdb-index board. + +load_lib dwarf.exp + +require dwarf2_support + +standard_testfile .c -dw.S + +set asm_file [standard_output_file $srcfile2] + +Dwarf::assemble $asm_file { + set outdir [standard_output_file ""] + set cu_debug_addr_label "invalid" + + # Debug info in the DWO file. + + # Definition of "int". + tu { + fission 1 + version 4 + } 0xCAFE1 the_type_1 { + type_unit {} { + the_type_1: base_type { + DW_AT_byte_size 4 DW_FORM_sdata + DW_AT_encoding @DW_ATE_signed + DW_AT_name int + } + } + } + + # Definition of structure type "struct_type". + tu { + fission 1 + version 4 + } 0xCAFE2 the_type_2 { + type_unit {} { + the_type_2: DW_TAG_structure_type { + DW_AT_name struct_type + DW_AT_byte_size 4 DW_FORM_data1 + } { + DW_TAG_member { + DW_AT_name field + DW_AT_type 0xCAFE1 DW_FORM_ref_sig8 + + # This is the expression that is going to be evaluated + # by printing the variable. + DW_AT_data_member_location { + # In order to coax GDB to actually run this + # expression in the evaluator, it needs to contain + # something that will make decode_locdesc unable + # to reduce it to a constant. That's what these 3 + # operations (which are ultimately a no-op) are for. + # Note that there is an implicit + # DW_OP_push_object_address done before the + # expression is evaluated. + DW_OP_dup + DW_OP_deref_size 1 + DW_OP_drop + } SPECIAL_expr + } + } + } + } + + # Definition of the CU. + cu { + fission 1 + version 4 + } { + set cu_debug_addr_label [debug_addr_label] + + compile_unit { + DW_AT_GNU_dwo_id 0xF00D DW_FORM_data8 + } { + DW_TAG_variable { + DW_AT_name global_var + DW_AT_type 0xCAFE2 DW_FORM_ref_sig8 + DW_AT_location { + DW_OP_GNU_addr_index [gdb_target_symbol buf] + } SPECIAL_expr + } + } + } + + # Debug info in the main file. + + # Stub for "int" type unit. + tu { + version 4 + } 0xCAFE1 "" { + type_unit { + DW_AT_GNU_dwo_name ${::gdb_test_file_name}.dwo DW_FORM_strp + DW_AT_comp_dir ${outdir} + } { + } + } + + # Stub for "struct_type" type unit. + tu { + version 4 + } 0xCAFE2 "" { + type_unit { + DW_AT_GNU_dwo_name ${::gdb_test_file_name}.dwo DW_FORM_strp + DW_AT_comp_dir ${outdir} + } { + } + } + + # Stub for the CU. + cu { + version 4 + } { + compile_unit { + DW_AT_GNU_dwo_name ${::gdb_test_file_name}.dwo DW_FORM_strp + DW_AT_comp_dir ${outdir} + DW_AT_GNU_dwo_id 0xF00D DW_FORM_data8 + DW_AT_GNU_addr_base $cu_debug_addr_label + } {} + } +} + +set object_file [standard_output_file ${testfile}.o] +if { [build_executable_and_dwo_files "${testfile}.exp" ${binfile} {nodebug} \ + [list $asm_file {nodebug split-dwo} ${object_file}] \ + [list $srcfile {nodebug}]] } { + return -1 +} + +proc run_test { testfile } { + clean_restart ${testfile} + + # This print would cause the DW_AT_data_member_location expression to be + # evaluated and cause the crash. + gdb_test "print/x global_var" " = {field = 0x11222211}" +} + +run_test $testfile + +# Try adding a gdb-index and dwarf-5 style index, and then rerun the test. +foreach_with_prefix index_type { gdb dwarf5 } { + set binfile_with_index ${binfile}-idx-${index_type} + set testfile_with_index ${testfile}-idx-${index_type} + + remote_exec build "cp $binfile $binfile_with_index" + + if { $index_type eq "gdb" } { + set style "" + } else { + set style "-dwarf-5" + } + + # This is expected to fail if the binary already has an index. + if {[ensure_gdb_index $binfile_with_index $style] != 1} { + unsupported "couldn't add $index_type index" + return + } + + run_test $testfile_with_index +} -- 2.47.3