off-trunk gdb.fortran/dwarf-stride.exp no longer PASSes [Re: [PATCH 0/2] fort_dyn_array: Enable basic Fortran dynamic array support]

Message ID 20151027212954.GA19347@host1.jankratochvil.net
State New, archived
Headers

Commit Message

Jan Kratochvil Oct. 27, 2015, 9:29 p.m. UTC
  [ Now with the testfile. ]
On Tue, 27 Oct 2015 22:28:31 +0100, Jan Kratochvil wrote:

Hello,

this is not a regression for FSF GDB; but at least these Fedora versions
	gdb-7.8.2-39.fc21.x86_64
	gdb-7.9.1-17.fc22.x86_64
	gdb-7.10-29.fc24.x86_64
PASS the attached testcase
	p c40pt(1)^M
	$1 = '0-hello', ' ' <repeats 33 times>^M
	(gdb) PASS: gdb.fortran/dwarf-stride.exp: p c40pt(1)
	p c40pt(2)^M
	$2 = '1-hello', ' ' <repeats 33 times>^M
	(gdb) PASS: gdb.fortran/dwarf-stride.exp: p c40pt(2)
while current FSF trunk b80c3053162ec5533e120ee4e4ed30296d4c5fb2 FAILs on:
	p c40pt(1)^M
	$1 = '0-hello', ' ' <repeats 33 times>^M
	(gdb) PASS: gdb.fortran/dwarf-stride.exp: p c40pt(1)
	p c40pt(2)^M
	$2 = '\001\000\000\000\061-hello', ' ' <repeats 29 times>^M
	(gdb) FAIL: gdb.fortran/dwarf-stride.exp: p c40pt(2)

Curiously the Fedora GDB versions are based on:
	[PATCH 00/23] Fortran dynamic array support
	https://sourceware.org/ml/gdb-patches/2014-06/msg00108.html
	https://github.com/intel-gdb/vla/tree/vla-fortran
	commit 511bff520372ffc10fa2ff569c176bdf1e6e475d
	(although I cannot find that commit hash in that github repo now)

Couldn't the Intel patchset regress this testcase during its upstreaming?

I haven't yet tried to really fix/debug it.


Thanks,
Jan
  

Comments

Joel Brobecker Oct. 28, 2015, 4:19 p.m. UTC | #1
> Couldn't the Intel patchset regress this testcase during its upstreaming?
> I haven't yet tried to really fix/debug it.

Possibly, but I don't know. Keven, can you take a look, please?
  
Jan Kratochvil Oct. 29, 2015, 10:36 p.m. UTC | #2
On Wed, 28 Oct 2015 17:19:07 +0100, Joel Brobecker wrote:
> Possibly, but I don't know. Keven, can you take a look, please?

Apparently the VLA patchset is not yet checked-in completely, for example the
off-trunk patchset formerly contained:

   attr = dwarf2_attr (die, DW_AT_string_length, cu);
   if (attr)
     {
-      length = DW_UNSND (attr);
+      if (attr_form_is_block (attr))
+        {

but current FSF GDB HEAD does not yet recognize attr_form_is_block for
DW_AT_string_length at all so it just cannot work yet.

I will have to apply only remaining parts of the former off-trunk patchset but
that is sure off-topic for upstream GDB.


Jan
  
Joel Brobecker Nov. 4, 2015, 9:48 p.m. UTC | #3
> Apparently the VLA patchset is not yet checked-in completely, for example the
> off-trunk patchset formerly contained:
> 
>    attr = dwarf2_attr (die, DW_AT_string_length, cu);
>    if (attr)
>      {
> -      length = DW_UNSND (attr);
> +      if (attr_form_is_block (attr))
> +        {
> 
> but current FSF GDB HEAD does not yet recognize attr_form_is_block for
> DW_AT_string_length at all so it just cannot work yet.
> 
> I will have to apply only remaining parts of the former off-trunk patchset but
> that is sure off-topic for upstream GDB.

Just to be sure - did we regress on something in the master branch?
Or is this just a regression on your branch when you only rely on
the changes that were made on master?

The changes that were first submitted early on in the process were
too big, and I suspect some of them were also OBE, so I asked that
the patches be submitted piecemeal, to allow me to understand why
each change was made. This allowed us to eventually get a good chunk
of the work in, but as you can see, some chunks appear to still be
missing...
  
Jan Kratochvil Nov. 4, 2015, 10:08 p.m. UTC | #4
On Wed, 04 Nov 2015 22:48:15 +0100, Joel Brobecker wrote:
> Just to be sure - did we regress on something in the master branch?

No.


> Or is this just a regression on your branch when you only rely on
> the changes that were made on master?

Yes.  While it is offtopic for upstream the current Fedora Rawhide
(gdb-7.10.50.20151027-30.fc24) is now rebased on top of FSF GDB master not
regressing the VLAs.


> This allowed us to eventually get a good chunk of the work in, but as you
> can see, some chunks appear to still be missing...

http://pkgs.fedoraproject.org/cgit/gdb.git/tree/gdb-vla-intel.patch

Although maybe there is some dead code/data in the patch, not sure.


Jan
  
Keven Boell Nov. 5, 2015, 7:31 a.m. UTC | #5
Hi Jan,

As you have already seen, only the basic Fortran VLA support was merged to the repository.
If you like I can also send the updated remaining Fortran VLA patches to the mailing list.

Keven

On 04.11.2015 23:08, Jan Kratochvil wrote:
> On Wed, 04 Nov 2015 22:48:15 +0100, Joel Brobecker wrote:
>> Just to be sure - did we regress on something in the master branch?
> 
> No.
> 
> 
>> Or is this just a regression on your branch when you only rely on
>> the changes that were made on master?
> 
> Yes.  While it is offtopic for upstream the current Fedora Rawhide
> (gdb-7.10.50.20151027-30.fc24) is now rebased on top of FSF GDB master not
> regressing the VLAs.
> 
> 
>> This allowed us to eventually get a good chunk of the work in, but as you
>> can see, some chunks appear to still be missing...
> 
> http://pkgs.fedoraproject.org/cgit/gdb.git/tree/gdb-vla-intel.patch
> 
> Although maybe there is some dead code/data in the patch, not sure.
> 
> 
> Jan
>
  
Jan Kratochvil Nov. 5, 2015, 8:22 a.m. UTC | #6
Hi Keven,

On Thu, 05 Nov 2015 08:31:47 +0100, Keven Boell wrote:
> As you have already seen, only the basic Fortran VLA support was merged to
> the repository.

OK, thanks.


> If you like I can also send the updated remaining Fortran VLA patches to the
> mailing list.

Unaware what is your plan with them (wrt upstreaming) but sure it would be
helpful at least for me as they may be in a better shape than what I keep
rebasing since your 2014-06 post.

Besides that mentioned
	http://pkgs.fedoraproject.org/cgit/gdb.git/tree/gdb-vla-intel.patch
I carry also these fixups:
	http://pkgs.fedoraproject.org/cgit/gdb.git/tree/gdb-vla-intel-logical-not.patch
	http://pkgs.fedoraproject.org/cgit/gdb.git/tree/gdb-vla-intel-stringbt-fix.patch
	http://pkgs.fedoraproject.org/cgit/gdb.git/tree/gdb-vla-intel-04of23-fix.patch


Thanks,
Jan
  
Keven Boell Nov. 5, 2015, 4:04 p.m. UTC | #7
Hi Jan,

Here is the next patch from the series:
	https://sourceware.org/ml/gdb-patches/2015-11/msg00186.html

Keven

On 05.11.2015 09:22, Jan Kratochvil wrote:
> Hi Keven,
> 
> On Thu, 05 Nov 2015 08:31:47 +0100, Keven Boell wrote:
>> As you have already seen, only the basic Fortran VLA support was merged to
>> the repository.
> 
> OK, thanks.
> 
> 
>> If you like I can also send the updated remaining Fortran VLA patches to the
>> mailing list.
> 
> Unaware what is your plan with them (wrt upstreaming) but sure it would be
> helpful at least for me as they may be in a better shape than what I keep
> rebasing since your 2014-06 post.
> 
> Besides that mentioned
> 	http://pkgs.fedoraproject.org/cgit/gdb.git/tree/gdb-vla-intel.patch
> I carry also these fixups:
> 	http://pkgs.fedoraproject.org/cgit/gdb.git/tree/gdb-vla-intel-logical-not.patch
> 	http://pkgs.fedoraproject.org/cgit/gdb.git/tree/gdb-vla-intel-stringbt-fix.patch
> 	http://pkgs.fedoraproject.org/cgit/gdb.git/tree/gdb-vla-intel-04of23-fix.patch
> 
> 
> Thanks,
> Jan
>
  

Patch

diff --git a/gdb/testsuite/gdb.fortran/dwarf-stride.exp b/gdb/testsuite/gdb.fortran/dwarf-stride.exp
new file mode 100644
index 0000000..d7b8bea
--- /dev/null
+++ b/gdb/testsuite/gdb.fortran/dwarf-stride.exp
@@ -0,0 +1,42 @@ 
+# Copyright 2009 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 2 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, write to the Free Software
+# Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.  
+
+# This file was written by Jan Kratochvil <jan.kratochvil@redhat.com>.
+
+# This file is part of the gdb testsuite.  Array element stride must not be
+# specified in the number of elements but in a number of bytes instead.
+# Original problem:
+# (gdb) p c40pt(1)
+# $1 = '0-hello', ' ' <repeats 33 times>
+# (gdb) p c40pt(2)
+# warning: Fortran array stride not divisible by the element size
+
+set testfile dwarf-stride
+set srcfile ${testfile}.f90
+
+if { [prepare_for_testing ${testfile}.exp ${testfile} ${srcfile} {debug f90}] } {
+    return -1
+}
+
+if ![runto MAIN__] then {
+    perror "couldn't run to breakpoint MAIN__"
+    continue
+}
+
+gdb_breakpoint [gdb_get_line_number "break-here"]
+gdb_continue_to_breakpoint "break-here" ".*break-here.*"
+gdb_test "p c40pt(1)" " = '0-hello.*"
+gdb_test "p c40pt(2)" " = '1-hello.*"
diff --git a/gdb/testsuite/gdb.fortran/dwarf-stride.f90 b/gdb/testsuite/gdb.fortran/dwarf-stride.f90
new file mode 100644
index 0000000..e492b3a
--- /dev/null
+++ b/gdb/testsuite/gdb.fortran/dwarf-stride.f90
@@ -0,0 +1,40 @@ 
+! Copyright 2009 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 2 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, write to the Free Software
+! Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+!
+! File written by Alan Matsuoka.
+
+program repro
+
+  type small_stride
+     character*40 long_string
+     integer      small_pad
+  end type small_stride
+
+  type(small_stride), dimension (20), target :: unpleasant
+  character*40, pointer, dimension(:):: c40pt
+
+  integer i
+
+  do i = 0,19
+     unpleasant(i+1)%small_pad = i+1
+     unpleasant(i+1)%long_string = char (ichar('0') + i) // '-hello'
+  end do
+
+  c40pt => unpleasant%long_string
+
+  print *, c40pt  ! break-here
+
+end program repro