[cfe-commits] r150086 - /cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

[Jordy Rose]

On Feb 9, 2012, at 3:13, Anna Zaks wrote:

> Author: zaks
> Date: Wed Feb  8 14:13:28 2012
> New Revision: 150086
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=150086&view=rev
> Log:
> [analyzer] MallocChecker: convert from using evalCall to
> post visit of CallExpr.
> 
> In general, we should avoid using evalCall as it leads to interference
> with other checkers.

(disclaimer: rambling thoughts on the commit message, rather than the commit itself)

I think saying "avoid evalCall" is the wrong way to put it. The purpose of evalCall, as I see it, is /either/ to say "this checker provides a return value for the call", or "this checker completely models the semantics of the call". I think part of your point is that doing the latter is usually impossible. However, using eval::Call should mean that even system globals are not invalidated, while using check::PostStmt<CallExpr> says they are. For, say, free(), the former is probably appropriate. (There's no attribute like [[const]] or [[pure]] for this, huh.)

The problem comes when multiple checkers would be able to handle the evaluation of a call...one of them will win, and the other one's semantics will not be used. I think that's the other half of your point. And of course if a function is inlined, /no/ checker should be providing a return value anymore.

It's sort of a problem that you can't provide a return value without also handling argument and global invalidation. If we had full aliasing constraints, such that a checker could assumeEQ(returnVal, inputVal), then we'd probably have no need for evalCall; every CE could start with a conjured symbol, which each checker could then constrain/alias to the right value. Already we can handle constant integers (and null) like this, but things like "the first input", "the length of the string", or "an alloca region" which are themselves symbolic, won't work properly. (Constraints will be copied rather than aliased in the case of symbols, and there's NO way to make a symbolic region actually refer to an alloca region AFAIK.)

I guess my first pass at a long-term guideline is "Unless you need to set the return value to something non-symbolic, do not use evalCall. If you do use evalCall, please remember to invalidate the arguments (and globals) as appropriate."

And yes, I realize I'm blowing a commit message all out of proportion, but this is something that will eventually go in the checker developer manual, right?

Jordy