On 10/20/21 7:22 AM, Richard Biener via Gcc-patches wrote:
This maps -ftrapv to -fsanitize=signed-integer-overflow
-fsanitize-undefined-trap-on-error, effectively removing
flag_trapv (or rather making it always false).
It sounds like C/C++ programmers might benefit from this change
but users of the option in other languages would not. I'm sure
they'd appreciate a heads up on the upcoming removal of a feature
so they could adjust to it. Issuing a warning would be one way
to give them such a heads up, while keeping the existing behavior
for a release, and then removing it.
This has implications on language support - while -ftrapv
was formerly universally available the mapping restricts it
to the C family of frontends.
It also raises questions on mixing -ftrapv with -fsanitize
flags, specifically with other recovery options for the
undefined sanitizer since -fsanitize-undefined-trap-on-error
cannot be restricted to the signed-integer-overflow part at
the moment. To more closely map behavior we could add
-fsanitize=trapv where with a single option we could also
simply alias -ftrapv to that.
Code quality wise a simple signed add compiles to
movl %edi, %eax
addl %esi, %eax
jo .L5
...
.L5:
ud2
compared to
call __addvsi3
and it has less of the bugs -ftrapv has. The IL will
not contain a PLUS_EXPR but a .UBSAN_CHECK_ADD internal
function call which has rudimentary support throughout
optimizers but is not recognized as possibly terminating
the program so
int foo (int i, int j, int *p, int k)
{
int tem = i + j;
*p = 0;
if (k)
return tem;
return 0;
}
will be optimized to perform the add only conditional
and the possibly NULL *p dereference first (note the
same happens with the "legacy" -ftrapv). The behavior
with -fnon-call-exceptions is also different as the
internal functions are marked as not throwing and
as seen above the actual kind of trap can change (SIGILL
vs. SIGABRT).
One question is whether -ftrapv makes signed integer overflow
well-defined (to trap)
Trapping isn't well-defined in the C/C++ sense of the word.
It's still undefined behavior, even if it's documented that
way. (Same way dereferencing a null pointer is undefined,
even if it results in SIGBUS.)
Martin
like -fwrapv makes it wrap. If so
the the above behavior is ill-formed. Not sure how
sanitizers position themselves with respect to this and
whether the current behavior is OK there. The patch below
instruments signed integer ops but leaves them undefined
so the compiler still has to be careful as to not introduce
new signed overflow (but at least that won't trap).
Currently -fwrapv -fsanitize=signed-integer-overflow will
not instrument any signed operations for example.
I do consider the option to simply make -ftrapv do nothing
but warn that people should use UBSAN - that wouldn't
imply semantics are 1:1 the same (which they are not).
Bootstrapped and tested on x86_64-unknown-linux-gnu, regresses
FAIL: gcc.dg/vect/trapv-vect-reduc-4.c scan-tree-dump-times vect "Detected
reduc
tion\\\\." 3
FAIL: gcc.dg/vect/trapv-vect-reduc-4.c scan-tree-dump-times vect "using an
in-or
der \\\\(fold-left\\\\) reduction" 1
FAIL: gcc.dg/vect/trapv-vect-reduc-4.c scan-tree-dump-times vect
"vectorized 3 l
oops" 1
where the vectorizer doesn't know the UBSAN IFNs.
2021-10-20 Richard Biener <rguent...@suse.de>
* opts.c (common_handle_option): Handle -ftrapv like
-fsanitize=signed-integer-overflow
-fsanitize-undefined-trap-on-error and do not set
flag_trapv.
---
gcc/opts.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/gcc/opts.c b/gcc/opts.c
index 65fe192a198..909d2a031ff 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -3022,7 +3022,21 @@ common_handle_option (struct gcc_options *opts,
case OPT_ftrapv:
if (value)
- opts->x_flag_wrapv = 0;
+ {
+ opts->x_flag_wrapv = 0;
+ opts->x_flag_sanitize
+ = parse_sanitizer_options ("signed-integer-overflow",
+ loc, code, opts->x_flag_sanitize,
+ value, false);
+ if (!opts_set->x_flag_sanitize_undefined_trap_on_error)
+ opts->x_flag_sanitize_undefined_trap_on_error = 1;
+ /* This keeps overflow undefined and not trap. Specifically
+ it does no longer allow to catch exceptions together with
+ -fnon-call-exceptions. It also makes -ftrapv cease to
+ work with non-C-family languages since ubsan only works for
+ those. */
+ opts->x_flag_trapv = 0;
+ }
break;
case OPT_fstrict_overflow:
