From: Ted Kremenek <kremenek@apple.com>
Date: Mon, 23 Nov 2009 22:22:01 +0000 (+0000)
Subject: Clean up the Checker API a little more, resolving some hidden bugs
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=19d67b52b73c04ef8eb663980330a1de2b47c845;p=clang

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/include/clang/Analysis/PathSensitive/Checker.h b/include/clang/Analysis/PathSensitive/Checker.h
index 1fb92a61de..e94096ba4f 100644
--- a/include/clang/Analysis/PathSensitive/Checker.h
+++ b/include/clang/Analysis/PathSensitive/Checker.h
@@ -40,24 +40,22 @@ class CheckerContext {
   SaveAndRestore<ProgramPoint::Kind> OldPointKind;
   SaveOr OldHasGen;
   const GRState *state;
-
+  const Stmt *statement;
+  const unsigned size;
 public:
   CheckerContext(ExplodedNodeSet &dst, GRStmtNodeBuilder &builder,
                  GRExprEngine &eng, ExplodedNode *pred,
                  const void *tag, ProgramPoint::Kind K,
-                 const GRState *st = 0)
+                 const Stmt *stmt = 0, const GRState *st = 0)
     : Dst(dst), B(builder), Eng(eng), Pred(pred),
       OldSink(B.BuildSinks),
       OldTag(B.Tag, tag),
       OldPointKind(B.PointKind, K),
       OldHasGen(B.HasGeneratedNode),
-      state(st) {}
-
-  ~CheckerContext() {
-    if (!B.BuildSinks && !B.HasGeneratedNode)
-      Dst.Add(Pred);
-  }
+      state(st), statement(stmt), size(Dst.size()) {}
 
+  ~CheckerContext();
+  
   ConstraintManager &getConstraintManager() {
       return Eng.getConstraintManager();
   }
@@ -83,27 +81,66 @@ public:
     return getBugReporter().getSourceManager();
   }
 
-  ExplodedNode *GenerateNode(const Stmt *S, bool markAsSink = false) {
-    return GenerateNode(S, getState(), markAsSink);
+  ExplodedNode *GenerateNode(bool autoTransition = true) {
+    assert(statement && "Only transitions with statements currently supported");
+    ExplodedNode *N = GenerateNodeImpl(statement, getState(), false);
+    if (N && autoTransition)
+      Dst.Add(N);
+    return N;
+  }
+  
+  ExplodedNode *GenerateNode(const Stmt *stmt, const GRState *state,
+                             bool autoTransition = true) {
+    assert(state);
+    ExplodedNode *N = GenerateNodeImpl(stmt, state, false);
+    if (N && autoTransition)
+      addTransition(N);
+    return N;
   }
 
-  ExplodedNode *GenerateNode(const Stmt* S, const GRState *state,
-                             bool markAsSink = false) {
-    ExplodedNode *node = B.generateNode(S, state, Pred);
-
-    if (markAsSink && node)
-      node->markAsSink();
+  ExplodedNode *GenerateNode(const GRState *state, bool autoTransition = true) {
+    assert(statement && "Only transitions with statements currently supported");
+    ExplodedNode *N = GenerateNodeImpl(statement, state, false);
+    if (N && autoTransition)
+      addTransition(N);
+    return N;
+  }
 
-    return node;
+  ExplodedNode *GenerateSink(const Stmt *stmt, const GRState *state = 0) {
+    return GenerateNodeImpl(stmt, state ? state : getState(), true);
+  }
+  
+  ExplodedNode *GenerateSink(const GRState *state = 0) {
+    assert(statement && "Only transitions with statements currently supported");
+    return GenerateNodeImpl(statement, state ? state : getState(), true);
   }
 
   void addTransition(ExplodedNode *node) {
     Dst.Add(node);
   }
+  
+  void addTransition(const GRState *state) {
+    assert(state);
+    if (state != getState() || 
+        (state && state != B.GetState(Pred)))
+      GenerateNode(state, true);
+    else
+      Dst.Add(Pred);
+  }
 
   void EmitReport(BugReport *R) {
     Eng.getBugReporter().EmitReport(R);
   }
+
+private:
+  ExplodedNode *GenerateNodeImpl(const Stmt* stmt, const GRState *state,
+                             bool markAsSink) {
+    ExplodedNode *node = B.generateNode(stmt, state, Pred);
+    if (markAsSink && node)
+      node->markAsSink();
+    return node;
+  }
+  
 };
 
 class Checker {
@@ -118,7 +155,7 @@ private:
                 ExplodedNode *Pred, void *tag, bool isPrevisit) {
     CheckerContext C(Dst, Builder, Eng, Pred, tag,
                      isPrevisit ? ProgramPoint::PreStmtKind :
-                     ProgramPoint::PostStmtKind);
+                     ProgramPoint::PostStmtKind, S);
     if (isPrevisit)
       _PreVisit(C, S);
     else
@@ -134,7 +171,7 @@ private:
                     bool isPrevisit) {
     CheckerContext C(Dst, Builder, Eng, Pred, tag,
                      isPrevisit ? ProgramPoint::PreStmtKind :
-                     ProgramPoint::PostStmtKind);
+                     ProgramPoint::PostStmtKind, StoreE);
     assert(isPrevisit && "Only previsit supported for now.");
     PreVisitBind(C, AssignE, StoreE, location, val);
   }
@@ -149,7 +186,7 @@ private:
                         void *tag, bool isLoad) {
     CheckerContext C(Dst, Builder, Eng, Pred, tag,
                      isLoad ? ProgramPoint::PreLoadKind :
-                     ProgramPoint::PreStoreKind, state);
+                     ProgramPoint::PreStoreKind, S, state);
     VisitLocation(C, S, location);
   }
 
@@ -157,7 +194,7 @@ private:
                           GRExprEngine &Eng, const Stmt *S, ExplodedNode *Pred,
                           SymbolReaper &SymReaper, void *tag) {
     CheckerContext C(Dst, Builder, Eng, Pred, tag, 
-                     ProgramPoint::PostPurgeDeadSymbolsKind);
+                     ProgramPoint::PostPurgeDeadSymbolsKind, S);
     EvalDeadSymbols(C, S, SymReaper);
   }
 
diff --git a/lib/Analysis/ArrayBoundChecker.cpp b/lib/Analysis/ArrayBoundChecker.cpp
index 549a22bec1..3d8b3b3d1b 100644
--- a/lib/Analysis/ArrayBoundChecker.cpp
+++ b/lib/Analysis/ArrayBoundChecker.cpp
@@ -62,8 +62,7 @@ void ArrayBoundChecker::VisitLocation(CheckerContext &C, const Stmt *S, SVal l){
   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 @@ void ArrayBoundChecker::VisitLocation(CheckerContext &C, const Stmt *S, SVal l){
       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 01e1a1fcf6..8668c75c75 100644
--- a/lib/Analysis/AttrNonNullChecker.cpp
+++ b/lib/Analysis/AttrNonNullChecker.cpp
@@ -39,7 +39,6 @@ void clang::RegisterAttrNonNullChecker(GRExprEngine &Eng) {
 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 @@ void AttrNonNullChecker::PreVisitCallExpr(CheckerContext &C,
     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 @@ void AttrNonNullChecker::PreVisitCallExpr(CheckerContext &C,
 
   // 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 5a88a7af93..424a0b3b25 100644
--- a/lib/Analysis/BasicObjCFoundationChecks.cpp
+++ b/lib/Analysis/BasicObjCFoundationChecks.cpp
@@ -563,12 +563,11 @@ void ClassReleaseChecker::PreVisitObjCMessageExpr(CheckerContext &C,
     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 ccd4a3333e..7c6fc7ed3f 100644
--- a/lib/Analysis/CastToStructChecker.cpp
+++ b/lib/Analysis/CastToStructChecker.cpp
@@ -59,7 +59,7 @@ void CastToStructChecker::PreVisitCastExpr(CheckerContext &C,
 
   // 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 985b1e0a1d..0d907e5016 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 a8f5af34a7..4c4091cbc2 100644
--- a/lib/Analysis/DereferenceChecker.cpp
+++ b/lib/Analysis/DereferenceChecker.cpp
@@ -56,8 +56,7 @@ void DereferenceChecker::VisitLocation(CheckerContext &C, const Stmt *S,
                                        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 @@ void DereferenceChecker::VisitLocation(CheckerContext &C, const Stmt *S,
   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 @@ void DereferenceChecker::VisitLocation(CheckerContext &C, const Stmt *S,
       // 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 a8630f1008..4052637043 100644
--- a/lib/Analysis/DivZeroChecker.cpp
+++ b/lib/Analysis/DivZeroChecker.cpp
@@ -63,7 +63,7 @@ void DivZeroChecker::PreVisitBinaryOperator(CheckerContext &C,
   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 @@ void DivZeroChecker::PreVisitBinaryOperator(CheckerContext &C,
 
   // 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 80096dcb70..d8adaafa60 100644
--- a/lib/Analysis/FixedAddressChecker.cpp
+++ b/lib/Analysis/FixedAddressChecker.cpp
@@ -53,7 +53,7 @@ void FixedAddressChecker::PreVisitBinaryOperator(CheckerContext &C,
   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 4f3440d88a..f657aebdf6 100644
--- a/lib/Analysis/GRExprEngine.cpp
+++ b/lib/Analysis/GRExprEngine.cpp
@@ -1292,9 +1292,13 @@ void GRExprEngine::EvalLocation(ExplodedNodeSet &Dst, Stmt *S,
     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 995720b1f8..a16125df77 100644
--- a/lib/Analysis/MallocChecker.cpp
+++ b/lib/Analysis/MallocChecker.cpp
@@ -112,9 +112,7 @@ void MallocChecker::MallocMem(CheckerContext &C, const CallExpr *CE) {
   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 @@ void MallocChecker::FreeMem(CheckerContext &C, const CallExpr *CE) {
 
   // 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 @@ void MallocChecker::FreeMem(CheckerContext &C, const CallExpr *CE) {
   // 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 @@ void MallocChecker::EvalDeadSymbols(CheckerContext &C, const Stmt *S,
       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 @@ void MallocChecker::PreVisitReturnStmt(CheckerContext &C, const ReturnStmt *S) {
   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 93823484e1..6bf1a3fc5d 100644
--- a/lib/Analysis/PointerArithChecker.cpp
+++ b/lib/Analysis/PointerArithChecker.cpp
@@ -53,7 +53,7 @@ void PointerArithChecker::PreVisitBinaryOperator(CheckerContext &C,
   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 4c7906f4be..50f502507d 100644
--- a/lib/Analysis/PointerSubChecker.cpp
+++ b/lib/Analysis/PointerSubChecker.cpp
@@ -61,7 +61,7 @@ void PointerSubChecker::PreVisitBinaryOperator(CheckerContext &C,
   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 44887b2625..8a1929464e 100644
--- a/lib/Analysis/ReturnPointerRangeChecker.cpp
+++ b/lib/Analysis/ReturnPointerRangeChecker.cpp
@@ -70,7 +70,7 @@ void ReturnPointerRangeChecker::PreVisitReturnStmt(CheckerContext &C,
   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 @@ void ReturnPointerRangeChecker::PreVisitReturnStmt(CheckerContext &C,
       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 e4be8712d0..e8a014af29 100644
--- a/lib/Analysis/ReturnStackAddressChecker.cpp
+++ b/lib/Analysis/ReturnStackAddressChecker.cpp
@@ -53,7 +53,7 @@ void ReturnStackAddressChecker::PreVisitReturnStmt(CheckerContext &C,
   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 796c7608c8..48163b4d86 100644
--- a/lib/Analysis/ReturnUndefChecker.cpp
+++ b/lib/Analysis/ReturnUndefChecker.cpp
@@ -50,7 +50,7 @@ void ReturnUndefChecker::PreVisitReturnStmt(CheckerContext &C,
   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 ea7d971fdb..e717f6bfe2 100644
--- a/lib/Analysis/UndefinedArgChecker.cpp
+++ b/lib/Analysis/UndefinedArgChecker.cpp
@@ -47,7 +47,7 @@ void clang::RegisterUndefinedArgChecker(GRExprEngine &Eng) {
 
 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 @@ void UndefinedArgChecker::PreVisitCallExpr(CheckerContext &C,
   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 @@ void UndefinedArgChecker::PreVisitObjCMessageExpr(CheckerContext &C,
 
   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 @@ void UndefinedArgChecker::PreVisitObjCMessageExpr(CheckerContext &C,
   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 887c7755fe..3ae0c579e9 100644
--- a/lib/Analysis/UndefinedArraySubscriptChecker.cpp
+++ b/lib/Analysis/UndefinedArraySubscriptChecker.cpp
@@ -41,7 +41,7 @@ void
 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 0e911ffab0..1c5b25cd8e 100644
--- a/lib/Analysis/UndefinedAssignmentChecker.cpp
+++ b/lib/Analysis/UndefinedAssignmentChecker.cpp
@@ -48,7 +48,7 @@ void UndefinedAssignmentChecker::PreVisitBind(CheckerContext &C,
   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 799a73e293..9a3436c4c9 100644
--- a/lib/Analysis/VLASizeChecker.cpp
+++ b/lib/Analysis/VLASizeChecker.cpp
@@ -55,7 +55,7 @@ void VLASizeChecker::PreVisitDeclStmt(CheckerContext &C, const DeclStmt *DS) {
 
   if (sizeV.isUndef()) {
     // Generate an error node.
-    ExplodedNode *N = C.GenerateNode(DS, true);    
+    ExplodedNode *N = C.GenerateSink();
     if (!N)
       return;
     
@@ -78,7 +78,7 @@ void VLASizeChecker::PreVisitDeclStmt(CheckerContext &C, const DeclStmt *DS) {
   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 @@ void VLASizeChecker::PreVisitDeclStmt(CheckerContext &C, const DeclStmt *DS) {
   }
  
   // From this point on, assume that the size is not zero.
-  if (state != stateNotZero)
-    C.addTransition(C.GenerateNode(DS, stateNotZero));  
+  C.addTransition(stateNotZero);
 }