Bug 37861 - [4.3 Regression] Bogus array bounds warning
Summary: [4.3 Regression] Bogus array bounds warning
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: middle-end (show other bugs)
Version: 4.3.2
: P2 normal
Target Milestone: 4.3.4
Assignee: Martin Jambor
URL:
Keywords: diagnostic
: 35279 (view as bug list)
Depends on:
Blocks:
 
Reported: 2008-10-17 08:26 UTC by Richard Biener
Modified: 2009-02-28 22:46 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work: 4.2.4 4.4.0
Known to fail: 4.3.2
Last reconfirmed: 2008-11-10 18:39:51


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Richard Biener 2008-10-17 08:26:28 UTC
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>;
Comment 1 Martin Jambor 2008-10-30 17:43:52 UTC
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%] 

}
Comment 2 Manuel López-Ibáñez 2008-10-30 18:43:24 UTC
So what is this? Is the warning logic wrong or is the IR wrong? It seems to me that IR is valid.
Comment 3 Martin Jambor 2008-10-31 17:52:56 UTC
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;
}
Comment 4 Martin Jambor 2008-10-31 18:01:29 UTC
(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.
Comment 5 Martin Jambor 2008-11-04 15:51:25 UTC
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;

Comment 6 Martin Jambor 2008-11-05 16:08:12 UTC
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

Comment 7 Jakub Jelinek 2008-11-05 20:50:06 UTC
Fixed on the trunk so far.
Comment 8 Martin Jambor 2008-11-10 10:06:31 UTC
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;
 


Comment 9 Andrew Pinski 2008-11-14 23:59:57 UTC
*** Bug 35279 has been marked as a duplicate of this bug. ***
Comment 10 Martin Jambor 2008-12-02 14:32:33 UTC
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

Comment 11 Richard Biener 2009-01-24 10:20:54 UTC
GCC 4.3.3 is being released, adjusting target milestone.
Comment 12 Martin Jambor 2009-02-28 00:30:51 UTC
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

Comment 13 Martin Jambor 2009-02-28 18:33:45 UTC
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

Comment 14 Martin Jambor 2009-02-28 22:46:21 UTC
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