[v3,2/4] gdb, types: Resolve pointer types dynamically

  From: "Ijaz, Abdul B" <abdul.b.ijaz@intel.com>

This commit allows pointers to be dynamic types (on the outmost
level).  Similar to references, a pointer is considered a dynamic type
if its target type is a dynamic type and it is on the outmost level.

The pointer resolution follows the one of references.

This generally makes the GDB output more verbose.  We are able to print
more details about a pointer's target like the dimension of an array.

In Fortran, if we have a pointer to a dynamic type

  type buffer
    real, dimension(:), pointer :: ptr
  end type buffer
  type(buffer), pointer :: buffer_ptr
  allocate (buffer_ptr)
  allocate (buffer_ptr%ptr (5))

which then gets allocated, we now resolve the dynamic type before
printing the pointer's type:

Before:

  (gdb) ptype buffer_ptr
  type = PTR TO -> ( Type buffer
    real(kind=4) :: alpha(:)
  End Type buffer )

After:

  (gdb) ptype buffer_ptr
  type = PTR TO -> ( Type buffer
    real(kind=4) :: alpha(5)
  End Type buffer )

Similarly in C++ we can dynamically resolve e.g. pointers to arrays:

  int len = 3;
  int arr[len];
  int (*ptr)[len];
  int ptr = &arr;

Once the pointer is assigned one gets:

Before:

  (gdb) p ptr
  $1 = (int (*)[variable length]) 0x123456
  (gdb) ptype ptr
  type = int (*)[variable length]

After:

  (gdb) p ptr
  $1 = (int (*)[3]) 0x123456
  (gdb) ptype ptr
  type = int (*)[3]

For more examples see the modified/added test cases.
---
 gdb/gdbtypes.c                                |   7 +-
 gdb/testsuite/gdb.cp/vla-cxx.cc               |   4 +
 gdb/testsuite/gdb.cp/vla-cxx.exp              |  15 +++
 gdb/testsuite/gdb.dwarf2/dynarr-ptr.exp       |  16 +--
 .../gdb.fortran/pointer-to-pointer.exp        |   2 +-
 gdb/testsuite/gdb.fortran/pointers.exp        | 115 ++++++++++++++++++
 gdb/testsuite/gdb.fortran/pointers.f90        |  29 +++++
 gdb/valprint.c                                |   6 -
 8 files changed, 177 insertions(+), 17 deletions(-)
 create mode 100644 gdb/testsuite/gdb.fortran/pointers.exp
  

Hello,

I've been working a bit with dynamic types lately, but I'm not confident
enough to provide a Reviewed-by.

In any case, some small nits. Everything else LGTM FWIW.

Abdul Basit Ijaz via Gdb-patches <gdb-patches@sourceware.org> writes:

> diff --git a/gdb/testsuite/gdb.dwarf2/dynarr-ptr.exp b/gdb/testsuite/gdb.dwarf2/dynarr-ptr.exp
> index 6e4a331eca8..d6b20086c89 100644
> --- a/gdb/testsuite/gdb.dwarf2/dynarr-ptr.exp
> +++ b/gdb/testsuite/gdb.dwarf2/dynarr-ptr.exp
> @@ -154,7 +154,7 @@ gdb_test "print foo.three_ptr.all'length" \
>           " = 3"
>  
>  gdb_test "ptype foo.three_ptr.all" \
> -         " = array \\(<>\\) of integer"
> +    " = array \\(1 \\.\\. 3\\) of integer"

The new indentation level is inconsistent with the rest of the file.
Please preserve the original indentation level.

>  
>  # foo.three_ptr
>  
> @@ -177,7 +177,7 @@ gdb_test "print foo.three_ptr'length" \
>           " = 3"
>  
>  gdb_test "ptype foo.three_ptr" \
> -         " = access array \\(<>\\) of integer"
> +    " = access array \\(1 \\.\\. 3\\) of integer"

Ditto for the other changes in this file.

> diff --git a/gdb/testsuite/gdb.fortran/pointers.exp b/gdb/testsuite/gdb.fortran/pointers.exp
> new file mode 100644
> index 00000000000..ca2195bbfe6
> --- /dev/null
> +++ b/gdb/testsuite/gdb.fortran/pointers.exp
> @@ -0,0 +1,115 @@
> +# Copyright 2016-2023 Free Software Foundation, Inc.

This is a new file. The copyright starts in 2023.

> +
> +# 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/>.
  

>>>>> "Thiago" == Thiago Jung Bauermann via Gdb-patches <gdb-patches@sourceware.org> writes:

>> gdb_test "ptype foo.three_ptr.all" \
>> -         " = array \\(<>\\) of integer"
>> +    " = array \\(1 \\.\\. 3\\) of integer"

Thiago> The new indentation level is inconsistent with the rest of the file.
Thiago> Please preserve the original indentation level.

I think gdb standardized on 4 space indent for Tcl, but some files are
still wrong here.  However normally I think we'd want reindentations to
be a separate patch.  Fine to do that as a prelude if it makes your task
easier.

Tom
  

>>>>> Abdul Basit Ijaz via Gdb-patches <gdb-patches@sourceware.org> writes:

> -  /* We only want to recognize references at the outermost level.  */
> -  if (top_level && type->code () == TYPE_CODE_REF)
> +  /* We only want to recognize references and pointers at the outermost
> +     level.  */
> +  if (top_level
> +      && (type->code () == TYPE_CODE_REF || type->code () == TYPE_CODE_PTR))

Pre-existing but I wonder why this code checks TYPE_CODE_REF and not
TYPE_CODE_RVALUE_REF.

> diff --git a/gdb/valprint.c b/gdb/valprint.c
> index b65dda15c04..c71ae089f46 100644
> --- a/gdb/valprint.c
> +++ b/gdb/valprint.c
> @@ -1156,12 +1156,6 @@ value_check_printable (struct value *val, struct ui_file *stream,
>        return 0;
>      }
 
> -  if (type_not_associated (val->type ()))
> -    {
> -      val_print_not_associated (stream);
> -      return 0;
> -    }

I don't really know anything about Fortran, so I don't know why this
code was here in the first place, nor what its removal might mean.
Could you say why this is being removed?

Tom
  

Hi Tom and Thiago,

Thanks a lot for the feedback.

>> gdb_test "ptype foo.three_ptr.all" \
>> -         " = array \\(<>\\) of integer"
>> +    " = array \\(1 \\.\\. 3\\) of integer"

Thiago> The new indentation level is inconsistent with the rest of the file.
Thiago> Please preserve the original indentation level.

Tom > I think gdb standardized on 4 space indent for Tcl, but some files are still wrong here.  However normally I think we'd want reindentations to be a separate patch.  Fine to do that as a prelude if it makes your task easier.

I will add an extra patch to this series for fixing existing indentation problem in test gdb.dwarf2/dynarr-ptr.exp and then add changes to test on top of it.


Thanks & Best Regards
Abdul Basit

-----Original Message-----
From: Tom Tromey <tom@tromey.com> 
Sent: Tuesday, October 10, 2023 9:46 PM
To: Thiago Jung Bauermann via Gdb-patches <gdb-patches@sourceware.org>
Cc: Ijaz, Abdul B <abdul.b.ijaz@intel.com>; Thiago Jung Bauermann <thiago.bauermann@linaro.org>; simark@simark.ca; tom@tromey.com
Subject: Re: [PATCH v3 2/4] gdb, types: Resolve pointer types dynamically

>>>>> "Thiago" == Thiago Jung Bauermann via Gdb-patches <gdb-patches@sourceware.org> writes:

>> gdb_test "ptype foo.three_ptr.all" \
>> -         " = array \\(<>\\) of integer"
>> +    " = array \\(1 \\.\\. 3\\) of integer"

Thiago> The new indentation level is inconsistent with the rest of the file.
Thiago> Please preserve the original indentation level.

I think gdb standardized on 4 space indent for Tcl, but some files are still wrong here.  However normally I think we'd want reindentations to be a separate patch.  Fine to do that as a prelude if it makes your task easier.

Tom
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
  

Hi Thiago,

Thanks for the feedback.

> The new indentation level is inconsistent with the rest of the file.
> Please preserve the original indentation level.

Replied in other email also that will add an extra patch in this series to fix indentation in this test.

> This is a new file. The copyright starts in 2023.

Will update it to 2024 in v4 series.

Best Regards,
Abdul Basit

-----Original Message-----
From: Thiago Jung Bauermann <thiago.bauermann@linaro.org> 
Sent: Tuesday, October 3, 2023 2:08 AM
To: Ijaz, Abdul B <abdul.b.ijaz@intel.com>
Cc: simark@simark.ca; tom@tromey.com; gdb-patches@sourceware.org
Subject: Re: [PATCH v3 2/4] gdb, types: Resolve pointer types dynamically


Hello,

I've been working a bit with dynamic types lately, but I'm not confident enough to provide a Reviewed-by.

In any case, some small nits. Everything else LGTM FWIW.

Abdul Basit Ijaz via Gdb-patches <gdb-patches@sourceware.org> writes:

> diff --git a/gdb/testsuite/gdb.dwarf2/dynarr-ptr.exp 
> b/gdb/testsuite/gdb.dwarf2/dynarr-ptr.exp
> index 6e4a331eca8..d6b20086c89 100644
> --- a/gdb/testsuite/gdb.dwarf2/dynarr-ptr.exp
> +++ b/gdb/testsuite/gdb.dwarf2/dynarr-ptr.exp
> @@ -154,7 +154,7 @@ gdb_test "print foo.three_ptr.all'length" \
>           " = 3"
>  
>  gdb_test "ptype foo.three_ptr.all" \
> -         " = array \\(<>\\) of integer"
> +    " = array \\(1 \\.\\. 3\\) of integer"

The new indentation level is inconsistent with the rest of the file.
Please preserve the original indentation level.

>  
>  # foo.three_ptr
>  
> @@ -177,7 +177,7 @@ gdb_test "print foo.three_ptr'length" \
>           " = 3"
>  
>  gdb_test "ptype foo.three_ptr" \
> -         " = access array \\(<>\\) of integer"
> +    " = access array \\(1 \\.\\. 3\\) of integer"

Ditto for the other changes in this file.

> diff --git a/gdb/testsuite/gdb.fortran/pointers.exp 
> b/gdb/testsuite/gdb.fortran/pointers.exp
> new file mode 100644
> index 00000000000..ca2195bbfe6
> --- /dev/null
> +++ b/gdb/testsuite/gdb.fortran/pointers.exp
> @@ -0,0 +1,115 @@
> +# Copyright 2016-2023 Free Software Foundation, Inc.

This is a new file. The copyright starts in 2023.

> +
> +# 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/>.

--
Thiago
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
  

Hi Tom,

Thanks a lot for the feedback.

>> -  /* We only want to recognize references at the outermost level.  */
>> -  if (top_level && type->code () == TYPE_CODE_REF)
>> +  /* We only want to recognize references and pointers at the outermost
>> +     level.  */
>> +  if (top_level
>> +      && (type->code () == TYPE_CODE_REF || type->code () == 
>> + TYPE_CODE_PTR))

Tom > Pre-existing but I wonder why this code checks TYPE_CODE_REF and not TYPE_CODE_RVALUE_REF.

Here we are checking for LVALUE or Pointers dynamic value and for them need to resolve the target memory to resolve pointer/memory at outmost level.  So as far as I understood checking for rvalue here would not be needed.  In case you have some case to try then please let me know I can give it a try.

>> -  if (type_not_associated (val->type ()))
>> -    {
>> -      val_print_not_associated (stream);
>> -      return 0;
>> -    }

Tom > I don't really know anything about Fortran, so I don't know why this code was here in the first place, nor what its removal might mean.

This was originally added at time to support DW_AT_Associated/Allocated DIEs for Fortran dynamic arrays but this is a redundant code so no need to keep it. Also in existing testsuite there is no affect with or without it.  Will mention in the commit message in V4 series.

Best Regards,
Abdul Basit

-----Original Message-----
From: Tom Tromey <tom@tromey.com> 
Sent: Tuesday, October 10, 2023 9:50 PM
To: Abdul Basit Ijaz via Gdb-patches <gdb-patches@sourceware.org>
Cc: Ijaz, Abdul B <abdul.b.ijaz@intel.com>; simark@simark.ca; tom@tromey.com
Subject: Re: [PATCH v3 2/4] gdb, types: Resolve pointer types dynamically

>>>>> Abdul Basit Ijaz via Gdb-patches <gdb-patches@sourceware.org> writes:

> -  /* We only want to recognize references at the outermost level.  */
> -  if (top_level && type->code () == TYPE_CODE_REF)
> +  /* We only want to recognize references and pointers at the outermost
> +     level.  */
> +  if (top_level
> +      && (type->code () == TYPE_CODE_REF || type->code () == 
> + TYPE_CODE_PTR))

Pre-existing but I wonder why this code checks TYPE_CODE_REF and not TYPE_CODE_RVALUE_REF.

> diff --git a/gdb/valprint.c b/gdb/valprint.c index 
> b65dda15c04..c71ae089f46 100644
> --- a/gdb/valprint.c
> +++ b/gdb/valprint.c
> @@ -1156,12 +1156,6 @@ value_check_printable (struct value *val, struct ui_file *stream,
>        return 0;
>      }
 
> -  if (type_not_associated (val->type ()))
> -    {
> -      val_print_not_associated (stream);
> -      return 0;
> -    }

I don't really know anything about Fortran, so I don't know why this code was here in the first place, nor what its removal might mean.
Could you say why this is being removed?

Tom
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928