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 <[email protected]> wrote: >> On Wed, Jul 18, 2012 at 9:59 PM, Ted Kremenek <[email protected]> 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 _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
