[cfe-commits] r70885 - in /cfe/trunk: lib/Analysis/GRSimpleVals.cpp lib/Analysis/GRSimpleVals.h test/Analysis/null-deref-ps.c
Ted Kremenek
kremenek at apple.com
Mon May 4 10:53:12 PDT 2009
Author: kremenek
Date: Mon May 4 12:53:11 2009
New Revision: 70885
URL: http://llvm.org/viewvc/llvm-project?rev=70885&view=rev
Log:
Fix false positive null dereference by unifying code paths in GRSimpleVals for
'==' and '!=' (some code in the '!=' was not replicated in the '==' code,
causing some constraints to get lost).
Modified:
cfe/trunk/lib/Analysis/GRSimpleVals.cpp
cfe/trunk/lib/Analysis/GRSimpleVals.h
cfe/trunk/test/Analysis/null-deref-ps.c
Modified: cfe/trunk/lib/Analysis/GRSimpleVals.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/GRSimpleVals.cpp?rev=70885&r1=70884&r2=70885&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/GRSimpleVals.cpp (original)
+++ cfe/trunk/lib/Analysis/GRSimpleVals.cpp Mon May 4 12:53:11 2009
@@ -249,15 +249,11 @@
Loc L, Loc R) {
switch (Op) {
-
default:
- return UnknownVal();
-
+ return UnknownVal();
case BinaryOperator::EQ:
- return EvalEQ(Eng, L, R);
-
case BinaryOperator::NE:
- return EvalNE(Eng, L, R);
+ return EvalEquality(Eng, L, R, Op == BinaryOperator::EQ);
}
}
@@ -286,14 +282,14 @@
// FIXME: All this logic will be revamped when we have MemRegion::getLocation()
// implemented.
-SVal GRSimpleVals::EvalEQ(GRExprEngine& Eng, Loc L, Loc R) {
+SVal GRSimpleVals::EvalEquality(GRExprEngine& Eng, Loc L, Loc R, bool isEqual) {
BasicValueFactory& BasicVals = Eng.getBasicVals();
switch (L.getSubKind()) {
default:
- assert(false && "EQ not implemented for this Loc.");
+ assert(false && "EQ/NE not implemented for this Loc.");
return UnknownVal();
case loc::ConcreteIntKind:
@@ -302,8 +298,21 @@
bool b = cast<loc::ConcreteInt>(L).getValue() ==
cast<loc::ConcreteInt>(R).getValue();
+ // Are we computing '!='? Flip the result.
+ if (!isEqual)
+ b = !b;
+
return NonLoc::MakeIntTruthVal(BasicVals, b);
}
+ else if (SymbolRef Sym = R.getAsSymbol()) {
+ const SymIntExpr * SE =
+ Eng.getSymbolManager().getSymIntExpr(Sym,
+ isEqual ? BinaryOperator::EQ
+ : BinaryOperator::NE,
+ cast<loc::ConcreteInt>(L).getValue(),
+ Eng.getContext().IntTy);
+ return nonloc::SymExprVal(SE);
+ }
break;
@@ -311,7 +320,9 @@
if (SymbolRef LSym = L.getAsLocSymbol()) {
if (isa<loc::ConcreteInt>(R)) {
const SymIntExpr *SE =
- Eng.getSymbolManager().getSymIntExpr(LSym, BinaryOperator::EQ,
+ Eng.getSymbolManager().getSymIntExpr(LSym,
+ isEqual ? BinaryOperator::EQ
+ : BinaryOperator::NE,
cast<loc::ConcreteInt>(R).getValue(),
Eng.getContext().IntTy);
@@ -323,58 +334,10 @@
// Fall-through.
case loc::GotoLabelKind:
- return NonLoc::MakeIntTruthVal(BasicVals, L == R);
- }
-
- return NonLoc::MakeIntTruthVal(BasicVals, false);
-}
-
-SVal GRSimpleVals::EvalNE(GRExprEngine& Eng, Loc L, Loc R) {
-
- BasicValueFactory& BasicVals = Eng.getBasicVals();
-
- switch (L.getSubKind()) {
-
- default:
- assert(false && "NE not implemented for this Loc.");
- return UnknownVal();
-
- case loc::ConcreteIntKind:
-
- if (isa<loc::ConcreteInt>(R)) {
- bool b = cast<loc::ConcreteInt>(L).getValue() !=
- cast<loc::ConcreteInt>(R).getValue();
-
- return NonLoc::MakeIntTruthVal(BasicVals, b);
- }
- else if (SymbolRef Sym = R.getAsSymbol()) {
- const SymIntExpr * SE =
- Eng.getSymbolManager().getSymIntExpr(Sym, BinaryOperator::NE,
- cast<loc::ConcreteInt>(L).getValue(),
- Eng.getContext().IntTy);
- return nonloc::SymExprVal(SE);
- }
-
- break;
-
- case loc::MemRegionKind: {
- if (SymbolRef LSym = L.getAsLocSymbol()) {
- if (isa<loc::ConcreteInt>(R)) {
- const SymIntExpr* SE =
- Eng.getSymbolManager().getSymIntExpr(LSym, BinaryOperator::NE,
- cast<loc::ConcreteInt>(R).getValue(),
- Eng.getContext().IntTy);
- return nonloc::SymExprVal(SE);
- }
- }
- // Fall through:
- }
-
- case loc::GotoLabelKind:
- return NonLoc::MakeIntTruthVal(BasicVals, L != R);
+ return NonLoc::MakeIntTruthVal(BasicVals, isEqual ? L == R : L != R);
}
- return NonLoc::MakeIntTruthVal(BasicVals, true);
+ return NonLoc::MakeIntTruthVal(BasicVals, isEqual ? false : true);
}
//===----------------------------------------------------------------------===//
Modified: cfe/trunk/lib/Analysis/GRSimpleVals.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/GRSimpleVals.h?rev=70885&r1=70884&r2=70885&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/GRSimpleVals.h (original)
+++ cfe/trunk/lib/Analysis/GRSimpleVals.h Mon May 4 12:53:11 2009
@@ -77,10 +77,8 @@
protected:
- // Equality operators for Locs.
-
- SVal EvalEQ(GRExprEngine& Engine, Loc L, Loc R);
- SVal EvalNE(GRExprEngine& Engine, Loc L, Loc R);
+ // Equality (==, !=) operators for Locs.
+ SVal EvalEquality(GRExprEngine& Engine, Loc L, Loc R, bool isEqual);
};
} // end clang namespace
Modified: cfe/trunk/test/Analysis/null-deref-ps.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/null-deref-ps.c?rev=70885&r1=70884&r2=70885&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/null-deref-ps.c (original)
+++ cfe/trunk/test/Analysis/null-deref-ps.c Mon May 4 12:53:11 2009
@@ -139,9 +139,9 @@
if (((void*)0) != x)
return x;
-
- // THIS IS WRONG. THIS NEEDS TO BE FIXED.
- *p = 1; // expected-warning{{null}}
+
+ // If we reach here then 'p' is not null.
+ *p = 1; // no-warning
return x;
}
More information about the cfe-commits
mailing list