Bug 30655 - Undue out-of-bounds warning
Summary: Undue out-of-bounds warning
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: fortran (show other bugs)
Version: 4.3.0
: P3 normal
Target Milestone: 4.3.0
Assignee: Francois-Xavier Coudert
URL:
Keywords: diagnostic, patch
Depends on:
Blocks: Fortran_bounds_checking
  Show dependency treegraph
 
Reported: 2007-01-31 14:17 UTC by Francois-Xavier Coudert
Modified: 2007-03-24 20:21 UTC (History)
4 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail: 4.1.2 4.2.0 4.3.0
Last reconfirmed: 2007-03-14 10:48:06


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Francois-Xavier Coudert 2007-01-31 14:17:57 UTC
gfortran should not report an out-of-bounds access on the following code:

$ cat a.f90
  integer i(12), j
  j = -1
  i(0:j) = 42
  end
$ gfortran a.f90                       
a.f90:3.4:

  i(0:j) = 42
   1
Warning: Array reference at (1) is out of bounds


Compiling with -fbounds-check and running it doesn't error, which means the -fbounds-check code is generated fine. But, somewhere, the front-end is over-zealous (it doesn't know about stride). I thought I had that fixed at some point...
Comment 1 Thomas Koenig 2007-01-31 18:13:13 UTC
Really?

This is fine:

  integer i(12), j
  i(0:-1) = 42
  end

In your test case, the compiler has a hard time detecting the
value of j, and the lower bound would be incorrect if j >=1.
Comment 2 Francois-Xavier Coudert 2007-01-31 22:05:59 UTC
Around resolve.c:2518:

      if (((compare_bound_int (ar->stride[i], 0) == CMP_GT
            || ar->stride[i] == NULL)
           && compare_bound (AR_START, AR_END) != CMP_GT)
          || (compare_bound_int (ar->stride[i], 0) == CMP_LT
              && compare_bound (AR_START, AR_END) != CMP_LT))

when I wrote that code, I stupidly assumed that compare_bound(...) != CMP_GT was equivalent to compare_bound(...) == CMP_LT || compare_bound(...) == CMP_EQ, and it's wrong because there's also CMP_UNKNOWN!

The following trivial patch fixes it (I also create a variable to hold the comparison of AR_START and AR_END, because we use it multiple times).


Index: resolve.c
===================================================================
--- resolve.c   (revision 121280)
+++ resolve.c   (working copy)
@@ -2499,48 +2499,51 @@
       break;
 
     case AR_SECTION:
-      if (compare_bound_int (ar->stride[i], 0) == CMP_EQ)
-       {
-         gfc_error ("Illegal stride of zero at %L", &ar->c_where[i]);
-         return FAILURE;
-       }
-
+      {
 #define AR_START (ar->start[i] ? ar->start[i] : as->lower[i])
 #define AR_END (ar->end[i] ? ar->end[i] : as->upper[i])
 
-      if (compare_bound (AR_START, AR_END) == CMP_EQ
-         && (compare_bound (AR_START, as->lower[i]) == CMP_LT
-             || compare_bound (AR_START, as->upper[i]) == CMP_GT))
-       goto bound;
+       comparison comp_start_end = compare_bound (AR_START, AR_END);
 
-      if (((compare_bound_int (ar->stride[i], 0) == CMP_GT
-           || ar->stride[i] == NULL)
-          && compare_bound (AR_START, AR_END) != CMP_GT)
-         || (compare_bound_int (ar->stride[i], 0) == CMP_LT
-             && compare_bound (AR_START, AR_END) != CMP_LT))
-       {
-         if (compare_bound (AR_START, as->lower[i]) == CMP_LT)
-           goto bound;
-         if (compare_bound (AR_START, as->upper[i]) == CMP_GT)
-           goto bound;
-       }
+       if (compare_bound_int (ar->stride[i], 0) == CMP_EQ)
+         {
+           gfc_error ("Illegal stride of zero at %L", &ar->c_where[i]);
+           return FAILURE;
+         }
 
-      mpz_init (last_value);
-      if (compute_last_value_for_triplet (AR_START, AR_END, ar->stride[i],
-                                         last_value))
-       {
-         if (compare_bound_mpz_t (as->lower[i], last_value) == CMP_GT
-             || compare_bound_mpz_t (as->upper[i], last_value) == CMP_LT)
-           {
-             mpz_clear (last_value);
+       if (comp_start_end == CMP_EQ
+           && (compare_bound (AR_START, as->lower[i]) == CMP_LT
+               || compare_bound (AR_START, as->upper[i]) == CMP_GT))
+         goto bound;
+
+       if (((compare_bound_int (ar->stride[i], 0) == CMP_GT
+             || ar->stride[i] == NULL)
+            && (comp_start_end == CMP_LT || comp_start_end == CMP_EQ))
+           || (compare_bound_int (ar->stride[i], 0) == CMP_LT
+               && (comp_start_end == CMP_GT || comp_start_end == CMP_EQ)))
+         {
+           if (compare_bound (AR_START, as->lower[i]) == CMP_LT)
              goto bound;
-           }
-       }
-      mpz_clear (last_value);
+           if (compare_bound (AR_START, as->upper[i]) == CMP_GT)
+             goto bound;
+         }
 
+       mpz_init (last_value);
+       if (compute_last_value_for_triplet (AR_START, AR_END, ar->stride[i],
+                                           last_value))
+         {
+           if (compare_bound_mpz_t (as->lower[i], last_value) == CMP_GT
+               || compare_bound_mpz_t (as->upper[i], last_value) == CMP_LT)
+             {
+               mpz_clear (last_value);
+               goto bound;
+             }
+         }
+        mpz_clear (last_value);
+
 #undef AR_START
 #undef AR_END
-
+      }
       break;
 
     default:
Comment 3 Francois-Xavier Coudert 2007-03-24 20:20:06 UTC
Subject: Bug 30655

Author: fxcoudert
Date: Sat Mar 24 20:19:51 2007
New Revision: 123187

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

	* expr.c (check_dimension): Fix logic of comparisons.

	* gfortran.dg/bounds_check_6.f90: New test.

Added:
    trunk/gcc/testsuite/gfortran.dg/bounds_check_6.f90
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/resolve.c
    trunk/gcc/testsuite/ChangeLog

Comment 4 Francois-Xavier Coudert 2007-03-24 20:21:40 UTC
Fixed on 4.3, will not backport.