Re: [PATCH] Fix type of imported variable for arraydim.exp

[Tom de Vries <tdevries@suse.de>]

On 7/30/26 6:59 PM, Tom Tromey wrote:
> The test code for gdb.ada/arraydim.exp imports a variable using a
> dummy type.  Then the test tries to print the type of this variable.
> This works ok with GCC, because the import is emitted as a
> declaration; but this fails with gnat-llvm, where a definition is
> emitted.
> 
> This seems to be a test bug to me.  This patch fixes the problem by
> using the correct type here.

Hi Tom,

thanks for fixing this.

Do I understand it correctly that there are two (independent) issues:
- decl vs def: fixed in arraydim.exp
- dummy type vs correct type: fixed in arraydim/foo.adb
?

Anyway, the changes in arraydim.exp look fine to me.

Given that it's Ada, I can't really comment on the changes in 
arraydim/foo.adb.

I did wonder if the test-case still functioned as regression test after 
these changes, so I reverse-applied commit 8a48ac9579f ("wrong dimension 
found in ada-lang.c:ada_array_bound_from_type"):
...
diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 3c6c9af488f..0f28ce65239 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -3360,7 +3360,7 @@ ada_index_type (struct type *type, int n, const 
char *name)
  static LONGEST
  ada_array_bound_from_type (struct type *arr_type, int n, int which)
  {
-  struct type *type, *index_type_desc, *index_type;
+  struct type *type, *elt_type, *index_type_desc, *index_type;
    int i;

    gdb_assert (which == 0 || which == 1);
@@ -3376,6 +3376,10 @@ ada_array_bound_from_type (struct type *arr_type, 
int n, int which)
    else
      type = arr_type;

+  elt_type = type;
+  for (i = n; i > 1; i--)
+    elt_type = type->target_type ();
+
    if (type->is_fixed_instance ())
      {
        /* The array has already been fixed, so we do not need to
@@ -3393,14 +3397,7 @@ ada_array_bound_from_type (struct type *arr_type, 
int n, int which)
      index_type = to_fixed_range_type (index_type_desc->field (n - 
1).type (),
  				      NULL);
    else
-    {
-      struct type *elt_type = check_typedef (type);
-
-      for (i = 1; i < n; i++)
-	elt_type = check_typedef (elt_type->target_type ());
-
-      index_type = elt_type->index_type ();
-    }
+    index_type = elt_type->index_type ();

    return (which == 0
  	  ? ada_discrete_type_low_bound (index_type)
...
and managed to detect the regression with and without this patch.

Reviewed-By: Tom de Vries <tdevries@suse.de>

Thanks,
- Tom

> ---
>   gdb/testsuite/gdb.ada/arraydim.exp     | 7 ++++++-
>   gdb/testsuite/gdb.ada/arraydim/foo.adb | 7 ++-----
>   2 files changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/gdb/testsuite/gdb.ada/arraydim.exp b/gdb/testsuite/gdb.ada/arraydim.exp
> index 7b84a7af9b7..815a91ffa82 100644
> --- a/gdb/testsuite/gdb.ada/arraydim.exp
> +++ b/gdb/testsuite/gdb.ada/arraydim.exp
> @@ -52,8 +52,13 @@ gdb_test "print m'first(3)" " = 4"
>   gdb_test "print m'last(3)" " = 6"
>   gdb_test "print m'length(3)" " = 3"
>   
> +# With GCC the test shows "int" as the element type, but with
> +# gnat-llvm it shows "integer"; both of these are reasonable enough so
> +# we accept either.  The difference here is because GCC emits a
> +# declaration in the DWARF for foo.o, but gnat-llvm emits a
> +# definition.
>   gdb_test "ptype global_3dim_for_gdb_testing" \
> -    "array \\(0 \\.\\. 0, 0 \\.\\. 1, 0 \\.\\. 2\\) of int"
> +    [quotemeta "array (0 .. 0, 0 .. 1, 0 .. 2) of @/(int|integer)/"]
>   
>   gdb_test "print global_3dim_for_gdb_testing'first" " = 0"
>   gdb_test "print global_3dim_for_gdb_testing'last" " = 0"
> diff --git a/gdb/testsuite/gdb.ada/arraydim/foo.adb b/gdb/testsuite/gdb.ada/arraydim/foo.adb
> index 86204956d00..8da63e1af93 100644
> --- a/gdb/testsuite/gdb.ada/arraydim/foo.adb
> +++ b/gdb/testsuite/gdb.ada/arraydim/foo.adb
> @@ -18,11 +18,8 @@ procedure Foo is
>      type Multi is array (1 .. 1, 2 .. 3, 4 .. 6) of Integer;
>      M : Multi := (others => (others => (others => 0)));
>   
> -   --  Use a fake type for importing our C multi-dimensional array.
> -   --  It's only to make sure the C unit gets linked in, regardless
> -   --  of possible optimizations.
> -   type Void_Star is access integer;
> -   E : Void_Star;
> +   type C_Multi is array (0 .. 0, 0 .. 1, 0 .. 2) of Integer;
> +   E : C_Multi;
>      pragma Import (C, E, "global_3dim_for_gdb_testing");
>   begin
>      Do_Nothing (M'Address);  -- STOP
> 
> base-commit: 6d1be0b90e837e4c82eaaf6f9e8c7da7227902e1