Re: [PATCH] Remove -fstrict-overflow, default to undefined signed integer and pointer overflow
On 27 April 2017 at 17:32, Jeff Law wrote: > On 04/26/2017 05:31 AM, Richard Biener wrote: >> >> >> The following removes the third state we had apart from signed integer >> overflow wrapping and being undefined. It makes signed integer overflow >> undefined, consistently at all optimization levels. -fno-strict-overflow >> stays as a backward compatible way to avoid optimizations that rely on >> signed integer overflow being undefined by making it wrapping >> (this is also the reason of using !flag_wrapv in >> POINTER_TYPE_OVERFLOW_UNDEFINED rather than a new option, for now). >> >> Surprisingly there's no UBSAN integer overflow testsuite fallout, >> foldings that happen before instrumentation (which is done after >> into-SSA) and rely on signed integer overflow being undefined will >> cause false negatives. If that turns out to be a problem the >> flag_strict_overflow flag can be re-introduced (not that this would >> be my preference) and it can be unset after UBSAN instrumentation >> is finished. >> >> The main motivation for aliasing -fstrict-overflow to -f[no-]wrapv >> is that with -fno-strict-overflow (and thus -O1 at the moment) you get >> the worst of both worlds, you can't optimize based on the undefinedness >> but you also cannot rely on wrapping behavior (to know that >> re-association will not introduce undefined behavior). Using -fwrapv >> for -fno-strict-overflow makes it clear what the semantics are. >> >> Bootstrapped and tested on x86_64-unknown-linux-gnu. >> >> I opened PR80525 for the appearant mishandling of (a + 1) && (a + 1) >> with -Wlogical-op when overflow is undefined. >> >> If there are no further comments I plan to install this after 7.1 >> is released. I consider the Ada FE change obvious. >> >> The next step is to get rid of all that ugly -Wstrict-overflow code >> in VRP. strict-overflow warnings from folding were already >> detoriating with moving stuff to match.pd where it isn't easy to >> preserve those. Ripping those out can be done later, it's not >> blocking other stuff, and eventually somebody picks up -Wstrict-overflow >> to warn for some cases from the FEs. >> >> changes.html/porting_to.html will need to have instructions how to >> use ubsan to get at the real problems in code. > > This all sounds good to me. > > jeff Hi, This patch (r247495) causes regressions in fortran on aarch64/arm: - PASS now FAIL [PASS => FAIL]: Executed from: gfortran.dg/dg.exp gfortran.dg/coarray_lock_7.f90 -O scan-tree-dump-times original "_gfortran_caf_lock \\(caf_token.., \\(3 - \\(integer\\(kind=4\\)\\) parm...dim\\[0\\].lbound\\) \\+ \\(integer\\(kind=4\\)\\) MAX_EXPR <\\(parm...dim\\[0\\].ubound - parm...dim\\[0\\].lbound\\) \\+ 1, 0> \\* \\(3 - \\(integer\\(kind=4\\)\\) parm...dim\\[1\\].lbound\\), 0, 0B, &ii, 0B, 0\\);|_gfortran_caf_lock \\(caf_token.1, \\(3 - parm...dim\\[0\\].lbound\\) \\+ MAX_EXPR <\\(parm...dim\\[0\\].ubound - parm...dim\\[0\\].lbound\\) \\+ 1, 0> \\* \\(3 - parm...dim\\[1\\].lbound\\), 0, 0B, &ii, 0B, 0\\);" 1 gfortran.dg/coarray_lock_7.f90 -O scan-tree-dump-times original "_gfortran_caf_unlock \\(caf_token.., \\(2 - \\(integer\\(kind=4\\)\\) parm...dim\\[0\\].lbound\\) \\+ \\(integer\\(kind=4\\)\\) MAX_EXPR <\\(parm...dim\\[0\\].ubound - parm...dim\\[0\\].lbound\\) \\+ 1, 0> \\* \\(3 - \\(integer\\(kind=4\\)\\) parm...dim\\[1\\].lbound\\), 0, &ii, 0B, 0\\);|_gfortran_caf_unlock \\(caf_token.., \\(2 - parm...dim\\[0\\].lbound\\) \\+ MAX_EXPR <\\(parm...dim\\[0\\].ubound - parm...dim\\[0\\].lbound\\) \\+ 1, 0> \\* \\(3 - parm...dim\\[1\\].lbound\\), 0, &ii, 0B, 0\\);" 1 Thanks, Christophe
Re: [PATCH] Remove -fstrict-overflow, default to undefined signed integer and pointer overflow
On 04/26/2017 05:31 AM, Richard Biener wrote: The following removes the third state we had apart from signed integer overflow wrapping and being undefined. It makes signed integer overflow undefined, consistently at all optimization levels. -fno-strict-overflow stays as a backward compatible way to avoid optimizations that rely on signed integer overflow being undefined by making it wrapping (this is also the reason of using !flag_wrapv in POINTER_TYPE_OVERFLOW_UNDEFINED rather than a new option, for now). Surprisingly there's no UBSAN integer overflow testsuite fallout, foldings that happen before instrumentation (which is done after into-SSA) and rely on signed integer overflow being undefined will cause false negatives. If that turns out to be a problem the flag_strict_overflow flag can be re-introduced (not that this would be my preference) and it can be unset after UBSAN instrumentation is finished. The main motivation for aliasing -fstrict-overflow to -f[no-]wrapv is that with -fno-strict-overflow (and thus -O1 at the moment) you get the worst of both worlds, you can't optimize based on the undefinedness but you also cannot rely on wrapping behavior (to know that re-association will not introduce undefined behavior). Using -fwrapv for -fno-strict-overflow makes it clear what the semantics are. Bootstrapped and tested on x86_64-unknown-linux-gnu. I opened PR80525 for the appearant mishandling of (a + 1) && (a + 1) with -Wlogical-op when overflow is undefined. If there are no further comments I plan to install this after 7.1 is released. I consider the Ada FE change obvious. The next step is to get rid of all that ugly -Wstrict-overflow code in VRP. strict-overflow warnings from folding were already detoriating with moving stuff to match.pd where it isn't easy to preserve those. Ripping those out can be done later, it's not blocking other stuff, and eventually somebody picks up -Wstrict-overflow to warn for some cases from the FEs. changes.html/porting_to.html will need to have instructions how to use ubsan to get at the real problems in code. This all sounds good to me. jeff
Re: [PATCH] Remove -fstrict-overflow, default to undefined signed integer and pointer overflow
> If there are no further comments I plan to install this after 7.1 > is released. I consider the Ada FE change obvious. For the record: yes, it is, since the whole patch is a no-op for Ada. -- Eric Botcazou
[PATCH] Remove -fstrict-overflow, default to undefined signed integer and pointer overflow
The following removes the third state we had apart from signed integer overflow wrapping and being undefined. It makes signed integer overflow undefined, consistently at all optimization levels. -fno-strict-overflow stays as a backward compatible way to avoid optimizations that rely on signed integer overflow being undefined by making it wrapping (this is also the reason of using !flag_wrapv in POINTER_TYPE_OVERFLOW_UNDEFINED rather than a new option, for now). Surprisingly there's no UBSAN integer overflow testsuite fallout, foldings that happen before instrumentation (which is done after into-SSA) and rely on signed integer overflow being undefined will cause false negatives. If that turns out to be a problem the flag_strict_overflow flag can be re-introduced (not that this would be my preference) and it can be unset after UBSAN instrumentation is finished. The main motivation for aliasing -fstrict-overflow to -f[no-]wrapv is that with -fno-strict-overflow (and thus -O1 at the moment) you get the worst of both worlds, you can't optimize based on the undefinedness but you also cannot rely on wrapping behavior (to know that re-association will not introduce undefined behavior). Using -fwrapv for -fno-strict-overflow makes it clear what the semantics are. Bootstrapped and tested on x86_64-unknown-linux-gnu. I opened PR80525 for the appearant mishandling of (a + 1) && (a + 1) with -Wlogical-op when overflow is undefined. If there are no further comments I plan to install this after 7.1 is released. I consider the Ada FE change obvious. The next step is to get rid of all that ugly -Wstrict-overflow code in VRP. strict-overflow warnings from folding were already detoriating with moving stuff to match.pd where it isn't easy to preserve those. Ripping those out can be done later, it's not blocking other stuff, and eventually somebody picks up -Wstrict-overflow to warn for some cases from the FEs. changes.html/porting_to.html will need to have instructions how to use ubsan to get at the real problems in code. Thanks, Richard. 2017-04-26 Richard Biener * common.opt (fstrict-overflow): Alias negative to fwrapv. * doc/invoke.texi (fstrict-overflow): Remove all traces of -fstrict-overflow documentation. * tree.h (TYPE_OVERFLOW_UNDEFINED): Do not test flag_strict_overflow. (POINTER_TYPE_OVERFLOW_UNDEFINED): Test !flag_wrapv instead of flag_strict_overflow. * ipa-inline.c (can_inline_edge_p): Do not test flag_strict_overflow. * lto-opts.c (lto_write_options): Do not stream it. * lto-wrapper.c (merge_and_complain): Do not handle it. * opts.c (default_options_table): Do not set -fstrict-overflow. (finish_options): Likewise do not clear it when sanitizing. * simplify-rtx.c (simplify_const_relational_operation): Do not test flag_strict_overflow. ada/ * gcc-interface/misc.c (gnat_post_options): Do not set -fstrict-overflow. * c-c++-common/Wlogical-op-1.c: Add -fwrapv to restore previous behavior. * gcc.target/i386/pr46253.c: Make i unsigned to avoid warning. Index: gcc/ada/gcc-interface/misc.c === --- gcc/ada/gcc-interface/misc.c(revision 247273) +++ gcc/ada/gcc-interface/misc.c(working copy) @@ -266,10 +266,6 @@ gnat_post_options (const char **pfilenam if (!global_options_set.x_flag_diagnostics_show_caret) global_dc->show_caret = false; - /* Set strict overflow by default for Ada. */ - if (!global_options_set.x_flag_strict_overflow) -global_options.x_flag_strict_overflow = true; - /* Warn only if STABS is not the default: we don't want to emit a warning if the user did not use a -gstabs option. */ if (PREFERRED_DEBUGGING_TYPE != DBX_DEBUG && write_symbols == DBX_DEBUG) Index: gcc/common.opt === --- gcc/common.opt (revision 247273) +++ gcc/common.opt (working copy) @@ -2342,8 +2342,8 @@ Common Report Var(flag_strict_aliasing) Assume strict aliasing rules apply. fstrict-overflow -Common Report Var(flag_strict_overflow) Optimization -Treat signed overflow as undefined. +Common NegativeAlias Alias(fwrapv) +Treat signed overflow as undefined. Negated as -fwrapv. fsync-libcalls Common Report Var(flag_sync_libcalls) Init(1) Index: gcc/doc/invoke.texi === --- gcc/doc/invoke.texi (revision 247273) +++ gcc/doc/invoke.texi (working copy) @@ -420,7 +420,7 @@ Objective-C and Objective-C++ Dialects}. -fsplit-paths @gol -fsplit-wide-types -fssa-backprop -fssa-phiopt @gol -fstdarg-opt -fstore-merging -fstrict-aliasing @gol --fstrict-overflow -fthread-jumps -ftracer -ftree-bit-ccp @gol +-fthread-jumps -ftracer -ftree-bit-ccp @gol -ftree-builtin-call-dce -ftree-ccp -ftree-ch @gol -ftree-coalesce-va