RE: GDB/Fortran: Support for Assumed Rank Zero.

["Potharla, Rupesh" <Rupesh.Potharla@amd.com>]

[-- Attachment #1: Type: text/plain, Size: 8114 bytes --]

[AMD Official Use Only]

Thanks Kevin,

Apologies for missing out on these minor things. Followed GDB standard for comments and updated the attached patch. Regarding XFAIL for flang, I looked at all possible cases in the testcase where this can be handled and came up this solution which will not suppress the actual failure case. As the issue will be fixed by the compiler soon, added the condition based on test number.  


Manual testing
--------------------
./flang -gdwarf-5 assumedrank.f90 -o assumedrank

(gdb) br test_rank
Breakpoint 1 at 0x2024c1: file assumedrank.f90, line 46.
(gdb) r
Starting program: /home/rupesh/rank/binutils-gdb/gdb/assumedrank
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".

Breakpoint 1, arank::test_rank (answer=...) at assumedrank.f90:46
46          print *, RANK(answer)     ! Test Breakpoint
(gdb) p rank(answer)
$2 = 2
(gdb) p answer
$3 = ()
(gdb) ptype answer
type = real (:,28827820490920:20)
(gdb) f 1
#1  0x0000000000202380 in arank () at assumedrank.f90:34
34        call test_rank (array0)
(gdb) p rank(array0)
$4 = 0

GDB testrun
------------------
make check RUNTESTFLAGS=' CFLAGS_FOR_TARGET='-gdwarf-5' CC_FOR_TARGET="/home/rupesh/STAGING-BUILD-2787/bin/clang" F90_FOR_TARGET="/home/rupesh/STAGING-BUILD-2787/bin/flang -Wno-unused-command-line-argument" ' TESTS="gdb.fortran/assumedrank.exp"


Testcase behavior without XFAIL for FLANG 
-----------------------------------------------------------------------
Breakpoint 2, arank::test_rank (answer=<error reading variable: value requires 3516574024176 bytes, which is more than max-value-size>) at /home/rupesh/rank/binutils-gdb/gdb/testsuite/gdb.fortran/assumedrank.f90:46^M
46          print *, RANK(answer)     ! Test Breakpoint^M
(gdb) print rank(answer)^M
$1 = 2^M
(gdb) PASS: gdb.fortran/assumedrank.exp: test 0: get valueof "rank(answer)"
print answer^M
value requires 3516574024176 bytes, which is more than max-value-size^M
(gdb) FAIL: gdb.fortran/assumedrank.exp: test 0: get valueof "answer"
up^M
#1  0x0000000000202410 in arank () at /home/rupesh/rank/binutils-gdb/gdb/testsuite/gdb.fortran/assumedrank.f90:34^M
34        call test_rank (array0)^M
(gdb) PASS: gdb.fortran/assumedrank.exp: test 0: found the name of a test array to check against
p rank(array0)^M
$2 = 0^M
(gdb) FAIL: gdb.fortran/assumedrank.exp: test 0: p rank(array0)
print array0^M
$3 = 0^M
(gdb) PASS: gdb.fortran/assumedrank.exp: test 0: get valueof "array0"
FAIL: gdb.fortran/assumedrank.exp: test 0: answer array contains the expected contents
continue^M
Continuing.^M


Regards,
Rupesh P


> -----Original Message-----
> From: Kevin Buettner <kevinb@redhat.com>
> Sent: Friday, April 15, 2022 2:59 AM
> To: Potharla, Rupesh <Rupesh.Potharla@amd.com>
> Cc: gdb-patches@sourceware.org; George, Jini Susan
> <JiniSusan.George@amd.com>; Parasuraman, Hariharan
> <Hariharan.Parasuraman@amd.com>; Sharma, Alok Kumar
> <AlokKumar.Sharma@amd.com>
> Subject: Re: GDB/Fortran: Support for Assumed Rank Zero.
> 
> [CAUTION: External Email]
> 
> Hi Rupesh,
> 
> After reading the comments more closely, I found a few more nits; see
> below.  I apologize for missing these on the first pass.
> 
> I also have a question about how you're testing with clang / flang.
> 
> Kevin
> 
> On Thu, 14 Apr 2022 10:30:38 +0000
> "Potharla, Rupesh" <Rupesh.Potharla@amd.com> wrote:
> 
> > If a variable is passed to function in FORTRAN as an argument the
> > variable is treated as an array with rank zero. GDB currently does not
> > support the case for assumed rank 0. This patch provides support for
> > assumed rank 0 and updates the testcase as well.
> 
> Please put two spaces after the period in each sentence.  See:
> 
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww
> .gnu.org%2Fprep%2Fstandards%2Fstandards.html%23Comments&amp;data
> =04%7C01%7Crupesh.potharla%40amd.com%7C0702c595b4bd4a5c196e08d
> a1e5dc536%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C6378556
> 85341831678%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQ
> IjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=ZExuLy
> keRrU7AyRd%2FAMW3FAk3RPLdsBa2GfPz%2FoMLWk%3D&amp;reserved=0
> 
> > Without patch:
> > Breakpoint 1, arank::sub1 (a=<error reading variable:
> >   failed to resolve dynamic array rank>) at assumedrank.f90:11
> > 11       PRINT *, RANK(a)
> > (gdb) p a
> > failed to resolve dynamic array rank
> > (gdb) p rank(a)
> > failed to resolve dynamic array rank
> >
> > With patch:
> > Breakpoint 1, arank::sub1 (a=0) at assumedrank.f90:11
> > 11       PRINT *, RANK(a)
> > (gdb) p a
> > $1 = 0
> > (gdb) p rank(a)
> > $2 = 0
> > ---
> >  gdb/gdbtypes.c                            | 11 +++++++----
> >  gdb/gdbtypes.h                            |  1 -
> >  gdb/testsuite/gdb.fortran/assumedrank.exp |  7 +++++++
> >  gdb/testsuite/gdb.fortran/assumedrank.f90 |  3 +++
> >  4 files changed, 17 insertions(+), 5 deletions(-)
> >
> > diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c index
> > 49ecb199b07..21db5aafc88 100644
> > --- a/gdb/gdbtypes.c
> > +++ b/gdb/gdbtypes.c
> > @@ -2398,10 +2398,13 @@ resolve_dynamic_array_or_string (struct type
> > *type,
> >
> >        if (rank == 0)
> >       {
> > -       /* The dynamic property list juggling below was from the original
> > -          patch.  I don't understand what this is all about, so I've
> > -          commented it out for now and added the following error.  */
> > -       error (_("failed to resolve dynamic array rank"));
> > +       /* Rank is zero, if a variable is passed as an argument to a
> > +          function. GDB considers the variable as an array so discard
> > +          the array type and return the target type which is of
> > + variable. */
> 
> Two spaces after after each period (end of sentence) in above comment, too.
> 
> > +          type->main_type->target_type->main_type->dyn_prop_list =
> > +                                  type->main_type->dyn_prop_list;
> > +          type = TYPE_TARGET_TYPE (type);
> > +          return type;
> >       }
> >        else if (type->code () == TYPE_CODE_STRING && rank != 1)
> >       {
> [...]
> > diff --git a/gdb/testsuite/gdb.fortran/assumedrank.exp
> > b/gdb/testsuite/gdb.fortran/assumedrank.exp
> > index 69cd168125f..49b03e2f87f 100644
> > --- a/gdb/testsuite/gdb.fortran/assumedrank.exp
> > +++ b/gdb/testsuite/gdb.fortran/assumedrank.exp
> > @@ -14,6 +14,7 @@
> >  # along with this program.  If not, see
> <https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fww
> w.gnu.org%2Flicenses%2F&amp;data=04%7C01%7Crupesh.potharla%40amd
> .com%7C0702c595b4bd4a5c196e08da1e5dc536%7C3dd8961fe4884e608e11a
> 82d994e183d%7C0%7C0%7C637855685341831678%7CUnknown%7CTWFpbG
> Zsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6
> Mn0%3D%7C3000&amp;sdata=VRqHxcI9QcHWJ3sAPo94R0HPoz%2F6ti4cLsi
> WyQWCIw8%3D&amp;reserved=0> .
> >
> >  # Testing GDB's implementation of ASSUMED RANK arrays.
> > +#Until the assumed rank zero is fixed in clang, XFAIL this case for clang.
> 
> Given that you've already put a comment about this below, I think the above
> comment could be deleted.  However, if you prefer to retain it, then insert a
> space after the '#'.
> 
> >  if {[skip_fortran_tests]} { return -1 }
> >
> > @@ -58,6 +59,12 @@ while { $test_count < 500 } {
> >           }
> >       }
> >
> > +     # xfail rank 0 for clang
> > +     if {$test_count == 1 && [test_compiler_info {clang-*}]} {
> > +        xfail "clang compiler does not support rank0"
> > +           continue
> > +     }
> > +
> >       if ($found_final_breakpoint) {
> >           break
> >       }
> 
> I'm not convinced that this is the best way to xfail the rank 0 test.
> However, after I applied only your testsuite changes, I couldn't figure out
> how to test with flang instead of gfortran.  Can you tell me how this is done?
> 
> Kevin

[-- Attachment #2: 0001-gdb-fortran-Support-for-assumed-rank-zero.patch --]
[-- Type: application/octet-stream, Size: 3893 bytes --]

From 8198f4b4792a3d3433820fb6911cc06fd6e4f5c9 Mon Sep 17 00:00:00 2001
From: rupothar <rupesh.potharla@amd.com>
Date: Fri, 8 Apr 2022 16:05:41 +0530
Subject: [PATCH] gdb/fortran: Support for assumed rank zero

If a variable is passed to function in FORTRAN as an argument the
variable is treated as an array with rank zero.  GDB currently does
not support the case for assumed rank 0.  This patch provides support
for assumed rank 0 and updates the testcase as well.

Without patch:
Breakpoint 1, arank::sub1 (a=<error reading variable:
  failed to resolve dynamic array rank>) at assumedrank.f90:11
11       PRINT *, RANK(a)
(gdb) p a
failed to resolve dynamic array rank
(gdb) p rank(a)
failed to resolve dynamic array rank

With patch:
Breakpoint 1, arank::sub1 (a=0) at assumedrank.f90:11
11       PRINT *, RANK(a)
(gdb) p a
$1 = 0
(gdb) p rank(a)
$2 = 0
---
 gdb/gdbtypes.c                            | 11 +++++++----
 gdb/gdbtypes.h                            |  1 -
 gdb/testsuite/gdb.fortran/assumedrank.exp |  6 ++++++
 gdb/testsuite/gdb.fortran/assumedrank.f90 |  3 +++
 4 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 49ecb199b07..395bd63a251 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -2398,10 +2398,13 @@ resolve_dynamic_array_or_string (struct type *type,
 
       if (rank == 0)
 	{
-	  /* The dynamic property list juggling below was from the original
-	     patch.  I don't understand what this is all about, so I've
-	     commented it out for now and added the following error.  */
-	  error (_("failed to resolve dynamic array rank"));
+	  /* Rank is zero, if a variable is passed as an argument to a
+	     function.  GDB considers the variable as an array so discard
+	     the array type and return the target type which is of variable.  */
+          type->main_type->target_type->main_type->dyn_prop_list =
+                                  type->main_type->dyn_prop_list;
+          type = TYPE_TARGET_TYPE (type);
+          return type;
 	}
       else if (type->code () == TYPE_CODE_STRING && rank != 1)
 	{
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index 769328cc9cd..7437e1db8ab 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -2092,7 +2092,6 @@ extern void allocate_gnat_aux_type (struct type *);
 #define TYPE_REFERENCE_TYPE(thistype) (thistype)->reference_type
 #define TYPE_RVALUE_REFERENCE_TYPE(thistype) (thistype)->rvalue_reference_type
 #define TYPE_CHAIN(thistype) (thistype)->chain
-#define TYPE_DYN_PROP(thistype)  TYPE_MAIN_TYPE(thistype)->dyn_prop_list
 /* * Note that if thistype is a TYPEDEF type, you have to call check_typedef.
    But check_typedef does set the TYPE_LENGTH of the TYPEDEF type,
    so you only have to call check_typedef once.  Since allocate_value
diff --git a/gdb/testsuite/gdb.fortran/assumedrank.exp b/gdb/testsuite/gdb.fortran/assumedrank.exp
index 69cd168125f..b91b5b0a28f 100644
--- a/gdb/testsuite/gdb.fortran/assumedrank.exp
+++ b/gdb/testsuite/gdb.fortran/assumedrank.exp
@@ -58,6 +58,12 @@ while { $test_count < 500 } {
 	    }
 	}
 
+	# XFAIL rank 0 for flang.
+	if {$test_count == 1 && [test_compiler_info {clang-*}]} {
+	   xfail "clang compiler does not support rank0"
+           continue
+	}
+
 	if ($found_final_breakpoint) {
 	    break
 	}
diff --git a/gdb/testsuite/gdb.fortran/assumedrank.f90 b/gdb/testsuite/gdb.fortran/assumedrank.f90
index 7f077c3f014..7f7cf2c1f3e 100644
--- a/gdb/testsuite/gdb.fortran/assumedrank.f90
+++ b/gdb/testsuite/gdb.fortran/assumedrank.f90
@@ -19,16 +19,19 @@
 
 PROGRAM  arank
 
+  REAL :: array0
   REAL :: array1(10)
   REAL :: array2(1, 2)
   REAL :: array3(3, 4, 5)
   REAL :: array4(4, 5, 6, 7)
 
+  array0 = 0
   array1 = 1.0
   array2 = 2.0
   array3 = 3.0
   array4 = 4.0
 
+  call test_rank (array0)
   call test_rank (array1)
   call test_rank (array2)
   call test_rank (array3)
-- 
2.17.1