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).

llvm-svn: 70885
This commit is contained in:
Ted Kremenek 2009-05-04 17:53:11 +00:00
parent 5dbfa3fadd
commit 250d59f33f
3 changed files with 27 additions and 66 deletions

View File

@ -249,15 +249,11 @@ SVal GRSimpleVals::EvalBinOp(GRExprEngine& Eng, BinaryOperator::Opcode Op,
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 @@ SVal GRSimpleVals::EvalBinOp(GRExprEngine& Eng, BinaryOperator::Opcode Op,
// 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 @@ SVal GRSimpleVals::EvalEQ(GRExprEngine& Eng, Loc L, Loc R) {
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 @@ SVal GRSimpleVals::EvalEQ(GRExprEngine& Eng, Loc L, Loc R) {
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 @@ SVal GRSimpleVals::EvalEQ(GRExprEngine& Eng, Loc L, Loc R) {
// Fall-through.
case loc::GotoLabelKind:
return NonLoc::MakeIntTruthVal(BasicVals, L == R);
return NonLoc::MakeIntTruthVal(BasicVals, isEqual ? L == R : 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, true);
return NonLoc::MakeIntTruthVal(BasicVals, isEqual ? false : true);
}
//===----------------------------------------------------------------------===//

View File

@ -77,10 +77,8 @@ public:
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

View File

@ -139,9 +139,9 @@ int* f7c(int *x) {
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;
}