[PATCH/RFA] PR gdb/648

[PATCH/RFA] PR gdb/648

[David Lecomber]

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

This is a fix for a long-standing bug with multi-dimensional arrays in
fortran.  When printing individual elements of an array,
evaluate_subexp_standard understood the dimensions of the array to be in
the opposite order to that of the whatis or entire array printing.  This
led to bug 648, which was present for G77 compiled code, but the problem
was reversed for commercial compilers such as Intel or Portland.

G77 puts things in row-major order, as far as I can ascertain all other
fortran compilers do column major.  This is handled in the dwarf2read
change, and now column major array types are reversed during the reading
to fit with GDB's struct type.

2004-08-06  David Lecomber  <dsl@sources.redhat.com>
 
        Fix PR gdb/648
        * dwarf2read.c (read_array_type): Handle column major arrays
        correctly.  Assume column major for Fortran except with G77
        compiler.
        * eval.c (evaluate_subexp_standard): Assume Fortran arrays are
        oriented large to small in type structure.


Attached is a test program and test script, should return true for all 
comparisons..  

I'm sure there'll be some comments, so please feel free to suggest some
reformatting!

David.


[-- Attachment #2: parts.patches --]
[-- Type: text/x-patch, Size: 5862 bytes --]

Index: gdb/dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.156
diff -c -p -r1.156 dwarf2read.c
*** gdb/dwarf2read.c	6 Jul 2004 19:29:30 -0000	1.156
--- gdb/dwarf2read.c	28 Jul 2004 13:03:05 -0000
*************** read_array_type (struct die_info *die, s
*** 3718,3726 ****
  
    /* Dwarf2 dimensions are output from left to right, create the
       necessary array types in backwards order.  */
    type = element_type;
!   while (ndim-- > 0)
!     type = create_array_type (NULL, type, range_types[ndim]);
  
    /* Understand Dwarf2 support for vector types (like they occur on
       the PowerPC w/ AltiVec).  Gcc just adds another attribute to the
--- 3720,3751 ----
  
    /* Dwarf2 dimensions are output from left to right, create the
       necessary array types in backwards order.  */
+ 
+   /* Take account of array ordering, if possible.  */
    type = element_type;
! 
!   attr = dwarf2_attr (die, DW_AT_ordering, cu);
! 
!   /* Trust this attribute if present.  If not, assume col-major for FORTRAN
!      unless the compiler is GNU F77 which puts things the opposite way to the
!      dwarf2 spec.  
! 
!      FIXME: dsl/2004-07-20: If G77 is ever fixed by checking version string of
!      producer.
!    */
!   if ((attr && (DW_SND(attr) == DW_ORD_col_major))
!       || (!attr && ( cu->language == language_fortran && 
! 		     ( !cu->producer || !strstr(cu->producer, "GNU F77" )))))
!     {
!       int i = 0;
!       while (i < ndim) 
! 	type = create_array_type (NULL, type, range_types[i++]);
!     }
!   else 
!     {
!       while (ndim-- > 0)
! 	type = create_array_type (NULL, type, range_types[ndim]);
!     }
  
    /* Understand Dwarf2 support for vector types (like they occur on
       the PowerPC w/ AltiVec).  Gcc just adds another attribute to the
Index: gdb/eval.c
===================================================================
RCS file: /cvs/src/src/gdb/eval.c,v
retrieving revision 1.41
diff -c -p -r1.41 eval.c
*** gdb/eval.c	8 Apr 2004 21:18:12 -0000	1.41
--- gdb/eval.c	28 Jul 2004 13:03:07 -0000
*************** evaluate_subexp_standard (struct type *e
*** 1610,1618 ****
  
      multi_f77_subscript:
        {
! 	int subscript_array[MAX_FORTRAN_DIMS + 1];	/* 1-based array of 
! 							   subscripts, max == 7 */
! 	int array_size_array[MAX_FORTRAN_DIMS + 1];
  	int ndimensions = 1, i;
  	struct type *tmp_type;
  	int offset_item;	/* The array offset where the item lives */
--- 1610,1617 ----
  
      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 */
*************** evaluate_subexp_standard (struct type *e
*** 1630,1636 ****
  	   let us actually find out where this element exists in the array. */
  
  	offset_item = 0;
! 	for (i = 1; i <= nargs; i++)
  	  {
  	    /* Evaluate each subscript, It must be a legal integer in F77 */
  	    arg2 = evaluate_subexp_with_coercion (exp, pos, noside);
--- 1629,1636 ----
  	   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);
*************** evaluate_subexp_standard (struct type *e
*** 1638,1644 ****
--- 1638,1648 ----
  	    /* Fill in the subscript and array size arrays */
  
  	    subscript_array[i] = value_as_long (arg2);
+ 	  }
  
+ 	/* Internal type of array is arranged right to left */
+ 	for (i = 0; i < nargs; i++)
+ 	  {
  	    retcode = f77_get_dynamic_upperbound (tmp_type, &upper);
  	    if (retcode == BOUND_FETCH_ERROR)
  	      error ("Cannot obtain dynamic upper bound");
*************** evaluate_subexp_standard (struct type *e
*** 1647,1657 ****
  	    if (retcode == BOUND_FETCH_ERROR)
  	      error ("Cannot obtain dynamic lower bound");
  
! 	    array_size_array[i] = upper - lower + 1;
  
  	    /* Zero-normalize subscripts so that offsetting will work. */
  
! 	    subscript_array[i] -= lower;
  
  	    /* If we are at the bottom of a multidimensional 
  	       array type then keep a ptr to the last ARRAY
--- 1651,1661 ----
  	    if (retcode == BOUND_FETCH_ERROR)
  	      error ("Cannot obtain dynamic lower bound");
  
! 	    array_size_array[nargs - i - 1] = upper - lower + 1;
  
  	    /* Zero-normalize subscripts so that offsetting will work. */
  
! 	    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
*************** evaluate_subexp_standard (struct type *e
*** 1661,1677 ****
  	       of base element type that we apply a simple 
  	       offset to. */
  
! 	    if (i < nargs)
  	      tmp_type = check_typedef (TYPE_TARGET_TYPE (tmp_type));
  	  }
  
  	/* Now let us calculate the offset for this item */
  
! 	offset_item = subscript_array[ndimensions];
  
! 	for (i = ndimensions - 1; i >= 1; i--)
  	  offset_item =
! 	    array_size_array[i] * offset_item + subscript_array[i];
  
  	/* Construct a value node with the value of the offset */
  
--- 1665,1681 ----
  	       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 */
  
! 	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];
  
  	/* Construct a value node with the value of the offset */
  

[-- Attachment #3: arrays.gdb --]
[-- Type: text/plain, Size: 354 bytes --]

set height 0
b arrays.f:45
r
p onedi(4).eq.4
p twodi(4,5).eq.(4*5)
p twodi(14,3).eq.(14*3)
p threedi(2,3,4).eq.(2*3+4)
p threedi(1,4,5).eq.(1*4+5)
p threedi(3,4,5).eq.(3*4+5)
p fourdi(1,4,2,3).eq.(1*4+2*3)
p fourdi(2,4,3,3).eq.(2*4+3*3)
p fourdi(3,3,2,3).eq.(3*3+2*3)
p fourdi(3,2,1,2).eq.(3*2+1*2)
whatis onedi
whatis twodi
whatis threedi
whatis fourdi

[-- Attachment #4: arrays.f --]
[-- Type: text/x-fortran, Size: 2547 bytes --]

      program first


      INTEGER X,Y,Z
      INTEGER i, j, k, l
      INTEGER onedi(10)
      INTEGER twodi(20,10)
      INTEGER threedi(3, 5, 7)
      INTEGER fourdi(4, 5, 7, 11)

      REAL*4 onedf(10)
      REAL*4 twodf(20,10)
      REAL*4 threedf(3, 5, 7)
      REAL*4 fourdf(4, 5, 7, 11)


      DO i=1,10
         onedi(i) = i
      ENDDO

      DO i=1,20
         DO j=1,10
            twodi(i,j) = i * j
         ENDDO
      ENDDO

      DO i=1,3
         DO j=1,5
            DO k=1,7
               threedi(i,j,k) = i * j + k
            ENDDO
         ENDDO
      ENDDO

      DO i=1,3
         DO j=1,5
            DO k=1,7
               DO m=1,11
                  fourdi(i,j,k,m) = i * j + k * m
               ENDDO
            ENDDO
         ENDDO
      ENDDO
      

      call oneif(onedi)
      call twoif(twodi)
      call threeif(threedi)
      call fourif(fourdi)






      DO i=1,10
         onedf(i) = i
      ENDDO

      DO i=1,20
         DO j=1,10
            twodf(i,j) = i * j
         ENDDO
      ENDDO

      DO i=1,3
         DO j=1,5
            DO k=1,7
               threedf(i,j,k) = i * j + k
            ENDDO
         ENDDO
      ENDDO

      DO i=1,3
         DO j=1,5
            DO k=1,7
               DO m=1,11
                  fourdf(i,j,k,m) = i * j + k * m
               ENDDO
            ENDDO
         ENDDO
      ENDDO
      

      call oneff(onedf)
      call twoff(twodf)
      call threeff(threedf)
      call fourff(fourdf)

      call known(fourdf, m)

      END


      SUBROUTINE oneif (array1)
      INTEGER array1(*)
      array1(1) = 1
      RETURN
      END

      SUBROUTINE twoif (array2)
      INTEGER array2(20, *)
      array2(1,1) = 11
      RETURN
      END


      SUBROUTINE threeif (array3)
      INTEGER array3(3, 5, *)
      array3(1,1,1) = 111
      RETURN
      END

      SUBROUTINE fourif (array4)
      INTEGER array4(4, 5, 7, *)
      array4(1,1,1,1) = 1111
      RETURN
      END 

      SUBROUTINE oneff (array1)
      REAL*4 array1(*)
      array1(1) = 1
      RETURN
      END


      SUBROUTINE twoff (array2)
      REAL*4 array2(20, *)
      array2(1,1) = 11
      RETURN
      END

      SUBROUTINE threeff (array3)
      REAL*4 array3(3, 5, *)
      array3(1,1,1) = 111
      RETURN
      END

      SUBROUTINE fourff (array4)
      REAL*4 array4(4, 5, 7, *)
      array4(1,1,1,1) = 1111
      RETURN
      END 

      SUBROUTINE known (array4, i)
      INTEGER i
      REAL*4 array4(4, 5, 7, i)
      array4(1,1,1,1) = 1111
      RETURN
      END 

Re: [PATCH/RFA] PR gdb/648

[David Lecomber]

Hi,

Is anyone looking into this -- I can't commit without a go-ahead!  

Cheers
David

On Fri, 2004-08-06 at 23:10, David Lecomber wrote:
> This is a fix for a long-standing bug with multi-dimensional arrays in
> fortran.  When printing individual elements of an array,
> evaluate_subexp_standard understood the dimensions of the array to be in
> the opposite order to that of the whatis or entire array printing.  This
> led to bug 648, which was present for G77 compiled code, but the problem
> was reversed for commercial compilers such as Intel or Portland.
> 
> G77 puts things in row-major order, as far as I can ascertain all other
> fortran compilers do column major.  This is handled in the dwarf2read
> change, and now column major array types are reversed during the reading
> to fit with GDB's struct type.
> 
> 2004-08-06  David Lecomber  <dsl@sources.redhat.com>
>  
>         Fix PR gdb/648
>         * dwarf2read.c (read_array_type): Handle column major arrays
>         correctly.  Assume column major for Fortran except with G77
>         compiler.
>         * eval.c (evaluate_subexp_standard): Assume Fortran arrays are
>         oriented large to small in type structure.
> 
> 
> Attached is a test program and test script, should return true for all 
> comparisons..  
> 
> I'm sure there'll be some comments, so please feel free to suggest some
> reformatting!
> 
> David.
> 

Re: [PATCH/RFA] PR gdb/648

[Michael Chastain]

Sorry, I dropped your packet with the test program in it.

(That is, it arrived in my mailbox just fine, but I did not
 add it to my to-do list.)

Someone needs to turn your program into a dejagnu script.
I can do that.

I'll get back to you by Sunday morning.

Michael C

Re: [PATCH/RFA] PR gdb/648

[Jim Blandy]

David Lecomber <david@streamline-computing.com> writes:
> 2004-08-06  David Lecomber  <dsl@sources.redhat.com>
>  
>         Fix PR gdb/648
>         * dwarf2read.c (read_array_type): Handle column major arrays
>         correctly.  Assume column major for Fortran except with G77
>         compiler.
>         * eval.c (evaluate_subexp_standard): Assume Fortran arrays are
>         oriented large to small in type structure.

Thanks very much for the patch and test program.  It was helpful to be
able to actually look at what GNU F77 was producing for a real Fortran
program.

For the dwarf2read.c part, I think the essential approach is fine.
However:

- Rather than recognizing Fortran specially in read_array_type, I'd
  rather have a new member of 'struct language_defn' giving the array
  ordering for that language.

- I'd like to have the logic that chooses a byte order pulled out of
  read_array_type into its own function that takes a die, a cu, and
  returns a DW_ORD_ value.  That function would take care of looking
  for a DW_AT_ordering attribute, recognizing the appropriate versions
  of GNU F77, and so on.  read_array_type should call that function
  and build the type appropriately.

Re: [PATCH/RFA] PR gdb/648

[Michael Chastain]

The test suite part of this will take a while.

There are no FORTRAN programs in the test suite yet.  There are two
scripts in gdb.fortran which exercise gdb's fortran expression handler,
but no actual programs!

So I am going to add some infrastructure and then add the first actual
FORTRAN program.  I am working on that right now.  After that, various
platforms need to adjust to the shock and maybe disable themselves in
skip_fortran_tests.

I estimate it will take about a week before I can add real fortran
tests, like yours.

Michael C

Re: [PATCH/RFA] PR gdb/648

[David Lecomber]

On Fri, 2004-08-13 at 23:10, Jim Blandy wrote:
> David Lecomber <david@streamline-computing.com> writes:
> > 2004-08-06  David Lecomber  <dsl@sources.redhat.com>
> >  
> >         Fix PR gdb/648
> >         * dwarf2read.c (read_array_type): Handle column major arrays
> >         correctly.  Assume column major for Fortran except with G77
> >         compiler.
> >         * eval.c (evaluate_subexp_standard): Assume Fortran arrays are
> >         oriented large to small in type structure.
> 
> Thanks very much for the patch and test program.  It was helpful to be
> able to actually look at what GNU F77 was producing for a real Fortran
> program.
> 
> For the dwarf2read.c part, I think the essential approach is fine.
> However:
> 
> - Rather than recognizing Fortran specially in read_array_type, I'd
>   rather have a new member of 'struct language_defn' giving the array
>   ordering for that language.
> 
> - I'd like to have the logic that chooses a byte order pulled out of
>   read_array_type into its own function that takes a die, a cu, and
>   returns a DW_ORD_ value.  That function would take care of looking
>   for a DW_AT_ordering attribute, recognizing the appropriate versions
>   of GNU F77, and so on.  read_array_type should call that function
>   and build the type appropriately.

Re: [PATCH/RFA] PR gdb/648

[David Lecomber]

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

> - Rather than recognizing Fortran specially in read_array_type, I'd
>   rather have a new member of 'struct language_defn' giving the array
>   ordering for that language.
> 
> - I'd like to have the logic that chooses a byte order pulled out of
>   read_array_type into its own function that takes a die, a cu, and
>   returns a DW_ORD_ value.  That function would take care of looking
>   for a DW_AT_ordering attribute, recognizing the appropriate versions
>   of GNU F77, and so on.  read_array_type should call that function
>   and build the type appropriately.

Ok, here's round two!   It passes my Fortran test case ok, but I haven't
run through the normal tests.  I've had to guess that all other
languages are row major (where it actually is relevant..) and I cannot
think of any exceptions other than Fortran.

2004-08-16  David Lecomber  <david@streamline-computing.com>
 
        Fix PR gdb/648
        * language.h (enum array_ordering): New enum.
        * language.h (struct language_defn): New la_array_ordering
attribute.
        * language.c (unknown_language_defn, auto_language_defn)
        (local_language_defn): Ditto.
        * ada-lang.c (ada_language_defn): Ditto.
        * c-lang.c (c_language_defn, cplus_language_defn)
        (asm_language_defn, minimal_language_defn): Ditto.
        * f-lang.c (f_language_defn): Ditto.
        * jv-lang.c (java_language_defn): Ditto.
        * m2-lang.c (f_language_defn): Ditto.
        * objc-lang.c (objc_language_defn): Ditto.
        * p-lang.c (pascal_language_defn): Ditto.
        * scm-lang.c (scm_language_defn): Ditto.
        * eval.c (evaluate_subexp_standard): Assume Fortran arrays are
        oriented large to small in type structure.
         


[-- Attachment #2: lang.patches --]
[-- Type: text/x-patch, Size: 13623 bytes --]

Index: gdb/ada-lang.c
===================================================================
RCS file: /cvs/src/src/gdb/ada-lang.c,v
retrieving revision 1.50
diff -c -p -r1.50 ada-lang.c
*** gdb/ada-lang.c	10 Aug 2004 21:16:12 -0000	1.50
--- gdb/ada-lang.c	16 Aug 2004 12:01:57 -0000
*************** const struct language_defn ada_language_
*** 10166,10171 ****
--- 10166,10172 ----
    ada_lookup_symbol,
    ada_lookup_minimal_symbol,
  #endif /* GNAT_GDB */
+   array_row_major,
    &ada_exp_descriptor,
    parse,
    ada_error,
Index: gdb/c-lang.c
===================================================================
RCS file: /cvs/src/src/gdb/c-lang.c,v
retrieving revision 1.31
diff -c -p -r1.31 c-lang.c
*** gdb/c-lang.c	28 Jul 2004 15:18:06 -0000	1.31
--- gdb/c-lang.c	16 Aug 2004 12:01:58 -0000
*************** const struct language_defn c_language_de
*** 570,575 ****
--- 570,576 ----
    range_check_off,
    type_check_off,
    case_sensitive_on,
+   array_row_major,
    &exp_descriptor_standard,
    c_preprocess_and_parse,
    c_error,
*************** const struct language_defn cplus_languag
*** 631,636 ****
--- 632,638 ----
    range_check_off,
    type_check_off,
    case_sensitive_on,
+   array_row_major,
    &exp_descriptor_standard,
    c_preprocess_and_parse,
    c_error,
*************** const struct language_defn asm_language_
*** 669,674 ****
--- 671,677 ----
    range_check_off,
    type_check_off,
    case_sensitive_on,
+   array_row_major,
    &exp_descriptor_standard,
    c_preprocess_and_parse,
    c_error,
*************** const struct language_defn minimal_langu
*** 712,717 ****
--- 715,721 ----
    range_check_off,
    type_check_off,
    case_sensitive_on,
+   array_row_major,
    &exp_descriptor_standard,
    c_preprocess_and_parse,
    c_error,
Index: gdb/dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.158
diff -c -p -r1.158 dwarf2read.c
*** gdb/dwarf2read.c	2 Aug 2004 01:25:57 -0000	1.158
--- gdb/dwarf2read.c	16 Aug 2004 12:02:11 -0000
*************** static CORE_ADDR decode_locdesc (struct 
*** 848,853 ****
--- 848,855 ----
  
  static void read_array_type (struct die_info *, struct dwarf2_cu *);
  
+ static int read_array_order (struct die_info *, struct dwarf2_cu *);
+ 
  static void read_tag_pointer_type (struct die_info *, struct dwarf2_cu *);
  
  static void read_tag_ptr_to_member_type (struct die_info *,
*************** read_array_type (struct die_info *die, s
*** 3724,3732 ****
  
    /* Dwarf2 dimensions are output from left to right, create the
       necessary array types in backwards order.  */
    type = element_type;
!   while (ndim-- > 0)
!     type = create_array_type (NULL, type, range_types[ndim]);
  
    /* Understand Dwarf2 support for vector types (like they occur on
       the PowerPC w/ AltiVec).  Gcc just adds another attribute to the
--- 3726,3745 ----
  
    /* Dwarf2 dimensions are output from left to right, create the
       necessary array types in backwards order.  */
+ 
    type = element_type;
! 
!   if (read_array_order (die, cu) == DW_ORD_col_major)
!     {
!       int i = 0;
!       while (i < ndim)
! 	type = create_array_type (NULL, type, range_types[i++]);
!     }
!   else
!     {
!       while (ndim-- > 0)
! 	type = create_array_type (NULL, type, range_types[ndim]);
!     }
  
    /* Understand Dwarf2 support for vector types (like they occur on
       the PowerPC w/ AltiVec).  Gcc just adds another attribute to the
*************** read_array_type (struct die_info *die, s
*** 3744,3749 ****
--- 3757,3788 ----
    die->type = type;
  }
  
+ static int
+ read_array_order (struct die_info *die, struct dwarf2_cu *cu) 
+ {
+   struct attribute *attr;
+ 
+   attr = dwarf2_attr (die, DW_AT_ordering, cu);
+ 
+   if (attr) return DW_SND (attr);
+ 
+   if (cu->language == language_fortran &&
+       cu->producer && strstr (cu->producer, "GNU F77"))
+     {
+       return DW_ORD_row_major;
+     }
+ 
+   switch (cu->language_defn->la_array_ordering) 
+     {
+     case array_column_major:
+       return DW_ORD_col_major;
+     case array_row_major:
+     default:
+       return DW_ORD_row_major;
+     };
+ }
+ 
+ 
  /* First cut: install each common block member as a global variable.  */
  
  static void
Index: gdb/eval.c
===================================================================
RCS file: /cvs/src/src/gdb/eval.c,v
retrieving revision 1.41
diff -c -p -r1.41 eval.c
*** gdb/eval.c	8 Apr 2004 21:18:12 -0000	1.41
--- gdb/eval.c	16 Aug 2004 12:02:15 -0000
*************** evaluate_subexp_standard (struct type *e
*** 1610,1618 ****
  
      multi_f77_subscript:
        {
! 	int subscript_array[MAX_FORTRAN_DIMS + 1];	/* 1-based array of 
! 							   subscripts, max == 7 */
! 	int array_size_array[MAX_FORTRAN_DIMS + 1];
  	int ndimensions = 1, i;
  	struct type *tmp_type;
  	int offset_item;	/* The array offset where the item lives */
--- 1610,1617 ----
  
      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 */
*************** evaluate_subexp_standard (struct type *e
*** 1630,1636 ****
  	   let us actually find out where this element exists in the array. */
  
  	offset_item = 0;
! 	for (i = 1; i <= nargs; i++)
  	  {
  	    /* Evaluate each subscript, It must be a legal integer in F77 */
  	    arg2 = evaluate_subexp_with_coercion (exp, pos, noside);
--- 1629,1636 ----
  	   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);
*************** evaluate_subexp_standard (struct type *e
*** 1638,1644 ****
--- 1638,1648 ----
  	    /* Fill in the subscript and array size arrays */
  
  	    subscript_array[i] = value_as_long (arg2);
+ 	  }
  
+ 	/* Internal type of array is arranged right to left */
+ 	for (i = 0; i < nargs; i++)
+ 	  {
  	    retcode = f77_get_dynamic_upperbound (tmp_type, &upper);
  	    if (retcode == BOUND_FETCH_ERROR)
  	      error ("Cannot obtain dynamic upper bound");
*************** evaluate_subexp_standard (struct type *e
*** 1647,1657 ****
  	    if (retcode == BOUND_FETCH_ERROR)
  	      error ("Cannot obtain dynamic lower bound");
  
! 	    array_size_array[i] = upper - lower + 1;
  
  	    /* Zero-normalize subscripts so that offsetting will work. */
  
! 	    subscript_array[i] -= lower;
  
  	    /* If we are at the bottom of a multidimensional 
  	       array type then keep a ptr to the last ARRAY
--- 1651,1661 ----
  	    if (retcode == BOUND_FETCH_ERROR)
  	      error ("Cannot obtain dynamic lower bound");
  
! 	    array_size_array[nargs - i - 1] = upper - lower + 1;
  
  	    /* Zero-normalize subscripts so that offsetting will work. */
  
! 	    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
*************** evaluate_subexp_standard (struct type *e
*** 1661,1677 ****
  	       of base element type that we apply a simple 
  	       offset to. */
  
! 	    if (i < nargs)
  	      tmp_type = check_typedef (TYPE_TARGET_TYPE (tmp_type));
  	  }
  
  	/* Now let us calculate the offset for this item */
  
! 	offset_item = subscript_array[ndimensions];
  
! 	for (i = ndimensions - 1; i >= 1; i--)
  	  offset_item =
! 	    array_size_array[i] * offset_item + subscript_array[i];
  
  	/* Construct a value node with the value of the offset */
  
--- 1665,1681 ----
  	       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 */
  
! 	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];
  
  	/* Construct a value node with the value of the offset */
  
Index: gdb/f-lang.c
===================================================================
RCS file: /cvs/src/src/gdb/f-lang.c,v
retrieving revision 1.25
diff -c -p -r1.25 f-lang.c
*** gdb/f-lang.c	28 Jul 2004 02:46:22 -0000	1.25
--- gdb/f-lang.c	16 Aug 2004 12:02:18 -0000
*************** const struct language_defn f_language_de
*** 462,467 ****
--- 462,468 ----
    range_check_on,
    type_check_on,
    case_sensitive_off,
+   array_column_major,
    &exp_descriptor_standard,
    f_parse,			/* parser */
    f_error,			/* parser error function */
Index: gdb/jv-lang.c
===================================================================
RCS file: /cvs/src/src/gdb/jv-lang.c,v
retrieving revision 1.33
diff -c -p -r1.33 jv-lang.c
*** gdb/jv-lang.c	28 Jul 2004 15:18:06 -0000	1.33
--- gdb/jv-lang.c	16 Aug 2004 12:02:20 -0000
*************** const struct language_defn java_language
*** 1088,1093 ****
--- 1088,1094 ----
    range_check_off,
    type_check_off,
    case_sensitive_on,
+   array_row_major,
    &exp_descriptor_java,
    java_parse,
    java_error,
Index: gdb/language.c
===================================================================
RCS file: /cvs/src/src/gdb/language.c,v
retrieving revision 1.48
diff -c -p -r1.48 language.c
*** gdb/language.c	28 Jul 2004 04:33:49 -0000	1.48
--- gdb/language.c	16 Aug 2004 12:02:23 -0000
*************** const struct language_defn unknown_langu
*** 1294,1299 ****
--- 1294,1300 ----
    NULL,
    range_check_off,
    type_check_off,
+   array_row_major,
    case_sensitive_on,
    &exp_descriptor_standard,
    unk_lang_parser,
*************** const struct language_defn auto_language
*** 1333,1338 ****
--- 1334,1340 ----
    NULL,
    range_check_off,
    type_check_off,
+   array_row_major,
    case_sensitive_on,
    &exp_descriptor_standard,
    unk_lang_parser,
*************** const struct language_defn local_languag
*** 1372,1377 ****
--- 1374,1380 ----
    range_check_off,
    type_check_off,
    case_sensitive_on,
+   array_row_major,
    &exp_descriptor_standard,
    unk_lang_parser,
    unk_lang_error,
Index: gdb/language.h
===================================================================
RCS file: /cvs/src/src/gdb/language.h,v
retrieving revision 1.29
diff -c -p -r1.29 language.h
*** gdb/language.h	28 Jul 2004 04:33:49 -0000	1.29
--- gdb/language.h	16 Aug 2004 12:02:24 -0000
*************** extern enum case_mode
*** 96,101 ****
--- 96,112 ----
    }
  case_mode;
  
+ /* array_ordering ==
+    array_row_major:     Arrays are in row major order
+    array_column_major:  Arrays are in column major order.*/
+ 
+ extern enum array_ordering
+   {
+     array_row_major, array_column_major
+   } 
+ array_ordering;
+ 
+ 
  /* case_sensitivity ==
     case_sensitive_on:   Case sensitivity in name matching is used
     case_sensitive_off:  Case sensitivity in name matching is not used  */
*************** struct language_defn
*** 187,192 ****
--- 198,206 ----
      /* Default case sensitivity */
      enum case_sensitivity la_case_sensitivity;
  
+     /* Multi-dimensional array ordering */
+     enum array_ordering la_array_ordering;
+ 
      /* Definitions related to expression printing, prefixifying, and
         dumping */
  
Index: gdb/m2-lang.c
===================================================================
RCS file: /cvs/src/src/gdb/m2-lang.c,v
retrieving revision 1.18
diff -c -p -r1.18 m2-lang.c
*** gdb/m2-lang.c	28 Jul 2004 02:46:23 -0000	1.18
--- gdb/m2-lang.c	16 Aug 2004 12:02:25 -0000
*************** const struct language_defn m2_language_d
*** 415,420 ****
--- 415,421 ----
    range_check_on,
    type_check_on,
    case_sensitive_on,
+   array_row_major,
    &exp_descriptor_standard,
    m2_parse,			/* parser */
    m2_error,			/* parser error function */
Index: gdb/objc-lang.c
===================================================================
RCS file: /cvs/src/src/gdb/objc-lang.c,v
retrieving revision 1.37
diff -c -p -r1.37 objc-lang.c
*** gdb/objc-lang.c	28 Jul 2004 02:46:23 -0000	1.37
--- gdb/objc-lang.c	16 Aug 2004 12:02:27 -0000
*************** const struct language_defn objc_language
*** 659,664 ****
--- 659,665 ----
    range_check_off,
    type_check_off,
    case_sensitive_on,
+   array_row_major,
    &exp_descriptor_standard,
    objc_parse,
    objc_error,
Index: gdb/p-lang.c
===================================================================
RCS file: /cvs/src/src/gdb/p-lang.c,v
retrieving revision 1.20
diff -c -p -r1.20 p-lang.c
*** gdb/p-lang.c	28 Jul 2004 02:46:23 -0000	1.20
--- gdb/p-lang.c	16 Aug 2004 12:02:28 -0000
*************** const struct language_defn pascal_langua
*** 451,456 ****
--- 451,457 ----
    range_check_on,
    type_check_on,
    case_sensitive_on,
+   array_row_major,
    &exp_descriptor_standard,
    pascal_parse,
    pascal_error,
Index: gdb/scm-lang.c
===================================================================
RCS file: /cvs/src/src/gdb/scm-lang.c,v
retrieving revision 1.26
diff -c -p -r1.26 scm-lang.c
*** gdb/scm-lang.c	28 Jul 2004 15:18:06 -0000	1.26
--- gdb/scm-lang.c	16 Aug 2004 12:02:29 -0000
*************** const struct language_defn scm_language_
*** 248,253 ****
--- 248,254 ----
    range_check_off,
    type_check_off,
    case_sensitive_off,
+   array_row_major,
    &exp_descriptor_scm,
    scm_parse,
    c_error,

Re: [PATCH/RFA] PR gdb/648

[Jim Blandy]

Thanks for the revisions!

Any reason read_array_ordering can't return 'enum
dwarf_array_dim_ordering' instead of 'int'?

The special case for GNU F77 needs a comment --- just explaining that
F77 does things backwards as of Aug 2004, etc. etc.  Basically what
you said in your original post, explaining why the special case was
necessary.

With those changes, the dwarf2read.c portions of this change are
approved.  So I think the eval.c stuff is all that's left for someone
to review.

Re: [PATCH/RFA] PR gdb/648 (eval.c approval reqd)

[David Lecomber]

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

I've made the enum change, and added comments -- the revised patches are
attached.  Anyone eval.c maintainer ready to approve these changes?


Cheers
David


2004-08-16  David Lecomber  <david@streamline-computing.com>
 
        Fix PR gdb/648
        * language.h (enum array_ordering): New enum.
        * language.h (struct language_defn): New la_array_ordering
attribute.
        * language.c (unknown_language_defn, auto_language_defn)
        (local_language_defn): Ditto.
        * ada-lang.c (ada_language_defn): Ditto.
        * c-lang.c (c_language_defn, cplus_language_defn)
        (asm_language_defn, minimal_language_defn): Ditto.
        * f-lang.c (f_language_defn): Ditto.
        * jv-lang.c (java_language_defn): Ditto.
        * m2-lang.c (f_language_defn): Ditto.
        * objc-lang.c (objc_language_defn): Ditto.
        * p-lang.c (pascal_language_defn): Ditto.
        * scm-lang.c (scm_language_defn): Ditto.
        * eval.c (evaluate_subexp_standard): Assume Fortran arrays are
        oriented large to small in type structure.
         


On Wed, 2004-08-18 at 17:32, Jim Blandy wrote:
> Thanks for the revisions!
> 
> Any reason read_array_ordering can't return 'enum
> dwarf_array_dim_ordering' instead of 'int'?
> 
> The special case for GNU F77 needs a comment --- just explaining that
> F77 does things backwards as of Aug 2004, etc. etc.  Basically what
> you said in your original post, explaining why the special case was
> necessary.
> 
> With those changes, the dwarf2read.c portions of this change are
> approved.  So I think the eval.c stuff is all that's left for someone
> to review.

[-- Attachment #2: lang.patches --]
[-- Type: text/x-patch, Size: 13916 bytes --]

Index: gdb/ada-lang.c
===================================================================
RCS file: /cvs/src/src/gdb/ada-lang.c,v
retrieving revision 1.50
diff -c -p -r1.50 ada-lang.c
*** gdb/ada-lang.c	10 Aug 2004 21:16:12 -0000	1.50
--- gdb/ada-lang.c	20 Aug 2004 13:23:41 -0000
*************** const struct language_defn ada_language_
*** 10166,10171 ****
--- 10166,10172 ----
    ada_lookup_symbol,
    ada_lookup_minimal_symbol,
  #endif /* GNAT_GDB */
+   array_row_major,
    &ada_exp_descriptor,
    parse,
    ada_error,
Index: gdb/c-lang.c
===================================================================
RCS file: /cvs/src/src/gdb/c-lang.c,v
retrieving revision 1.31
diff -c -p -r1.31 c-lang.c
*** gdb/c-lang.c	28 Jul 2004 15:18:06 -0000	1.31
--- gdb/c-lang.c	20 Aug 2004 13:23:43 -0000
*************** const struct language_defn c_language_de
*** 570,575 ****
--- 570,576 ----
    range_check_off,
    type_check_off,
    case_sensitive_on,
+   array_row_major,
    &exp_descriptor_standard,
    c_preprocess_and_parse,
    c_error,
*************** const struct language_defn cplus_languag
*** 631,636 ****
--- 632,638 ----
    range_check_off,
    type_check_off,
    case_sensitive_on,
+   array_row_major,
    &exp_descriptor_standard,
    c_preprocess_and_parse,
    c_error,
*************** const struct language_defn asm_language_
*** 669,674 ****
--- 671,677 ----
    range_check_off,
    type_check_off,
    case_sensitive_on,
+   array_row_major,
    &exp_descriptor_standard,
    c_preprocess_and_parse,
    c_error,
*************** const struct language_defn minimal_langu
*** 712,717 ****
--- 715,721 ----
    range_check_off,
    type_check_off,
    case_sensitive_on,
+   array_row_major,
    &exp_descriptor_standard,
    c_preprocess_and_parse,
    c_error,
Index: gdb/dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.158
diff -c -p -r1.158 dwarf2read.c
*** gdb/dwarf2read.c	2 Aug 2004 01:25:57 -0000	1.158
--- gdb/dwarf2read.c	20 Aug 2004 13:24:01 -0000
*************** static CORE_ADDR decode_locdesc (struct 
*** 848,853 ****
--- 848,856 ----
  
  static void read_array_type (struct die_info *, struct dwarf2_cu *);
  
+ static enum dwarf_array_dim_ordering read_array_order (struct die_info *, 
+ 						       struct dwarf2_cu *);
+ 
  static void read_tag_pointer_type (struct die_info *, struct dwarf2_cu *);
  
  static void read_tag_ptr_to_member_type (struct die_info *,
*************** read_array_type (struct die_info *die, s
*** 3724,3732 ****
  
    /* Dwarf2 dimensions are output from left to right, create the
       necessary array types in backwards order.  */
    type = element_type;
!   while (ndim-- > 0)
!     type = create_array_type (NULL, type, range_types[ndim]);
  
    /* Understand Dwarf2 support for vector types (like they occur on
       the PowerPC w/ AltiVec).  Gcc just adds another attribute to the
--- 3727,3746 ----
  
    /* Dwarf2 dimensions are output from left to right, create the
       necessary array types in backwards order.  */
+ 
    type = element_type;
! 
!   if (read_array_order (die, cu) == DW_ORD_col_major)
!     {
!       int i = 0;
!       while (i < ndim)
! 	type = create_array_type (NULL, type, range_types[i++]);
!     }
!   else
!     {
!       while (ndim-- > 0)
! 	type = create_array_type (NULL, type, range_types[ndim]);
!     }
  
    /* Understand Dwarf2 support for vector types (like they occur on
       the PowerPC w/ AltiVec).  Gcc just adds another attribute to the
*************** read_array_type (struct die_info *die, s
*** 3744,3749 ****
--- 3758,3797 ----
    die->type = type;
  }
  
+ static enum dwarf_array_dim_ordering
+ read_array_order (struct die_info *die, struct dwarf2_cu *cu) 
+ {
+   struct attribute *attr;
+ 
+   attr = dwarf2_attr (die, DW_AT_ordering, cu);
+ 
+   if (attr) return DW_SND (attr);
+ 
+   /*
+     GNU F77 is a special case, as at 08/2004 arrays are the opposite
+     way to the dwarf2 specification.
+ 
+     FIXME: dsl/2004-8-20: If G77 is ever fixed, this will also need 
+     version checking.
+   */
+ 
+   if (cu->language == language_fortran &&
+       cu->producer && strstr (cu->producer, "GNU F77"))
+     {
+       return DW_ORD_row_major;
+     }
+ 
+   switch (cu->language_defn->la_array_ordering) 
+     {
+     case array_column_major:
+       return DW_ORD_col_major;
+     case array_row_major:
+     default:
+       return DW_ORD_row_major;
+     };
+ }
+ 
+ 
  /* First cut: install each common block member as a global variable.  */
  
  static void
Index: gdb/eval.c
===================================================================
RCS file: /cvs/src/src/gdb/eval.c,v
retrieving revision 1.41
diff -c -p -r1.41 eval.c
*** gdb/eval.c	8 Apr 2004 21:18:12 -0000	1.41
--- gdb/eval.c	20 Aug 2004 13:24:05 -0000
*************** evaluate_subexp_standard (struct type *e
*** 1610,1618 ****
  
      multi_f77_subscript:
        {
! 	int subscript_array[MAX_FORTRAN_DIMS + 1];	/* 1-based array of 
! 							   subscripts, max == 7 */
! 	int array_size_array[MAX_FORTRAN_DIMS + 1];
  	int ndimensions = 1, i;
  	struct type *tmp_type;
  	int offset_item;	/* The array offset where the item lives */
--- 1610,1617 ----
  
      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 */
*************** evaluate_subexp_standard (struct type *e
*** 1630,1636 ****
  	   let us actually find out where this element exists in the array. */
  
  	offset_item = 0;
! 	for (i = 1; i <= nargs; i++)
  	  {
  	    /* Evaluate each subscript, It must be a legal integer in F77 */
  	    arg2 = evaluate_subexp_with_coercion (exp, pos, noside);
--- 1629,1636 ----
  	   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);
*************** evaluate_subexp_standard (struct type *e
*** 1638,1644 ****
--- 1638,1648 ----
  	    /* Fill in the subscript and array size arrays */
  
  	    subscript_array[i] = value_as_long (arg2);
+ 	  }
  
+ 	/* Internal type of array is arranged right to left */
+ 	for (i = 0; i < nargs; i++)
+ 	  {
  	    retcode = f77_get_dynamic_upperbound (tmp_type, &upper);
  	    if (retcode == BOUND_FETCH_ERROR)
  	      error ("Cannot obtain dynamic upper bound");
*************** evaluate_subexp_standard (struct type *e
*** 1647,1657 ****
  	    if (retcode == BOUND_FETCH_ERROR)
  	      error ("Cannot obtain dynamic lower bound");
  
! 	    array_size_array[i] = upper - lower + 1;
  
  	    /* Zero-normalize subscripts so that offsetting will work. */
  
! 	    subscript_array[i] -= lower;
  
  	    /* If we are at the bottom of a multidimensional 
  	       array type then keep a ptr to the last ARRAY
--- 1651,1661 ----
  	    if (retcode == BOUND_FETCH_ERROR)
  	      error ("Cannot obtain dynamic lower bound");
  
! 	    array_size_array[nargs - i - 1] = upper - lower + 1;
  
  	    /* Zero-normalize subscripts so that offsetting will work. */
  
! 	    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
*************** evaluate_subexp_standard (struct type *e
*** 1661,1677 ****
  	       of base element type that we apply a simple 
  	       offset to. */
  
! 	    if (i < nargs)
  	      tmp_type = check_typedef (TYPE_TARGET_TYPE (tmp_type));
  	  }
  
  	/* Now let us calculate the offset for this item */
  
! 	offset_item = subscript_array[ndimensions];
  
! 	for (i = ndimensions - 1; i >= 1; i--)
  	  offset_item =
! 	    array_size_array[i] * offset_item + subscript_array[i];
  
  	/* Construct a value node with the value of the offset */
  
--- 1665,1681 ----
  	       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 */
  
! 	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];
  
  	/* Construct a value node with the value of the offset */
  
Index: gdb/f-lang.c
===================================================================
RCS file: /cvs/src/src/gdb/f-lang.c,v
retrieving revision 1.25
diff -c -p -r1.25 f-lang.c
*** gdb/f-lang.c	28 Jul 2004 02:46:22 -0000	1.25
--- gdb/f-lang.c	20 Aug 2004 13:24:09 -0000
*************** const struct language_defn f_language_de
*** 462,467 ****
--- 462,468 ----
    range_check_on,
    type_check_on,
    case_sensitive_off,
+   array_column_major,
    &exp_descriptor_standard,
    f_parse,			/* parser */
    f_error,			/* parser error function */
Index: gdb/jv-lang.c
===================================================================
RCS file: /cvs/src/src/gdb/jv-lang.c,v
retrieving revision 1.33
diff -c -p -r1.33 jv-lang.c
*** gdb/jv-lang.c	28 Jul 2004 15:18:06 -0000	1.33
--- gdb/jv-lang.c	20 Aug 2004 13:24:10 -0000
*************** const struct language_defn java_language
*** 1088,1093 ****
--- 1088,1094 ----
    range_check_off,
    type_check_off,
    case_sensitive_on,
+   array_row_major,
    &exp_descriptor_java,
    java_parse,
    java_error,
Index: gdb/language.c
===================================================================
RCS file: /cvs/src/src/gdb/language.c,v
retrieving revision 1.48
diff -c -p -r1.48 language.c
*** gdb/language.c	28 Jul 2004 04:33:49 -0000	1.48
--- gdb/language.c	20 Aug 2004 13:24:12 -0000
*************** const struct language_defn unknown_langu
*** 1294,1299 ****
--- 1294,1300 ----
    NULL,
    range_check_off,
    type_check_off,
+   array_row_major,
    case_sensitive_on,
    &exp_descriptor_standard,
    unk_lang_parser,
*************** const struct language_defn auto_language
*** 1333,1338 ****
--- 1334,1340 ----
    NULL,
    range_check_off,
    type_check_off,
+   array_row_major,
    case_sensitive_on,
    &exp_descriptor_standard,
    unk_lang_parser,
*************** const struct language_defn local_languag
*** 1372,1377 ****
--- 1374,1380 ----
    range_check_off,
    type_check_off,
    case_sensitive_on,
+   array_row_major,
    &exp_descriptor_standard,
    unk_lang_parser,
    unk_lang_error,
Index: gdb/language.h
===================================================================
RCS file: /cvs/src/src/gdb/language.h,v
retrieving revision 1.29
diff -c -p -r1.29 language.h
*** gdb/language.h	28 Jul 2004 04:33:49 -0000	1.29
--- gdb/language.h	20 Aug 2004 13:24:14 -0000
*************** extern enum case_mode
*** 96,101 ****
--- 96,112 ----
    }
  case_mode;
  
+ /* array_ordering ==
+    array_row_major:     Arrays are in row major order
+    array_column_major:  Arrays are in column major order.*/
+ 
+ extern enum array_ordering
+   {
+     array_row_major, array_column_major
+   } 
+ array_ordering;
+ 
+ 
  /* case_sensitivity ==
     case_sensitive_on:   Case sensitivity in name matching is used
     case_sensitive_off:  Case sensitivity in name matching is not used  */
*************** struct language_defn
*** 187,192 ****
--- 198,206 ----
      /* Default case sensitivity */
      enum case_sensitivity la_case_sensitivity;
  
+     /* Multi-dimensional array ordering */
+     enum array_ordering la_array_ordering;
+ 
      /* Definitions related to expression printing, prefixifying, and
         dumping */
  
Index: gdb/m2-lang.c
===================================================================
RCS file: /cvs/src/src/gdb/m2-lang.c,v
retrieving revision 1.18
diff -c -p -r1.18 m2-lang.c
*** gdb/m2-lang.c	28 Jul 2004 02:46:23 -0000	1.18
--- gdb/m2-lang.c	20 Aug 2004 13:24:15 -0000
*************** const struct language_defn m2_language_d
*** 415,420 ****
--- 415,421 ----
    range_check_on,
    type_check_on,
    case_sensitive_on,
+   array_row_major,
    &exp_descriptor_standard,
    m2_parse,			/* parser */
    m2_error,			/* parser error function */
Index: gdb/objc-lang.c
===================================================================
RCS file: /cvs/src/src/gdb/objc-lang.c,v
retrieving revision 1.37
diff -c -p -r1.37 objc-lang.c
*** gdb/objc-lang.c	28 Jul 2004 02:46:23 -0000	1.37
--- gdb/objc-lang.c	20 Aug 2004 13:24:19 -0000
*************** const struct language_defn objc_language
*** 659,664 ****
--- 659,665 ----
    range_check_off,
    type_check_off,
    case_sensitive_on,
+   array_row_major,
    &exp_descriptor_standard,
    objc_parse,
    objc_error,
Index: gdb/p-lang.c
===================================================================
RCS file: /cvs/src/src/gdb/p-lang.c,v
retrieving revision 1.20
diff -c -p -r1.20 p-lang.c
*** gdb/p-lang.c	28 Jul 2004 02:46:23 -0000	1.20
--- gdb/p-lang.c	20 Aug 2004 13:24:20 -0000
*************** const struct language_defn pascal_langua
*** 451,456 ****
--- 451,457 ----
    range_check_on,
    type_check_on,
    case_sensitive_on,
+   array_row_major,
    &exp_descriptor_standard,
    pascal_parse,
    pascal_error,
Index: gdb/scm-lang.c
===================================================================
RCS file: /cvs/src/src/gdb/scm-lang.c,v
retrieving revision 1.26
diff -c -p -r1.26 scm-lang.c
*** gdb/scm-lang.c	28 Jul 2004 15:18:06 -0000	1.26
--- gdb/scm-lang.c	20 Aug 2004 13:24:22 -0000
*************** const struct language_defn scm_language_
*** 248,253 ****
--- 248,254 ----
    range_check_off,
    type_check_off,
    case_sensitive_off,
+   array_row_major,
    &exp_descriptor_scm,
    scm_parse,
    c_error,

Re: [PATCH/RFA] PR gdb/648 (eval.c approval reqd)

[David Lecomber]

Oops, missed out a line on the ChangeLog -- 

	* dwarf2read.c (read_array_order): New function.
	(read_array_type): Use read_array_order to check row/column
	major ordering.

>         Fix PR gdb/648
>         * language.h (enum array_ordering): New enum.
>         * language.h (struct language_defn): New la_array_ordering
> attribute.
>         * language.c (unknown_language_defn, auto_language_defn)
>         (local_language_defn): Ditto.
>         * ada-lang.c (ada_language_defn): Ditto.
>         * c-lang.c (c_language_defn, cplus_language_defn)
>         (asm_language_defn, minimal_language_defn): Ditto.
>         * f-lang.c (f_language_defn): Ditto.
>         * jv-lang.c (java_language_defn): Ditto.
>         * m2-lang.c (f_language_defn): Ditto.
>         * objc-lang.c (objc_language_defn): Ditto.
>         * p-lang.c (pascal_language_defn): Ditto.
>         * scm-lang.c (scm_language_defn): Ditto.
>         * eval.c (evaluate_subexp_standard): Assume Fortran arrays are
>         oriented large to small in type structure.
>          
> 
> 
> On Wed, 2004-08-18 at 17:32, Jim Blandy wrote:
> > Thanks for the revisions!
> > 
> > Any reason read_array_ordering can't return 'enum
> > dwarf_array_dim_ordering' instead of 'int'?
> > 
> > The special case for GNU F77 needs a comment --- just explaining that
> > F77 does things backwards as of Aug 2004, etc. etc.  Basically what
> > you said in your original post, explaining why the special case was
> > necessary.
> > 
> > With those changes, the dwarf2read.c portions of this change are
> > approved.  So I think the eval.c stuff is all that's left for someone
> > to review.

Re: [PATCH/RFA] PR gdb/648

[Andrew Cagney]

> So I think the eval.c stuff is all that's left for someone
> to review.

Jim, can you handle this?  Since you're most familar with the change it 
will hopefully be straight forward.

Andrew

Re: [PATCH/RFA] PR gdb/648

[Jim Blandy]

Andrew Cagney <cagney@gnu.org> writes:
> > So I think the eval.c stuff is all that's left for someone
> > to review.
> 
> Jim, can you handle this?  Since you're most familar with the change
> it will hopefully be straight forward.

Sure.

Re: [PATCH/RFA] PR gdb/648 (eval.c approval reqd)

[Jim Blandy]

So, I'm just trying to get my head around what's going on here.  Would
you say the following comment change would be correct?

*** gdbtypes.h.~1.58.~	2004-08-09 17:42:14.000000000 -0500
--- gdbtypes.h	2004-08-25 00:51:03.000000000 -0500
*************** enum type_code
*** 82,88 ****
    {
      TYPE_CODE_UNDEF,		/* Not used; catches errors */
      TYPE_CODE_PTR,		/* Pointer type */
!     TYPE_CODE_ARRAY,		/* Array type with lower & upper bounds. */
      TYPE_CODE_STRUCT,		/* C struct or Pascal record */
      TYPE_CODE_UNION,		/* C union or Pascal variant part */
      TYPE_CODE_ENUM,		/* Enumeration type */
--- 82,108 ----
    {
      TYPE_CODE_UNDEF,		/* Not used; catches errors */
      TYPE_CODE_PTR,		/* Pointer type */
! 
!     /* Array type with lower & upper bounds.
! 
!        Regardless of the language, GDB represents multidimensional
!        array types the way C does: as arrays of arrays.  So an
!        instance of a GDB array type T can always be seen as a series
!        of instances of TYPE_TARGET_TYPE (T) laid out sequentially in
!        memory.
! 
!        Row-major languages like C lay out multi-dimensional arrays so
!        that incrementing the rightmost index in a subscripting
!        expression results in the smallest change in the address of the
!        element referred to.  Column-major languages like Fortran lay
!        them out so that incrementing the leftmost index results in the
!        smallest change.
! 
!        This means that, in column-major languages, working our way
!        from type to target type corresponds to working through indices
!        from right to left, not left to right.  */
!     TYPE_CODE_ARRAY,
! 
      TYPE_CODE_STRUCT,		/* C struct or Pascal record */
      TYPE_CODE_UNION,		/* C union or Pascal variant part */
      TYPE_CODE_ENUM,		/* Enumeration type */

Re: [PATCH/RFA] PR gdb/648 (eval.c approval reqd)

[David Lecomber]

Hi Jim, 

Yes, that's correct.  

Cheers
David

On Wed, 2004-08-25 at 06:52, Jim Blandy wrote:
> So, I'm just trying to get my head around what's going on here.  Would
> you say the following comment change would be correct?
> 
> *** gdbtypes.h.~1.58.~	2004-08-09 17:42:14.000000000 -0500
> --- gdbtypes.h	2004-08-25 00:51:03.000000000 -0500
> *************** enum type_code
> *** 82,88 ****
>     {
>       TYPE_CODE_UNDEF,		/* Not used; catches errors */
>       TYPE_CODE_PTR,		/* Pointer type */
> !     TYPE_CODE_ARRAY,		/* Array type with lower & upper bounds. */
>       TYPE_CODE_STRUCT,		/* C struct or Pascal record */
>       TYPE_CODE_UNION,		/* C union or Pascal variant part */
>       TYPE_CODE_ENUM,		/* Enumeration type */
> --- 82,108 ----
>     {
>       TYPE_CODE_UNDEF,		/* Not used; catches errors */
>       TYPE_CODE_PTR,		/* Pointer type */
> ! 
> !     /* Array type with lower & upper bounds.
> ! 
> !        Regardless of the language, GDB represents multidimensional
> !        array types the way C does: as arrays of arrays.  So an
> !        instance of a GDB array type T can always be seen as a series
> !        of instances of TYPE_TARGET_TYPE (T) laid out sequentially in
> !        memory.
> ! 
> !        Row-major languages like C lay out multi-dimensional arrays so
> !        that incrementing the rightmost index in a subscripting
> !        expression results in the smallest change in the address of the
> !        element referred to.  Column-major languages like Fortran lay
> !        them out so that incrementing the leftmost index results in the
> !        smallest change.
> ! 
> !        This means that, in column-major languages, working our way
> !        from type to target type corresponds to working through indices
> !        from right to left, not left to right.  */
> !     TYPE_CODE_ARRAY,
> ! 
>       TYPE_CODE_STRUCT,		/* C struct or Pascal record */
>       TYPE_CODE_UNION,		/* C union or Pascal variant part */
>       TYPE_CODE_ENUM,		/* Enumeration type */

Re: [PATCH/RFA] PR gdb/648 (eval.c approval reqd)

[Jim Blandy]

In your original post, you said, "G77 puts things in row-major order".
It looks to me as if G77 puts the *types* in row-major order (contrary
to the Dwarf spec, which wants them in the order they appear in source
code), but it locates the actual arrays in memory in column-major
order:

    (gdb) b arrays.f:45
    Breakpoint 1 at 0x8048769: file arrays.f, line 45.
    (gdb) r
    Starting program: /rigel/jimb/gdb/bugs/648/arrays

    Breakpoint 1, MAIN__ () at arrays.f:46
    46            call oneif(onedi)
    Current language:  auto; currently fortran
    (gdb) x/40wd &twodi
    0xbfffe600:     1       2       3       4
    0xbfffe610:     5       6       7       8
    0xbfffe620:     9       10      11      12
    0xbfffe630:     13      14      15      16
    0xbfffe640:     17      18      19      20
    0xbfffe650:     2       4       6       8
    0xbfffe660:     10      12      14      16
    0xbfffe670:     18      20      22      24
    0xbfffe680:     26      28      30      32
    0xbfffe690:     34      36      38      40
    (gdb) ptype twodi
    type = integer (20,10)
    (gdb)

This shows that varying the leftmost index yields the smallest change
in element address, which is column-major.  So the arrays are actually
laid out correctly; it's just the types in the Dwarf 2 info that are
wrong.

If that's all correct, then this patch is approved.

It seems to me that there's a separate bug in f-valprint.c.  The
output below is incorrect, right?  It's printed as a series of
columns, not a series of rows.  Or is that what users of a Fortran
debugger would expect?

    (gdb) p twodi
    $1 = (( 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18,
    19, 20) ( 2, 4, 6, 8, 10, 12, 14, 16, 18, 20, 22, 24, 26, 28, 30, 32,
    34, 36, 38, 40) ( 3, 6, 9, 12, 15, 18, 21, 24, 27, 30, 33, 36, 39, 42,
    45, 48, 51, 54, 57, 60) ( 4, 8, 12, 16, 20, 24, 28, 32, 36, 40, 44,
    48, 52, 56, 60, 64, 68, 72, 76, 80) ( 5, 10, 15, 20, 25, 30, 35, 40,
    45, 50, 55, 60, 65, 70, 75, 80, 85, 90, 95, 100) ( 6, 12, 18, 24, 30,
    36, 42, 48, 54, 60, 66, 72, 78, 84, 90, 96, 102, 108, 114, 120) ( 7,
    14, 21, 28, 35, 42, 49, 56, 63, 70, 77, 84, 91, 98, 105, 112, 119,
    126, 133, 140) ( 8, 16, 24, 32, 40, 48, 56, 64, 72, 80, 88, 96, 104,
    112, 120, 128, 136, 144, 152, 160) ( 9, 18, 27, 36, 45, 54, 63, 72,
    81, 90, 99, 108, 117, 126, 135, 144, 153, 162, 171, 180) ( 10, 20, 30,
    40, 50, 60, 70, 80, 90, 100, 110, 120, 130, 140, 150, 160, 170, 180,
    190, 200) )
    (gdb)

Re: [PATCH/RFA] PR gdb/648 (eval.c approval reqd)

[David Lecomber]

Hi Jim


On Fri, 2004-08-27 at 00:59, Jim Blandy wrote:
> In your original post, you said, "G77 puts things in row-major order".
> It looks to me as if G77 puts the *types* in row-major order (contrary
> to the Dwarf spec, which wants them in the order they appear in source
> code), but it locates the actual arrays in memory in column-major
> order:
[..]
> This shows that varying the leftmost index yields the smallest change
> in element address, which is column-major.  So the arrays are actually
> laid out correctly; it's just the types in the Dwarf 2 info that are
> wrong.

Yes -- I'm in agreement here, although I hadn't actually thought of that
before.  If that weren't the case, then it wouldn't be possible to link
g77 output with other compilers,..

> If that's all correct, then this patch is approved.

Good-o,

> It seems to me that there's a separate bug in f-valprint.c.  The
> output below is incorrect, right?  It's printed as a series of
> columns, not a series of rows.  Or is that what users of a Fortran
> debugger would expect?
> 
>     (gdb) p twodi
>     $1 = (( 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18,
>     19, 20) ( 2, 4, 6, 8, 10, 12, 14, 16, 18, 20, 22, 24, 26, 28, 30, 32,
>     34, 36, 38, 40) ( 3, 6, 9, 12, 15, 18, 21, 24, 27, 30, 33, 36, 39, 42,
> [..]

A Fortran user would expect this like this, so it's not a bug.  Suppose
a Fortran-er is working with an array of 3-d points, for some finite
element application.  To ensure the x,y,z are in adjacent memory for
better cache behaviour, there'd be an array of REAL(3, 100000) -- which
when printed by GDB will produce a list of points (x, y, z). They'd want
that.

Cheers
David

Re: [PATCH/RFA] PR gdb/648 (eval.c approval reqd)

[Andrew Cagney]

>>> It seems to me that there's a separate bug in f-valprint.c.  The
>>> output below is incorrect, right?  It's printed as a series of
>>> columns, not a series of rows.  Or is that what users of a Fortran
>>> debugger would expect?
>>> 
>>>     (gdb) p twodi
>>>     $1 = (( 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18,
>>>     19, 20) ( 2, 4, 6, 8, 10, 12, 14, 16, 18, 20, 22, 24, 26, 28, 30, 32,
>>>     34, 36, 38, 40) ( 3, 6, 9, 12, 15, 18, 21, 24, 27, 30, 33, 36, 39, 42,
>>> [..]
> 
> 
> A Fortran user would expect this like this, so it's not a bug.  Suppose
> a Fortran-er is working with an array of 3-d points, for some finite
> element application.  To ensure the x,y,z are in adjacent memory for
> better cache behaviour, there'd be an array of REAL(3, 100000) -- which
> when printed by GDB will produce a list of points (x, y, z). They'd want
> that.

Where were we at with the testcase?  I've a fuzzy memory of it hitting a 
problem with test infrastructure?

Andrew

Re: [PATCH/RFA] PR gdb/648 (eval.c approval reqd)

[Michael Chastain]

Andrew Cagney <cagney@gnu.org> wrote:
> Where were we at with the testcase?  I've a fuzzy memory of it hitting a 
> problem with test infrastructure?

Yeah, big time.

There are no FORTRAN programs in the test suite.

Before I add any FORTRAN programs, I am going to overhaul the code
that connects "language type" (such as c or f77) to "compiler name"
(such as gcc or g77).

I've started that twice now, but it's a big ball of hair.

Michael

Re: [PATCH/RFA] PR gdb/648 (eval.c approval reqd)

[Andrew Cagney]

> Andrew Cagney <cagney@gnu.org> wrote:
> 
>>> Where were we at with the testcase?  I've a fuzzy memory of it hitting a 
>>> problem with test infrastructure?
> 
> 
> Yeah, big time.
> 
> There are no FORTRAN programs in the test suite.
> 
> Before I add any FORTRAN programs, I am going to overhaul the code
> that connects "language type" (such as c or f77) to "compiler name"
> (such as gcc or g77).
> 
> I've started that twice now, but it's a big ball of hair.

Is there anything we can do in the interum.  Even a gdb.fortran tmp-hack 
to run fortran directly?  Once the infrastructure is in place, that can 
easily be replaced.

Its important that, as these new language features get added, the tests 
demonstrating that they work, also get included.

Andrew

Re: [PATCH/RFA] PR gdb/648 (eval.c approval reqd)

[Michael Chastain]

I'm going to fix a lot of infrastructure before adding the first
FORTRAN program.

After years of infrastructure neglect, I'm not going to add more
features that depend on crappy infrastructure.  Nobody's cleaned up the
last round of code duplication from adding ADA yet.

And with FORTRAN in particular, people want to test non-gcc
FORTRAN compilers.

Re: [PATCH/RFA] PR gdb/648 (eval.c approval reqd)

[Andrew Cagney]

I can see your point, and it's valid.

I was thinking of a very simple procedure:

	proc deprecated_compile_with_gnu_fortran { program } { .... }

and have the fortran tests use that.

Attatched to the procedure would be a comment stating that anyone 
wanting to modify the testsuite to support non-GNU fortran will need to 
implement the infrastructure you describe described.

That way the fortran test directory can be populated with tests 
simultaneous to, or even prior to, adding the feature.  While at the 
same time, establishing a very clear acceptance criteria for anyone 
wanting to hack things for non-GNU compilers.

how does this sound?
Andrew

Re: [PATCH/RFA] PR gdb/648 (eval.c approval reqd)

[Michael Chastain]

Andrew Cagney <cagney@gnu.org> wrote:
> 	proc deprecated_compile_with_gnu_fortran { program } { .... }

Ewww.

But you do have a lot of experience in gdb balancing feature additions
with infrastructure cleanup, so I have to give some weight to that.

So, okay.  If you, or somebody, wants to do that, go for it.

. put it in a new file, lib/fortran-support.exp.
. the deprecated_* name is a nice touch.

Michael

Re: [PATCH/RFA] PR gdb/648 (eval.c approval reqd)

[Andrew Cagney]

> Andrew Cagney <cagney@gnu.org> wrote:
> 
>>> 	proc deprecated_compile_with_gnu_fortran { program } { .... }
> 
> 
> Ewww.

*2 :-)

> But you do have a lot of experience in gdb balancing feature additions
> with infrastructure cleanup, so I have to give some weight to that.
> 
> So, okay.  If you, or somebody, wants to do that, go for it.
> 
> . put it in a new file, lib/fortran-support.exp.
> . the deprecated_* name is a nice touch.

David, this makes the testcase possible, can that be added.

Andrew