[Bug fortran/31725] New: overwriting of neighbouring character array

[Bug fortran/31725] New: overwriting of neighbouring character array

[KnowlesPJ at Cardiff dot ac dot uk]

In the following program, at line 68, array zsymelr is overwritten during the
assignment to zsymelr. The output contains 'This should not happen' when the
bug is present. Other compilers produce correct output, for which the last line
reads 'symelr->symel'

$ gfortran -v
Using built-in specs.
Target: i386-apple-darwin8.9.1
Configured with: ../gcc-4.3-20070316/configure --enable-languages=fortran,c++
Thread model: posix
gcc version 4.3.0 20070316 (experimental)


-- 
           Summary: overwriting of neighbouring character array
           Product: gcc
           Version: 4.3.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: fortran
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: KnowlesPJ at Cardiff dot ac dot uk
 GCC build triplet: i386-apple-darwin8.9.1
  GCC host triplet: i386-apple-darwin8.9.1
GCC target triplet: i386-apple-darwin8.9.1


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

[Bug fortran/31725] overwriting of neighbouring character array

[KnowlesPJ at Cardiff dot ac dot uk]

------- Comment #1 from KnowlesPJ at Cardiff dot ac dot uk  2007-04-27 10:34 -------
Created an attachment (id=13455)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=13455&action=view)
failing program


-- 


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

[Bug fortran/31725] overwriting of neighbouring character array

[burnus at gcc dot gnu dot org]

------- Comment #2 from burnus at gcc dot gnu dot org  2007-04-27 15:47 -------
The program is broken / invalid:

      function lenstr(s)
      character*(*) s
      do 1 l=ll,1,-1
      if (s(l:l).ne.' ') goto 2
1     continue

Question: What value has "ll" ? 0, 325, -4326 ?

Depending on the compiler, anything may happen. Using NAG f95 I simply get a
coredump, using gfortran I got "This should not happen". The Intel compiler
silently sets this number to zero and runs "l = 0, 1, -1". I think neither does
the right thing.

If I add
  ll = 0
all my compilers produce the same result - however meaningless/-full it might
be.

By the way, using Fortran 95, you have the function LEN_TRIM which gives the
string length without tailing blanks.

If there remains another bug, please reopen.


-- 

burnus at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|                            |INVALID


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

[Bug fortran/31725] overwriting of neighbouring character array

[KnowlesPJ at Cardiff dot ac dot uk]

------- Comment #3 from KnowlesPJ at Cardiff dot ac dot uk  2007-04-27 16:13 -------
I don't agree with this analysis, as ll is certainly defined in char3.f as
attached -
ll=len(s)
Please try with the program exactly as supplied!
Yes, you are right, this function is the same as len_trim but all I am doing is
pulling a small extract from a legacy code and I wanted simply to expose the
problem not tidy up.  But if I replace lenstr by len_trim the error still
occurs.


-- 

KnowlesPJ at Cardiff dot ac dot uk changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |UNCONFIRMED
         Resolution|INVALID                     |


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

[Bug fortran/31725] overwriting of neighbouring character array

[burnus at gcc dot gnu dot org]

------- Comment #4 from burnus at gcc dot gnu dot org  2007-04-27 18:59 -------
(Slabs oneself because of deleting important lines while deleting "only"
comments.)

Thanks for being persistent!


Reduced test case, should print twice "X,Y" but prints (once) ",Y".

The problem is that for
        zsymel(3)(lenstr(zsymel(3))+1:)='X'

lenstr() is called trice instead of only once.
The assignment should be:   string(1:3) = 'X  '

gfortran divides this into two parts: Assigning the 'X' and assigning the two
spaces; unfortunally, it calls strlen inbetween, which gives "1" after the "X"
has been assigned. And thus:

Thus instead of string(1:1) = 'X'; string(2:3) = '  '
         we do: string(1:1) = 'X'; string(1:2) = '  '

Or in the beauty of the dump:

__builtin_memmove ((char[1:_zsymel] *) &(*zsymel.0)[2][lenstr (&(*zsymel.0)[2],
_zsymel) + 1]{lb: 1 sz: 1}, "X", 1);
__builtin_memset ((char[1:_zsymel] *) &(*zsymel.0)[2][lenstr (&(*zsymel.0)[2],
_zsymel) + 1]{lb: 1 sz: 1} + 1B, 32, (int8) D

In addition it is called before in order to test whether an assignment should
be done at all:
  D.1374 = MAX_EXPR <_zsymel - lenstr (&(*zsymel.0)[2], _zsymel), 0>;

As lenstr could have side effects and because of speed, it makes sense to call
it only once and not twice or - as here - trice.

Example program:
      program char3
      implicit none
      character(3), dimension(3) :: zsymel,zsymelr
      common /xx/ zsymel, zsymelr
      integer :: znsymelr
      zsymel = (/ 'X', 'Y', ' ' /)
      zsymelr= (/ 'X', 'Y', ' ' /)
      znsymelr=2
      call check_zsymel(zsymel,zsymelr,znsymelr)
      contains
      subroutine check_zsymel(zsymel,zsymelr,znsymelr)
        implicit none
        integer znsymelr, isym
        character(*) zsymel(*),zsymelr(*)
        zsymel(3)(lenstr(zsymel(3))+1:)='X'
        write (*,10) (trim(zsymelr(isym)),isym=1,znsymelr)
10      format(3(a,:,','))
      end subroutine check_zsymel
      function lenstr(s)
        character(len=*),intent(in) :: s
        integer :: lenstr
        print *, 's: ',len_trim(s)
        lenstr = len_trim(s)
      end function lenstr
      end program


-- 

burnus at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|0                           |1
  GCC build triplet|i386-apple-darwin8.9.1      |
   GCC host triplet|i386-apple-darwin8.9.1      |
 GCC target triplet|i386-apple-darwin8.9.1      |
           Keywords|                            |wrong-code
   Last reconfirmed|0000-00-00 00:00:00         |2007-04-27 18:59:17
               date|                            |


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

[Bug fortran/31725] String assigment of type string(function():) = 'X' produces wrong code

[burnus at gcc dot gnu dot org]

-- 

burnus at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |burnus at gcc dot gnu dot
                   |                            |org
      Known to fail|                            |4.3.0 4.2.0 4.1.2
            Summary|overwriting of neighbouring |String assigment of type
                   |character array             |string(function():) = 'X'
                   |                            |produces wrong code
   Target Milestone|---                         |4.3.0


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

[Bug fortran/31725] substring bound evaluated multiple times: wrong code for string(function():) = 'X'

[fxcoudert at gcc dot gnu dot org]

------- Comment #5 from fxcoudert at gcc dot gnu dot org  2007-05-04 22:15 -------
Here's a patch that avoids multiple evaluation of substring start and end
indices. It was tested on the testcase, also by changing the (start:) range
into a (:end) range, and also by using -fbounds-check (which evaluated the
start and end indices a few other times). It's not fully regtested yet, though.



Index: trans-expr.c
===================================================================
--- trans-expr.c        (revision 124285)
+++ trans-expr.c        (working copy)
@@ -261,6 +261,9 @@ gfc_conv_substring (gfc_se * se, gfc_ref
     gfc_conv_string_parameter (se);
   else
     {
+      /* Avoid multiple evaluation of substring start.  */
+      start.expr = gfc_evaluate_now (start.expr, &se->pre);
+
       /* Change the start of the string.  */
       if (TYPE_STRING_FLAG (TREE_TYPE (se->expr)))
        tmp = se->expr;
@@ -279,6 +282,8 @@ gfc_conv_substring (gfc_se * se, gfc_ref
       gfc_conv_expr_type (&end, ref->u.ss.end, gfc_charlen_type_node);
       gfc_add_block_to_block (&se->pre, &end.pre);
     }
+  end.expr = gfc_evaluate_now (end.expr, &se->pre);
+
   if (flag_bounds_check)
     {
       tree nonempty = fold_build2 (LE_EXPR, boolean_type_node,


-- 

fxcoudert at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at gcc dot gnu   |fxcoudert at gcc dot gnu dot
                   |dot org                     |org
             Status|NEW                         |ASSIGNED
           Keywords|                            |patch
   Last reconfirmed|2007-04-27 18:59:17         |2007-05-04 22:15:38
               date|                            |


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

[Bug fortran/31725] substring bound evaluated multiple times: wrong code for string(function():) = 'X'

[fxcoudert at gcc dot gnu dot org]

------- Comment #6 from fxcoudert at gcc dot gnu dot org  2007-05-14 20:29 -------
Subject: Bug 31725

Author: fxcoudert
Date: Mon May 14 19:29:33 2007
New Revision: 124720

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=124720
Log:
        PR fortran/31725

        * trans-expr.c (gfc_conv_substring): Evaluate substring bounds
        only once.

        * gfortran.dg/substr_4.f: New test.

Added:
    trunk/gcc/testsuite/gfortran.dg/substr_4.f
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/trans-expr.c
    trunk/gcc/testsuite/ChangeLog


-- 


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

[Bug fortran/31725] [4.1/4.2 only] substring bound evaluated multiple times: wrong code for string(function():) = 'X'

[fxcoudert at gcc dot gnu dot org]

-- 

fxcoudert at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to fail|4.3.0 4.2.0 4.1.2           |4.2.0 4.1.2
      Known to work|                            |4.3.0
            Summary|substring bound evaluated   |[4.1/4.2 only] substring
                   |multiple times: wrong code  |bound evaluated multiple
                   |for string(function():) =   |times: wrong code for
                   |'X'                         |string(function():) = 'X'
   Target Milestone|4.3.0                       |4.2.1


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

[Bug fortran/31725] [4.1/4.2 only] substring bound evaluated multiple times: wrong code for string(function():) = 'X'

[fxcoudert at gcc dot gnu dot org]

------- Comment #7 from fxcoudert at gcc dot gnu dot org  2007-05-16 14:36 -------
Subject: Bug 31725

Author: fxcoudert
Date: Wed May 16 13:36:03 2007
New Revision: 124769

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=124769
Log:
        PR fortran/31725

        * trans-expr.c (gfc_conv_substring): Evaluate substring bounds
        only once.

        * gfortran.dg/substr_4.f: New test.

Added:
    branches/gcc-4_2-branch/gcc/testsuite/gfortran.dg/substr_4.f
      - copied unchanged from r124720,
trunk/gcc/testsuite/gfortran.dg/substr_4.f
Modified:
    branches/gcc-4_2-branch/gcc/fortran/ChangeLog
    branches/gcc-4_2-branch/gcc/fortran/trans-expr.c
    branches/gcc-4_2-branch/gcc/testsuite/ChangeLog


-- 


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

[Bug fortran/31725] [4.1 only] substring bound evaluated multiple times: wrong code for string(function():) = 'X'

[fxcoudert at gcc dot gnu dot org]

------- Comment #8 from fxcoudert at gcc dot gnu dot org  2007-05-16 15:01 -------
Fixed on mainline and 4.2.1


-- 

fxcoudert at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
      Known to work|4.3.0                       |4.3.0 4.2.1
         Resolution|                            |FIXED
            Summary|[4.1/4.2 only] substring    |[4.1 only] substring bound
                   |bound evaluated multiple    |evaluated multiple times:
                   |times: wrong code for       |wrong code for
                   |string(function():) = 'X'   |string(function():) = 'X'


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