diff mbox

[V4,00/21] Fortran dynamic array support

Message ID 20160816135920.GA26624@host1.jankratochvil.net
State New
Headers show

Commit Message

Jan Kratochvil Aug. 16, 2016, 1:59 p.m. UTC 
Hi Bernhard,

On Tue, 09 Aug 2016 09:55:05 +0200, Heckel, Bernhard wrote:
> I took a look at "p varw" issue.
> 
> >From my point of view, the debug information for  the location of varw is wrong. 
> 
> Set a breakpoint at line 68 and run.
> "P &varw" in GDB gives me same location as the print of "loc(z(2,4,6))" out of the fortran testcase.
> Nevertheless, "loc(varw)" shows me a different location. Investigating on the dwarf debug info, GDB address calculation
> of "varw" is correct. From my point of view, the location expression of "varw" is wrong.
> 
> If you manual examine the content at "loc(varw)" you see the right content.
> 
> FYI: I added a variable "dummy" in subroutine "foo" in order to get the content of fbreg and do manual address calculation. 
> 
> I attached the testcase + dwarf

primarily Fedora 24 GDB (and many Fedora releases back) - with the older Intel
VLA patch series - does PASS the testcase fine with gfortran DWARF so
completely wrong the DWARF is not.

Also you talk here only about starting address of 'varw'.  But that is not
a problem, the starting address in inferior + in GDB do match:
  varw(1, 1, 1) = 6
vs.
$23 = (( ( 6, 5, 1, 5, 5) ( 3, 3, 3, 5, 5) ( 5, 5, 5, 3, 3) ( 3, 5, 5, 5, 5) ) ( ( 5, 3, 3, 3, 5) ( 5, 5, 5, 5, 3) ( 3, 3, 3, 3, 3) ( 3, 3, 3, 3, 3) ) ( ( 3, 3, 3, 3, 3) ( 3, 3, 3, 3, 3) ( 3, 3, 3, 3, 3) ( 3, 3, 3, 3, 3) ) )
(gdb) FAIL: gdb.fortran/dynamic.exp: p varw filled
 - the value 6 is really the first (1,1,1) value printed.
The starting address is nothing special for GDB as starting address is
adjusted/offseted by the inferior caller, GDB just reads the inferior-computed
address.

What is wrong is accessing 'varw' elements not in the first row of the array
by GDB - because GDB needs to know the stride for it.
Stride is described there in DWARF:
    <231>   DW_AT_upper_bound : 11 byte block: 31 97 23 28 6 97 23 20 6 1c 22   (DW_OP_lit1; DW_OP_push_object_address; DW_OP_plus_uconst: 40; DW_OP_deref; DW_OP_push_object_address; DW_OP_plus_uconst: 32; DW_OP_deref; DW_OP_minus; DW_OP_plus)
    <23d>   DW_AT_byte_stride : 6 byte block: 97 23 18 6 34 1e  (DW_OP_push_object_address; DW_OP_plus_uconst: 24; DW_OP_deref; DW_OP_lit4; DW_OP_mul)
and that just fetches the values from gfortran array descriptor - otherwise the
inferior Fortran function 'foo' would not work.

Why do you think gfortran has the DWARF wrong?  Do your GDB patches PASS with
ifort?  I no longer have ifort - I got a license from Markus Metzger many
years ago, it was somehow complicated in Intel to get it for me and the
license laster only one year.  You would have to send me the ifort-built
binary (or .s file).


I have tried now to re-apply a part of the old working Intel VLA patch of
value_subscripted_rvalue():
   unsigned int elt_size = TYPE_LENGTH (elt_type);
-  unsigned int elt_offs = elt_size * longest_to_int (index - lowerbound);
+  unsigned int elt_offs = longest_to_int (index - lowerbound);
+  LONGEST elt_stride = TYPE_BYTE_STRIDE (TYPE_INDEX_TYPE (array_type));
   struct value *v;

+  if (elt_stride > 0)
+    elt_offs *= elt_stride;
+  else if (elt_stride < 0)
+    {
+      int offs = (elt_offs + 1) * elt_stride;
+
+      elt_offs = TYPE_LENGTH (array_type) + offs;
+    }
+  else
+    elt_offs *= elt_size;
+

But it does not work (attached).  Maybe because the stride of an array now
appears to be stored into TYPE_FIELD_BITSIZE of the array itself (not its type)
while with old Intel VLA patchset there was a special field TYPE_BYTE_STRIDE
for that.


Thanks,
Jan

Comments

Heckel, Bernhard Aug. 19, 2016, 9:58 a.m. UTC | #1 
On 16/08/2016 15:59, Jan Kratochvil wrote:
> Hi Bernhard,
>
> On Tue, 09 Aug 2016 09:55:05 +0200, Heckel, Bernhard wrote:
>> I took a look at "p varw" issue.
>>
>> >From my point of view, the debug information for  the location of varw is wrong.
>>
>> Set a breakpoint at line 68 and run.
>> "P &varw" in GDB gives me same location as the print of "loc(z(2,4,6))" out of the fortran testcase.
>> Nevertheless, "loc(varw)" shows me a different location. Investigating on the dwarf debug info, GDB address calculation
>> of "varw" is correct. From my point of view, the location expression of "varw" is wrong.
>>
>> If you manual examine the content at "loc(varw)" you see the right content.
>>
>> FYI: I added a variable "dummy" in subroutine "foo" in order to get the content of fbreg and do manual address calculation.
>>
>> I attached the testcase + dwarf
> primarily Fedora 24 GDB (and many Fedora releases back) - with the older Intel
> VLA patch series - does PASS the testcase fine with gfortran DWARF so
> completely wrong the DWARF is not.
>
> Also you talk here only about starting address of 'varw'.  But that is not
> a problem, the starting address in inferior + in GDB do match:
>    varw(1, 1, 1) = 6
> vs.
> $23 = (( ( 6, 5, 1, 5, 5) ( 3, 3, 3, 5, 5) ( 5, 5, 5, 3, 3) ( 3, 5, 5, 5, 5) ) ( ( 5, 3, 3, 3, 5) ( 5, 5, 5, 5, 3) ( 3, 3, 3, 3, 3) ( 3, 3, 3, 3, 3) ) ( ( 3, 3, 3, 3, 3) ( 3, 3, 3, 3, 3) ( 3, 3, 3, 3, 3) ( 3, 3, 3, 3, 3) ) )
> (gdb) FAIL: gdb.fortran/dynamic.exp: p varw filled
>   - the value 6 is really the first (1,1,1) value printed.
> The starting address is nothing special for GDB as starting address is
> adjusted/offseted by the inferior caller, GDB just reads the inferior-computed
> address.
>
> What is wrong is accessing 'varw' elements not in the first row of the array
> by GDB - because GDB needs to know the stride for it.
> Stride is described there in DWARF:
>      <231>   DW_AT_upper_bound : 11 byte block: 31 97 23 28 6 97 23 20 6 1c 22   (DW_OP_lit1; DW_OP_push_object_address; DW_OP_plus_uconst: 40; DW_OP_deref; DW_OP_push_object_address; DW_OP_plus_uconst: 32; DW_OP_deref; DW_OP_minus; DW_OP_plus)
>      <23d>   DW_AT_byte_stride : 6 byte block: 97 23 18 6 34 1e  (DW_OP_push_object_address; DW_OP_plus_uconst: 24; DW_OP_deref; DW_OP_lit4; DW_OP_mul)
> and that just fetches the values from gfortran array descriptor - otherwise the
> inferior Fortran function 'foo' would not work.
>
> Why do you think gfortran has the DWARF wrong?  Do your GDB patches PASS with
> ifort?  I no longer have ifort - I got a license from Markus Metzger many
> years ago, it was somehow complicated in Intel to get it for me and the
> license laster only one year.  You would have to send me the ifort-built
> binary (or .s file).
>
>
> I have tried now to re-apply a part of the old working Intel VLA patch of
> value_subscripted_rvalue():
>     unsigned int elt_size = TYPE_LENGTH (elt_type);
> -  unsigned int elt_offs = elt_size * longest_to_int (index - lowerbound);
> +  unsigned int elt_offs = longest_to_int (index - lowerbound);
> +  LONGEST elt_stride = TYPE_BYTE_STRIDE (TYPE_INDEX_TYPE (array_type));
>     struct value *v;
>
> +  if (elt_stride > 0)
> +    elt_offs *= elt_stride;
> +  else if (elt_stride < 0)
> +    {
> +      int offs = (elt_offs + 1) * elt_stride;
> +
> +      elt_offs = TYPE_LENGTH (array_type) + offs;
> +    }
> +  else
> +    elt_offs *= elt_size;
> +
>
> But it does not work (attached).  Maybe because the stride of an array now
> appears to be stored into TYPE_FIELD_BITSIZE of the array itself (not its type)
> while with old Intel VLA patchset there was a special field TYPE_BYTE_STRIDE
> for that.
>
>
> Thanks,
> Jan
Hi Jan,

here is the missing patch in your environment.
https://sourceware.org/ml/gdb-patches/2015-01/msg00385.html
This patch handles strides in DWARF and your fortran program.

Christoph's patch series was about strides in user input, for example 
"print my_vla(1:10:2)"

I will create a branch about all stride patches in the next weeks.



Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
Jan Kratochvil Aug. 21, 2016, 5:03 p.m. UTC | #2 
On Fri, 19 Aug 2016 11:58:09 +0200, Bernhard Heckel wrote:
> here is the missing patch in your environment.
> https://sourceware.org/ml/gdb-patches/2015-01/msg00385.html
> This patch handles strides in DWARF and your fortran program.

OK, thanks, it does fix:
	-FAIL: gdb.fortran/dynamic.exp: p varw filled
	+PASS: gdb.fortran/dynamic.exp: p varw filled
	-FAIL: gdb.fortran/vla-stride.exp: print odd-elements
	+PASS: gdb.fortran/vla-stride.exp: print odd-elements
	-FAIL: gdb.fortran/vla-stride.exp: print last odd-element
	+PASS: gdb.fortran/vla-stride.exp: print last odd-element

Although it regresses:
	 print pvla^M
	-$7 = (5)^M
	-(gdb) PASS: gdb.fortran/vla-stride.exp: print single-element
	+value requires 4294967288 bytes, which is more than max-value-size^M
	+(gdb) FAIL: gdb.fortran/vla-stride.exp: print single-element
	-$8 = 5^M
	-(gdb) PASS: gdb.fortran/vla-stride.exp: print one single-element
	+Insufficient memory in host GDB for object of size 4294967288 bytes, maximum allowed 536870911 bytes.^M
	+(gdb) FAIL: gdb.fortran/vla-stride.exp: print one single-element


> I will create a branch about all stride patches in the next weeks.

That would be probably best, thanks.


Jan
Heckel, Bernhard Aug. 23, 2016, 1:34 p.m. UTC | #3 
On 21/08/2016 19:03, Jan Kratochvil wrote:
> On Fri, 19 Aug 2016 11:58:09 +0200, Bernhard Heckel wrote:
>> here is the missing patch in your environment.
>> https://sourceware.org/ml/gdb-patches/2015-01/msg00385.html
>> This patch handles strides in DWARF and your fortran program.
> OK, thanks, it does fix:
> 	-FAIL: gdb.fortran/dynamic.exp: p varw filled
> 	+PASS: gdb.fortran/dynamic.exp: p varw filled
> 	-FAIL: gdb.fortran/vla-stride.exp: print odd-elements
> 	+PASS: gdb.fortran/vla-stride.exp: print odd-elements
> 	-FAIL: gdb.fortran/vla-stride.exp: print last odd-element
> 	+PASS: gdb.fortran/vla-stride.exp: print last odd-element
>
> Although it regresses:
> 	 print pvla^M
> 	-$7 = (5)^M
> 	-(gdb) PASS: gdb.fortran/vla-stride.exp: print single-element
> 	+value requires 4294967288 bytes, which is more than max-value-size^M
> 	+(gdb) FAIL: gdb.fortran/vla-stride.exp: print single-element
> 	-$8 = 5^M
> 	-(gdb) PASS: gdb.fortran/vla-stride.exp: print one single-element
> 	+Insufficient memory in host GDB for object of size 4294967288 bytes, maximum allowed 536870911 bytes.^M
> 	+(gdb) FAIL: gdb.fortran/vla-stride.exp: print one single-element
>
>
>> I will create a branch about all stride patches in the next weeks.
> That would be probably best, thanks.
>
>
> Jan
Hi Jan,

created a branch with all stride patches.
I don't see regression on RH7.1, gcc 4.8.3-9


Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
diff mbox

Patch

--- ./gdb/valarith.c	2016-08-16 15:36:10.680313175 +0200
+++ ./gdb/valarith.c	2016-08-16 15:33:18.696930843 +0200
@@ -193,9 +193,22 @@  value_subscripted_rvalue (struct value *
   struct type *array_type = check_typedef (value_type (array));
   struct type *elt_type = check_typedef (TYPE_TARGET_TYPE (array_type));
   ULONGEST elt_size = type_length_units (elt_type);
-  ULONGEST elt_offs = elt_size * (index - lowerbound);
+  ULONGEST elt_offs = index - lowerbound;
+  LONGEST elt_stride = TYPE_FIELD_BITSIZE (array_type, 0);
   struct value *v;
 
+if (elt_stride) fprintf(stderr,"value_subscripted_rvalue: elt_stride=%ld\n",elt_stride);
+  if (elt_stride > 0)
+    elt_offs *= elt_stride;
+  else if (elt_stride < 0)
+    {
+      int offs = (elt_offs + 1) * elt_stride;
+
+      elt_offs = TYPE_LENGTH (array_type) + offs;
+    }
+  else
+    elt_offs *= elt_size;
+
   if (index < lowerbound || (!TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (array_type)
 			     && elt_offs >= type_length_units (array_type)))
     {
--- ./gdb/valops.c	2016-08-16 15:36:10.694313288 +0200
+++ ./gdb/valops.c	2016-08-16 15:04:54.963343948 +0200
@@ -3826,7 +3826,7 @@  value_slice_1 (struct value *array, int
   struct type *elt_type = check_typedef (TYPE_TARGET_TYPE (array_type));
   unsigned int elt_size, elt_offs;
   /* ATTRIBUTE_UNUSED: VLA bug: https://sourceware.org/ml/gdb-patches/2016-08/msg00099.html */
-  LONGEST elt_stride ATTRIBUTE_UNUSED, ary_high_bound, ary_low_bound;
+  LONGEST elt_stride, ary_high_bound, ary_low_bound;
   struct value *v;
   int slice_range_size, i = 0, row_count = 1, elem_count = 1;
 
@@ -3863,7 +3863,17 @@  value_slice_1 (struct value *array, int
   elt_offs = lowbound - ary_low_bound;
   elt_stride = TYPE_LENGTH (TYPE_INDEX_TYPE (array_type));
 
-  elt_offs *= elt_size;
+fprintf(stderr,"elt_stride=%d\n",elt_stride);
+  if (elt_stride > 0)
+    elt_offs *= elt_stride;
+  else if (elt_stride < 0)
+    {
+      int offs = (elt_offs + 1) * elt_stride;
+
+      elt_offs = TYPE_LENGTH (array_type) + offs;
+    }
+  else
+    elt_offs *= elt_size;
 
   /* Check for valid user input.  In case of Fortran this was already done
      in the calling function.  */