[cfe-commits] r150846 - in /cfe/trunk: lib/StaticAnalyzer/Checkers/CStringChecker.cpp lib/StaticAnalyzer/Checkers/InterCheckerAPI.h lib/StaticAnalyzer/Checkers/MallocChecker.cpp test/Analysis/malloc.c

[Jordy Rose]

On Feb 23, 2012, at 0:36, Anna Zaks wrote:

> 
> On Feb 18, 2012, at 5:34 AM, Jordy Rose wrote:
> 
>> Does MallocChecker really "need" to know this? I mean, I doubt CStringCheckerBasic is a huge performance hit, but it seems like someone who wants the analyzer to model C library string functions should enable CStringChecker themselves.
> 
> Malloc Checker directly depends on the evaluation of the CString functions. In particular, it needs to know that when strcpy is evaluated, the return value is equal to the input parameter. You can see the malloc false positive fixed by this commit at the very end of the diff.

>>> +// Rely on the CString checker evaluation of the strcpy API to convey that the result of strcpy is equal to p.
>>> +void symbolLostWithStrcpy(char *s) {
>>> +  char *p = malloc(12);
>>> +  p = strcpy(p, s);
>>> +  free(p);
>>> +}

Yes, I see that there was a false positive here. But it seems that MallocChecker directly depends on the evaluation of /every/ alias-returning function. The string functions happen to be fairly ubiquitous and present in the same library, so I can see that it might be worth special-casing them, but I personally don't consider understanding strcpy to be an /inherent/ feature of MallocChecker. If the analyzer in general doesn't know about string functions, you shouldn't be surprised when MallocChecker doesn't know about the string functions either.

Then again, since it doesn't cost /that/ much, maybe this is a case of "Clang should Do The Right Thing" by default. So I withdraw my objection.

Jordy