]> git.ipfire.org Git - thirdparty/binutils-gdb.git/commitdiff
gdb: force lval_memory for value components with dynamic data location
authorSimon Marchi <simon.marchi@efficios.com>
Tue, 18 Nov 2025 03:38:24 +0000 (22:38 -0500)
committerSimon Marchi <simon.marchi@polymtl.ca>
Mon, 24 Nov 2025 04:29:45 +0000 (23:29 -0500)
Using the test program from the added test, I get this internal error:

    $ ./gdb -q -nx --data-directory=data-directory testsuite/outputs/gdb.dwarf2/data-loc-cast/data-loc-cast -ex "with language fortran -- p (outer_type) g_outer_var" -batch
    $1 = ( /home/smarchi/src/binutils-gdb/gdb/value.c:1697: internal-error: set_component_location: Assertion `this->lval () == lval_memory' failed.

The test added by this patch mimics a problem that was reported when
debugging a Fortran program.  The situation is:

 - The DWARF defines a Fortran-style array type, with a
   DW_AT_data_location attribute.
 - It then defines a structure type (outer_type) with a field
   (outer_type::assoc) of that array type.
 - Trying to cast a minimal symbol (g_outer_var) to that structure type
   leads to the internal error shown above.

The use case for this is: for some reason, a variable of type S isn't
described in the debug info, but GDB still knows about type S.  The user
is trying to say: I know that the variable at this address is an S, show
it to me.

A Fortran-style array doesn't hold the data directly.  It's a structure
(a descriptor) that contains a pointer to the data and possible other
properties, like the length of the array.  The DW_AT_data_location
attribute contains a DWARF expression that yields the location the
actual data, given the location of the descriptor.  In GDB's type
system, this translates to a dynamic property of kind
DYN_PROP_DATA_LOCATION pointing to the DWARF expression.  Before
instantiating a value with such a dynamic type, the dynamic type must
first be resolved, which is done by function resolve_dynamic_type.  That
is, for each dynamic property, compute a concrete value for the specific
instance we're dealing with here.  The result of resolve_dynamic_type is
a type that is just like the input type, but where all dynamic
properties now have concrete, constant values.

Here's the timeline of what happens when doing "p (outer_type)
g_outer_var":

 1. We start in var_msym_value_operation::evaluate_for_cast.

 2. We go in function value_cast, in this branch:

      else if (arg2->lval () == lval_memory)
return value_at_lazy (to_type, arg2->address ());

    ... where to_type is the "outer_type" structure and arg2 is the
    minsym.  This effectively builds a value pretending that there
    exists an instance of outer_type at the address of the minsym, which
    is what we want.  The resulting value is "lval_memory".

 3. Somewhere inside value_at_lazy, there is a call to
    resolve_dynamic_type:

    #0  resolve_dynamic_type (type=0x7e0ff1cfd560, valaddr=..., addr=0x4340, in_frame=0x7bfff0b3f100) at /home/smarchi/src/binutils-gdb/gdb/gdbtypes.c:3011
    #1  0x000055556687dcba in value_from_contents_and_address (type=0x7e0ff1cfd560, valaddr=0x0, address=0x4340, frame=...) at /home/smarchi/src/binutils-gdb/gdb/value.c:3669
    #2  0x00005555667ec527 in get_value_at (type=0x7e0ff1cfd560, addr=0x4340, frame=..., lazy=1) at /home/smarchi/src/binutils-gdb/gdb/valops.c:992
    #3  0x00005555667ec79f in value_at_lazy (type=0x7e0ff1cfd560, addr=0x4340, frame=...) at /home/smarchi/src/binutils-gdb/gdb/valops.c:1039
    #4  0x00005555667e902b in value_cast (type=0x7e0ff1cfd560, arg2=0x7d0ff1c35540) at /home/smarchi/src/binutils-gdb/gdb/valops.c:645

    This is good, it returns a structure type where the type of field
    "assoc" has a constant DYN_PROP_DATA_LOCATION property that holds
    the memory address where the data for this array resides.

 4. Back in var_msym_value_operation::evaluate_for_cast, we do:

      val = value_cast (to_type, val);

      /* Don't allow e.g. '&(int)var_with_no_debug_info'.  */
      if (val->lval () == lval_memory)
        {
          if (val->lazy ())
            val->fetch_lazy ();
          val->set_lval (not_lval);
        }

    This is meant to make GDB behave more or less like C, where the
    result of a cast is not an lvalue, of which you can't take the
    address, for instance.  I am not an expert in this area, but Pedro
    explained that this lval thing in GDB actually conflates two things:

      - where is the value (memory, register, only in GDB's mind, etc)
      - is this an lvalue (is it assignable, can you take its address,
etc)

    Here, we would ideally want to say that the value is not an lvalue,
    but still say that it lives at a given address in memory.  But since
    the two concepts are conflated, we set it to "not_lval", which means
    "not an lval and does not exist on target".

    If there was a way to say "non-lvalue" and "in memory", I think that
    the bug that follows would be hidden.

 5. When printing the value, the value-printing code attempts to fetch
    the "assoc" field of the struct using value::primitive_field, which
    then goes into value::set_component_location.  In
    set_component_location there is this code, which is where we find
    the assert that fails:

      /* If the COMPONENT has a dynamic location, and is an
 lval_internalvar_component, then we change it to a lval_memory.

 Usually a component of an internalvar is created non-lazy, and has
 its content immediately copied from the parent internalvar.
 However, for components with a dynamic location, the content of
 the component is not contained within the parent, but is instead
 accessed indirectly.  Further, the component will be created as a
 lazy value.

 By changing the type of the component to lval_memory we ensure
 that value_fetch_lazy can successfully load the component.

 This solution isn't ideal, but a real fix would require values to
 carry around both the parent value contents, and the contents of
 any dynamic fields within the parent.  This is a substantial
 change to how values work in GDB.  */
      if (this->lval () == lval_internalvar_component)
{
  gdb_assert (lazy ());
  m_lval = lval_memory;
}
      else
gdb_assert (this->lval () == lval_memory);

    I think that what this comment is really trying to say is: if a
    structure is an internalvar, and a field of that structure has a
    dynamic data location, then the actual data is not contained in the
    internalvar, it is in memory.

    The message for the commit that introduced that code (3c8c6de21da
    "gdb: user variables with components of dynamic type")) confirms it,
    I think.  The message also goes on to explain that we could imagine
    a world where the internalvar outer struct value would also capture
    the (indirect) contents of the array field.  The internalvar value
    would then be completely self-contained.  I imagine this could be
    useful in some cases, but we don't have that today.

    The comment makes it sound like it's a hack, but I actually think it
    makes sense.  This is what is really happening.

    This all assumes that the result of the DW_AT_data_location is a
    location in memory.  I guess this is true for all practical purposes
    today, but it would be possible for DW_AT_data_location to yield a
    register, a composite, or even "undefined" (meaning optimized out)
    as  a location (which would be even easier to implement with the
    upcoming DWARF 6 "Location descriptions on the DWARF stack" feature
    [1]).  In that case, the location that we set for the array
    component should reflect whatever the DWARF expression returned.
    But that is future work, for now, we assume that the data location
    can only be in memory.

So, the fix is basically to always set the location of a value
sub-component to "memory", because we assume (for now) that the result
of all DW_AT_data_location expressions will be memory locations.

Now, referring back to point 4 above: if the code in
var_msym_value_operation::evaluate_for_cast could in fact set the value
to "non-lvalue in memory", then we wouldn't hit the assert in
value::set_component_location, because the subcomponent would have
inherited lval_memory from the parent structure.  However, I still think
that the fix in this patch is valid on its own.  Imagine that the DWARF
says that the outer struct (and thus the array descriptor) is in a
register.  Once we evaluate the DW_AT_data_location of the array field,
the value describing the actual data is still in memory.

The takeaway is that regardless of the location of the descriptor, the
DW_AT_data_location expression always returns a memory location (for
now), so we should always set the location of that value to memory.  In
other words, we apply the same logic as commit 3c8c6de21da regardless of
the location of the outer value".

Finally, a note about the test:  I made the array content very long (100
elements), because I did spot some issues where GDB would conflate the
size of the array data with the size of the array field, within the
outer structure.  The test does not expose these issues and I didn't try
to fix them here (one thing at a time), but making the array size very
large might help spot these issues in the future.

[1] https://dwarfstd.org/issues/230524.1.html

Change-Id: Ib10a77b62cd168fc7c08702e0f6dd47b5ac0f097
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33575
Approved-By: Tom Tromey <tom@tromey.com>
gdb/testsuite/gdb.dwarf2/data-loc-cast.c [new file with mode: 0644]
gdb/testsuite/gdb.dwarf2/data-loc-cast.exp [new file with mode: 0644]
gdb/value.c

diff --git a/gdb/testsuite/gdb.dwarf2/data-loc-cast.c b/gdb/testsuite/gdb.dwarf2/data-loc-cast.c
new file mode 100644 (file)
index 0000000..b5fbd40
--- /dev/null
@@ -0,0 +1,69 @@
+/* Copyright (C) 2024-2025 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/>.  */
+
+#include <limits.h>
+#include <stddef.h>
+#include <stdint.h>
+
+#define ARRAY_SIZE(a) (sizeof (a) / sizeof ((a)[0]))
+
+/* Use uintptr_t as a general purpose integer type to reduce the number of
+   types we need to define in the DWARF, and to make arithmetic simpler.  */
+
+struct array_type
+{
+  uintptr_t *data;
+  uintptr_t count;
+  uintptr_t lower_bound;
+};
+
+struct outer_type
+{
+  struct array_type assoc;
+  struct array_type non_assoc;
+};
+
+/* We spotted some code paths in GDB that would confuse the size of the
+   `.assoc` array with the size of the `.assoc` field within `g_outer_var`,
+   which is bogus.  Make the array quite large, to make any such bugs more
+   apparent.  */
+
+static uintptr_t g_outer_var_assoc_data[]
+  = { 1,  2,  3,  4,  5,  6,  7,  8,  9,  10, 11, 12, 13, 14, 15, 16, 17,
+      18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34,
+      35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51,
+      52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63, 64, 65, 66, 67, 68,
+      69, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, 80, 81, 82, 83, 84, 85,
+      86, 87, 88, 89, 90, 91, 92, 93, 94, 95, 96, 97, 98, 99, 100 };
+
+struct outer_type g_outer_var = {
+  .assoc = {
+    .data = g_outer_var_assoc_data,
+    .count = ARRAY_SIZE (g_outer_var_assoc_data),
+    .lower_bound = 6,
+  },
+
+  .non_assoc = {
+    .data = NULL,
+    .count = INT_MAX,
+    .lower_bound = INT_MAX,
+  },
+};
+
+int
+main ()
+{}
diff --git a/gdb/testsuite/gdb.dwarf2/data-loc-cast.exp b/gdb/testsuite/gdb.dwarf2/data-loc-cast.exp
new file mode 100644 (file)
index 0000000..3a420eb
--- /dev/null
@@ -0,0 +1,170 @@
+# 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/>.
+
+# Test casting some values to a type with dynamic properties.
+
+load_lib dwarf.exp
+
+require dwarf2_support
+
+standard_testfile .c -dw.S
+
+if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile}] } {
+    return
+}
+
+set asm_file [standard_output_file $srcfile2]
+Dwarf::assemble $asm_file {
+    cu {version 5} {
+       DW_TAG_compile_unit {
+           DW_AT_language @DW_LANG_Fortran95
+           DW_AT_name     FOO.F90
+           DW_AT_comp_dir /tmp
+       } {
+           set ptr_size [get_sizeof "void *" -1]
+
+           # int
+           declare_labels int_type_label
+           int_type_label: DW_TAG_base_type {
+               DW_AT_name int
+               DW_AT_byte_size 4 DW_FORM_udata
+               DW_AT_encoding @DW_ATE_signed
+           }
+
+           # char
+           declare_labels char_type_label
+           char_type_label: DW_TAG_base_type {
+               DW_AT_name char
+               DW_AT_byte_size 1 DW_FORM_udata
+               DW_AT_encoding @DW_ATE_signed
+           }
+
+           # uintptr_t
+           declare_labels uintptr_type_label
+           uintptr_type_label: DW_TAG_base_type {
+               DW_AT_name uintptr_t
+               DW_AT_byte_size $ptr_size DW_FORM_udata
+               DW_AT_encoding @DW_ATE_signed
+           }
+
+           # Array type.
+           declare_labels array_type_label
+           array_type_label: DW_TAG_array_type {
+               DW_AT_type :$uintptr_type_label
+               DW_AT_data_location {
+                   DW_OP_push_object_address
+                   DW_OP_deref
+               } SPECIAL_expr
+               DW_AT_associated {
+                   DW_OP_push_object_address
+                   DW_OP_deref
+                   DW_OP_lit0
+                   DW_OP_ne
+               } SPECIAL_expr
+           } {
+               DW_TAG_subrange_type {
+                   DW_AT_type :$uintptr_type_label
+                   DW_AT_count {
+                       DW_OP_push_object_address
+                       DW_OP_plus_uconst $ptr_size
+                       DW_OP_deref
+                   } SPECIAL_expr
+                   DW_AT_lower_bound {
+                       DW_OP_push_object_address
+                       DW_OP_plus_uconst [expr {$ptr_size * 2}]
+                       DW_OP_deref
+                   } SPECIAL_expr
+               }
+           }
+
+           # Structure type with the array type as a member.
+           declare_labels outer_type_label
+           set sizeof_outer_type [expr {6 * $ptr_size}]
+           outer_type_label: DW_TAG_structure_type {
+               DW_AT_name outer_type
+               DW_AT_byte_size $sizeof_outer_type DW_FORM_udata
+           } {
+               DW_TAG_member {
+                   DW_AT_name assoc
+                   DW_AT_type :$array_type_label
+                   DW_AT_data_member_location 0 DW_FORM_udata
+               }
+
+               DW_TAG_member {
+                   DW_AT_name non_assoc
+                   DW_AT_type :$array_type_label
+                   DW_AT_data_member_location [expr {$ptr_size * 3}] DW_FORM_udata
+               }
+           }
+
+           # Structure instance.
+           DW_TAG_variable {
+               DW_AT_name g_outer
+               DW_AT_type :$outer_type_label
+               DW_AT_location {
+                   DW_OP_addr [gdb_target_symbol g_outer_var]
+               } SPECIAL_expr
+           }
+       }
+    }
+}
+
+if { [prepare_for_testing "failed to prepare" ${testfile} \
+         [list $srcfile $asm_file] {nodebug}] } {
+    return
+}
+
+if {![runto_main]} {
+    return
+}
+
+# Generate a comma-separated sequence of numbers from LOW to HIGH (inclusive).
+#
+# For example
+#
+#   make_num_sequence 10 12
+#
+# returns
+#
+#   10, 11, 12
+proc make_num_sequence { low high } {
+    set nums {}
+
+    for {set x $low} {$x <= $high} {incr x} {
+       lappend nums $x
+    }
+
+    return [join $nums ", "]
+}
+
+set one_to_one_hundred [make_num_sequence 1 100]
+
+# Set the language to Fortran, to enable support for dynamic array types.
+gdb_test_no_output "set language fortran"
+
+# Verify normal operation.
+gdb_test "p g_outer" " = \\( assoc = \\($one_to_one_hundred\\), non_assoc = <not associated> \\)"
+gdb_test "p g_outer%assoc" " = \\($one_to_one_hundred\\)"
+gdb_test "p g_outer%non_assoc" " = <not associated>"
+gdb_test "ptype g_outer%assoc" " = uintptr_t \\(6:105\\)"
+gdb_test "ptype g_outer%non_assoc" " = uintptr_t \\(:\\)"
+
+# Cast g_outer_var (a minimal symbol) to outer_type.  This would trigger an
+# assert.
+gdb_test "p (outer_type) g_outer_var" " = \\( assoc = \\($one_to_one_hundred\\), non_assoc = <not associated> \\)"
+gdb_test "p ((outer_type) g_outer_var)%assoc" "= \\($one_to_one_hundred\\)"
+gdb_test "p ((outer_type) g_outer_var)%non_assoc" " = <not associated>"
+gdb_test "ptype ((outer_type) g_outer_var)%assoc" " = uintptr_t \\(6:105\\)"
+gdb_test "ptype ((outer_type) g_outer_var)%non_assoc" " = uintptr_t \\(:\\)"
index 7ee212ab2a2028a16921b4cd0726f13ee4e77a61..b7ca663d998ef447bf0369bdab95d71a6730b48a 100644 (file)
@@ -1671,31 +1671,22 @@ value::set_component_location (const struct value *whole)
   if (dynamic_prop *dyn_prop = type->dyn_prop (DYN_PROP_DATA_LOCATION);
       dyn_prop != nullptr && dyn_prop->is_constant ())
     {
-      /* If the COMPONENT has a dynamic location, and is an
-        lval_internalvar_component, then we change it to a lval_memory.
-
-        Usually a component of an internalvar is created non-lazy, and has
-        its content immediately copied from the parent internalvar.
-        However, for components with a dynamic location, the content of
-        the component is not contained within the parent, but is instead
-        accessed indirectly.  Further, the component will be created as a
-        lazy value.
-
-        By changing the type of the component to lval_memory we ensure
-        that value_fetch_lazy can successfully load the component.
-
-        This solution isn't ideal, but a real fix would require values to
-        carry around both the parent value contents, and the contents of
-        any dynamic fields within the parent.  This is a substantial
-        change to how values work in GDB.  */
-      if (this->lval () == lval_internalvar_component)
-       {
-         gdb_assert (lazy ());
-         m_lval = lval_memory;
-       }
-      else
-       gdb_assert (this->lval () == lval_memory);
-
+      /* If the COMPONENT has a dynamic location, force it to have lval_memory.
+
+        The data location, obtained by evaluating the DYN_PROP_DATA_LOCATION
+        property (typically a DWARF DW_AT_data_location attribute), isn't
+        related to the location of the object.  Even if the object is, let's
+        say, a register, evaluating DYN_PROP_DATA_LOCATION yields a different
+        location.  At the moment, we assume that it yields a memory location.
+        But in theory, a DW_AT_data_location attribute could yield any kind of
+        location.
+
+        Even in the case of internalvars, only the content of the object itself
+        is copied to the internalvar.  If the object has a DYN_PROP_DATA_LOCATION
+        property pointing outside the object, that data isn't captured in the
+        internalvar.  */
+      gdb_assert (lazy ());
+      m_lval = lval_memory;
       set_address (dyn_prop->const_val ());
     }
 }