[PATCH] Multi-dimensional Fortran arrays issue PR11104

[PATCH] Multi-dimensional Fortran arrays issue PR11104

[Andrew Burgess]

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

The attached patch fixes issue PR11104 (http://sourceware.org/bugzilla/show_bug.cgi?id=11104 ), relating to printing multidimensional Fortran arrays.

This has also been mentioned on the gdb list recently: http://sourceware.org/ml/gdb/2010-09/msg00004.html

The tar file contains a test case for this issue, it adds two files to gdb/testsuite/gdb.fortran .

If people think that it looks sane then could it be applied please, or let me know if there are any changes needed, I've not submitted many patches before.


Thanks,
Andrew



[-- Attachment #2: arrays2.patch --]
[-- Type: application/octet-stream, Size: 4128 bytes --]

Index: gdb/eval.c
===================================================================
RCS file: /cvs/src/src/gdb/eval.c,v
retrieving revision 1.139
diff -c -p -r1.139 eval.c
*** gdb/eval.c	11 Aug 2010 16:48:26 -0000	1.139
--- gdb/eval.c	16 Sep 2010 07:54:41 -0000
*************** evaluate_subexp_standard (struct type *e
*** 2306,2321 ****
  
  	    subscript_array[nargs - i - 1] -= lower;
  
! 	    /* If we are at the bottom of a multidimensional 
! 	       array type then keep a ptr to the last ARRAY
! 	       type around for use when calling value_subscript()
! 	       below. This is done because we pretend to value_subscript
! 	       that we actually have a one-dimensional array 
! 	       of base element type that we apply a simple 
! 	       offset to. */
! 
! 	    if (i < nargs - 1)
! 	      tmp_type = check_typedef (TYPE_TARGET_TYPE (tmp_type));
  	  }
  
  	/* Now let us calculate the offset for this item */
--- 2306,2312 ----
  
  	    subscript_array[nargs - i - 1] -= lower;
  
! 	    tmp_type = check_typedef (TYPE_TARGET_TYPE (tmp_type));
  	  }
  
  	/* Now let us calculate the offset for this item */
*************** evaluate_subexp_standard (struct type *e
*** 2326,2339 ****
  	  offset_item =
  	    array_size_array[i - 1] * offset_item + subscript_array[i - 1];
  
- 	/* Let us now play a dirty trick: we will take arg1 
- 	   which is a value node pointing to the topmost level
- 	   of the multidimensional array-set and pretend
- 	   that it is actually a array of the final element 
- 	   type, this will ensure that value_subscript()
- 	   returns the correct type value */
- 
- 	deprecated_set_value_type (arg1, tmp_type);
  	return value_subscripted_rvalue (arg1, offset_item, 0);
        }
  
--- 2317,2322 ----
Index: gdb/valarith.c
===================================================================
RCS file: /cvs/src/src/gdb/valarith.c,v
retrieving revision 1.86
diff -c -p -r1.86 valarith.c
*** gdb/valarith.c	11 Aug 2010 16:48:26 -0000	1.86
--- gdb/valarith.c	16 Sep 2010 07:54:42 -0000
*************** struct value *
*** 193,202 ****
  value_subscripted_rvalue (struct value *array, LONGEST index, int lowerbound)
  {
    struct type *array_type = check_typedef (value_type (array));
!   struct type *elt_type = check_typedef (TYPE_TARGET_TYPE (array_type));
!   unsigned int elt_size = TYPE_LENGTH (elt_type);
!   unsigned int elt_offs = elt_size * longest_to_int (index - lowerbound);
    struct value *v;
  
    if (index < lowerbound || (!TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (array_type)
  			     && elt_offs >= TYPE_LENGTH (array_type)))
--- 193,211 ----
  value_subscripted_rvalue (struct value *array, LONGEST index, int lowerbound)
  {
    struct type *array_type = check_typedef (value_type (array));
!   struct type *elt_type = array_type; 
!   unsigned int elt_size, elt_offs;
    struct value *v;
+   
+   /* Peel of the array indices until we reach the array element type */
+   do {
+     elt_type = TYPE_TARGET_TYPE(elt_type);
+   }
+   while ( TYPE_CODE(elt_type) == TYPE_CODE_ARRAY);
+ 
+   elt_type = check_typedef(elt_type);
+   elt_size = TYPE_LENGTH (elt_type);
+   elt_offs = elt_size * longest_to_int (index - lowerbound);
  
    if (index < lowerbound || (!TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (array_type)
  			     && elt_offs >= TYPE_LENGTH (array_type)))
Index: ChangeLog
===================================================================
RCS file: /cvs/src/src/ChangeLog,v
retrieving revision 1.928
diff -c -p -r1.928 ChangeLog
*** ChangeLog	18 Jul 2010 08:12:39 -0000	1.928
--- ChangeLog	16 Sep 2010 08:00:41 -0000
***************
*** 1,3 ****
--- 1,13 ----
+ 2010-09-16 Andrew Burgess <aburgess@broadcom.com>
+ 
+  	* valarith.c (value_subscripted_rvalue) Walk through
+  	multi-dimensional arrays to find the element type for the
+  	array. Allows the upper bound check to work with multi-dimensional
+  	arrays.
+  	* eval.c (evaluate_subexp_standard) Remove hack from
+  	multi_f77_subscript case now that multi-dimensional arrays are
+  	supported in valarith.c
+ 
  2010-07-17  Jack Howarth  <howarth@bromo.med.uc.edu>
  
  	PR target/44862

[-- Attachment #3: multi-dim.tar.bz2 --]
[-- Type: application/octet-stream, Size: 1446 bytes --]

RE: [PATCH] Multi-dimensional Fortran arrays issue PR11104

[Andrew Burgess]

Hi,

Was wondering if there was anything else I should do to get this patch considered for inclusion? I'm happy to make any changes required; this is my first gdb patch.

Thanks,
Andrew


> -----Original Message-----
> From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] On Behalf Of Andrew Burgess
> Sent: 16 September 2010 09:59
> To: gdb-patches@sourceware.org
> Subject: [PATCH] Multi-dimensional Fortran arrays issue PR11104
> 
> The attached patch fixes issue PR11104
> (http://sourceware.org/bugzilla/show_bug.cgi?id=11104 ), relating to
> printing multidimensional Fortran arrays.
> 
> This has also been mentioned on the gdb list recently:
> http://sourceware.org/ml/gdb/2010-09/msg00004.html
> 
> The tar file contains a test case for this issue, it adds two files to
> gdb/testsuite/gdb.fortran .
> 
> If people think that it looks sane then could it be applied please, or
> let me know if there are any changes needed, I've not submitted many
> patches before.
> 
> 
> Thanks,
> Andrew
> 

Re: [PATCH] Multi-dimensional Fortran arrays issue PR11104

[Jan Kratochvil]

On Mon, 18 Oct 2010 09:53:03 +0200, Andrew Burgess wrote:
> Was wondering if there was anything else I should do to get this patch
> considered for inclusion? I'm happy to make any changes required; this is my
> first gdb patch.

multi-dim.tar.bz2 (.exp + .f90) should be submitted as a normal patch, you can
at least do `diff -u /dev/null gdb/testsuite/gdb.fortran/file.exp'.

ChangeLog should be submitted as a text, not as patch.

There are several minor issues with formatting:
	http://www.gnu.org/prep/standards/

(I have not yet checked the real code of the patch.)


This functionality is implemented by archer-jankratochvil-vla from:
	http://sourceware.org/gdb/wiki/ArcherBranchManagement

(out of bounds access is not detected by the VLA patchset though)

which is currently not submitted for a review as it has more prerequisites.


Regards,
Jan

RE: [PATCH] Multi-dimensional Fortran arrays issue PR11104

[Andrew Burgess]

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

Jan, thanks for taking the time to look through the original patch.

> On 18 October 2010 22:20, Jan Kratochvil wrote:
> 
> multi-dim.tar.bz2 (.exp + .f90) should be submitted as a normal patch,

Done.

> 
> ChangeLog should be submitted as a text, not as patch.
> 

I've included the ChangeLog entries within the patch file as plain text before the start of that patch content, this seems to be what others have done, but I can pull them out if this is not correct.

> There are several minor issues with formatting:
> 	http://www.gnu.org/prep/standards/

I believe that these have all been addressed. If I've still missed anything then please let me know.

> 
> This functionality is implemented by archer-jankratochvil-vla from:
> 	http://sourceware.org/gdb/wiki/ArcherBranchManagement
> 
> (out of bounds access is not detected by the VLA patchset though)
> 
> which is currently not submitted for a review as it has more
> prerequisites.

As I had the patch pretty much ready I cleaned it up anyway. I guess it's up to someone else to decide to merge this or wait for the Archer branch to land.

Thanks,

Andrew




[-- Attachment #2: arrays.test.patch --]
[-- Type: application/octet-stream, Size: 4232 bytes --]

gdb/testsuite/ChangeLog:

2010-10-19  Andrew Burgess  <aburgess@broadcom.com>

	* gdb.fortran/multi-dim.f90: New test program.
	* gdb.fortran/multi-dim.exp: Run new test to check basic printing
          of multi-dimensional fortran arrays. Tests PR gdb/11104.


diff -uNr clean/gdb-7.2.50.20101019/gdb/testsuite/gdb.fortran/multi-dim.exp working/gdb-7.2.50.20101019/gdb/testsuite/gdb.fortran/multi-dim.exp
--- clean/gdb-7.2.50.20101019/gdb/testsuite/gdb.fortran/multi-dim.exp	1970-01-01 01:00:00.000000000 +0100
+++ working/gdb-7.2.50.20101019/gdb/testsuite/gdb.fortran/multi-dim.exp	2010-10-19 15:53:13.928699000 +0100
@@ -0,0 +1,79 @@
+# Copyright 2010 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# This file was written by Wu Zhou. (woodzltc@cn.ibm.com)
+
+# This file is part of the gdb testsuite.  It contains tests for evaluating
+# Fortran subarray expression.
+
+if $tracelevel then {
+	strace $tracelevel
+}
+
+if { [skip_fortran_tests] } { return -1 }
+
+set testfile "multi-dim"
+set srcfile ${testfile}.f90
+set binfile ${objdir}/${subdir}/${testfile}
+
+if {[gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
+	executable {debug f77}] != ""} {
+    return -1
+}
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+if ![runto MAIN__] then {
+    perror "Couldn't run to MAIN__"
+    continue
+}
+
+gdb_test "break 20" \
+    "Breakpoint.*at.* file .*$srcfile, line 20\\." \
+    "breakpoint at line 20"
+
+gdb_test "continue" \
+    "Breakpoint 2, test \(\).*at.*multi-dim\.f90:20.*" \
+    "Continue to breakpoint"
+
+gdb_test "print foo(2,3,4)" \
+    "\\\$1 = 20" \
+    "print the contents of the array"
+
+gdb_test "print foo(0,0,0)" \
+    "no such vector element" \
+    "print an invalid array index (0,0,0)"
+
+gdb_test "print foo(2,3,5)" \
+    "no such vector element" \
+    "print an invalid array index (2,3,5)"
+
+gdb_test "print foo(2,4,4)" \
+    "no such vector element" \
+    "print an invalid array index (2,4,4)"
+
+gdb_test "print foo(3,3,4)" \
+    "no such vector element" \
+    "print an invalid array index (3,3,4)"
+
+gdb_test "print foo" \
+    "\\\$2 = \\(\\( \\( 10, 10\\) \\( 10, 10\\) \\( 10, 10\\) \\) \\( \\( 10, 10\\) \\( 10, 10\\) \\( 10, 10\\) \\) \\( \\( 10, 10\\) \\( 10, 10\\) \\( 10, 10\\) \\) \\( \\( 10, 10\\) \\( 10, 10\\) \\( 10, 20\\) \\) \\)" \
+    "print full contents of the array"
+
+gdb_exit
+
diff -uNr clean/gdb-7.2.50.20101019/gdb/testsuite/gdb.fortran/multi-dim.f90 working/gdb-7.2.50.20101019/gdb/testsuite/gdb.fortran/multi-dim.f90
--- clean/gdb-7.2.50.20101019/gdb/testsuite/gdb.fortran/multi-dim.f90	1970-01-01 01:00:00.000000000 +0100
+++ working/gdb-7.2.50.20101019/gdb/testsuite/gdb.fortran/multi-dim.f90	2010-10-19 15:53:13.972696000 +0100
@@ -0,0 +1,21 @@
+! Copyright 2010 Free Software Foundation, Inc.
+!
+! This program is free software; you can redistribute it and/or modify
+! it under the terms of the GNU General Public License as published by
+! the Free Software Foundation; either version 3 of the License, or
+! (at your option) any later version.
+! 
+! This program is distributed in the hope that it will be useful,
+! but WITHOUT ANY WARRANTY; without even the implied warranty of
+! MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+! GNU General Public License for more details.
+! 
+! You should have received a copy of the GNU General Public License
+! along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+program test
+  integer :: foo (2, 3, 4)
+  foo (:, :, :) = 10
+  foo (2, 3, 4) = 20
+  foo (2, 3, 4) = 20
+end

[-- Attachment #3: arrays.patch --]
[-- Type: application/octet-stream, Size: 3113 bytes --]

gdb/ChangeLog:

2010-10-19  Andrew Burgess  <aburgess@broadcom.com>

  	* valarith.c (value_subscripted_rvalue) Walk through
  	multi-dimensional arrays to find the element type for the
  	array. Allows the upper bound check to work with multi-dimensional
  	arrays. Fix PR gdb/11104.
  	* eval.c (evaluate_subexp_standard) Remove hack from
  	multi_f77_subscript case now that multi-dimensional arrays are
  	supported.  Fix PR gdb/11104.


Index: gdb/eval.c
===================================================================
RCS file: /cvs/src/src/gdb/eval.c,v
retrieving revision 1.139
diff -u -r1.139 eval.c
--- gdb/eval.c	11 Aug 2010 16:48:26 -0000	1.139
+++ gdb/eval.c	19 Oct 2010 08:15:37 -0000
@@ -2306,16 +2306,7 @@
 
 	    subscript_array[nargs - i - 1] -= lower;
 
-	    /* If we are at the bottom of a multidimensional 
-	       array type then keep a ptr to the last ARRAY
-	       type around for use when calling value_subscript()
-	       below. This is done because we pretend to value_subscript
-	       that we actually have a one-dimensional array 
-	       of base element type that we apply a simple 
-	       offset to. */
-
-	    if (i < nargs - 1)
-	      tmp_type = check_typedef (TYPE_TARGET_TYPE (tmp_type));
+	    tmp_type = check_typedef (TYPE_TARGET_TYPE (tmp_type));
 	  }
 
 	/* Now let us calculate the offset for this item */
@@ -2326,14 +2317,6 @@
 	  offset_item =
 	    array_size_array[i - 1] * offset_item + subscript_array[i - 1];
 
-	/* Let us now play a dirty trick: we will take arg1 
-	   which is a value node pointing to the topmost level
-	   of the multidimensional array-set and pretend
-	   that it is actually a array of the final element 
-	   type, this will ensure that value_subscript()
-	   returns the correct type value */
-
-	deprecated_set_value_type (arg1, tmp_type);
 	return value_subscripted_rvalue (arg1, offset_item, 0);
       }
 
Index: gdb/valarith.c
===================================================================
RCS file: /cvs/src/src/gdb/valarith.c,v
retrieving revision 1.87
diff -u -r1.87 valarith.c
--- gdb/valarith.c	8 Oct 2010 16:50:53 -0000	1.87
+++ gdb/valarith.c	19 Oct 2010 08:15:40 -0000
@@ -193,10 +193,20 @@
 value_subscripted_rvalue (struct value *array, LONGEST index, int lowerbound)
 {
   struct type *array_type = check_typedef (value_type (array));
-  struct type *elt_type = check_typedef (TYPE_TARGET_TYPE (array_type));
-  unsigned int elt_size = TYPE_LENGTH (elt_type);
-  unsigned int elt_offs = elt_size * longest_to_int (index - lowerbound);
+  struct type *elt_type = array_type; 
+  unsigned int elt_size, elt_offs;
   struct value *v;
+  
+  /* Peel of the array indices until we reach the array element type */
+  do
+    {
+      elt_type = TYPE_TARGET_TYPE (elt_type);
+    }
+  while (TYPE_CODE (elt_type) == TYPE_CODE_ARRAY);
+
+  elt_type = check_typedef (elt_type);
+  elt_size = TYPE_LENGTH (elt_type);
+  elt_offs = elt_size * longest_to_int (index - lowerbound);
 
   if (index < lowerbound || (!TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (array_type)
 			     && elt_offs >= TYPE_LENGTH (array_type)))

RE: [PATCH] Multi-dimensional Fortran arrays issue PR11104

[Andrew Burgess]

Hi,

If anyone has the time please could this patch be reviewed. I realise that there's an overlap with some of the code currently in the Archer VLA branch, but as this keeps some of the bounds checking in place I hoped this might be acceptable.
The revised patch includes the ChangeLog entry as text ahead of the patch content, I've since been told that's not the preferred format and will split it out if that would make people life easier.
I'm happy to make any changes requested.

At the time I submitted the patch I built & tested on x86-64 with no regressions.

Thanks,

Andrew




> -----Original Message-----
> From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] On Behalf Of Andrew Burgess
> Sent: 19 October 2010 17:23
> To: gdb-patches@sourceware.org
> Cc: Jan Kratochvil
> Subject: RE: [PATCH] Multi-dimensional Fortran arrays issue PR11104
> 
> Jan, thanks for taking the time to look through the original patch.
> 
> > On 18 October 2010 22:20, Jan Kratochvil wrote:
> >
> > multi-dim.tar.bz2 (.exp + .f90) should be submitted as a normal
> patch,
> 
> Done.
> 
> >
> > ChangeLog should be submitted as a text, not as patch.
> >
> 
> I've included the ChangeLog entries within the patch file as plain text
> before the start of that patch content, this seems to be what others
> have done, but I can pull them out if this is not correct.
> 
> > There are several minor issues with formatting:
> > 	http://www.gnu.org/prep/standards/
> 
> I believe that these have all been addressed. If I've still missed
> anything then please let me know.
> 
> >
> > This functionality is implemented by archer-jankratochvil-vla from:
> > 	http://sourceware.org/gdb/wiki/ArcherBranchManagement
> >
> > (out of bounds access is not detected by the VLA patchset though)
> >
> > which is currently not submitted for a review as it has more
> > prerequisites.
> 
> As I had the patch pretty much ready I cleaned it up anyway. I guess
> it's up to someone else to decide to merge this or wait for the Archer
> branch to land.
> 
> Thanks,
> 
> Andrew
> 
> 

Re: [PATCH] Multi-dimensional Fortran arrays issue PR11104

[Jan Kratochvil]

On Wed, 17 Nov 2010 09:44:56 +0100, Andrew Burgess wrote:
> tested on x86-64 with no regressions.

There is a regression on {x86_64,x86_64-m32,i686}-fedora14-linux-gnu:
	-PASS: gdb.python/py-value.exp: Test multiple subscript
	+FAIL: gdb.python/py-value.exp: Test multiple subscript
	-PASS: gdb.python/py-value.exp: Test multiple subscript
	+FAIL: gdb.python/py-value.exp: Test multiple subscript


This is not a real review, just some simple parts before I get to the core.


> +  /* Peel of the array indices until we reach the array element type */
> +  do
> +    {
> +      elt_type = TYPE_TARGET_TYPE (elt_type);
> +    }
> +  while (TYPE_CODE (elt_type) == TYPE_CODE_ARRAY);

Here should be check_typedef during each peel.  I do not have a break
reproducer but at least fpc-2.4.2-0.1.rc1.fc15.x86_64 produces such
typedef-wrapped arrays and they are valid DWARF:
	program p;
	type a = array[1..2] of integer;
	type b = array[1..2] of a;
	var
	  v : b;
	begin
	  v[1, 1] := 1;
	  writeln (v[1, 1]);
	end.

Just Pascal does not use this function but anyway.


> +set testfile "multi-dim"
> +set srcfile ${testfile}.f90
> +set binfile ${objdir}/${subdir}/${testfile}
> +
> +if {[gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
> +	executable {debug f77}] != ""} {
> +    return -1
> +}
> +
> +gdb_exit
> +gdb_start
> +gdb_reinitialize_dir $srcdir/$subdir
> +gdb_load ${binfile}

Here could be prepare_for_testing (OK, not used everywhere now).


> +gdb_test "break 20" \
> +    "Breakpoint.*at.* file .*$srcfile, line 20\\." \
> +    "breakpoint at line 20"

Here should be gdb_get_line_number call as hardcoded line numbers (20) make
later updates difficult.


> +gdb_test "continue" \
> +    "Breakpoint 2, test \(\).*at.*multi-dim\.f90:20.*" \
> +    "Continue to breakpoint"
[...]
> +gdb_test "print foo" \
> +    "\\\$2 = \\(\\( \\( 10, 10\\) \\( 10, 10\\) \\( 10, 10\\) \\) \\( \\( 10, 10\\) \\( 10, 10\\) \\( 10, 10\\) \\) \\( \\( 10, 10\\) \\( 10, 10\\) \\( 10, 10\\) \\) \\( \\( 10, 10\\) \\( 10, 10\\) \\( 10, 20\\) \\) \\)" \

Hardcoded breakpoint and or output ($2) numbers also make later updates
difficult.


> +gdb_exit

Not needed at the testfile end.


Thanks,
Jan

Re: [PATCH] Multi-dimensional Fortran arrays issue PR11104

[Jan Kratochvil]

On Wed, 17 Nov 2010 09:44:56 +0100, Andrew Burgess wrote:
> I'm happy to make any changes requested.

I find there a problem it does not check all the bounds like the compiler does.
I did not try to fix it up myself to say more.  Sorry for so late review.

gdb_test "print foo(0,4,4)" \
    "no such vector element" \
    "print an invalid array index (0,4,4)"

patched 7.2.50.20101123-cvs:
print foo(0,4,4)
$2 = 20
(gdb) FAIL: gdb.fortran/multi-dim.exp: print an invalid array index (0,4,4)

gcc-gfortran-4.5.1-4.fc14.x86_64:
  foo (0, 4, 4) = 20
Warning: Array reference at (1) is out of bounds (0 < 1) in dimension 1
Warning: Array reference at (1) is out of bounds (4 > 3) in dimension 2


Thanks,
Jan

Re: [PATCH] Multi-dimensional Fortran arrays issue PR11104

[Andrew Burgess]

On 23/11/2010 08:02, Jan Kratochvil wrote:
> On Wed, 17 Nov 2010 09:44:56 +0100, Andrew Burgess wrote:
>> I'm happy to make any changes requested.
>
> I find there a problem it does not check all the bounds like the compiler does.
> I did not try to fix it up myself to say more.  Sorry for so late review.

Thanks for taking the time to review this patch. The previous regression 
you pointed out (relating to the python interface) already led me to 
this same issue. Below is a revised patch that I think should solve this 
issue.

I've not had time to look at the tests yet, but will try to do that in 
the next day or so.

Thanks,
Andrew





--- gdb-7.2.50.20101122_clean/gdb/eval.c	2010-11-05 14:31:27.000000000 +0000
+++ gdb-7.2.50.20101122/gdb/eval.c	2010-11-23 09:57:57.703480325 +0000
@@ -2324,15 +2324,12 @@
      multi_f77_subscript:
        {
  	int subscript_array[MAX_FORTRAN_DIMS];
-	int array_size_array[MAX_FORTRAN_DIMS];
  	int ndimensions = 1, i;
-	struct type *tmp_type;
-	int offset_item;	/* The array offset where the item lives */
+	struct value *array = arg1;

  	if (nargs > MAX_FORTRAN_DIMS)
  	  error (_("Too many subscripts for F77 (%d Max)"), MAX_FORTRAN_DIMS);

-	tmp_type = check_typedef (value_type (arg1));
  	ndimensions = calc_f77_array_dims (type);

  	if (nargs != ndimensions)
@@ -2343,59 +2340,34 @@
  	/* Now that we know we have a legal array subscript expression
  	   let us actually find out where this element exists in the array. */

-	offset_item = 0;
  	/* Take array indices left to right */
  	for (i = 0; i < nargs; i++)
  	  {
  	    /* Evaluate each subscript, It must be a legal integer in F77 */
  	    arg2 = evaluate_subexp_with_coercion (exp, pos, noside);

-	    /* Fill in the subscript and array size arrays */
-
+	    /* Fill in the subscript array */
  	    subscript_array[i] = value_as_long (arg2);
  	  }

  	/* Internal type of array is arranged right to left */
-	for (i = 0; i < nargs; i++)
+	for (i = nargs; i > 0; i--)
  	  {
-	    upper = f77_get_upperbound (tmp_type);
-	    lower = f77_get_lowerbound (tmp_type);
+	    struct type* array_type = check_typedef (value_type (array));
+	    int offset = subscript_array[i - 1];
+	    upper = f77_get_upperbound (array_type);
+	    lower = f77_get_lowerbound (array_type);

-	    array_size_array[nargs - i - 1] = upper - lower + 1;
+	    if ( (offset < lower) || (offset > upper) )
+	      error (_("Subscript number %d out of bounds, range %d to %d"), 
i, lower, upper);

  	    /* Zero-normalize subscripts so that offsetting will work. */
+	    offset -= lower;

-	    subscript_array[nargs - i - 1] -= lower;
-
-	    /* If we are at the bottom of a multidimensional
-	       array type then keep a ptr to the last ARRAY
-	       type around for use when calling value_subscript()
-	       below. This is done because we pretend to value_subscript
-	       that we actually have a one-dimensional array
-	       of base element type that we apply a simple
-	       offset to. */
-
-	    if (i < nargs - 1)
-	      tmp_type = check_typedef (TYPE_TARGET_TYPE (tmp_type));
+	    array = value_subscripted_rvalue (array, offset, 0);
  	  }

-	/* Now let us calculate the offset for this item */
-
-	offset_item = subscript_array[ndimensions - 1];
-
-	for (i = ndimensions - 1; i > 0; --i)
-	  offset_item =
-	    array_size_array[i - 1] * offset_item + subscript_array[i - 1];
-
-	/* Let us now play a dirty trick: we will take arg1
-	   which is a value node pointing to the topmost level
-	   of the multidimensional array-set and pretend
-	   that it is actually a array of the final element
-	   type, this will ensure that value_subscript()
-	   returns the correct type value */
-
-	deprecated_set_value_type (arg1, tmp_type);
-	return value_subscripted_rvalue (arg1, offset_item, 0);
+	return array;
        }

      case BINOP_LOGICAL_AND:

Re: [PATCH] Multi-dimensional Fortran arrays issue PR11104

[Jan Kratochvil]

On Tue, 23 Nov 2010 11:04:43 +0100, Andrew Burgess wrote:
> Below is a revised patch that I think should solve this issue.

It has a regression for:

        dimension a(10)
        call sub(a,10)
        end
        subroutine sub(a,n)
        dimension a(n)
        a(5) = 5
        end

(gdb) b 6
(gdb) run
(gdb) p a(5)
Subscript number 1 out of bounds, range 1 to 1


Thanks,
Jan

Re: [PATCH] Multi-dimensional Fortran arrays issue PR11104

[Jan Kratochvil]

On Tue, 23 Nov 2010 18:23:24 +0100, Jan Kratochvil wrote:
>         dimension a(10)
>         call sub(a,10)
>         end
>         subroutine sub(a,n)
>         dimension a(n)
>         a(5) = 5
>         end
> 
> (gdb) b 6
> (gdb) run
> (gdb) p a(5)
> Subscript number 1 out of bounds, range 1 to 1

BTW I intended to show it for arrays with undefined length such as:

        dimension a(10)
        call sub(a,10)
        end
        subroutine sub(a)
        dimension a(*)
        a(5) = 5
        end

FSF GDB recognizes the dynamic length (former example) as undefined length
	type = real(kind=4) (*)
while FSF GDB recognizes the undefined length (latter example) as -1
	type = real(kind=4) (-1)
which does not work in any case.

For VLA GDB the former example gets the correct length
	type = real(kind=4) (10)
while VLA GDB recognizes the undefined length correctly:
	type = real(kind=4) (*)

So that your patch could be compatible with both.


Thanks,
Jan

Re: [PATCH] Multi-dimensional Fortran arrays issue PR11104

[Jan Kratochvil]

On Mon, 18 Oct 2010 09:53:03 +0200, Andrew Burgess wrote:
> Was wondering if there was anything else I should do to get this patch
> considered for inclusion?

The patch should be also provided by `diff -u'.


Thanks,
Jan