maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

Fix: <rdar://problem/7275774> Static analyzer warns about NULL pointer when 

adding assert

This fix required a few changes:

SimpleSValuator:
- Eagerly replace a symbolic value with its constant value in EvalBinOpNN
  when it is constrained to a constant.  This allows us to better constant fold
  values along a path.
- Handle trivial case of '<', '>' comparison of pointers when the two pointers
  are exactly the same.

RegionStoreManager:

llvm-svn: 83358
This commit is contained in:
Ted Kremenek 2009-10-06 01:39:48 +00:00
parent c3031a9643
commit 8ec5771dcb
8 changed files with 77 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
clang/include/clang/Analysis/PathSensitive/GRExprEngine.h
View File

@ -548,12 +548,14 @@ protected:
public:
SVal EvalBinOp(BinaryOperator::Opcode op, NonLoc L, NonLoc R, QualType T) {
return SVator.EvalBinOpNN(op, L, R, T);
SVal EvalBinOp(const GRState *state, BinaryOperator::Opcode op,
NonLoc L, NonLoc R, QualType T) {
return SVator.EvalBinOpNN(state, op, L, R, T);
}
SVal EvalBinOp(BinaryOperator::Opcode op, NonLoc L, SVal R, QualType T) {
return R.isValid() ? SVator.EvalBinOpNN(op, L, cast<NonLoc>(R), T) : R;
SVal EvalBinOp(const GRState *state, BinaryOperator::Opcode op,
NonLoc L, SVal R, QualType T) {
return R.isValid() ? SVator.EvalBinOpNN(state, op, L, cast<NonLoc>(R), T) : R;
}
SVal EvalBinOp(const GRState *ST, BinaryOperator::Opcode Op,

6
clang/include/clang/Analysis/PathSensitive/GRState.h
View File

@ -259,6 +259,8 @@ public:
SVal getSVal(const MemRegion* R) const;
SVal getSValAsScalarOrLoc(const MemRegion *R) const;
const llvm::APSInt *getSymVal(SymbolRef sym);
bool scanReachableSymbols(SVal val, SymbolVisitor& visitor) const;
@ -566,6 +568,10 @@ public:
// Out-of-line method definitions for GRState.
//===----------------------------------------------------------------------===//
inline const llvm::APSInt *GRState::getSymVal(SymbolRef sym) {
return getStateManager().getSymVal(this, sym);
}
inline const VarRegion* GRState::getRegion(const VarDecl *D,
const LocationContext *LC) const {
return getStateManager().getRegionManager().getVarRegion(D, LC);

4
clang/include/clang/Analysis/PathSensitive/SValuator.h
View File

@ -67,8 +67,8 @@ public:
virtual SVal EvalComplement(NonLoc val) = 0;
virtual SVal EvalBinOpNN(BinaryOperator::Opcode Op, NonLoc lhs,
NonLoc rhs, QualType resultTy) = 0;
virtual SVal EvalBinOpNN(const GRState *state, BinaryOperator::Opcode Op,
NonLoc lhs, NonLoc rhs, QualType resultTy) = 0;
virtual SVal EvalBinOpLL(BinaryOperator::Opcode Op, Loc lhs, Loc rhs,
QualType resultTy) = 0;

2
clang/lib/Analysis/GRExprEngine.cpp
View File

@ -2545,7 +2545,7 @@ void GRExprEngine::VisitUnaryOperator(UnaryOperator* U, ExplodedNode* Pred,
}
else {
nonloc::ConcreteInt X(getBasicVals().getValue(0, Ex->getType()));
Result = EvalBinOp(BinaryOperator::EQ, cast<NonLoc>(V), X,
Result = EvalBinOp(state, BinaryOperator::EQ, cast<NonLoc>(V), X,
U->getType());
}

33
clang/lib/Analysis/RegionStore.cpp
View File

@ -826,7 +826,10 @@ SVal RegionStoreManager::EvalBinOp(const GRState *state,
// Not yet handled.
case MemRegion::VarRegionKind:
case MemRegion::StringRegionKind:
case MemRegion::StringRegionKind: {
}
// Fall-through.
case MemRegion::CompoundLiteralRegionKind:
case MemRegion::FieldRegionKind:
case MemRegion::ObjCObjectRegionKind:
@ -851,17 +854,27 @@ SVal RegionStoreManager::EvalBinOp(const GRState *state,
SVal Idx = ER->getIndex();
nonloc::ConcreteInt* Base = dyn_cast<nonloc::ConcreteInt>(&Idx);
nonloc::ConcreteInt* Offset = dyn_cast<nonloc::ConcreteInt>(&R);
// Only support concrete integer indexes for now.
if (Base && Offset) {
// FIXME: Should use SValuator here.
SVal NewIdx = Base->evalBinOp(ValMgr, Op,
// For now, only support:
// (a) concrete integer indices that can easily be resolved
// (b) 0 + symbolic index
if (Base) {
if (nonloc::ConcreteInt *Offset = dyn_cast<nonloc::ConcreteInt>(&R)) {
// FIXME: Should use SValuator here.
SVal NewIdx =
Base->evalBinOp(ValMgr, Op,
cast<nonloc::ConcreteInt>(ValMgr.convertToArrayIndex(*Offset)));
const MemRegion* NewER =
MRMgr.getElementRegion(ER->getElementType(), NewIdx, ER->getSuperRegion(),
getContext());
return ValMgr.makeLoc(NewER);
const MemRegion* NewER =
MRMgr.getElementRegion(ER->getElementType(), NewIdx,
ER->getSuperRegion(), getContext());
return ValMgr.makeLoc(NewER);
}
if (0 == Base->getValue()) {
const MemRegion* NewER =
MRMgr.getElementRegion(ER->getElementType(), R,
ER->getSuperRegion(), getContext());
return ValMgr.makeLoc(NewER);
}
}
return UnknownVal();

2
clang/lib/Analysis/SValuator.cpp
View File

@ -43,7 +43,7 @@ SVal SValuator::EvalBinOp(const GRState *ST, BinaryOperator::Opcode Op,
return EvalBinOpLN(ST, Op, cast<Loc>(R), cast<NonLoc>(L), T);
}
return EvalBinOpNN(Op, cast<NonLoc>(L), cast<NonLoc>(R), T);
return EvalBinOpNN(ST, Op, cast<NonLoc>(L), cast<NonLoc>(R), T);
}
DefinedOrUnknownSVal SValuator::EvalEQ(const GRState *ST,

26
clang/lib/Analysis/SimpleSValuator.cpp
View File

@ -29,8 +29,8 @@ public:
virtual SVal EvalMinus(NonLoc val);
virtual SVal EvalComplement(NonLoc val);
virtual SVal EvalBinOpNN(BinaryOperator::Opcode op, NonLoc lhs, NonLoc rhs,
QualType resultTy);
virtual SVal EvalBinOpNN(const GRState *state, BinaryOperator::Opcode op,
NonLoc lhs, NonLoc rhs, QualType resultTy);
virtual SVal EvalBinOpLL(BinaryOperator::Opcode op, Loc lhs, Loc rhs,
QualType resultTy);
virtual SVal EvalBinOpLN(const GRState *state, BinaryOperator::Opcode op,
@ -206,7 +206,8 @@ static SVal EvalEquality(ValueManager &ValMgr, Loc lhs, Loc rhs, bool isEqual,
return ValMgr.makeTruthVal(isEqual ? lhs == rhs : lhs != rhs, resultTy);
}
SVal SimpleSValuator::EvalBinOpNN(BinaryOperator::Opcode op,
SVal SimpleSValuator::EvalBinOpNN(const GRState *state,
BinaryOperator::Opcode op,
NonLoc lhs, NonLoc rhs,
QualType resultTy) {
// Handle trivial case where left-side and right-side are the same.
@ -342,8 +343,18 @@ SVal SimpleSValuator::EvalBinOpNN(BinaryOperator::Opcode op,
}
}
case nonloc::SymbolValKind: {
nonloc::SymbolVal *slhs = cast<nonloc::SymbolVal>(&lhs);
SymbolRef Sym = slhs->getSymbol();
// Does the symbol simplify to a constant?
if (Sym->getType(ValMgr.getContext())->isIntegerType())
if (const llvm::APSInt *Constant = state->getSymVal(Sym)) {
lhs = nonloc::ConcreteInt(*Constant);
continue;
}
if (isa<nonloc::ConcreteInt>(rhs)) {
return ValMgr.makeNonLoc(cast<nonloc::SymbolVal>(lhs).getSymbol(), op,
return ValMgr.makeNonLoc(slhs->getSymbol(), op,
cast<nonloc::ConcreteInt>(rhs).getValue(),
resultTy);
}
@ -362,6 +373,13 @@ SVal SimpleSValuator::EvalBinOpLL(BinaryOperator::Opcode op, Loc lhs, Loc rhs,
case BinaryOperator::EQ:
case BinaryOperator::NE:
return EvalEquality(ValMgr, lhs, rhs, op == BinaryOperator::EQ, resultTy);
case BinaryOperator::LT:
case BinaryOperator::GT:
// FIXME: Generalize. For now, just handle the trivial case where
// the two locations are identical.
if (lhs == rhs)
return ValMgr.makeTruthVal(false, resultTy);
return UnknownVal();
}
}

16
clang/test/Analysis/misc-ps-region-store.m
View File

@ -287,3 +287,19 @@ int rdar_7261075(void) {
return 0;
}
// <rdar://problem/7275774> false path due to limited pointer
// arithmetic constraints
void rdar_7275774(void *data, unsigned n) {
if (!(data || n == 0))
return;
unsigned short *p = (unsigned short*) data;
unsigned short *q = p + (n / 2);
if (p < q) {
// If we reach here, 'p' cannot be null. If 'p' is null, then 'n' must
// be '0', meaning that this branch is not feasible.
*p = *q; // no-warning
}
}