[RFC][Draft patch] Introduce IntegerSanitizer in GCC.

[Maxim Ostapenko]

On 11/07/16 18:05, Jakub Jelinek wrote:
> On Tue, Jul 05, 2016 at 10:31:31AM +0300, Maxim Ostapenko wrote:
>> CC'ing Jakub, Marek and Kostya, sanitizer maintainers in GCC.

Jakub, thanks for your summary.

> I'm not convinced it is a good idea, that is why we've intentionally left it
> out when adding UBSan support, IMHO such an option defines substantially
> different languages.

The reason why I thought about -fsanitize=unsigned-integer-overflow 
would be useful is that people still hit on undesired integer overflows 
in their code (that may even lead to security vulnerabilities), despite 
the fact some people intentionally rely on them. Of course those people 
can just ignore -fsanitize=unsigned-integer-overflow. But others might 
find this option to be useful in their projects (e.g. Android people), 
because it finds real bugs in their code (even this very draft version 
managed to find several bugs in OSS projects).

>
> The wrapping behavior of unsigned integer arithmetics is
> integral part of the language, lots of code obviously relies on it.
> E.g. in unsigned arithmetics, x - y and t = -y; x + t is the same thing,
> while the patch would treat that differently.  Or would it even allow
> unary negation of non-zero unsigned values?  On IRC I've mentioned e.g.
> loops with unsigned iterator and bounds that can iterate forward or backward
> at runtime, like:
> void foo (unsigned start, unsigned end, unsigned step)
> {
>    for (unsigned i = start; i != end; i += step)
>      ...
> }
> would one have to rewrite this to do something like: "step_is_negative" ? i -= -step : i += step
> instead?  How would code portably say that for a given arithmetics it really
> does assume wrapping behavior?  Replacing x = y + z; with
> (void) __builtin_add_overflow (y, z, &x);
> is not sufficiently portable, trying x = (y + z) & ~(unsigned) 0;
> I assume would still trigger, because the unsigned arithmetics still
> "overflows".

True, one should rewrite this like "step_is_negative" ? i -= -step : i 
+= step to avoid false positive report here. Another option would be 
just to ignore a FP report here or use some post processing to filter it 
out. Anyway, there is a bunch of situations where we can't simply 
determine whether overflow is desired or not and people would need to 
decide by themselves.

>
> I'm really surprised you didn't run into many more "issues" during your
> tests.

Well, I used some heuristics (e.g. guessing the real type of LHS of 
binary expression) to reduce number of these "issues" during my tests. 
Actually, I didn't see many such cases:

void foo (unsigned start, unsigned end, unsigned step)
{
   for (unsigned i = start; i != end; i += step)
     ...
}

instead, much more common were:

void bar(char *start, unsigned offset)
{
    ...
    char *end = start + offset;
}

with negative offset. And here we need a special treatment (I treated 
'offset' as a signed value when expanding a ISAN_CHECK_ADD builtin in 
expand_addsub_overflow, this is not portable, but works most of the time).

-Maxim
>
> 	Jakub
>
>