[cfe-commits] r61105 - in /cfe/trunk: include/clang/Analysis/PathSensitive/GRExprEngine.h include/clang/Analysis/ProgramPoint.h lib/Analysis/GRExprEngine.cpp
Ted Kremenek
kremenek at apple.com
Tue Dec 16 14:02:30 PST 2008
Author: kremenek
Date: Tue Dec 16 16:02:27 2008
New Revision: 61105
URL: http://llvm.org/viewvc/llvm-project?rev=61105&view=rev
Log:
ProgramPoint:
- Added four new ProgramPoint types that subclass PostStmt for use in
GRExprEngine::EvalLocation:
- PostOutOfBoundsCheckFailed
- PostUndefLocationCheckFailed
- PostNullCheckFailed
- PostLocationChecksSucceed
These were created because of a horribly subtle caching bug in EvalLocation
where a node representing an "bug condition" in EvalLocation (e.g. a null
dereference) could be re-used as the "non-bug condition" because the Store did
not contain any information to differentiate between the two. The extra
program points just disables any accidental caching between EvalLocation and
its callers.
GRExprEngine:
- EvalLocation now returns a NodeTy* instead of GRState*. This should be used as the "vetted" predecessor for EvalLoad/EvalStore.
Modified:
cfe/trunk/include/clang/Analysis/PathSensitive/GRExprEngine.h
cfe/trunk/include/clang/Analysis/ProgramPoint.h
cfe/trunk/lib/Analysis/GRExprEngine.cpp
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=61105&r1=61104&r2=61105&view=diff
==============================================================================
--- cfe/trunk/include/clang/Analysis/PathSensitive/GRExprEngine.h (original)
+++ cfe/trunk/include/clang/Analysis/PathSensitive/GRExprEngine.h Tue Dec 16 16:02:27 2008
@@ -686,9 +686,8 @@
void EvalLoad(NodeSet& Dst, Expr* Ex, NodeTy* Pred,
const GRState* St, SVal location, bool CheckOnly = false);
- const GRState* EvalLocation(Stmt* Ex, NodeTy* Pred,
- const GRState* St, SVal location,
- bool isLoad = false);
+ NodeTy* EvalLocation(Stmt* Ex, NodeTy* Pred,
+ const GRState* St, SVal location);
void EvalReturn(NodeSet& Dst, ReturnStmt* s, NodeTy* Pred);
Modified: cfe/trunk/include/clang/Analysis/ProgramPoint.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/ProgramPoint.h?rev=61105&r1=61104&r2=61105&view=diff
==============================================================================
--- cfe/trunk/include/clang/Analysis/ProgramPoint.h (original)
+++ cfe/trunk/include/clang/Analysis/ProgramPoint.h Tue Dec 16 16:02:27 2008
@@ -27,7 +27,12 @@
public:
enum Kind { BlockEdgeKind=0, BlockEntranceKind, BlockExitKind,
// Keep the following four together and in this order.
- PostStmtKind, PostLoadKind, PostStoreKind,
+ PostStmtKind,
+ PostLocationChecksSucceedKind,
+ PostOutOfBoundsCheckFailedKind,
+ PostNullCheckFailedKind,
+ PostUndefLocationCheckFailedKind,
+ PostLoadKind, PostStoreKind,
PostPurgeDeadSymbolsKind };
private:
@@ -144,6 +149,46 @@
return k >= PostStmtKind && k <= PostPurgeDeadSymbolsKind;
}
};
+
+class PostLocationChecksSucceed : public PostStmt {
+public:
+ PostLocationChecksSucceed(const Stmt* S)
+ : PostStmt(S, PostLocationChecksSucceedKind) {}
+
+ static bool classof(const ProgramPoint* Location) {
+ return Location->getKind() == PostLocationChecksSucceedKind;
+ }
+};
+
+class PostOutOfBoundsCheckFailed : public PostStmt {
+public:
+ PostOutOfBoundsCheckFailed(const Stmt* S)
+ : PostStmt(S, PostOutOfBoundsCheckFailedKind) {}
+
+ static bool classof(const ProgramPoint* Location) {
+ return Location->getKind() == PostOutOfBoundsCheckFailedKind;
+ }
+};
+
+class PostUndefLocationCheckFailed : public PostStmt {
+public:
+ PostUndefLocationCheckFailed(const Stmt* S)
+ : PostStmt(S, PostUndefLocationCheckFailedKind) {}
+
+ static bool classof(const ProgramPoint* Location) {
+ return Location->getKind() == PostUndefLocationCheckFailedKind;
+ }
+};
+
+class PostNullCheckFailed : public PostStmt {
+public:
+ PostNullCheckFailed(const Stmt* S)
+ : PostStmt(S, PostNullCheckFailedKind) {}
+
+ static bool classof(const ProgramPoint* Location) {
+ return Location->getKind() == PostNullCheckFailedKind;
+ }
+};
class PostLoad : public PostStmt {
public:
@@ -156,7 +201,7 @@
class PostStore : public PostStmt {
public:
- PostStore(const Stmt* S) : PostStmt(S, PostLoadKind) {}
+ PostStore(const Stmt* S) : PostStmt(S, PostStoreKind) {}
static bool classof(const ProgramPoint* Location) {
return Location->getKind() == PostStoreKind;
Modified: cfe/trunk/lib/Analysis/GRExprEngine.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/GRExprEngine.cpp?rev=61105&r1=61104&r2=61105&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/GRExprEngine.cpp (original)
+++ cfe/trunk/lib/Analysis/GRExprEngine.cpp Tue Dec 16 16:02:27 2008
@@ -950,11 +950,13 @@
assert (Builder && "GRStmtNodeBuilder must be defined.");
// Evaluate the location (checks for bad dereferences).
- St = EvalLocation(Ex, Pred, St, location);
+ Pred = EvalLocation(Ex, Pred, St, location);
- if (!St)
+ if (!Pred)
return;
+ St = GetState(Pred);
+
// Proceed with the store.
unsigned size = Dst.size();
@@ -980,13 +982,14 @@
const GRState* St, SVal location,
bool CheckOnly) {
- // Evaluate the location (checks for bad dereferences).
+ // Evaluate the location (checks for bad dereferences).
+ Pred = EvalLocation(Ex, Pred, St, location);
- St = EvalLocation(Ex, Pred, St, location, true);
-
- if (!St)
+ if (!Pred)
return;
+ St = GetState(Pred);
+
// Proceed with the load.
ProgramPoint::Kind K = ProgramPoint::PostLoadKind;
@@ -997,9 +1000,12 @@
// loads aren't fully implemented. Eventually this option will go away.
assert(!CheckOnly);
- if (CheckOnly)
- MakeNode(Dst, Ex, Pred, St, K);
- else if (location.isUnknown()) {
+ if (CheckOnly) {
+ Dst.Add(Pred);
+ return;
+ }
+
+ if (location.isUnknown()) {
// This is important. We must nuke the old binding.
MakeNode(Dst, Ex, Pred, BindExpr(St, Ex, UnknownVal()), K);
}
@@ -1019,26 +1025,27 @@
MakeNode(Dst, Ex, *I, (*I)->getState());
}
-const GRState* GRExprEngine::EvalLocation(Stmt* Ex, NodeTy* Pred,
- const GRState* St,
- SVal location, bool isLoad) {
+GRExprEngine::NodeTy* GRExprEngine::EvalLocation(Stmt* Ex, NodeTy* Pred,
+ const GRState* St,
+ SVal location) {
// Check for loads/stores from/to undefined values.
if (location.isUndef()) {
- ProgramPoint::Kind K =
- isLoad ? ProgramPoint::PostLoadKind : ProgramPoint::PostStmtKind;
-
- if (NodeTy* Succ = Builder->generateNode(Ex, St, Pred, K)) {
- Succ->markAsSink();
- UndefDeref.insert(Succ);
+ NodeTy* N =
+ Builder->generateNode(Ex, St, Pred,
+ ProgramPoint::PostUndefLocationCheckFailedKind);
+
+ if (N) {
+ N->markAsSink();
+ UndefDeref.insert(N);
}
- return NULL;
+ return 0;
}
// Check for loads/stores from/to unknown locations. Treat as No-Ops.
if (location.isUnknown())
- return St;
+ return Pred;
// During a load, one of two possible situations arise:
// (1) A crash, because the location (pointer) was NULL.
@@ -1067,12 +1074,10 @@
// We don't use "MakeNode" here because the node will be a sink
// and we have no intention of processing it later.
-
- ProgramPoint::Kind K =
- isLoad ? ProgramPoint::PostLoadKind : ProgramPoint::PostStmtKind;
+ NodeTy* NullNode =
+ Builder->generateNode(Ex, StNull, Pred,
+ ProgramPoint::PostNullCheckFailedKind);
- NodeTy* NullNode = Builder->generateNode(Ex, StNull, Pred, K);
-
if (NullNode) {
NullNode->markAsSink();
@@ -1081,9 +1086,12 @@
else ExplicitNullDeref.insert(NullNode);
}
}
+
+ if (!isFeasibleNotNull)
+ return 0;
// Check for out-of-bound array access.
- if (isFeasibleNotNull && isa<loc::MemRegionVal>(LV)) {
+ if (isa<loc::MemRegionVal>(LV)) {
const MemRegion* R = cast<loc::MemRegionVal>(LV).getRegion();
if (const ElementRegion* ER = dyn_cast<ElementRegion>(R)) {
// Get the index of the accessed element.
@@ -1101,13 +1109,10 @@
false, isFeasibleOutBound);
if (isFeasibleOutBound) {
- // Report warning.
-
- // Make sink node manually.
- ProgramPoint::Kind K = isLoad ? ProgramPoint::PostLoadKind
- : ProgramPoint::PostStoreKind;
-
- NodeTy* OOBNode = Builder->generateNode(Ex, StOutBound, Pred, K);
+ // Report warning. Make sink node manually.
+ NodeTy* OOBNode =
+ Builder->generateNode(Ex, StOutBound, Pred,
+ ProgramPoint::PostOutOfBoundsCheckFailedKind);
if (OOBNode) {
OOBNode->markAsSink();
@@ -1119,11 +1124,16 @@
}
}
- return isFeasibleInBound ? StInBound : NULL;
+ if (!isFeasibleInBound)
+ return 0;
+
+ StNotNull = StInBound;
}
}
- return isFeasibleNotNull ? StNotNull : NULL;
+ // Generate a new node indicating the checks succeed.
+ return Builder->generateNode(Ex, StNotNull, Pred,
+ ProgramPoint::PostLocationChecksSucceedKind);
}
//===----------------------------------------------------------------------===//
@@ -1444,12 +1454,12 @@
// Get the current state. Use 'EvalLocation' to determine if it is a null
// pointer, etc.
Stmt* elem = S->getElement();
- GRStateRef state = GRStateRef(EvalLocation(elem, Pred, GetState(Pred),
- ElementV, false),
- getStateManager());
- if (!state)
+ Pred = EvalLocation(elem, Pred, GetState(Pred), ElementV);
+ if (!Pred)
return;
+
+ GRStateRef state = GRStateRef(GetState(Pred), getStateManager());
// Handle the case where the container still has elements.
QualType IntTy = getContext().IntTy;
@@ -2710,49 +2720,47 @@
assert (false);
break;
- case ProgramPoint::PostLoadKind:
- case ProgramPoint::PostPurgeDeadSymbolsKind:
- case ProgramPoint::PostStmtKind: {
- const PostStmt& L = cast<PostStmt>(Loc);
- Stmt* S = L.getStmt();
- SourceLocation SLoc = S->getLocStart();
-
- Out << S->getStmtClassName() << ' ' << (void*) S << ' ';
- llvm::raw_os_ostream OutS(Out);
- S->printPretty(OutS);
- OutS.flush();
-
- if (SLoc.isFileID()) {
- Out << "\\lline="
- << GraphPrintSourceManager->getLineNumber(SLoc) << " col="
- << GraphPrintSourceManager->getColumnNumber(SLoc) << "\\l";
- }
-
- if (GraphPrintCheckerState->isImplicitNullDeref(N))
- Out << "\\|Implicit-Null Dereference.\\l";
- else if (GraphPrintCheckerState->isExplicitNullDeref(N))
- Out << "\\|Explicit-Null Dereference.\\l";
- else if (GraphPrintCheckerState->isUndefDeref(N))
- Out << "\\|Dereference of undefialied value.\\l";
- else if (GraphPrintCheckerState->isUndefStore(N))
- Out << "\\|Store to Undefined Loc.";
- else if (GraphPrintCheckerState->isExplicitBadDivide(N))
- Out << "\\|Explicit divide-by zero or undefined value.";
- else if (GraphPrintCheckerState->isImplicitBadDivide(N))
- Out << "\\|Implicit divide-by zero or undefined value.";
- else if (GraphPrintCheckerState->isUndefResult(N))
- Out << "\\|Result of operation is undefined.";
- else if (GraphPrintCheckerState->isNoReturnCall(N))
- Out << "\\|Call to function marked \"noreturn\".";
- else if (GraphPrintCheckerState->isBadCall(N))
- Out << "\\|Call to NULL/Undefined.";
- else if (GraphPrintCheckerState->isUndefArg(N))
- Out << "\\|Argument in call is undefined";
-
- break;
- }
-
default: {
+ if (isa<PostStmt>(Loc)) {
+ const PostStmt& L = cast<PostStmt>(Loc);
+ Stmt* S = L.getStmt();
+ SourceLocation SLoc = S->getLocStart();
+
+ Out << S->getStmtClassName() << ' ' << (void*) S << ' ';
+ llvm::raw_os_ostream OutS(Out);
+ S->printPretty(OutS);
+ OutS.flush();
+
+ if (SLoc.isFileID()) {
+ Out << "\\lline="
+ << GraphPrintSourceManager->getLineNumber(SLoc) << " col="
+ << GraphPrintSourceManager->getColumnNumber(SLoc) << "\\l";
+ }
+
+ if (GraphPrintCheckerState->isImplicitNullDeref(N))
+ Out << "\\|Implicit-Null Dereference.\\l";
+ else if (GraphPrintCheckerState->isExplicitNullDeref(N))
+ Out << "\\|Explicit-Null Dereference.\\l";
+ else if (GraphPrintCheckerState->isUndefDeref(N))
+ Out << "\\|Dereference of undefialied value.\\l";
+ else if (GraphPrintCheckerState->isUndefStore(N))
+ Out << "\\|Store to Undefined Loc.";
+ else if (GraphPrintCheckerState->isExplicitBadDivide(N))
+ Out << "\\|Explicit divide-by zero or undefined value.";
+ else if (GraphPrintCheckerState->isImplicitBadDivide(N))
+ Out << "\\|Implicit divide-by zero or undefined value.";
+ else if (GraphPrintCheckerState->isUndefResult(N))
+ Out << "\\|Result of operation is undefined.";
+ else if (GraphPrintCheckerState->isNoReturnCall(N))
+ Out << "\\|Call to function marked \"noreturn\".";
+ else if (GraphPrintCheckerState->isBadCall(N))
+ Out << "\\|Call to NULL/Undefined.";
+ else if (GraphPrintCheckerState->isUndefArg(N))
+ Out << "\\|Argument in call is undefined";
+
+ break;
+ }
+
const BlockEdge& E = cast<BlockEdge>(Loc);
Out << "Edge: (B" << E.getSrc()->getBlockID() << ", B"
<< E.getDst()->getBlockID() << ')';
More information about the cfe-commits
mailing list