Traps for signed arithmetic overflow

I think I had better say something about this.

You are right; there is no overflow. But who needs such apparently
deficient (if not incomplete at all) behavior? People could start
arguing that integer promotion is mandated by the C standard and is what
'portable' code might rely on. Yes you are right, totally right about a
_very_ poor excuse: The so-called 'portable' code should not rely on the `int`
-> `short` conversion either, which is implementation-defined and not so
portable after all. It is a shame for C to have integer promotion. It is
a flaw for UBSan to discriminate overflows on `short`.

>
>>
>>>
>>>> when compiling this example with gcc -O2 -ftrapv:
>>>> [...]
>>> Try with "-fsanitize=undefined
>>> -fsanitize-undefined-trap-on-error". -ftrapv is quite broken and not
>>> really maintained. I think fixing it would mean making it an alias for
>>> the sanitizer version.
>>
>> Thanks for the quick reply. That does indeed produce exactly the code I
>> want.
>>
>> It doesn't seem so detect overflow for short or char types, though:
>
> That's because there is no overflow. Read about integer promotion.

I think I had better say something about this.

You are right; there is no overflow. But who needs such apparently
deficient (if not incomplete at all) behavior? People could start
arguing that integer promotion is mandated by the C standard and is what
'portable' code might rely on. Yes you are right, totally right about a
_very_ poor excuse: The so-called 'portable' code should not rely on the `int`
-> `short` conversion either, which is implementation-defined and not so
portable after all. It is a shame for C to have integer promotion.

It is a flaw for UBSan to discriminate overflows on `short`.

It is not an ubsan flaw.
There is no overflow happening, therefore it has nothing to report.

First, the sub-int types are promoted to int-sized types, then
those promoted values are added (and this addition is what the ubsan sanitizes),
and then the result is implicitly cast back to the original type.

Naturally, sometimes that is lossy. That is indeed an issue.
There are several bugs about this:
https://bugs.llvm.org/show_bug.cgi?id=21530
https://github.com/google/sanitizers/issues/940

If you look at the release notes, you may see that clang-7 has
gained -fsanitize=implicit-conversion (https://reviews.llvm.org/D48958),
which *does* detect just *this* kind of issues. Yay.

TBN, the version in clang-7 is *very* partial (only detects lossy
truncations), so if you want to use that,
you should use trunk, in which more cases are now detected - sign
changes (https://reviews.llvm.org/D50250)
compound assignment operations (https://reviews.llvm.org/D53949).

Though trunk still does not catch everything - i still need to look
into unary inc/dec, bitfields; atomics.

--
Best regards,
LH_Mouse

Roman.

-fsanitize=undefined instruments undefined behaviour. This isn't undefined
behaviour. Also, both -fsanitize=signed-integer-overflow and -ftrapv are
documented to only do things for addition, subtraction, and multiplication
(not conversion).

If you want a warning for implementation-defined behaviour, sure, not many
people will oppose that (it will warn all of the time, making it not very
useful, but hey). Still, it should be a separate option. Implementation-
defined behaviour is not undefined, after all.

Segher

It is a design flaw in GCC, which should have chosen the
"implementation-defined signal" solution, as allowed by the
C standard. This would be much more secure.

As allowed by C99 and later, which was after GCC chose it's
implementation-defined behaviour for those conversions. And raising a
signal here would not be appreciated by all developers.

Adding a conversion sanitizer seems like a good solution, as it allows
optional checking, when the developer requests it. The
implementation-defined behaviour doesn't need to raise a signal that
way.