[analyzer] Add generateErrorNode() APIs to CheckerContext

[dcoughlin]

Summary

The analyzer trims unnecessary nodes from the exploded graph before reporting path diagnostics. However, in some cases it can trim all nodes (including the error node), leading to an assertion failure (see https://llvm.org/bugs/show_bug.cgi?id=24184).

This patch addresses the issue by adding two new APIs to CheckerContext to explicitly create error nodes. Unless the client provides a custom tag, these APIs tag the node with the checker's tag -- preventing it from being trimmed. The generateErrorNode() method creates a sink error node, while generateNonFatalErrorNode() creates an error node for a path that should continue being explored.

The intent is that one of these two methods should be used whenever a checker creates an error node.

I've updated the checkers to use these APIs. You'll notice that these APIs (unlike addTransition() and generateSink()) do not take an explicit Pred node. This is because I did not find any error nodes in the checkers that were created with an explicit node that was different than the default (the CheckerContext's Pred node). Do you think this is a reasonable simplification, or should we permit checkers to specify Pred explicitly?

I also added a test case written by Ying Yi as part of http://reviews.llvm.org/D12163 (that patch originally addressed this issue but was reverted because it introduced false positive regressions).

[jordan_rose]

include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h

321 ↗

(On Diff #34480)

What does "has made a mistake" mean? What is the mistake and how will they discover it?

[zaks.anna]

include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h

229 ↗

(On Diff #34480)

Most likely there are not much uses of this left and to avoid confusion we could require State and Pred inputs. What do you think?

244 ↗

(On Diff #34480)

Please, use a non-null Pred for clarity

lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp

89 ↗

(On Diff #34480)

We should not generate a transition on an early exit here..

116 ↗

(On Diff #34480)

Same here, no reason to generate a transition unless we are ready to report a bug. I'just pass C.generateNonFatalErrorNode() to llvm::make_unique<BugReport>.

lib/StaticAnalyzer/Checkers/FixedAddressChecker.cpp

53 ↗

(On Diff #34480)

Can this ever fail? In some cases we just assume it won't in others we tests..

Maybe it only fails when we cache out?

[xazax.hun]

In general I like this change, the node handling of the checkers are more readable and reflects the intent in a clearer way. I have some comments inline.

include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h

244 ↗

(On Diff #34480)

The following workflow is not supported by this API: a checker that generates multiple transition in the same callback (the generated nodes are added sequentially to the path), and one of the might be an error node.

This also applies to generateNonFatalErrorNode.

In case we would like to improve the documentation it might be useful to give some pointers to the users which when should an error node be considered as fatal.

322 ↗

(On Diff #34480)

As a slightly related note: is it documented anywhere what "cache out" means? Maybe it would be great to refer to that document or write it if it is not written yet.

[jordan_rose]

lib/StaticAnalyzer/Checkers/FixedAddressChecker.cpp

53 ↗

(On Diff #34480)

It does fail when we cache out, and I think we can still cache out if Pred has a different tag the second time around.

[zaks.anna]

include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h

244 ↗

(On Diff #34480)

• We could introduce another API that requires a Pred node.
• The workflow is supported right now by directly using addTransition API.

lib/StaticAnalyzer/Checkers/FixedAddressChecker.cpp

53 ↗

(On Diff #34480)

There some instances in this patch where we do not check if the returned node is null. We should be consistent.

[dcoughlin]

include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h

229 ↗

(On Diff #34480)

There are 7 uses left. Requiring State and Pred seems like the right thing to me. I will change it.

321 ↗

(On Diff #34480)

The mistake this guard protects against is adding a transition from a state to itself, which would normally cause a cache out. My understanding is that the whole point of this guard is to silently swallow that mistake. I found that surprising, which is why I added the comment.

lib/StaticAnalyzer/Checkers/FixedAddressChecker.cpp

53 ↗

(On Diff #34480)

Ok, I'll go through and add checks where they are missing.

[zaks.anna]

include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h

321 ↗

(On Diff #34480)

Should we add an assert here? I wonder if/how much it will get triggered.

[dcoughlin]

Added checks for null generated error nodes in the few cases in checkers were they were missing and updated comments.

include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h

321 ↗

(On Diff #34480)

I tried adding an assert to the inverse of the 'if' condition here. The DereferenceChecker, CallAndMessageChecker, and DynamicTypePropagation all trigger it. Added a note about this in a comment.

322 ↗

(On Diff #34480)

I've defined "cache out" in the comment.

lib/StaticAnalyzer/Checkers/FixedAddressChecker.cpp

53 ↗

(On Diff #34480)

There were 9 locations where checks for null error generated error nodes were missing. I added them.

[xazax.hun]

include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h

328 ↗

(On Diff #34917)

I think the most common form of relying on this branch is adding a transition to an unchanged state.

[chapuni]

Did you forget to update examples/analyzer-plugin? Fixed in r247862.

cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h

public:
   /// @param Tag The tag to uniquely identify the creation site.
   ExplodedNode *addTransition(ProgramStateRef State,
                               ExplodedNode *Pred,
                               const ProgramPointTag *Tag = nullptr) {
     return addTransitionImpl(State, false, Pred, Tag);
   }
 
   /// \brief Generate a sink node. Generating a sink stops exploration of the
-  /// given path.
-  ExplodedNode *generateSink(ProgramStateRef State = nullptr,
-                             ExplodedNode *Pred = nullptr,
+  /// given path. To create a sink node for the purpose of reporting an error,
+  /// checkers should use generateErrorNode() instead.
+  ExplodedNode *generateSink(ProgramStateRef State, ExplodedNode *Pred,
                              const ProgramPointTag *Tag = nullptr) {
     return addTransitionImpl(State ? State : getState(), true, Pred, Tag);
   }
 
+  /// \brief Generate a transition to a node that will be used to report
+  /// an error. This node will be a sink. That is, it will stop exploration of
+  /// the given path.
+  ///
+  /// @param State The state of the generated node.
+  /// @param Tag The tag to uniquely identify the creation site. If null,
+  ///        the default tag for the checker will be used.
+  ExplodedNode *generateErrorNode(ProgramStateRef State = nullptr,
+                                  const ProgramPointTag *Tag = nullptr) {
+    return generateSink(State, Pred,
+                       (Tag ? Tag : Location.getTag()));
+  }
+
+  /// \brief Generate a transition to a node that will be used to report
+  /// an error. This node will not be a sink. That is, exploration will
+  /// continue along this path.
+  ///
+  /// @param State The state of the generated node.
+  /// @param Tag The tag to uniquely identify the creation site. If null,
+  ///        the default tag for the checker will be used.
+  ExplodedNode *
+  generateNonFatalErrorNode(ProgramStateRef State = nullptr,
+                            const ProgramPointTag *Tag = nullptr) {
+    return addTransition(State, (Tag ? Tag : Location.getTag()));
+  }
+
   /// \brief Emit the diagnostics report.
   void emitReport(std::unique_ptr<BugReport> R) {
     Changed = true;
     Eng.getBugReporter().emitReport(std::move(R));
   }
 
   /// \brief Get the declaration of the called function (path-sensitive).
   const FunctionDecl *getCalleeDecl(const CallExpr *CE) const;
public:
   /// \sa clang::Lexer::getSpelling(), clang::Lexer::getImmediateMacroName().
   StringRef getMacroNameOrSpelling(SourceLocation &Loc);
 
 private:
   ExplodedNode *addTransitionImpl(ProgramStateRef State,
                                  bool MarkAsSink,
                                  ExplodedNode *P = nullptr,
                                  const ProgramPointTag *Tag = nullptr) {
+    // The analyzer may stop exploring if it sees a state it has previously
+    // visited ("cache out"). The early return here is a defensive check to
+    // prevent accidental caching out by checker API clients. Unless there is a
+    // tag or the the client checker has requested that the generated node be
+    // marked as a sink, we assume that a client requesting a transition to a
+    // state that is the same as the predecessor state has made a mistake. We
+    // return the predecessor rather than cache out.
+    //
+    // TODO: We could potentially change the return to an assertion to alert
+    // clients to their mistake, but several checkers (including
+    // DereferenceChecker, CallAndMessageChecker, and DynamicTypePropagation)
+    // rely upon the defensive behavior and would need to be updated.
     if (!State || (State == Pred->getState() && !Tag && !MarkAsSink))
       return Pred;
 
     Changed = true;
     const ProgramPoint &LocalLoc = (Tag ? Location.withTag(Tag) : Location);
     if (!P)
       P = Pred;
 

cfe/trunk/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp

void ArrayBoundChecker::checkLocation(SVal l, bool isLoad, const Stmt* LoadS,
   // Get the size of the array.
   DefinedOrUnknownSVal NumElements
     = C.getStoreManager().getSizeInElements(state, ER->getSuperRegion(),
                                             ER->getValueType());
 
   ProgramStateRef StInBound = state->assumeInBound(Idx, NumElements, true);
   ProgramStateRef StOutBound = state->assumeInBound(Idx, NumElements, false);
   if (StOutBound && !StInBound) {
-    ExplodedNode *N = C.generateSink(StOutBound);
+    ExplodedNode *N = C.generateErrorNode(StOutBound);
     if (!N)
       return;
 
     if (!BT)
       BT.reset(new BuiltinBug(
           this, "Out-of-bound array access",
           "Access out-of-bound array element (buffer overflow)"));
 

cfe/trunk/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp

void ArrayBoundCheckerV2::checkLocation(SVal location, bool isLoad,
   if (state != originalState)
     checkerContext.addTransition(state);
 }
 
 void ArrayBoundCheckerV2::reportOOB(CheckerContext &checkerContext,
                                     ProgramStateRef errorState,
                                     OOB_Kind kind) const {
 
-  ExplodedNode *errorNode = checkerContext.generateSink(errorState);
+  ExplodedNode *errorNode = checkerContext.generateErrorNode(errorState);
   if (!errorNode)
     return;
 
   if (!BT)
     BT.reset(new BuiltinBug(this, "Out-of-bound access"));
 
   // FIXME: This diagnostics are preliminary.  We should get far better
   // diagnostics for explaining buffer overruns.

cfe/trunk/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp

 }
 
 void NilArgChecker::warnIfNilExpr(const Expr *E,
                                   const char *Msg,
                                   CheckerContext &C) const {
   ProgramStateRef State = C.getState();
   if (State->isNull(C.getSVal(E)).isConstrainedTrue()) {
 
-    if (ExplodedNode *N = C.generateSink()) {
+    if (ExplodedNode *N = C.generateErrorNode()) {
       generateBugReport(N, Msg, E->getSourceRange(), E, C);
     }
 
   }
 }
 
 void NilArgChecker::warnIfNilArg(CheckerContext &C,
                                  const ObjCMethodCall &msg,
                                  unsigned int Arg,
                                  FoundationClass Class,
                                  bool CanBeSubscript) const {
   // Check if the argument is nil.
   ProgramStateRef State = C.getState();
   if (!State->isNull(msg.getArgSVal(Arg)).isConstrainedTrue())
       return;
 
-  if (ExplodedNode *N = C.generateSink()) {
+  if (ExplodedNode *N = C.generateErrorNode()) {
     SmallString<128> sbuf;
     llvm::raw_svector_ostream os(sbuf);
 
     if (CanBeSubscript && msg.getMessageKind() == OCM_Subscript) {
 
       if (Class == FC_NSArray) {
         os << "Array element cannot be nil";
       } else if (Class == FC_NSDictionary) {
if (!T->isIntegralOrEnumerationType())
     return;
 
   uint64_t SourceSize = Ctx.getTypeSize(T);
 
   // CHECK: is SourceSize == TargetSize
   if (SourceSize == TargetSize)
     return;
 
-  // Generate an error.  Only generate a sink if 'SourceSize < TargetSize';
-  // otherwise generate a regular node.
+  // Generate an error.  Only generate a sink error node
+  // if 'SourceSize < TargetSize'; otherwise generate a non-fatal error node.
   //
   // FIXME: We can actually create an abstract "CFNumber" object that has
   //  the bits initialized to the provided values.
   //
-  if (ExplodedNode *N = SourceSize < TargetSize ? C.generateSink()
-                                                : C.addTransition()) {
+  ExplodedNode *N = SourceSize < TargetSize ? C.generateErrorNode()
+                                            : C.generateNonFatalErrorNode();
+  if (N) {
     SmallString<128> sbuf;
     llvm::raw_svector_ostream os(sbuf);
 
     os << (SourceSize == 8 ? "An " : "A ")
        << SourceSize << " bit integer is used to initialize a CFNumber "
                         "object that represents "
        << (TargetSize == 8 ? "an " : "a ")
        << TargetSize << " bit integer. ";
void CFRetainReleaseChecker::checkPreStmt(const CallExpr *CE,
   // Make an expression asserting that they're equal.
   DefinedOrUnknownSVal ArgIsNull = svalBuilder.evalEQ(state, zero, *DefArgVal);
 
   // Are they equal?
   ProgramStateRef stateTrue, stateFalse;
   std::tie(stateTrue, stateFalse) = state->assume(ArgIsNull);
 
   if (stateTrue && !stateFalse) {
-    ExplodedNode *N = C.generateSink(stateTrue);
+    ExplodedNode *N = C.generateErrorNode(stateTrue);
     if (!N)
       return;
 
     const char *description;
     if (FuncII == Retain)
       description = "Null pointer argument in call to CFRetain";
     else if (FuncII == Release)
       description = "Null pointer argument in call to CFRelease";
if (msg.isInstanceMessage())
     return;
   const ObjCInterfaceDecl *Class = msg.getReceiverInterface();
   assert(Class);
 
   Selector S = msg.getSelector();
   if (!(S == releaseS || S == retainS || S == autoreleaseS || S == drainS))
     return;
 
-  if (ExplodedNode *N = C.addTransition()) {
+  if (ExplodedNode *N = C.generateNonFatalErrorNode()) {
     SmallString<200> buf;
     llvm::raw_svector_ostream os(buf);
 
     os << "The '";
     S.print(os);
     os << "' message should be sent to instances "
           "of class '" << Class->getName()
        << "' and not the class directly";
if (C.getASTContext().isObjCNSObjectType(ArgTy))
       continue;
 
     // Ignore CF references, which can be toll-free bridged.
     if (coreFoundation::isCFObjectRef(ArgTy))
       continue;
 
     // Generate only one error node to use for all bug reports.
     if (!errorNode.hasValue())
-      errorNode = C.addTransition();
+      errorNode = C.generateNonFatalErrorNode();
 
     if (!errorNode.getValue())
       continue;
 
     SmallString<128> sbuf;
     llvm::raw_svector_ostream os(sbuf);
 
     StringRef TypeName = GetReceiverInterfaceName(msg);
void ObjCLoopChecker::checkPostStmt(const ObjCForCollectionStmt *FCS,
   // Otherwise, this is a branch that goes through the loop body.
   } else {
     State = checkCollectionNonNil(C, State, FCS);
     State = checkElementNonNil(C, State, FCS);
     State = assumeCollectionNonEmpty(C, State, FCS, /*Assumption*/true);
   }
 
   if (!State)
-    C.generateSink();
+    C.generateSink(C.getState(), C.getPredecessor());
   else if (State != C.getState())
     C.addTransition(State);
 }
 
 bool ObjCLoopChecker::isCollectionCountMethod(const ObjCMethodCall &M,
                                               CheckerContext &C) const {
   Selector S = M.getSelector();
   // Initialize the identifiers on first use.

cfe/trunk/lib/StaticAnalyzer/Checkers/BoolAssignmentChecker.cpp

class BoolAssignmentChecker : public Checker< check::Bind > {
     void emitReport(ProgramStateRef state, CheckerContext &C) const;
   public:
     void checkBind(SVal loc, SVal val, const Stmt *S, CheckerContext &C) const;
   };
 } // end anonymous namespace
 
 void BoolAssignmentChecker::emitReport(ProgramStateRef state,
                                        CheckerContext &C) const {
-  if (ExplodedNode *N = C.addTransition(state)) {
+  if (ExplodedNode *N = C.generateNonFatalErrorNode(state)) {
     if (!BT)
       BT.reset(new BuiltinBug(this, "Assignment of a non-Boolean value"));
     C.emitReport(llvm::make_unique<BugReport>(*BT, BT->getDescription(), N));
   }
 }
 
 static bool isBooleanType(QualType Ty) {
   if (Ty->isBooleanType()) // C++ or C99

cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp

ProgramStateRef CStringChecker::checkNonNull(CheckerContext &C,
 
   ProgramStateRef stateNull, stateNonNull;
   std::tie(stateNull, stateNonNull) = assumeZero(C, state, l, S->getType());
 
   if (stateNull && !stateNonNull) {
     if (!Filter.CheckCStringNullArg)
       return nullptr;
 
-    ExplodedNode *N = C.generateSink(stateNull);
+    ExplodedNode *N = C.generateErrorNode(stateNull);
     if (!N)
       return nullptr;
 
     if (!BT_Null)
       BT_Null.reset(new BuiltinBug(
           Filter.CheckNameCStringNullArg, categories::UnixAPI,
           "Null pointer argument in call to byte string function"));
 
ProgramStateRef CStringChecker::CheckLocation(CheckerContext &C,
   DefinedOrUnknownSVal Size = Extent.castAs<DefinedOrUnknownSVal>();
 
   // Get the index of the accessed element.
   DefinedOrUnknownSVal Idx = ER->getIndex().castAs<DefinedOrUnknownSVal>();
 
   ProgramStateRef StInBound = state->assumeInBound(Idx, Size, true);
   ProgramStateRef StOutBound = state->assumeInBound(Idx, Size, false);
   if (StOutBound && !StInBound) {
-    ExplodedNode *N = C.generateSink(StOutBound);
+    ExplodedNode *N = C.generateErrorNode(StOutBound);
     if (!N)
       return nullptr;
 
     if (!BT_Bounds) {
       BT_Bounds.reset(new BuiltinBug(
           Filter.CheckNameCStringOutOfBounds, "Out-of-bound array access",
           "Byte string function accesses out-of-bound array element"));
     }
ProgramStateRef CStringChecker::CheckOverlap(CheckerContext &C,
 
   // assume the two expressions don't overlap.
   assert(stateFalse);
   return stateFalse;
 }
 
 void CStringChecker::emitOverlapBug(CheckerContext &C, ProgramStateRef state,
                                   const Stmt *First, const Stmt *Second) const {
-  ExplodedNode *N = C.generateSink(state);
+  ExplodedNode *N = C.generateErrorNode(state);
   if (!N)
     return;
 
   if (!BT_Overlap)
     BT_Overlap.reset(new BugType(Filter.CheckNameCStringBufferOverlap,
                                  categories::UnixAPI, "Improper arguments"));
 
   // Generate a report for this bug.
SVal willOverflow = svalBuilder.evalBinOpNN(state, BO_GT, left,
                                                 *maxMinusRightNL, cmpTy);
 
     ProgramStateRef stateOverflow, stateOkay;
     std::tie(stateOverflow, stateOkay) =
       state->assume(willOverflow.castAs<DefinedOrUnknownSVal>());
 
     if (stateOverflow && !stateOkay) {
       // We have an overflow. Emit a bug report.
-      ExplodedNode *N = C.generateSink(stateOverflow);
+      ExplodedNode *N = C.generateErrorNode(stateOverflow);
       if (!N)
         return nullptr;
 
       if (!BT_AdditionOverflow)
         BT_AdditionOverflow.reset(
             new BuiltinBug(Filter.CheckNameCStringOutOfBounds, "API",
                            "Sum of expressions causes overflow"));
 
SVal CStringChecker::getCStringLength(CheckerContext &C, ProgramStateRef &state,
   if (!MR) {
     // If we can't get a region, see if it's something we /know/ isn't a
     // C string. In the context of locations, the only time we can issue such
     // a warning is for labels.
     if (Optional<loc::GotoLabel> Label = Buf.getAs<loc::GotoLabel>()) {
       if (!Filter.CheckCStringNotNullTerm)
         return UndefinedVal();
 
-      if (ExplodedNode *N = C.addTransition(state)) {
+      if (ExplodedNode *N = C.generateNonFatalErrorNode(state)) {
         if (!BT_NotCString)
           BT_NotCString.reset(new BuiltinBug(
               Filter.CheckNameCStringNotNullTerm, categories::UnixAPI,
               "Argument is not a null-terminated string."));
 
         SmallString<120> buf;
         llvm::raw_svector_ostream os(buf);
         assert(CurrentFunctionDescription);
case MemRegion::ElementRegionKind:
     return UnknownVal();
   default:
     // Other regions (mostly non-data) can't have a reliable C string length.
     // In this case, an error is emitted and UndefinedVal is returned.
     // The caller should always be prepared to handle this case.
     if (!Filter.CheckCStringNotNullTerm)
       return UndefinedVal();
 
-    if (ExplodedNode *N = C.addTransition(state)) {
+    if (ExplodedNode *N = C.generateNonFatalErrorNode(state)) {
       if (!BT_NotCString)
         BT_NotCString.reset(new BuiltinBug(
             Filter.CheckNameCStringNotNullTerm, categories::UnixAPI,
             "Argument is not a null-terminated string."));
 
       SmallString<120> buf;
       llvm::raw_svector_ostream os(buf);
 

cfe/trunk/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp

bool uninitRefOrPointer(CheckerContext &C, const SVal &V,
                           const SourceRange &ArgRange,
                           const Expr *ArgEx, std::unique_ptr<BugType> &BT,
                           const ParmVarDecl *ParamDecl, const char *BD) const;
 };
 } // end anonymous namespace
 
 void CallAndMessageChecker::emitBadCall(BugType *BT, CheckerContext &C,
                                         const Expr *BadE) {
-  ExplodedNode *N = C.generateSink();
+  ExplodedNode *N = C.generateErrorNode();
   if (!N)
     return;
 
   auto R = llvm::make_unique<BugReport>(*BT, BT->getName(), N);
   if (BadE) {
     R->addRange(BadE->getSourceRange());
     if (BadE->isGLValue())
       BadE = bugreporter::getDerefExpr(BadE);
bool CallAndMessageChecker::uninitRefOrPointer(CheckerContext &C,
 
   if(!ParamDecl->getType()->getPointeeType().isConstQualified())
     return false;
 
   if (const MemRegion *SValMemRegion = V.getAsRegion()) {
     const ProgramStateRef State = C.getState();
     const SVal PSV = State->getSVal(SValMemRegion);
     if (PSV.isUndef()) {
-      if (ExplodedNode *N = C.generateSink()) {
+      if (ExplodedNode *N = C.generateErrorNode()) {
         LazyInit_BT(BD, BT);
         auto R = llvm::make_unique<BugReport>(*BT, Message, N);
         R->addRange(ArgRange);
         if (ArgEx) {
           bugreporter::trackNullOrUndefValue(N, ArgEx, *R);
         }
         C.emitReport(std::move(R));
       }
bool CallAndMessageChecker::PreVisitProcessArg(CheckerContext &C,
                                                const ParmVarDecl *ParamDecl
                                                ) const {
   const char *BD = "Uninitialized argument value";
 
   if (uninitRefOrPointer(C, V, ArgRange, ArgEx, BT, ParamDecl, BD))
     return true;
 
   if (V.isUndef()) {
-    if (ExplodedNode *N = C.generateSink()) {
+    if (ExplodedNode *N = C.generateErrorNode()) {
       LazyInit_BT(BD, BT);
 
       // Generate a report for this bug.
       StringRef Desc =
           describeUninitializedArgumentInCall(Call, IsFirstArgument);
       auto R = llvm::make_unique<BugReport>(*BT, Desc, N);
       R->addRange(ArgRange);
       if (ArgEx)
if (Optional<nonloc::LazyCompoundVal> LV =
     };
 
     const LazyCompoundValData *D = LV->getCVData();
     FindUninitializedField F(C.getState()->getStateManager().getStoreManager(),
                              C.getSValBuilder().getRegionManager(),
                              D->getStore());
 
     if (F.Find(D->getRegion())) {
-      if (ExplodedNode *N = C.generateSink()) {
+      if (ExplodedNode *N = C.generateErrorNode()) {
         LazyInit_BT(BD, BT);
         SmallString<512> Str;
         llvm::raw_svector_ostream os(Str);
         os << "Passed-by-value struct argument contains uninitialized data";
 
         if (F.FieldChain.size() == 1)
           os << " (e.g., field: '" << *F.FieldChain[0] << "')";
         else {

 }
 
 void CallAndMessageChecker::checkPreStmt(const CXXDeleteExpr *DE,
                                          CheckerContext &C) const {
 
   SVal Arg = C.getSVal(DE->getArgument());
   if (Arg.isUndef()) {
     StringRef Desc;
-    ExplodedNode *N = C.generateSink();
+    ExplodedNode *N = C.generateErrorNode();
     if (!N)
       return;
     if (!BT_cxx_delete_undef)
       BT_cxx_delete_undef.reset(
           new BuiltinBug(this, "Uninitialized argument value"));
     if (DE->isArrayFormAsWritten())
       Desc = "Argument to 'delete[]' is uninitialized";
     else
void CallAndMessageChecker::checkPreCall(const CallEvent &Call,
 
   const Decl *D = Call.getDecl();
   const FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(D);
   if (FD) {
     // If we have a declaration, we can make sure we pass enough parameters to
     // the function.
     unsigned Params = FD->getNumParams();
     if (Call.getNumArgs() < Params) {
-      ExplodedNode *N = C.generateSink();
+      ExplodedNode *N = C.generateErrorNode();
       if (!N)
         return;
 
       LazyInit_BT("Function call with too few arguments", BT_call_few_args);
 
       SmallString<512> Str;
       llvm::raw_svector_ostream os(Str);
       os << "Function taking " << Params << " argument"
void CallAndMessageChecker::checkPreCall(const CallEvent &Call,
   // If we make it here, record our assumptions about the callee.
   C.addTransition(State);
 }
 
 void CallAndMessageChecker::checkPreObjCMessage(const ObjCMethodCall &msg,
                                                 CheckerContext &C) const {
   SVal recVal = msg.getReceiverSVal();
   if (recVal.isUndef()) {
-    if (ExplodedNode *N = C.generateSink()) {
+    if (ExplodedNode *N = C.generateErrorNode()) {
       BugType *BT = nullptr;
       switch (msg.getMessageKind()) {
       case OCM_Message:
         if (!BT_msg_undef)
           BT_msg_undef.reset(new BuiltinBug(this,
                                             "Receiver in message expression "
                                             "is an uninitialized value"));
         BT = BT_msg_undef.get();
if (CanRetTy != Ctx.VoidTy && C.getLocationContext()->getParentMap()
     if (CanRetTy.getTypePtr()->isReferenceType()||
         (voidPtrSize < returnTypeSize &&
          !(supportsNilWithFloatRet(Ctx.getTargetInfo().getTriple()) &&
            (Ctx.FloatTy == CanRetTy ||
             Ctx.DoubleTy == CanRetTy ||
             Ctx.LongDoubleTy == CanRetTy ||
             Ctx.LongLongTy == CanRetTy ||
             Ctx.UnsignedLongLongTy == CanRetTy)))) {
-      if (ExplodedNode *N = C.generateSink(state, nullptr, &Tag))
+      if (ExplodedNode *N = C.generateErrorNode(state, &Tag))
         emitNilReceiverBug(C, Msg, N);
       return;
     }
 
     // Handle the safe cases where the return value is 0 if the
     // receiver is nil.
     //
     // FIXME: For now take the conservative approach that we only

cfe/trunk/lib/StaticAnalyzer/Checkers/CastSizeChecker.cpp

if (typeSize.isZero())
     return;
 
   if (regionSize % typeSize == 0)
     return;
 
   if (evenFlexibleArraySize(Ctx, regionSize, typeSize, ToPointeeTy))
     return;
 
-  if (ExplodedNode *errorNode = C.generateSink()) {
+  if (ExplodedNode *errorNode = C.generateErrorNode()) {
     if (!BT)
       BT.reset(new BuiltinBug(this, "Cast region with wrong size.",
                                     "Cast a region whose size is not a multiple"
                                     " of the destination type size."));
     auto R = llvm::make_unique<BugReport>(*BT, BT->getDescription(), errorNode);
     R->addRange(CE->getSourceRange());
     C.emitReport(std::move(R));
   }
 }
 
 void ento::registerCastSizeChecker(CheckerManager &mgr) {
   mgr.registerChecker<CastSizeChecker>();
 }

cfe/trunk/lib/StaticAnalyzer/Checkers/CastToStructChecker.cpp

if (!ToPointeeTy->isStructureOrClassType())
     return;
 
   // We allow cast from void*.
   if (OrigPointeeTy->isVoidType())
     return;
 
   // Now the cast-to-type is struct pointer, the original type is not void*.
   if (!OrigPointeeTy->isRecordType()) {
-    if (ExplodedNode *N = C.addTransition()) {
+    if (ExplodedNode *N = C.generateNonFatalErrorNode()) {
       if (!BT)
         BT.reset(
             new BuiltinBug(this, "Cast from non-struct type to struct type",
                            "Casting a non-structure type to a structure type "
                            "and accessing a field can lead to memory access "
                            "errors or data corruption."));
       auto R = llvm::make_unique<BugReport>(*BT, BT->getDescription(), N);
       R->addRange(CE->getSourceRange());
       C.emitReport(std::move(R));
     }
   }
 }
 
 void ento::registerCastToStructChecker(CheckerManager &mgr) {
   mgr.registerChecker<CastToStructChecker>();
 }

cfe/trunk/lib/StaticAnalyzer/Checkers/ChrootChecker.cpp

void ChrootChecker::checkPreStmt(const CallExpr *CE, CheckerContext &C) const {
   // Ingnore chroot and chdir.
   if (FD->getIdentifier() == II_chroot || FD->getIdentifier() == II_chdir)
     return;
 
   // If jail state is ROOT_CHANGED, generate BugReport.
   void *const* k = C.getState()->FindGDM(ChrootChecker::getTag());
   if (k)
     if (isRootChanged((intptr_t) *k))
-      if (ExplodedNode *N = C.addTransition()) {
+      if (ExplodedNode *N = C.generateNonFatalErrorNode()) {
         if (!BT_BreakJail)
           BT_BreakJail.reset(new BuiltinBug(
               this, "Break out of jail", "No call of chdir(\"/\") immediately "
                                          "after chroot"));
         C.emitReport(llvm::make_unique<BugReport>(
             *BT_BreakJail, BT_BreakJail->getDescription(), N));
       }
 
   return;
 }
 
 void ento::registerChrootChecker(CheckerManager &mgr) {
   mgr.registerChecker<ChrootChecker>();
 }

cfe/trunk/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp

case Stmt::ObjCIvarRefExprClass: {
       break;
     }
   }
 }
 
 void DereferenceChecker::reportBug(ProgramStateRef State, const Stmt *S,
                                    CheckerContext &C, bool IsBind) const {
   // Generate an error node.
-  ExplodedNode *N = C.generateSink(State);
+  ExplodedNode *N = C.generateErrorNode(State);
   if (!N)
     return;
 
   // We know that 'location' cannot be non-null.  This is what
   // we call an "explicit" null dereference.
   if (!BT_null)
     BT_null.reset(new BuiltinBug(this, "Dereference of null pointer"));
 
void DereferenceChecker::reportBug(ProgramStateRef State, const Stmt *S,
 
   C.emitReport(std::move(report));
 }
 
 void DereferenceChecker::checkLocation(SVal l, bool isLoad, const Stmt* S,
                                        CheckerContext &C) const {
   // Check for dereference of an undefined value.
   if (l.isUndef()) {
-    if (ExplodedNode *N = C.generateSink()) {
+    if (ExplodedNode *N = C.generateErrorNode()) {
       if (!BT_undef)
         BT_undef.reset(
             new BuiltinBug(this, "Dereference of undefined pointer value"));
 
       auto report =
           llvm::make_unique<BugReport>(*BT_undef, BT_undef->getDescription(), N);
       bugreporter::trackNullOrUndefValue(N, bugreporter::getDerefExpr(S),
                                          *report);
if (nullState) {
     if (!notNullState) {
       reportBug(nullState, S, C);
       return;
     }
 
     // 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.generateSink(nullState)) {
+    if (ExplodedNode *N = C.generateSink(nullState, C.getPredecessor())) {
       ImplicitNullDerefEvent event = {l, isLoad, N, &C.getBugReporter(),
                                       /*IsDirectDereference=*/false};
       dispatchEvent(event);
     }
   }
 
   // From this point forward, we know that the location is not null.
   C.addTransition(notNullState);
void DereferenceChecker::checkBind(SVal L, SVal V, const Stmt *S,
   if (StNull) {
     if (!StNonNull) {
       reportBug(StNull, S, C, /*isBind=*/true);
       return;
     }
 
     // At this point the value could be either null or non-null.
     // Record this as an "implicit" null dereference.
-    if (ExplodedNode *N = C.generateSink(StNull)) {
+    if (ExplodedNode *N = C.generateSink(StNull, C.getPredecessor())) {
       ImplicitNullDerefEvent event = {V, /*isLoad=*/true, N,
                                       &C.getBugReporter(),
                                       /*IsDirectDereference=*/false};
       dispatchEvent(event);
     }
   }
 
   // Unlike a regular null dereference, initializing a reference with a

cfe/trunk/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp

 public:
   void checkPreStmt(const BinaryOperator *B, CheckerContext &C) const;
 };
 } // end anonymous namespace
 
 void DivZeroChecker::reportBug(const char *Msg,
                                ProgramStateRef StateZero,
                                CheckerContext &C) const {
-  if (ExplodedNode *N = C.generateSink(StateZero)) {
+  if (ExplodedNode *N = C.generateErrorNode(StateZero)) {
     if (!BT)
       BT.reset(new BuiltinBug(this, "Division by zero"));
 
     auto R = llvm::make_unique<BugReport>(*BT, Msg, N);
     bugreporter::trackNullOrUndefValue(N, bugreporter::GetDenomExpr(N), *R);
     C.emitReport(std::move(R));
   }
 }

cfe/trunk/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp

if (StFalse)
       return "FALSE";
     else
       llvm_unreachable("Invalid constraint; neither true or false.");
   }
 }
 
 void ExprInspectionChecker::analyzerEval(const CallExpr *CE,
                                          CheckerContext &C) const {
-  ExplodedNode *N = C.getPredecessor();
-  const LocationContext *LC = N->getLocationContext();
+  const LocationContext *LC = C.getPredecessor()->getLocationContext();
 
   // A specific instantiation of an inlined function may have more constrained
   // values than can generally be assumed. Skip the check.
   if (LC->getCurrentStackFrame()->getParent() != nullptr)
     return;
 
   if (!BT)
     BT.reset(new BugType(this, "Checking analyzer assumptions", "debug"));
 
+  ExplodedNode *N = C.generateNonFatalErrorNode();
+  if (!N)
+    return;
   C.emitReport(
       llvm::make_unique<BugReport>(*BT, getArgumentValueString(CE, C), N));
 }
 
 void ExprInspectionChecker::analyzerWarnIfReached(const CallExpr *CE,
                                                   CheckerContext &C) const {
-  ExplodedNode *N = C.getPredecessor();
 
   if (!BT)
     BT.reset(new BugType(this, "Checking analyzer assumptions", "debug"));
 
+  ExplodedNode *N = C.generateNonFatalErrorNode();
+  if (!N)
+    return;
   C.emitReport(llvm::make_unique<BugReport>(*BT, "REACHABLE", N));
 }
 
 void ExprInspectionChecker::analyzerCheckInlined(const CallExpr *CE,
                                                  CheckerContext &C) const {
-  ExplodedNode *N = C.getPredecessor();
-  const LocationContext *LC = N->getLocationContext();
+  const LocationContext *LC = C.getPredecessor()->getLocationContext();
 
   // An inlined function could conceivably also be analyzed as a top-level
   // function. We ignore this case and only emit a message (TRUE or FALSE)
   // when we are analyzing it as an inlined function. This means that
   // clang_analyzer_checkInlined(true) should always print TRUE, but
   // clang_analyzer_checkInlined(false) should never actually print anything.
   if (LC->getCurrentStackFrame()->getParent() == nullptr)
     return;
 
   if (!BT)
     BT.reset(new BugType(this, "Checking analyzer assumptions", "debug"));
 
+  ExplodedNode *N = C.generateNonFatalErrorNode();
+  if (!N)
+    return;
   C.emitReport(
       llvm::make_unique<BugReport>(*BT, getArgumentValueString(CE, C), N));
 }
 
 void ExprInspectionChecker::analyzerCrash(const CallExpr *CE,
                                           CheckerContext &C) const {
   LLVM_BUILTIN_TRAP;
 }
 
 void ento::registerExprInspectionChecker(CheckerManager &Mgr) {
   Mgr.registerChecker<ExprInspectionChecker>();
 }
 

cfe/trunk/lib/StaticAnalyzer/Checkers/FixedAddressChecker.cpp

if (!T->isPointerType())
     return;
 
   ProgramStateRef state = C.getState();
   SVal RV = state->getSVal(B->getRHS(), C.getLocationContext());
 
   if (!RV.isConstant() || RV.isZeroConstant())
     return;
 
-  if (ExplodedNode *N = C.addTransition()) {
+  if (ExplodedNode *N = C.generateNonFatalErrorNode()) {
     if (!BT)
       BT.reset(
           new BuiltinBug(this, "Use fixed address",
                          "Using a fixed address is not portable because that "
                          "address will probably not be valid in all "
                          "environments or platforms."));
     auto R = llvm::make_unique<BugReport>(*BT, BT->getDescription(), N);
     R->addRange(B->getRHS()->getSourceRange());
     C.emitReport(std::move(R));
   }
 }
 
 void ento::registerFixedAddressChecker(CheckerManager &mgr) {
   mgr.registerChecker<FixedAddressChecker>();
 }

cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp

bool GenericTaintChecker::generateReportIfTainted(const Expr *E,
 
   // Check for taint.
   ProgramStateRef State = C.getState();
   if (!State->isTainted(getPointedToSymbol(C, E)) &&
       !State->isTainted(E, C.getLocationContext()))
     return false;
 
   // Generate diagnostic.
-  if (ExplodedNode *N = C.addTransition()) {
+  if (ExplodedNode *N = C.generateNonFatalErrorNode()) {
     initBugType();
     auto report = llvm::make_unique<BugReport>(*BT, Msg, N);
     report->addRange(E->getSourceRange());
     C.emitReport(std::move(report));
     return true;
   }
   return false;
 }

cfe/trunk/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp

 // Report deallocator mismatch. Remove the region from tracking - reporting a
 // missing free error after this one is redundant.
 void MacOSKeychainAPIChecker::
   generateDeallocatorMismatchReport(const AllocationPair &AP,
                                     const Expr *ArgExpr,
                                     CheckerContext &C) const {
   ProgramStateRef State = C.getState();
   State = State->remove<AllocatedData>(AP.first);
-  ExplodedNode *N = C.addTransition(State);
+  ExplodedNode *N = C.generateNonFatalErrorNode(State);
 
   if (!N)
     return;
   initBugType();
   SmallString<80> sbuf;
   llvm::raw_svector_ostream os(sbuf);
   unsigned int PDeallocIdx =
                FunctionsToTrack[AP.second->AllocatorIdx].DeallocatorIdx;
if (idx != InvalidIdx) {
 
     const Expr *ArgExpr = CE->getArg(paramIdx);
     if (SymbolRef V = getAsPointeeSymbol(ArgExpr, C))
       if (const AllocationState *AS = State->get<AllocatedData>(V)) {
         if (!definitelyReturnedError(AS->Region, State, C.getSValBuilder())) {
           // Remove the value from the state. The new symbol will be added for
           // tracking when the second allocator is processed in checkPostStmt().
           State = State->remove<AllocatedData>(V);
-          ExplodedNode *N = C.addTransition(State);
+          ExplodedNode *N = C.generateNonFatalErrorNode(State);
           if (!N)
             return;
           initBugType();
           SmallString<128> sbuf;
           llvm::raw_svector_ostream os(sbuf);
           unsigned int DIdx = FunctionsToTrack[AS->AllocatorIdx].DeallocatorIdx;
           os << "Allocated data should be released before another call to "
               << "the allocator: missing a call to '"
void MacOSKeychainAPIChecker::checkPreStmt(const CallExpr *CE,
   // TODO: We might want a more precise diagnostic for double free
   // (that would involve tracking all the freed symbols in the checker state).
   if (!AS || RegionArgIsBad) {
     // It is possible that this is a false positive - the argument might
     // have entered as an enclosing function parameter.
     if (isEnclosingFunctionParam(ArgExpr))
       return;
 
-    ExplodedNode *N = C.addTransition(State);
+    ExplodedNode *N = C.generateNonFatalErrorNode(State);
     if (!N)
       return;
     initBugType();
     auto Report = llvm::make_unique<BugReport>(
         *BT, "Trying to free data which has not been allocated.", N);
     Report->addRange(ArgExpr->getSourceRange());
     if (AS)
       Report->markInteresting(AS->Region);
if (PDeallocIdx != idx || (FunctionsToTrack[idx].Kind == ErrorAPI)) {
     generateDeallocatorMismatchReport(AP, ArgExpr, C);
     return;
   }
 
   // If the buffer can be null and the return status can be an error,
   // report a bad call to free.
   if (State->assume(ArgSVal.castAs<DefinedSVal>(), false) &&
       !definitelyDidnotReturnError(AS->Region, State, C.getSValBuilder())) {
-    ExplodedNode *N = C.addTransition(State);
+    ExplodedNode *N = C.generateNonFatalErrorNode(State);
     if (!N)
       return;
     initBugType();
     auto Report = llvm::make_unique<BugReport>(
         *BT, "Only call free if a valid (non-NULL) buffer was returned.", N);
     Report->addVisitor(llvm::make_unique<SecKeychainBugVisitor>(ArgSM));
     Report->addRange(ArgExpr->getSourceRange());
     Report->markInteresting(AS->Region);
void MacOSKeychainAPIChecker::checkDeadSymbols(SymbolReaper &SR,
   }
   if (!Changed) {
     // Generate the new, cleaned up state.
     C.addTransition(State);
     return;
   }
 
   static CheckerProgramPointTag Tag(this, "DeadSymbolsLeak");
-  ExplodedNode *N = C.addTransition(C.getState(), C.getPredecessor(), &Tag);
+  ExplodedNode *N = C.generateNonFatalErrorNode(C.getState(), &Tag);
+  if (!N)
+    return;
 
   // Generate the error reports.
   for (const auto P : Errors)
     C.emitReport(generateAllocatedDataNotReleasedReport(P, N, C));
 
   // Generate the new, cleaned up state.
   C.addTransition(State, N);
 }

cfe/trunk/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp

void MacOSXAPIChecker::CheckDispatchOnce(CheckerContext &C, const CallExpr *CE,
   // Check if the first argument is stack allocated.  If so, issue a warning
   // because that's likely to be bad news.
   ProgramStateRef state = C.getState();
   const MemRegion *R =
     state->getSVal(CE->getArg(0), C.getLocationContext()).getAsRegion();
   if (!R || !isa<StackSpaceRegion>(R->getMemorySpace()))
     return;
 
-  ExplodedNode *N = C.generateSink(state);
+  ExplodedNode *N = C.generateErrorNode(state);
   if (!N)
     return;
 
   if (!BT_dispatchOnce)
     BT_dispatchOnce.reset(new BugType(this, "Improper use of 'dispatch_once'",
                                       "API Misuse (Apple)"));
 
   // Handle _dispatch_once.  In some versions of the OS X SDK we have the case

cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

if (!ChecksEnabled[CK_MallocChecker] &&
       !ChecksEnabled[CK_NewDeleteChecker])
     return;
 
   Optional<MallocChecker::CheckKind> CheckKind =
       getCheckIfTracked(C, DeallocExpr);
   if (!CheckKind.hasValue())
     return;
 
-  if (ExplodedNode *N = C.generateSink()) {
+  if (ExplodedNode *N = C.generateErrorNode()) {
     if (!BT_BadFree[*CheckKind])
       BT_BadFree[*CheckKind].reset(
           new BugType(CheckNames[*CheckKind], "Bad free", "Memory Error"));
 
     SmallString<100> buf;
     llvm::raw_svector_ostream os(buf);
 
     const MemRegion *MR = ArgVal.getAsRegion();
void MallocChecker::ReportFreeAlloca(CheckerContext &C, SVal ArgVal,
 
   if (ChecksEnabled[CK_MallocChecker])
     CheckKind = CK_MallocChecker;
   else if (ChecksEnabled[CK_MismatchedDeallocatorChecker])
     CheckKind = CK_MismatchedDeallocatorChecker;
   else
     return;
 
-  if (ExplodedNode *N = C.generateSink()) {
+  if (ExplodedNode *N = C.generateErrorNode()) {
     if (!BT_FreeAlloca[*CheckKind])
       BT_FreeAlloca[*CheckKind].reset(
           new BugType(CheckNames[*CheckKind], "Free alloca()", "Memory Error"));
 
     auto R = llvm::make_unique<BugReport>(
         *BT_FreeAlloca[*CheckKind],
         "Memory allocated by alloca() should not be deallocated", N);
     R->markInteresting(ArgVal.getAsRegion());
     R->addRange(Range);
     C.emitReport(std::move(R));
   }
 }
 
 void MallocChecker::ReportMismatchedDealloc(CheckerContext &C,
                                             SourceRange Range,
                                             const Expr *DeallocExpr,
                                             const RefState *RS,
                                             SymbolRef Sym,
                                             bool OwnershipTransferred) const {
 
   if (!ChecksEnabled[CK_MismatchedDeallocatorChecker])
     return;
 
-  if (ExplodedNode *N = C.generateSink()) {
+  if (ExplodedNode *N = C.generateErrorNode()) {
     if (!BT_MismatchedDealloc)
       BT_MismatchedDealloc.reset(
           new BugType(CheckNames[CK_MismatchedDeallocatorChecker],
                       "Bad deallocator", "Memory Error"));
 
     SmallString<100> buf;
     llvm::raw_svector_ostream os(buf);
 
if (!ChecksEnabled[CK_MallocChecker] &&
       !ChecksEnabled[CK_NewDeleteChecker])
     return;
 
   Optional<MallocChecker::CheckKind> CheckKind =
       getCheckIfTracked(C, AllocExpr);
   if (!CheckKind.hasValue())
     return;
 
-  ExplodedNode *N = C.generateSink();
+  ExplodedNode *N = C.generateErrorNode();
   if (!N)
     return;
 
   if (!BT_OffsetFree[*CheckKind])
     BT_OffsetFree[*CheckKind].reset(
         new BugType(CheckNames[*CheckKind], "Offset free", "Memory Error"));
 
   SmallString<100> buf;
void MallocChecker::ReportUseAfterFree(CheckerContext &C, SourceRange Range,
   if (!ChecksEnabled[CK_MallocChecker] &&
       !ChecksEnabled[CK_NewDeleteChecker])
     return;
 
   Optional<MallocChecker::CheckKind> CheckKind = getCheckIfTracked(C, Sym);
   if (!CheckKind.hasValue())
     return;
 
-  if (ExplodedNode *N = C.generateSink()) {
+  if (ExplodedNode *N = C.generateErrorNode()) {
     if (!BT_UseFree[*CheckKind])
       BT_UseFree[*CheckKind].reset(new BugType(
           CheckNames[*CheckKind], "Use-after-free", "Memory Error"));
 
     auto R = llvm::make_unique<BugReport>(*BT_UseFree[*CheckKind],
                                          "Use of memory after it is freed", N);
 
     R->markInteresting(Sym);
void MallocChecker::ReportDoubleFree(CheckerContext &C, SourceRange Range,
   if (!ChecksEnabled[CK_MallocChecker] &&
       !ChecksEnabled[CK_NewDeleteChecker])
     return;
 
   Optional<MallocChecker::CheckKind> CheckKind = getCheckIfTracked(C, Sym);
   if (!CheckKind.hasValue())
     return;
 
-  if (ExplodedNode *N = C.generateSink()) {
+  if (ExplodedNode *N = C.generateErrorNode()) {
     if (!BT_DoubleFree[*CheckKind])
       BT_DoubleFree[*CheckKind].reset(
           new BugType(CheckNames[*CheckKind], "Double free", "Memory Error"));
 
     auto R = llvm::make_unique<BugReport>(
         *BT_DoubleFree[*CheckKind],
         (Released ? "Attempt to free released memory"
                   : "Attempt to free non-owned memory"),
void MallocChecker::ReportDoubleDelete(CheckerContext &C, SymbolRef Sym) const {
 
   if (!ChecksEnabled[CK_NewDeleteChecker])
     return;
 
   Optional<MallocChecker::CheckKind> CheckKind = getCheckIfTracked(C, Sym);
   if (!CheckKind.hasValue())
     return;
 
-  if (ExplodedNode *N = C.generateSink()) {
+  if (ExplodedNode *N = C.generateErrorNode()) {
     if (!BT_DoubleDelete)
       BT_DoubleDelete.reset(new BugType(CheckNames[CK_NewDeleteChecker],
                                         "Double delete", "Memory Error"));
 
     auto R = llvm::make_unique<BugReport>(
         *BT_DoubleDelete, "Attempt to delete released memory", N);
 
     R->markInteresting(Sym);
if (!ChecksEnabled[CK_MallocChecker] &&
       !ChecksEnabled[CK_NewDeleteChecker])
     return;
 
   Optional<MallocChecker::CheckKind> CheckKind = getCheckIfTracked(C, Sym);
 
   if (!CheckKind.hasValue())
     return;
 
-  if (ExplodedNode *N = C.generateSink()) {
+  if (ExplodedNode *N = C.generateErrorNode()) {
     if (!BT_UseZerroAllocated[*CheckKind])
       BT_UseZerroAllocated[*CheckKind].reset(new BugType(
           CheckNames[*CheckKind], "Use of zero allocated", "Memory Error"));
 
     auto R = llvm::make_unique<BugReport>(*BT_UseZerroAllocated[*CheckKind],
                                          "Use of zero-allocated memory", N);
 
     R->addRange(Range);
if (SymReaper.isDead(I->first) ||
       state = state->remove<FreeReturnValue>(I->first);
     }
   }
 
   // Generate leak node.
   ExplodedNode *N = C.getPredecessor();
   if (!Errors.empty()) {
     static CheckerProgramPointTag Tag("MallocChecker", "DeadSymbolsLeak");
-    N = C.addTransition(C.getState(), C.getPredecessor(), &Tag);
+    N = C.generateNonFatalErrorNode(C.getState(), &Tag);
+    if (N) {
       for (SmallVectorImpl<SymbolRef>::iterator
            I = Errors.begin(), E = Errors.end(); I != E; ++I) {
         reportLeak(*I, N, C);
       }
     }
+  }
 
   C.addTransition(state->set<RegionState>(RS), N);
 }
 
 void MallocChecker::checkPreCall(const CallEvent &Call,
                                  CheckerContext &C) const {
 
   if (const CXXDestructorCall *DC = dyn_cast<CXXDestructorCall>(&Call)) {

cfe/trunk/lib/StaticAnalyzer/Checkers/NSAutoreleasePoolChecker.cpp

void NSAutoreleasePoolChecker::checkPreObjCMessage(const ObjCMethodCall &msg,
   // Sending 'release' message?
   if (msg.getSelector() != releaseS)
     return;
 
   if (!BT)
     BT.reset(new BugType(this, "Use -drain instead of -release",
                          "API Upgrade (Apple)"));
 
-  ExplodedNode *N = C.addTransition();
+  ExplodedNode *N = C.generateNonFatalErrorNode();
   if (!N) {
     assert(0);
     return;
   }
 
   auto Report = llvm::make_unique<BugReport>(
       *BT, "Use -drain instead of -release when using NSAutoreleasePool and "
            "garbage collection", N);
   Report->addRange(msg.getSourceRange());
   C.emitReport(std::move(Report));
 }
 
 void ento::registerNSAutoreleasePoolChecker(CheckerManager &mgr) {
   if (mgr.getLangOpts().getGC() != LangOptions::NonGC)
     mgr.registerChecker<NSAutoreleasePoolChecker>();
 }

cfe/trunk/lib/StaticAnalyzer/Checkers/NoReturnFunctionChecker.cpp

if (const IdentifierInfo *II = CE.getCalleeIdentifier()) {
             .Case("_XCAssertionFailureHandler", true)
             .Case("_DTAssertionFailureHandler", true)
             .Case("_TSAssertionFailureHandler", true)
             .Default(false);
     }
   }
 
   if (BuildSinks)
-    C.generateSink();
+    C.generateSink(C.getState(), C.getPredecessor());
 }
 
 void NoReturnFunctionChecker::checkPostObjCMessage(const ObjCMethodCall &Msg,
                                                    CheckerContext &C) const {
   // Check if the method is annotated with analyzer_noreturn.
   if (const ObjCMethodDecl *MD = Msg.getDecl()) {
     MD = MD->getCanonicalDecl();
     if (MD->hasAttr<AnalyzerNoReturnAttr>()) {
-      C.generateSink();
+      C.generateSink(C.getState(), C.getPredecessor());
       return;
     }
   }
 
   // HACK: This entire check is to handle two messages in the Cocoa frameworks:
   // -[NSAssertionHandler
   //    handleFailureInMethod:object:file:lineNumber:description:]
   // -[NSAssertionHandler
lazyInitKeywordSelector(HandleFailureInMethodSel, C.getASTContext(),
                             "handleFailureInMethod", "object", "file",
                             "lineNumber", "description", nullptr);
     if (Sel != HandleFailureInMethodSel)
       return;
     break;
   }
 
   // If we got here, it's one of the messages we care about.
-  C.generateSink();
+  C.generateSink(C.getState(), C.getPredecessor());
 }
 
 void ento::registerNoReturnFunctionChecker(CheckerManager &mgr) {
   mgr.registerChecker<NoReturnFunctionChecker>();
 }

cfe/trunk/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp

for (unsigned idx = 0; idx < NumArgs; ++idx) {
     ConstraintManager &CM = C.getConstraintManager();
     ProgramStateRef stateNotNull, stateNull;
     std::tie(stateNotNull, stateNull) = CM.assumeDual(state, *DV);
 
     if (stateNull) {
       if (!stateNotNull) {
         // Generate an error node.  Check for a null node in case
         // we cache out.
-        if (ExplodedNode *errorNode = C.generateSink(stateNull)) {
+        if (ExplodedNode *errorNode = C.generateErrorNode(stateNull)) {
 
           std::unique_ptr<BugReport> R;
           if (haveAttrNonNull)
             R = genReportNullAttrNonNull(errorNode, ArgE);
           else if (haveRefTypeParam)
             R = genReportReferenceToNullPointer(errorNode, ArgE);
 
           // Highlight the range of the argument that was null.
           R->addRange(Call.getArgSourceRange(idx));
 
           // Emit the bug report.
           C.emitReport(std::move(R));
         }
 
         // Always return.  Either we cached out or we just emitted an error.
         return;
       }
-      if (ExplodedNode *N = C.generateSink(stateNull)) {
+      if (ExplodedNode *N = C.generateSink(stateNull, C.getPredecessor())) {
         ImplicitNullDerefEvent event = {
             V, false, N, &C.getBugReporter(),
             /*IsDirectDereference=*/haveRefTypeParam};
         dispatchEvent(event);
       }
     }
 
     // If a pointer value passed the check we should assume that it is

cfe/trunk/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp

void NullabilityChecker::checkPreStmt(const ReturnStmt *S,
 
   Nullability StaticNullability =
       getNullabilityAnnotation(FuncType->getReturnType());
 
   if (Filter.CheckNullReturnedFromNonnull &&
       Nullness == NullConstraint::IsNull &&
       StaticNullability == Nullability::Nonnull) {
     static CheckerProgramPointTag Tag(this, "NullReturnedFromNonnull");
-    ExplodedNode *N = C.generateSink(State, C.getPredecessor(), &Tag);
+    ExplodedNode *N = C.generateErrorNode(State, &Tag);
+    if (!N)
+      return;
     reportBugIfPreconditionHolds(ErrorKind::NilReturnedToNonnull, N, nullptr, C,
                                  RetExpr);
     return;
   }
 
   const MemRegion *Region = getTrackRegion(*RetSVal);
   if (!Region)
     return;
for (const ParmVarDecl *Param : Call.parameters()) {
 
     Nullability ParamNullability = getNullabilityAnnotation(Param->getType());
     Nullability ArgStaticNullability =
         getNullabilityAnnotation(ArgExpr->getType());
 
     if (Filter.CheckNullPassedToNonnull && Nullness == NullConstraint::IsNull &&
         ArgStaticNullability != Nullability::Nonnull &&
         ParamNullability == Nullability::Nonnull) {
-      ExplodedNode *N = C.generateSink(State);
+      ExplodedNode *N = C.generateErrorNode(State);
+      if (!N)
+        return;
       reportBugIfPreconditionHolds(ErrorKind::NilPassedToNonnull, N, nullptr, C,
                                    ArgExpr);
       return;
     }
 
     const MemRegion *Region = getTrackRegion(*ArgSVal);
     if (!Region)
       continue;
if (SymbolRef Sym = ValDefOrUnknown->getAsSymbol())
     ValNullability = getNullabilityAnnotation(Sym->getType());
 
   Nullability LocNullability = getNullabilityAnnotation(LocType);
   if (Filter.CheckNullPassedToNonnull &&
       RhsNullness == NullConstraint::IsNull &&
       ValNullability != Nullability::Nonnull &&
       LocNullability == Nullability::Nonnull) {
     static CheckerProgramPointTag Tag(this, "NullPassedToNonnull");
-    ExplodedNode *N = C.generateSink(State, C.getPredecessor(), &Tag);
+    ExplodedNode *N = C.generateErrorNode(State, &Tag);
+    if (!N)
+      return;
     reportBugIfPreconditionHolds(ErrorKind::NilAssignedToNonnull, N, nullptr, C,
                                  S);
     return;
   }
   // Intentionally missing case: '0' is bound to a reference. It is handled by
   // the DereferenceChecker.
 
   const MemRegion *ValueRegion = getTrackRegion(*ValDefOrUnknown);

cfe/trunk/lib/StaticAnalyzer/Checkers/ObjCAtSyncChecker.cpp

void ObjCAtSyncChecker::checkPreStmt(const ObjCAtSynchronizedStmt *S,
                                      CheckerContext &C) const {
 
   const Expr *Ex = S->getSynchExpr();
   ProgramStateRef state = C.getState();
   SVal V = state->getSVal(Ex, C.getLocationContext());
 
   // Uninitialized value used for the mutex?
   if (V.getAs<UndefinedVal>()) {
-    if (ExplodedNode *N = C.generateSink()) {
+    if (ExplodedNode *N = C.generateErrorNode()) {
       if (!BT_undef)
         BT_undef.reset(new BuiltinBug(this, "Uninitialized value used as mutex "
                                             "for @synchronized"));
       auto report =
           llvm::make_unique<BugReport>(*BT_undef, BT_undef->getDescription(), N);
       bugreporter::trackNullOrUndefValue(N, Ex, *report);
       C.emitReport(std::move(report));
     }
     return;
   }
 
   if (V.isUnknown())
     return;
 
   // Check for null mutexes.
   ProgramStateRef notNullState, nullState;
   std::tie(notNullState, nullState) = state->assume(V.castAs<DefinedSVal>());
 
   if (nullState) {
     if (!notNullState) {
       // Generate an error node.  This isn't a sink since
       // a null mutex just means no synchronization occurs.
-      if (ExplodedNode *N = C.addTransition(nullState)) {
+      if (ExplodedNode *N = C.generateNonFatalErrorNode(nullState)) {
         if (!BT_null)
           BT_null.reset(new BuiltinBug(
               this, "Nil value used as mutex for @synchronized() "
                     "(no synchronization will occur)"));
         auto report =
             llvm::make_unique<BugReport>(*BT_null, BT_null->getDescription(), N);
         bugreporter::trackNullOrUndefValue(N, Ex, *report);
 

cfe/trunk/lib/StaticAnalyzer/Checkers/ObjCContainersChecker.cpp

if (IdxVal.isUnknownOrUndef())
       return;
     DefinedSVal Idx = IdxVal.castAs<DefinedSVal>();
 
     // Now, check if 'Idx in [0, Size-1]'.
     const QualType T = IdxExpr->getType();
     ProgramStateRef StInBound = State->assumeInBound(Idx, *Size, true, T);
     ProgramStateRef StOutBound = State->assumeInBound(Idx, *Size, false, T);
     if (StOutBound && !StInBound) {
-      ExplodedNode *N = C.generateSink(StOutBound);
+      ExplodedNode *N = C.generateErrorNode(StOutBound);
       if (!N)
         return;
       initBugType();
       auto R = llvm::make_unique<BugReport>(*BT, "Index is out of bounds", N);
       R->addRange(IdxExpr->getSourceRange());
       C.emitReport(std::move(R));
       return;
     }

cfe/trunk/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp

void ObjCSelfInitChecker::checkForInvalidSelf(const Expr *E, CheckerContext &C,
 
   if (!C.getState()->get<CalledInit>())
     return;
 
   if (!isInvalidSelf(E, C))
     return;
 
   // Generate an error node.
-  ExplodedNode *N = C.generateSink();
+  ExplodedNode *N = C.generateErrorNode();
   if (!N)
     return;
 
   if (!BT)
     BT.reset(new BugType(this, "Missing \"self = [(super or self) init...]\"",
                          categories::CoreFoundationObjectiveC));
   C.emitReport(llvm::make_unique<BugReport>(*BT, errorStr, N));
 }

cfe/trunk/lib/StaticAnalyzer/Checkers/PointerArithChecker.cpp

void PointerArithChecker::checkPreStmt(const BinaryOperator *B,
   if (!LR || !RV.isConstant())
     return;
 
   // If pointer arithmetic is done on variables of non-array type, this often
   // means behavior rely on memory organization, which is dangerous.
   if (isa<VarRegion>(LR) || isa<CodeTextRegion>(LR) ||
       isa<CompoundLiteralRegion>(LR)) {
 
-    if (ExplodedNode *N = C.addTransition()) {
+    if (ExplodedNode *N = C.generateNonFatalErrorNode()) {
       if (!BT)
         BT.reset(
             new BuiltinBug(this, "Dangerous pointer arithmetic",
                            "Pointer arithmetic done on non-array variables "
                            "means reliance on memory layout, which is "
                            "dangerous."));
       auto R = llvm::make_unique<BugReport>(*BT, BT->getDescription(), N);
       R->addRange(B->getSourceRange());
       C.emitReport(std::move(R));
     }
   }
 }
 
 void ento::registerPointerArithChecker(CheckerManager &mgr) {
   mgr.registerChecker<PointerArithChecker>();
 }

cfe/trunk/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp

void PointerSubChecker::checkPreStmt(const BinaryOperator *B,
 
   if (BaseLR == BaseRR)
     return;
 
   // Allow arithmetic on different symbolic regions.
   if (isa<SymbolicRegion>(BaseLR) || isa<SymbolicRegion>(BaseRR))
     return;
 
-  if (ExplodedNode *N = C.addTransition()) {
+  if (ExplodedNode *N = C.generateNonFatalErrorNode()) {
     if (!BT)
       BT.reset(
           new BuiltinBug(this, "Pointer subtraction",
                          "Subtraction of two pointers that do not point to "
                          "the same memory chunk may cause incorrect result."));
     auto R = llvm::make_unique<BugReport>(*BT, BT->getDescription(), N);
     R->addRange(B->getSourceRange());
     C.emitReport(std::move(R));
   }
 }
 
 void ento::registerPointerSubChecker(CheckerManager &mgr) {
   mgr.registerChecker<PointerSubChecker>();
 }

cfe/trunk/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp

void PthreadLockChecker::AcquireLock(CheckerContext &C, const CallExpr *CE,
 
   DefinedSVal retVal = X.castAs<DefinedSVal>();
 
   if (const LockState *LState = state->get<LockMap>(lockR)) {
     if (LState->isLocked()) {
       if (!BT_doublelock)
         BT_doublelock.reset(new BugType(this, "Double locking",
                                         "Lock checker"));
-      ExplodedNode *N = C.generateSink();
+      ExplodedNode *N = C.generateErrorNode();
       if (!N)
         return;
       auto report = llvm::make_unique<BugReport>(
           *BT_doublelock, "This lock has already been acquired", N);
       report->addRange(CE->getArg(0)->getSourceRange());
       C.emitReport(std::move(report));
       return;
     } else if (LState->isDestroyed()) {
void PthreadLockChecker::ReleaseLock(CheckerContext &C, const CallExpr *CE,
 
   ProgramStateRef state = C.getState();
 
   if (const LockState *LState = state->get<LockMap>(lockR)) {
     if (LState->isUnlocked()) {
       if (!BT_doubleunlock)
         BT_doubleunlock.reset(new BugType(this, "Double unlocking",
                                           "Lock checker"));
-      ExplodedNode *N = C.generateSink();
+      ExplodedNode *N = C.generateErrorNode();
       if (!N)
         return;
       auto Report = llvm::make_unique<BugReport>(
           *BT_doubleunlock, "This lock has already been unlocked", N);
       Report->addRange(CE->getArg(0)->getSourceRange());
       C.emitReport(std::move(Report));
       return;
     } else if (LState->isDestroyed()) {
       reportUseDestroyedBug(C, CE);
       return;
     }
   }
 
   LockSetTy LS = state->get<LockSet>();
 
   // FIXME: Better analysis requires IPA for wrappers.
 
   if (!LS.isEmpty()) {
     const MemRegion *firstLockR = LS.getHead();
     if (firstLockR != lockR) {
       if (!BT_lor)
         BT_lor.reset(new BugType(this, "Lock order reversal", "Lock checker"));
-      ExplodedNode *N = C.generateSink();
+      ExplodedNode *N = C.generateErrorNode();
       if (!N)
         return;
       auto report = llvm::make_unique<BugReport>(
           *BT_lor, "This was not the most recently acquired lock. Possible "
                    "lock order reversal", N);
       report->addRange(CE->getArg(0)->getSourceRange());
       C.emitReport(std::move(report));
       return;
if (LState->isLocked()) {
     Message = "This lock is still locked";
   } else {
     Message = "This lock has already been destroyed";
   }
 
   if (!BT_destroylock)
     BT_destroylock.reset(new BugType(this, "Destroy invalid lock",
                                      "Lock checker"));
-  ExplodedNode *N = C.generateSink();
+  ExplodedNode *N = C.generateErrorNode();
   if (!N)
     return;
   auto Report = llvm::make_unique<BugReport>(*BT_destroylock, Message, N);
   Report->addRange(CE->getArg(0)->getSourceRange());
   C.emitReport(std::move(Report));
 }
 
 void PthreadLockChecker::InitLock(CheckerContext &C, const CallExpr *CE,
if (LState->isLocked()) {
     Message = "This lock is still being held";
   } else {
     Message = "This lock has already been initialized";
   }
 
   if (!BT_initlock)
     BT_initlock.reset(new BugType(this, "Init invalid lock",
                                   "Lock checker"));
-  ExplodedNode *N = C.generateSink();
+  ExplodedNode *N = C.generateErrorNode();
   if (!N)
     return;
   auto Report = llvm::make_unique<BugReport>(*BT_initlock, Message, N);
   Report->addRange(CE->getArg(0)->getSourceRange());
   C.emitReport(std::move(Report));
 }
 
 void PthreadLockChecker::reportUseDestroyedBug(CheckerContext &C,
                                                const CallExpr *CE) const {
   if (!BT_destroylock)
     BT_destroylock.reset(new BugType(this, "Use destroyed lock",
                                      "Lock checker"));
-  ExplodedNode *N = C.generateSink();
+  ExplodedNode *N = C.generateErrorNode();
   if (!N)
     return;
   auto Report = llvm::make_unique<BugReport>(
       *BT_destroylock, "This lock has already been destroyed", N);
   Report->addRange(CE->getArg(0)->getSourceRange());
   C.emitReport(std::move(Report));
 }
 
 void ento::registerPthreadLockChecker(CheckerManager &mgr) {
   mgr.registerChecker<PthreadLockChecker>();
 }

cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp

void RetainCountChecker::processNonLeakError(ProgramStateRef St,
   //   [_contentView retain];
   //   [_contentView removeFromSuperview];
   //   [self addSubview:_contentView]; // invalidates 'self'
   //   [_contentView release];
   if (const RefVal *RV = getRefBinding(St, Sym))
     if (RV->getIvarAccessHistory() != RefVal::IvarAccessHistory::None)
       return;
 
-  ExplodedNode *N = C.generateSink(St);
+  ExplodedNode *N = C.generateErrorNode(St);
   if (!N)
     return;
 
   CFRefBug *BT;
   switch (ErrorKind) {
     default:
       llvm_unreachable("Unhandled error.");
     case RefVal::ErrorUseAfterRelease:

cfe/trunk/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp

void ReturnPointerRangeChecker::checkPreStmt(const ReturnStmt *RS,
 
   DefinedOrUnknownSVal NumElements
     = C.getStoreManager().getSizeInElements(state, ER->getSuperRegion(),
                                            ER->getValueType());
 
   ProgramStateRef StInBound = state->assumeInBound(Idx, NumElements, true);
   ProgramStateRef StOutBound = state->assumeInBound(Idx, NumElements, false);
   if (StOutBound && !StInBound) {
-    ExplodedNode *N = C.generateSink(StOutBound);
+    ExplodedNode *N = C.generateErrorNode(StOutBound);
 
     if (!N)
       return;
 
     // FIXME: This bug correspond to CWE-466.  Eventually we should have bug
     // types explicitly reference such exploit categories (when applicable).
     if (!BT)
       BT.reset(new BuiltinBug(

cfe/trunk/lib/StaticAnalyzer/Checkers/ReturnUndefChecker.cpp

void ReturnUndefChecker::checkPreStmt(const ReturnStmt *RS,
   if (RT->isReferenceType()) {
     checkReference(C, RetE, RetVal.castAs<DefinedOrUnknownSVal>());
     return;
   }
 }
 
 static void emitBug(CheckerContext &C, BuiltinBug &BT, const Expr *RetE,
                     const Expr *TrackingE = nullptr) {
-  ExplodedNode *N = C.generateSink();
+  ExplodedNode *N = C.generateErrorNode();
   if (!N)
     return;
 
   auto Report = llvm::make_unique<BugReport>(BT, BT.getDescription(), N);
 
   Report->addRange(RetE->getSourceRange());
   bugreporter::trackNullOrUndefValue(N, TrackingE ? TrackingE : RetE, *Report);
 

cfe/trunk/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp

for (StreamMapTy::iterator I = TrackedStreams.begin(),
     if (isLeaked(Sym, I->second, IsSymDead, State))
       LeakedStreams.push_back(Sym);
 
     // Remove the dead symbol from the streams map.
     if (IsSymDead)
       State = State->remove<StreamMap>(Sym);
   }
 
-  ExplodedNode *N = C.addTransition(State);
+  ExplodedNode *N = C.generateNonFatalErrorNode(State);
+  if (!N)
+    return;
   reportLeaks(LeakedStreams, C, N);
 }
 
 void SimpleStreamChecker::reportDoubleClose(SymbolRef FileDescSym,
                                             const CallEvent &Call,
                                             CheckerContext &C) const {
   // We reached a bug, stop exploring the path here by generating a sink.
-  ExplodedNode *ErrNode = C.generateSink();
+  ExplodedNode *ErrNode = C.generateErrorNode();
   // If we've already reached this node on another path, return.
   if (!ErrNode)
     return;
 
   // Generate the report.
   auto R = llvm::make_unique<BugReport>(*DoubleCloseBugType,
       "Closing a previously closed file stream", ErrNode);
   R->addRange(Call.getSourceRange());

cfe/trunk/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp

else {
     llvm_unreachable("Invalid region in ReturnStackAddressChecker.");
   }
 
   return range;
 }
 
 void StackAddrEscapeChecker::EmitStackError(CheckerContext &C, const MemRegion *R,
                                           const Expr *RetE) const {
-  ExplodedNode *N = C.generateSink();
+  ExplodedNode *N = C.generateErrorNode();
 
   if (!N)
     return;
 
   if (!BT_returnstack)
     BT_returnstack.reset(
         new BuiltinBug(this, "Return of address to stack-allocated memory"));
 
void StackAddrEscapeChecker::checkEndFunction(CheckerContext &Ctx) const {
 
   CallBack cb(Ctx);
   state->getStateManager().getStoreManager().iterBindings(state->getStore(),cb);
 
   if (cb.V.empty())
     return;
 
   // Generate an error node.
-  ExplodedNode *N = Ctx.addTransition(state);
+  ExplodedNode *N = Ctx.generateNonFatalErrorNode(state);
   if (!N)
     return;
 
   if (!BT_stackleak)
     BT_stackleak.reset(
         new BuiltinBug(this, "Stack address stored into global variable",
                        "Stack address was saved into a global variable. "
                        "This is dangerous because the address will become "

cfe/trunk/lib/StaticAnalyzer/Checkers/StreamChecker.cpp

void StreamChecker::Fseek(CheckerContext &C, const CallExpr *CE) const {
 
   if (!CI)
     return;
 
   int64_t x = CI->getValue().getSExtValue();
   if (x >= 0 && x <= 2)
     return;
 
-  if (ExplodedNode *N = C.addTransition(state)) {
+  if (ExplodedNode *N = C.generateNonFatalErrorNode(state)) {
     if (!BT_illegalwhence)
       BT_illegalwhence.reset(
           new BuiltinBug(this, "Illegal whence argument",
                          "The whence argument to fseek() should be "
                          "SEEK_SET, SEEK_END, or SEEK_CUR."));
     C.emitReport(llvm::make_unique<BugReport>(
         *BT_illegalwhence, BT_illegalwhence->getDescription(), N));
   }
ProgramStateRef StreamChecker::CheckNullStream(SVal SV, ProgramStateRef state,
   if (!DV)
     return nullptr;
 
   ConstraintManager &CM = C.getConstraintManager();
   ProgramStateRef stateNotNull, stateNull;
   std::tie(stateNotNull, stateNull) = CM.assumeDual(state, *DV);
 
   if (!stateNotNull && stateNull) {
-    if (ExplodedNode *N = C.generateSink(stateNull)) {
+    if (ExplodedNode *N = C.generateErrorNode(stateNull)) {
       if (!BT_nullfp)
         BT_nullfp.reset(new BuiltinBug(this, "NULL stream pointer",
                                        "Stream pointer might be NULL."));
       C.emitReport(llvm::make_unique<BugReport>(
           *BT_nullfp, BT_nullfp->getDescription(), N));
     }
     return nullptr;
   }
ProgramStateRef StreamChecker::CheckDoubleClose(const CallExpr *CE,
 
   // If the file stream is not tracked, return.
   if (!SS)
     return state;
 
   // Check: Double close a File Descriptor could cause undefined behaviour.
   // Conforming to man-pages
   if (SS->isClosed()) {
-    ExplodedNode *N = C.generateSink();
+    ExplodedNode *N = C.generateErrorNode();
     if (N) {
       if (!BT_doubleclose)
         BT_doubleclose.reset(new BuiltinBug(
             this, "Double fclose", "Try to close a file Descriptor already"
                                    " closed. Cause undefined behaviour."));
       C.emitReport(llvm::make_unique<BugReport>(
           *BT_doubleclose, BT_doubleclose->getDescription(), N));
     }
for (SymbolReaper::dead_iterator I = SymReaper.dead_begin(),
          E = SymReaper.dead_end(); I != E; ++I) {
     SymbolRef Sym = *I;
     ProgramStateRef state = C.getState();
     const StreamState *SS = state->get<StreamMap>(Sym);
     if (!SS)
       continue;
 
     if (SS->isOpened()) {
-      ExplodedNode *N = C.generateSink();
+      ExplodedNode *N = C.generateErrorNode();
       if (N) {
         if (!BT_ResourceLeak)
           BT_ResourceLeak.reset(new BuiltinBug(
               this, "Resource Leak",
               "Opened File never closed. Potential Resource leak."));
         C.emitReport(llvm::make_unique<BugReport>(
             *BT_ResourceLeak, BT_ResourceLeak->getDescription(), N));
       }
     }
   }
 }
 
 void ento::registerStreamChecker(CheckerManager &mgr) {
   mgr.registerChecker<StreamChecker>();
 }

cfe/trunk/lib/StaticAnalyzer/Checkers/TaintTesterChecker.cpp

 
 void TaintTesterChecker::checkPostStmt(const Expr *E,
                                        CheckerContext &C) const {
   ProgramStateRef State = C.getState();
   if (!State)
     return;
 
   if (State->isTainted(E, C.getLocationContext())) {
-    if (ExplodedNode *N = C.addTransition()) {
+    if (ExplodedNode *N = C.generateNonFatalErrorNode()) {
       initBugType();
       auto report = llvm::make_unique<BugReport>(*BT, "tainted",N);
       report->addRange(E->getSourceRange());
       C.emitReport(std::move(report));
     }
   }
 }
 
 void ento::registerTaintTesterChecker(CheckerManager &mgr) {
   mgr.registerChecker<TaintTesterChecker>();
 }

cfe/trunk/lib/StaticAnalyzer/Checkers/TestAfterDivZeroChecker.cpp

bool TestAfterDivZeroChecker::hasDivZeroMap(SVal Var,
   if (!SR)
     return false;
 
   ZeroState ZS(SR, C.getBlockID(), C.getStackFrame());
   return C.getState()->contains<DivZeroMap>(ZS);
 }
 
 void TestAfterDivZeroChecker::reportBug(SVal Val, CheckerContext &C) const {
-  if (ExplodedNode *N = C.generateSink(C.getState())) {
+  if (ExplodedNode *N = C.generateErrorNode(C.getState())) {
     if (!DivZeroBug)
       DivZeroBug.reset(new BuiltinBug(this, "Division by zero"));
 
     auto R = llvm::make_unique<BugReport>(
         *DivZeroBug, "Value being compared against zero has already been used "
                      "for division",
         N);
 

cfe/trunk/lib/StaticAnalyzer/Checkers/UndefBranchChecker.cpp

 }
 
 void UndefBranchChecker::checkBranchCondition(const Stmt *Condition,
                                               CheckerContext &Ctx) const {
   SVal X = Ctx.getState()->getSVal(Condition, Ctx.getLocationContext());
   if (X.isUndef()) {
     // Generate a sink node, which implicitly marks both outgoing branches as
     // infeasible.
-    ExplodedNode *N = Ctx.generateSink();
+    ExplodedNode *N = Ctx.generateErrorNode();
     if (N) {
       if (!BT)
         BT.reset(new BuiltinBug(
             this, "Branch condition evaluates to a garbage value"));
 
       // What's going on here: we want to highlight the subexpression of the
       // condition that is the most likely source of the "uninitialized
       // branch condition."  We do a recursive walk of the condition's

cfe/trunk/lib/StaticAnalyzer/Checkers/UndefCapturedBlockVarChecker.cpp

for (; I != E; ++I) {
     const VarDecl *VD = VR->getDecl();
 
     if (VD->hasAttr<BlocksAttr>() || !VD->hasLocalStorage())
       continue;
 
     // Get the VarRegion associated with VD in the local stack frame.
     if (Optional<UndefinedVal> V =
           state->getSVal(I.getOriginalRegion()).getAs<UndefinedVal>()) {
-      if (ExplodedNode *N = C.generateSink()) {
+      if (ExplodedNode *N = C.generateErrorNode()) {
         if (!BT)
           BT.reset(
               new BuiltinBug(this, "uninitialized variable captured by block"));
 
         // Generate a bug report.
         SmallString<128> buf;
         llvm::raw_svector_ostream os(buf);
 

cfe/trunk/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp

if (state->getSVal(B, LCtx).isUndef()) {
     // This should allow to swap partially uninitialized structs
     // (radar://14129997)
     if (const FunctionDecl *EnclosingFunctionDecl =
         dyn_cast<FunctionDecl>(C.getStackFrame()->getDecl()))
       if (C.getCalleeName(EnclosingFunctionDecl) == "swap")
         return;
 
     // Generate an error node.
-    ExplodedNode *N = C.generateSink();
+    ExplodedNode *N = C.generateErrorNode();
     if (!N)
       return;
 
     if (!BT)
       BT.reset(
           new BuiltinBug(this, "Result of operation is garbage or undefined"));
 
     SmallString<256> sbuf;

cfe/trunk/lib/StaticAnalyzer/Checkers/UndefinedArraySubscriptChecker.cpp

UndefinedArraySubscriptChecker::checkPreStmt(const ArraySubscriptExpr *A,
 
   // Sema generates anonymous array variables for copying array struct fields.
   // Don't warn if we're in an implicitly-generated constructor.
   const Decl *D = C.getLocationContext()->getDecl();
   if (const CXXConstructorDecl *Ctor = dyn_cast<CXXConstructorDecl>(D))
     if (Ctor->isDefaulted())
       return;
 
-  ExplodedNode *N = C.generateSink();
+  ExplodedNode *N = C.generateErrorNode();
   if (!N)
     return;
   if (!BT)
     BT.reset(new BuiltinBug(this, "Array subscript is undefined"));
 
   // Generate a report for this bug.
   auto R = llvm::make_unique<BugReport>(*BT, BT->getName(), N);
   R->addRange(A->getIdx()->getSourceRange());
   bugreporter::trackNullOrUndefValue(N, A->getIdx(), *R);
   C.emitReport(std::move(R));
 }
 
 void ento::registerUndefinedArraySubscriptChecker(CheckerManager &mgr) {
   mgr.registerChecker<UndefinedArraySubscriptChecker>();
 }

cfe/trunk/lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp

void UndefinedAssignmentChecker::checkBind(SVal location, SVal val,
   // Do not report assignments of uninitialized values inside swap functions.
   // This should allow to swap partially uninitialized structs
   // (radar://14129997)
   if (const FunctionDecl *EnclosingFunctionDecl =
       dyn_cast<FunctionDecl>(C.getStackFrame()->getDecl()))
     if (C.getCalleeName(EnclosingFunctionDecl) == "swap")
       return;
 
-  ExplodedNode *N = C.generateSink();
+  ExplodedNode *N = C.generateErrorNode();
 
   if (!N)
     return;
 
   const char *str = "Assigned value is garbage or undefined";
 
   if (!BT)
     BT.reset(new BuiltinBug(this, str));

cfe/trunk/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp

 //===----------------------------------------------------------------------===//
 // "open" (man 2 open)
 //===----------------------------------------------------------------------===//
 
 void UnixAPIChecker::ReportOpenBug(CheckerContext &C,
                                    ProgramStateRef State,
                                    const char *Msg,
                                    SourceRange SR) const {
-  ExplodedNode *N = C.generateSink(State);
+  ExplodedNode *N = C.generateErrorNode(State);
   if (!N)
     return;
 
   LazyInitialize(BT_open, "Improper use of 'open'");
 
   auto Report = llvm::make_unique<BugReport>(*BT_open, Msg, N);
   Report->addRange(SR);
   C.emitReport(std::move(Report));
void UnixAPIChecker::CheckPthreadOnce(CheckerContext &C,
   // Check if the first argument is stack allocated.  If so, issue a warning
   // because that's likely to be bad news.
   ProgramStateRef state = C.getState();
   const MemRegion *R =
     state->getSVal(CE->getArg(0), C.getLocationContext()).getAsRegion();
   if (!R || !isa<StackSpaceRegion>(R->getMemorySpace()))
     return;
 
-  ExplodedNode *N = C.generateSink(state);
+  ExplodedNode *N = C.generateErrorNode(state);
   if (!N)
     return;
 
   SmallString<256> S;
   llvm::raw_svector_ostream os(S);
   os << "Call to 'pthread_once' uses";
   if (const VarRegion *VR = dyn_cast<VarRegion>(R))
     os << " the local variable '" << VR->getDecl()->getName() << '\'';

 
 // Generates an error report, indicating that the function whose name is given
 // will perform a zero byte allocation.
 // Returns false if an error occurred, true otherwise.
 bool UnixAPIChecker::ReportZeroByteAllocation(CheckerContext &C,
                                               ProgramStateRef falseState,
                                               const Expr *arg,
                                               const char *fn_name) const {
-  ExplodedNode *N = C.generateSink(falseState);
+  ExplodedNode *N = C.generateErrorNode(falseState);
   if (!N)
     return false;
 
   LazyInitialize(BT_mallocZero,
                  "Undefined allocation of 0 bytes (CERT MEM04-C; CWE-131)");
 
   SmallString<256> S;
   llvm::raw_svector_ostream os(S);

cfe/trunk/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp

 };
 } // end anonymous namespace
 
 void VLASizeChecker::reportBug(VLASize_Kind Kind,
                                const Expr *SizeE,
                                ProgramStateRef State,
                                CheckerContext &C) const {
   // Generate an error node.
-  ExplodedNode *N = C.generateSink(State);
+  ExplodedNode *N = C.generateErrorNode(State);
   if (!N)
     return;
 
   if (!BT)
     BT.reset(new BuiltinBug(
         this, "Dangerous variable-length array (VLA) declaration"));
 
   SmallString<256> buf;

cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp

 }
 
 void BugReporter::Register(BugType *BT) {
   BugTypes = F.add(BugTypes, BT);
 }
 
 void BugReporter::emitReport(std::unique_ptr<BugReport> R) {
   if (const ExplodedNode *E = R->getErrorNode()) {
+    // An error node must either be a sink or have a tag, otherwise
+    // it could get reclaimed before the path diagnostic is created.
+    assert((E->isSink() || E->getLocation().getTag()) &&
+            "Error node must either be a sink or have a tag");
+
     const AnalysisDeclContext *DeclCtx =
         E->getLocationContext()->getAnalysisDeclContext();
     // The source of autosynthesized body can be handcrafted AST or a model
     // file. The locations from handcrafted ASTs have no valid source locations
     // and have to be discarded. Locations from model files should be preserved
     // for processing and reporting.
     if (DeclCtx->isBodyAutosynthesized() &&
         !DeclCtx->isBodyAutosynthesizedFromModelFile())

cfe/trunk/test/Analysis/PR24184.cpp

+// RUN: %clang_cc1 -w -analyze -analyzer-eagerly-assume -fcxx-exceptions -analyzer-checker=core -analyzer-checker=alpha.core.PointerArithm,alpha.core.CastToStruct -analyzer-max-loop 64 -verify %s
+// RUN: %clang_cc1 -w -analyze -analyzer-checker=core -analyzer-checker=cplusplus -fcxx-exceptions -analyzer-checker alpha.core.PointerArithm,alpha.core.CastToStruct -analyzer-max-loop 63 -verify %s
+
+// These tests used to hit an assertion in the bug report. Test case from http://llvm.org/PR24184.
+typedef struct {
+  int cbData;
+  unsigned pbData;
+} CRYPT_DATA_BLOB;
+
+typedef enum { DT_NONCE_FIXED } DATA_TYPE;
+int a;
+typedef int *vcreate_t(int *, DATA_TYPE, int, int);
+void fn1(unsigned, unsigned) {
+  char b = 0;
+  for (; 1; a++, &b + a * 0) // expected-warning{{Pointer arithmetic done on non-array variables means reliance on memory layout, which is dangerous}}
+    ;
+}
+
+vcreate_t fn2;
+struct A {
+  CRYPT_DATA_BLOB value;
+  int m_fn1() {
+    int c;
+    value.pbData == 0;
+    fn1(0, 0);
+  }
+};
+struct B {
+  A IkeHashAlg;
+  A IkeGType;
+  A NoncePhase1_r;
+};
+class C {
+  int m_fn2(B *);
+  void m_fn3(B *, int, int, int);
+};
+int C::m_fn2(B *p1) {
+  int *d;
+  int e = p1->IkeHashAlg.m_fn1();
+  unsigned f = p1->IkeGType.m_fn1(), h;
+  int g;
+  d = fn2(0, DT_NONCE_FIXED, (char)0, p1->NoncePhase1_r.value.cbData);
+  h = 0 | 0;
+  m_fn3(p1, 0, 0, 0);
+}
+
+// case 2:
+typedef struct {
+  int cbData;
+  unsigned char *pbData;
+} CRYPT_DATA_BLOB_1;
+typedef unsigned uint32_t;
+void fn1_1(void *p1, const void *p2) { p1 != p2; }
+
+void fn2_1(uint32_t *p1, unsigned char *p2, uint32_t p3) {
+  unsigned i = 0;
+  for (0; i < p3; i++)
+    fn1_1(p1 + i, p2 + i * 0);    // expected-warning{{Pointer arithmetic done on non-array variables means reliance on memory layout, which is dangerous}}
+}
+
+struct A_1 {
+  CRYPT_DATA_BLOB_1 value;
+  uint32_t m_fn1() {
+    uint32_t a;
+    if (value.pbData)
+      fn2_1(&a, value.pbData, value.cbData);
+    return 0;
+  }
+};
+struct {
+  A_1 HashAlgId;
+} *b;
+void fn3() {
+  uint32_t c, d;
+  d = b->HashAlgId.m_fn1();
+  d << 0 | 0 | 0;
+  c = 0;
+  0 | 1 << 0 | 0 && b;
+}
+
+// case 3:
+struct ST {
+  char c;
+};
+char *p;
+int foo1(ST);
+int foo2() {
+  ST *p1 = (ST *)(p);      // expected-warning{{Casting a non-structure type to a structure type and accessing a field can lead to memory access errors or data corruption}}
+  while (p1->c & 0x0F || p1->c & 0x07)
+    p1 = p1 + foo1(*p1);
+}
+
+int foo3(int *node) {
+  int i = foo2();
+  if (i)
+    return foo2();
+}

cfe/trunk/test/Analysis/malloc.c

 struct HasPtr {
   char *p;
 };
 
 char* reallocButNoMalloc(struct HasPtr *a, int c, int size) {
   int *s;
   char *b = realloc(a->p, size);
   char *m = realloc(a->p, size); // expected-warning {{Attempt to free released memory}}
-  return a->p;
+  // We don't expect a use-after-free for a->P here because the warning above
+  // is a sink.
+  return a->p; // no-warning
 }
 
 // We should not warn in this case since the caller will presumably free a->p in all cases.
 int reallocButNoMallocPR13674(struct HasPtr *a, int c, int size) {
   int *s;
   char *b = realloc(a->p, size);
   if (b == 0)
     return -1;