[cfe-commits] r160494 - /cfe/trunk/lib/Analysis/UninitializedValues.cpp

Richard Smith richard at metafoo.co.uk
Tue Jul 24 18:12:05 PDT 2012


On Tue, Jul 24, 2012 at 5:54 PM, Jordan Rose <jordan_rose at apple.com> wrote:

> On Jul 24, 2012, at 5:25 PM, Jordan Rose wrote:
> > On Jul 24, 2012, at 2:04 PM, Richard Smith wrote:
> >> On Fri, Jul 20, 2012 at 4:03 PM, Richard Smith <richard at metafoo.co.uk>
> wrote:
> >> On Wed, Jul 18, 2012 at 9:59 PM, Ted Kremenek <kremenek at apple.com>
> wrote:
> >> Author: kremenek
> >> Date: Wed Jul 18 23:59:05 2012
> >> New Revision: 160494
> >>
> >> URL: http://llvm.org/viewvc/llvm-project?rev=160494&view=rev
> >> Log:
> >> Simplify UninitializedValues.cpp by removing logic to handle the
> previous (imprecise) representation
> >> of '&&' and '||' in the CFG.  This is no longer needed
> >>
> >> Sadly, that appears to be untrue. We now produce a bogus
> -Wuninitialized warning on this:
> >>
> >> int x(int*); int f(bool b) { int n = (b || x(&n)) ? 0 : n; return n; }
> >>
> >> More generally, && and || as the LHS of a ?: still produce a CFG with
> false edges.
> >>
> >> Fixed in r160691.
> >
> > This broke one of our internal buildbots for the static analyzer, on
> precisely this case (|| and ?:) in ctype.h. Here's a simplified test case:
> >
> > // clang -cc1 -analyze -analyzer-checker=core -x c
> > int isctype(char c, unsigned long f)
> > {
> >        return (c < 1 || c > 10) ? 0 : !!(c & f);
> > }
> >
> > This is the assertion:
> >
> >> Assertion failed: (X.isUndef()), function VisitGuardedExpr, file
> ExprEngineC.cpp, line 597.
> >
> > I don't remember /why/ we have to pass the decision Expr through an
> UndefinedVal here, but we're clearly confused by this. I think we're still
> expecting to see the || in the CFG before the ?:, and with this change that
> doesn't seem to be the case anymore.
>
> More info: when the analyzer sees a ||, it walks back through the path to
> find out if we took the true path or the false path, and then uses that to
> decide which expression to use as the branch value. ?: expects to get that
> answer from its condition, but we aren't evaluating the || anymore.
>
> I'm not sure what's the right thing to do. This CFG is /mostly/ equivalent
> to the previous ones, and it is more efficient...but it means that clients
> have to reason about branches with short-circuit conditions specially. It's
> just not an issue in the analyzer for 'if' and friends because those are
> top-level statements -- they don't need to be visited for a value as well
> as a branch.
>
> Ted probably has some guidance here.
>

I don't know enough about the static analyzer to formulate a fix here. If
this is causing you an immediate problem, please feel free to revert both
r160691 and r160494 (Ted's change caused a -Werror build break for us).
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120724/145fbdd4/attachment.html>


More information about the cfe-commits mailing list