[cfe-commits] r86464 - in /cfe/trunk: include/clang/Analysis/LocalCheckers.h include/clang/Frontend/Analyses.def lib/Analysis/CheckSizeofPointer.cpp lib/Frontend/AnalysisConsumer.cpp

[Sebastian Redl]

Zhongxing Xu wrote:
> 2009/11/10 Ted Kremenek <kremenek at apple.com>:
>   
>> I wasn't arguing that we make the check malloc() specific, and I don't think
>> we should because the problem is more general than just looking at pointers
>> returned from malloc().  We just need to make it smart enough that when it
>> gives a warning, it is:
>>
>> (a) correct most of the time
>>
>> (b) gives a meaningful diagnostic that people understand how they screwed up
>> and how they can fix the issue
>>
>> I don't think we need to make it malloc() specific, but we can educate the
>> check about malloc() if that helps accomplish (a) and (b).
>>     
>
> Make sense. Let's start with malloc. I'm fine to remove this too general check.
>   
I think there are three situations where a programmer would use 
sizeof(pointer):
1) Mistakenly thinking that this gives a buffer size. This is what we 
want to warn about.
2) Actually allocating memory for a pointer or array of pointers. This 
can be recognized because the result of the sizeof would be used in some 
memory allocation scheme that should end up with a pointer of type 
"pointer to our original pointer".
3) Doing something that needs to know the size of a pointer for platform 
decisions. I don't think such code should ever appear in runtime 
expressions.

But when it's about avoiding false positives, whitelisting is usually 
more effective. The real problem is that the user thinks that sizeof 
gives information about the size of the pointee, not the pointer. Such a 
belief would be evident in the use of the sizeof result. If it is used 
in pointer arithmetic with the pointer the size was taken of, we've got 
a problem:
int *end = start + sizeof(start);
If it is used to allocate memory which isn't meant to store such 
pointers, we've got a problem:
char *copy = malloc(sizeof(original)); strcpy(copy, original);
This should trigger on all types except typeof(original)*, actually:
wchar_t *converted = malloc(sizeof(original) * sizeof(wchar_t)); 
convert(converted, original);
If the user thinks he can compare the sizes of two buffers, we've got a 
problem:
if(sizeof(buffer1) == sizeof(buffer2))

So, track the value resulting from sizeof(ptr) and trigger on three 
uses: arithmetic with ptr, or a pointer of the same type, allocation 
which results in a pointer type other than typeof(ptr)*, and comparison 
with another sizeof result coming from the same pointer type. I think 
that would be a pretty good heuristic.

Sebastian