[cfe-commits] r86538 - in /cfe/trunk: lib/Analysis/CMakeLists.txt lib/Analysis/GRExprEngineInternalChecks.cpp lib/Analysis/GRExprEngineInternalChecks.h lib/Analysis/PointerArithChecker.cpp test/Analysis/ptr-arith.c

[Ted Kremenek]

On Nov 9, 2009, at 6:02 PM, Zhongxing Xu wrote:

> 2009/11/10 Ted Kremenek <kremenek at apple.com>:
>> Hi Zhongxing,
>>
>> I'm not convinced this is a good check, as there are a bunch of  
>> valid cases
>> where you would do this.
>>
>> First, I think this has the potential to flag a bunch of false  
>> positives on
>> OO code, like Gtk+.  One thing I've noticed is that some C code  
>> tries to
>> compute the start of the object from the address of a field (using
>> offsetof).  That seems reasonable to me, and appears frequently in  
>> certain
>> kinds of code bases.
>>
>> Second, consider C++ iterators.  For example, in our own code base  
>> we might
>> write:
>>
>>  SourceRange R = ...
>>  foo(&R, &R+1);
>>
>> where the '&R+1' represents the "end" iterator.  This is a case  
>> where a
>> variable is treated as an array of size 1.  We'd flag a warning  
>> every time
>> this idiom appears.  That doesn't seem useful.
>
> Sounds reasonable. If this checker cause many false positives I'd like
> to remove it.

Okay.  Let's grind it through real code and see what we get.

>
>>
>> It seems to me that buffer overflow checking, where an actual load/ 
>> store is
>> done on a invalid offset, will catch more real issues.  Are there  
>> cases that
>> we will miss with buffer overflow checking that this one will cover?
>
> We don't have a suitable scheme to represent the location of
>
> int x;
> &x + 1;
>
> Shall we create an element region with offset 1 and super region of a
> scalar type?

Yeah, it seems like ElementRegion(VarRegion(x), 1) would represent  
that location just fine, even that loading/storing from that address  
is bad.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20091109/f642eea3/attachment.html>