[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

[Anna Zaks]

On Feb 23, 2012, at 5:28 AM, Jordy Rose wrote:

> 
> On Feb 23, 2012, at 0:36, Anna Zaks wrote:
> 
>> 
>> On Feb 18, 2012, at 5:34 AM, Jordy Rose wrote:
>> 
> 
>>>> +// 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.

I don't fully get your point here.. I think implementing domain specific knowledge in the checkers rather then the analyzer core is the right solution. Reasoning about non-malloc related functions makes the Malloc checker more powerful. -> Setting the dependency between the two is the way to solve the problem.

Ideally, CStringBasic should only include the function evaluation logic (and skip the CStringNullArg warning). It's a cleanup, which we should definitely do. But other then that, I do not see issues with this solution.

> 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
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120223/a56aacc9/attachment.html>