From 440c44a96ab56806e40ee260f327fbeaea47b265 Mon Sep 17 00:00:00 2001 From: Devin Coughlin Date: Wed, 16 Sep 2015 22:03:05 +0000 Subject: [PATCH] [analyzer] Add generateErrorNode() APIs to CheckerContext. 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 commit 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. This commit updates the checkers to use these APIs. These APIs (unlike addTransition() and generateSink()) do not take an explicit Pred node. This is because there are not any error nodes in the checkers that were created with an explicit different than the default (the CheckerContext's Pred node). It also changes generateSink() to require state and pred nodes (previously these were optional) to reduce confusion. Additionally, there were several cases where checkers did check whether a generated node could be null; we now explicitly check for null in these places. This commit also includes 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). Differential Revision: http://reviews.llvm.org/D12780 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@247859 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../Core/PathSensitive/CheckerContext.h | 44 ++++++++- .../Checkers/ArrayBoundChecker.cpp | 2 +- .../Checkers/ArrayBoundCheckerV2.cpp | 2 +- .../Checkers/BasicObjCFoundationChecks.cpp | 21 ++-- .../Checkers/BoolAssignmentChecker.cpp | 2 +- .../Checkers/CStringChecker.cpp | 12 +-- .../Checkers/CallAndMessageChecker.cpp | 16 +-- .../Checkers/CastSizeChecker.cpp | 2 +- .../Checkers/CastToStructChecker.cpp | 2 +- lib/StaticAnalyzer/Checkers/ChrootChecker.cpp | 2 +- .../Checkers/DereferenceChecker.cpp | 8 +- .../Checkers/DivZeroChecker.cpp | 2 +- .../Checkers/ExprInspectionChecker.cpp | 16 ++- .../Checkers/FixedAddressChecker.cpp | 2 +- .../Checkers/GenericTaintChecker.cpp | 2 +- .../Checkers/MacOSKeychainAPIChecker.cpp | 12 ++- .../Checkers/MacOSXAPIChecker.cpp | 2 +- lib/StaticAnalyzer/Checkers/MallocChecker.cpp | 24 ++--- .../Checkers/NSAutoreleasePoolChecker.cpp | 2 +- .../Checkers/NoReturnFunctionChecker.cpp | 6 +- .../Checkers/NonNullParamChecker.cpp | 4 +- .../Checkers/NullabilityChecker.cpp | 12 ++- .../Checkers/ObjCAtSyncChecker.cpp | 4 +- .../Checkers/ObjCContainersChecker.cpp | 2 +- .../Checkers/ObjCSelfInitChecker.cpp | 2 +- .../Checkers/PointerArithChecker.cpp | 2 +- .../Checkers/PointerSubChecker.cpp | 2 +- .../Checkers/PthreadLockChecker.cpp | 12 +-- .../Checkers/RetainCountChecker.cpp | 2 +- .../Checkers/ReturnPointerRangeChecker.cpp | 2 +- .../Checkers/ReturnUndefChecker.cpp | 2 +- .../Checkers/SimpleStreamChecker.cpp | 6 +- .../Checkers/StackAddrEscapeChecker.cpp | 4 +- lib/StaticAnalyzer/Checkers/StreamChecker.cpp | 8 +- .../Checkers/TaintTesterChecker.cpp | 2 +- .../Checkers/TestAfterDivZeroChecker.cpp | 2 +- .../Checkers/UndefBranchChecker.cpp | 2 +- .../Checkers/UndefCapturedBlockVarChecker.cpp | 2 +- .../Checkers/UndefResultChecker.cpp | 2 +- .../UndefinedArraySubscriptChecker.cpp | 2 +- .../Checkers/UndefinedAssignmentChecker.cpp | 2 +- .../Checkers/UnixAPIChecker.cpp | 6 +- .../Checkers/VLASizeChecker.cpp | 2 +- lib/StaticAnalyzer/Core/BugReporter.cpp | 5 + test/Analysis/PR24184.cpp | 97 +++++++++++++++++++ test/Analysis/malloc.c | 4 +- 46 files changed, 267 insertions(+), 106 deletions(-) create mode 100644 test/Analysis/PR24184.cpp diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h b/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h index a4ff133b4b..f777cd79ba 100644 --- a/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h +++ b/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h @@ -224,13 +224,39 @@ public: } /// \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 R) { Changed = true; @@ -287,6 +313,18 @@ private: 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; diff --git a/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp b/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp index 45706c2f2f..c092610afe 100644 --- a/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp @@ -62,7 +62,7 @@ void ArrayBoundChecker::checkLocation(SVal l, bool isLoad, const Stmt* LoadS, 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; diff --git a/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp b/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp index bc790d4584..f4de733bd7 100644 --- a/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp +++ b/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp @@ -182,7 +182,7 @@ void ArrayBoundCheckerV2::reportOOB(CheckerContext &checkerContext, ProgramStateRef errorState, OOB_Kind kind) const { - ExplodedNode *errorNode = checkerContext.generateSink(errorState); + ExplodedNode *errorNode = checkerContext.generateErrorNode(errorState); if (!errorNode) return; diff --git a/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp b/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp index 6adf39f375..e157478433 100644 --- a/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp +++ b/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp @@ -140,7 +140,7 @@ void NilArgChecker::warnIfNilExpr(const Expr *E, 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); } @@ -157,7 +157,7 @@ void NilArgChecker::warnIfNilArg(CheckerContext &C, 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); @@ -489,14 +489,15 @@ void CFNumberCreateChecker::checkPreStmt(const CallExpr *CE, 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); @@ -589,7 +590,7 @@ void CFRetainReleaseChecker::checkPreStmt(const CallExpr *CE, std::tie(stateTrue, stateFalse) = state->assume(ArgIsNull); if (stateTrue && !stateFalse) { - ExplodedNode *N = C.generateSink(stateTrue); + ExplodedNode *N = C.generateErrorNode(stateTrue); if (!N) return; @@ -656,7 +657,7 @@ void ClassReleaseChecker::checkPreObjCMessage(const ObjCMethodCall &msg, 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); @@ -800,7 +801,7 @@ void VariadicMethodTypeChecker::checkPreObjCMessage(const ObjCMethodCall &msg, // Generate only one error node to use for all bug reports. if (!errorNode.hasValue()) - errorNode = C.addTransition(); + errorNode = C.generateNonFatalErrorNode(); if (!errorNode.getValue()) continue; @@ -1025,7 +1026,7 @@ void ObjCLoopChecker::checkPostStmt(const ObjCForCollectionStmt *FCS, } if (!State) - C.generateSink(); + C.generateSink(C.getState(), C.getPredecessor()); else if (State != C.getState()) C.addTransition(State); } diff --git a/lib/StaticAnalyzer/Checkers/BoolAssignmentChecker.cpp b/lib/StaticAnalyzer/Checkers/BoolAssignmentChecker.cpp index b4ac316fdc..f26f73129e 100644 --- a/lib/StaticAnalyzer/Checkers/BoolAssignmentChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/BoolAssignmentChecker.cpp @@ -32,7 +32,7 @@ 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(*BT, BT->getDescription(), N)); diff --git a/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/lib/StaticAnalyzer/Checkers/CStringChecker.cpp index 477bea74d8..07c341400e 100644 --- a/lib/StaticAnalyzer/Checkers/CStringChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/CStringChecker.cpp @@ -229,7 +229,7 @@ ProgramStateRef CStringChecker::checkNonNull(CheckerContext &C, if (!Filter.CheckCStringNullArg) return nullptr; - ExplodedNode *N = C.generateSink(stateNull); + ExplodedNode *N = C.generateErrorNode(stateNull); if (!N) return nullptr; @@ -292,7 +292,7 @@ ProgramStateRef CStringChecker::CheckLocation(CheckerContext &C, 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; @@ -525,7 +525,7 @@ ProgramStateRef CStringChecker::CheckOverlap(CheckerContext &C, 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; @@ -585,7 +585,7 @@ ProgramStateRef CStringChecker::checkAdditionOverflow(CheckerContext &C, 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; @@ -706,7 +706,7 @@ SVal CStringChecker::getCStringLength(CheckerContext &C, ProgramStateRef &state, 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, @@ -766,7 +766,7 @@ SVal CStringChecker::getCStringLength(CheckerContext &C, ProgramStateRef &state, 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, diff --git a/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp b/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp index 33a24d816c..750cbda459 100644 --- a/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp @@ -97,7 +97,7 @@ private: void CallAndMessageChecker::emitBadCall(BugType *BT, CheckerContext &C, const Expr *BadE) { - ExplodedNode *N = C.generateSink(); + ExplodedNode *N = C.generateErrorNode(); if (!N) return; @@ -169,7 +169,7 @@ bool CallAndMessageChecker::uninitRefOrPointer(CheckerContext &C, 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(*BT, Message, N); R->addRange(ArgRange); @@ -200,7 +200,7 @@ bool CallAndMessageChecker::PreVisitProcessArg(CheckerContext &C, 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. @@ -265,7 +265,7 @@ bool CallAndMessageChecker::PreVisitProcessArg(CheckerContext &C, 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); @@ -338,7 +338,7 @@ void CallAndMessageChecker::checkPreStmt(const CXXDeleteExpr *DE, 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) @@ -395,7 +395,7 @@ void CallAndMessageChecker::checkPreCall(const CallEvent &Call, // the function. unsigned Params = FD->getNumParams(); if (Call.getNumArgs() < Params) { - ExplodedNode *N = C.generateSink(); + ExplodedNode *N = C.generateErrorNode(); if (!N) return; @@ -443,7 +443,7 @@ 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: @@ -559,7 +559,7 @@ void CallAndMessageChecker::HandleNilReceiver(CheckerContext &C, 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; } diff --git a/lib/StaticAnalyzer/Checkers/CastSizeChecker.cpp b/lib/StaticAnalyzer/Checkers/CastSizeChecker.cpp index 0d683f96df..a5ed64d160 100644 --- a/lib/StaticAnalyzer/Checkers/CastSizeChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/CastSizeChecker.cpp @@ -131,7 +131,7 @@ void CastSizeChecker::checkPreStmt(const CastExpr *CE,CheckerContext &C) const { 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" diff --git a/lib/StaticAnalyzer/Checkers/CastToStructChecker.cpp b/lib/StaticAnalyzer/Checkers/CastToStructChecker.cpp index ba3024d78a..fa7841356e 100644 --- a/lib/StaticAnalyzer/Checkers/CastToStructChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/CastToStructChecker.cpp @@ -56,7 +56,7 @@ void CastToStructChecker::checkPreStmt(const CastExpr *CE, // 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", diff --git a/lib/StaticAnalyzer/Checkers/ChrootChecker.cpp b/lib/StaticAnalyzer/Checkers/ChrootChecker.cpp index cefdb06453..3ad1996db8 100644 --- a/lib/StaticAnalyzer/Checkers/ChrootChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/ChrootChecker.cpp @@ -140,7 +140,7 @@ void ChrootChecker::checkPreStmt(const CallExpr *CE, CheckerContext &C) const { 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 " diff --git a/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp b/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp index 0df3186090..8d41a25c28 100644 --- a/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp @@ -91,7 +91,7 @@ DereferenceChecker::AddDerefSource(raw_ostream &os, 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; @@ -184,7 +184,7 @@ 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")); @@ -219,7 +219,7 @@ void DereferenceChecker::checkLocation(SVal l, bool isLoad, 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.generateSink(nullState)) { + if (ExplodedNode *N = C.generateSink(nullState, C.getPredecessor())) { ImplicitNullDerefEvent event = {l, isLoad, N, &C.getBugReporter(), /*IsDirectDereference=*/false}; dispatchEvent(event); @@ -257,7 +257,7 @@ void DereferenceChecker::checkBind(SVal L, SVal V, const Stmt *S, // 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}; diff --git a/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp b/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp index 211a1c1d0c..5985023056 100644 --- a/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp @@ -35,7 +35,7 @@ public: 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")); diff --git a/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp b/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp index 7dc0a87459..60c6aa534d 100644 --- a/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp @@ -86,8 +86,7 @@ static const char *getArgumentValueString(const CallExpr *CE, 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. @@ -97,24 +96,28 @@ void ExprInspectionChecker::analyzerEval(const CallExpr *CE, if (!BT) BT.reset(new BugType(this, "Checking analyzer assumptions", "debug")); + ExplodedNode *N = C.generateNonFatalErrorNode(); + if (!N) + return; C.emitReport( llvm::make_unique(*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(*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) @@ -127,6 +130,9 @@ void ExprInspectionChecker::analyzerCheckInlined(const CallExpr *CE, if (!BT) BT.reset(new BugType(this, "Checking analyzer assumptions", "debug")); + ExplodedNode *N = C.generateNonFatalErrorNode(); + if (!N) + return; C.emitReport( llvm::make_unique(*BT, getArgumentValueString(CE, C), N)); } diff --git a/lib/StaticAnalyzer/Checkers/FixedAddressChecker.cpp b/lib/StaticAnalyzer/Checkers/FixedAddressChecker.cpp index ae226f727c..3fe89f96a4 100644 --- a/lib/StaticAnalyzer/Checkers/FixedAddressChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/FixedAddressChecker.cpp @@ -50,7 +50,7 @@ void FixedAddressChecker::checkPreStmt(const BinaryOperator *B, 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", diff --git a/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp b/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp index cb2bf108ef..1d8085108c 100644 --- a/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp @@ -640,7 +640,7 @@ bool GenericTaintChecker::generateReportIfTainted(const Expr *E, return false; // Generate diagnostic. - if (ExplodedNode *N = C.addTransition()) { + if (ExplodedNode *N = C.generateNonFatalErrorNode()) { initBugType(); auto report = llvm::make_unique(*BT, Msg, N); report->addRange(E->getSourceRange()); diff --git a/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp b/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp index 932d3d6fff..547aa7540d 100644 --- a/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp @@ -255,7 +255,7 @@ void MacOSKeychainAPIChecker:: CheckerContext &C) const { ProgramStateRef State = C.getState(); State = State->remove(AP.first); - ExplodedNode *N = C.addTransition(State); + ExplodedNode *N = C.generateNonFatalErrorNode(State); if (!N) return; @@ -301,7 +301,7 @@ void MacOSKeychainAPIChecker::checkPreStmt(const CallExpr *CE, // 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(V); - ExplodedNode *N = C.addTransition(State); + ExplodedNode *N = C.generateNonFatalErrorNode(State); if (!N) return; initBugType(); @@ -364,7 +364,7 @@ void MacOSKeychainAPIChecker::checkPreStmt(const CallExpr *CE, if (isEnclosingFunctionParam(ArgExpr)) return; - ExplodedNode *N = C.addTransition(State); + ExplodedNode *N = C.generateNonFatalErrorNode(State); if (!N) return; initBugType(); @@ -430,7 +430,7 @@ void MacOSKeychainAPIChecker::checkPreStmt(const CallExpr *CE, // report a bad call to free. if (State->assume(ArgSVal.castAs(), false) && !definitelyDidnotReturnError(AS->Region, State, C.getSValBuilder())) { - ExplodedNode *N = C.addTransition(State); + ExplodedNode *N = C.generateNonFatalErrorNode(State); if (!N) return; initBugType(); @@ -584,7 +584,9 @@ void MacOSKeychainAPIChecker::checkDeadSymbols(SymbolReaper &SR, } 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) diff --git a/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp b/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp index 4ce936aa0f..4cbe97b260 100644 --- a/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp @@ -62,7 +62,7 @@ void MacOSXAPIChecker::CheckDispatchOnce(CheckerContext &C, const CallExpr *CE, if (!R || !isa(R->getMemorySpace())) return; - ExplodedNode *N = C.generateSink(state); + ExplodedNode *N = C.generateErrorNode(state); if (!N) return; diff --git a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp index c95b2fd7f4..d5c5cc1dba 100644 --- a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -1592,7 +1592,7 @@ void MallocChecker::ReportBadFree(CheckerContext &C, SVal ArgVal, 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")); @@ -1637,7 +1637,7 @@ void MallocChecker::ReportFreeAlloca(CheckerContext &C, SVal ArgVal, 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")); @@ -1661,7 +1661,7 @@ void MallocChecker::ReportMismatchedDealloc(CheckerContext &C, 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], @@ -1720,7 +1720,7 @@ void MallocChecker::ReportOffsetFree(CheckerContext &C, SVal ArgVal, if (!CheckKind.hasValue()) return; - ExplodedNode *N = C.generateSink(); + ExplodedNode *N = C.generateErrorNode(); if (!N) return; @@ -1774,7 +1774,7 @@ void MallocChecker::ReportUseAfterFree(CheckerContext &C, SourceRange Range, 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")); @@ -1801,7 +1801,7 @@ void MallocChecker::ReportDoubleFree(CheckerContext &C, SourceRange Range, 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")); @@ -1829,7 +1829,7 @@ void MallocChecker::ReportDoubleDelete(CheckerContext &C, SymbolRef Sym) const { 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")); @@ -1856,7 +1856,7 @@ void MallocChecker::ReportUseZeroAllocated(CheckerContext &C, 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")); @@ -2150,10 +2150,12 @@ void MallocChecker::checkDeadSymbols(SymbolReaper &SymReaper, ExplodedNode *N = C.getPredecessor(); if (!Errors.empty()) { static CheckerProgramPointTag Tag("MallocChecker", "DeadSymbolsLeak"); - N = C.addTransition(C.getState(), C.getPredecessor(), &Tag); - for (SmallVectorImpl::iterator + N = C.generateNonFatalErrorNode(C.getState(), &Tag); + if (N) { + for (SmallVectorImpl::iterator I = Errors.begin(), E = Errors.end(); I != E; ++I) { - reportLeak(*I, N, C); + reportLeak(*I, N, C); + } } } diff --git a/lib/StaticAnalyzer/Checkers/NSAutoreleasePoolChecker.cpp b/lib/StaticAnalyzer/Checkers/NSAutoreleasePoolChecker.cpp index 5d9b32fa91..0e7894788c 100644 --- a/lib/StaticAnalyzer/Checkers/NSAutoreleasePoolChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/NSAutoreleasePoolChecker.cpp @@ -62,7 +62,7 @@ void NSAutoreleasePoolChecker::checkPreObjCMessage(const ObjCMethodCall &msg, 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; diff --git a/lib/StaticAnalyzer/Checkers/NoReturnFunctionChecker.cpp b/lib/StaticAnalyzer/Checkers/NoReturnFunctionChecker.cpp index ba82d1d1d4..8d0a060fc4 100644 --- a/lib/StaticAnalyzer/Checkers/NoReturnFunctionChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/NoReturnFunctionChecker.cpp @@ -81,7 +81,7 @@ void NoReturnFunctionChecker::checkPostCall(const CallEvent &CE, } if (BuildSinks) - C.generateSink(); + C.generateSink(C.getState(), C.getPredecessor()); } void NoReturnFunctionChecker::checkPostObjCMessage(const ObjCMethodCall &Msg, @@ -90,7 +90,7 @@ void NoReturnFunctionChecker::checkPostObjCMessage(const ObjCMethodCall &Msg, if (const ObjCMethodDecl *MD = Msg.getDecl()) { MD = MD->getCanonicalDecl(); if (MD->hasAttr()) { - C.generateSink(); + C.generateSink(C.getState(), C.getPredecessor()); return; } } @@ -136,7 +136,7 @@ void NoReturnFunctionChecker::checkPostObjCMessage(const ObjCMethodCall &Msg, } // 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) { diff --git a/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp b/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp index a7586c412a..1f82ab94af 100644 --- a/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp @@ -143,7 +143,7 @@ void NonNullParamChecker::checkPreCall(const CallEvent &Call, 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 R; if (haveAttrNonNull) @@ -161,7 +161,7 @@ void NonNullParamChecker::checkPreCall(const CallEvent &Call, // 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}; diff --git a/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp b/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp index 74a7e38e94..137dedbf4c 100644 --- a/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp @@ -499,7 +499,9 @@ void NullabilityChecker::checkPreStmt(const ReturnStmt *S, 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; @@ -569,7 +571,9 @@ void NullabilityChecker::checkPreCall(const CallEvent &Call, 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; @@ -891,7 +895,9 @@ void NullabilityChecker::checkBind(SVal L, SVal V, const Stmt *S, 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; diff --git a/lib/StaticAnalyzer/Checkers/ObjCAtSyncChecker.cpp b/lib/StaticAnalyzer/Checkers/ObjCAtSyncChecker.cpp index a7b92b4c67..cbaa5c2359 100644 --- a/lib/StaticAnalyzer/Checkers/ObjCAtSyncChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/ObjCAtSyncChecker.cpp @@ -43,7 +43,7 @@ void ObjCAtSyncChecker::checkPreStmt(const ObjCAtSynchronizedStmt *S, // Uninitialized value used for the mutex? if (V.getAs()) { - 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")); @@ -66,7 +66,7 @@ void ObjCAtSyncChecker::checkPreStmt(const ObjCAtSynchronizedStmt *S, 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() " diff --git a/lib/StaticAnalyzer/Checkers/ObjCContainersChecker.cpp b/lib/StaticAnalyzer/Checkers/ObjCContainersChecker.cpp index 14f4969c44..0203d79cd0 100644 --- a/lib/StaticAnalyzer/Checkers/ObjCContainersChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/ObjCContainersChecker.cpp @@ -139,7 +139,7 @@ void ObjCContainersChecker::checkPreStmt(const CallExpr *CE, 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(); diff --git a/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp b/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp index 193f515d1c..f344dd09c7 100644 --- a/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp @@ -153,7 +153,7 @@ void ObjCSelfInitChecker::checkForInvalidSelf(const Expr *E, CheckerContext &C, return; // Generate an error node. - ExplodedNode *N = C.generateSink(); + ExplodedNode *N = C.generateErrorNode(); if (!N) return; diff --git a/lib/StaticAnalyzer/Checkers/PointerArithChecker.cpp b/lib/StaticAnalyzer/Checkers/PointerArithChecker.cpp index 313fe7c43e..e3369677af 100644 --- a/lib/StaticAnalyzer/Checkers/PointerArithChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/PointerArithChecker.cpp @@ -51,7 +51,7 @@ void PointerArithChecker::checkPreStmt(const BinaryOperator *B, if (isa(LR) || isa(LR) || isa(LR)) { - if (ExplodedNode *N = C.addTransition()) { + if (ExplodedNode *N = C.generateNonFatalErrorNode()) { if (!BT) BT.reset( new BuiltinBug(this, "Dangerous pointer arithmetic", diff --git a/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp b/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp index a3b02d9974..2d33ebc261 100644 --- a/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp @@ -60,7 +60,7 @@ void PointerSubChecker::checkPreStmt(const BinaryOperator *B, if (isa(BaseLR) || isa(BaseRR)) return; - if (ExplodedNode *N = C.addTransition()) { + if (ExplodedNode *N = C.generateNonFatalErrorNode()) { if (!BT) BT.reset( new BuiltinBug(this, "Pointer subtraction", diff --git a/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp b/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp index 4b8c0b7a98..426c97dbd4 100644 --- a/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp @@ -142,7 +142,7 @@ void PthreadLockChecker::AcquireLock(CheckerContext &C, const CallExpr *CE, 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( @@ -204,7 +204,7 @@ void PthreadLockChecker::ReleaseLock(CheckerContext &C, const CallExpr *CE, 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( @@ -227,7 +227,7 @@ void PthreadLockChecker::ReleaseLock(CheckerContext &C, const CallExpr *CE, 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( @@ -272,7 +272,7 @@ void PthreadLockChecker::DestroyLock(CheckerContext &C, const CallExpr *CE, 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(*BT_destroylock, Message, N); @@ -307,7 +307,7 @@ void PthreadLockChecker::InitLock(CheckerContext &C, const CallExpr *CE, 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(*BT_initlock, Message, N); @@ -320,7 +320,7 @@ void PthreadLockChecker::reportUseDestroyedBug(CheckerContext &C, 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( diff --git a/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp b/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp index 490a242191..0610beb7bf 100644 --- a/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp @@ -3306,7 +3306,7 @@ void RetainCountChecker::processNonLeakError(ProgramStateRef St, if (RV->getIvarAccessHistory() != RefVal::IvarAccessHistory::None) return; - ExplodedNode *N = C.generateSink(St); + ExplodedNode *N = C.generateErrorNode(St); if (!N) return; diff --git a/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp b/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp index d94299c050..19fa0fb193 100644 --- a/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp @@ -62,7 +62,7 @@ void ReturnPointerRangeChecker::checkPreStmt(const ReturnStmt *RS, 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; diff --git a/lib/StaticAnalyzer/Checkers/ReturnUndefChecker.cpp b/lib/StaticAnalyzer/Checkers/ReturnUndefChecker.cpp index 2668ac1e1e..c5e826a84b 100644 --- a/lib/StaticAnalyzer/Checkers/ReturnUndefChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/ReturnUndefChecker.cpp @@ -80,7 +80,7 @@ void ReturnUndefChecker::checkPreStmt(const ReturnStmt *RS, 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; diff --git a/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp b/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp index e1b2eaf46d..7026a2ec16 100644 --- a/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp @@ -200,7 +200,9 @@ void SimpleStreamChecker::checkDeadSymbols(SymbolReaper &SymReaper, State = State->remove(Sym); } - ExplodedNode *N = C.addTransition(State); + ExplodedNode *N = C.generateNonFatalErrorNode(State); + if (!N) + return; reportLeaks(LeakedStreams, C, N); } @@ -208,7 +210,7 @@ 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; diff --git a/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp b/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp index f57cd70fd4..2eefb93e23 100644 --- a/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp @@ -94,7 +94,7 @@ SourceRange StackAddrEscapeChecker::genName(raw_ostream &os, const MemRegion *R, void StackAddrEscapeChecker::EmitStackError(CheckerContext &C, const MemRegion *R, const Expr *RetE) const { - ExplodedNode *N = C.generateSink(); + ExplodedNode *N = C.generateErrorNode(); if (!N) return; @@ -211,7 +211,7 @@ void StackAddrEscapeChecker::checkEndFunction(CheckerContext &Ctx) const { return; // Generate an error node. - ExplodedNode *N = Ctx.addTransition(state); + ExplodedNode *N = Ctx.generateNonFatalErrorNode(state); if (!N) return; diff --git a/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/lib/StaticAnalyzer/Checkers/StreamChecker.cpp index f9dfd38e1f..82b01fe814 100644 --- a/lib/StaticAnalyzer/Checkers/StreamChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/StreamChecker.cpp @@ -271,7 +271,7 @@ void StreamChecker::Fseek(CheckerContext &C, const CallExpr *CE) const { 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", @@ -349,7 +349,7 @@ ProgramStateRef StreamChecker::CheckNullStream(SVal SV, ProgramStateRef state, 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.")); @@ -378,7 +378,7 @@ ProgramStateRef StreamChecker::CheckDoubleClose(const CallExpr *CE, // 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( @@ -406,7 +406,7 @@ void StreamChecker::checkDeadSymbols(SymbolReaper &SymReaper, continue; if (SS->isOpened()) { - ExplodedNode *N = C.generateSink(); + ExplodedNode *N = C.generateErrorNode(); if (N) { if (!BT_ResourceLeak) BT_ResourceLeak.reset(new BuiltinBug( diff --git a/lib/StaticAnalyzer/Checkers/TaintTesterChecker.cpp b/lib/StaticAnalyzer/Checkers/TaintTesterChecker.cpp index 6e2477579f..2e0529015c 100644 --- a/lib/StaticAnalyzer/Checkers/TaintTesterChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/TaintTesterChecker.cpp @@ -48,7 +48,7 @@ void TaintTesterChecker::checkPostStmt(const Expr *E, return; if (State->isTainted(E, C.getLocationContext())) { - if (ExplodedNode *N = C.addTransition()) { + if (ExplodedNode *N = C.generateNonFatalErrorNode()) { initBugType(); auto report = llvm::make_unique(*BT, "tainted",N); report->addRange(E->getSourceRange()); diff --git a/lib/StaticAnalyzer/Checkers/TestAfterDivZeroChecker.cpp b/lib/StaticAnalyzer/Checkers/TestAfterDivZeroChecker.cpp index 638701da8a..b794d2f86b 100644 --- a/lib/StaticAnalyzer/Checkers/TestAfterDivZeroChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/TestAfterDivZeroChecker.cpp @@ -167,7 +167,7 @@ bool TestAfterDivZeroChecker::hasDivZeroMap(SVal Var, } 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")); diff --git a/lib/StaticAnalyzer/Checkers/UndefBranchChecker.cpp b/lib/StaticAnalyzer/Checkers/UndefBranchChecker.cpp index 268e957ea5..ed17610e41 100644 --- a/lib/StaticAnalyzer/Checkers/UndefBranchChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/UndefBranchChecker.cpp @@ -62,7 +62,7 @@ void UndefBranchChecker::checkBranchCondition(const Stmt *Condition, 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( diff --git a/lib/StaticAnalyzer/Checkers/UndefCapturedBlockVarChecker.cpp b/lib/StaticAnalyzer/Checkers/UndefCapturedBlockVarChecker.cpp index 0f426b5d54..17fe8610da 100644 --- a/lib/StaticAnalyzer/Checkers/UndefCapturedBlockVarChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/UndefCapturedBlockVarChecker.cpp @@ -74,7 +74,7 @@ UndefCapturedBlockVarChecker::checkPostStmt(const BlockExpr *BE, // Get the VarRegion associated with VD in the local stack frame. if (Optional V = state->getSVal(I.getOriginalRegion()).getAs()) { - if (ExplodedNode *N = C.generateSink()) { + if (ExplodedNode *N = C.generateErrorNode()) { if (!BT) BT.reset( new BuiltinBug(this, "uninitialized variable captured by block")); diff --git a/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp b/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp index 8ae8694ca7..38d2aa6d8f 100644 --- a/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp @@ -50,7 +50,7 @@ void UndefResultChecker::checkPostStmt(const BinaryOperator *B, return; // Generate an error node. - ExplodedNode *N = C.generateSink(); + ExplodedNode *N = C.generateErrorNode(); if (!N) return; diff --git a/lib/StaticAnalyzer/Checkers/UndefinedArraySubscriptChecker.cpp b/lib/StaticAnalyzer/Checkers/UndefinedArraySubscriptChecker.cpp index c80732f274..fe07eafd28 100644 --- a/lib/StaticAnalyzer/Checkers/UndefinedArraySubscriptChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/UndefinedArraySubscriptChecker.cpp @@ -46,7 +46,7 @@ UndefinedArraySubscriptChecker::checkPreStmt(const ArraySubscriptExpr *A, if (Ctor->isDefaulted()) return; - ExplodedNode *N = C.generateSink(); + ExplodedNode *N = C.generateErrorNode(); if (!N) return; if (!BT) diff --git a/lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp b/lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp index 81c96c4860..7a31efc8ce 100644 --- a/lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp @@ -46,7 +46,7 @@ void UndefinedAssignmentChecker::checkBind(SVal location, SVal val, if (C.getCalleeName(EnclosingFunctionDecl) == "swap") return; - ExplodedNode *N = C.generateSink(); + ExplodedNode *N = C.generateErrorNode(); if (!N) return; diff --git a/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp b/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp index cf580ad465..4b78c20583 100644 --- a/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp @@ -77,7 +77,7 @@ 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; @@ -182,7 +182,7 @@ void UnixAPIChecker::CheckPthreadOnce(CheckerContext &C, if (!R || !isa(R->getMemorySpace())) return; - ExplodedNode *N = C.generateSink(state); + ExplodedNode *N = C.generateErrorNode(state); if (!N) return; @@ -231,7 +231,7 @@ 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; diff --git a/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp b/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp index 8cd3aac747..e3b2ed2223 100644 --- a/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp @@ -46,7 +46,7 @@ void VLASizeChecker::reportBug(VLASize_Kind Kind, ProgramStateRef State, CheckerContext &C) const { // Generate an error node. - ExplodedNode *N = C.generateSink(State); + ExplodedNode *N = C.generateErrorNode(State); if (!N) return; diff --git a/lib/StaticAnalyzer/Core/BugReporter.cpp b/lib/StaticAnalyzer/Core/BugReporter.cpp index a3abb1887e..f70a7d04ae 100644 --- a/lib/StaticAnalyzer/Core/BugReporter.cpp +++ b/lib/StaticAnalyzer/Core/BugReporter.cpp @@ -3224,6 +3224,11 @@ void BugReporter::Register(BugType *BT) { void BugReporter::emitReport(std::unique_ptr 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 diff --git a/test/Analysis/PR24184.cpp b/test/Analysis/PR24184.cpp new file mode 100644 index 0000000000..db0df6f9c3 --- /dev/null +++ b/test/Analysis/PR24184.cpp @@ -0,0 +1,97 @@ +// 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(); +} diff --git a/test/Analysis/malloc.c b/test/Analysis/malloc.c index 662df4c28b..3a7247c473 100644 --- a/test/Analysis/malloc.c +++ b/test/Analysis/malloc.c @@ -1386,7 +1386,9 @@ 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. -- 2.40.0