Sorry to be late to the party guys. We should just fix this in the analyzer. I'll take a look.
On Jul 24, 2012, at 6:12 PM, Richard Smith <[email protected]> wrote: > On Tue, Jul 24, 2012 at 5:54 PM, Jordan Rose <[email protected]> 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 <[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. > > 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).
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
