Bug 38669 - [4.3/4.4 Regression] Array bounds violation for arguments of elemental subroutine
Summary: [4.3/4.4 Regression] Array bounds violation for arguments of elemental subrou...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: fortran (show other bugs)
Version: 4.3.3
: P4 normal
Target Milestone: 4.3.3
Assignee: Mikael Morin
URL:
Keywords:
Depends on:
Blocks: 32834
  Show dependency treegraph
 
Reported: 2008-12-30 10:31 UTC by Harald Anlauf
Modified: 2010-11-11 00:17 UTC (History)
4 users (show)

See Also:
Host:
Target:
Build:
Known to work: 4.3.2
Known to fail: 4.3.3 4.4.0
Last reconfirmed: 2008-12-30 11:17:36


Attachments
fix (714 bytes, patch)
2008-12-30 16:45 UTC, Mikael Morin
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Harald Anlauf 2008-12-30 10:31:45 UTC
Hi,

the following code dies now with a reported array bounds violation,
although it should be correct and used to work.

% cat gfcbug84.f90
module gfcbug84
  implicit none
contains
  elemental subroutine tq_tvgh (t, p)
    real ,intent (out)            :: t
    real ,intent (in)             :: p
    t=p
  end subroutine tq_tvgh
end module gfcbug84
!-------------------
program gfcbu84_main
  use gfcbug84
  implicit none
  integer           :: jplev, k_lev
  real, allocatable :: p(:)
  type t
     real, pointer  :: av (:,:)
  end type t
  type(t)           :: var_f

  jplev = 42
  k_lev = 1
  allocate (p(jplev))
  allocate (var_f% av(jplev,1))
  var_f% av = 0
  write (0,*) "associated:", associated (var_f% av)
  write (0,*) "shape     :", shape      (var_f% av)
  call tq_tvgh (var_f% av (k_lev:,1), p(k_lev:))
  print *, "OK"
end program gfcbu84_main
% gfc43 gfcbug84.f90 -fbounds-check -O1 -g -fbacktrace
% ./a.out
 associated: T
 shape     :          42           1
At line 28 of file gfcbug84.f90
Fortran runtime error: Array reference out of bounds for array 'var_f', lower bound of dimension 2 exceeded(1 < 4294967296)

Backtrace for this error:
  + function gfcbu84_main (0x4021BD)
    at line 28 of file gfcbug84.f90
  + /lib64/libc.so.6(__libc_start_main+0xe6) [0x2b4825335436]
% gfc44 gfcbug84.f90 -fbounds-check -O0 -g -fbacktrace
./a.out
 associated: T
 shape     :          42           1
At line 28 of file gfcbug84.f90
Fortran runtime error: Array reference out of bounds for array 'var_f', upper bound of dimension 2  exceeded (1 > 0)

Backtrace for this error:
  + function gfcbu84_main (0x4028D1)
    at line 28 of file gfcbug84.f90
  + /lib64/libc.so.6(__libc_start_main+0xe6) [0x2aed6fd0f436]


I have tested with Tobias' snapshots.

Known to work:
gcc-4.3-x86_64-2008-12-21-r142852.tar.gz
gcc-trunk-x86_64-2008-12-15-r142760.tar.gz

Known to fail:
gcc-4.3-x86_64-2008-12-22-r142864.tar.gz
gcc-trunk-x86_64-2008-12-16-r142777.tar.gz

Cheers,
-ha
Comment 1 Daniel Franke 2008-12-30 11:55:08 UTC
Maybe:
r142766 | mikael | 2008-12-15 19:08:42 +0100 (Mon, 15 Dec 2008) | 14 lines

Added Mikael as CC.
Comment 2 Mikael Morin 2008-12-30 14:04:02 UTC
without derived types:

program gfcbu84_main
! use gfcbug84
  implicit none
  integer           :: jplev, k_lev
  real :: p(42)
  real, pointer :: q(:)
  jplev = 42
  k_lev = 1
  allocate (q(jplev))
  call tq_tvgh (q(k_lev:), p(k_lev:))

  contains
  elemental subroutine tq_tvgh (t, p)
    real ,intent (out)            :: t
    real ,intent (in)             :: p
    t=p
  end subroutine tq_tvgh
end program gfcbu84_main




The bound temporaries (D.1559,D.1560) are used before being defined. 

    integer(kind=8) D.1560;
    integer(kind=8) D.1559;
    integer(kind=8) D.1558;
    real(kind=4)[0:] * D.1557;

    D.1563 = (integer(kind=8)) k_lev;
    D.1564 = q.dim[0].ubound;
    parm.2.dtype = 281;
    D.1566 = q.dim[0].stride;
    parm.2.dim[0].lbound = 1;
    parm.2.dim[0].ubound = (1 - D.1563) + D.1564;
    parm.2.dim[0].stride = NON_LVALUE_EXPR <D.1566>;
    parm.2.data = (void *) &(*(real(kind=4)[0:] *) q.data)[(D.1563 - q.dim[0].lbound) * D.1566];
    parm.2.offset = 0;
    D.1569 = D.1560 - D.1559;
    atmp.3.dtype = 281;
    atmp.3.dim[0].stride = 1;
    atmp.3.dim[0].lbound = 0;
    atmp.3.dim[0].ubound = D.1569;
(...)
    D.1559 = (integer(kind=8)) k_lev;
    D.1560 = q.dim[0].ubound;


My commit is probably the culprit (what else?), but I don't understand how it could be.
Comment 3 Mikael Morin 2008-12-30 15:02:53 UTC
At revision 142760, there is no temporary, so there is no bug. 
That's something I missed in my patch, that's true. 
The bug is still there however.   
Change this:
call tq_tvgh (var_f% av (k_lev:,1), p(k_lev:))
to this:
call tq_tvgh (var_f% av (k_lev:,1), (p(k_lev:)))
Comment 4 Mikael Morin 2008-12-30 16:45:13 UTC
Created attachment 17016 [details]
fix

Does anyone know the use of the block variable I remove in this patch?
Comment 5 Paul Thomas 2008-12-31 10:47:22 UTC
(In reply to comment #4)
> Created an attachment (id=17016) [edit]
> fix
> 
> Does anyone know the use of the block variable I remove in this patch?
> 

Yes indeed.  When I wrote this function, this block of code needed to be merged as  the comments says:
	  /* Generate the temporary.  Merge the block so that the
	     declarations are put at the right binding level.  */

The second sentence needs to go in your patch:-)

What I have no recall of is why the block had to be merged.  I believe that it was conditional on the intent of the argument.

Cheers

Paul
Comment 6 Mikael Morin 2009-01-04 00:40:51 UTC
(In reply to comment #5)
> (In reply to comment #4)
> > Created an attachment (id=17016) [edit]
> > fix
> > 
> > Does anyone know the use of the block variable I remove in this patch?
> > 
> 
> Yes indeed.  When I wrote this function, this block of code needed to be merged
> as  the comments says:
>           /* Generate the temporary.  Merge the block so that the
>              declarations are put at the right binding level.  */
> 
> The second sentence needs to go in your patch:-)
> 
> What I have no recall of is why the block had to be merged.  I believe that it
> was conditional on the intent of the argument.

That's not what I was asking, sorry. I'll try to be clearer. 
The block variable seems to hold nothing. 

271    gfc_start_block (&block);
272    tmp = gfc_typenode_for_spec (&e->ts);
273    tmp = gfc_trans_create_temp_array (&se->pre, &se->post,
274                                       &tmp_loop, info, tmp,
275                                       false, true, false);
276    gfc_add_modify_expr (&se->pre, size, tmp);
277    tmp = fold_convert (pvoid_type_node, info->data);
278    gfc_add_modify_expr (&se->pre, data, tmp);
279    gfc_merge_block_scope (&block);

The above is your initial commit. 
As gfc_trans_create_temp_array is putting his code in se->pre and se->post, 
it looks like no code goes to the block variable which is useless then. 
Or do I miss something?

This is largely off-topic though. It is just something I wondered when I looked at the code. 
Comment 7 Mikael Morin 2009-01-04 19:12:36 UTC
Subject: Bug 38669

Author: mikael
Date: Sun Jan  4 19:12:16 2009
New Revision: 143057

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=143057
Log:
2009-01-04  Mikael Morin  <mikael.morin@tele2.fr>

	PR fortran/35681
	* ChangeLog-2008: Fix function name.

	PR fortran/38487
	* dependency.c (gfc_check_argument_var_dependency):
	Move the check for pointerness inside the if block
	so that it doesn't affect the return value.

	PR fortran/38669
	* trans-stmt.c (gfc_trans_call):
	Add the dependency code after the loop bounds calculation one.

2009-01-04  Mikael Morin  <mikael.morin@tele2.fr>

	PR fortran/38669
	* gfortran.dg/elemental_dependency_3.f90: New test.
	* gfortran.dg/elemental_subroutine_7.f90: New test.


Added:
    trunk/gcc/testsuite/gfortran.dg/elemental_dependency_3.f90
    trunk/gcc/testsuite/gfortran.dg/elemental_subroutine_7.f90
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/ChangeLog-2008
    trunk/gcc/fortran/dependency.c
    trunk/gcc/fortran/trans-stmt.c
    trunk/gcc/testsuite/ChangeLog

Comment 8 Mikael Morin 2009-01-05 13:37:46 UTC
(In reply to comment #7)
It caused PR 38726
Comment 9 Mikael Morin 2009-01-05 18:44:28 UTC
Subject: Bug 38669

Author: mikael
Date: Mon Jan  5 18:44:09 2009
New Revision: 143084

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=143084
Log:
2009-01-05  Mikael Morin  <mikael.morin@tele2.fr>

	PR fortran/38669
	PR fortran/38726
	* gfortran.dg/elemental_subroutine_7.f90: 
	Fix p values so that it can be used as vector subscript.


Modified:
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/gfortran.dg/elemental_subroutine_7.f90

Comment 10 Mikael Morin 2009-01-06 21:57:31 UTC
Subject: Bug 38669

Author: mikael
Date: Tue Jan  6 21:57:19 2009
New Revision: 143134

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=143134
Log:
2009-01-06  Mikael Morin  <mikael.morin@tele2.fr>

	PR fortran/38669
	* gfortran.dg/elemental_dependency_3.f90:
	Add the final tree dump cleanup.


Modified:
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/gfortran.dg/elemental_dependency_3.f90

Comment 11 Richard Biener 2009-01-08 23:01:43 UTC
Huh, the regression state of this bug looks weird.  So we have a patch applied
on trunk, but known-to-fail is both 4.3.3 and 4.4.0 - but 4.3.2 works?  And
the target milestone is 4.4.0?  If it is really a regression on the branch
(4.3.3 vs. 4.3.2) it definitely should be 4.3.3.  Changed as such, to get on
the radar for 4.3.3 again.

Please adjust known-to-work and known-to-fail accordingly.
Comment 12 Mikael Morin 2009-01-14 20:53:35 UTC
Subject: Bug 38669

Author: mikael
Date: Wed Jan 14 20:53:18 2009
New Revision: 143383

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=143383
Log:
2009-01-14  Mikael Morin  <mikael.morin@tele2.fr>

	PR fortran/35681
	* ChangeLog: Fix function name.

	PR fortran/38487
	* dependency.c (gfc_check_argument_var_dependency):
	Move the check for pointerness inside the if block
	so that it doesn't affect the return value.

	PR fortran/38669
	* trans-stmt.c (gfc_trans_call):
	Add the dependency code after the loop bounds calculation one.

2009-01-14  Mikael Morin  <mikael.morin@tele2.fr>

	PR fortran/38669
	* gfortran.dg/elemental_dependency_3.f90: New test.
	* gfortran.dg/elemental_subroutine_7.f90: New test.


Added:
    branches/gcc-4_3-branch/gcc/testsuite/gfortran.dg/elemental_dependency_3.f90
    branches/gcc-4_3-branch/gcc/testsuite/gfortran.dg/elemental_subroutine_7.f90
Modified:
    branches/gcc-4_3-branch/gcc/fortran/ChangeLog
    branches/gcc-4_3-branch/gcc/fortran/dependency.c
    branches/gcc-4_3-branch/gcc/fortran/trans-stmt.c
    branches/gcc-4_3-branch/gcc/testsuite/ChangeLog

Comment 13 Mikael Morin 2009-01-14 21:12:24 UTC
Fixed on trunk(4.4) and 4.3.
Thanks for the report!
Comment 14 Jan Hubicka 2010-11-11 00:17:38 UTC
Author: hubicka
Date: Thu Nov 11 00:17:34 2010
New Revision: 166579

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=166579
Log:

	PR tree-optimize/38669
	* gcc.dg/tree-ssa/pr38699.c

Added:
    trunk/gcc/testsuite/gcc.dg/tree-ssa/pr38699.c
Modified:
    trunk/gcc/testsuite/ChangeLog