[Bug middle-end/104986] New: [12 Regression] bogus writing 1 byte into a region of size 0 with -fwrapv and -O2 -fpeel-loops

[Bug middle-end/104986] New: [12 Regression] bogus writing 1 byte into a region of size 0 with -fwrapv and -O2 -fpeel-loops

[andres at anarazel dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104986

            Bug ID: 104986
           Summary: [12 Regression] bogus writing 1 byte into a region of
                    size 0 with -fwrapv and -O2 -fpeel-loops
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: middle-end
          Assignee: unassigned at gcc dot gnu.org
          Reporter: andres at anarazel dot de
  Target Milestone: ---

Hi,

recently started seeing bogus warnings using gcc 12 to build postgres. I
reduced the problem using cvise with some manual cleanups / improvements
afterwards - certainly doesn't quite make sense anymore, but afaics shows a
problem.

Originally I hit this with -O3, but found that -O2 -fpeel-loops is sufficient
to trigger the problem.

repro: https://godbolt.org/z/ejK9h6von

code:
struct inet_struct {
  char family;
  char ipaddr[16];
};

void
inetnot(struct inet_struct *dst1, struct inet_struct *dst2, struct inet_struct
*src) {
  int nb = src->family ? 4 : 6;
  char *psrc = src->ipaddr;
  char *pdst = dst1 ? dst1->ipaddr : dst2->ipaddr;
  while (nb-- > 0)
    pdst[nb] = psrc[nb];
}


gcc-12 -fwrapv -O2 -fpeel-loops -c network2.i

network2.i: In function ‘inetnot’:
network2.i:12:14: warning: writing 1 byte into a region of size 0
[-Wstringop-overflow=]
   12 |     pdst[nb] = psrc[nb];
      |     ~~~~~~~~~^~~~~~~~~~
network2.i:3:8: note: at offset -1 into destination object ‘ipaddr’ of size 16
    3 |   char ipaddr[16];
      |        ^~~~~~
network2.i:3:8: note: at offset -1 into destination object ‘ipaddr’ of size 16


which afaics is bogus, because the loop terminates before reaching offset -1,
the condition is > 0, not >= 0. So the post decrement can't lead to -1 being
reached.

version: gcc version 12.0.1 20220314 (experimental) [master
r12-7638-g823b3b79cd2] (Debian 12-20220313-1) 

Regards,

Andres

[Bug middle-end/104986] [12 Regression] bogus writing 1 byte into a region of size 0 with -fwrapv and -O2 -fpeel-loops

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104986

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |diagnostic
   Target Milestone|---                         |12.0

[Bug middle-end/104986] [12 Regression] bogus writing 1 byte into a region of size 0 with -fwrapv and -O2 -fpeel-loops since r12-4698-gf6d012338bf87f42

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104986

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
            Summary|[12 Regression] bogus       |[12 Regression] bogus
                   |writing 1 byte into a       |writing 1 byte into a
                   |region of size 0 with       |region of size 0 with
                   |-fwrapv and -O2             |-fwrapv and -O2
                   |-fpeel-loops                |-fpeel-loops since
                   |                            |r12-4698-gf6d012338bf87f42
                 CC|                            |aldyh at gcc dot gnu.org,
                   |                            |marxin at gcc dot gnu.org
   Last reconfirmed|                            |2022-03-21
     Ever confirmed|0                           |1

--- Comment #1 from Martin Liška <marxin at gcc dot gnu.org> ---
Started with r12-4698-gf6d012338bf87f42.

[Bug middle-end/104986] [12 Regression] bogus writing 1 byte into a region of size 0 with -fwrapv and -O2 -fpeel-loops since r12-4698-gf6d012338bf87f42

[aldyh at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104986

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |amacleod at redhat dot com,
                   |                            |siddhesh at gcc dot gnu.org

--- Comment #2 from Aldy Hernandez <aldyh at gcc dot gnu.org> ---
[Siddhesh, I don't know if this is something you're looking at.  If not, feel
free to unsubscribe.]

This may have first been visible with r12-4698-gf6d012338bf87f42, but it isn't
threader related as it can be reproduced with -fno-thread-jumps.

I can actually reproduce it without VRP[12] (but not without evrp).

+ ./cc1 a.c -fdump-tree-all-details -quiet -I/tmp -O2 -fpeel-loops -fwrapv
-fno-thread-jumps -fdisable-tr
ee-vrp2 -fdisable-tree-vrp1
cc1: note: disable pass tree-vrp2 for functions in the range of [0, 4294967295]
cc1: note: disable pass tree-vrp1 for functions in the range of [0, 4294967295]
a.c: In function ‘inetnot’:
a.c:12:14: warning: writing 1 byte into a region of size 0
[-Wstringop-overflow=]
etc
etc

Disabling evrp causes the problem to go away, but all these middle end warnings
are very sensitive to better ranges, so who knows.

For that matter, the only transformation evrp is doing is correct:

@@ -74,7 +90,7 @@
   <bb 9> :
   # nb_8 = PHI <iftmp.0_9(7), nb_23(8)>
   nb_23 = nb_8 + -1;
-  if (nb_8 > 0)
+  if (nb_8 != 0)
     goto <bb 8>; [INV]
   else
     goto <bb 10>; [INV]

...which is merely changing the conditional in the decreasing loop from > 0
into != 0.

It looks like a problem elsewhere.

[Bug middle-end/104986] [12 Regression] bogus writing 1 byte into a region of size 0 with -fwrapv and -O2 -fpeel-loops since r12-4698-gf6d012338bf87f42

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104986

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |missed-optimization

--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
This is peeling leaving us with unreachable code we warn on and somehow
while figuring prephitmp_30 + -6 is -1 we don't figure nb_58 is zero on
the path to bb9.

I think I've seen this backward-forward dependency issue with ranger before
in another missed optimization PR.

[Bug middle-end/104986] [12 Regression] bogus writing 1 byte into a region of size 0 with -fwrapv and -O2 -fpeel-loops since r12-4698-gf6d012338bf87f42

[aldyh at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104986

--- Comment #4 from Aldy Hernandez <aldyh at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #3)
> This is peeling leaving us with unreachable code we warn on and somehow
> while figuring prephitmp_30 + -6 is -1 we don't figure nb_58 is zero on
> the path to bb9.
> 
> I think I've seen this backward-forward dependency issue with ranger before
> in another missed optimization PR.

This is not ranger, but the legacy conditional folding in vr-values.  It's the
test_for_singularity() code that turns x<=0 where x is [0,6] into x!=0.

[Bug middle-end/104986] [12 Regression] bogus writing 1 byte into a region of size 0 with -fwrapv and -O2 -fpeel-loops since r12-4698-gf6d012338bf87f42

[amacleod at redhat dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104986

--- Comment #5 from Andrew Macleod <amacleod at redhat dot com> ---
(In reply to Richard Biener from comment #3)
> This is peeling leaving us with unreachable code we warn on and somehow
> while figuring prephitmp_30 + -6 is -1 we don't figure nb_58 is zero on
> the path to bb9.
> 
> I think I've seen this backward-forward dependency issue with ranger before
> in another missed optimization PR.

(In reply to Richard Biener from comment #3)
> This is peeling leaving us with unreachable code we warn on and somehow
> while figuring prephitmp_30 + -6 is -1 we don't figure nb_58 is zero on
> the path to bb9.
> 
> I think I've seen this backward-forward dependency issue with ranger before
> in another missed optimization PR.

DOM3 is the pass which converted prephitmp_30 + -6 to -1, and left nb_58 as is.

Ranger appears to see it just fine. Looking at VRP2, 

Folding statement: nb_68 = prephitmp_30 + -5;
Queued stmt for removal.  Folds to: 0
<...>
Folding statement: if (nb_68 != 0)
gimple_simplified to if (0 != 0)
Folded into: if (0 != 0)

strlen runs just before vrp2, and if I put a VRP pass immediately before
strlen1, then the warning goes away.

Looking at the strlen1 output, it only ever asks ranger about 6 names:

334      range_of_expr(_36) at stmt _52 = iftmp.1_17 + _36;
         TRUE : (334) range_of_expr (_36) sizetype [3, 5]
maybe_invalidate called for *_52 = _60;
maybe_invalidate returns 1
342      range_of_expr(_69) at stmt _71 = iftmp.1_7 + _69;
         TRUE : (342) range_of_expr (_69) sizetype [3, 5]
maybe_invalidate called for *_71 = _72;
maybe_invalidate returns 1
348      range_of_expr(_6) at stmt _31 = iftmp.1_76 + _6;
         TRUE : (348) range_of_expr (_6) sizetype [2, 4]
maybe_invalidate called for *_31 = _32;
maybe_invalidate returns 1
367      range_of_expr(_37) at stmt _39 = iftmp.1_76 + _37;
         TRUE : (367) range_of_expr (_37) sizetype [1, 3]
maybe_invalidate called for *_39 = _40;
  statement may clobber object _31 0 bytes in size

maybe_invalidate returns 1
375      range_of_expr(_45) at stmt _47 = iftmp.1_76 + _45;
         TRUE : (375) range_of_expr (_45) sizetype [0, 2]
maybe_invalidate called for *_47 = _48;
  statement may clobber object _39 0 bytes in size
maybe_invalidate returns 1
383      range_of_expr(_53) at stmt _55 = iftmp.1_76 + _53;
         TRUE : (383) range_of_expr (_53) sizetype [1, 1]

Is there a reason it runs before vrp2?

[Bug middle-end/104986] [12 Regression] bogus writing 1 byte into a region of size 0 with -fwrapv and -O2 -fpeel-loops since r12-4698-gf6d012338bf87f42

[aldyh at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104986

--- Comment #6 from Aldy Hernandez <aldyh at gcc dot gnu.org> ---
(In reply to Aldy Hernandez from comment #4)
> (In reply to Richard Biener from comment #3)
> > This is peeling leaving us with unreachable code we warn on and somehow
> > while figuring prephitmp_30 + -6 is -1 we don't figure nb_58 is zero on
> > the path to bb9.
> > 
> > I think I've seen this backward-forward dependency issue with ranger before
> > in another missed optimization PR.
> 
> This is not ranger, but the legacy conditional folding in vr-values.  It's
> the test_for_singularity() code that turns x<=0 where x is [0,6] into x!=0.

Ooops, sorry I totally missed Richi's comment on prephitmp.  I see Andrew
answered though :).

[Bug middle-end/104986] [12 Regression] bogus writing 1 byte into a region of size 0 with -fwrapv and -O2 -fpeel-loops since r12-4698-gf6d012338bf87f42

[amacleod at redhat dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104986

--- Comment #7 from Andrew Macleod <amacleod at redhat dot com> ---
and I forgot to show to dom3 output which did the transformation:

j.c.195t.dom3:Match-and-simplified (sizetype) nb_66 to 18446744073709551615
j.c.195t.dom3:Optimizing statement _2 = (sizetype) nb_66;
j.c.195t.dom3:  Replaced 'nb_66' with constant '-1'
j.c.195t.dom3:  nb_66 = -1;

but for whatever reason it could not reduce nb_58 to anything better than [-1,
0]

nb_58 = prephitmp_30 + -5;
Intersecting
  int [-1, 0]
and
  int [-1, 4]
to
  int [-1, 0]

[Bug middle-end/104986] [12 Regression] bogus writing 1 byte into a region of size 0 with -fwrapv and -O2 -fpeel-loops since r12-4698-gf6d012338bf87f42

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104986

--- Comment #8 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Andrew Macleod from comment #5)
> Looking at the strlen1 output, it only ever asks ranger about 6 names:
> 
> 334      range_of_expr(_36) at stmt _52 = iftmp.1_17 + _36;
>          TRUE : (334) range_of_expr (_36) sizetype [3, 5]
> maybe_invalidate called for *_52 = _60;
> maybe_invalidate returns 1
> 342      range_of_expr(_69) at stmt _71 = iftmp.1_7 + _69;
>          TRUE : (342) range_of_expr (_69) sizetype [3, 5]
> maybe_invalidate called for *_71 = _72;
> maybe_invalidate returns 1
> 348      range_of_expr(_6) at stmt _31 = iftmp.1_76 + _6;
>          TRUE : (348) range_of_expr (_6) sizetype [2, 4]
> maybe_invalidate called for *_31 = _32;
> maybe_invalidate returns 1
> 367      range_of_expr(_37) at stmt _39 = iftmp.1_76 + _37;
>          TRUE : (367) range_of_expr (_37) sizetype [1, 3]
> maybe_invalidate called for *_39 = _40;
>   statement may clobber object _31 0 bytes in size
>   
> maybe_invalidate returns 1
> 375      range_of_expr(_45) at stmt _47 = iftmp.1_76 + _45;
>          TRUE : (375) range_of_expr (_45) sizetype [0, 2]
> maybe_invalidate called for *_47 = _48;
>   statement may clobber object _39 0 bytes in size
> maybe_invalidate returns 1
> 383      range_of_expr(_53) at stmt _55 = iftmp.1_76 + _53;
>          TRUE : (383) range_of_expr (_53) sizetype [1, 1]

So maybe strlen can invoke ranger on GIMPLE_CONDs to mark unreachable
code on the fly?  IIRC domwalk has some ability to skip not executable
edges so if the walk makes sure to properly initialize that for
outgoing edges it might work in that it would refrain from
optimizing/diagnosing
in unreachable regions.

> Is there a reason it runs before vrp2?

Likely historical or the idea that VRP can do something with its results,
like propagate constants (but there's now a CCP pass after it).

[Bug middle-end/104986] [12 Regression] bogus writing 1 byte into a region of size 0 with -fwrapv and -O2 -fpeel-loops since r12-4698-gf6d012338bf87f42

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104986

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
The strlen pass itself for its original purpose doesn't use the ranger at all,
it is just the warning code that uses the strlen infrastructure and is invoked
during the pass that does.

[Bug middle-end/104986] [12/13 Regression] bogus writing 1 byte into a region of size 0 with -fwrapv and -O2 -fpeel-loops since r12-4698-gf6d012338bf87f42

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104986

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|12.0                        |12.2

--- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 12.1 is being released, retargeting bugs to GCC 12.2.

[Bug middle-end/104986] [12/13 Regression] bogus writing 1 byte into a region of size 0 with -fwrapv and -O2 -fpeel-loops since r12-4698-gf6d012338bf87f42

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104986

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P2

[Bug middle-end/104986] [12/13/14 Regression] bogus writing 1 byte into a region of size 0 with -fwrapv and -O2 -fpeel-loops since r12-4698-gf6d012338bf87f42

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104986

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|12.3                        |12.4

--- Comment #12 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 12.3 is being released, retargeting bugs to GCC 12.4.

[Bug middle-end/104986] [12/13/14 Regression] bogus writing 1 byte into a region of size 0 with -fwrapv and -O2 -fpeel-loops since r12-4698-gf6d012338bf87f42

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104986

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |needs-bisection
      Known to work|                            |13.1.0

--- Comment #13 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Looks like SCEV can figure out the bounds more correctly in GCC 13+ which fixes
the warning and the missed optimization.

GCC 12.3.0:
```
Estimating # of iterations of loop 1
Matching expression match.pd:2119, generic-match.cc:693
Matching expression match.pd:2122, generic-match.cc:753
Matching expression match.pd:2129, generic-match.cc:776
Analyzing # of iterations of loop 1
  exit condition [prephitmp_30, + , -1] != 0
  bounds on difference of bases: -5 ... 1
Applying pattern match.pd:182, generic-match.cc:30997
Applying pattern match.pd:5698, generic-match.cc:11425
Matching expression match.pd:2119, generic-match.cc:693
Matching expression match.pd:2122, generic-match.cc:753
Matching expression match.pd:2129, generic-match.cc:776
Applying pattern match.pd:354, generic-match.cc:5780
  result:
    # of iterations (unsigned int) prephitmp_30, bounded by 4294967295
Statement (exit)if (nb_24 != 0)
 is executed at most (unsigned int) prephitmp_30 (bounded by 4294967295) + 1
times in loop 1.
```

While GCC 13+:
```
Estimating # of iterations of loop 1
Matching expression match.pd:2404, generic-match.cc:727
Matching expression match.pd:2407, generic-match.cc:787
Matching expression match.pd:2414, generic-match.cc:810
Analyzing # of iterations of loop 1
  exit condition [prephitmp_30, + , -1] != 0
  bounds on difference of bases: -5 ... 0
Applying pattern match.pd:182, generic-match.cc:33250
Applying pattern match.pd:6155, generic-match.cc:21253
Matching expression match.pd:2404, generic-match.cc:727
Matching expression match.pd:2407, generic-match.cc:787
Matching expression match.pd:2414, generic-match.cc:810
Applying pattern match.pd:365, generic-match.cc:21998
  result:
    # of iterations (unsigned int) prephitmp_30, bounded by 5
Statement (exit)if (nb_24 != 0)
 is executed at most (unsigned int) prephitmp_30 (bounded by 5) + 1 times in
loop 1.
```