From: Tom Tromey Date: Tue, 13 May 2025 19:41:26 +0000 (-0600) Subject: Fix regression with dynamic array bounds X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=9b026264097aa5f328a2d23f985fde961b41ef2e;p=thirdparty%2Fbinutils-gdb.git Fix regression with dynamic array bounds Kévin discovered that commit ba005d32b0f ("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 --- diff --git a/gdb/dwarf2/cu.h b/gdb/dwarf2/cu.h index 9f7678936a7..5338dfe4b45 100644 --- a/gdb/dwarf2/cu.h +++ b/gdb/dwarf2/cu.h @@ -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 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; diff --git a/gdb/dwarf2/loc.c b/gdb/dwarf2/loc.c index fa1b4eb3123..dee6127b02d 100644 --- a/gdb/dwarf2/loc.c +++ b/gdb/dwarf2/loc.c @@ -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. */ diff --git a/gdb/dwarf2/loc.h b/gdb/dwarf2/loc.h index ebe5c31fc5a..ee88fd4db69 100644 --- a/gdb/dwarf2/loc.h +++ b/gdb/dwarf2/loc.h @@ -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; }; }; diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c index 75e13159c64..7de32bc7766 100644 --- a/gdb/dwarf2/read.c +++ b/gdb/dwarf2/read.c @@ -698,12 +698,6 @@ struct field_info we're reading. */ std::vector 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 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 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; } diff --git a/gdb/testsuite/gdb.dwarf2/ada-array-bound.c b/gdb/testsuite/gdb.dwarf2/ada-array-bound.c new file mode 100644 index 00000000000..5a7d397d1c8 --- /dev/null +++ b/gdb/testsuite/gdb.dwarf2/ada-array-bound.c @@ -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 . */ + +/* 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; +} diff --git a/gdb/testsuite/gdb.dwarf2/ada-array-bound.exp b/gdb/testsuite/gdb.dwarf2/ada-array-bound.exp new file mode 100644 index 00000000000..f48df7baf1f --- /dev/null +++ b/gdb/testsuite/gdb.dwarf2/ada-array-bound.exp @@ -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 . + +# 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))"]