[Bug c/69224] New: -Warray-bounds false positive with -O3 and struct pointer parameter

classic Classic list List threaded Threaded
20 messages Options
Reply | Threaded
Open this post in threaded view
|

[Bug c/69224] New: -Warray-bounds false positive with -O3 and struct pointer parameter

msebor at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69224

            Bug ID: 69224
           Summary: -Warray-bounds false positive with -O3 and struct
                    pointer parameter
           Product: gcc
           Version: 5.3.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: nightstrike at gmail dot com
  Target Milestone: ---

There are a lot of -Warray-bounds false positives at -O3, but this one is
interesting:

struct S {
        int a;
        int b;
        int c;
        int d;
        int e;
        float x[5];
        float y[5];      // Comment these two lines out to
        float z[5 - 1];  // remove the warning
};
void f(struct S *s, float a[], float **b, float c[]) {
        if ((s->b == 1) && (s->d > 0)) {
                for (int i = 0; i < s->a; i++) {
                        if (a[i] != 0.0) {
                                for (int j = 0; j < s->d - 1; j++) {
                                        if ((c[i] >= s->x[j]) && (c[i] <=
s->x[j + 1])) {
                                                b[2*j][i] = s->x[j];
                                                break;
                                        }
                                }
                        }
                }
        }
}



If you comment out those two variables in the struct, both of which are unused,
then the warning goes away.

Note that in this particular case, since the loop boundary is passed in at run
time, there's no way for the compiler to ever know if we are exceeding the
array bounds.

I've included a few of the other -Warray-bounds bugs in the See Also lists in
the hopes that maybe some of these would be related.
Reply | Threaded
Open this post in threaded view
|

[Bug c/69224] -Warray-bounds false positive with -O3 and struct pointer parameter

msebor at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69224

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2016-01-11
                 CC|                            |hubicka at gcc dot gnu.org
     Ever confirmed|0                           |1

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
If you comment the line x is a trailing array we don't know its size for and
thus
we don't warn.

As with (many?) other cases this happens because complete peeling added one
copy too much (too conservative number of iteration upper bound):

  _72 = s_47->x[4];
  if (_39 >= _72)
    goto <bb 20>;
  else
    goto <bb 23>;

  <bb 20>:
  _74 = s_47->x[5];
  if (_39 <= _74)
    goto <bb 22>;
  else
    goto <bb 23>;

so we have a x[5] in the IL and VRP can't prove it is not reachable either
(obviously).

niter analysis does:

Statement _22 = s_7(D)->x[j_2];
 is executed at most 4 (bounded by 4) + 1 times in loop 2.

Statement _25 = s_7(D)->x[_24];
 is executed at most 3 (bounded by 3) + 1 times in loop 2.

Loop 2 iterates at most 5 times.

t.c:16:33: note: loop with 6 iterations completely unrolled
Latch of last iteration was marked by __builtin_unreachable ().
Forced statement unreachable: _25 = s_7(D)->x[_24];
Forced statement unreachable: _22 = s_7(D)->x[j_2];

it looks like the conditionally read s->x[j] is not properly marked
unreachable.
Reply | Threaded
Open this post in threaded view
|

[Bug tree-optimization/69224] [4.9/5/6 Regression] -Warray-bounds false positive with -O3 and struct pointer parameter

msebor at gcc dot gnu.org
In reply to this post by msebor at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69224

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|c                           |tree-optimization
      Known to work|                            |4.6.4, 4.7.3
           See Also|                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=68963
   Target Milestone|---                         |4.9.4
            Summary|-Warray-bounds false        |[4.9/5/6 Regression]
                   |positive with -O3 and       |-Warray-bounds false
                   |struct pointer parameter    |positive with -O3 and
                   |                            |struct pointer parameter
      Known to fail|                            |4.8.5, 6.0

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
Possibly related to PR68963.
Reply | Threaded
Open this post in threaded view
|

[Bug tree-optimization/69224] [4.9/5/6 Regression] -Warray-bounds false positive with -O3 and struct pointer parameter

msebor at gcc dot gnu.org
In reply to this post by msebor at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69224

--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
Ok, so we have

                for (int j = 0; j < s->d - 1; j++) {
                    if ((c[i] >= s->x[j]) && (c[i] <= s->x[j + 1])) {
                        b[2*j][i] = s->x[j];
                        break;
                    }
                }

and we can conclude j is in [0, 4] because of the c[i] >= s->x[j] check.
But we can _not_ conclude it is in [0, 3] because of the short-circuited
c[i] <= s->x[j+1] check.  So we're left with

 if (c[4] >= s->x[4])
  {
    if (c[5] <= s->x[j + 1])
      {
 ...
      }
  }

in the last iteration.  The only issue is that we warn "is above array bounds"
rather than "may be" (well, if the access is executed then it _is_ above
array bounds, we never warn "maybe", all array-bound warnigns are "maybe"
in some way, unless in main() and always executable).

So I think we can't do anything here but not optimize (completely peel).

The issue is that usually VRP doesn't warn if it gets a range for the
access that is [0, 5] because its ranges are conservative and that would
result in way too many diagnostics.  Now the complete peeling makes the
ranges very precise (single element) and we get those false positive
warnings.

Fact is that at runtime unless s->d is "proper" (I guess it's 5) the
access may be out of bounds.
Reply | Threaded
Open this post in threaded view
|

[Bug tree-optimization/69224] [4.9/5/6 Regression] -Warray-bounds false positive with -O3 and struct pointer parameter

msebor at gcc dot gnu.org
In reply to this post by msebor at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69224

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

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

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
Ok, so the issue is that discover_iteration_bound_by_body_walk adds one to the
upper bound derived for s->x[j] (5) because

      /* Exit terminates loop at given iteration, while non-exits produce
undefined
         effect on the next iteration.  */

and after unrolling to 6 iterations we do

Latch of last iteration was marked by __builtin_unreachable ().
Forced statement unreachable: _25 = s_7(D)->x[_24];
Forced statement unreachable: _22 = s_7(D)->x[j_2];

so figure both reads have undefined behavior in iteration 6.  But we never
try to force stmts unreachable in earlier iterations - s->x[j + 1] is
already undefined in iteration 5 which would end up removing that iteration
completely.  remove_exits_and_undefined_stmts only has links to stmts in
the original loop body (which will become the last iteration), so marking
stmts unreachable would need to be done during
gimple_duplicate_loop_to_header_edge itself to catch this case.

Or avoid the  + 1 in discover_iteration_bound_by_body_walk for unconditionally
executed "bounds".

So the following untested patch fixes the issue by unrolling one iteration
less.

Index: gcc/tree-ssa-loop-niter.c
===================================================================
--- gcc/tree-ssa-loop-niter.c   (revision 233661)
+++ gcc/tree-ssa-loop-niter.c   (working copy)
@@ -3171,9 +3171,12 @@ discover_iteration_bound_by_body_walk (s
     {
       widest_int bound = elt->bound;

-      /* Exit terminates loop at given iteration, while non-exits produce
undefined
-        effect on the next iteration.  */
-      if (!elt->is_exit)
+      /* Exit terminates loop at given iteration, while non-exits produce
+        undefined effect on the next iteration.  Unless they are always
+        executed.  */
+      if (!elt->is_exit
+         && ! dominated_by_p (CDI_DOMINATORS, loop->latch,
+                              gimple_bb (elt->stmt)))
        {
          bound += 1;
          /* If an overflow occurred, ignore the result.  */
@@ -3203,7 +3206,9 @@ discover_iteration_bound_by_body_walk (s
   for (elt = loop->bounds; elt; elt = elt->next)
     {
       widest_int bound = elt->bound;
-      if (!elt->is_exit)
+      if (!elt->is_exit
+         && ! dominated_by_p (CDI_DOMINATORS, loop->latch,
+                              gimple_bb (elt->stmt)))
        {
          bound += 1;
          /* If an overflow occurred, ignore the result.  */
Reply | Threaded
Open this post in threaded view
|

[Bug tree-optimization/69224] [4.9/5/6 Regression] -Warray-bounds false positive with -O3 and struct pointer parameter

msebor at gcc dot gnu.org
In reply to this post by msebor at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69224

--- Comment #5 from Richard Biener <rguenth at gcc dot gnu.org> ---
It miscompiles gcc.dg/tree-ssa/pr23391.c and more
(gcc.c-torture/execute/20000422-1.c is another example).

Looks like latch vs. stmt execution issues to me as the affected stmts
are only after the IV exit test and thus the dominance check doesn't work.
So we'd want a header post-dominated by stmt check instead.  But it isn't
that easy as at the time we unroll this PRs testcase the IV exit test
is in the loop header - it didn't have loop header copying applied.  Which
is probably the source of the whole issue.  Indeed doing loop header copying
before unrolling fixes the issue as well.
Reply | Threaded
Open this post in threaded view
|

[Bug tree-optimization/69224] [4.9/5/6 Regression] -Warray-bounds false positive with -O3 and struct pointer parameter

msebor at gcc dot gnu.org
In reply to this post by msebor at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69224

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P2
Reply | Threaded
Open this post in threaded view
|

[Bug tree-optimization/69224] [4.9/5/6/7 Regression] -Warray-bounds false positive with -O3 and struct pointer parameter

msebor at gcc dot gnu.org
In reply to this post by msebor at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69224

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.9.4                       |5.5

--- Comment #6 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 4.9 branch is being closed
Reply | Threaded
Open this post in threaded view
|

[Bug tree-optimization/69224] [5/6/7 Regression] -Warray-bounds false positive with -O3 and struct pointer parameter

msebor at gcc dot gnu.org
In reply to this post by msebor at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69224

Aldy Hernandez <aldyh at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|2016-01-11 00:00:00         |2017-2-1
                 CC|                            |aldyh at gcc dot gnu.org
      Known to work|                            |4.7.4
      Known to fail|                            |4.8.1, 4.8.2, 4.8.3, 4.8.4,
                   |                            |7.0

--- Comment #7 from Aldy Hernandez <aldyh at gcc dot gnu.org> ---
Using http://gcc.godbolt.org/, I see:

4.7.4 works

4.8.1 - 4.9.x has one warning

5.1 has three warnings

5.2 - 6.x has one warning

Compiling a recent trunk r245057 has one warning as well.
Reply | Threaded
Open this post in threaded view
|

[Bug tree-optimization/69224] [5/6/7/8 Regression] -Warray-bounds false positive with -O3 and struct pointer parameter

msebor at gcc dot gnu.org
In reply to this post by msebor at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69224

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|5.5                         |6.5

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 5 branch is being closed
Reply | Threaded
Open this post in threaded view
|

[Bug tree-optimization/69224] [6/7 Regression] -Warray-bounds false positive with -O3 and struct pointer parameter

msebor at gcc dot gnu.org
In reply to this post by msebor at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69224

Jeffrey A. Law <law at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |law at redhat dot com
            Summary|[6/7/8 Regression]          |[6/7 Regression]
                   |-Warray-bounds false        |-Warray-bounds false
                   |positive with -O3 and       |positive with -O3 and
                   |struct pointer parameter    |struct pointer parameter

--- Comment #9 from Jeffrey A. Law <law at redhat dot com> ---
Another fixed by Richi's 83202 changes.
Reply | Threaded
Open this post in threaded view
|

[Bug tree-optimization/69224] [6/7 Regression] -Warray-bounds false positive with -O3 and struct pointer parameter

msebor at gcc dot gnu.org
In reply to this post by msebor at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69224

--- Comment #10 from Jeffrey A. Law <law at gcc dot gnu.org> ---
Author: law
Date: Wed Dec  6 23:50:58 2017
New Revision: 255457

URL: https://gcc.gnu.org/viewcvs?rev=255457&root=gcc&view=rev
Log:
        PR tree-optimization/69224
        PR tree-optimization/80907
        PR tree-optimization/82286
        * gcc.dg/pr69224.c: New test.
        * gcc.dg/pr80907.c: New test.
        * gcc.dg/pr82286.c: New test.

Added:
    trunk/gcc/testsuite/gcc.dg/pr69224.c
    trunk/gcc/testsuite/gcc.dg/pr80907.c
    trunk/gcc/testsuite/gcc.dg/pr82286.c
Modified:
    trunk/gcc/testsuite/ChangeLog
Reply | Threaded
Open this post in threaded view
|

[Bug tree-optimization/69224] [6/7 Regression] -Warray-bounds false positive with -O3 and struct pointer parameter

msebor at gcc dot gnu.org
In reply to this post by msebor at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69224

--- Comment #11 from nightstrike <nightstrike at gmail dot com> ---
Will this be back ported to 6 and 7?
Reply | Threaded
Open this post in threaded view
|

[Bug tree-optimization/69224] [6/7 Regression] -Warray-bounds false positive with -O3 and struct pointer parameter

msebor at gcc dot gnu.org
In reply to this post by msebor at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69224

--- Comment #12 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to nightstrike from comment #11)
> Will this be back ported to 6 and 7?

This PR itself didn't see any patch, the patch that fixed it was probably
(didn't double-check) r255267 which itself isn't a regression fix but
an optimization enhancement.  So, no - "unlikely".
Reply | Threaded
Open this post in threaded view
|

[Bug tree-optimization/69224] [6/7 Regression] -Warray-bounds false positive with -O3 and struct pointer parameter

msebor at gcc dot gnu.org
In reply to this post by msebor at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69224

--- Comment #13 from Jeffrey A. Law <law at redhat dot com> ---
Agreed.  I don't see a lot of value in backporting this fix to the release
branches.  One could argue that decision means this should move to CLOSED as
it's been fixed for gcc-8 and the trunk.
Reply | Threaded
Open this post in threaded view
|

[Bug tree-optimization/69224] [6/7 Regression] -Warray-bounds false positive with -O3 and struct pointer parameter

msebor at gcc dot gnu.org
In reply to this post by msebor at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69224

--- Comment #14 from rguenther at suse dot de <rguenther at suse dot de> ---
On Mon, 23 Jul 2018, law at redhat dot com wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69224
>
> --- Comment #13 from Jeffrey A. Law <law at redhat dot com> ---
> Agreed.  I don't see a lot of value in backporting this fix to the release
> branches.  One could argue that decision means this should move to CLOSED as
> it's been fixed for gcc-8 and the trunk.

We're keeping regression bugs open until branch close for tracking
purposes and also to easily find dups w/o searching closed bugs.
Reply | Threaded
Open this post in threaded view
|

[Bug tree-optimization/69224] [6/7 Regression] -Warray-bounds false positive with -O3 and struct pointer parameter

msebor at gcc dot gnu.org
In reply to this post by msebor at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69224

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|6.5                         |7.4

--- Comment #15 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 6 branch is being closed
Reply | Threaded
Open this post in threaded view
|

[Bug tree-optimization/69224] [7 Regression] -Warray-bounds false positive with -O3 and struct pointer parameter

msebor at gcc dot gnu.org
In reply to this post by msebor at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69224

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|7.4                         |7.5
Reply | Threaded
Open this post in threaded view
|

[Bug tree-optimization/69224] [7 Regression] -Warray-bounds false positive with -O3 and struct pointer parameter

msebor at gcc dot gnu.org
In reply to this post by msebor at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69224
Bug 69224 depends on bug 83202, which changed state.

Bug 83202 Summary: Try joining operations on consecutive array elements during tree vectorization
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83202

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED
Reply | Threaded
Open this post in threaded view
|

[Bug tree-optimization/69224] [7 Regression] -Warray-bounds false positive with -O3 and struct pointer parameter

msebor at gcc dot gnu.org
In reply to this post by msebor at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69224

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
      Known to work|                            |8.1.0
         Resolution|---                         |FIXED
   Target Milestone|7.5                         |8.0

--- Comment #16 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed in GCC8.