[cfe-commits] PATCH: Enhance array bounds checking

[Kaelyn Uhrain]

I'm still looking at the pointer arithmetic warnings to determine which are
false positives, but I realized I screwed up my previous stats a bit as I
forgot to account for duplicated/repeated warnings (e.g. the same header
included in multiple compilation units and so generating the same warnings
multiple times). For unique warnings, the stats are:

- 16.5% increase in warnings from before my patch (originally reported a 24%
increase)
- 47% of those new warnings being about pointer arithmetic
- 6.7% of all of the warnings emitted with my patch applied are concerning
pointer arithmetic. (originally reported 8.3%)
- The new pointer arithmetic warnings represent a 7.7% increase in warnings
from before my patch, not 10%.

I'll send another email once I have a feel for how noisy the pointer
arithmetic warnings are.

Cheers,
Kaelyn

On Mon, Jul 18, 2011 at 5:54 PM, Kaelyn Uhrain <rikka at google.com> wrote:

> Ted,
>
> You're welcome. I'll try to figure out what fraction of the pointer
> arithmetic warnings are false positives (requires a bit of manual digging on
> my part to determine if the code is indeed buggy or if it is valid /
> intended). For the overall 24% increase in warnings, keep in mind that over
> half of that is the existing bounds checking now being applied to cases
> where it wasn't before, i.e.:
>
> char *foo[5];
> foo[77];  // -Warray-bounds already found
> &foo[77];  // -Warray-bounds currently misses
> *foo[77];  // -Warray-bounds currently misses
>
> The function that did the bounds checking would never catch the last two
> cases because it would see a UnaryOperator (in the above cases for the '&'
> and the '*') and skip the expression instead of looking inside the
> UnaryOperator expression for the array subscripting.
>
> The new pointer arithmetic bounds checking only represents a 10% increase
> in warnings--and IMHO that is the only part where the number of false
> positives introduced might be an issue.
>
> Cheers,
> Kaelyn
>
>
> On Mon, Jul 18, 2011 at 5:36 PM, Ted Kremenek <kremenek at apple.com> wrote:
>
>> Hi Kaelyn,
>>
>> Thanks for the statistics.  What would be good to know is what fraction of
>> these are false positives (i.e., are these all real bugs)?  A small random
>> sample might be helpful.  A 24% increase in warnings is fairly substantial,
>> and we don't want to do that unless there is a real benefit.
>>
>> Ted
>>
>> On Jul 18, 2011, at 5:18 PM, Kaelyn Uhrain wrote:
>>
>> Ted,
>>
>> On Thu, Jul 14, 2011 at 5:04 PM, Ted Kremenek <kremenek at apple.com> wrote:
>>
>>>
>>> The only other issue: should this be controlled under a separate warning
>>> flag, at least initially so we can experiment with this new warning and see
>>> how noisy it is?  E.g. "-Warray-bounds-pointer-arithmetic".
>>>
>>
>> I've tested the -Warray-bounds changes against the Google codebase and my
>> patch increases the number of warnings from -Warray-bounds by 24%. Of the
>> new warnings, 57.33% are for array indexes that most likely weren't picked
>> up before because of unary operators like & or * (11.1% of all the warnings
>> now emitted), and the remaining 42.67% are from out-of-bounds pointer
>> arithmetic (8.3% of all the warnings from -Warray-bounds).
>>
>> Cheers,
>> Kaelyn
>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20110719/20925b47/attachment.html>