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

[Zhongxing Xu]

2009/11/10 Sebastian Redl <sebastian.redl at getdesigned.at>:
> 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
>

This is enlightening. These cases are covered by current simple
heuristic: only issue a warning when sizeof is applied on a direct
VarDecl reference. We'll see if that is too general.