Fix regression with dynamic array bounds

Kévin discovered that commit ba005d3 ("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 <[email protected]>

Author: tromey

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 @@ struct dwarf2_cu
      .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;
 

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.  */

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;
   };
 };
 

gdb/dwarf2/read.c

@@ -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;
 	    }

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 <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;
+}

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 <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))"]