public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* GDB/Fortran: Support for Assumed Rank Zero.
@ 2022-04-13  9:55 Potharla, Rupesh
  2022-04-13 18:27 ` Kevin Buettner
  0 siblings, 1 reply; 21+ messages in thread
From: Potharla, Rupesh @ 2022-04-13  9:55 UTC (permalink / raw)
  To: gdb-patches
  Cc: George, Jini Susan, Parasuraman, Hariharan, Sharma, Alok Kumar

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

[AMD Official Use Only]


Hi,



Requesting to review the attached patch.





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

Regards,
Rupesh P


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

From 33e5f26535788bc413ff48ae58d9d82faefcf439 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                            | 10 ++++++----
 gdb/testsuite/gdb.fortran/assumedrank.exp |  7 +++++++
 gdb/testsuite/gdb.fortran/assumedrank.f90 |  3 +++
 3 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 49ecb199b07..c2d142ac3cc 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -2398,10 +2398,12 @@ 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_DYN_PROP(TYPE_TARGET_TYPE(type)) = TYPE_DYN_PROP(type);
+          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 <http://www.gnu.org/licenses/> .
 
 # Testing GDB's implementation of ASSUMED RANK arrays.
+#Until the assumed rank zero is fixed in clang, XFAIL this case for clang.
 
 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
 	}
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


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: GDB/Fortran: Support for Assumed Rank Zero.
  2022-04-13  9:55 GDB/Fortran: Support for Assumed Rank Zero Potharla, Rupesh
@ 2022-04-13 18:27 ` Kevin Buettner
  2022-04-14 10:30   ` Potharla, Rupesh
  0 siblings, 1 reply; 21+ messages in thread
From: Kevin Buettner @ 2022-04-13 18:27 UTC (permalink / raw)
  To: Potharla, Rupesh via Gdb-patches
  Cc: Potharla, Rupesh, George, Jini Susan, Parasuraman, Hariharan,
	Sharma, Alok Kumar

Hi Rupesh,

Thanks for working on this problem.  I found a couple of nits; see
below.

On Wed, 13 Apr 2022 09:55:01 +0000
"Potharla, Rupesh via Gdb-patches" <gdb-patches@sourceware.org> wrote:

> diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
> index 49ecb199b07..c2d142ac3cc 100644
> --- a/gdb/gdbtypes.c
> +++ b/gdb/gdbtypes.c
> @@ -2398,10 +2398,12 @@ 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_DYN_PROP(TYPE_TARGET_TYPE(type)) = TYPE_DYN_PROP(type);
> +          type = TYPE_TARGET_TYPE(type);
> +          return type;
>  	}
>        else if (type->code () == TYPE_CODE_STRING && rank != 1)
>  	{
> 

GDB's coding standard requires that things like TYPE_TARGET_TYPE(type)
be written with a space before the left paren, i.e. TYPE_TARGET_TYPE (type).

See: https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards

Also, regarding TYPE_DYN_PROP, it appears to me that this macro is no
longer being used.  I've noticed that other code in gdbtypes.c is
referencing the dyn_prop_list field directly.

Kevin


^ permalink raw reply	[flat|nested] 21+ messages in thread

* RE: GDB/Fortran: Support for Assumed Rank Zero.
  2022-04-13 18:27 ` Kevin Buettner
@ 2022-04-14 10:30   ` Potharla, Rupesh
  2022-04-14 21:28     ` Kevin Buettner
  0 siblings, 1 reply; 21+ messages in thread
From: Potharla, Rupesh @ 2022-04-14 10:30 UTC (permalink / raw)
  To: Kevin Buettner, Potharla, Rupesh via Gdb-patches
  Cc: George, Jini Susan, Parasuraman, Hariharan, Sharma, Alok Kumar

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

[AMD Official Use Only]

Thanks Kevin, 

Requesting to review the updated patch with suggested changes. TYPE_DYN_PROP is added by me as part of assumed rank feature(e1ac856e3aee2f95e567e5fbca17e41b72957cdb) only for this case. Since we are no longer using it removed the macro. 


Regards,
Rupesh P

> -----Original Message-----
> From: Kevin Buettner <kevinb@redhat.com>
> Sent: Wednesday, April 13, 2022 11:58 PM
> To: Potharla, Rupesh via Gdb-patches <gdb-patches@sourceware.org>
> Cc: Potharla, Rupesh <Rupesh.Potharla@amd.com>; 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,
> 
> Thanks for working on this problem.  I found a couple of nits; see below.
> 
> On Wed, 13 Apr 2022 09:55:01 +0000
> "Potharla, Rupesh via Gdb-patches" <gdb-patches@sourceware.org> wrote:
> 
> > diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c index
> > 49ecb199b07..c2d142ac3cc 100644
> > --- a/gdb/gdbtypes.c
> > +++ b/gdb/gdbtypes.c
> > @@ -2398,10 +2398,12 @@ 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_DYN_PROP(TYPE_TARGET_TYPE(type)) =
> TYPE_DYN_PROP(type);
> > +          type = TYPE_TARGET_TYPE(type);
> > +          return type;
> >       }
> >        else if (type->code () == TYPE_CODE_STRING && rank != 1)
> >       {
> >
> 
> GDB's coding standard requires that things like TYPE_TARGET_TYPE(type) be
> written with a space before the left paren, i.e. TYPE_TARGET_TYPE (type).
> 
> See:
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsour
> ceware.org%2Fgdb%2Fwiki%2FInternals%2520GDB-C-Coding-
> Standards&amp;data=04%7C01%7Crupesh.potharla%40amd.com%7C4f7745
> 3bd29d4333881d08da1d7b55bf%7C3dd8961fe4884e608e11a82d994e183d%
> 7C0%7C0%7C637854712954812288%7CUnknown%7CTWFpbGZsb3d8eyJWIjo
> iMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C30
> 00&amp;sdata=pqbYaZKCkhvCLBDXdq9obe8NJUpFlOWK3nhPCtAWMlY%3D&
> amp;reserved=0
> 
> Also, regarding TYPE_DYN_PROP, it appears to me that this macro is no
> longer being used.  I've noticed that other code in gdbtypes.c is referencing
> the dyn_prop_list field directly.
> 
> Kevin

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

From e1ac856e3aee2f95e567e5fbca17e41b72957cdb 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 |  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. */
+          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..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 <http://www.gnu.org/licenses/> .
 
 # Testing GDB's implementation of ASSUMED RANK arrays.
+#Until the assumed rank zero is fixed in clang, XFAIL this case for clang.
 
 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
 	}
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


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: GDB/Fortran: Support for Assumed Rank Zero.
  2022-04-14 10:30   ` Potharla, Rupesh
@ 2022-04-14 21:28     ` Kevin Buettner
  2022-04-15 13:33       ` Potharla, Rupesh
  0 siblings, 1 reply; 21+ messages in thread
From: Kevin Buettner @ 2022-04-14 21:28 UTC (permalink / raw)
  To: Potharla, Rupesh
  Cc: gdb-patches, George, Jini Susan, Parasuraman, Hariharan, Sharma,
	Alok Kumar

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://www.gnu.org/prep/standards/standards.html#Comments

> 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 <http://www.gnu.org/licenses/> .
>  
>  # 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


^ permalink raw reply	[flat|nested] 21+ messages in thread

* RE: GDB/Fortran: Support for Assumed Rank Zero.
  2022-04-14 21:28     ` Kevin Buettner
@ 2022-04-15 13:33       ` Potharla, Rupesh
  2022-04-15 19:31         ` Kevin Buettner
  0 siblings, 1 reply; 21+ messages in thread
From: Potharla, Rupesh @ 2022-04-15 13:33 UTC (permalink / raw)
  To: Kevin Buettner, gdb-patches
  Cc: George, Jini Susan, Parasuraman, Hariharan, Sharma, Alok Kumar

[-- 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


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: GDB/Fortran: Support for Assumed Rank Zero.
  2022-04-15 13:33       ` Potharla, Rupesh
@ 2022-04-15 19:31         ` Kevin Buettner
  2022-04-16 18:29           ` Potharla, Rupesh
  0 siblings, 1 reply; 21+ messages in thread
From: Kevin Buettner @ 2022-04-15 19:31 UTC (permalink / raw)
  To: Potharla, Rupesh
  Cc: gdb-patches, George, Jini Susan, Parasuraman, Hariharan, Sharma,
	Alok Kumar

On Fri, 15 Apr 2022 13:33:03 +0000
"Potharla, Rupesh" <Rupesh.Potharla@amd.com> wrote:

> Apologies for missing out on these minor things.  Followed GDB
> standard for comments and updated the attached patch.

The comments look good now.  Thanks for fixing them.

> 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. 

I suggest using setup_xfail within the loop when $test_count is 1.

Kevin


^ permalink raw reply	[flat|nested] 21+ messages in thread

* RE: GDB/Fortran: Support for Assumed Rank Zero.
  2022-04-15 19:31         ` Kevin Buettner
@ 2022-04-16 18:29           ` Potharla, Rupesh
  2022-04-18 13:31             ` Tom Tromey
  2022-04-18 15:33             ` Kevin Buettner
  0 siblings, 2 replies; 21+ messages in thread
From: Potharla, Rupesh @ 2022-04-16 18:29 UTC (permalink / raw)
  To: Kevin Buettner
  Cc: gdb-patches, George, Jini Susan, Parasuraman, Hariharan, Sharma,
	Alok Kumar

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

[AMD Official Use Only]

Thanks Kevin, 

Requesting to review the attached patch with suggested code changes. 


Regards,
Rupesh P

> -----Original Message-----
> From: Kevin Buettner <kevinb@redhat.com>
> Sent: Saturday, April 16, 2022 1:01 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]
> 
> On Fri, 15 Apr 2022 13:33:03 +0000
> "Potharla, Rupesh" <Rupesh.Potharla@amd.com> wrote:
> 
> > Apologies for missing out on these minor things.  Followed GDB
> > standard for comments and updated the attached patch.
> 
> The comments look good now.  Thanks for fixing them.
> 
> > 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.
> 
> I suggest using setup_xfail within the loop when $test_count is 1.
> 
> Kevin

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

From e5082f82b46bc84f1b193d48decb72a49c261110 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 |  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..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..bd058e01e89 100644
--- a/gdb/testsuite/gdb.fortran/assumedrank.exp
+++ b/gdb/testsuite/gdb.fortran/assumedrank.exp
@@ -58,6 +58,13 @@ while { $test_count < 500 } {
 	    }
 	}
 
+	# XFAIL rank 0 for flang.
+	if {$test_count == 1 && [test_compiler_info {clang-*}]} {
+	   setup_xfail "*-*-*"
+	   fail "compiler does not support rank 0"
+	   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


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: GDB/Fortran: Support for Assumed Rank Zero.
  2022-04-16 18:29           ` Potharla, Rupesh
@ 2022-04-18 13:31             ` Tom Tromey
  2022-04-18 15:25               ` Potharla, Rupesh
  2022-04-18 15:33             ` Kevin Buettner
  1 sibling, 1 reply; 21+ messages in thread
From: Tom Tromey @ 2022-04-18 13:31 UTC (permalink / raw)
  To: Potharla, Rupesh via Gdb-patches
  Cc: Kevin Buettner, Potharla, Rupesh, George, Jini Susan,
	Parasuraman, Hariharan, Sharma, Alok Kumar

>>>>> Potharla, Rupesh via Gdb-patches <gdb-patches@sourceware.org> writes:

> Requesting to review the attached patch with suggested code changes. 

> +          type->main_type->target_type->main_type->dyn_prop_list =
> +                                  type->main_type->dyn_prop_list;

In the gdb style, the '=' goes on the next line, and also the
continuation line is just indented 2 spaces.

thanks,
Tom

^ permalink raw reply	[flat|nested] 21+ messages in thread

* RE: GDB/Fortran: Support for Assumed Rank Zero.
  2022-04-18 13:31             ` Tom Tromey
@ 2022-04-18 15:25               ` Potharla, Rupesh
  2022-04-20 15:22                 ` Andrew Burgess
  0 siblings, 1 reply; 21+ messages in thread
From: Potharla, Rupesh @ 2022-04-18 15:25 UTC (permalink / raw)
  To: Tom Tromey, Potharla, Rupesh via Gdb-patches
  Cc: Kevin Buettner, George, Jini Susan, Parasuraman, Hariharan,
	Sharma, Alok Kumar

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

[AMD Official Use Only]

Thanks Tom,

Made the suggested change and attached the updated patch. 

Regards,
Rupesh P


> -----Original Message-----
> From: Tom Tromey <tom@tromey.com>
> Sent: Monday, April 18, 2022 7:01 PM
> To: Potharla, Rupesh via Gdb-patches <gdb-patches@sourceware.org>
> Cc: Kevin Buettner <kevinb@redhat.com>; Potharla, Rupesh
> <Rupesh.Potharla@amd.com>; 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]
> 
> >>>>> Potharla, Rupesh via Gdb-patches <gdb-patches@sourceware.org>
> writes:
> 
> > Requesting to review the attached patch with suggested code changes.
> 
> > +          type->main_type->target_type->main_type->dyn_prop_list =
> > +                                  type->main_type->dyn_prop_list;
> 
> In the gdb style, the '=' goes on the next line, and also the continuation line is
> just indented 2 spaces.
> 
> thanks,
> Tom

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

From 5357e50f3083d6fcbdcab4ca8932ae123d32773b 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 |  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..afedd1f61b7 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..bd058e01e89 100644
--- a/gdb/testsuite/gdb.fortran/assumedrank.exp
+++ b/gdb/testsuite/gdb.fortran/assumedrank.exp
@@ -58,6 +58,13 @@ while { $test_count < 500 } {
 	    }
 	}
 
+	# XFAIL rank 0 for flang.
+	if {$test_count == 1 && [test_compiler_info {clang-*}]} {
+	   setup_xfail "*-*-*"
+	   fail "compiler does not support rank 0"
+	   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


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: GDB/Fortran: Support for Assumed Rank Zero.
  2022-04-16 18:29           ` Potharla, Rupesh
  2022-04-18 13:31             ` Tom Tromey
@ 2022-04-18 15:33             ` Kevin Buettner
  2022-04-19 17:30               ` Kevin Buettner
  1 sibling, 1 reply; 21+ messages in thread
From: Kevin Buettner @ 2022-04-18 15:33 UTC (permalink / raw)
  To: Potharla, Rupesh
  Cc: gdb-patches, George, Jini Susan, Parasuraman, Hariharan, Sharma,
	Alok Kumar

On Sat, 16 Apr 2022 18:29:46 +0000
"Potharla, Rupesh" <Rupesh.Potharla@amd.com> wrote:

> diff --git a/gdb/testsuite/gdb.fortran/assumedrank.exp b/gdb/testsuite/gdb.fortran/assumedrank.exp
> index 69cd168125f..bd058e01e89 100644
> --- a/gdb/testsuite/gdb.fortran/assumedrank.exp
> +++ b/gdb/testsuite/gdb.fortran/assumedrank.exp
> @@ -58,6 +58,13 @@ while { $test_count < 500 } {
>  	    }
>  	}
>  
> +	# XFAIL rank 0 for flang.
> +	if {$test_count == 1 && [test_compiler_info {clang-*}]} {
> +	   setup_xfail "*-*-*"
> +	   fail "compiler does not support rank 0"
> +	   continue
> +	}
> +
>  	if ($found_final_breakpoint) {
>  	    break
>  	}
> 

Now that you're using setup_xfail, this should be moved into the loop
above.  Also, you should remove the fail and the continue.  You'll
want to use setup_xfail just before running the test for which you want
to turn a possible FAIL into an XFAIL.

I'm also (still) wondering how to run the testsuite to use flang
instead of gfortran since it'd be nice to test to see if the setup_xfail
actually works.  I spent just enough time looking at it last week
to convince myself that GDB's testing infrastructure currently lacks the
mechanisms to do this.  However, I could be wrong, so if someone
knows how it's done, I'd like to learn about it...

Kevin


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: GDB/Fortran: Support for Assumed Rank Zero.
  2022-04-18 15:33             ` Kevin Buettner
@ 2022-04-19 17:30               ` Kevin Buettner
  2022-04-20  0:29                 ` Potharla, Rupesh
  0 siblings, 1 reply; 21+ messages in thread
From: Kevin Buettner @ 2022-04-19 17:30 UTC (permalink / raw)
  To: Kevin Buettner via Gdb-patches
  Cc: Kevin Buettner, Potharla, Rupesh, George, Jini Susan,
	Parasuraman, Hariharan, Sharma, Alok Kumar

On Mon, 18 Apr 2022 08:33:28 -0700
Kevin Buettner via Gdb-patches <gdb-patches@sourceware.org> wrote:

> I'm also (still) wondering how to run the testsuite to use flang
> instead of gfortran since it'd be nice to test to see if the setup_xfail
> actually works.  I spent just enough time looking at it last week
> to convince myself that GDB's testing infrastructure currently lacks the
> mechanisms to do this.  However, I could be wrong, so if someone
> knows how it's done, I'd like to learn about it...

It seems that the answer is to run the test as follows:

make check TESTS="gdb.fortran/assumedrank.exp" RUNTESTFLAGS="F90_FOR_TARGET=flang"

(I thought that I had tried this last week.)

When I run the above command on my Fedora 35 machine, I see:

# of expected passes		500
# of unexpected failures	2503
# of unresolved testcases	500

So it seems that this test doesn't work very well with flang at the
moment.

That being the case, while I'm not against adding setup_xfail for
clang, it doesn't seem all that useful due to the dismal results
when testing against flang.

Kevin


^ permalink raw reply	[flat|nested] 21+ messages in thread

* RE: GDB/Fortran: Support for Assumed Rank Zero.
  2022-04-19 17:30               ` Kevin Buettner
@ 2022-04-20  0:29                 ` Potharla, Rupesh
  2022-04-20  6:32                   ` Kempke, Nils-Christian
  2022-04-20 15:29                   ` Andrew Burgess
  0 siblings, 2 replies; 21+ messages in thread
From: Potharla, Rupesh @ 2022-04-20  0:29 UTC (permalink / raw)
  To: Kevin Buettner, Kevin Buettner via Gdb-patches
  Cc: George, Jini Susan, Parasuraman, Hariharan, Sharma, Alok Kumar

[AMD Official Use Only]


>On Mon, 18 Apr 2022 08:33:28 -0700
>Kevin Buettner via Gdb-patches <gdb-patches@sourceware.org> wrote:
>
>> I'm also (still) wondering how to run the testsuite to use flang
>> instead of gfortran since it'd be nice to test to see if the
>> setup_xfail actually works.  I spent just enough time looking at it
>> last week to convince myself that GDB's testing infrastructure
>> currently lacks the mechanisms to do this.  However, I could be wrong,
>> so if someone knows how it's done, I'd like to learn about it...
>
>It seems that the answer is to run the test as follows:
>
>make check TESTS="gdb.fortran/assumedrank.exp"
>RUNTESTFLAGS="F90_FOR_TARGET=flang"
>
>(I thought that I had tried this last week.)
>
>When I run the above command on my Fedora 35 machine, I see:
>
># of expected passes            500
># of unexpected failures        2503
># of unresolved testcases       500
>
But the command should only run assumedrank.exp testcase, looks like it ran all the tests under gdb.fortran.

gdb summary after running assumedrank.exp with flang:

                === gdb Summary ===

# of expected passes            25
# of expected failures          1

>So it seems that this test doesn't work very well with flang at the moment.
>
>That being the case, while I'm not against adding setup_xfail for clang, it
>doesn't seem all that useful due to the dismal results when testing against
>flang.
>
 

        if {$test_count == 1 && [test_compiler_info {clang-*}]} {
           setup_xfail "*-*-*"
           fail "compiler does not support rank 0"
           continue
        }

I think failing the testcase at the iteration entry and continuing to test next case is the best way to handle these dismal results.  
Please let me know your suggestions/comments on this.    

Regards,
Rupesh P


^ permalink raw reply	[flat|nested] 21+ messages in thread

* RE: GDB/Fortran: Support for Assumed Rank Zero.
  2022-04-20  0:29                 ` Potharla, Rupesh
@ 2022-04-20  6:32                   ` Kempke, Nils-Christian
  2022-04-20 15:38                     ` Kevin Buettner
  2022-04-20 15:29                   ` Andrew Burgess
  1 sibling, 1 reply; 21+ messages in thread
From: Kempke, Nils-Christian @ 2022-04-20  6:32 UTC (permalink / raw)
  To: Potharla, Rupesh, Kevin Buettner, Kevin Buettner via Gdb-patches
  Cc: George, Jini Susan, Parasuraman, Hariharan, Sharma, Alok Kumar



> -----Original Message-----
> From: Gdb-patches <gdb-patches-bounces+nils-
> christian.kempke=intel.com@sourceware.org> On Behalf Of Potharla,
> Rupesh via Gdb-patches
> Sent: Wednesday, April 20, 2022 2:29 AM
> To: Kevin Buettner <kevinb@redhat.com>; Kevin Buettner via Gdb-patches
> <gdb-patches@sourceware.org>
> Cc: 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.
> 
> [AMD Official Use Only]
> 
> 
> >On Mon, 18 Apr 2022 08:33:28 -0700
> >Kevin Buettner via Gdb-patches <gdb-patches@sourceware.org> wrote:
> >
> >> I'm also (still) wondering how to run the testsuite to use flang
> >> instead of gfortran since it'd be nice to test to see if the
> >> setup_xfail actually works.  I spent just enough time looking at it
> >> last week to convince myself that GDB's testing infrastructure
> >> currently lacks the mechanisms to do this.  However, I could be wrong,
> >> so if someone knows how it's done, I'd like to learn about it...
> >
> >It seems that the answer is to run the test as follows:
> >
> >make check TESTS="gdb.fortran/assumedrank.exp"
> >RUNTESTFLAGS="F90_FOR_TARGET=flang"
> >
> >(I thought that I had tried this last week.)
> >
> >When I run the above command on my Fedora 35 machine, I see:
> >
> ># of expected passes            500
> ># of unexpected failures        2503
> ># of unresolved testcases       500
> >
> But the command should only run assumedrank.exp testcase, looks like it ran
> all the tests under gdb.fortran.
> 
> gdb summary after running assumedrank.exp with flang:
> 
>                 === gdb Summary ===
> 
> # of expected passes            25
> # of expected failures          1
> 
> >So it seems that this test doesn't work very well with flang at the moment.
> >
> >That being the case, while I'm not against adding setup_xfail for clang, it
> >doesn't seem all that useful due to the dismal results when testing against
> >flang.
> >
> 
> 
>         if {$test_count == 1 && [test_compiler_info {clang-*}]} {
>            setup_xfail "*-*-*"
>            fail "compiler does not support rank 0"
>            continue
>         }
> 
> I think failing the testcase at the iteration entry and continuing to test next
> case is the best way to handle these dismal results.
> Please let me know your suggestions/comments on this.
> 
> Regards,
> Rupesh P

Please not a problem with this - if one wants to use a Fortran compiler other than
gfortran one always has to also supply the respective c-compiler in order for it to
get recognized properly. This is a shortcoming in the current compiler
identification - I'm actually working on a patch for that.

But running
   make check TESTS="gdb.fortran/assumedrank.exp" RUNTESTFLAGS="F90_FOR_TARGET=flang"
and looking at the log will show that the compiler identification actually identified
gcc and gfortran as the compiler used (it will still try to compile with flang but the identified
compiler in get_/test_compiler_info is gcc..). From my run in gdb.log:

   get_compiler_info: gcc-9-3-0
   UNTESTED: gdb.fortran/assumedrank.exp: compiler does not support assumed rank
   testcase /.../gdb/testsuite/gdb.fortran/assumedrank.exp completed in 0 seconds

                   === gdb Summary ===

   # of untested testcases         1

The correct command would probably be

   make check TESTS="gdb.fortran/assumedrank.exp" RUNTESTFLAGS="F90_FOR_TARGET=flang
      CC_FOR_TARGET=clang"

which leads to output like:

   Running .../testsuite/gdb.fortran/pointers.exp ...
   get_compiler_info: clang-10-0-0

Cheers,
Nils
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


^ permalink raw reply	[flat|nested] 21+ messages in thread

* RE: GDB/Fortran: Support for Assumed Rank Zero.
  2022-04-18 15:25               ` Potharla, Rupesh
@ 2022-04-20 15:22                 ` Andrew Burgess
  2022-04-20 19:08                   ` Potharla, Rupesh
  0 siblings, 1 reply; 21+ messages in thread
From: Andrew Burgess @ 2022-04-20 15:22 UTC (permalink / raw)
  To: Potharla, Rupesh, Tom Tromey, Potharla, Rupesh via Gdb-patches
  Cc: George, Jini Susan, Parasuraman, Hariharan, Sharma, Alok Kumar

"Potharla, Rupesh via Gdb-patches" <gdb-patches@sourceware.org> writes:

> [AMD Official Use Only]
>
> Thanks Tom,
>
> Made the suggested change and attached the updated patch. 
>
> Regards,
> Rupesh P
>
>
>> -----Original Message-----
>> From: Tom Tromey <tom@tromey.com>
>> Sent: Monday, April 18, 2022 7:01 PM
>> To: Potharla, Rupesh via Gdb-patches <gdb-patches@sourceware.org>
>> Cc: Kevin Buettner <kevinb@redhat.com>; Potharla, Rupesh
>> <Rupesh.Potharla@amd.com>; 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]
>> 
>> >>>>> Potharla, Rupesh via Gdb-patches <gdb-patches@sourceware.org>
>> writes:
>> 
>> > Requesting to review the attached patch with suggested code changes.
>> 
>> > +          type->main_type->target_type->main_type->dyn_prop_list =
>> > +                                  type->main_type->dyn_prop_list;
>> 
>> In the gdb style, the '=' goes on the next line, and also the continuation line is
>> just indented 2 spaces.
>> 
>> thanks,
>> Tom
> From 5357e50f3083d6fcbdcab4ca8932ae123d32773b 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 |  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..afedd1f61b7 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;

I don't think this is correct, but I'm not completely sure what the
correct thing to do is.

TYPE here was created with a call to copy_type.  After this call TYPE is
a copy of the original dynamic type, but I believe that this is only a
shallow copy (see copy_type in gdbtypes.c), so target_type will point at
the same type object as the original dynamic type.

When you copy the dyn_prop_list like you do above you are modifying the
original target_type object.  The next time this function is called
we'll end up modifying the same target_type object again.  I don't think
this is correct.

Also I don't think we are supposed to be creating multiple pointers to
the same dyn_prop_list object like you're doing, I think you need to
call copy_dynamic_prop_list.

That said, I'm a little concerned that by just copying over the
properties like this you're discarding any dynamic properties that might
already exist on the target_type - though I'm not sure if it's possible
to create a target_type that actually has any dynamic properties of its
own.... maybe that's a problem we can leave until such a case crops up?
I do suspect it might only be the data location that you really care
about, so maybe we should only be copying that property?

Anyway, if we don't worry about dynamic properties that might already
exist on target_type, then the following code seems to work, but I've
only done a quick test:

      if (rank == 0)
	{
	  /* Rank is zero if a variable is passed as an argument to a
	     function.  In this case the resolved type should not be an
	     array, but should instead be that of an array element.  */
	  struct type *dynamic_array_type = type;
	  type = copy_type (TYPE_TARGET_TYPE (dynamic_array_type));
	  struct dynamic_prop_list *prop_list
	    = TYPE_MAIN_TYPE (dynamic_array_type)->dyn_prop_list;
	  if (prop_list != nullptr)
	    {
	      struct obstack *obstack
		= &type->objfile_owner ()->objfile_obstack;
	      TYPE_MAIN_TYPE (type)->dyn_prop_list
		= copy_dynamic_prop_list (obstack, prop_list);
	    }
	  return type;
	}

You'll notice I also changed the comment within this block as I found
the original comment hard to understand.

What do you think?

Thanks,
Andrew


>  	}
>        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..bd058e01e89 100644
> --- a/gdb/testsuite/gdb.fortran/assumedrank.exp
> +++ b/gdb/testsuite/gdb.fortran/assumedrank.exp
> @@ -58,6 +58,13 @@ while { $test_count < 500 } {
>  	    }
>  	}
>  
> +	# XFAIL rank 0 for flang.
> +	if {$test_count == 1 && [test_compiler_info {clang-*}]} {
> +	   setup_xfail "*-*-*"
> +	   fail "compiler does not support rank 0"
> +	   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


^ permalink raw reply	[flat|nested] 21+ messages in thread

* RE: GDB/Fortran: Support for Assumed Rank Zero.
  2022-04-20  0:29                 ` Potharla, Rupesh
  2022-04-20  6:32                   ` Kempke, Nils-Christian
@ 2022-04-20 15:29                   ` Andrew Burgess
  2022-04-20 19:20                     ` Potharla, Rupesh
  1 sibling, 1 reply; 21+ messages in thread
From: Andrew Burgess @ 2022-04-20 15:29 UTC (permalink / raw)
  To: Potharla, Rupesh, Kevin Buettner, Kevin Buettner via Gdb-patches
  Cc: George, Jini Susan, Parasuraman, Hariharan, Sharma, Alok Kumar

"Potharla, Rupesh via Gdb-patches" <gdb-patches@sourceware.org> writes:

> [AMD Official Use Only]
>
>
>>On Mon, 18 Apr 2022 08:33:28 -0700
>>Kevin Buettner via Gdb-patches <gdb-patches@sourceware.org> wrote:
>>
>>> I'm also (still) wondering how to run the testsuite to use flang
>>> instead of gfortran since it'd be nice to test to see if the
>>> setup_xfail actually works.  I spent just enough time looking at it
>>> last week to convince myself that GDB's testing infrastructure
>>> currently lacks the mechanisms to do this.  However, I could be wrong,
>>> so if someone knows how it's done, I'd like to learn about it...
>>
>>It seems that the answer is to run the test as follows:
>>
>>make check TESTS="gdb.fortran/assumedrank.exp"
>>RUNTESTFLAGS="F90_FOR_TARGET=flang"
>>
>>(I thought that I had tried this last week.)
>>
>>When I run the above command on my Fedora 35 machine, I see:
>>
>># of expected passes            500
>># of unexpected failures        2503
>># of unresolved testcases       500
>>
> But the command should only run assumedrank.exp testcase, looks like it ran all the tests under gdb.fortran.
>
> gdb summary after running assumedrank.exp with flang:
>
>                 === gdb Summary ===
>
> # of expected passes            25
> # of expected failures          1
>
>>So it seems that this test doesn't work very well with flang at the moment.
>>
>>That being the case, while I'm not against adding setup_xfail for clang, it
>>doesn't seem all that useful due to the dismal results when testing against
>>flang.
>>
>  
>
>         if {$test_count == 1 && [test_compiler_info {clang-*}]} {
>            setup_xfail "*-*-*"
>            fail "compiler does not support rank 0"
>            continue
>         }

It is possible to call 'xfail' directly, so you could just do:

  xfail "compiler does not support rank 0"

But I don't think that's correct really, xfail should be used when a
specific test is actually being run, so we'd expect to do something
like:

  xfail $test_name

See gdb.ada/call_pn.exp for examples.

I wonder if really 'unsupported' would be a better choice here - the
description in the Dejagnu manual says:

  UNSUPPORTED
       There is no support for the tested case.  This may mean that a
       conditional feature of an operating system, or of a compiler, is
       not implemented....

Which seems to describe this case perfectly, so:

    if {$test_count == 1 && [test_compiler_info {clang-*}]} {
      unsupported "compiler does not support rank 0"
      continue
    }

Thoughts?

Thanks,
Andrew


>
> I think failing the testcase at the iteration entry and continuing to test next case is the best way to handle these dismal results.  
> Please let me know your suggestions/comments on this.    
>
> Regards,
> Rupesh P


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: GDB/Fortran: Support for Assumed Rank Zero.
  2022-04-20  6:32                   ` Kempke, Nils-Christian
@ 2022-04-20 15:38                     ` Kevin Buettner
  0 siblings, 0 replies; 21+ messages in thread
From: Kevin Buettner @ 2022-04-20 15:38 UTC (permalink / raw)
  To: Kempke, Nils-Christian
  Cc: gdb-patches, Potharla, Rupesh, George, Jini Susan, Parasuraman,
	Hariharan, Sharma, Alok Kumar

On Wed, 20 Apr 2022 06:32:25 +0000
"Kempke, Nils-Christian" <nils-christian.kempke@intel.com> wrote:

> But running
>    make check TESTS="gdb.fortran/assumedrank.exp" RUNTESTFLAGS="F90_FOR_TARGET=flang"
> and looking at the log will show that the compiler identification actually identified
> gcc and gfortran as the compiler used (it will still try to compile with flang but the identified
> compiler in get_/test_compiler_info is gcc..). From my run in gdb.log:
> 
>    get_compiler_info: gcc-9-3-0
>    UNTESTED: gdb.fortran/assumedrank.exp: compiler does not support assumed rank
>    testcase /.../gdb/testsuite/gdb.fortran/assumedrank.exp completed in 0 seconds
> 
>                    === gdb Summary ===
> 
>    # of untested testcases         1
> 
> The correct command would probably be
> 
>    make check TESTS="gdb.fortran/assumedrank.exp" RUNTESTFLAGS="F90_FOR_TARGET=flang
>       CC_FOR_TARGET=clang"
> 
> which leads to output like:
> 
>    Running .../testsuite/gdb.fortran/pointers.exp ...
>    get_compiler_info: clang-10-0-0

Thanks for this info.  I do indeed see similar results when setting both
F90_FOR_TARGET and CC_FOR_TARGET.

Kevin


^ permalink raw reply	[flat|nested] 21+ messages in thread

* RE: GDB/Fortran: Support for Assumed Rank Zero.
  2022-04-20 15:22                 ` Andrew Burgess
@ 2022-04-20 19:08                   ` Potharla, Rupesh
  2022-04-22 14:38                     ` Andrew Burgess
  0 siblings, 1 reply; 21+ messages in thread
From: Potharla, Rupesh @ 2022-04-20 19:08 UTC (permalink / raw)
  To: Andrew Burgess, Tom Tromey, Potharla, Rupesh via Gdb-patches
  Cc: George, Jini Susan, Parasuraman, Hariharan, Sharma, Alok Kumar

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

[AMD Official Use Only]

Thanks, Andrew,

Made suggested code changes and attached the updated patch. Requesting to review the code changes.  


Regards,
Rupesh P

>> From 5357e50f3083d6fcbdcab4ca8932ae123d32773b 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 |  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..afedd1f61b7 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;
>
>I don't think this is correct, but I'm not completely sure what the correct thing
>to do is.
>
>TYPE here was created with a call to copy_type.  After this call TYPE is a copy
>of the original dynamic type, but I believe that this is only a shallow copy (see
>copy_type in gdbtypes.c), so target_type will point at the same type object as
>the original dynamic type.
>
>When you copy the dyn_prop_list like you do above you are modifying the
>original target_type object.  The next time this function is called we'll end up
>modifying the same target_type object again.  I don't think this is correct.
>
>Also I don't think we are supposed to be creating multiple pointers to the
>same dyn_prop_list object like you're doing, I think you need to call
>copy_dynamic_prop_list.
>
>That said, I'm a little concerned that by just copying over the properties like
>this you're discarding any dynamic properties that might already exist on the
>target_type - though I'm not sure if it's possible to create a target_type that
>actually has any dynamic properties of its own.... maybe that's a problem we
>can leave until such a case crops up?
>I do suspect it might only be the data location that you really care about, so
>maybe we should only be copying that property?

Yes, the change was made only for data location for printing the value.  

>Anyway, if we don't worry about dynamic properties that might already exist
>on target_type, then the following code seems to work, but I've only done a
>quick test:

So far, I have not seen dynamic properties for array elements for rank0. I picked up the below code changes and updated the patch.   

>      if (rank == 0)
>        {
>          /* Rank is zero if a variable is passed as an argument to a
>             function.  In this case the resolved type should not be an
>             array, but should instead be that of an array element.  */
>          struct type *dynamic_array_type = type;
>          type = copy_type (TYPE_TARGET_TYPE (dynamic_array_type));
>          struct dynamic_prop_list *prop_list
>            = TYPE_MAIN_TYPE (dynamic_array_type)->dyn_prop_list;
>          if (prop_list != nullptr)
>            {
>              struct obstack *obstack
>                = &type->objfile_owner ()->objfile_obstack;
>              TYPE_MAIN_TYPE (type)->dyn_prop_list
>                = copy_dynamic_prop_list (obstack, prop_list);
>            }
>          return type;
>        }
>
>You'll notice I also changed the comment within this block as I found the
>original comment hard to understand.

Apologies for the hard comment, Thanks for the correction. 

>What do you think?
>
>Thanks,
>Andrew
>
>
>>       }
>>        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..bd058e01e89 100644
>> --- a/gdb/testsuite/gdb.fortran/assumedrank.exp
>> +++ b/gdb/testsuite/gdb.fortran/assumedrank.exp
>> @@ -58,6 +58,13 @@ while { $test_count < 500 } {
>>           }
>>       }
>>
>> +     # XFAIL rank 0 for flang.
>> +     if {$test_count == 1 && [test_compiler_info {clang-*}]} {
>> +        setup_xfail "*-*-*"
>> +        fail "compiler does not support rank 0"
>> +        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

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

From fbfa2f4c26eeef7547603aafafb9525d66d1b172 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                            | 21 ++++++++++++++++-----
 gdb/gdbtypes.h                            |  4 +++-
 gdb/testsuite/gdb.fortran/assumedrank.exp |  6 ++++++
 gdb/testsuite/gdb.fortran/assumedrank.f90 |  3 +++
 4 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 49ecb199b07..47cf442a2d5 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -2398,10 +2398,21 @@ 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.  In this case the resolved type should not be an
+	     array, but should instead be that of an array element.  */
+	  struct type *dynamic_array_type = type;
+	  type = copy_type (TYPE_TARGET_TYPE (dynamic_array_type));
+	  struct dynamic_prop_list *prop_list
+	    = TYPE_MAIN_TYPE (dynamic_array_type)->dyn_prop_list;
+	  if (prop_list != nullptr)
+	    {
+	      struct obstack *obstack
+		= &type->objfile_owner ()->objfile_obstack;
+	      TYPE_MAIN_TYPE (type)->dyn_prop_list
+		= copy_dynamic_prop_list (obstack, prop_list);
+	    }
+	  return type;
 	}
       else if (type->code () == TYPE_CODE_STRING && rank != 1)
 	{
@@ -5589,7 +5600,7 @@ create_copied_types_hash (struct objfile *objfile)
 
 /* Recursively copy (deep copy) a dynamic attribute list of a type.  */
 
-static struct dynamic_prop_list *
+struct dynamic_prop_list *
 copy_dynamic_prop_list (struct obstack *objfile_obstack,
 			struct dynamic_prop_list *list)
 {
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index 769328cc9cd..99273d14c1f 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
@@ -2882,6 +2881,9 @@ extern struct type *copy_type_recursive (struct objfile *objfile,
 					 struct type *type,
 					 htab_t copied_types);
 
+extern struct dynamic_prop_list * copy_dynamic_prop_list
+  (struct obstack *objfile_obstack, struct dynamic_prop_list *list);
+
 extern struct type *copy_type (const struct type *type);
 
 extern bool types_equal (struct type *, struct type *);
diff --git a/gdb/testsuite/gdb.fortran/assumedrank.exp b/gdb/testsuite/gdb.fortran/assumedrank.exp
index 69cd168125f..e9429b44a9a 100644
--- a/gdb/testsuite/gdb.fortran/assumedrank.exp
+++ b/gdb/testsuite/gdb.fortran/assumedrank.exp
@@ -58,6 +58,12 @@ while { $test_count < 500 } {
 	    }
 	}
 
+	# Currently, flang does not support rank0.
+	if {$test_count == 1 && [test_compiler_info {clang-*}]} {
+	   unsupported "compiler does not support rank 0"
+	   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


^ permalink raw reply	[flat|nested] 21+ messages in thread

* RE: GDB/Fortran: Support for Assumed Rank Zero.
  2022-04-20 15:29                   ` Andrew Burgess
@ 2022-04-20 19:20                     ` Potharla, Rupesh
  0 siblings, 0 replies; 21+ messages in thread
From: Potharla, Rupesh @ 2022-04-20 19:20 UTC (permalink / raw)
  To: Andrew Burgess, Kevin Buettner, Kevin Buettner via Gdb-patches
  Cc: George, Jini Susan, Parasuraman, Hariharan, Sharma, Alok Kumar

[AMD Official Use Only]



>-----Original Message-----
>From: Andrew Burgess <aburgess@redhat.com>
>Sent: Wednesday, April 20, 2022 9:00 PM
>To: Potharla, Rupesh <Rupesh.Potharla@amd.com>; Kevin Buettner
><kevinb@redhat.com>; Kevin Buettner via Gdb-patches <gdb-
>patches@sourceware.org>
>Cc: 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]
>
>"Potharla, Rupesh via Gdb-patches" <gdb-patches@sourceware.org> writes:
>
>> [AMD Official Use Only]
>>
>>
>>>On Mon, 18 Apr 2022 08:33:28 -0700
>>>Kevin Buettner via Gdb-patches <gdb-patches@sourceware.org> wrote:
>>>
>>>> I'm also (still) wondering how to run the testsuite to use flang
>>>> instead of gfortran since it'd be nice to test to see if the
>>>> setup_xfail actually works.  I spent just enough time looking at it
>>>> last week to convince myself that GDB's testing infrastructure
>>>> currently lacks the mechanisms to do this.  However, I could be
>>>> wrong, so if someone knows how it's done, I'd like to learn about it...
>>>
>>>It seems that the answer is to run the test as follows:
>>>
>>>make check TESTS="gdb.fortran/assumedrank.exp"
>>>RUNTESTFLAGS="F90_FOR_TARGET=flang"
>>>
>>>(I thought that I had tried this last week.)
>>>
>>>When I run the above command on my Fedora 35 machine, I see:
>>>
>>># of expected passes            500
>>># of unexpected failures        2503
>>># of unresolved testcases       500
>>>
>> But the command should only run assumedrank.exp testcase, looks like it
>ran all the tests under gdb.fortran.
>>
>> gdb summary after running assumedrank.exp with flang:
>>
>>                 === gdb Summary ===
>>
>> # of expected passes            25
>> # of expected failures          1
>>
>>>So it seems that this test doesn't work very well with flang at the moment.
>>>
>>>That being the case, while I'm not against adding setup_xfail for
>>>clang, it doesn't seem all that useful due to the dismal results when
>>>testing against flang.
>>>
>>
>>
>>         if {$test_count == 1 && [test_compiler_info {clang-*}]} {
>>            setup_xfail "*-*-*"
>>            fail "compiler does not support rank 0"
>>            continue
>>         }
>
>It is possible to call 'xfail' directly, so you could just do:
>
>  xfail "compiler does not support rank 0"
>
>But I don't think that's correct really, xfail should be used when a specific test
>is actually being run, so we'd expect to do something
>like:
>
>  xfail $test_name
>
>See gdb.ada/call_pn.exp for examples.
>
>I wonder if really 'unsupported' would be a better choice here - the
>description in the Dejagnu manual says:
>
>  UNSUPPORTED
>       There is no support for the tested case.  This may mean that a
>       conditional feature of an operating system, or of a compiler, is
>       not implemented....
>
>Which seems to describe this case perfectly, so:
>
>    if {$test_count == 1 && [test_compiler_info {clang-*}]} {
>      unsupported "compiler does not support rank 0"
>      continue
>    }
>
>Thoughts?

I started with xfail then realized xfail is not used in fortran testcases. Unsupported looks like the best option for this case, changing it to unsupported.    

>Thanks,
>Andrew
>
>
>>
>> I think failing the testcase at the iteration entry and continuing to test next
>case is the best way to handle these dismal results.
>> Please let me know your suggestions/comments on this.
>>
>> Regards,
>> Rupesh P

^ permalink raw reply	[flat|nested] 21+ messages in thread

* RE: GDB/Fortran: Support for Assumed Rank Zero.
  2022-04-20 19:08                   ` Potharla, Rupesh
@ 2022-04-22 14:38                     ` Andrew Burgess
  2022-04-25  6:33                       ` Potharla, Rupesh
  0 siblings, 1 reply; 21+ messages in thread
From: Andrew Burgess @ 2022-04-22 14:38 UTC (permalink / raw)
  To: Potharla, Rupesh, Tom Tromey, Potharla, Rupesh via Gdb-patches
  Cc: George, Jini Susan, Parasuraman, Hariharan, Sharma, Alok Kumar

"Potharla, Rupesh via Gdb-patches" <gdb-patches@sourceware.org> writes:

> [AMD Official Use Only]
>
> Thanks, Andrew,
>
> Made suggested code changes and attached the updated patch. Requesting
> to review the code changes.

Thanks, this all looks fine to me, except for one small issue...

> From fbfa2f4c26eeef7547603aafafb9525d66d1b172 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                            | 21 ++++++++++++++++-----
>  gdb/gdbtypes.h                            |  4 +++-
>  gdb/testsuite/gdb.fortran/assumedrank.exp |  6 ++++++
>  gdb/testsuite/gdb.fortran/assumedrank.f90 |  3 +++
>  4 files changed, 28 insertions(+), 6 deletions(-)
> 
> diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
> index 49ecb199b07..47cf442a2d5 100644
> --- a/gdb/gdbtypes.c
> +++ b/gdb/gdbtypes.c
> @@ -2398,10 +2398,21 @@ 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.  In this case the resolved type should not be an
> +	     array, but should instead be that of an array element.  */
> +	  struct type *dynamic_array_type = type;
> +	  type = copy_type (TYPE_TARGET_TYPE (dynamic_array_type));
> +	  struct dynamic_prop_list *prop_list
> +	    = TYPE_MAIN_TYPE (dynamic_array_type)->dyn_prop_list;
> +	  if (prop_list != nullptr)
> +	    {
> +	      struct obstack *obstack
> +		= &type->objfile_owner ()->objfile_obstack;
> +	      TYPE_MAIN_TYPE (type)->dyn_prop_list
> +		= copy_dynamic_prop_list (obstack, prop_list);
> +	    }
> +	  return type;
>  	}
>        else if (type->code () == TYPE_CODE_STRING && rank != 1)
>  	{
> @@ -5589,7 +5600,7 @@ create_copied_types_hash (struct objfile *objfile)
>  
>  /* Recursively copy (deep copy) a dynamic attribute list of a type.  */
>  
> -static struct dynamic_prop_list *
> +struct dynamic_prop_list *
>  copy_dynamic_prop_list (struct obstack *objfile_obstack,
>  			struct dynamic_prop_list *list)
>  {

Please leave this as 'static', and just add a forward declaration within
the gdbtypes.c.

With that change, this is OK to commit.

Thanks,
Andrew

> diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
> index 769328cc9cd..99273d14c1f 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
> @@ -2882,6 +2881,9 @@ extern struct type *copy_type_recursive (struct objfile *objfile,
>  					 struct type *type,
>  					 htab_t copied_types);
>  
> +extern struct dynamic_prop_list * copy_dynamic_prop_list
> +  (struct obstack *objfile_obstack, struct dynamic_prop_list *list);
> +
>  extern struct type *copy_type (const struct type *type);
>  
>  extern bool types_equal (struct type *, struct type *);
> diff --git a/gdb/testsuite/gdb.fortran/assumedrank.exp b/gdb/testsuite/gdb.fortran/assumedrank.exp
> index 69cd168125f..e9429b44a9a 100644
> --- a/gdb/testsuite/gdb.fortran/assumedrank.exp
> +++ b/gdb/testsuite/gdb.fortran/assumedrank.exp
> @@ -58,6 +58,12 @@ while { $test_count < 500 } {
>  	    }
>  	}
>  
> +	# Currently, flang does not support rank0.
> +	if {$test_count == 1 && [test_compiler_info {clang-*}]} {
> +	   unsupported "compiler does not support rank 0"
> +	   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


^ permalink raw reply	[flat|nested] 21+ messages in thread

* RE: GDB/Fortran: Support for Assumed Rank Zero.
  2022-04-22 14:38                     ` Andrew Burgess
@ 2022-04-25  6:33                       ` Potharla, Rupesh
  2022-04-25  8:47                         ` Andrew Burgess
  0 siblings, 1 reply; 21+ messages in thread
From: Potharla, Rupesh @ 2022-04-25  6:33 UTC (permalink / raw)
  To: Andrew Burgess, Tom Tromey, Potharla, Rupesh via Gdb-patches
  Cc: George, Jini Susan, Parasuraman, Hariharan, Sharma, Alok Kumar

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

[AMD Official Use Only - General]

Thanks Andrew,

>> -static struct dynamic_prop_list *
>> +struct dynamic_prop_list *
>>  copy_dynamic_prop_list (struct obstack *objfile_obstack,
>>                       struct dynamic_prop_list *list)  {
>
>Please leave this as 'static', and just add a forward declaration within the
>gdbtypes.c.
>
>With that change, this is OK to commit.

Made the code changes suggested and attached the patch which I am planning to commit. 


Regards,
Rupesh P



>-----Original Message-----
>From: Andrew Burgess <aburgess@redhat.com>
>Sent: Friday, April 22, 2022 8:09 PM
>To: Potharla, Rupesh <Rupesh.Potharla@amd.com>; Tom Tromey
><tom@tromey.com>; Potharla, Rupesh via Gdb-patches <gdb-
>patches@sourceware.org>
>Cc: 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]
>
>"Potharla, Rupesh via Gdb-patches" <gdb-patches@sourceware.org> writes:
>
>> [AMD Official Use Only]
>>
>> Thanks, Andrew,
>>
>> Made suggested code changes and attached the updated patch. Requesting
>> to review the code changes.
>
>Thanks, this all looks fine to me, except for one small issue...
>
>> From fbfa2f4c26eeef7547603aafafb9525d66d1b172 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                            | 21 ++++++++++++++++-----
>>  gdb/gdbtypes.h                            |  4 +++-
>>  gdb/testsuite/gdb.fortran/assumedrank.exp |  6 ++++++
>>  gdb/testsuite/gdb.fortran/assumedrank.f90 |  3 +++
>>  4 files changed, 28 insertions(+), 6 deletions(-)
>>
>> diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c index
>> 49ecb199b07..47cf442a2d5 100644
>> --- a/gdb/gdbtypes.c
>> +++ b/gdb/gdbtypes.c
>> @@ -2398,10 +2398,21 @@ 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.  In this case the resolved type should not be an
>> +          array, but should instead be that of an array element.  */
>> +       struct type *dynamic_array_type = type;
>> +       type = copy_type (TYPE_TARGET_TYPE (dynamic_array_type));
>> +       struct dynamic_prop_list *prop_list
>> +         = TYPE_MAIN_TYPE (dynamic_array_type)->dyn_prop_list;
>> +       if (prop_list != nullptr)
>> +         {
>> +           struct obstack *obstack
>> +             = &type->objfile_owner ()->objfile_obstack;
>> +           TYPE_MAIN_TYPE (type)->dyn_prop_list
>> +             = copy_dynamic_prop_list (obstack, prop_list);
>> +         }
>> +       return type;
>>       }
>>        else if (type->code () == TYPE_CODE_STRING && rank != 1)
>>       {
>> @@ -5589,7 +5600,7 @@ create_copied_types_hash (struct objfile
>> *objfile)
>>
>>  /* Recursively copy (deep copy) a dynamic attribute list of a type.
>> */
>>
>> -static struct dynamic_prop_list *
>> +struct dynamic_prop_list *
>>  copy_dynamic_prop_list (struct obstack *objfile_obstack,
>>                       struct dynamic_prop_list *list)  {
>
>Please leave this as 'static', and just add a forward declaration within the
>gdbtypes.c.
>
>With that change, this is OK to commit.
>
>Thanks,
>Andrew
>
>> diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h index
>> 769328cc9cd..99273d14c1f 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
>> @@ -2882,6 +2881,9 @@ extern struct type *copy_type_recursive (struct
>objfile *objfile,
>>                                        struct type *type,
>>                                        htab_t copied_types);
>>
>> +extern struct dynamic_prop_list * copy_dynamic_prop_list
>> +  (struct obstack *objfile_obstack, struct dynamic_prop_list *list);
>> +
>>  extern struct type *copy_type (const struct type *type);
>>
>>  extern bool types_equal (struct type *, struct type *); diff --git
>> a/gdb/testsuite/gdb.fortran/assumedrank.exp
>> b/gdb/testsuite/gdb.fortran/assumedrank.exp
>> index 69cd168125f..e9429b44a9a 100644
>> --- a/gdb/testsuite/gdb.fortran/assumedrank.exp
>> +++ b/gdb/testsuite/gdb.fortran/assumedrank.exp
>> @@ -58,6 +58,12 @@ while { $test_count < 500 } {
>>           }
>>       }
>>
>> +     # Currently, flang does not support rank0.
>> +     if {$test_count == 1 && [test_compiler_info {clang-*}]} {
>> +        unsupported "compiler does not support rank 0"
>> +        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

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

From 91988817256be685a4b1ec7db7ef408492be6ff6 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                            | 22 ++++++++++++++++++----
 gdb/gdbtypes.h                            |  1 -
 gdb/testsuite/gdb.fortran/assumedrank.exp |  6 ++++++
 gdb/testsuite/gdb.fortran/assumedrank.f90 |  3 +++
 4 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 49ecb199b07..2a51372a037 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -47,6 +47,9 @@
 /* The value of an invalid conversion badness.  */
 #define INVALID_CONVERSION 100
 
+static struct dynamic_prop_list *
+copy_dynamic_prop_list (struct obstack *, struct dynamic_prop_list *);
+
 /* Initialize BADNESS constants.  */
 
 const struct rank LENGTH_MISMATCH_BADNESS = {INVALID_CONVERSION,0};
@@ -2398,10 +2401,21 @@ 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.  In this case the resolved type should not be an
+	     array, but should instead be that of an array element.  */
+	  struct type *dynamic_array_type = type;
+	  type = copy_type (TYPE_TARGET_TYPE (dynamic_array_type));
+	  struct dynamic_prop_list *prop_list
+	    = TYPE_MAIN_TYPE (dynamic_array_type)->dyn_prop_list;
+	  if (prop_list != nullptr)
+	    {
+	      struct obstack *obstack
+		= &type->objfile_owner ()->objfile_obstack;
+	      TYPE_MAIN_TYPE (type)->dyn_prop_list
+		= copy_dynamic_prop_list (obstack, prop_list);
+	    }
+	  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..e9429b44a9a 100644
--- a/gdb/testsuite/gdb.fortran/assumedrank.exp
+++ b/gdb/testsuite/gdb.fortran/assumedrank.exp
@@ -58,6 +58,12 @@ while { $test_count < 500 } {
 	    }
 	}
 
+	# Currently, flang does not support rank0.
+	if {$test_count == 1 && [test_compiler_info {clang-*}]} {
+	   unsupported "compiler does not support rank 0"
+	   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


^ permalink raw reply	[flat|nested] 21+ messages in thread

* RE: GDB/Fortran: Support for Assumed Rank Zero.
  2022-04-25  6:33                       ` Potharla, Rupesh
@ 2022-04-25  8:47                         ` Andrew Burgess
  0 siblings, 0 replies; 21+ messages in thread
From: Andrew Burgess @ 2022-04-25  8:47 UTC (permalink / raw)
  To: Potharla, Rupesh, Tom Tromey, Potharla, Rupesh via Gdb-patches
  Cc: George, Jini Susan, Parasuraman, Hariharan, Sharma, Alok Kumar

"Potharla, Rupesh" <Rupesh.Potharla@amd.com> writes:

> [AMD Official Use Only - General]
>
> Thanks Andrew,
>
>>> -static struct dynamic_prop_list *
>>> +struct dynamic_prop_list *
>>>  copy_dynamic_prop_list (struct obstack *objfile_obstack,
>>>                       struct dynamic_prop_list *list)  {
>>
>>Please leave this as 'static', and just add a forward declaration within the
>>gdbtypes.c.
>>
>>With that change, this is OK to commit.
>
> Made the code changes suggested and attached the patch which I am
> planning to commit.

LGTM.

Thanks,
Andrew

>
>
> Regards,
> Rupesh P
>
>
>
>>-----Original Message-----
>>From: Andrew Burgess <aburgess@redhat.com>
>>Sent: Friday, April 22, 2022 8:09 PM
>>To: Potharla, Rupesh <Rupesh.Potharla@amd.com>; Tom Tromey
>><tom@tromey.com>; Potharla, Rupesh via Gdb-patches <gdb-
>>patches@sourceware.org>
>>Cc: 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]
>>
>>"Potharla, Rupesh via Gdb-patches" <gdb-patches@sourceware.org> writes:
>>
>>> [AMD Official Use Only]
>>>
>>> Thanks, Andrew,
>>>
>>> Made suggested code changes and attached the updated patch. Requesting
>>> to review the code changes.
>>
>>Thanks, this all looks fine to me, except for one small issue...
>>
>>> From fbfa2f4c26eeef7547603aafafb9525d66d1b172 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                            | 21 ++++++++++++++++-----
>>>  gdb/gdbtypes.h                            |  4 +++-
>>>  gdb/testsuite/gdb.fortran/assumedrank.exp |  6 ++++++
>>>  gdb/testsuite/gdb.fortran/assumedrank.f90 |  3 +++
>>>  4 files changed, 28 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c index
>>> 49ecb199b07..47cf442a2d5 100644
>>> --- a/gdb/gdbtypes.c
>>> +++ b/gdb/gdbtypes.c
>>> @@ -2398,10 +2398,21 @@ 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.  In this case the resolved type should not be an
>>> +          array, but should instead be that of an array element.  */
>>> +       struct type *dynamic_array_type = type;
>>> +       type = copy_type (TYPE_TARGET_TYPE (dynamic_array_type));
>>> +       struct dynamic_prop_list *prop_list
>>> +         = TYPE_MAIN_TYPE (dynamic_array_type)->dyn_prop_list;
>>> +       if (prop_list != nullptr)
>>> +         {
>>> +           struct obstack *obstack
>>> +             = &type->objfile_owner ()->objfile_obstack;
>>> +           TYPE_MAIN_TYPE (type)->dyn_prop_list
>>> +             = copy_dynamic_prop_list (obstack, prop_list);
>>> +         }
>>> +       return type;
>>>       }
>>>        else if (type->code () == TYPE_CODE_STRING && rank != 1)
>>>       {
>>> @@ -5589,7 +5600,7 @@ create_copied_types_hash (struct objfile
>>> *objfile)
>>>
>>>  /* Recursively copy (deep copy) a dynamic attribute list of a type.
>>> */
>>>
>>> -static struct dynamic_prop_list *
>>> +struct dynamic_prop_list *
>>>  copy_dynamic_prop_list (struct obstack *objfile_obstack,
>>>                       struct dynamic_prop_list *list)  {
>>
>>Please leave this as 'static', and just add a forward declaration within the
>>gdbtypes.c.
>>
>>With that change, this is OK to commit.
>>
>>Thanks,
>>Andrew
>>
>>> diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h index
>>> 769328cc9cd..99273d14c1f 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
>>> @@ -2882,6 +2881,9 @@ extern struct type *copy_type_recursive (struct
>>objfile *objfile,
>>>                                        struct type *type,
>>>                                        htab_t copied_types);
>>>
>>> +extern struct dynamic_prop_list * copy_dynamic_prop_list
>>> +  (struct obstack *objfile_obstack, struct dynamic_prop_list *list);
>>> +
>>>  extern struct type *copy_type (const struct type *type);
>>>
>>>  extern bool types_equal (struct type *, struct type *); diff --git
>>> a/gdb/testsuite/gdb.fortran/assumedrank.exp
>>> b/gdb/testsuite/gdb.fortran/assumedrank.exp
>>> index 69cd168125f..e9429b44a9a 100644
>>> --- a/gdb/testsuite/gdb.fortran/assumedrank.exp
>>> +++ b/gdb/testsuite/gdb.fortran/assumedrank.exp
>>> @@ -58,6 +58,12 @@ while { $test_count < 500 } {
>>>           }
>>>       }
>>>
>>> +     # Currently, flang does not support rank0.
>>> +     if {$test_count == 1 && [test_compiler_info {clang-*}]} {
>>> +        unsupported "compiler does not support rank 0"
>>> +        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
> From 91988817256be685a4b1ec7db7ef408492be6ff6 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                            | 22 ++++++++++++++++++----
>  gdb/gdbtypes.h                            |  1 -
>  gdb/testsuite/gdb.fortran/assumedrank.exp |  6 ++++++
>  gdb/testsuite/gdb.fortran/assumedrank.f90 |  3 +++
>  4 files changed, 27 insertions(+), 5 deletions(-)
>
> diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
> index 49ecb199b07..2a51372a037 100644
> --- a/gdb/gdbtypes.c
> +++ b/gdb/gdbtypes.c
> @@ -47,6 +47,9 @@
>  /* The value of an invalid conversion badness.  */
>  #define INVALID_CONVERSION 100
>  
> +static struct dynamic_prop_list *
> +copy_dynamic_prop_list (struct obstack *, struct dynamic_prop_list *);
> +
>  /* Initialize BADNESS constants.  */
>  
>  const struct rank LENGTH_MISMATCH_BADNESS = {INVALID_CONVERSION,0};
> @@ -2398,10 +2401,21 @@ 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.  In this case the resolved type should not be an
> +	     array, but should instead be that of an array element.  */
> +	  struct type *dynamic_array_type = type;
> +	  type = copy_type (TYPE_TARGET_TYPE (dynamic_array_type));
> +	  struct dynamic_prop_list *prop_list
> +	    = TYPE_MAIN_TYPE (dynamic_array_type)->dyn_prop_list;
> +	  if (prop_list != nullptr)
> +	    {
> +	      struct obstack *obstack
> +		= &type->objfile_owner ()->objfile_obstack;
> +	      TYPE_MAIN_TYPE (type)->dyn_prop_list
> +		= copy_dynamic_prop_list (obstack, prop_list);
> +	    }
> +	  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..e9429b44a9a 100644
> --- a/gdb/testsuite/gdb.fortran/assumedrank.exp
> +++ b/gdb/testsuite/gdb.fortran/assumedrank.exp
> @@ -58,6 +58,12 @@ while { $test_count < 500 } {
>  	    }
>  	}
>  
> +	# Currently, flang does not support rank0.
> +	if {$test_count == 1 && [test_compiler_info {clang-*}]} {
> +	   unsupported "compiler does not support rank 0"
> +	   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


^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2022-04-25  8:47 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-13  9:55 GDB/Fortran: Support for Assumed Rank Zero Potharla, Rupesh
2022-04-13 18:27 ` Kevin Buettner
2022-04-14 10:30   ` Potharla, Rupesh
2022-04-14 21:28     ` Kevin Buettner
2022-04-15 13:33       ` Potharla, Rupesh
2022-04-15 19:31         ` Kevin Buettner
2022-04-16 18:29           ` Potharla, Rupesh
2022-04-18 13:31             ` Tom Tromey
2022-04-18 15:25               ` Potharla, Rupesh
2022-04-20 15:22                 ` Andrew Burgess
2022-04-20 19:08                   ` Potharla, Rupesh
2022-04-22 14:38                     ` Andrew Burgess
2022-04-25  6:33                       ` Potharla, Rupesh
2022-04-25  8:47                         ` Andrew Burgess
2022-04-18 15:33             ` Kevin Buettner
2022-04-19 17:30               ` Kevin Buettner
2022-04-20  0:29                 ` Potharla, Rupesh
2022-04-20  6:32                   ` Kempke, Nils-Christian
2022-04-20 15:38                     ` Kevin Buettner
2022-04-20 15:29                   ` Andrew Burgess
2022-04-20 19:20                     ` Potharla, Rupesh

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).