<br><br><div class="gmail_quote">2009/3/30 Ben Laurie <span dir="ltr"><<a href="mailto:benl@google.com">benl@google.com</a>></span><br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<div><div></div><div class="h5">On Fri, Feb 27, 2009 at 6:02 AM, Ted Kremenek <<a href="mailto:kremenek@apple.com">kremenek@apple.com</a>> wrote:<br>
><br>
> On Feb 26, 2009, at 2:18 AM, Ben Laurie wrote:<br>
><br>
>> On Wed, Feb 25, 2009 at 5:27 PM, Ted Kremenek <<a href="mailto:kremenek@apple.com">kremenek@apple.com</a>> wrote:<br>
>>><br>
>>> On Feb 25, 2009, at 8:56 AM, Ben Laurie wrote:<br>
>>><br>
>>>> I'm interested in looking at detecting "known bad" patterns, for<br>
>>>> example:<br>
>>>><br>
>>>> (<expr> & 0) == 0 (this example is stolen from FindBugs)<br>
><br>
> Quick question. Did you literally want to look for this syntax in the AST<br>
> (i.e., the integer literal '0') or did you want to use the dataflow analysis<br>
> to see cases when an expression evaluates to '0' and then is bitwise-ANDed<br>
> with <expr>? Is the motivation of this check is that it is trivially true?<br>
><br>
>>>> if(<non-boolean value>) (cause of recent OpenSSL vuln)<br>
><br>
> This seems a little too vague to me. People check non-boolean values all<br>
> the time in 'if' conditions. Were you thinking about cases when <non<br>
> boolean value> was negative? Positive numbers seem completely legit to me<br>
> (e.g., some flag was set in a bitmask, etc.).<br>
><br>
> In either case, both of these seem like "auditor" checks. For the first<br>
> check if all you wanted to do is grep the syntax, we can easily add another<br>
> check that gets called by AnalysisConsumer. Check out the functions<br>
> "ActionXXX" in AnalysisConsumer.cpp to see all the places where the various<br>
> checks are called. An ActionXXX can get called for each function, an entire<br>
> Objective-C class, or the entire translation unit. For example:<br>
><br>
> static void ActionWarnDeadStores(AnalysisManager& mgr) {<br>
> if (LiveVariables* L = mgr.getLiveVariables()) {<br>
> BugReporter BR(mgr);<br>
> CheckDeadStores(*L, BR);<br>
> }<br>
> }<br>
><br>
> This action gets called for each function/method declaration. The action<br>
> queries the AnalysisManager (the thing that runs the checks) for the live<br>
> variables analysis result on the current function (which gets computed<br>
> on-the-fly). The function 'CheckDeadStores' is then called with (a) the<br>
> results of the live variables analysis and (b) and BugReporter object which<br>
> is used to issue bug diagnostics to the user. Analyses get registered in<br>
> the Analyses.def file, where the "scope" of the analysis is declaratively<br>
> specified. For example:<br>
><br>
> ANALYSIS(WarnDeadStores, "warn-dead-stores",<br>
> "Warn about stores to dead variables", Code)<br>
><br>
> Here "Code" means "code declaration"; that is a FunctionDecl or an<br>
> ObjCMethodDecl.<br>
><br>
> A syntactic check can just walk the AST. A good example of a syntactic<br>
> check is CheckObjCDealloc.cpp. Although this check will one day be revamped<br>
> to do more real analysis, you can see how it walks the AST using iterators<br>
> and does various pattern matching. Syntactic checks are fairly easy to<br>
> implement in this way and hook up to AnalysisConsumer. The down-side is<br>
> that they can be fairly dumb if the property you are checking requires any<br>
> real smarts.<br>
><br>
> Another possible way to implement an "auditor" is to use the GRAuditor<br>
> interface defined in GRAuditor.h:<br>
><br>
> template <typename STATE><br>
> class GRAuditor {<br>
> public:<br>
> typedef ExplodedNode<STATE> NodeTy;<br>
> typedef typename STATE::ManagerTy ManagerTy;<br>
><br>
> virtual ~GRAuditor() {}<br>
> virtual bool Audit(NodeTy* N, ManagerTy& M) = 0;<br>
> };<br>
><br>
> Checks that subclass GRAuditor<const GRState*> can be registered with a<br>
> GRExprEngine and have their "Audit" method called after the path engine<br>
> evaluates all expressions of a given type. The Auditor is then free to<br>
> inspect the analysis state at that point and flag errors. A good example of<br>
> this kind of check is in BasicObjCFoundationChecks.cpp. The class in<br>
> question is AuditCFNumberCreate, which flags misuses of the function<br>
> CFNumberCreate<br>
> (<a href="http://developer.apple.com/documentation/CoreFOundation/Reference/CFNumberRef/Reference/reference.html" target="_blank">http://developer.apple.com/documentation/CoreFOundation/Reference/CFNumberRef/Reference/reference.html</a>).<br>
> That function just interposes on the path engine at every CallExpr and<br>
> checks the arguments in the function call for illegal values.<br>
><br>
> This is probably enough for one email. I'm more than happy to dig into more<br>
> details depending on what direction you decide to go in. Also, these<br>
> interfaces are obviously evolving. We haven't written many checks, so any<br>
> idea of how to refactor the interface, make it better, etc., are welcome.<br>
<br>
</div></div>I found some time to take a look at this today, and I'm puzzled: I<br>
can't see where auditors choose what type they are interested in?</blockquote><div><br>Is this what you are looking for:<br><br>GRExprEngineInternalChecks.cpp:470:<br> AddCheck(new CheckAttrNonNull(BR), Stmt::CallExprClass);</div>
</div><br>