Hi, Matthew. In pointer expressions like "p != 0", the 0 gets converted to a pointer before the comparison, so that goes through evalBinOpLL(). Do you have any solid examples where this is failing?
Jordan On Jun 22, 2013, at 18:49 , Matthew Dempsky <[email protected]> wrote: > evalBinOpLN() has a special case for detection that the RHS is zero, > in which case it simply returns the LHS. This works for arithmetic > like "p + 0" and "p - 0", but it's wrong for comparisons like "p != 0" > and "p == 0". > > For one, it means the returned SVal expression has the wrong type > sometimes (pointer instead of bool), but it leads to other issues too. > E.g., evalEQ(St, evalEQ(St, P, Null), evalEQ(St, Q, Null)) returns an > SVal representing "p == q" instead of "!p == !q". > > I was hoping to make a simple debug.ExprInspection test case like: > > int a, b; > clang_analyzer_eval((&a == 0) == (&b == 0)); // expected-warning{{TRUE}} > > But that passes even without this fix, so I'm not sure how to unit > test this change. Suggestions welcome. > > > Index: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp > =================================================================== > --- clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp (revision > 184494) > +++ clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp (working copy) > @@ -838,10 +838,6 @@ > BinaryOperator::Opcode op, > Loc lhs, NonLoc rhs, QualType resultTy) { > > - // Special case: rhs is a zero constant. > - if (rhs.isZeroConstant()) > - return lhs; > - > // Special case: 'rhs' is an integer that has the same width as a pointer > and > // we are using the integer location in a comparison. Normally this cannot > be > // triggered, but transfer functions like those for > OSCompareAndSwapBarrier32 > @@ -865,6 +861,10 @@ > > // We are dealing with pointer arithmetic. > > + // Special case: rhs is a zero constant. > + if (rhs.isZeroConstant()) > + return lhs; > + > // Handle pointer arithmetic on constant values. > if (Optional<nonloc::ConcreteInt> rhsInt = > rhs.getAs<nonloc::ConcreteInt>()) { > if (Optional<loc::ConcreteInt> lhsInt = lhs.getAs<loc::ConcreteInt>()) { > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
