gdb/fortran: Handle dynamic string types when printing types

Message ID 20200714100351.1140497-1-andrew.burgess@embecosm.com
State New
Headers show
Series
  • gdb/fortran: Handle dynamic string types when printing types
Related show

Commit Message

Andrew Burgess July 14, 2020, 10:03 a.m.
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(-)

-- 
2.25.4

Comments

Hannes Domani via Gdb-patches July 15, 2020, 2:10 a.m. | #1
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
Andrew Burgess July 15, 2020, 7:52 a.m. | #2
* 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
Tom de Vries July 15, 2020, 1:03 p.m. | #3
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
Andrew Burgess July 15, 2020, 3:11 p.m. | #4
* 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

Patch

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