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

Jordan Rose jordan_rose at apple.com
Tue Jul 24 17:54:36 PDT 2012


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.

Jordan



More information about the cfe-commits mailing list