Hi all!
I found that UBSan uses vrp pass to optimize generated checks. Keeping
in mind that vrp pass is about performance not stability I found example
where UBSan may skip true positive.
Example came from spec2006 perlbench:
int ext;
int
Perl_do_sv_dump()
{
int freq[10];
int i;
int max = 0;
int t = INT_MAX - 20;
if (max < ext)
max = ext;
for (i = 0; i <= max; i++)
if (freq[i])
ext = 0;
t += i; <<< (*)
return t;
}
vrp pass here sets vrp('i') to [0..10] in assumption that 'freq[i]' wont
violate array bound (vrp uses loop iteration number calculation, see
adjust_range_with_scev in tree-vrp.c). This means that UBSAN_CHECK_ADD
build for (*) should be deleted as redundant (and actually it is deleted
by vrp pass). So if at the execution max = 30, freq[5] != 0 uncaught
overflow will occur.
There are also some unsafe code in functions
ubsan_expand_si_overflow_addsub_check,
ubsan_expand_si_overflow_mul_check which uses get_range_info to reduce
checks number. As seen before vrp usage for sanitizers may decrease
quality of error detection.
Potentially unsafe vrp decisions should preferably not be exploited by
any sanitizer pass. I have several ideas how we could achieve that:
1) Do not use vrp info for sanitizers.
2) Do not use unsafe conclusions during vrp pass if sanitizers are enabled.
3) Make two versions of value range data safe and unsafe.
P.S.
To reproduce the issue use appropriate GCC options
(-fsanitize=signed-integer-overflow -S -O3) and attached trivial patch
which disables UBSan check build in inner loops. This patch is needed
because currently iteration number calculation used by vrp pass do not
supports yet UBSAN_CHECK_[ADD/SUB/MUL].
--Marat
diff --git a/gcc/ubsan.c b/gcc/ubsan.c
index dde0418..97dbf33 100644
--- a/gcc/ubsan.c
+++ b/gcc/ubsan.c
@@ -1014,6 +1014,10 @@ instrument_si_overflow (gimple_stmt_iterator gsi)
|| GET_MODE_BITSIZE (TYPE_MODE (lhstype)) != TYPE_PRECISION (lhstype))
return;
+ // skip for inner loops
+ if (gimple_bb(stmt)->loop_father->inner == NULL)
+ return;
+
switch (code)
{
case MINUS_EXPR: