Regression happens between 2007-05-25-r125057 and 2007-05-29-r125159.

I assume this is another incarnation of this bugs but leads to a segfault and a slightly different valgrind output:

MODULE TEST
CONTAINS
PURE FUNCTION s2a_3(s1,s2,s3) RESULT(a)
    CHARACTER(LEN=*), INTENT(IN)             :: s1, s2, s3
    CHARACTER(LEN=1000), DIMENSION(3)        :: a

  a(1)=s1; a(2)=s2; a(3)=s3
END FUNCTION
END MODULE

USE TEST
character(LEN=80) :: b(3)
b=s2a_3("Distribution by marix blocks", "Distribution by matrix rows",&
         "Distribution by matrix columns")
write(6,*) b(3)
END

==21561==    at 0x4A21AF0: memset (mc_replace_strmem.c:490)
==21561==    by 0x400A8B: __test_MOD_s2a_3 (in /users/vondele/g95/a.out)
==21561==    by 0x40089D: MAIN__ (in /users/vondele/g95/a.out)
==21561==    by 0x400B4B: main (fmain.c:22)
==21561==  Address 0x7FF005B80 is not stack'd, malloc'd or (recently) free'd
==21561==
==21561== Process terminating with default action of signal 11 (SIGSEGV)
==21561==  Access not within mapped region at address 0x7FF005B80
==21561==    at 0x4A21AF0: memset (mc_replace_strmem.c:490)
==21561==    by 0x400A8B: __test_MOD_s2a_3 (in /users/vondele/g95/a.out)
==21561==    by 0x40089D: MAIN__ (in /users/vondele/g95/a.out)
==21561==    by 0x400B4B: main (fmain.c:22)

More regression hunting: FAILS with r125059,  Works with r125057.
Conclusion: The patch causing or revealing this wrong-code error is:

r125058 | rguenth | 2007-05-25 11:07:29 +0200 (Fr, 25 Mai 2007) | 10 lines
2007-05-24  Richard Guenther  <rguenther@suse.de>
        Andrew Pinski  <andrew_pinski@playstation.sony.com>
        PR tree-optimization/31982

Andrew or Richard, could you have a look?

> I assume this is another incarnation of this bugs but leads to a segfault and
> a slightly different valgrind output
This test works for me with gfortran as of this morning and also with gfortran r12505 (on x86_64-unknown-linux-gnu, no valgrind error).

Plus forwprop only does:
  D.1046_37 = &(*__result.0_23)[0];
....
  D.1048_41 = (char *) _s1_2(D);
  D.1049_42 = D.1046_37 + D.1048_41;

Into:
  D.1046_37 = &(*__result.0_23)[0];
...
  D.1048_41 = (char *) _s1_2(D);
  D.1049_42 = &(*__result.0_23)[_s1_2(D)];

Which is not wrong.
I wonder if my patch at: http://gcc.gnu.org/ml/gcc-patches/2007-05/msg01769.html

Fixes the issue.

(In reply to comment #5)
> I wonder if my patch at:
> http://gcc.gnu.org/ml/gcc-patches/2007-05/msg01769.html
> Fixes the issue.

both testcases still fail for me at -O2 with this morning's compiler (which should include the above mentioned patch).

suspect that this is a front end issue, where something wrong is being generated for s2a_3. It seems to trigger only if a is a function results, and s1,s2,s3 are len=*. 

I see it also with today's compiler on i686-darwin:

$ gfortran test.f90 -O2 && ./a.out 
 a.>bb  ccc 

Also see failure at optimizations -O1, -O2, -O3 on x86-64-gnu-linuz (intel)

The segfault is still happening with today's code. This is the simplest case I find to trigger it. 

MODULE TEST
CONTAINS
FUNCTION s2a_3(s1) RESULT(a)
    CHARACTER(LEN=*), INTENT(IN)             :: s1
    CHARACTER(LEN=1000) :: a(3)
    a(1)=s1
END FUNCTION
END MODULE

USE TEST
character(LEN=1000) :: b(3)
b=s2a_3(REPEAT("1",101))
write(6,*) b(1)
END

The point where things segfault is the assignment a(1)=s1, and in particular the  corresponding memset (which adds blanks to the rest of the string). This memset much be getting a wrong pointer to start with, according to valgrind:

==23016== Invalid write of size 1
==23016==    at 0x4A1AAF0: memset (mc_replace_strmem.c:490)
==23016==    by 0x400A4D: __test_MOD_s2a_3 (test2.f90:6)
==23016==    by 0x40098B: MAIN__ (test2.f90:12)
==23016==    by 0x400A8B: main (fmain.c:22)
==23016==  Address 0x7FF017FA8 is not stack'd, malloc'd or (recently) free'd

the bug only happens if a is at the same time
1) a function result
2) an array
3) of character variables with a len known at compile time.

in the dump tree original, the corresponding memset line looks like:

__builtin_memset (&(*__result.0)[NON_LVALUE_EXPR <stride.0> + offset.1] + (char *) (int8) D.1374, 32, 1000 - (int8) D.1374);

but I can't see how I could debug further.... pretty please, can somebody look into that... I'd like to be able to test all the stuff that went in since the bug was opened a few weeks ago.

(In reply to comment #10)
> but I can't see how I could debug further.... pretty please, can somebody look
> into that... I'd like to be able to test all the stuff that went in since the
> bug was opened a few weeks ago.

I don't have much time right now, but here's what I saw and how I would debug it further: by using -O1 and -fdump-tree-optimized, you can get a dump of the optimized tree. The memset call is changed by the optimizer into:

    __builtin_memset (&(*__result.0)[_s1], 32, 1000 - _s1);

Compared to your original tree, where stride.0 = 1 and offset.1 = -stride.0, and D.1374 = _s1, they look equivalent, except for the transformation of &(*array)[_s1] into &(*array)[0] + (char *) _s1. I think that should not be a problem, except if there is some type mismatch. Further debugging probably needs to find the part of the front-end that generates this memset call (easily done by grepping for BUILT_IN_MEMSET in the front-end files), setting a break-point there and looking into the types of each operands. This last operation can be done within gdb by calling the debug_tree() function on the trees you're interested in.

When (and if) I get some time after my PhD defence (next monday), I'll try to look into it; after all, I am interested in cp2k myself, even though it doesn't yet work for what I need it to do ;-)

Subject: Re:  [4.3 Regression] Miscompilation with -O1

On 15 Jun 2007 15:12:02 -0000, fxcoudert at gcc dot gnu dot org
<gcc-bugzilla@gcc.gnu.org> wrote:
> When (and if) I get some time after my PhD defence (next monday), I'll try to
> look into it; after all, I am interested in cp2k myself, even though it doesn't
> yet work for what I need it to do ;-)

note this might get cleared up with pointer_plus.
I have not tried it there yet but i will be posting a patch and committing soon.

--Pinski

(In reply to comment #12)

> note this might get cleared up with pointer_plus.
> I have not tried it there yet but i will be posting a patch and committing
> soon.

I've tried current trunk (which should contain pointer_plus), but the test case still segfaults.

A slight modification of the example in comment #2:

MODULE TEST
CONTAINS
FUNCTION s2a_3(s1) RESULT(a)
    CHARACTER(LEN=*), INTENT(IN)             :: s1
    CHARACTER(LEN=LEN(s1)) :: a(3)
    a(1)=s1
END FUNCTION
END MODULE

USE TEST
character(LEN=1000) :: b(3)
b=s2a_3(REPEAT("1",101))
write(6,*) b(1)
END

yields something that works at any level of optimization.  Note that the main program remains untouched, except for all the code that comes from the interface evaluation of the character length.  Oddly, the POINTER_PLUS bit remains unchanged.  I have stared at the code, trying to understand where an optimization senstivity might come in, with no success. Perhaps, somebody smarter than me will see the difference.

Paul  
 

We have an extra:
(insn 39 38 40 t.f90:7 (parallel [
            (set (reg:SI 73)
                (ashift:SI (reg:SI 68 [ _s1 ])
                    (const_int 2 [0x2])))
            (clobber (reg:CC 17 flags))
        ]) -1 (nil))

After the patch.

I was wrong in marking this as a middle-end issue.
We have:
  char[0:D.1026][1:4] * __result.0;
  char * temp.87;
...
  temp.87 = &(*__result.0)[0];
  __builtin_memset (temp.87 + (<unnamed-unsigned:32>) _s1, 32, 4 - _s1);
----------- cut ----------

The midde-end thinks it can combine &(*__result.0)[0] + (<unnamed-unsigned:32>) _s1 to just &(*__result.0)[(<unnamed-unsigned:32>) _s1] when it should have combined it to &(*__result.0)[0][(<unnamed-unsigned:32>) _s1]

So
&(*__result.0)[0]
is wrong, it should have been:
&(*__result.0)[0][0].

Let me see if I can figure out where to fix the front-end.

Here is the fix which I am testing, basically instead of creating (typeof(array[0] "*")&array we create &array[lb]:
Index: trans.c
===================================================================
--- trans.c     (revision 125777)
+++ trans.c     (working copy)
@@ -266,7 +266,14 @@ gfc_build_addr_expr (tree type, tree t)
       && TREE_CODE (base_type) == ARRAY_TYPE
       && TYPE_MAIN_VARIANT (TREE_TYPE (type))
         == TYPE_MAIN_VARIANT (TREE_TYPE (base_type)))
-    natural_type = type;
+    {
+      tree min_val = size_zero_node;
+      tree type_domain = TYPE_DOMAIN (base_type);
+      if (type_domain && TYPE_MIN_VALUE (type_domain))
+       min_val = TYPE_MIN_VALUE (type_domain);
+      t = build4 (ARRAY_REF, TREE_TYPE (type), t, min_val, NULL_TREE, NULL_TREE);
+      natural_type = type;
+    }
   else
     natural_type = build_pointer_type (base_type);

Patch tested OK on x86-64-Gnu-Linux.

I am also able to compile and run cp2k where before I was getting a segfault.

(In reply to comment #17)
> Here is the fix which I am testing, basically instead of creating
> (typeof(array[0] "*")&array we create &array[lb]:

did this fix test OK ? Since it fixes the CP2K issue, I would hope that it could be posted for review soon.

(In reply to comment #19)
> did this fix test OK ? Since it fixes the CP2K issue, I would hope that it
> could be posted for review soon.

The patch is OK to commit along with a testcase from this PR.

Subject: Re:  [4.3 Regression] Miscompilation with -O1

> did this fix test OK ? Since it fixes the CP2K issue, I would hope that it
> could be posted for review soon.

I can't get to testing this patch fully until Friday evening (PST) at
the earliest.  Sorry about that.  My machine was crashing and then I
left for Japan on Monday and won't get access to a machine until
Friday at the earliest with all the meetings in Japan.

-- Pinski

It would be nice if a fortraner could make the testcase not output to stdout/err
but to /dev/null instead.

open(6,'/dev/null')

didn't work for me ;)

Just to make a testcase suitable for the testsuite.

stupid me.  open(6,file='/dev/null')  works.

(In reply to comment #22)
> It would be nice if a fortraner could make the testcase not output to
> stdout/err
> but to /dev/null instead.
> 
> open(6,'/dev/null')
> 
> didn't work for me ;)
> 
> Just to make a testcase suitable for the testsuite.

The following testcase, for an unpatched compiler, aborts at -O1, works fine at -O0, and doesn't write to stdout/stderr... I guess that fits the testsuite ?

MODULE TEST
CONTAINS
PURE FUNCTION s2a_3(s1,s2,s3) RESULT(a)
    CHARACTER(LEN=*), INTENT(IN)             :: s1, s2, s3
    CHARACTER(LEN=4), DIMENSION(3)        :: a

  a(1)=s1; a(2)=s2; a(3)=s3
END FUNCTION
END MODULE

USE TEST
character(len=12) :: line
write(line,'(3A4)') s2a_3("a","bb","ccc")
IF (line.NE."a   bb  ccc") CALL ABORT()
END

Here is a testcase which also checks the resulting array is correct:
MODULE TEST
CONTAINS
PURE FUNCTION s2a_3(s1,s2,s3) RESULT(a)
    CHARACTER(LEN=*), INTENT(IN)             :: s1, s2, s3
    CHARACTER(LEN=1000), DIMENSION(3)        :: a

  a(1)=s1; a(2)=s2; a(3)=s3
END FUNCTION
END MODULE

USE TEST
character(LEN=80) :: b(3)
b=s2a_3("Distribution by marix blocks", "Distribution by matrix rows",&
         "Distribution by matrix columns")
if (b(1) .eq. "Distribution by marix blocks") call abort();
if (b(2) .eq. "Distribution by matrix rows") call abort();
if (b(3) .eq. "Distribution by matrix columns") call abort();
END

(In reply to comment #25)
> Here is a testcase which also checks the resulting array is correct:
oh s/eq/ne/ I did not test mine and it is 11pm and I have to get up early in the morning.

Subject: Re:  [4.3 Regression] Miscompilation with -O1

On Wed, 20 Jun 2007, jv244 at cam dot ac dot uk wrote:

> 
> 
> ------- Comment #24 from jv244 at cam dot ac dot uk  2007-06-20 14:03 -------
> (In reply to comment #22)
> > It would be nice if a fortraner could make the testcase not output to
> > stdout/err
> > but to /dev/null instead.
> > 
> > open(6,'/dev/null')
> > 
> > didn't work for me ;)
> > 
> > Just to make a testcase suitable for the testsuite.
> 
> The following testcase, for an unpatched compiler, aborts at -O1, works fine at
> -O0, and doesn't write to stdout/stderr... I guess that fits the testsuite ?

Yep, I'll use that.

Thanks,
Richard.
 
> MODULE TEST
> CONTAINS
> PURE FUNCTION s2a_3(s1,s2,s3) RESULT(a)
>     CHARACTER(LEN=*), INTENT(IN)             :: s1, s2, s3
>     CHARACTER(LEN=4), DIMENSION(3)        :: a
> 
>   a(1)=s1; a(2)=s2; a(3)=s3
> END FUNCTION
> END MODULE
> 
> USE TEST
> character(len=12) :: line
> write(line,'(3A4)') s2a_3("a","bb","ccc")
> IF (line.NE."a   bb  ccc") CALL ABORT()
> END
> 
> 
> 

Subject: Bug 32140

Author: rguenth
Date: Wed Jun 20 14:57:10 2007
New Revision: 125886

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=125886
Log:
2007-06-20  Andrew Pinski  <andrew_pinski@playstation.sony.com>
	Richard Guenther  <rguenther@suse.de>

	PR fortran/32140
	* trans.c (gfc_build_addr_expr): Use the correct types.

	* gfortran.fortran-torture/execute/pr32140.f90: New testcase.

Added:
    trunk/gcc/testsuite/gfortran.fortran-torture/execute/pr32140.f90
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/trans.c
    trunk/gcc/testsuite/ChangeLog

Fixe.d