[Bug fortran/57023] New: [4.7/4.8/4.9 Regression] Not packing arrays with changing variable used for size

[Bug fortran/57023] New: [4.7/4.8/4.9 Regression] Not packing arrays with changing variable used for size

[tkoenig at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57023

             Bug #: 57023
           Summary: [4.7/4.8/4.9 Regression] Not packing arrays with
                    changing variable used for size
    Classification: Unclassified
           Product: gcc
           Version: 4.9.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: fortran
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: tkoenig@gcc.gnu.org


Doing what the program below does is _extremely_ bad style -
changing n which is used for specifying the array size.

However, I think it is legal, and the expected output
is

1 1 1 1 0
1 1 1 1 0
1 1 1 1 0
1 1 1 1 0
0 0 0 0 0

Actual output is

 1 1 1 1 1
 1 1 1 1 1
 1 1 1 1 1
 1 0 0 0 0
 0 0 0 0 0

module mymod
contains
  subroutine foo(a,n)
    integer, dimension(n,n), intent(inout) :: a
    integer :: n
    n = n - 1
    call baz(a(1:n,1:n),n)
  end subroutine foo

  subroutine baz(a,n)
    integer, dimension(n,n), intent(inout) :: a
    a = 1
  end subroutine baz
end module mymod

program main
  use mymod
  integer, dimension(5,5) :: a
  n = 5
  a = 0
  call foo(a,n)
  print '(5I2)',a
end program main

The problem is in gfc_full_array_ref_p, which uses gfc_dep_compare_expr
to compare the upper bounds (n against n) without noting that n
has been changed in the meantime.

Proposed solution:

a) Always warn about such horrible code

b) If something like that happens, don't assume a full reference,
   and don't set the contiguous argument to gfc_full_array_ref_p to
   true.

The code was introduced in rev. 156749, btw, quite some time ago.

[Bug fortran/57023] [4.7/4.8/4.9 Regression] Not packing arrays with changing variable used for size

[tkoenig at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57023

--- Comment #2 from Thomas Koenig <tkoenig at gcc dot gnu.org> 2013-04-21 15:54:00 UTC ---
We also have to watch out for

module mymod
contains
  subroutine foo(a,n)
    integer, dimension(n,n), intent(inout) :: a
    integer :: n
    call decrement(n)
    call baz(a(1:n,1:n),n)
  end subroutine foo

  subroutine baz(a,n)
    integer, dimension(n,n), intent(inout) :: a
    a = 1
  end subroutine baz

  subroutine decrement(n)
    integer :: n
    n = n - 1
  end subroutine decrement
end module mymod

program main
  use mymod
  integer, dimension(5,5) :: a
  n = 5
  a = 0
  call foo(a,n)
  print '(5I2)',a
end program main

[Bug fortran/57023] [4.7/4.8/4.9 Regression] Not packing arrays with changing variable used for size

[burnus at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57023

Tobias Burnus <burnus at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |burnus at gcc dot gnu.org
   Target Milestone|---                         |4.7.4

[Bug fortran/57023] [4.7/4.8/4.9 Regression] Not packing arrays with changing variable used for size

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57023

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P4

[Bug fortran/57023] [4.7/4.8/4.9/4.10 Regression] Not packing arrays with changing variable used for size

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57023

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.7.4                       |4.8.4

--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
The 4.7 branch is being closed, moving target milestone to 4.8.4.

[Bug fortran/57023] [4.8/4.9/5 Regression] Not packing arrays with changing variable used for size

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57023

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.8.4                       |4.8.5

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 4.8.4 has been released.

[Bug fortran/57023] [4.8/4.9/5 Regression] Not packing arrays with changing variable used for size

[tkoenig at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57023

--- Comment #5 from Thomas Koenig <tkoenig at gcc dot gnu.org> ---
I have something.  Let's see if it works...

[Bug fortran/57023] [4.8/4.9/5 Regression] Not packing arrays with changing variable used for size

[tkoenig at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57023

Thomas Koenig <tkoenig at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |tkoenig at gcc dot gnu.org
           Assignee|unassigned at gcc dot gnu.org      |tkoenig at gcc dot gnu.org

--- Comment #6 from Thomas Koenig <tkoenig at gcc dot gnu.org> ---
Created attachment 34493
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=34493&action=edit
Proposed patch

This patch creates a temporary if a bound of the
array contains a dummy variable which is not
INTENT(IN) (so it could potentially be changed
by the user).

Modern code should always have INTENT(IN) for
something passed as array bounds, anyway.  We
should be correct for all cases, but not try
to optimize the pathological ones.

[Bug fortran/57023] [4.8/4.9/5 Regression] Not packing arrays with changing variable used for size

[tkoenig at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57023

--- Comment #7 from Thomas Koenig <tkoenig at gcc dot gnu.org> ---
Author: tkoenig
Date: Wed Jan 21 19:40:54 2015
New Revision: 219963

URL: https://gcc.gnu.org/viewcvs?rev=219963&root=gcc&view=rev
Log:
2015-01-21  Thomas Koenig  <tkoenig@netcologne.de>

    PR fortran/57023
    * dependency.c (callback_dummy_intent_not_int):  New function.
    (dummy_intent_not_in):  New function.
    (gfc_full_array_ref_p):  Use dummy_intent_not_in.

2015-01-21  Thomas Koenig  <tkoenig@netcologne.de>

    PR fortran/57023
    * gfortran.dg/internal_pack_15.f90:  New test.


Added:
    trunk/gcc/testsuite/gfortran.dg/internal_pack_15.f90
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/dependency.c
    trunk/gcc/testsuite/ChangeLog

[Bug fortran/57023] [4.8/4.9 Regression] Not packing arrays with changing variable used for size

[tkoenig at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57023

--- Comment #8 from Thomas Koenig <tkoenig at gcc dot gnu.org> ---
Author: tkoenig
Date: Sat Jan 24 12:50:51 2015
New Revision: 220080

URL: https://gcc.gnu.org/viewcvs?rev=220080&root=gcc&view=rev
Log:
2015-01-24  Thomas Koenig  <tkoenig@netcologne.de>

    Backport from trunk
    PR fortran/57023
    * dependency.c (callback_dummy_intent_not_int):  New function.
    (dummy_intent_not_in):  New function.
    (gfc_full_array_ref_p):  Use dummy_intent_not_in.

2015-01-24  Thomas Koenig  <tkoenig@netcologne.de>

    Backport from trunk
    PR fortran/57023
    * gfortran.dg/internal_pack_15.f90:  New test.

Added:
    branches/gcc-4_9-branch/gcc/testsuite/gfortran.dg/internal_pack_15.f90
Modified:
    branches/gcc-4_9-branch/gcc/fortran/ChangeLog
    branches/gcc-4_9-branch/gcc/fortran/dependency.c
    branches/gcc-4_9-branch/gcc/testsuite/ChangeLog

[Bug fortran/57023] [4.8 Regression] Not packing arrays with changing variable used for size

[tkoenig at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57023

--- Comment #9 from Thomas Koenig <tkoenig at gcc dot gnu.org> ---
Author: tkoenig
Date: Sat Jan 24 15:20:56 2015
New Revision: 220082

URL: https://gcc.gnu.org/viewcvs?rev=220082&root=gcc&view=rev
Log:
2015-01-24  Thomas Koenig  <tkoenig@netcologne.de>

    Backport from trunk
    PR fortran/57023
    * dependency.c (callback_dummy_intent_not_int):  New function.
    (dummy_intent_not_in):  New function.
    (gfc_full_array_ref_p):  Use dummy_intent_not_in.

2015-01-24  Thomas Koenig  <tkoenig@netcologne.de>

    Backport from trunk
    PR fortran/57023
    * gfortran.dg/internal_pack_15.f90:  New test.


Modified:
    branches/gcc-4_8-branch/gcc/fortran/ChangeLog
    branches/gcc-4_8-branch/gcc/fortran/dependency.c
    branches/gcc-4_8-branch/gcc/testsuite/ChangeLog

[Bug fortran/57023] [4.8 Regression] Not packing arrays with changing variable used for size

[tkoenig at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57023

Thomas Koenig <tkoenig at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #10 from Thomas Koenig <tkoenig at gcc dot gnu.org> ---
Fixed on all open branches, closing.