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

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

My issue (and really, I don't care so strongly in this case; I just want to make it clear) is that let's say someday we model the /entire/ C standard library in the analyzer, and we do it so well that we're basically inlining every call. A lot of those calls do or don't invalidate regions and may or may not return aliases of their arguments. Will MallocChecker then require /every/ C-library-related checker to be turned on when we run it?

As a concrete example, the pthread_setspecific whitelist entry could be fixed by modelling pthread_getspecific as well. That probably goes into another checker, the ThreadSpecificData checker or something. Does enabling MallocChecker automatically enable the ThreadSpecificData checker, even though a program may never use pthread_*specific or even threads at all?

Implementing domain-specific knowledge about the string functions should indeed go in the CStringChecker. Implementing domain-specific knowledge about malloc functions should go in the MallocChecker. I don't consider knowledge about string functions to be part of MallocChecker, and so I don't know if enabling MallocChecker should automatically include the evaluation of string functions.

Perhaps the right long-term solution is to make this a "soft" dependency -- if the user explicitly disables the C string checks, MallocChecker won't re-enable them. (A "hard" dependency would print an error message and exit if the checker's not available.) Currently we don't have the infrastructure for that, though, and it's true that the quality of results /is/ better with CStringChecker's participation, so I guess I'll just leave it at this.

Jordy