[PATCH 5/6] gdb/dwarf: store addr/offset/ref_addr sizes in dwarf2_per_cu

[Simon Marchi <simon.marchi@efficios.com>]

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 <error: Cannot access memory at address 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 <error: Cannot access memory at address 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 <minimal_language_defn>, 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
are 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
---
 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  | 161 ++++++++++++++++++
 4 files changed, 241 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 df6e784025dc..a8b9798c4e8e 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -2832,6 +2832,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);
@@ -3075,6 +3083,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.  */
@@ -18520,53 +18537,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 95f00e617c7a..c09536e73412 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 @@ struct dwarf2_per_cu
      any of the current compilation units are processed.  */
   packed<bool, 1> queued;
 
-private:
-  /* True if HEADER has been read in.  */
-  mutable packed<bool, 1> 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<bool, 1> files_read;
@@ -219,6 +213,14 @@ struct dwarf2_per_cu
      not.  */
   std::atomic<bool> scanned;
 
+private:
+  /* Sizes for an address, an offset, and a section offset.  These fields are
+     set by cutu_reader when the unit is read.  */
+  unsigned char m_addr_size = 0;
+  unsigned char m_offset_size = 0;
+  unsigned char m_ref_addr_size = 0;
+
+public:
   /* Our index in the unshared "symtabs" vector.  */
   unsigned index = 0;
 
@@ -231,12 +233,6 @@ struct dwarf2_per_cu
   /* 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 @@ struct dwarf2_per_cu
   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;
+  int 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 (int addr_size)
+  { m_addr_size = addr_size; }
 
   /* Return the offset size given in the compilation unit header for
      this CU.  */
-  int offset_size () const;
+  int 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 (int offset_size)
+  {  m_offset_size = offset_size; }
 
   /* Return the DW_FORM_ref_addr size given in the compilation unit
      header for this CU.  */
-  int ref_addr_size () const;
+  int 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.  */
+  void set_ref_addr_size (int 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 000000000000..1a84f5096334
--- /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
+   <http://www.gnu.org/licenses/>.  */
+
+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 000000000000..43bbe8f48436
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/fission-type-unit-locexpr.exp
@@ -0,0 +1,161 @@
+# 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 <http://www.gnu.org/licenses/>.
+
+# 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
+}
+
+if { [is_remote host] } {
+    gdb_remote_download host $dwo_file
+}
+
+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}"
-- 
2.51.2