[cfe-dev] Analyzer discrepancy with __builtin_alloca

[austin seipp]

Hi Ted,

Thanks for the reply. I just found BuiltinFunctionChecker and see that
it deals with both __builtin_alloca and __builtin_expect, but there's
still something awry here I think.

For one, as I said before, I modified checkPreStmt to tell me both
when getCalleeName returns an empty StringRef (i.e. it is ignored,)
and in the case it doesn't, output the callee name. But per the output
above, it neither says the call exists, or that it skipped it due to
the empty StringRef - so it seems checkPreStmt is in fact -never-
called in the case of __builtin_alloca! I played around with
BuiltinFunctionChecker's route of getting the FunctionDecl and looking
if it corresponds to a builtin's ID, but as you would expect - no
dice.

The other concerning thing is that BuiltinFunctionChecker also has
logic to deal with __builtin_expect, but despite that it appears
perfectly normally as you would expect when checkPreStmt is invoked in
the testcase. There is no difference semantically or AST-wise between
__builtin_expect and any other function call here - you can just look
at the textual name. So if there -is- a discrepancy, it most certainly
seems to concern itself only with __builtin_alloca. Or,
__builtin_alloca is being handled correctly, and __builtin_expect is
in fact the buggy case here (my brain tells me it is likely the
former, since as far as the analyzer is concerned, I think __builtin's
would be recognized as a regular call expression like any other.)
Either way something weird is going on that should be fixed.

I went ahead and added a simplistic evalCall callback to
UnixAPIChecker to see if it could identify the call to
__builtin_alloca, much in the same way BuiltinFunctionChecker does.
This *does not work either* in fact! So now I'm pretty darn confused.

I think there's very clearly a bug or something lurking here, but I'm
at a loss as to how to proceed. Attached is my patch to UnixAPIChecker
with some helper debugging output, and appropriate modifications to
the testsuite which should expose the problem (the compiler will
complain that there was an expected warning, but none was found.)
Since the crappy debug output will fudge up the 'make test' runner I'd
just advise checking unix-fns.c for the easy case.

Please excuse the inclusion of iostream - a blatant violation of LLVM
coding practice; I just needed a quick and simple dump. I also
apologize that I had to include the small refactoring and valloc
addition I mentioned as part of the patch, but the work was already
there. I hope you find it useful in making sense of things.

Thanks for the help, and sorry for the long-winded rant.

PS. There seems to be further weirdness upon review of my test output
that doesn't concern itself with builtins, but, I'll go ahead and
mention it and possibly put it into another cfe-dev thread, or a
bugzilla ticket - look closely at the unix-fns.c analysis test, and
then look at the output produced in my example with my little patch.
The weird part is how the analyzer identifies the first 4 calls being
'open, open, close, open', in that order, but in the source code, the
analyzer *should* be seeing open, close, open, close! In fact, it only
sees 1 close call, period! After those 4 calls, all further calls are
identified properly, 'in order' as you would expect to see. I may be
missing something, but that seems to indicate something Very Bad is
happening.

On Fri, Jan 6, 2012 at 4:51 PM, Ted Kremenek <kremenek at apple.com> wrote:
> Hi Austin,
>
> The logic for __builtin_alloca is being handled by BuiltinFunctionChecker, which explicitly models the semantics of the function.  It does this by responding to 'evalCall()'.  That should not interfere with the checkPreStmt() callback in UnixAPIChecker, and if that is the case there is a bug here.
>
> What is likely the case is that builtins are just modeled slightly differently in the AST.  I suggest that you take a look at BuiltinFunctionChecker for how we recognize calls to __builtin_alloca and see if that approach works for you.  In any case, please follow-up on the list to see if that fixes or doesn't fix the issue you are seeing.
>
> Cheers,
> Ted
>
> On Jan 6, 2012, at 2:38 PM, austin seipp wrote:
>
>> Hello,
>>
>> I was working on adding alloca/valloc checks (as well as a small
>> cleanup) to UnixAPIChecker following the recent changes to detect
>> zero-sized allocations in calloc/realloc (SVN r147500,) when I noticed
>> a strange issue when attempting to identify calls to __builtin_alloca.
>>
>> 'alloca' seems to dissolve to __builtin_alloca via a CPP #define on OS
>> X, so for the check to work and be 'complete' it needs to identify
>> both names. However, it seems as if the checker callback -
>> checkPreStmt on a CallExpr - is not invoked for calls to
>> __builtin_alloca, but it does work just dandy in the case of say,
>> __builtin_expect. So presumably I'm missing something important and
>> it's expected behavior, or this is a bug I guess.
>>
>> A little more concrete: I amended ./test/Analysis/unix-fns.c to
>> contain calls for alloca - with a prototyped definition so there
>> aren't complaints about implicit declarations - *and* calls to
>> __builtin_alloca, both of which should trigger a 0-sized allocation
>> warning. I also modified UnixAPIChecker to dump the name of every
>> callee it comes across, or dump some trivial output if it skips the
>> call due to getCalleeName returning an empty StringRef.
>>
>> The results look like this on my machine:
>>
>> $ /Users/a/code/llvm-build/Debug+Asserts/bin/clang -cc1
>> -internal-isystem
>> /Users/a/code/llvm-build/Debug+Asserts/bin/../lib/clang/3.1/include
>> -triple x86_64-apple-darwin10 -analyze
>> -analyzer-checker=unix.API,osx.API
>> /Users/a/code/llvm/tools/clang/test/Analysis/unix-fns.c
>> -analyzer-store=region -fblocks -verify
>> fname = open
>> fname = open
>> fname = close
>> fname = open
>> fname = __builtin_expect
>> fname = dispatch_once
>> fname = __builtin_expect
>> fname = dispatch_once
>> fname = pthread_once
>> fname = pthread_once
>> fname = malloc
>> fname = malloc
>> fname = calloc
>> fname = calloc
>> fname = calloc
>> fname = realloc
>> fname = realloc
>> fname = alloca
>> fname = alloca
>> fname = valloc
>> fname = valloc
>>
>> The calls to __builtin_alloca should effectively appear in the same
>> spot as the 'alloca' calls, but they clearly aren't. There also isn't
>> any 'no name' output indicating the call was skipped as a result of an
>> empty StringRef from getCalleeName. So it seems as if checkPreStmt for
>> a CallExpr completely skips over __builtin_alloca. The calls do appear
>> in a dump of the CFG as you would likely expect.
>>
>> Is __builtin_alloca handled specially in the analyzer somewhere, and
>> thus I'm missing something? If it is and this is unavoidable, I'll
>> just back out those changes and send a patch containing the valloc
>> addition as well as the small cleanup if they're worth it. But I don't
>> see off hand why it can't work for alloca as well, so I thought I'd
>> ask.
>>
>> --
>> Regards,
>> Austin
>> _______________________________________________
>> cfe-dev mailing list
>> cfe-dev at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>



-- 
Regards,
Austin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: no-dana-only-zuul.patch
Type: application/octet-stream
Size: 9444 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20120106/6a7ead02/attachment.obj>