[Bug other/52754] New: [4.7 regression, miscompilation] indirect indexing broken with -fpredictive-commoning

[Bug other/52754] New: [4.7 regression, miscompilation] indirect indexing broken with -fpredictive-commoning

[kretz at kde dot org]

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

             Bug #: 52754
           Summary: [4.7 regression, miscompilation] indirect indexing
                    broken with -fpredictive-commoning
    Classification: Unclassified
           Product: gcc
           Version: 4.7.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: other
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: kretz@kde.org


Created attachment 27022
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=27022
testcase

The attached testcase is compiled to access the indexes array at bogus offsets.
If line 8 is removed and line 7 written as
    unsigned int indexes[8] = { 0, 1, 2, 3, 4, 5, 6, 7 };
the error disappears.

% /opt/gcc-4.7.0/bin/g++ -msse2 -v -O1 -fpredictive-commoning main.cpp
Using built-in specs.
COLLECT_GCC=/opt/gcc-4.7.0/bin/g++
COLLECT_LTO_WRAPPER=/opt/gcc-4.7.0/libexec/gcc/x86_64-linux-gnu/4.7.0/lto-wrapper
Target: x86_64-linux-gnu
Configured with: ./configure --prefix=/opt/gcc-4.7.0 --build=x86_64-linux-gnu
--host=x86_64-linux-gnu --enable-languages=c,c++,fortran
--with-gmp=/opt/gcc-4.7.0 --with-mpfr=/opt/gcc-4.7.0 --with-ppl=/opt/gcc-4.7.0
--with-cloog=/opt/gcc-4.7.0 --with-libelf=/opt/gcc-4.7.0
--with-mpc=/opt/gcc-4.7.0 --enable-lto
Thread model: posix
gcc version 4.7.0 (GCC)
COLLECT_GCC_OPTIONS='-msse2' '-v' '-O1' '-fpredictive-commoning'
'-shared-libgcc' '-mtune=generic' '-march=x86-64'
 /opt/gcc-4.7.0/libexec/gcc/x86_64-linux-gnu/4.7.0/cc1plus -quiet -v
-imultiarch x86_64-linux-gnu -D_GNU_SOURCE main.cpp -quiet -dumpbase main.cpp
-msse2 -mtune=generic -march=x86-64 -auxbase main -O1 -version
-fpredictive-commoning -o /tmp/ccmv6Zx9.s
GNU C++ (GCC) version 4.7.0 (x86_64-linux-gnu)
        compiled by GNU C version 4.7.0, GMP version 5.0.4, MPFR version 3.1.0,
MPC version 0.9
GGC heuristics: --param ggc-min-expand=100 --param ggc-min-heapsize=131072
ignoring nonexistent directory "/usr/local/include/x86_64-linux-gnu"
ignoring nonexistent directory
"/opt/gcc-4.7.0/lib/gcc/x86_64-linux-gnu/4.7.0/../../../../x86_64-linux-gnu/include"
#include "..." search starts here:
#include <...> search starts here:
 /opt/gcc-4.7.0/lib/gcc/x86_64-linux-gnu/4.7.0/../../../../include/c++/4.7.0

/opt/gcc-4.7.0/lib/gcc/x86_64-linux-gnu/4.7.0/../../../../include/c++/4.7.0/x86_64-linux-gnu

/opt/gcc-4.7.0/lib/gcc/x86_64-linux-gnu/4.7.0/../../../../include/c++/4.7.0/backward
 /opt/gcc-4.7.0/lib/gcc/x86_64-linux-gnu/4.7.0/include
 /usr/local/include
 /opt/gcc-4.7.0/include
 /opt/gcc-4.7.0/lib/gcc/x86_64-linux-gnu/4.7.0/include-fixed
 /usr/include/x86_64-linux-gnu
 /usr/include
End of search list.
GNU C++ (GCC) version 4.7.0 (x86_64-linux-gnu)
        compiled by GNU C version 4.7.0, GMP version 5.0.4, MPFR version 3.1.0,
MPC version 0.9
GGC heuristics: --param ggc-min-expand=100 --param ggc-min-heapsize=131072
Compiler executable checksum: 32eb1a24aeca8b97e0c59b4b063f668f
COLLECT_GCC_OPTIONS='-msse2' '-v' '-O1' '-fpredictive-commoning'
'-shared-libgcc' '-mtune=generic' '-march=x86-64'

/opt/gcc-4.7.0/lib/gcc/x86_64-linux-gnu/4.7.0/../../../../x86_64-linux-gnu/bin/as
--64 -o /tmp/cca4x2R5.o /tmp/ccmv6Zx9.s
COMPILER_PATH=/opt/gcc-4.7.0/libexec/gcc/x86_64-linux-gnu/4.7.0/:/opt/gcc-4.7.0/libexec/gcc/x86_64-linux-gnu/4.7.0/:/opt/gcc-4.7.0/libexec/gcc/x86_64-linux-gnu/:/opt/gcc-4.7.0/lib/gcc/x86_64-linux-gnu/4.7.0/:/opt/gcc-4.7.0/lib/gcc/x86_64-linux-gnu/:/opt/gcc-4.7.0/lib/gcc/x86_64-linux-gnu/4.7.0/../../../../x86_64-linux-gnu/bin/
LIBRARY_PATH=/opt/gcc-4.7.0/lib/gcc/x86_64-linux-gnu/4.7.0/:/opt/gcc-4.7.0/lib/gcc/x86_64-linux-gnu/4.7.0/../../../../lib64/:/lib/x86_64-linux-gnu/:/lib/../lib64/:/usr/lib/x86_64-linux-gnu/:/opt/gcc-4.7.0/lib/gcc/x86_64-linux-gnu/4.7.0/../../../../x86_64-linux-gnu/lib/:/opt/gcc-4.7.0/lib/gcc/x86_64-linux-gnu/4.7.0/../../../:/lib/:/usr/lib/
COLLECT_GCC_OPTIONS='-msse2' '-v' '-O1' '-fpredictive-commoning'
'-shared-libgcc' '-mtune=generic' '-march=x86-64'
 /opt/gcc-4.7.0/libexec/gcc/x86_64-linux-gnu/4.7.0/collect2 --eh-frame-hdr -m
elf_x86_64 -dynamic-linker /lib64/ld-linux-x86-64.so.2
/usr/lib/x86_64-linux-gnu/crt1.o /usr/lib/x86_64-linux-gnu/crti.o
/opt/gcc-4.7.0/lib/gcc/x86_64-linux-gnu/4.7.0/crtbegin.o
-L/opt/gcc-4.7.0/lib/gcc/x86_64-linux-gnu/4.7.0
-L/opt/gcc-4.7.0/lib/gcc/x86_64-linux-gnu/4.7.0/../../../../lib64
-L/lib/x86_64-linux-gnu -L/lib/../lib64 -L/usr/lib/x86_64-linux-gnu
-L/opt/gcc-4.7.0/lib/gcc/x86_64-linux-gnu/4.7.0/../../../../x86_64-linux-gnu/lib
-L/opt/gcc-4.7.0/lib/gcc/x86_64-linux-gnu/4.7.0/../../.. /tmp/cca4x2R5.o
-lstdc++ -lm -lgcc_s -lgcc -lc -lgcc_s -lgcc
/opt/gcc-4.7.0/lib/gcc/x86_64-linux-gnu/4.7.0/crtend.o
/usr/lib/x86_64-linux-gnu/crtn.o

% ./a.out
[1]    7716 bus error  ./a.out

[Bug other/52754] [4.7/4.8 Regression] indirect indexing broken with -fpredictive-commoning

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
           Keywords|                            |wrong-code
   Last reconfirmed|                            |2012-03-28
                 CC|                            |jakub at gcc dot gnu.org
     Ever Confirmed|0                           |1
            Summary|[4.7 regression,            |[4.7/4.8 Regression]
                   |miscompilation] indirect    |indirect indexing broken
                   |indexing broken with        |with -fpredictive-commoning
                   |-fpredictive-commoning      |
   Target Milestone|---                         |4.7.1

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-03-28 16:05:53 UTC ---
Started with http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=178028

[Bug other/52754] [4.7/4.8 Regression] indirect indexing broken with -fpredictive-commoning

[jakub at gcc dot gnu.org]

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

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-03-28 16:48:51 UTC ---
Seems ref_at_iteration is buggy, when iter is negative, but ARRAY_REF's index
is TYPE_UNSIGNED smaller than size of pointer, computing the ARRAY_REF's index
in the smaller unsigned type isn't a good idea, because then we end up with
those
0xfffffffdU etc. indexes while we want -3L.
Wonder if it is generally ok to do the computation in a wider type (if any),
or if we should just handle the simple cases (integer_onep (iv.step) ? ) and
punt otherwise.

[Bug other/52754] [4.7/4.8 Regression] indirect indexing broken with -fpredictive-commoning

[jakub at gcc dot gnu.org]

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

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-03-28 17:22:15 UTC ---
Created attachment 27023
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=27023
gcc48-pr52754.patch

Patch I had in mind.  Seems to fix the testcase.  Richard, what do you think
about this?

[Bug other/52754] [4.7/4.8 Regression] indirect indexing broken with -fpredictive-commoning

[rguenth at gcc dot gnu.org]

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

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

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

--- Comment #4 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-03-29 09:58:02 UTC ---
Mine.

[Bug other/52754] [4.7/4.8 Regression] indirect indexing broken with -fpredictive-commoning

[kretz at kde dot org]

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

--- Comment #5 from Matthias Kretz <kretz at kde dot org> 2012-03-29 10:24:19 UTC ---
I just tested the patch on the last 4.7 snapshot (20120324). My unit test
apparently runs without failure, but I still get incorrect warnings "warning:
array subscript is above array bounds [-Warray-bounds]".

[Bug other/52754] [4.7/4.8 Regression] indirect indexing broken with -fpredictive-commoning

[rguenth at gcc dot gnu.org]

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

--- Comment #6 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-03-30 10:22:45 UTC ---
We indeed should not create negative array indices (well, out-of-bound array
indices).  The issue why this happens is that we transform

 const unsigned int * ii = (const unsigned int *) &indexes[i];
 *(ii + 12)

to

 MEM[(const unsigned int *)&indexes + 12B][i_3];

via

            ii = &indexes[i];
            D.3849 = ii + 12;
            D.3850 = *D.3849;

forwprop1 (still ok):

  ii_9 = &indexes[i_3];
  D.3850_11 = MEM[(const unsigned int *)ii_9 + 12B];

forwprop2 (bogus):

  D.3850_11 = MEM[(const unsigned int *)&indexes + 12B][i_3];

[Bug other/52754] [4.7/4.8 Regression] indirect indexing broken with -fpredictive-commoning

[rguenth at gcc dot gnu.org]

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

--- Comment #7 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-03-30 10:45:28 UTC ---
Created attachment 27042
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=27042
alternative patch

[Bug other/52754] [4.7/4.8 Regression] indirect indexing broken with -fpredictive-commoning

[kretz at kde dot org]

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

--- Comment #8 from Matthias Kretz <kretz at kde dot org> 2012-03-30 11:13:59 UTC ---
(In reply to comment #7)
> alternative patch

Just tested it on the 4.7-20120324 snapshot and all bogus warnings are gone, as
are the crashes. No regressions in my testsuite.

[Bug other/52754] [4.7/4.8 Regression] indirect indexing broken with -fpredictive-commoning

[jakub at gcc dot gnu.org]

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

--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-03-30 11:31:58 UTC ---
(In reply to comment #7)
> Created attachment 27042 [details]
> alternative patch

I'm not against it, but what if the source code and/or some other pass result
in similar ARRAY_REF?  To me pcom still looks potentially buggy...

[Bug other/52754] [4.7/4.8 Regression] indirect indexing broken with -fpredictive-commoning

[rguenther at suse dot de]

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

--- Comment #10 from rguenther at suse dot de <rguenther at suse dot de> 2012-03-30 11:41:35 UTC ---
On Fri, 30 Mar 2012, jakub at gcc dot gnu.org wrote:

> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52754
> 
> --- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-03-30 11:31:58 UTC ---
> (In reply to comment #7)
> > Created attachment 27042 [details]
> > alternative patch
> 
> I'm not against it, but what if the source code and/or some other pass result
> in similar ARRAY_REF?  To me pcom still looks potentially buggy...

If that happens the other pass is broken or the source invokes undefined
behavior.  I had the following patch which fixed the issue, too:

Index: tree-predcom.c
===================================================================
--- tree-predcom.c      (revision 186007)
+++ tree-predcom.c      (working copy)
@@ -1414,9 +1414,11 @@ ref_at_iteration (struct loop *loop, tre
        }
       else
        {
-         val = fold_build2 (MULT_EXPR, type, iv.step,
-                            build_int_cst_type (type, iter));
-         val = fold_build2 (PLUS_EXPR, type, iv.base, val);
+         tree itype = signed_type_for (type);
+         val = fold_build2 (MULT_EXPR, itype, fold_convert (itype, 
iv.step),
+                            build_int_cst (itype, iter));
+         val = fold_build2 (PLUS_EXPR, itype,
+                            fold_convert (itype, iv.base), val);
        }
       *idx_p = unshare_expr (val);
     }

as we know iter is negative.  We could also change the caller so
we pass a positive count and instead subtract it from the base
value, though that's still going to get possibly negative
(and in case iv.base is not a constant we will not know this here).

The above would not be a real fix, instead we would need to cater
for a large unsigned iv.base, thus, promote val and iv.base to
[u]intptr_t.

But - we can't really "win" here for array indices that ultimatively
will be out-of-bound.  If some GCC pass re-constructs invalid
array-refs it has to be fixed.

[Bug other/52754] [4.7/4.8 Regression] indirect indexing broken with -fpredictive-commoning

[rguenth at gcc dot gnu.org]

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

--- Comment #11 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-03-30 13:41:31 UTC ---
Author: rguenth
Date: Fri Mar 30 13:41:24 2012
New Revision: 186012

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=186012
Log:
2012-03-30  Richard Guenther  <rguenther@suse.de>

    PR tree-optimization/52754
    * tree-ssa-forwprop.c (forward_propagate_addr_expr_1): Only
    propagate arbitrary addresses into really plain dereferences.

    * gcc.target/i386/pr52754.c: New testcase.

Added:
    trunk/gcc/testsuite/gcc.target/i386/pr52754.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-ssa-forwprop.c

[Bug other/52754] [4.7 Regression] indirect indexing broken with -fpredictive-commoning

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |4.8.0
            Summary|[4.7/4.8 Regression]        |[4.7 Regression] indirect
                   |indirect indexing broken    |indexing broken with
                   |with -fpredictive-commoning |-fpredictive-commoning

--- Comment #12 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-03-30 13:42:14 UTC ---
Fixed on trunk sofar.

[Bug other/52754] [4.7 Regression] indirect indexing broken with -fpredictive-commoning

[rguenth at gcc dot gnu.org]

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

--- Comment #13 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-04-03 11:55:30 UTC ---
Author: rguenth
Date: Tue Apr  3 11:55:24 2012
New Revision: 186105

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=186105
Log:
2012-04-03  Richard Guenther  <rguenther@suse.de>

    Backport from mainline
    2012-03-06  Richard Guenther  <rguenther@suse.de>

    PR middle-end/52493
    * tree-ssa-alias.c (ptr_derefs_may_alias_p): Robustify.

    * gcc.dg/torture/pr52493.c: New testcase.

    2012-03-23  Richard Guenther  <rguenther@suse.de>

    PR tree-optimization/52678
    * tree-vectorizer.h (struct _stmt_vec_info): Add
    loop_phi_evolution_part member.
    (STMT_VINFO_LOOP_PHI_EVOLUTION_PART): New define.
    * tree-vect-loop.c (vect_analyze_scalar_cycles_1): Initialize
    STMT_VINFO_LOOP_PHI_EVOLUTION_PART.
    * tree-vect-loop-manip.c (vect_update_ivs_after_vectorizer):
    Use the cached evolution part and the PHI nodes value from
    the loop preheader edge instead of re-analyzing the evolution.

    * gfortran.dg/pr52678.f: New testcase.

    2012-03-26  Richard Guenther  <rguenther@suse.de>

    PR tree-optimization/52701
    * tree-vect-loop.c (vect_analyze_scalar_cycles_1): Always
    compute and set the evolution part of PHI nodes.

    * gfortran.dg/pr52701.f90: New testcase.

    2012-03-30  Richard Guenther  <rguenther@suse.de>

    PR tree-optimization/52754
    * tree-ssa-forwprop.c (forward_propagate_addr_expr_1): Only
    propagate arbitrary addresses into really plain dereferences.

    * gcc.target/i386/pr52754.c: New testcase.

Added:
    branches/gcc-4_7-branch/gcc/testsuite/gcc.dg/torture/pr52493.c
    branches/gcc-4_7-branch/gcc/testsuite/gcc.target/i386/pr52754.c
    branches/gcc-4_7-branch/gcc/testsuite/gfortran.dg/pr52678.f
    branches/gcc-4_7-branch/gcc/testsuite/gfortran.dg/pr52701.f90
Modified:
    branches/gcc-4_7-branch/gcc/ChangeLog
    branches/gcc-4_7-branch/gcc/testsuite/ChangeLog
    branches/gcc-4_7-branch/gcc/tree-ssa-alias.c
    branches/gcc-4_7-branch/gcc/tree-ssa-forwprop.c
    branches/gcc-4_7-branch/gcc/tree-vect-loop-manip.c
    branches/gcc-4_7-branch/gcc/tree-vect-loop.c
    branches/gcc-4_7-branch/gcc/tree-vectorizer.h

[Bug other/52754] [4.7 Regression] indirect indexing broken with -fpredictive-commoning

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
      Known to work|                            |4.7.1
         Resolution|                            |FIXED
      Known to fail|                            |4.7.0

--- Comment #14 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-04-03 11:55:56 UTC ---
Fixed.