Re: [RFC] UBSan unsafely uses VRP

[Richard Biener <richard dot guenther at gmail dot com>]

On Tue, Nov 11, 2014 at 3:15 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Tue, Nov 11, 2014 at 05:02:55PM +0300, Marat Zakirov wrote:
>> 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.
>
> Well, if max is >= 10, then you should get -fsanitize=bounds error already.
> -fsanitize=undefined already disables -faggressive-loop-optimizations,
> perhaps it can also disable other optimizations (I thought deriving number
> of iterations from assuming undefined behavior doesn't occur in loop stmts
> is already guarded by -faggressive-loop-optimizations though).

You could use -fno-strict-overflow ...

>> 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.
>
> Using VRP is completely intentional there, we don't want to generate too
> slow code if you decide you want to optimize your code (for -O0 VRP isn't
> performed of course).

Indeed.  Note that the strict-overflow warnings are already a bad burden
on VRP quality - a way out to me was always to track two lattices,
one assuming strict-overflow and one assuming wrapping overflow.
For strict-overflow warnings you then can compare simplification outcome
against two lattices and warn if the result differs.  Instead of that odd
+-INF(OVF) saturation.

Richard.

>         Jakub