[PATCH] gdb/fortran: Handle dynamic string types when printing types

[PATCH] gdb/fortran: Handle dynamic string types when printing types

[Andrew Burgess]

After commit:

  commit 8c2e4e0689ea244d0ed979171a3d09c9176b8175
  Date:   Sun Jul 12 22:58:51 2020 -0400

      gdb: add accessors to struct dynamic_prop

An existing bug was exposed in the Fortran type printing code.  When
GDB is asked to print the type of a function that takes a dynamic
string argument GDB will try to read the upper bound of the string.

The read of the upper bound is written as:

    if (type->bounds ()->high.kind () == PROP_UNDEFINED)
      // Treat the upper bound as unknown.
    else
      // Treat the upper bound as known and constant.

However, this is not good enough.  When printing a function type the
dynamic argument types will not have been resolved.  As a result the
dynamic property is not PROP_UNDEFINED, but nor is it constant.

By rewriting this code to specifically check for the PROP_CONST case,
and treating all other cases as the upper bound being unknown we avoid
incorrectly treating the dynamic property as being constant.

gdb/ChangeLog:

	* f-typeprint.c (f_type_print_base): Allow for dynamic types not
	being resolved.

gdb/testsuite/ChangeLog:

	* gdb.fortran/ptype-on-functions.exp: Add more tests.
	* gdb.fortran/ptype-on-functions.f90: Likewise.
---
 gdb/ChangeLog                                 |  5 ++++
 gdb/f-typeprint.c                             | 12 ++++++----
 gdb/testsuite/ChangeLog                       |  5 ++++
 .../gdb.fortran/ptype-on-functions.exp        |  6 +++++
 .../gdb.fortran/ptype-on-functions.f90        | 23 +++++++++++++++++++
 5 files changed, 47 insertions(+), 4 deletions(-)

diff --git a/gdb/f-typeprint.c b/gdb/f-typeprint.c
index 80dbfe11167..65ec93af9f4 100644
--- a/gdb/f-typeprint.c
+++ b/gdb/f-typeprint.c
@@ -406,16 +406,20 @@ f_type_print_base (struct type *type, struct ui_file *stream, int show,
       break;
 
     case TYPE_CODE_STRING:
-      /* Strings may have dynamic upperbounds (lengths) like arrays.  */
+      /* Strings may have dynamic upperbounds (lengths) like arrays.  We
+	 check specifically for the PROP_CONST case to indicate that the
+	 dynamic type has been resolved.  If we arrive here having been
+	 asked to print the type of a value with a dynamic type then the
+	 bounds will not have been resolved.  */
 
-      if (type->bounds ()->high.kind () == PROP_UNDEFINED)
-	fprintfi_filtered (level, stream, "character*(*)");
-      else
+      if (type->bounds ()->high.kind () == PROP_CONST)
 	{
 	  LONGEST upper_bound = f77_get_upperbound (type);
 
 	  fprintf_filtered (stream, "character*%s", pulongest (upper_bound));
 	}
+      else
+	fprintfi_filtered (level, stream, "character*(*)");
       break;
 
     case TYPE_CODE_STRUCT:
diff --git a/gdb/testsuite/gdb.fortran/ptype-on-functions.exp b/gdb/testsuite/gdb.fortran/ptype-on-functions.exp
index 8dc5f76d93a..dde6f48b853 100644
--- a/gdb/testsuite/gdb.fortran/ptype-on-functions.exp
+++ b/gdb/testsuite/gdb.fortran/ptype-on-functions.exp
@@ -43,3 +43,9 @@ gdb_test "ptype say_numbers" \
 
 gdb_test "ptype fun_ptr" \
     "type = PTR TO -> \\( integer\\(kind=4\\) \\(\\) \\(REF TO -> \\( integer\\(kind=4\\) \\)\\) \\)"
+
+gdb_test "ptype say_string" \
+    "type = void \\(character\\*\\(\\*\\), integer\\(kind=8\\)\\)"
+
+gdb_test "ptype say_array" \
+    "type = void \\(integer\\(kind=4\\) \\(:,:\\)\\)"
diff --git a/gdb/testsuite/gdb.fortran/ptype-on-functions.f90 b/gdb/testsuite/gdb.fortran/ptype-on-functions.f90
index a43dd3820d8..adf79e32591 100644
--- a/gdb/testsuite/gdb.fortran/ptype-on-functions.f90
+++ b/gdb/testsuite/gdb.fortran/ptype-on-functions.f90
@@ -63,6 +63,10 @@ program test
      integer function fun2 (x)
        integer :: x
      end function fun2
+
+     subroutine say_array (arr)
+       integer, dimension (:,:) :: arr
+     end subroutine say_array
   end interface
 
   type (Number) :: n1
@@ -70,7 +74,12 @@ program test
 
   procedure(fun1), pointer:: fun_ptr => NULL()
 
+  integer, dimension (5,5) :: array
+  array = 0
+
   call say_numbers (1,2,3)	! stop here
+  call say_string ('hello world')
+  call say_array (array (2:3, 2:4))
   print *, fun_ptr (3)
 
 end program test
@@ -87,3 +96,17 @@ integer function fun2 (x)
   fun2 = x + 2
 end function fun2
 
+subroutine say_string (str)
+  character(len=*) :: str
+  print *, str
+end subroutine say_string
+
+subroutine say_array (arr)
+  integer, dimension (:,:) :: arr
+  do i=LBOUND (arr, 2), UBOUND (arr, 2), 1
+     do j=LBOUND (arr, 1), UBOUND (arr, 1), 1
+        write(*, fmt="(i4)", advance="no") arr (j, i)
+     end do
+     print *, ""
+  end do
+end subroutine say_array
-- 
2.25.4

Re: [PATCH] gdb/fortran: Handle dynamic string types when printing types

[Simon Marchi]

On 2020-07-14 6:03 a.m., Andrew Burgess wrote:
> After commit:
> 
>   commit 8c2e4e0689ea244d0ed979171a3d09c9176b8175
>   Date:   Sun Jul 12 22:58:51 2020 -0400
> 
>       gdb: add accessors to struct dynamic_prop
> 
> An existing bug was exposed in the Fortran type printing code.  When
> GDB is asked to print the type of a function that takes a dynamic
> string argument GDB will try to read the upper bound of the string.
> 
> The read of the upper bound is written as:
> 
>     if (type->bounds ()->high.kind () == PROP_UNDEFINED)
>       // Treat the upper bound as unknown.
>     else
>       // Treat the upper bound as known and constant.
> 
> However, this is not good enough.  When printing a function type the
> dynamic argument types will not have been resolved.  As a result the
> dynamic property is not PROP_UNDEFINED, but nor is it constant.
> 
> By rewriting this code to specifically check for the PROP_CONST case,
> and treating all other cases as the upper bound being unknown we avoid
> incorrectly treating the dynamic property as being constant.
> 
> gdb/ChangeLog:
> 
> 	* f-typeprint.c (f_type_print_base): Allow for dynamic types not
> 	being resolved.
> 
> gdb/testsuite/ChangeLog:
> 
> 	* gdb.fortran/ptype-on-functions.exp: Add more tests.
> 	* gdb.fortran/ptype-on-functions.f90: Likewise.

Thanks, FWIW this LGTM.

Just wondering, if there were no existing test that caught this, how did you
find it?  In an external testsuite?

Simon

Re: [PATCH] gdb/fortran: Handle dynamic string types when printing types

[Andrew Burgess]

* Simon Marchi <simon.marchi@polymtl.ca> [2020-07-14 22:10:58 -0400]:

> On 2020-07-14 6:03 a.m., Andrew Burgess wrote:
> > After commit:
> > 
> >   commit 8c2e4e0689ea244d0ed979171a3d09c9176b8175
> >   Date:   Sun Jul 12 22:58:51 2020 -0400
> > 
> >       gdb: add accessors to struct dynamic_prop
> > 
> > An existing bug was exposed in the Fortran type printing code.  When
> > GDB is asked to print the type of a function that takes a dynamic
> > string argument GDB will try to read the upper bound of the string.
> > 
> > The read of the upper bound is written as:
> > 
> >     if (type->bounds ()->high.kind () == PROP_UNDEFINED)
> >       // Treat the upper bound as unknown.
> >     else
> >       // Treat the upper bound as known and constant.
> > 
> > However, this is not good enough.  When printing a function type the
> > dynamic argument types will not have been resolved.  As a result the
> > dynamic property is not PROP_UNDEFINED, but nor is it constant.
> > 
> > By rewriting this code to specifically check for the PROP_CONST case,
> > and treating all other cases as the upper bound being unknown we avoid
> > incorrectly treating the dynamic property as being constant.
> > 
> > gdb/ChangeLog:
> > 
> > 	* f-typeprint.c (f_type_print_base): Allow for dynamic types not
> > 	being resolved.
> > 
> > gdb/testsuite/ChangeLog:
> > 
> > 	* gdb.fortran/ptype-on-functions.exp: Add more tests.
> > 	* gdb.fortran/ptype-on-functions.f90: Likewise.
> 
> Thanks, FWIW this LGTM.

Thanks.

> 
> Just wondering, if there were no existing test that caught this, how did you
> find it?  In an external testsuite?

No, I was writing tests for another patch that's still WIP.  Honestly
I was surprised that this wasn't already covered.

Thanks,
Andrew

Re: [PATCH] gdb/fortran: Handle dynamic string types when printing types

[Tom de Vries]

On 7/14/20 12:03 PM, Andrew Burgess wrote:
> +gdb_test "ptype say_string" \
> +    "type = void \\(character\\*\\(\\*\\), integer\\(kind=8\\)\\)"

With gcc 7.5.0, I run into:
...
(gdb) ptype say_string^M
type = void (character*(*), integer(kind=4))^M
(gdb) FAIL: gdb.fortran/ptype-on-functions.exp: ptype say_string
...
but it passes for me with gcc 8.4.0.

This looks related to this ( https://gcc.gnu.org/gcc-8/changes.html ):
...
Character variables longer than HUGE(0) elements are now possible on
64-bit targets. Note that this changes the procedure call ABI for all
procedures with character arguments on 64-bit targets, as the type of
the hidden character length argument has changed. The hidden character
length argument is now of type INTEGER(C_SIZE_T).
...

Thanks,
- Tom

Re: [PATCH] gdb/fortran: Handle dynamic string types when printing types

[Andrew Burgess]

* Tom de Vries <tdevries@suse.de> [2020-07-15 15:03:29 +0200]:

> On 7/14/20 12:03 PM, Andrew Burgess wrote:
> > +gdb_test "ptype say_string" \
> > +    "type = void \\(character\\*\\(\\*\\), integer\\(kind=8\\)\\)"
> 
> With gcc 7.5.0, I run into:
> ...
> (gdb) ptype say_string^M
> type = void (character*(*), integer(kind=4))^M
> (gdb) FAIL: gdb.fortran/ptype-on-functions.exp: ptype say_string
> ...
> but it passes for me with gcc 8.4.0.
> 
> This looks related to this ( https://gcc.gnu.org/gcc-8/changes.html ):
> ...
> Character variables longer than HUGE(0) elements are now possible on
> 64-bit targets. Note that this changes the procedure call ABI for all
> procedures with character arguments on 64-bit targets, as the type of
> the hidden character length argument has changed. The hidden character
> length argument is now of type INTEGER(C_SIZE_T).
> ...

Thanks for bringing this to my attention.  I've committed the patch
below to resolve this failure.

Thanks,
Andrew

---

From 7d44f7715f23f73243f686a37c123a9c039faf78 Mon Sep 17 00:00:00 2001
From: Andrew Burgess <andrew.burgess@embecosm.com>
Date: Wed, 15 Jul 2020 16:05:24 +0100
Subject: [PATCH] gdb/testsuite: Update test pattern in ptype-on-functions.exp

It was pointed out that the recently added test
gdb.fortran/ptype-on-functions.exp fails on older versions of
gfortran.  This is because the ABI for passing string lengths changed
from a 4-byte to 8-byte value (on some targets).

This change is documented here:
  https://gcc.gnu.org/gcc-8/changes.html.

  Character variables longer than HUGE(0) elements are now possible on
  64-bit targets. Note that this changes the procedure call ABI for
  all procedures with character arguments on 64-bit targets, as the
  type of the hidden character length argument has changed. The hidden
  character length argument is now of type INTEGER(C_SIZE_T).

This commit just relaxes the pattern to accept any size of integer for
the string length argument.

gdb/testsuite/ChangeLog:

	* gdb.fortran/ptype-on-functions.exp: Make the result pattern more
	generic.
---
 gdb/testsuite/ChangeLog                          | 5 +++++
 gdb/testsuite/gdb.fortran/ptype-on-functions.exp | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/gdb/testsuite/gdb.fortran/ptype-on-functions.exp b/gdb/testsuite/gdb.fortran/ptype-on-functions.exp
index dde6f48b853..9d447530c26 100644
--- a/gdb/testsuite/gdb.fortran/ptype-on-functions.exp
+++ b/gdb/testsuite/gdb.fortran/ptype-on-functions.exp
@@ -45,7 +45,7 @@ gdb_test "ptype fun_ptr" \
     "type = PTR TO -> \\( integer\\(kind=4\\) \\(\\) \\(REF TO -> \\( integer\\(kind=4\\) \\)\\) \\)"
 
 gdb_test "ptype say_string" \
-    "type = void \\(character\\*\\(\\*\\), integer\\(kind=8\\)\\)"
+    "type = void \\(character\\*\\(\\*\\), integer\\(kind=\\d+\\)\\)"
 
 gdb_test "ptype say_array" \
     "type = void \\(integer\\(kind=4\\) \\(:,:\\)\\)"
-- 
2.25.4