Clean up the Checker API a little more, resolving some hidden bugs
along the way. Important changes:
1) To generate a sink node, use GenerateSink(); GenerateNode() is for
generating regular transitions. This makes the API clearer and also
allows us to use the 'bool' option to GenerateNode() for a different
purpose.
2) GenerateNode() now automatically adds the generated node to the
destination ExplodedNodeSet (autotransition) unless the client
specifies otherwise with a bool flag. Several checkers did not call
'addTransition()' after calling 'GenerateNode()', causing the
simulation path to be prematurely culled when a non-fail stop bug was
encountered.
3) Add variants of GenerateNode()/GenerateSink() that take neither a
Stmt* or a GRState*; most callers of GenerateNode() just pass in the
same Stmt* as provided when the CheckerContext object is created; we
can just use that the majority of the time. This cleanup also allows
us to potentially coelesce the APIs for evaluating branches and
end-of-paths (which currently directly use builders).
4) addTransition() no longer needs to be called except for a few
cases. We now have a variant of addTransition() that takes a
GRState*; this allows one to propagate the updated state without
caring about generating a new node explicitly. This nicely cleaned up
a bunch of cases that called autoTransition() with a bunch of
conditional logic surround the call (that common logic has now been
swallowed up by addTransition() itself).
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@89707 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/lib/Analysis/ArrayBoundChecker.cpp b/lib/Analysis/ArrayBoundChecker.cpp
index 549a22b..3d8b3b3 100644
--- a/lib/Analysis/ArrayBoundChecker.cpp
+++ b/lib/Analysis/ArrayBoundChecker.cpp
@@ -62,8 +62,7 @@
const GRState *StInBound = state->AssumeInBound(Idx, NumElements, true);
const GRState *StOutBound = state->AssumeInBound(Idx, NumElements, false);
if (StOutBound && !StInBound) {
- ExplodedNode *N = C.GenerateNode(S, StOutBound, true);
-
+ ExplodedNode *N = C.GenerateSink(StOutBound);
if (!N)
return;
@@ -80,7 +79,6 @@
new RangedBugReport(*BT, BT->getDescription(), N);
report->addRange(S->getSourceRange());
-
C.EmitReport(report);
}
}
diff --git a/lib/Analysis/AttrNonNullChecker.cpp b/lib/Analysis/AttrNonNullChecker.cpp
index 01e1a1f..8668c75 100644
--- a/lib/Analysis/AttrNonNullChecker.cpp
+++ b/lib/Analysis/AttrNonNullChecker.cpp
@@ -39,7 +39,6 @@
void AttrNonNullChecker::PreVisitCallExpr(CheckerContext &C,
const CallExpr *CE) {
const GRState *state = C.getState();
- const GRState *originalState = state;
// Check if the callee has a 'nonnull' attribute.
SVal X = state->getSVal(CE->getCallee());
@@ -74,7 +73,7 @@
if (stateNull && !stateNotNull) {
// Generate an error node. Check for a null node in case
// we cache out.
- if (ExplodedNode *errorNode = C.GenerateNode(CE, stateNull, true)) {
+ if (ExplodedNode *errorNode = C.GenerateSink(stateNull)) {
// Lazily allocate the BugType object if it hasn't already been
// created. Ownership is transferred to the BugReporter object once
@@ -109,6 +108,5 @@
// If we reach here all of the arguments passed the nonnull check.
// If 'state' has been updated generated a new node.
- if (state != originalState)
- C.addTransition(C.GenerateNode(CE, state));
+ C.addTransition(state);
}
diff --git a/lib/Analysis/BasicObjCFoundationChecks.cpp b/lib/Analysis/BasicObjCFoundationChecks.cpp
index 5a88a7a..424a0b3 100644
--- a/lib/Analysis/BasicObjCFoundationChecks.cpp
+++ b/lib/Analysis/BasicObjCFoundationChecks.cpp
@@ -563,12 +563,11 @@
BT = new APIMisuse("message incorrectly sent to class instead of class "
"instance");
- ExplodedNode *N = C.GenerateNode(ME, C.getState(), false);
+ ExplodedNode *N = C.GenerateNode();
+
if (!N)
return;
- C.addTransition(N);
-
llvm::SmallString<200> buf;
llvm::raw_svector_ostream os(buf);
diff --git a/lib/Analysis/CastToStructChecker.cpp b/lib/Analysis/CastToStructChecker.cpp
index ccd4a33..7c6fc7e 100644
--- a/lib/Analysis/CastToStructChecker.cpp
+++ b/lib/Analysis/CastToStructChecker.cpp
@@ -59,7 +59,7 @@
// Now the cast-to-type is struct pointer, the original type is not void*.
if (!OrigPointeeTy->isRecordType()) {
- if (ExplodedNode *N = C.GenerateNode(CE)) {
+ if (ExplodedNode *N = C.GenerateNode()) {
if (!BT)
BT = new BuiltinBug("Cast from non-struct type to struct type",
"Casting a non-structure type to a structure type "
diff --git a/lib/Analysis/Checker.cpp b/lib/Analysis/Checker.cpp
index 985b1e0..0d907e5 100644
--- a/lib/Analysis/Checker.cpp
+++ b/lib/Analysis/Checker.cpp
@@ -16,3 +16,20 @@
using namespace clang;
Checker::~Checker() {}
+
+CheckerContext::~CheckerContext() {
+ // Do we need to autotransition? 'Dst' can get populated in a variety of
+ // ways, including 'addTransition()' adding the predecessor node to Dst
+ // without actually generated a new node. We also shouldn't autotransition
+ // if we are building sinks or we generated a node and decided to not
+ // add it as a transition.
+ if (Dst.size() == size && !B.BuildSinks && !B.HasGeneratedNode) {
+ if (state && state != B.GetState(Pred)) {
+ static int autoTransitionTag = 0;
+ B.Tag = &autoTransitionTag;
+ addTransition(state);
+ }
+ else
+ Dst.Add(Pred);
+ }
+}
diff --git a/lib/Analysis/DereferenceChecker.cpp b/lib/Analysis/DereferenceChecker.cpp
index a8f5af3..4c4091c 100644
--- a/lib/Analysis/DereferenceChecker.cpp
+++ b/lib/Analysis/DereferenceChecker.cpp
@@ -56,8 +56,7 @@
SVal l) {
// Check for dereference of an undefined value.
if (l.isUndef()) {
- ExplodedNode *N = C.GenerateNode(S, true);
- if (N) {
+ if (ExplodedNode *N = C.GenerateSink()) {
if (!BT_undef)
BT_undef = new BuiltinBug("Dereference of undefined pointer value");
@@ -84,7 +83,7 @@
if (nullState) {
if (!notNullState) {
// Generate an error node.
- ExplodedNode *N = C.GenerateNode(S, nullState, true);
+ ExplodedNode *N = C.GenerateSink(nullState);
if (!N)
return;
@@ -106,13 +105,11 @@
// Otherwise, we have the case where the location could either be
// null or not-null. Record the error node as an "implicit" null
// dereference.
- if (ExplodedNode *N = C.GenerateNode(S, nullState, true))
+ if (ExplodedNode *N = C.GenerateSink(nullState))
ImplicitNullDerefNodes.push_back(N);
}
}
// From this point forward, we know that the location is not null.
- assert(notNullState);
- C.addTransition(state != nullState ? C.GenerateNode(S, notNullState) :
- C.getPredecessor());
+ C.addTransition(notNullState);
}
diff --git a/lib/Analysis/DivZeroChecker.cpp b/lib/Analysis/DivZeroChecker.cpp
index a8630f1..4052637 100644
--- a/lib/Analysis/DivZeroChecker.cpp
+++ b/lib/Analysis/DivZeroChecker.cpp
@@ -63,7 +63,7 @@
llvm::tie(stateNotZero, stateZero) = CM.AssumeDual(C.getState(), *DV);
if (stateZero && !stateNotZero) {
- if (ExplodedNode *N = C.GenerateNode(B, stateZero, true)) {
+ if (ExplodedNode *N = C.GenerateSink(stateZero)) {
if (!BT)
BT = new BuiltinBug("Division by zero");
@@ -80,6 +80,5 @@
// If we get here, then the denom should not be zero. We abandon the implicit
// zero denom case for now.
- if (stateNotZero != C.getState())
- C.addTransition(C.GenerateNode(B, stateNotZero));
+ C.addTransition(stateNotZero);
}
diff --git a/lib/Analysis/FixedAddressChecker.cpp b/lib/Analysis/FixedAddressChecker.cpp
index 80096dc..d8adaaf 100644
--- a/lib/Analysis/FixedAddressChecker.cpp
+++ b/lib/Analysis/FixedAddressChecker.cpp
@@ -53,7 +53,7 @@
if (!RV.isConstant() || RV.isZeroConstant())
return;
- if (ExplodedNode *N = C.GenerateNode(B)) {
+ if (ExplodedNode *N = C.GenerateNode()) {
if (!BT)
BT = new BuiltinBug("Use fixed address",
"Using a fixed address is not portable because that "
diff --git a/lib/Analysis/GRExprEngine.cpp b/lib/Analysis/GRExprEngine.cpp
index 4f3440d..f657aeb 100644
--- a/lib/Analysis/GRExprEngine.cpp
+++ b/lib/Analysis/GRExprEngine.cpp
@@ -1292,9 +1292,13 @@
Checker *checker = I->second;
for (ExplodedNodeSet::iterator NI = PrevSet->begin(), NE = PrevSet->end();
- NI != NE; ++NI)
- checker->GR_VisitLocation(*CurrSet, *Builder, *this, S, *NI, state,
+ NI != NE; ++NI) {
+ // Use the 'state' argument only when the predecessor node is the
+ // same as Pred. This allows us to catch updates to the state.
+ checker->GR_VisitLocation(*CurrSet, *Builder, *this, S, *NI,
+ *NI == Pred ? state : GetState(*NI),
location, tag, isLoad);
+ }
// Update which NodeSet is the current one.
PrevSet = CurrSet;
diff --git a/lib/Analysis/MallocChecker.cpp b/lib/Analysis/MallocChecker.cpp
index 995720b..a16125d 100644
--- a/lib/Analysis/MallocChecker.cpp
+++ b/lib/Analysis/MallocChecker.cpp
@@ -112,9 +112,7 @@
SymbolRef Sym = CallVal.getAsLocSymbol();
assert(Sym);
// Set the symbol's state to Allocated.
- const GRState *AllocState
- = state->set<RegionState>(Sym, RefState::getAllocated(CE));
- C.addTransition(C.GenerateNode(CE, AllocState));
+ C.addTransition(state->set<RegionState>(Sym, RefState::getAllocated(CE)));
}
void MallocChecker::FreeMem(CheckerContext &C, const CallExpr *CE) {
@@ -128,7 +126,7 @@
// Check double free.
if (RS->isReleased()) {
- ExplodedNode *N = C.GenerateNode(CE, true);
+ ExplodedNode *N = C.GenerateSink();
if (N) {
if (!BT_DoubleFree)
BT_DoubleFree = new BuiltinBug("Double free",
@@ -144,7 +142,7 @@
// Normal free.
const GRState *FreedState
= state->set<RegionState>(Sym, RefState::getReleased(CE));
- C.addTransition(C.GenerateNode(CE, FreedState));
+ C.addTransition(FreedState);
}
void MallocChecker::EvalDeadSymbols(CheckerContext &C, const Stmt *S,
@@ -158,7 +156,7 @@
return;
if (RS->isAllocated()) {
- ExplodedNode *N = C.GenerateNode(S, true);
+ ExplodedNode *N = C.GenerateSink();
if (N) {
if (!BT_Leak)
BT_Leak = new BuiltinBug("Memory leak",
@@ -213,7 +211,5 @@
if (RS->isAllocated())
state = state->set<RegionState>(Sym, RefState::getEscaped(S));
- ExplodedNode *N = C.GenerateNode(S, state);
- if (N)
- C.addTransition(N);
+ C.addTransition(state);
}
diff --git a/lib/Analysis/PointerArithChecker.cpp b/lib/Analysis/PointerArithChecker.cpp
index 9382348..6bf1a3f 100644
--- a/lib/Analysis/PointerArithChecker.cpp
+++ b/lib/Analysis/PointerArithChecker.cpp
@@ -53,7 +53,7 @@
if (isa<VarRegion>(LR) || isa<CodeTextRegion>(LR) ||
isa<CompoundLiteralRegion>(LR)) {
- if (ExplodedNode *N = C.GenerateNode(B)) {
+ if (ExplodedNode *N = C.GenerateNode()) {
if (!BT)
BT = new BuiltinBug("Dangerous pointer arithmetic",
"Pointer arithmetic done on non-array variables "
diff --git a/lib/Analysis/PointerSubChecker.cpp b/lib/Analysis/PointerSubChecker.cpp
index 4c7906f..50f5025 100644
--- a/lib/Analysis/PointerSubChecker.cpp
+++ b/lib/Analysis/PointerSubChecker.cpp
@@ -61,7 +61,7 @@
if (isa<SymbolicRegion>(BaseLR) || isa<SymbolicRegion>(BaseRR))
return;
- if (ExplodedNode *N = C.GenerateNode(B)) {
+ if (ExplodedNode *N = C.GenerateNode()) {
if (!BT)
BT = new BuiltinBug("Pointer subtraction",
"Subtraction of two pointers that do not point to "
diff --git a/lib/Analysis/ReturnPointerRangeChecker.cpp b/lib/Analysis/ReturnPointerRangeChecker.cpp
index 44887b2..8a19294 100644
--- a/lib/Analysis/ReturnPointerRangeChecker.cpp
+++ b/lib/Analysis/ReturnPointerRangeChecker.cpp
@@ -70,7 +70,7 @@
const GRState *StInBound = state->AssumeInBound(Idx, NumElements, true);
const GRState *StOutBound = state->AssumeInBound(Idx, NumElements, false);
if (StOutBound && !StInBound) {
- ExplodedNode *N = C.GenerateNode(RS, StOutBound, true);
+ ExplodedNode *N = C.GenerateSink(StOutBound);
if (!N)
return;
@@ -91,7 +91,6 @@
new RangedBugReport(*BT, BT->getDescription(), N);
report->addRange(RetE->getSourceRange());
-
C.EmitReport(report);
}
}
diff --git a/lib/Analysis/ReturnStackAddressChecker.cpp b/lib/Analysis/ReturnStackAddressChecker.cpp
index e4be871..e8a014a 100644
--- a/lib/Analysis/ReturnStackAddressChecker.cpp
+++ b/lib/Analysis/ReturnStackAddressChecker.cpp
@@ -53,7 +53,7 @@
if (!R || !R->hasStackStorage())
return;
- ExplodedNode *N = C.GenerateNode(RS, C.getState(), true);
+ ExplodedNode *N = C.GenerateSink();
if (!N)
return;
diff --git a/lib/Analysis/ReturnUndefChecker.cpp b/lib/Analysis/ReturnUndefChecker.cpp
index 796c760..48163b4 100644
--- a/lib/Analysis/ReturnUndefChecker.cpp
+++ b/lib/Analysis/ReturnUndefChecker.cpp
@@ -50,7 +50,7 @@
if (!C.getState()->getSVal(RetE).isUndef())
return;
- ExplodedNode *N = C.GenerateNode(RS, C.getState(), true);
+ ExplodedNode *N = C.GenerateSink();
if (!N)
return;
diff --git a/lib/Analysis/UndefinedArgChecker.cpp b/lib/Analysis/UndefinedArgChecker.cpp
index ea7d971..e717f6b 100644
--- a/lib/Analysis/UndefinedArgChecker.cpp
+++ b/lib/Analysis/UndefinedArgChecker.cpp
@@ -47,7 +47,7 @@
void UndefinedArgChecker::EmitBadCall(BugType *BT, CheckerContext &C,
const CallExpr *CE) {
- ExplodedNode *N = C.GenerateNode(CE, true);
+ ExplodedNode *N = C.GenerateSink();
if (!N)
return;
@@ -81,7 +81,7 @@
for (CallExpr::const_arg_iterator I = CE->arg_begin(), E = CE->arg_end();
I != E; ++I) {
if (C.getState()->getSVal(*I).isUndef()) {
- if (ExplodedNode *N = C.GenerateNode(CE, true)) {
+ if (ExplodedNode *N = C.GenerateSink()) {
if (!BT_call_arg)
BT_call_arg = new BuiltinBug("Pass-by-value argument in function call"
" is undefined");
@@ -104,7 +104,7 @@
if (const Expr *receiver = ME->getReceiver())
if (state->getSVal(receiver).isUndef()) {
- if (ExplodedNode *N = C.GenerateNode(ME, true)) {
+ if (ExplodedNode *N = C.GenerateSink()) {
if (!BT_msg_undef)
BT_msg_undef =
new BuiltinBug("Receiver in message expression is a garbage value");
@@ -122,7 +122,7 @@
for (ObjCMessageExpr::const_arg_iterator I = ME->arg_begin(), E = ME->arg_end();
I != E; ++I) {
if (state->getSVal(*I).isUndef()) {
- if (ExplodedNode *N = C.GenerateNode(ME, true)) {
+ if (ExplodedNode *N = C.GenerateSink()) {
if (!BT_msg_arg)
BT_msg_arg =
new BuiltinBug("Pass-by-value argument in message expression"
diff --git a/lib/Analysis/UndefinedArraySubscriptChecker.cpp b/lib/Analysis/UndefinedArraySubscriptChecker.cpp
index 887c775..3ae0c57 100644
--- a/lib/Analysis/UndefinedArraySubscriptChecker.cpp
+++ b/lib/Analysis/UndefinedArraySubscriptChecker.cpp
@@ -41,7 +41,7 @@
UndefinedArraySubscriptChecker::PreVisitArraySubscriptExpr(CheckerContext &C,
const ArraySubscriptExpr *A) {
if (C.getState()->getSVal(A->getIdx()).isUndef()) {
- if (ExplodedNode *N = C.GenerateNode(A, true)) {
+ if (ExplodedNode *N = C.GenerateSink()) {
if (!BT)
BT = new BuiltinBug("Array subscript is undefined");
diff --git a/lib/Analysis/UndefinedAssignmentChecker.cpp b/lib/Analysis/UndefinedAssignmentChecker.cpp
index 0e911ff..1c5b25c 100644
--- a/lib/Analysis/UndefinedAssignmentChecker.cpp
+++ b/lib/Analysis/UndefinedAssignmentChecker.cpp
@@ -48,7 +48,7 @@
if (!val.isUndef())
return;
- ExplodedNode *N = C.GenerateNode(StoreE, true);
+ ExplodedNode *N = C.GenerateSink();
if (!N)
return;
diff --git a/lib/Analysis/VLASizeChecker.cpp b/lib/Analysis/VLASizeChecker.cpp
index 799a73e..9a3436c 100644
--- a/lib/Analysis/VLASizeChecker.cpp
+++ b/lib/Analysis/VLASizeChecker.cpp
@@ -55,7 +55,7 @@
if (sizeV.isUndef()) {
// Generate an error node.
- ExplodedNode *N = C.GenerateNode(DS, true);
+ ExplodedNode *N = C.GenerateSink();
if (!N)
return;
@@ -78,7 +78,7 @@
llvm::tie(stateNotZero, stateZero) = state->Assume(sizeD);
if (stateZero && !stateNotZero) {
- ExplodedNode* N = C.GenerateNode(DS, stateZero, true);
+ ExplodedNode* N = C.GenerateSink(stateZero);
if (!BT_zero)
BT_zero = new BuiltinBug("Declared variable-length array (VLA) has zero "
"size");
@@ -92,6 +92,5 @@
}
// From this point on, assume that the size is not zero.
- if (state != stateNotZero)
- C.addTransition(C.GenerateNode(DS, stateNotZero));
+ C.addTransition(stateNotZero);
}