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

[robbinson defau via cfe-dev]

Devin,

Thank you for the information this will most definitely allow me to move
forward, thank you.


/DF


On Wed, Oct 28, 2015 at 10:51 PM, Devin Coughlin <dcoughlin at apple.com>
wrote:

>
> 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> 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/20151029/5790a0a3/attachment.html>