extern int printf (__const char *__restrict __format, ...); static int f2(char formatstr[10][100]) { int anz; for( anz = 0; anz < 10; ++anz ) { printf( "%d %s\n", anz, formatstr[anz] ); } return anz; } static char formatstr[10][100]; int main( void ) { int anz; anz = f2(formatstr); printf( " %d\n",anz); return 0; } with -O3 -Wall this reports t.c: In function ‘main’: t.c:6: warning: array subscript is above array bounds t.c:6: warning: array subscript is above array bounds t.c:6: warning: array subscript is above array bounds t.c:6: warning: array subscript is above array bounds t.c:6: warning: array subscript is above array bounds t.c:6: warning: array subscript is above array bounds t.c:6: warning: array subscript is above array bounds t.c:6: warning: array subscript is above array bounds because we end up with <bb 3>: # anz_23 = PHI <anz_6(4), anz_22(6)> D.1572_5 = (long unsigned int) anz_23; D.1573_7 = D.1572_5 * 100; D.1574_8 = &formatstr[0][D.1573_7]; printf (&"%d %s\n"[0], anz_23, D.1574_8); anz_9 = anz_23 + 1; <bb 4>: # anz_6 = PHI <anz_9(3)> if (anz_6 <= 9) goto <bb 3>; else goto <bb 5>;
Well, yes, we do generate that code. However, the loop is unrolled later on and the IR code on which the vrp complains later on actually is: main () { unsigned int ivtmp.27; unsigned int pretmp.17; int pretmp.16; unsigned int pretmp.15; unsigned int anz.0; unsigned int D.1258; char * D.1259; int anz; # BLOCK 2 freq:909 # PRED: ENTRY [100.0%] (fallthru,exec) D.1259_28 = &formatstr[0][0]; printf (&"%d %s\n"[0], 0, D.1259_28); D.1259_39 = &formatstr[0][100]; printf (&"%d %s\n"[0], 1, D.1259_39); D.1259_50 = &formatstr[0][200]; printf (&"%d %s\n"[0], 2, D.1259_50); D.1259_61 = &formatstr[0][300]; printf (&"%d %s\n"[0], 3, D.1259_61); D.1259_72 = &formatstr[0][400]; printf (&"%d %s\n"[0], 4, D.1259_72); D.1259_83 = &formatstr[0][500]; printf (&"%d %s\n"[0], 5, D.1259_83); D.1259_94 = &formatstr[0][600]; printf (&"%d %s\n"[0], 6, D.1259_94); D.1259_105 = &formatstr[0][700]; printf (&"%d %s\n"[0], 7, D.1259_105); D.1259_116 = &formatstr[0][800]; printf (&"%d %s\n"[0], 8, D.1259_116); D.1259_7 = &formatstr[0][900]; printf (&"%d %s\n"[0], 9, D.1259_7); printf (&" %d\n"[0], 10); return 0; # SUCC: EXIT [100.0%] }
So what is this? Is the warning logic wrong or is the IR wrong? It seems to me that IR is valid.
The test-case in the bug description leads to bogus warnings in the second run of the VRP pass. Yesterday me and Richi discussed the possibility of simply not-giving out any warnings in the second runs (as opposed to the first which would still generate the warnings it does). However, I have managed to modify the test case so that bogus warnings are spitted out in the first run and so this workaround would not really solve the problem: extern int printf (__const char *__restrict __format, ...); static int f3(int v) { int i,j = 0; for (i = 0; i <= v; i++) j++; return j; } static int f2(char formatstr[10][100]) { printf( "%d %s\n", 0, formatstr[f3(0)] ); printf( "%d %s\n", 1, formatstr[f3(1)] ); printf( "%d %s\n", 2, formatstr[f3(2)] ); printf( "%d %s\n", 3, formatstr[f3(3)] ); return 3; } static char formatstr[10][100]; int main( void ) { int anz; anz = f2(formatstr); printf( " %d\n",anz); return 0; }
(In reply to comment #2) > So what is this? Is the warning logic wrong or is the IR wrong? It seems to me > that IR is valid. > Well, it probabaly isn't. I guess the second index should not ever exceed its upper bound (100 in these test cases) and it blatantly does. The proper solution (again, as suggested by Richi today) therefore most probabaly is "not to re-create ARRAY_REF for multi-dimensional arrays" at some place in folding.
Right, so this is the most simple (albeit not yet tested) patch I've been able to come up with. I am not sure what overall impact this is going to have. I'll briefly try to come up with something more sophisticated... Index: gcc/tree-ssa-forwprop.c =================================================================== --- gcc/tree-ssa-forwprop.c (revision 141546) +++ gcc/tree-ssa-forwprop.c (working copy) @@ -812,6 +812,7 @@ forward_propagate_addr_expr_1 (tree name array_ref = TREE_OPERAND (def_rhs, 0); if (TREE_CODE (array_ref) != ARRAY_REF || TREE_CODE (TREE_TYPE (TREE_OPERAND (array_ref, 0))) != ARRAY_TYPE + || TREE_CODE (TREE_OPERAND (array_ref, 0)) == INDIRECT_REF || !integer_zerop (TREE_OPERAND (array_ref, 1))) return false;
Subject: Bug 37861 Author: jamborm Date: Wed Nov 5 16:06:38 2008 New Revision: 141613 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=141613 Log: 2008-11-05 Martin Jambor <mjambor@suse.cz> PR middle-end/37861 * tree-ssa-forwprop.c (forward_propagate_addr_expr_1): Don't turn pointer arithmetics into array_ref if the array is accessed through an indirect_ref. * testsuite/gcc.dg/Warray-bounds-5.c: New file. * testsuite/gcc.dg/Warray-bounds-6.c: New file. Added: trunk/gcc/testsuite/gcc.dg/Warray-bounds-5.c trunk/gcc/testsuite/gcc.dg/Warray-bounds-6.c Modified: trunk/gcc/ChangeLog trunk/gcc/testsuite/ChangeLog trunk/gcc/tree-ssa-forwprop.c
Fixed on the trunk so far.
The previous patch resulted into a regression on m32c-unknown-elf and thus I prepared a less intrusive one below. See also: * http://gcc.gnu.org/ml/gcc/2008-11/msg00058.html and * http://gcc.gnu.org/ml/gcc-patches/2008-11/msg00353.html The patch is pending approval in the mailing list. 2008-11-07 Martin Jambor <mjambor@suse.cz> * tree-ssa-forwprop.c (forward_propagate_addr_expr_1): Do not check for INDIRECT_REFs. (forward_propagate_addr_into_variable_array_index): Check that the offset is not computed from a MULT_EXPR, use is_gimple_assign rather than the gimple code directly. Index: gcc/tree-ssa-forwprop.c =================================================================== --- gcc/tree-ssa-forwprop.c (revision 141673) +++ gcc/tree-ssa-forwprop.c (working copy) @@ -613,19 +613,27 @@ forward_propagate_addr_into_variable_arr tree index; gimple offset_def, use_stmt = gsi_stmt (*use_stmt_gsi); - /* Try to find an expression for a proper index. This is either - a multiplication expression by the element size or just the - ssa name we came along in case the element size is one. */ + /* Get the offset's defining statement. */ + offset_def = SSA_NAME_DEF_STMT (offset); + + /* Try to find an expression for a proper index. This is either a + multiplication expression by the element size or just the ssa name we came + along in case the element size is one. In that case, however, we do not + allow multiplications because they can be computing index to a higher + level dimension (PR 37861). */ if (integer_onep (TYPE_SIZE_UNIT (TREE_TYPE (TREE_TYPE (def_rhs))))) - index = offset; - else { - /* Get the offset's defining statement. */ - offset_def = SSA_NAME_DEF_STMT (offset); + if (is_gimple_assign (offset_def) + && gimple_assign_rhs_code (offset_def) == MULT_EXPR) + return false; + index = offset; + } + else + { /* The statement which defines OFFSET before type conversion must be a simple GIMPLE_ASSIGN. */ - if (gimple_code (offset_def) != GIMPLE_ASSIGN) + if (!is_gimple_assign (offset_def)) return false; /* The RHS of the statement which defines OFFSET must be a @@ -802,9 +810,6 @@ forward_propagate_addr_expr_1 (tree name array_ref = TREE_OPERAND (def_rhs, 0); if (TREE_CODE (array_ref) != ARRAY_REF || TREE_CODE (TREE_TYPE (TREE_OPERAND (array_ref, 0))) != ARRAY_TYPE - /* Avoid accessing hidden multidimensional arrays in this way or VRP - might give out bogus warnings (see PR 37861) */ - || TREE_CODE (TREE_OPERAND (array_ref, 0)) == INDIRECT_REF || !integer_zerop (TREE_OPERAND (array_ref, 1))) return false;
*** Bug 35279 has been marked as a duplicate of this bug. ***
Subject: Bug 37861 Author: jamborm Date: Tue Dec 2 14:30:55 2008 New Revision: 142355 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=142355 Log: 2008-12-02 Martin Jambor <mjambor@suse.cz> PR middle-end/37861 * tree-ssa-forwprop.c (forward_propagate_addr_expr_1): Do not check for INDIRECT_REFs. (forward_propagate_addr_into_variable_array_index): Check that the offset is not computed from a MULT_EXPR, use is_gimple_assign rather than the gimple code directly. Modified: trunk/gcc/ChangeLog trunk/gcc/tree-ssa-forwprop.c
GCC 4.3.3 is being released, adjusting target milestone.
I have just posted a patch to fix this issue on the 4.3 branch to the mailing list: http://gcc.gnu.org/ml/gcc-patches/2009-02/msg01245.html
Subject: Bug 37861 Author: jamborm Date: Sat Feb 28 18:33:27 2009 New Revision: 144491 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=144491 Log: 2009-02-28 Martin Jambor <mjambor@suse.cz> Backport from mainline: 2008-12-02 Martin Jambor <mjambor@suse.cz> PR middle-end/37861 * tree-ssa-forwprop.c (forward_propagate_addr_into_variable_array_index): Check that the offset is not computed from a MULT_EXPR if element size is one. Modified: branches/gcc-4_3-branch/gcc/ChangeLog branches/gcc-4_3-branch/gcc/tree-ssa-forwprop.c
Fixed with revision 144491: te: Sat Feb 28 18:33:27 2009 New Revision: 144491 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=144491 Log: 2009-02-28 Martin Jambor <mjambor@suse.cz> Backport from mainline: 2008-12-02 Martin Jambor <mjambor@suse.cz> PR middle-end/37861 * tree-ssa-forwprop.c (forward_propagate_addr_into_variable_array_index): Check that the offset is not computed from a MULT_EXPR if element size is one. Modified: branches/gcc-4_3-branch/gcc/ChangeLog branches/gcc-4_3-branch/gcc/tree-ssa-forwprop.c