[cfe-dev] More static analysis...

[Zhongxing Xu]

2009/3/30 Ben Laurie <benl at google.com>

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


Is this what you are looking for:

GRExprEngineInternalChecks.cpp:470:
    AddCheck(new CheckAttrNonNull(BR), Stmt::CallExprClass);
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20090330/f8934446/attachment.html>