[cfe-commits] r150215 - in /cfe/trunk: lib/StaticAnalyzer/Checkers/MallocChecker.cpp test/Analysis/malloc-annotations.c test/Analysis/malloc.c

[Anna Zaks]

On Feb 11, 2012, at 6:58 PM, Jordy Rose wrote:

> 
> On Feb 10, 2012, at 8:11, Anna Zaks wrote:
> 
>>  ProgramStateRef state = C.getState();
>> +  if (!isa<DefinedOrUnknownSVal>(location))
>> +    return;
> 
> location.isUndef() might be a little prettier, for all of the instances of this in the commit.

I agree. I'll change that.
> It's not nice that we have to do this though...it's sort of boilerplate code for ANY argument in a post-call checker. What do you (and Ted, and everyone) think about skipping post-call checks if any arguments are undefined? (Unknown is okay, of course.)
> 
> When does this happen, anyway? Your test case should hit the "not-a-symbolic-region" error, but I don't know about the undefined argument value.
> 

This commit was more about being defensive.

CallAndMessageChecker, which is a part of core, will catch all the cases when a call is being passed an undefined argument and will generate a sink in that case. So unless we avoid scheduling core, we will never hit the issue. On the other hand, there is nothing that guarantees that Malloc will always run after core.

I think the best way to implement your suggestion is to support dependencies on checkers. Then, any checker which is processing calls could define itself as being dependent on CallAndMessageChecker. It could then assume that the arguments are defined. A partial solution for this could be to hardcode to always run this checker.

Just not issuing postVisit<CallExpr> if any of the arguments are undef is a much easier solution but it's a bit of a hack. Another issue is that we would introduce inconsistency:
- Someone experimenting with a new checker would run into it questioning why the post visit is suddenly not being executed.
- The behavior of preVisit<CallExpr> and postPost<CallExpr> will be different.

I do understand the pain of checking the argument. On the other hand, many checkers do not bother checking if the arguments are undefined, they just assume they are and everything works.

Anna.

> Jordy