[cfe-commits] r83358 - in /cfe/trunk: include/clang/Analysis/PathSensitive/GRExprEngine.h include/clang/Analysis/PathSensitive/GRState.h include/clang/Analysis/PathSensitive/SValuator.h lib/Analysis/GRExprEngine.cpp lib/Analysis/RegionStore.cpp lib/Analysis/SValuator.cpp lib/Analysis/SimpleSValuator.cpp test/Analysis/misc-ps-region-store.m
Ted Kremenek
kremenek at apple.com
Mon Oct 5 18:39:49 PDT 2009
Author: kremenek
Date: Mon Oct 5 20:39:48 2009
New Revision: 83358
URL: http://llvm.org/viewvc/llvm-project?rev=83358&view=rev
Log:
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:
Modified:
cfe/trunk/include/clang/Analysis/PathSensitive/GRExprEngine.h
cfe/trunk/include/clang/Analysis/PathSensitive/GRState.h
cfe/trunk/include/clang/Analysis/PathSensitive/SValuator.h
cfe/trunk/lib/Analysis/GRExprEngine.cpp
cfe/trunk/lib/Analysis/RegionStore.cpp
cfe/trunk/lib/Analysis/SValuator.cpp
cfe/trunk/lib/Analysis/SimpleSValuator.cpp
cfe/trunk/test/Analysis/misc-ps-region-store.m
Modified: cfe/trunk/include/clang/Analysis/PathSensitive/GRExprEngine.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/PathSensitive/GRExprEngine.h?rev=83358&r1=83357&r2=83358&view=diff
==============================================================================
--- cfe/trunk/include/clang/Analysis/PathSensitive/GRExprEngine.h (original)
+++ cfe/trunk/include/clang/Analysis/PathSensitive/GRExprEngine.h Mon Oct 5 20:39:48 2009
@@ -548,12 +548,14 @@
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,
Modified: cfe/trunk/include/clang/Analysis/PathSensitive/GRState.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/PathSensitive/GRState.h?rev=83358&r1=83357&r2=83358&view=diff
==============================================================================
--- cfe/trunk/include/clang/Analysis/PathSensitive/GRState.h (original)
+++ cfe/trunk/include/clang/Analysis/PathSensitive/GRState.h Mon Oct 5 20:39:48 2009
@@ -259,6 +259,8 @@
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 @@
// 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);
Modified: cfe/trunk/include/clang/Analysis/PathSensitive/SValuator.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/PathSensitive/SValuator.h?rev=83358&r1=83357&r2=83358&view=diff
==============================================================================
--- cfe/trunk/include/clang/Analysis/PathSensitive/SValuator.h (original)
+++ cfe/trunk/include/clang/Analysis/PathSensitive/SValuator.h Mon Oct 5 20:39:48 2009
@@ -67,8 +67,8 @@
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;
Modified: cfe/trunk/lib/Analysis/GRExprEngine.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/GRExprEngine.cpp?rev=83358&r1=83357&r2=83358&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/GRExprEngine.cpp (original)
+++ cfe/trunk/lib/Analysis/GRExprEngine.cpp Mon Oct 5 20:39:48 2009
@@ -2545,7 +2545,7 @@
}
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());
}
Modified: cfe/trunk/lib/Analysis/RegionStore.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/RegionStore.cpp?rev=83358&r1=83357&r2=83358&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/RegionStore.cpp (original)
+++ cfe/trunk/lib/Analysis/RegionStore.cpp Mon Oct 5 20:39:48 2009
@@ -826,7 +826,10 @@
// 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 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();
Modified: cfe/trunk/lib/Analysis/SValuator.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/SValuator.cpp?rev=83358&r1=83357&r2=83358&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/SValuator.cpp (original)
+++ cfe/trunk/lib/Analysis/SValuator.cpp Mon Oct 5 20:39:48 2009
@@ -43,7 +43,7 @@
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,
Modified: cfe/trunk/lib/Analysis/SimpleSValuator.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/SimpleSValuator.cpp?rev=83358&r1=83357&r2=83358&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/SimpleSValuator.cpp (original)
+++ cfe/trunk/lib/Analysis/SimpleSValuator.cpp Mon Oct 5 20:39:48 2009
@@ -29,8 +29,8 @@
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 @@
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 @@
}
}
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 @@
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();
}
}
Modified: cfe/trunk/test/Analysis/misc-ps-region-store.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/misc-ps-region-store.m?rev=83358&r1=83357&r2=83358&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/misc-ps-region-store.m (original)
+++ cfe/trunk/test/Analysis/misc-ps-region-store.m Mon Oct 5 20:39:48 2009
@@ -287,3 +287,19 @@
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
+ }
+}
+
More information about the cfe-commits
mailing list