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

[Maxim Ostapenko]

Hi!

Although in languages like C and C++ unsigned integer overflow reliably 
wraps around and well defined, sometimes it may indicate an error in 
code and lead to undesirable consequences and even security 
vulnerabilities 
(https://android-developers.blogspot.ru/2016/05/hardening-media-stack.html). 
Clang has corresponding '-fsanitize=unsigned-integer-overflow' option 
that may catch some of these errors (but misses some important cases 
such as pointers overflow), GCC has not and instruments only signed 
integer operations (S = S {+, -, *} S).

Although GCC has a nice framework for instrumenting even "mixed" binary 
operations (e.g. S = U + S) in middle end, implemented by Jakub some 
time ago (see expand_addsub_overflow, expand_mul_overflow etc in 
internal-fn.c), adding support for sanitizing unsigned and "mixed" 
binary operations seems to be quite problematic in terms of huge amount 
of false positive reports that may pop up in following cases:

1) Some code intentionally relies on overflows, e.g. when computing hash 
codes. Unfortunately, it seems that we cannot do anything with it on 
compiler side, programmer will need to do some filtering here by himself.

2) Unfortunately, it seems that C and C++ FEs don't provide suitable 
interfaces to get *LHS* real type when building binary expressions, so 
*build_binary_op* doesn't know about *LHS* real type and we can easily 
end up with false positive report e.g. in such code:

int
cmp (const char *s1, const char *s2)
{
   return strlen (s1) - strlen (s2);
}

because in gimple we would have something like this:

   unsigned int tmp, len1, len2;
   int result;
   .........
   tmp = len1 - len2;  <===== FP here if len1 < len2
   result = (int) tmp;
   return result;

3) GCC relies on unsigned integer overflows when constructing 
POINTER_PLUS expressions. We need a special treatment for this case, 
because it's quite common to have a negative offset from a given pointer 
and we don't want have false reports here.

This very draft patch (of course we would need more testcases, changelog 
entry etc) adds support for '-fsanitize=unsigned-integer-overflow' to 
GCC for PLUS, MINUS and MULT operations, extending its functionality to 
support pointers overflow and binary operations with different signess 
of theirs operands ("mixed" binary operations, say, S = U + S). It 
simply replaces all +, - and * by new ISAN_CHECK_{ADD, SUB, MUL} 
intrinsics in C and C++ FEs to be lowered later during intrinsics 
expansion. For example, if we have a + b expression, it would be 
replaced in FE by following call:

a + b -> ISAN_CHECK_ADD (a, b, a_unsiged_p, b_unsigned_p, result_unsigned_p)

where *a_unsiged_p* is 1 if *a* is unsigned and 0 otherwise. The same 
rule applies for *b_unsigned_p*. The *result_unsigned_p* value should 
reflect result type signess, but as mentioned in 2), we don't know it 
when building binary expression, so we can only guess it from arguments 
types and maybe change it (in a very hackish way) later (see 
*isan_maybe_change_ifn_sign_arg* function) in order to reduce number of 
FPs (in fact, these hacks can significantly reduce FPs ratio, ~ 30%). 
These values will be used in *expand_addsub_overflow* and 
*expand_mul_overflow* functions to compute overflow correctly.

This patch survives GCC bootstrap, I've also managed to build Firefox 
and Chromium with '-fsanitize=unsigned-integer-overflow' enabled. The 
tool found several bugs in opensource software:

1) Integer overflow when computing len parameter in "file" utility 
(http://bugs.gw.com/view.php?id=555)
2) Pointer overflow (nullptr - 1) in Google's skia library 
(https://bugs.chromium.org/p/skia/issues/detail?id=5415)
3) Unsigned integer overflow in HAL component in Firefox 
(https://bugzilla.mozilla.org/show_bug.cgi?id=1280514)

Is community interested in such a tool? Any feedback would be greatly 
appreciated, especially some points how we can figure out real result 
type for *LHS* of binary expressions in
*build_binary_op* in order to avoid unnecessary FPs and ugly hacks in FEs.

Thanks,
-Maxim
-------------- next part --------------
A non-text attachment was scrubbed...
Name: isan.diff
Type: text/x-diff
Size: 49134 bytes
Desc: not available
URL: <https://gcc.gnu.org/pipermail/gcc/attachments/20160704/7a2df5d1/attachment.bin>