gdb: fix coerce_pieced_ref for multi-piece values

Bug PR gdb/30693 describes a case where the following assertion can be
triggered:

  ../../gdb/dwarf2/loc.c:2213: internal-error: value* coerce_pieced_ref(const value*): Assertion `closure->pieces.size () == 1' failed.

The problem is that coerce_pieced_ref makes the following claim:

      /* gdb represents synthetic pointers as pieced values with a single
        piece.  */
      gdb_assert (closure != NULL);
      gdb_assert (closure->pieces.size () == 1);

But this is not really true.  If an aggregate type contains a synthetic
pointer, then it is possible that the aggregate type will have a
computed location consisting of multiple pieces.  When GDB prints the
fields of that aggregate type these fields are extracted by calling
value::primitive_field.  Within value::primitive_field the location of
the field is set by calling value::set_component_location.

When the parent value that holds the field has a computed location, the
field value gains a reference to the parent value's closure, this can be
seen in copy_pieced_value_closure in dwarf2/expr.c.

What this means is that, if the aggregate value has a multi-piece
computed location, then the synthetic pointer field will also have a
reference to that same multi-piece computed location, even if there is
really only a single piece that describes the synthetic pointer itself.

Some parts of GDB are already aware of this.  If we look at
check_pieced_synthetic_pointer which implements the
value::bits_synthetic_pointer function, you'll see that this function
searches through all of the pieces to find the piece that covers the
value we are looking for, it then checks if that piece is an implicit
pointer location.  But back in coerce_pieced_ref, after calling
value::bits_synthetic_pointer, we still make the assertion that there
will be just a single piece.

Fix this by copying the search through all pieces logic into
coerce_pieced_ref (see note on efficiency below).  We now search through
all the pieces looking for a piece that describes the location of the
synthetic pointer, and we then use that piece to form the pointer's
value.

There are some assertions in the new code, these align with how
check_pieced_synthetic_pointer operates.

In addition, there is an error for the case where multiple pieces are
used to describe the location of a synthetic pointer.  This case is
technically allowed by check_pieced_synthetic_pointer, but supporting
this would require changes to indirect_synthetic_pointer, so I propose
leaving that until we see such a case in the wild.

On efficiency, you'll notice that check_pieced_synthetic_pointer
performs a search through all the location pieces, and
coerce_pieced_ref also has to search through the pieces.  It would be
nice if this could be avoided in order to avoid multiple searches.
Currently though coerce_pieced_ref calls
value->bits_synthetic_pointer, which is an API that should be agnostic
to the underlying implementation, i.e. shouldn't need to know that the
implementation is computed, so passing pieces back would be harder.

Maybe coerce_pieced_ref could avoid the value::bits_synthetic_pointer
call, and instead call check_pieced_synthetic_pointer directly, or
some related helper function, and could get the pieces back that way.
But this breaks the cleanly structured API that we currently have.

For now I'm leaving things as they are.  My assumption is that the
number of pieces used to represent a value is pretty low, so the
search is actually pretty cheap.

There's a new test that uses the DWARF assembler to create a
representative example of a multi-piece aggregate that contains a
synthetic pointer member variable.  This test triggers the assertion
before this commit.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30693
Bug: https://bugzilla.redhat.com/show_bug.cgi?id=2467251

Approved-By: Tom Tromey <tom@tromey.com>

diff --git a/gdb/dwarf2/expr.c b/gdb/dwarf2/expr.c
index 38b77f317413259897fdd34ad0e99ece05f7e935..f732781b3334e56d1af775c8ffa14e314a144cca 100644 (file)
--- a/gdb/dwarf2/expr.c
+++ b/gdb/dwarf2/expr.c
@@ -610,14 +610,55 @@ coerce_pieced_ref (const value *value)
       frame_info_ptr frame
        = get_selected_frame (_("No frame selected."));
 
-      /* gdb represents synthetic pointers as pieced values with a single
-        piece.  */
-      gdb_assert (closure != NULL);
-      gdb_assert (closure->pieces.size () == 1);
+      gdb_assert (closure != nullptr);
+
+      /* The value::bits_synthetic_pointer will return true if multiple
+        pieces are used to cover VALUE, so long as each piece is
+        DWARF_VALUE_IMPLICIT_POINTER.  I guess maybe this is possible, but
+        we've not seen such a case in the wild yet.  For now then we look
+        in CLOSURE for a single DWARF_VALUE_IMPLICIT_POINTER piece that
+        covers VALUE.  */
+      LONGEST bit_offset = TARGET_CHAR_BIT * (value->embedded_offset ()
+                                             + value->offset ());
+      if (value->bitsize ())
+       bit_offset += value->bitpos ();
+      int bit_length = TARGET_CHAR_BIT * type->length ();
+
+      const dwarf_expr_piece *piece = nullptr;
+      for (size_t i = 0; i < closure->pieces.size (); i++)
+       {
+         const dwarf_expr_piece *p = &closure->pieces[i];
+         size_t this_size_bits = p->size;
+
+         if (bit_offset >= this_size_bits)
+           {
+             bit_offset -= this_size_bits;
+             continue;
+           }
+
+         /* value::bits_synthetic_pointer does allow for multiple
+            pieces to describe the location of a single synthetic
+            pointer, or for a synthetic pointer to not start at the
+            exact start of a piece, however, we don't currently
+            support this case.  */
+         if (bit_offset != 0 || bit_length != this_size_bits)
+           error (_("unsupported value-piece configuration "
+                    "bit_offset = %s, bit_length = %d, this_size_bits = %s"),
+                  plongest (bit_offset), bit_length,
+                  pulongest (this_size_bits));
+
+         piece = p;
+         break;
+       }
+
+      /* If value::bits_synthetic_pointer returned true then we should have
+        found a suitable piece.  */
+      gdb_assert (piece != nullptr);
+      gdb_assert (piece->location == DWARF_VALUE_IMPLICIT_POINTER);
 
       return indirect_synthetic_pointer
-       (closure->pieces[0].v.ptr.die_sect_off,
-        closure->pieces[0].v.ptr.offset,
+       (piece->v.ptr.die_sect_off,
+        piece->v.ptr.offset,
         closure->per_cu, closure->per_objfile, frame, type);
     }
   else

diff --git a/gdb/testsuite/gdb.dwarf2/multi-piece-primitive-field.c b/gdb/testsuite/gdb.dwarf2/multi-piece-primitive-field.c
new file mode 100644 (file)
index 0000000..7ec6b26
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/multi-piece-primitive-field.c
@@ -0,0 +1,25 @@
+/* Copyright (C) 2026 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   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/>.  */
+
+int target_var = 42;
+
+int
+main (void)
+{
+  asm ("main_label: .globl main_label");
+  return 0;
+}

diff --git a/gdb/testsuite/gdb.dwarf2/multi-piece-primitive-field.exp b/gdb/testsuite/gdb.dwarf2/multi-piece-primitive-field.exp
new file mode 100644 (file)
index 0000000..6349d43
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/multi-piece-primitive-field.exp
@@ -0,0 +1,130 @@
+# Copyright 2026 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/>.
+
+# Setup a struct variable that is described by a multi-piece DWARF location.
+# One of the struct's fields is a C++ reference type implemented via
+# DW_OP_GNU_implicit_pointer.  At one time, attempting to print such a field
+# would trigger an assertion in GDB.
+#
+# The cause was that primitive_field extracts the reference field and
+# set_component_location copies the parent's entire piece_closure (just
+# incrementing its refcount).  Then coerce_pieced_ref would assert
+# closure->pieces.size() == 1, which failed because the closure still had
+# all the parent's pieces.
+#
+# This test confirms that this issue has now been fixed.
+
+require allow_cplus_tests
+
+load_lib dwarf.exp
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+require dwarf2_support
+
+standard_testfile .c .S
+
+set asm_file [standard_output_file ${srcfile2}]
+
+# First compile the C file only, so we can query some type sizes.
+if {[prepare_for_testing "failed to prepare" ${testfile} ${srcfile}]} {
+    return
+}
+
+Dwarf::assemble ${asm_file} {
+    cu {} {
+       DW_TAG_compile_unit {
+           DW_AT_language @DW_LANG_C_plus_plus
+       } {
+           declare_labels int_label struct_label ref_type_label target_label
+           set int_size [get_sizeof "int" -1]
+           set addr_size [get_sizeof "void *" -1]
+           set struct_size [expr {$int_size + $addr_size}]
+
+           int_label: DW_TAG_base_type {
+               DW_AT_byte_size ${int_size} DW_FORM_udata
+               DW_AT_encoding @DW_ATE_signed
+               DW_AT_name "int"
+           }
+
+           ref_type_label: DW_TAG_reference_type {
+               DW_AT_byte_size ${addr_size} DW_FORM_udata
+               DW_AT_type :${int_label}
+           }
+
+           struct_label: DW_TAG_structure_type {
+               DW_AT_name "S"
+               DW_AT_byte_size ${struct_size} DW_FORM_udata
+           } {
+               DW_TAG_member {
+                   DW_AT_name "x"
+                   DW_AT_type :${int_label}
+                   DW_AT_data_member_location 0 DW_FORM_udata
+               }
+
+               DW_TAG_member {
+                   DW_AT_name "ref"
+                   DW_AT_type :${ref_type_label}
+                   DW_AT_data_member_location ${int_size} DW_FORM_udata
+               }
+           }
+
+           target_label: DW_TAG_variable {
+               DW_AT_name "target_var"
+               DW_AT_type :${int_label}
+               DW_AT_external 1 DW_FORM_flag
+               DW_AT_location {
+                   DW_OP_addr [gdb_target_symbol "target_var"]
+               } SPECIAL_expr
+           }
+
+           DW_TAG_subprogram {
+               MACRO_AT_func { "main" }
+               DW_AT_type :${int_label}
+               DW_AT_external 1 DW_FORM_flag
+           } {
+               DW_TAG_variable {
+                   DW_AT_name "s"
+                   DW_AT_type :${struct_label}
+                   DW_AT_location {
+                       DW_OP_const4u 123
+                       DW_OP_stack_value
+                       DW_OP_piece $int_size
+                       DW_OP_GNU_implicit_pointer $target_label 0
+                       DW_OP_piece $addr_size
+                   } SPECIAL_expr
+               }
+           }
+       }
+    }
+}
+
+# Now compile both C source and generated DWARF assembly.
+if {[prepare_for_testing "failed to prepare" ${testfile} \
+        [list ${asm_file} ${srcfile}] {}]} {
+    return
+}
+
+# Start the inferior so that 's' is in scope.
+if {![runto_main]} {
+    return
+}
+
+# Printing `s` used to crash with an assertion failure in coerce_pieced_ref
+# because the ref field inherited the parent struct's multi-piece closure.
+gdb_test "print s" " = {x = 123, ref = @$hex}" \
+    "print multi-piece struct with implicit pointer reference field"
+
+gdb_test "print s.ref" " = \\(int &\\) @$hex: 42" \
+    "print ref member of multi-piece struct"