[cfe-dev] [analyzer] functions enclosed in if() false positive

[Devin Coughlin via cfe-dev]

> On Oct 20, 2015, at 5:05 AM, robbinson defau via cfe-dev <cfe-dev at lists.llvm.org> wrote:
> 
> Hi list, 
> 
> I've been trying to build a checker for a function that is defined in a shared library. The prototype of these functions look (example for simplicity) like this:
> 
> int
> alloc_t(type_t **, int, int)
> 
> void
> free_t(type_t *);
> 
> In the actual code I want to check (thus not the library rather code that uses the library) I do:
> 
> type_t *ptr;
> 
> if (alloc_t(&ptr, 0, 0) != 0) {
>      // means alloc failure usually return 
>      return (1);
> }
> 
> // do something with *ptr
> 
> free_t(ptr);
> 
> The checker I wrote is more or less, a hybrid of the existing checkers in the clang repo and I used the PDF/video "writing a checker in 24 hours".
> 
> Its been well past 24 hours and I have a checker that works. However, the problem is is that I cant seem to educate the checker well enough, that if it finds the snippet:
> 
> if (alloc_t(&ptr, 0, 0) != 0) 
>     return
> 
> It should not "mark" the ptr  because != 0 means the allocation failed. 
> 
> When linking against the real library however, it does *not* work. (it seems the analyser cant figure out what the external library is returning) I also tried the approach used in the StreamChecker example, but those examples check for the arguments being non NULL which does not work in my case. (as the type_t is "untouched" when the alloc fails) 
> 
> So then I continued trying to wrap my head around check::BranchCondition, but to be honest, I have no clue how to unwind the things to a point where I can update the state (update the state using what? the function? arg0? create a new SymbolRef of what?) or how I can get my hands on the actual values confined with in the if(). Even if I could that far, I'd still would be in the dark on how to proceed. 
> 
> Im pretty sure this all due to my incomplete understanding of all of this, so any help is much appreciated!

You might try a strategy similar to the one that Yuri Gribov used for the vfork() checker (see <http://reviews.llvm.org/D14014 <http://reviews.llvm.org/D14014>> for the patch, this isn’t committed yet to trunk). That checker performs a case split on the return value from calls to vfork(). This causes the analyzer to explore two paths independently. On one path the analyzer knows that the return value is definitely 0 and on the other it would know that the return value is definitely not 0. For your checker, you could then (for example) mark the pointer as definitely unsafe on the path where the return value is not 0. Then in the “if” example you have above this would cause the analyzer to know that on the unsafe path the if condition is true — so the analyzed function returns before the pointer is used. You can take a look at  checkPostCall()  in the linked patch to see how Yuri does it. That checker creates two states (one for 0 and one for non-0) with the assume() method and calls addTransition() for each of them.

Now, case splits are very expensive because they require the analyzer to explore the following code twice (once for the case when the return is 0 and once for when it is not 0). You could avoid this case split on calls to alloc_t() by instead having your checker keep a map from MemRegion *s to SVals. Then, when your checker sees a postCall for alloc_t(&ptr, 0, 0) it would add a mapping from the MemRegion * for ‘ptr' to the SVal representing the return value from alloc_t. Then, whenever you see a dereference of the MemRegion for ptr you would look up the SVal for it in the map and check to see whether that SVal is definitely not 0. If so, your checker would emit a warning. This second approach is more “lazy”: it avoids the upfront case split and instead you check what the analyzer thinks it knows about the return value when ‘ptr' is used.

> When I create a simple stubs for the function I like to track and have it either return 0 or return 1, I can get it to work. I get the return value of the function and create a new SVal, and have it check if its 0 or anything larger then 0 (using evalBinOp).

One thing to note is that the analyzer will “inline” calls to functions if they’re bodies are available in the same translation unit. I suspect that in this case, the analyzer is looking into the bodies of your stubs and knows they always return 0 or 1. One thing you can do for testing is just provide prototypes for your stub functions. Then the analyzer will not have bodies to inline.

Devin
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20151028/4d502812/attachment.html>