Fix regression with dynamic array bounds

Kévin discovered that commit ba005d32b0 ("Handle dynamic field
properties") regressed a test in the internal AdaCore test suite.

The problem here is that, when writing that patch, I did not consider
the case where an array type's bounds might come from a member of a
structure -- but where the array is not defined in the structure's
scope.

In this scenario the field-resolution logic would trip this condition:

  /* Defensive programming in case we see unusual DWARF.  */
  if (fi == nullptr)
    return nullptr;

This patch reworks this area, partly backing out that commit, and
fixes the problem.

In the new code, I chose to simply duplicate the field's location
information.  This isn't totally ideal, in that it might result in
multiple copies of a baton.  However, this seemed nicer than tracking
the DIE/field correspondence for every field in every CU -- my
thinking here is that this particular dynamic scenario is relatively
rare overall.  Also, if the baton cost does prove onerous, we could
intern the batons somewhere.

Regression tested on x86-64 Fedora 41.  I also tested this using the
AdaCore internal test suite.

Tested-By: Simon Marchi <simon.marchi@efficios.com>
This commit is contained in:
Tom Tromey
2025-05-13 13:41:26 -06:00
parent 27ba92a50c
commit 9b02626409
6 changed files with 154 additions and 81 deletions

View File

@@ -27,8 +27,6 @@
#include "gdbsupport/unordered_set.h"
#include "dwarf2/die.h"
struct field_info;
/* Type used for delaying computation of method physnames.
See comments for compute_delayed_physnames. */
struct delayed_method_info
@@ -401,15 +399,6 @@ public:
.debug_str_offsets section (8 or 4, depending on the address size). */
std::optional<ULONGEST> str_offsets_base;
/* We may encounter a DIE where a property refers to a field in some
outer type. For example, in Ada, an array length may depend on a
field in some outer record. In this case, we need to be able to
stash a pointer to the 'struct field' into the appropriate
dynamic property baton. However, the fields aren't created until
the type has been fully processed, so we need a temporary
back-link to this object. */
struct field_info *field_info = nullptr;
/* Mark used when releasing cached dies. */
bool m_mark : 1;

View File

@@ -1747,7 +1747,7 @@ dwarf2_evaluate_property (const dynamic_prop *prop,
if (pinfo == NULL)
error (_("cannot find reference address for offset property"));
struct field resolved_field = *baton->field;
struct field resolved_field = baton->field;
resolve_dynamic_field (resolved_field, pinfo, initial_frame);
/* Storage for memory if we need to read it. */

View File

@@ -20,6 +20,7 @@
#ifndef GDB_DWARF2_LOC_H
#define GDB_DWARF2_LOC_H
#include "gdbtypes.h"
#include "dwarf2/expr.h"
struct symbol_computed_ops;
@@ -245,7 +246,7 @@ struct dwarf2_property_baton
struct dwarf2_loclist_baton loclist;
/* The location is stored in a field of PROPERTY_TYPE. */
struct field *field;
struct field field;
};
};

View File

@@ -698,12 +698,6 @@ struct field_info
we're reading. */
std::vector<variant_part_builder> variant_parts;
/* All the field batons that must be updated. This is only used
when a type's property depends on a field of this structure; for
example in Ada when an array's length may come from a field of an
enclosing record. */
gdb::unordered_map<dwarf2_property_baton *, sect_offset> field_batons;
/* Return the total number of fields (including baseclasses). */
int nfields () const
{
@@ -9985,6 +9979,29 @@ handle_member_location (struct die_info *die, struct dwarf2_cu *cu,
}
}
/* A helper that computes the location of a field. The CU and the
DW_TAG_member DIE are passed in. The results are stored in
*FP. */
static void
compute_field_location (dwarf2_cu *cu, die_info *die, field *fp)
{
/* Get type of field. */
fp->set_type (die_type (die, cu));
fp->set_loc_bitpos (0);
/* Get bit size of field (zero if none). */
attribute *attr = dwarf2_attr (die, DW_AT_bit_size, cu);
if (attr != nullptr)
fp->set_bitsize (attr->unsigned_constant ().value_or (0));
else
fp->set_bitsize (0);
/* Get bit offset of field. */
handle_member_location (die, cu, fp);
}
/* Add an aggregate field to the field list. */
static void
@@ -10040,20 +10057,7 @@ dwarf2_add_field (struct field_info *fip, struct die_info *die,
}
/* Data member other than a C++ static data member. */
/* Get type of field. */
fp->set_type (die_type (die, cu));
fp->set_loc_bitpos (0);
/* Get bit size of field (zero if none). */
attr = dwarf2_attr (die, DW_AT_bit_size, cu);
if (attr != nullptr)
fp->set_bitsize (attr->unsigned_constant ().value_or (0));
else
fp->set_bitsize (0);
/* Get bit offset of field. */
handle_member_location (die, cu, fp);
compute_field_location (cu, die, fp);
/* Get name of field. */
fieldname = dwarf2_name (die, cu);
@@ -11241,45 +11245,14 @@ handle_struct_member_die (struct die_info *child_die, struct type *type,
update_field_batons. If OFFSET is not found, NULL is returned. */
static dwarf2_property_baton *
find_field_create_baton (dwarf2_cu *cu, sect_offset offset)
find_field_create_baton (dwarf2_cu *cu, die_info *die)
{
field_info *fi = cu->field_info;
/* Defensive programming in case we see unusual DWARF. */
if (fi == nullptr)
return nullptr;
for (const auto &fld : fi->fields)
if (fld.offset == offset)
{
dwarf2_property_baton *result
= XOBNEW (&cu->per_objfile->objfile->objfile_obstack,
struct dwarf2_property_baton);
fi->field_batons[result] = offset;
return result;
}
return nullptr;
}
/* Update all the stored field property batons. FI is the field info
for the structure being created. TYPE is the corresponding struct
type with its fields already filled in. This fills in the correct
field for each baton that was stored while processing this
type. */
static void
update_field_batons (field_info *fi, struct type *type)
{
int n_bases = fi->baseclasses.size ();
for (auto &[baton, offset] : fi->field_batons)
{
for (int i = 0; i < fi->fields.size (); ++i)
{
if (fi->fields[i].offset == offset)
{
baton->field = &type->field (n_bases + i);
break;
}
}
}
dwarf2_property_baton *result
= XOBNEW (&cu->per_objfile->objfile->objfile_obstack,
struct dwarf2_property_baton);
memset (&result->field, 0, sizeof (result->field));
compute_field_location (cu, die, &result->field);
return result;
}
/* Finish creating a structure or union type, including filling in its
@@ -11303,9 +11276,6 @@ process_structure_scope (struct die_info *die, struct dwarf2_cu *cu)
struct field_info fi;
std::vector<struct symbol *> template_args;
scoped_restore save_field_info
= make_scoped_restore (&cu->field_info, &fi);
for (die_info *child_die : die->children ())
handle_struct_member_die (child_die, type, &fi, &template_args, cu);
@@ -11327,11 +11297,7 @@ process_structure_scope (struct die_info *die, struct dwarf2_cu *cu)
/* Attach fields and member functions to the type. */
if (fi.nfields () > 0)
{
dwarf2_attach_fields_to_type (&fi, type, cu);
update_field_batons (&fi, type);
}
dwarf2_attach_fields_to_type (&fi, type, cu);
if (!fi.fnfieldlists.empty ())
{
dwarf2_attach_fn_fields_to_type (&fi, type, cu);
@@ -13946,13 +13912,12 @@ attr_to_dynamic_prop (const struct attribute *attr, struct die_info *die,
case DW_AT_data_member_location:
case DW_AT_data_bit_offset:
{
baton = find_field_create_baton (cu, target_die->sect_off);
baton = find_field_create_baton (cu, target_die);
if (baton == nullptr)
return 0;
baton->property_type = read_type_die (target_die->parent,
target_cu);
baton->field = nullptr;
prop->set_field (baton);
break;
}

View File

@@ -0,0 +1,29 @@
/* 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/>. */
/* The data used for the structure. */
unsigned char our_data[] = { 3, 7, 11, 13 };
/* Dummy main function. */
int
main()
{
asm ("main_label: .globl main_label");
return 0;
}

View File

@@ -0,0 +1,89 @@
# 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 handling of an array type whose bound comes from the field of a
# structure.
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 -debug.S
# Set up the DWARF for the test.
set asm_file [standard_output_file $srcfile2]
Dwarf::assemble $asm_file {
global srcdir subdir srcfile
cu {} {
DW_TAG_compile_unit {
{DW_AT_language @DW_LANG_Ada95}
{DW_AT_name $srcfile}
} {
declare_labels byte array disc struct
byte: DW_TAG_base_type {
{DW_AT_byte_size 1 DW_FORM_sdata}
{DW_AT_encoding @DW_ATE_unsigned}
{DW_AT_name byte}
}
array: DW_TAG_array_type {
{DW_AT_name array_type}
{DW_AT_type :$byte}
} {
DW_TAG_subrange_type {
{DW_AT_type :$byte}
{DW_AT_upper_bound :$disc}
}
}
struct: DW_TAG_structure_type {
{DW_AT_name discriminated}
{DW_AT_byte_size 4 DW_FORM_sdata}
} {
disc: DW_TAG_member {
{DW_AT_name disc}
{DW_AT_type :$byte}
{DW_AT_data_member_location 0 DW_FORM_sdata}
}
DW_TAG_member {
{DW_AT_name nums}
{DW_AT_type :$array}
{DW_AT_data_member_location 1 DW_FORM_sdata}
}
}
DW_TAG_variable {
{DW_AT_name "value"}
{DW_AT_type :$struct}
{DW_AT_external 1 DW_FORM_flag}
{DW_AT_location {DW_OP_addr [gdb_target_symbol "our_data"]}
SPECIAL_expr}
}
}
}
}
if { [prepare_for_testing "failed to prepare" ${testfile} \
[list $srcfile $asm_file] {nodebug}] } {
return -1
}
gdb_test_no_output "set language ada"
gdb_test "print value" \
[string_to_regexp " = (disc => 3, nums => (7, 11, 13))"]