[Bug fortran/30003] New: Expressions with side effects in array references

[Bug fortran/30003] New: Expressions with side effects in array references

[eedelman at gcc dot gnu dot org]

(This bug was discovered during a discussion on the mailinglist starting here:
http://gcc.gnu.org/ml/fortran/2006-11/msg00636.html) 

This short program

erik:~$ cat foo.f90
program foo

    implicit none
    integer :: a(5), b(3)

    b = [ 1, 2, 3 ]
    a(1:bar(3)) = b

contains
    integer function bar(n)
        integer, intent(in) :: n
        print *, 'Hello!'
        bar = n
    end function bar
end program foo

should, when compiled and run, call bar() once, and, hence, print 'hello!'
once.  But when compiled without -fbounds-check, it doesn't print anything:

erik:~$ gfortran foo.f90 
erik:~$ a.out

This is because gfortran will actually never evaluate the expression of the
size of the LHS; it will just assume the LHS has the same size as the RHS. 
That would be OK if the expression of the size didn't have side effects, but in
a case like this, it gives wrong results.

If we compile with -fbounds-check, it gets really strange:

erik:~$ gfortran foo.f90 -fbounds-check
erik:~$ a.out
 Hello!
 Hello!
 Hello!
 Hello!
 Hello!
 Hello!
 Hello!
 Hello!
 Hello!
 Hello!
 Hello!
 Hello!
 Hello!


Here, bar() is called 13 times!  This is because bar() is called separately for
every check that the value of bar() is within bounds etc. Again, this would be
OK (albeit a bit inefficient) if bar() had no side effects, but with side
effects it gives wrong results.  

Also consider the case where bar():s result value depends on e.g. a SAVEd
variable, so that it returns different values every time it's called (even if
called with the same argument).  That could give _really_ wierd behaviour.


-- 
           Summary: Expressions with side effects in array references
           Product: gcc
           Version: 4.3.0
            Status: UNCONFIRMED
          Keywords: wrong-code
          Severity: normal
          Priority: P3
         Component: fortran
        AssignedTo: eedelman at gcc dot gnu dot org
        ReportedBy: eedelman at gcc dot gnu dot org


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

[Bug fortran/30003] Expressions with side effects in array references

[tobi at gcc dot gnu dot org]

-- 

tobi at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|0                           |1
   Last reconfirmed|0000-00-00 00:00:00         |2006-11-29 21:21:11
               date|                            |


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

[Bug fortran/30003] Expressions with side effects in array references

[eedelman at gcc dot gnu dot org]

------- Comment #1 from eedelman at gcc dot gnu dot org  2006-12-02 22:28 -------
I thought I would have time to do something about this bug, but I will not; not
in the next few weeks/months.  I therefore unassign it from me.  My appologies
to those who expected to be fixed soon.

If it's still unfixed (and unassigned) when I get time again, I'll pick it up
again.


-- 

eedelman at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|eedelman at gcc dot gnu dot |unassigned at gcc dot gnu
                   |org                         |dot org


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

[Bug fortran/30003] Expressions with side effects in array references

[pault at gcc dot gnu dot org]

------- Comment #2 from pault at gcc dot gnu dot org  2006-12-04 20:47 -------
(In reply to comment #1)
> I thought I would have time to do something about this bug, but I will not; not
> in the next few weeks/months.  I therefore unassign it from me.  My appologies
> to those who expected to be fixed soon.

That's a pity, Erik, but quite understandable.

I started playing with it at lunchtime.  It's quite an amusing little PR!

As running the test below shows, an array reference 'start' or 'stride' is
always handled correctly, whereas the 'end' is always handled incorrectly.

This suggests that the way to fix this is to add an 'end' field to gfc_ss_info
and to treat it exactly like start, even if it is not used in the scalarizer.

Paul

    implicit none
    integer :: a(5), b(3), cnt

    b = [ 1, 2, 3 ]

    cnt = 0
    a(bar(1):3) = b
    print *, "a(start) => ", cnt, " calls"

    cnt = 0
    a(1:bar(3)) = b
    print *, "a(end) => ", cnt, " calls"

    cnt = 0
    a(1:3:bar(1)) = b
    print *, "a(stride) => ", cnt, " calls"

    cnt = 0
    a(1:3) = b(bar(1):3)
    print *, "b(start) => ", cnt, " calls"

    cnt = 0
    a(1:3) = b(1:bar(3))
    print *, "b(end) => ", cnt, " calls"

    cnt = 0
    a(1:3) = b(1:3:bar(1))
    print *, "b(stride) => ", cnt, " calls"

contains
    integer function bar(n)
        integer, intent(in) :: n
        cnt = cnt + 1
        bar = n
    end function bar
end program foo



-- 


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

[Bug fortran/30003] Expressions with side effects in array references

[pault at gcc dot gnu dot org]

------- Comment #3 from pault at gcc dot gnu dot org  2006-12-04 22:44 -------
Created an attachment (id=12744)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=12744&action=view)
A fix for this PR

This is most of the way there, using the idea in #2; except that, for reasons I
do not understand, allocatable_function_1.f90 emits an extra gfc_internal_free.

Erik, if you have a moment, could you see if you can understand where the extra
free comes from?

Cheers

Paul


-- 

pault at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at gcc dot gnu   |pault at gcc dot gnu dot org
                   |dot org                     |
             Status|NEW                         |ASSIGNED


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

[Bug fortran/30003] Expressions with side effects in array references

[pault at gcc dot gnu dot org]

------- Comment #4 from pault at gcc dot gnu dot org  2006-12-05 08:25 -------
Duuuhhh!

> Erik, if you have a moment, could you see if you can understand where the
> extra free comes from?

That was the whole point, wasn't it? :-)

So the full patch will modify allocatable_function_1.f90 to expect 10 frees and
I will modify the comment about the rhs setting the loop size.

Regards

Paul


-- 


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

[Bug fortran/30003] Expressions with side effects in array references

[pault at gcc dot gnu dot org]

------- Comment #5 from pault at gcc dot gnu dot org  2006-12-05 09:39 -------
Created an attachment (id=12746)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=12746&action=view)
Patch and testcase for the PR

This regtests OK on Cygwin_NT/PIV.  I will submit it tonight.

Paul


-- 

pault at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #12744|0                           |1
        is obsolete|                            |


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

[Bug fortran/30003] Expressions with side effects in array references

[burnus at gcc dot gnu dot org]

------- Comment #6 from burnus at gcc dot gnu dot org  2006-12-05 15:07 -------
> Patch and testcase for the PR
> This regtests OK on Cygwin_NT/PIV.  I will submit it tonight.

It also regression tests ok on x86_64-unknown-linux-gnu; the patch itself also
looks ok.


-- 


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

[Bug fortran/30003] Expressions with side effects in array references

[tobi at gcc dot gnu dot org]

------- Comment #7 from tobi at gcc dot gnu dot org  2006-12-05 15:57 -------
Thanks Paul and Erik!

The patch regtests fine on i686-darwin together with my patch to enable
bounds-checking in the testsuite (and the workaround for PR29516, without which
gfortran is essentially unusable on i686-darwin).


-- 


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

[Bug fortran/30003] Expressions with side effects in array references

[patchapp at dberlin dot org]

------- Comment #8 from patchapp at dberlin dot org  2006-12-05 16:45 -------
Subject: Bug number PR30003

A patch for this bug has been added to the patch tracker.
The mailing list url for the patch is
http://gcc.gnu.org/ml/gcc-patches/2006-12/msg00306.html


-- 


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

[Bug fortran/30003] Expressions with side effects in array references

[pault at gcc dot gnu dot org]

------- Comment #9 from pault at gcc dot gnu dot org  2006-12-05 19:45 -------
Subject: Bug 30003

Author: pault
Date: Tue Dec  5 19:45:25 2006
New Revision: 119556

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=119556
Log:
2006-12-05  Paul Thomas  <pault@gcc.gnu.org>

        PR fortran/30003
        * trans-array.c (gfc_trans_create_temp_array): Set the section
        ends to zero.
        (gfc_conv_array_transpose): Likewise.
        (gfc_conv_section_startstride): Declare an expression for end,
        set it from a the array reference and evaluate it for the info
        structure. Zero the ends in the ss structure and set end, used
        in the bounds check, from the info structure.
        trans.h: Add and end array to the gfc_ss_info structure.

2006-12-05  Paul Thomas  <pault@gcc.gnu.org>

        PR fortran/30003
        * gfortran.dg/allocatable_function_1.f90: Increase the number
        of expected calls of free to 10; the lhs section reference is
        now evaluated so there is another call to bar.  Change the
        comment appropriately.
        * gfortran.dg/array_section_1.f90: New test.

Added:
    trunk/gcc/testsuite/gfortran.dg/array_section_1.f90
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/trans-array.c
    trunk/gcc/fortran/trans.h
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/gfortran.dg/allocatable_function_1.f90


-- 


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

[Bug fortran/30003] [4.2 and 4.1 only] Expressions with side effects in array references

[pault at gcc dot gnu dot org]

------- Comment #10 from pault at gcc dot gnu dot org  2006-12-09 15:17 -------
Subject: Bug 30003

Author: pault
Date: Sat Dec  9 15:17:16 2006
New Revision: 119690

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=119690
Log:
2006-12-09  Paul Thomas  <pault@gcc.gnu.org>

        Backports from trunk

        PR fortran/29821
        * resolve.c (resolve_operator): Only return result of
        gfc_simplify_expr if expression is constant.

        PR fortran/29912
        * trans-expr.c (gfc_trans_arrayfunc_assign): Return NULL if the
        lhs and rhs character lengths are not constant and equal for
        character array valued functions.

        PR fortran/29916
        * resolve.c (resolve_symbol): Allow host-associated variables
        in the specification expression of an array-valued function.
        * expr.c (check_restricted): Accept host-associated dummy
        array indices.

        PR fortran/30003
        * trans-array.c (gfc_trans_create_temp_array): Set the section
        ends to zero.
        (gfc_conv_array_transpose): Likewise.
        (gfc_conv_section_startstride): Declare an expression for end,
        set it from a the array reference and evaluate it for the info
        structure. Zero the ends in the ss structure and set end, used
        in the bounds check, from the info structure.
        trans.h: Add and end array to the gfc_ss_info structure.

2006-12-09  Paul Thomas  <pault@gcc.gnu.org>

        PR fortran/29821
        * gfortran.dg/parameter_array_section_1.f90: New test.

        PR fortran/29912
        * gfortran.dg/char_result_12.f90: New test.

        PR fortran/29916
        * gfortran.dg/host_dummy_index_1.f90: Added additional test.

        PR fortran/30003
        * gfortran.dg/allocatable_function_1.f90: Increase the number
        of expected calls of free to 10; the lhs section reference is
        now evaluated so there is another call to bar.  Change the
        comment appropriately.
        * gfortran.dg/array_section_1.f90: New test.


Added:
    branches/gcc-4_2-branch/gcc/testsuite/gfortran.dg/array_section_1.f90
    branches/gcc-4_2-branch/gcc/testsuite/gfortran.dg/char_result_12.f90
   
branches/gcc-4_2-branch/gcc/testsuite/gfortran.dg/parameter_array_section_1.f90
Modified:
    branches/gcc-4_2-branch/gcc/fortran/ChangeLog
    branches/gcc-4_2-branch/gcc/fortran/expr.c
    branches/gcc-4_2-branch/gcc/fortran/resolve.c
    branches/gcc-4_2-branch/gcc/fortran/trans-array.c
    branches/gcc-4_2-branch/gcc/fortran/trans-expr.c
    branches/gcc-4_2-branch/gcc/fortran/trans.h
    branches/gcc-4_2-branch/gcc/testsuite/ChangeLog
   
branches/gcc-4_2-branch/gcc/testsuite/gfortran.dg/allocatable_function_1.f90
    branches/gcc-4_2-branch/gcc/testsuite/gfortran.dg/host_dummy_index_1.f90


-- 


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

[Bug fortran/30003] [4.1 only] Expressions with side effects in array references

[pault at gcc dot gnu dot org]

------- Comment #11 from pault at gcc dot gnu dot org  2006-12-09 15:22 -------
Fixed in trunk and 4.2.  If I have time, I will patch 4.1 someday.

Paul


-- 

pault at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED
            Summary|[4.2 and 4.1 only]          |[4.1 only] Expressions with
                   |Expressions with side       |side effects in array
                   |effects in array references |references


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

[Bug fortran/30003] [4.1 only] Expressions with side effects in array references

[pault at gcc dot gnu dot org]

------- Comment #12 from pault at gcc dot gnu dot org  2006-12-09 17:48 -------
Subject: Bug 30003

Author: pault
Date: Sat Dec  9 17:47:45 2006
New Revision: 119694

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=119694
Log:
2006-12-09  Paul Thomas  <pault@gcc.gnu.org>

        Backports from trunk

        PR fortran/29821
        * resolve.c (resolve_operator): Only return result of
        gfc_simplify_expr if expression is constant.

        PR fortran/29912
        * trans-expr.c (gfc_trans_arrayfunc_assign): Return NULL if the
        lhs and rhs character lengths are not constant and equal for
        character array valued functions.

        PR fortran/29916
        * resolve.c (resolve_symbol): Allow host-associated variables
        in the specification expression of an array-valued function.
        * expr.c (check_restricted): Accept host-associated dummy
        array indices.

        PR fortran/30003
        * trans-array.c (gfc_trans_create_temp_array): Set the section
        ends to zero.
        (gfc_conv_section_startstride): Declare an expression for end,
        set it from a the array reference and evaluate it for the info
        structure. Zero the ends in the ss structure and set end, used
        in the bounds check, from the info structure.
        trans.h: Add and end array to the gfc_ss_info structure.

        PR fortran/29820
        * trans-array.c (gfc_get_derived_type): Once done, spread the
        backend_decl to all identical derived types in all sibling
        namespaces.

2006-12-09  Paul Thomas  <pault@gcc.gnu.org>

        PR fortran/29821
        * gfortran.dg/parameter_array_section_1.f90: New test.

        PR fortran/29912
        * gfortran.dg/char_result_12.f90: New test.

        PR fortran/29916
        * gfortran.dg/host_dummy_index_1.f90: Added additional test.

        PR fortran/30003
        * gfortran.dg/allocatable_function_1.f90: Increase the number
        of expected calls of free to 10; the lhs section reference is
        now evaluated so there is another call to bar.  Change the
        comment appropriately.
        * gfortran.dg/array_section_1.f90: New test.

        PR fortran/29820
        * gfortran.dg/used_types_13.f90: New test.


Added:
    branches/gcc-4_1-branch/gcc/testsuite/gfortran.dg/array_section_1.f90
    branches/gcc-4_1-branch/gcc/testsuite/gfortran.dg/char_result_12.f90
   
branches/gcc-4_1-branch/gcc/testsuite/gfortran.dg/parameter_array_section_1.f90
    branches/gcc-4_1-branch/gcc/testsuite/gfortran.dg/used_types_13.f90
Modified:
    branches/gcc-4_1-branch/gcc/fortran/ChangeLog
    branches/gcc-4_1-branch/gcc/fortran/expr.c
    branches/gcc-4_1-branch/gcc/fortran/resolve.c
    branches/gcc-4_1-branch/gcc/fortran/trans-array.c
    branches/gcc-4_1-branch/gcc/fortran/trans-expr.c
    branches/gcc-4_1-branch/gcc/fortran/trans-types.c
    branches/gcc-4_1-branch/gcc/fortran/trans.h
    branches/gcc-4_1-branch/gcc/testsuite/ChangeLog
    branches/gcc-4_1-branch/gcc/testsuite/gfortran.dg/host_dummy_index_1.f90


-- 


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