| Message ID | alpine.LSU.2.20.1708170914020.14191@zhemvz.fhfr.qr |
|---|---|
| State | New |
| Headers | show |
On Aug 17 2017, Richard Biener <rguenther@suse.de> wrote: > I was notifed I broke proper handling of undefined overflow in > multiplicative ops handling. The following resurrects previous > behavior (and adds a testcase). > > Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk. This breaks gfortran.dg/alloc_comp_auto_array_2.f90 on aarch64 with -mabi=ilp32 (only for -O3): FAIL: gfortran.dg/alloc_comp_auto_array_2.f90 -O3 -g (test for excess errors) Excess errors: /opt/gcc/gcc-20170818/gcc/testsuite/gfortran.dg/alloc_comp_auto_array_2.f90:33:0: Warning: '__builtin_memcpy' specified size between 2147483648 and 4294967295 exceeds maximum object size 2147483647 [-Wstringop-overflow=] Andreas.
On Sat, 19 Aug 2017, Andreas Schwab wrote: > On Aug 17 2017, Richard Biener <rguenther@suse.de> wrote: > > > I was notifed I broke proper handling of undefined overflow in > > multiplicative ops handling. The following resurrects previous > > behavior (and adds a testcase). > > > > Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk. > > This breaks gfortran.dg/alloc_comp_auto_array_2.f90 on aarch64 with > -mabi=ilp32 (only for -O3): > > FAIL: gfortran.dg/alloc_comp_auto_array_2.f90 -O3 -g (test for excess errors) > Excess errors: > /opt/gcc/gcc-20170818/gcc/testsuite/gfortran.dg/alloc_comp_auto_array_2.f90:33:0: Warning: '__builtin_memcpy' specified size between 2147483648 and 4294967295 exceeds maximum object size 2147483647 [-Wstringop-overflow=] I believe this is an issue that went latent when I broke VRP earlier. I have opened PR81908, will amend with some initial analysis. Richard.
On 08/21/2017 01:51 AM, Richard Biener wrote: > On Sat, 19 Aug 2017, Andreas Schwab wrote: > >> On Aug 17 2017, Richard Biener <rguenther@suse.de> wrote: >> >>> I was notifed I broke proper handling of undefined overflow in >>> multiplicative ops handling. The following resurrects previous >>> behavior (and adds a testcase). >>> >>> Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk. >> >> This breaks gfortran.dg/alloc_comp_auto_array_2.f90 on aarch64 with >> -mabi=ilp32 (only for -O3): >> >> FAIL: gfortran.dg/alloc_comp_auto_array_2.f90 -O3 -g (test for excess errors) >> Excess errors: >> /opt/gcc/gcc-20170818/gcc/testsuite/gfortran.dg/alloc_comp_auto_array_2.f90:33:0: Warning: '__builtin_memcpy' specified size between 2147483648 and 4294967295 exceeds maximum object size 2147483647 [-Wstringop-overflow=] > > I believe this is an issue that went latent when I broke VRP earlier. > > I have opened PR81908, will amend with some initial analysis. FWIW, the core of the problem is that the warning tends to either expose limitations in optimizations that were not written to make use of range information, or indicate additional optimization opportunities due to range information. In this case, since the only valid value in the range the memcpy argument is in (i.e., ~[0, INT_MAX]) is zero, the call could be eliminated. But this isn't noticed by any pass until the expander checks the call for validity. It seems to me that this could be handled by enhancing gimple-fold in two ways: 1) fold arguments whose range contains only one valid value into constants, and 2) transform calls with one or more arguments entirely in invalid ranges into __builtin_unreachable. I have been thinking prototyping this solution for a while but haven't gotten around to it yet so I can't say what problems it might run into. Martin
On Mon, 21 Aug 2017, Martin Sebor wrote: > On 08/21/2017 01:51 AM, Richard Biener wrote: > > On Sat, 19 Aug 2017, Andreas Schwab wrote: > > > > > On Aug 17 2017, Richard Biener <rguenther@suse.de> wrote: > > > > > > > I was notifed I broke proper handling of undefined overflow in > > > > multiplicative ops handling. The following resurrects previous > > > > behavior (and adds a testcase). > > > > > > > > Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk. > > > > > > This breaks gfortran.dg/alloc_comp_auto_array_2.f90 on aarch64 with > > > -mabi=ilp32 (only for -O3): > > > > > > FAIL: gfortran.dg/alloc_comp_auto_array_2.f90 -O3 -g (test for excess > > > errors) > > > Excess errors: > > > /opt/gcc/gcc-20170818/gcc/testsuite/gfortran.dg/alloc_comp_auto_array_2.f90:33:0: > > > Warning: '__builtin_memcpy' specified size between 2147483648 and > > > 4294967295 exceeds maximum object size 2147483647 [-Wstringop-overflow=] > > > > I believe this is an issue that went latent when I broke VRP earlier. > > > > I have opened PR81908, will amend with some initial analysis. > > FWIW, the core of the problem is that the warning tends to either > expose limitations in optimizations that were not written to make > use of range information, or indicate additional optimization > opportunities due to range information. In this case, since > the only valid value in the range the memcpy argument is in (i.e., > ~[0, INT_MAX]) is zero, the call could be eliminated. But this > isn't noticed by any pass until the expander checks the call for > validity. > > It seems to me that this could be handled by enhancing gimple-fold > in two ways: 1) fold arguments whose range contains only one valid > value into constants, and 2) transform calls with one or more > arguments entirely in invalid ranges into __builtin_unreachable. > > I have been thinking prototyping this solution for a while but > haven't gotten around to it yet so I can't say what problems it > might run into. Well, there will always be missed optimizations so the correct fix for this is to not warn about memcpy with size == 0. Sure, folding can be improved but the get_size_range code is broken and has to be fixed. Richard.
Index: gcc/tree-vrp.c =================================================================== --- gcc/tree-vrp.c (revision 251084) +++ gcc/tree-vrp.c (working copy) @@ -1614,6 +1614,8 @@ vrp_int_const_binop (enum tree_code code signop sign = TYPE_SIGN (TREE_TYPE (val1)); wide_int res; + *overflow_p = false; + switch (code) { case RSHIFT_EXPR: @@ -1685,8 +1687,6 @@ vrp_int_const_binop (enum tree_code code gcc_unreachable (); } - *overflow_p = overflow; - if (overflow && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (val1))) { @@ -1730,6 +1730,8 @@ vrp_int_const_binop (enum tree_code code TYPE_SIGN (TREE_TYPE (val1))); } + *overflow_p = overflow; + return res; } Index: gcc/testsuite/gcc.dg/tree-ssa/vrp117.c =================================================================== --- gcc/testsuite/gcc.dg/tree-ssa/vrp117.c (nonexistent) +++ gcc/testsuite/gcc.dg/tree-ssa/vrp117.c (working copy) @@ -0,0 +1,16 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-tree-evrp" } */ + +void link_error (void); + +void foo (int i) +{ + if (i > __INT_MAX__ - 10) + { + int j = i * 10; + if (j < i) + link_error (); + } +} + +/* { dg-final { scan-tree-dump-not "link_error" "evrp" } } */