diff options
Diffstat (limited to 'lib/StaticAnalyzer/Checkers')
81 files changed, 2088 insertions, 1296 deletions
diff --git a/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp b/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp index 58017acb4a246..8d4793e0802ff 100644 --- a/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp @@ -75,7 +75,8 @@ void ArrayBoundChecker::checkLocation(SVal l, bool isLoad, const Stmt* LoadS, // reference is outside the range. // Generate a report for this bug. - auto report = llvm::make_unique<BugReport>(*BT, BT->getDescription(), N); + auto report = + std::make_unique<PathSensitiveBugReport>(*BT, BT->getDescription(), N); report->addRange(LoadS->getSourceRange()); C.emitReport(std::move(report)); diff --git a/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp b/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp index 3bf8a1836b19b..8f3bf138cae4a 100644 --- a/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp +++ b/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp @@ -208,7 +208,7 @@ void ArrayBoundCheckerV2::checkLocation(SVal location, bool isLoad, SVal ByteOffset = rawOffset.getByteOffset(); if (isTainted(state, ByteOffset)) { reportOOB(checkerContext, state_exceedsUpperBound, OOB_Tainted, - llvm::make_unique<TaintBugVisitor>(ByteOffset)); + std::make_unique<TaintBugVisitor>(ByteOffset)); return; } } else if (state_exceedsUpperBound) { @@ -256,7 +256,7 @@ void ArrayBoundCheckerV2::reportOOB( break; } - auto BR = llvm::make_unique<BugReport>(*BT, os.str(), errorNode); + auto BR = std::make_unique<PathSensitiveBugReport>(*BT, os.str(), errorNode); BR->addVisitor(std::move(Visitor)); checkerContext.emitReport(std::move(BR)); } diff --git a/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp b/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp index e3fb4c3eb523f..325952fe4ed4d 100644 --- a/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp +++ b/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp @@ -211,7 +211,7 @@ void NilArgChecker::generateBugReport(ExplodedNode *N, if (!BT) BT.reset(new APIMisuse(this, "nil argument")); - auto R = llvm::make_unique<BugReport>(*BT, Msg, N); + auto R = std::make_unique<PathSensitiveBugReport>(*BT, Msg, N); R->addRange(Range); bugreporter::trackExpressionValue(N, E, *R); C.emitReport(std::move(R)); @@ -520,7 +520,7 @@ void CFNumberChecker::checkPreStmt(const CallExpr *CE, if (!BT) BT.reset(new APIMisuse(this, "Bad use of CFNumber APIs")); - auto report = llvm::make_unique<BugReport>(*BT, os.str(), N); + auto report = std::make_unique<PathSensitiveBugReport>(*BT, os.str(), N); report->addRange(CE->getArg(2)->getSourceRange()); C.emitReport(std::move(report)); } @@ -575,7 +575,7 @@ void CFRetainReleaseChecker::checkPreCall(const CallEvent &Call, OS << "Null pointer argument in call to " << cast<FunctionDecl>(Call.getDecl())->getName(); - auto report = llvm::make_unique<BugReport>(BT, OS.str(), N); + auto report = std::make_unique<PathSensitiveBugReport>(BT, OS.str(), N); report->addRange(Call.getArgSourceRange(0)); bugreporter::trackExpressionValue(N, Call.getArgExpr(0), *report); C.emitReport(std::move(report)); @@ -635,7 +635,7 @@ void ClassReleaseChecker::checkPreObjCMessage(const ObjCMethodCall &msg, "of class '" << Class->getName() << "' and not the class directly"; - auto report = llvm::make_unique<BugReport>(*BT, os.str(), N); + auto report = std::make_unique<PathSensitiveBugReport>(*BT, os.str(), N); report->addRange(msg.getSourceRange()); C.emitReport(std::move(report)); } @@ -788,7 +788,8 @@ void VariadicMethodTypeChecker::checkPreObjCMessage(const ObjCMethodCall &msg, ArgTy.print(os, C.getLangOpts()); os << "'"; - auto R = llvm::make_unique<BugReport>(*BT, os.str(), errorNode.getValue()); + auto R = std::make_unique<PathSensitiveBugReport>(*BT, os.str(), + errorNode.getValue()); R->addRange(msg.getArgSourceRange(I)); C.emitReport(std::move(R)); } diff --git a/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp b/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp index 009160fc98157..0eb3c3d1d0e6d 100644 --- a/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp @@ -126,7 +126,7 @@ bool BlockInCriticalSectionChecker::isLockFunction(const CallEvent &Call) const bool BlockInCriticalSectionChecker::isUnlockFunction(const CallEvent &Call) const { if (const auto *Dtor = dyn_cast<CXXDestructorCall>(&Call)) { - const auto *DRecordDecl = dyn_cast<CXXRecordDecl>(Dtor->getDecl()->getParent()); + const auto *DRecordDecl = cast<CXXRecordDecl>(Dtor->getDecl()->getParent()); auto IdentifierInfo = DRecordDecl->getIdentifier(); if (IdentifierInfo == IILockGuard || IdentifierInfo == IIUniqueLock) return true; @@ -173,7 +173,8 @@ void BlockInCriticalSectionChecker::reportBlockInCritSection( llvm::raw_string_ostream os(msg); os << "Call to blocking function '" << Call.getCalleeIdentifier()->getName() << "' inside of critical section"; - auto R = llvm::make_unique<BugReport>(*BlockInCritSectionBugType, os.str(), ErrNode); + auto R = std::make_unique<PathSensitiveBugReport>(*BlockInCritSectionBugType, + os.str(), ErrNode); R->addRange(Call.getSourceRange()); R->markInteresting(BlockDescSym); C.emitReport(std::move(R)); diff --git a/lib/StaticAnalyzer/Checkers/BoolAssignmentChecker.cpp b/lib/StaticAnalyzer/Checkers/BoolAssignmentChecker.cpp index de8763c1b7b57..1423b9c39b266 100644 --- a/lib/StaticAnalyzer/Checkers/BoolAssignmentChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/BoolAssignmentChecker.cpp @@ -34,7 +34,9 @@ void BoolAssignmentChecker::emitReport(ProgramStateRef 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)); + + C.emitReport( + std::make_unique<PathSensitiveBugReport>(*BT, BT->getDescription(), N)); } } diff --git a/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/lib/StaticAnalyzer/Checkers/CStringChecker.cpp index 44f4530781a8e..503c451670b8d 100644 --- a/lib/StaticAnalyzer/Checkers/CStringChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/CStringChecker.cpp @@ -48,10 +48,10 @@ public: DefaultBool CheckCStringBufferOverlap; DefaultBool CheckCStringNotNullTerm; - CheckName CheckNameCStringNullArg; - CheckName CheckNameCStringOutOfBounds; - CheckName CheckNameCStringBufferOverlap; - CheckName CheckNameCStringNotNullTerm; + CheckerNameRef CheckNameCStringNullArg; + CheckerNameRef CheckNameCStringOutOfBounds; + CheckerNameRef CheckNameCStringBufferOverlap; + CheckerNameRef CheckNameCStringNotNullTerm; }; CStringChecksFilter Filter; @@ -198,7 +198,8 @@ public: ProgramStateRef checkNonNull(CheckerContext &C, ProgramStateRef state, const Expr *S, - SVal l) const; + SVal l, + unsigned IdxOfArg) const; ProgramStateRef CheckLocation(CheckerContext &C, ProgramStateRef state, const Expr *S, @@ -277,7 +278,8 @@ CStringChecker::assumeZero(CheckerContext &C, ProgramStateRef state, SVal V, ProgramStateRef CStringChecker::checkNonNull(CheckerContext &C, ProgramStateRef state, - const Expr *S, SVal l) const { + const Expr *S, SVal l, + unsigned IdxOfArg) const { // If a previous check has failed, propagate the failure. if (!state) return nullptr; @@ -288,11 +290,13 @@ ProgramStateRef CStringChecker::checkNonNull(CheckerContext &C, if (stateNull && !stateNonNull) { if (Filter.CheckCStringNullArg) { SmallString<80> buf; - llvm::raw_svector_ostream os(buf); + llvm::raw_svector_ostream OS(buf); assert(CurrentFunctionDescription); - os << "Null pointer argument in call to " << CurrentFunctionDescription; + OS << "Null pointer argument in call to " << CurrentFunctionDescription + << ' ' << IdxOfArg << llvm::getOrdinalSuffix(IdxOfArg) + << " parameter"; - emitNullArgBug(C, stateNull, S, os.str()); + emitNullArgBug(C, stateNull, S, OS.str()); } return nullptr; } @@ -384,7 +388,7 @@ ProgramStateRef CStringChecker::CheckBufferAccess(CheckerContext &C, // Check that the first buffer is non-null. SVal BufVal = C.getSVal(FirstBuf); - state = checkNonNull(C, state, FirstBuf, BufVal); + state = checkNonNull(C, state, FirstBuf, BufVal, 1); if (!state) return nullptr; @@ -424,7 +428,7 @@ ProgramStateRef CStringChecker::CheckBufferAccess(CheckerContext &C, // If there's a second buffer, check it as well. if (SecondBuf) { BufVal = state->getSVal(SecondBuf, LCtx); - state = checkNonNull(C, state, SecondBuf, BufVal); + state = checkNonNull(C, state, SecondBuf, BufVal, 2); if (!state) return nullptr; @@ -566,7 +570,7 @@ void CStringChecker::emitOverlapBug(CheckerContext &C, ProgramStateRef state, categories::UnixAPI, "Improper arguments")); // Generate a report for this bug. - auto report = llvm::make_unique<BugReport>( + auto report = std::make_unique<PathSensitiveBugReport>( *BT_Overlap, "Arguments must not be overlapping buffers", N); report->addRange(First->getSourceRange()); report->addRange(Second->getSourceRange()); @@ -583,7 +587,7 @@ void CStringChecker::emitNullArgBug(CheckerContext &C, ProgramStateRef State, "Null pointer argument in call to byte string function")); BuiltinBug *BT = static_cast<BuiltinBug *>(BT_Null.get()); - auto Report = llvm::make_unique<BugReport>(*BT, WarningMsg, N); + auto Report = std::make_unique<PathSensitiveBugReport>(*BT, WarningMsg, N); Report->addRange(S->getSourceRange()); if (const auto *Ex = dyn_cast<Expr>(S)) bugreporter::trackExpressionValue(N, Ex, *Report); @@ -607,7 +611,7 @@ void CStringChecker::emitOutOfBoundsBug(CheckerContext &C, // FIXME: It would be nice to eventually make this diagnostic more clear, // e.g., by referencing the original declaration or by saying *why* this // reference is outside the range. - auto Report = llvm::make_unique<BugReport>(*BT, WarningMsg, N); + auto Report = std::make_unique<PathSensitiveBugReport>(*BT, WarningMsg, N); Report->addRange(S->getSourceRange()); C.emitReport(std::move(Report)); } @@ -622,7 +626,8 @@ void CStringChecker::emitNotCStringBug(CheckerContext &C, ProgramStateRef State, Filter.CheckNameCStringNotNullTerm, categories::UnixAPI, "Argument is not a null-terminated string.")); - auto Report = llvm::make_unique<BugReport>(*BT_NotCString, WarningMsg, N); + auto Report = + std::make_unique<PathSensitiveBugReport>(*BT_NotCString, WarningMsg, N); Report->addRange(S->getSourceRange()); C.emitReport(std::move(Report)); @@ -644,7 +649,8 @@ void CStringChecker::emitAdditionOverflowBug(CheckerContext &C, "This expression will create a string whose length is too big to " "be represented as a size_t"; - auto Report = llvm::make_unique<BugReport>(*BT_NotCString, WarningMsg, N); + auto Report = + std::make_unique<PathSensitiveBugReport>(*BT_NotCString, WarningMsg, N); C.emitReport(std::move(Report)); } } @@ -1163,7 +1169,7 @@ void CStringChecker::evalCopyCommon(CheckerContext &C, // Ensure the destination is not null. If it is NULL there will be a // NULL pointer dereference. - state = checkNonNull(C, state, Dest, destVal); + state = checkNonNull(C, state, Dest, destVal, 1); if (!state) return; @@ -1172,7 +1178,7 @@ void CStringChecker::evalCopyCommon(CheckerContext &C, // Ensure the source is not null. If it is NULL there will be a // NULL pointer dereference. - state = checkNonNull(C, state, Source, srcVal); + state = checkNonNull(C, state, Source, srcVal, 2); if (!state) return; @@ -1383,7 +1389,7 @@ void CStringChecker::evalstrLengthCommon(CheckerContext &C, const CallExpr *CE, const Expr *Arg = CE->getArg(0); SVal ArgVal = state->getSVal(Arg, LCtx); - state = checkNonNull(C, state, Arg, ArgVal); + state = checkNonNull(C, state, Arg, ArgVal, 1); if (!state) return; @@ -1541,14 +1547,14 @@ void CStringChecker::evalStrcpyCommon(CheckerContext &C, const CallExpr *CE, const Expr *Dst = CE->getArg(0); SVal DstVal = state->getSVal(Dst, LCtx); - state = checkNonNull(C, state, Dst, DstVal); + state = checkNonNull(C, state, Dst, DstVal, 1); if (!state) return; // Check that the source is non-null. const Expr *srcExpr = CE->getArg(1); SVal srcVal = state->getSVal(srcExpr, LCtx); - state = checkNonNull(C, state, srcExpr, srcVal); + state = checkNonNull(C, state, srcExpr, srcVal, 2); if (!state) return; @@ -1904,14 +1910,14 @@ void CStringChecker::evalStrcmpCommon(CheckerContext &C, const CallExpr *CE, // Check that the first string is non-null const Expr *s1 = CE->getArg(0); SVal s1Val = state->getSVal(s1, LCtx); - state = checkNonNull(C, state, s1, s1Val); + state = checkNonNull(C, state, s1, s1Val, 1); if (!state) return; // Check that the second string is non-null. const Expr *s2 = CE->getArg(1); SVal s2Val = state->getSVal(s2, LCtx); - state = checkNonNull(C, state, s2, s2Val); + state = checkNonNull(C, state, s2, s2Val, 2); if (!state) return; @@ -2038,14 +2044,14 @@ void CStringChecker::evalStrsep(CheckerContext &C, const CallExpr *CE) const { // Check that the search string pointer is non-null (though it may point to // a null string). SVal SearchStrVal = State->getSVal(SearchStrPtr, LCtx); - State = checkNonNull(C, State, SearchStrPtr, SearchStrVal); + State = checkNonNull(C, State, SearchStrPtr, SearchStrVal, 1); if (!State) return; // Check that the delimiter string is non-null. const Expr *DelimStr = CE->getArg(1); SVal DelimStrVal = State->getSVal(DelimStr, LCtx); - State = checkNonNull(C, State, DelimStr, DelimStrVal); + State = checkNonNull(C, State, DelimStr, DelimStrVal, 2); if (!State) return; @@ -2148,7 +2154,7 @@ void CStringChecker::evalMemset(CheckerContext &C, const CallExpr *CE) const { // Ensure the memory area is not null. // If it is NULL there will be a NULL pointer dereference. - State = checkNonNull(C, StateNonZeroSize, Mem, MemVal); + State = checkNonNull(C, StateNonZeroSize, Mem, MemVal, 1); if (!State) return; @@ -2195,7 +2201,7 @@ void CStringChecker::evalBzero(CheckerContext &C, const CallExpr *CE) const { // Ensure the memory area is not null. // If it is NULL there will be a NULL pointer dereference. - State = checkNonNull(C, StateNonZeroSize, Mem, MemVal); + State = checkNonNull(C, StateNonZeroSize, Mem, MemVal, 1); if (!State) return; @@ -2403,14 +2409,12 @@ bool ento::shouldRegisterCStringModeling(const LangOptions &LO) { void ento::register##name(CheckerManager &mgr) { \ CStringChecker *checker = mgr.getChecker<CStringChecker>(); \ checker->Filter.Check##name = true; \ - checker->Filter.CheckName##name = mgr.getCurrentCheckName(); \ + checker->Filter.CheckName##name = mgr.getCurrentCheckerName(); \ } \ \ - bool ento::shouldRegister##name(const LangOptions &LO) { \ - return true; \ - } + bool ento::shouldRegister##name(const LangOptions &LO) { return true; } - REGISTER_CHECKER(CStringNullArg) - REGISTER_CHECKER(CStringOutOfBounds) - REGISTER_CHECKER(CStringBufferOverlap) +REGISTER_CHECKER(CStringNullArg) +REGISTER_CHECKER(CStringOutOfBounds) +REGISTER_CHECKER(CStringBufferOverlap) REGISTER_CHECKER(CStringNotNullTerm) diff --git a/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp b/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp index b828ac0592363..d84fcc69a4925 100644 --- a/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp @@ -156,14 +156,21 @@ bool WalkAST::containsBadStrlcpyStrlcatPattern(const CallExpr *CE) { const Expr *DstArg = CE->getArg(0); const Expr *LenArg = CE->getArg(2); - const auto *DstArgDecl = dyn_cast<DeclRefExpr>(DstArg->IgnoreParenImpCasts()); - const auto *LenArgDecl = dyn_cast<DeclRefExpr>(LenArg->IgnoreParenLValueCasts()); + const auto *DstArgDRE = dyn_cast<DeclRefExpr>(DstArg->IgnoreParenImpCasts()); + const auto *LenArgDRE = + dyn_cast<DeclRefExpr>(LenArg->IgnoreParenLValueCasts()); uint64_t DstOff = 0; if (isSizeof(LenArg, DstArg)) return false; + // - size_t dstlen = sizeof(dst) - if (LenArgDecl) { - const auto *LenArgVal = dyn_cast<VarDecl>(LenArgDecl->getDecl()); + if (LenArgDRE) { + const auto *LenArgVal = dyn_cast<VarDecl>(LenArgDRE->getDecl()); + // If it's an EnumConstantDecl instead, then we're missing out on something. + if (!LenArgVal) { + assert(isa<EnumConstantDecl>(LenArgDRE->getDecl())); + return false; + } if (LenArgVal->getInit()) LenArg = LenArgVal->getInit(); } @@ -177,9 +184,10 @@ bool WalkAST::containsBadStrlcpyStrlcatPattern(const CallExpr *CE) { // Case when there is pointer arithmetic on the destination buffer // especially when we offset from the base decreasing the // buffer length accordingly. - if (!DstArgDecl) { - if (const auto *BE = dyn_cast<BinaryOperator>(DstArg->IgnoreParenImpCasts())) { - DstArgDecl = dyn_cast<DeclRefExpr>(BE->getLHS()->IgnoreParenImpCasts()); + if (!DstArgDRE) { + if (const auto *BE = + dyn_cast<BinaryOperator>(DstArg->IgnoreParenImpCasts())) { + DstArgDRE = dyn_cast<DeclRefExpr>(BE->getLHS()->IgnoreParenImpCasts()); if (BE->getOpcode() == BO_Add) { if ((IL = dyn_cast<IntegerLiteral>(BE->getRHS()->IgnoreParenImpCasts()))) { DstOff = IL->getValue().getZExtValue(); @@ -187,8 +195,9 @@ bool WalkAST::containsBadStrlcpyStrlcatPattern(const CallExpr *CE) { } } } - if (DstArgDecl) { - if (const auto *Buffer = dyn_cast<ConstantArrayType>(DstArgDecl->getType())) { + if (DstArgDRE) { + if (const auto *Buffer = + dyn_cast<ConstantArrayType>(DstArgDRE->getType())) { ASTContext &C = BR.getContext(); uint64_t BufferLen = C.getTypeSize(Buffer) / 8; auto RemainingBufferLen = BufferLen - DstOff; diff --git a/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp b/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp index 5a7eba0760fe5..2fcb765cd4eef 100644 --- a/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp @@ -49,7 +49,7 @@ class CallAndMessageChecker public: DefaultBool Check_CallAndMessageUnInitRefArg; - CheckName CheckName_CallAndMessageUnInitRefArg; + CheckerNameRef CheckName_CallAndMessageUnInitRefArg; void checkPreStmt(const CallExpr *CE, CheckerContext &C) const; void checkPreStmt(const CXXDeleteExpr *DE, CheckerContext &C) const; @@ -95,7 +95,7 @@ void CallAndMessageChecker::emitBadCall(BugType *BT, CheckerContext &C, if (!N) return; - auto R = llvm::make_unique<BugReport>(*BT, BT->getName(), N); + auto R = std::make_unique<PathSensitiveBugReport>(*BT, BT->getDescription(), N); if (BadE) { R->addRange(BadE->getSourceRange()); if (BadE->isGLValue()) @@ -175,7 +175,7 @@ bool CallAndMessageChecker::uninitRefOrPointer( if (PSV.isUndef()) { if (ExplodedNode *N = C.generateErrorNode()) { LazyInit_BT(BD, BT); - auto R = llvm::make_unique<BugReport>(*BT, Os.str(), N); + auto R = std::make_unique<PathSensitiveBugReport>(*BT, Os.str(), N); R->addRange(ArgRange); if (ArgEx) bugreporter::trackExpressionValue(N, ArgEx, *R); @@ -252,7 +252,7 @@ bool CallAndMessageChecker::PreVisitProcessArg(CheckerContext &C, SmallString<200> Buf; llvm::raw_svector_ostream Os(Buf); describeUninitializedArgumentInCall(Call, ArgumentNumber, Os); - auto R = llvm::make_unique<BugReport>(*BT, Os.str(), N); + auto R = std::make_unique<PathSensitiveBugReport>(*BT, Os.str(), N); R->addRange(ArgRange); if (ArgEx) @@ -295,7 +295,7 @@ bool CallAndMessageChecker::PreVisitProcessArg(CheckerContext &C, } // Generate a report for this bug. - auto R = llvm::make_unique<BugReport>(*BT, os.str(), N); + auto R = std::make_unique<PathSensitiveBugReport>(*BT, os.str(), N); R->addRange(ArgRange); if (ArgEx) @@ -358,7 +358,7 @@ void CallAndMessageChecker::checkPreStmt(const CXXDeleteExpr *DE, else Desc = "Argument to 'delete' is uninitialized"; BugType *BT = BT_cxx_delete_undef.get(); - auto R = llvm::make_unique<BugReport>(*BT, Desc, N); + auto R = std::make_unique<PathSensitiveBugReport>(*BT, Desc, N); bugreporter::trackExpressionValue(N, DE, *R); C.emitReport(std::move(R)); return; @@ -420,8 +420,8 @@ void CallAndMessageChecker::checkPreCall(const CallEvent &Call, << (Params == 1 ? "" : "s") << " is called with fewer (" << Call.getNumArgs() << ")"; - C.emitReport( - llvm::make_unique<BugReport>(*BT_call_few_args, os.str(), N)); + C.emitReport(std::make_unique<PathSensitiveBugReport>(*BT_call_few_args, + os.str(), N)); } } @@ -482,7 +482,7 @@ void CallAndMessageChecker::checkPreObjCMessage(const ObjCMethodCall &msg, } assert(BT && "Unknown message kind."); - auto R = llvm::make_unique<BugReport>(*BT, BT->getName(), N); + auto R = std::make_unique<PathSensitiveBugReport>(*BT, BT->getDescription(), N); const ObjCMessageExpr *ME = msg.getOriginExpr(); R->addRange(ME->getReceiverRange()); @@ -525,7 +525,8 @@ void CallAndMessageChecker::emitNilReceiverBug(CheckerContext &C, os << "' that will be garbage"; } - auto report = llvm::make_unique<BugReport>(*BT_msg_ret, os.str(), N); + auto report = + std::make_unique<PathSensitiveBugReport>(*BT_msg_ret, os.str(), N); report->addRange(ME->getReceiverRange()); // FIXME: This won't track "self" in messages to super. if (const Expr *receiver = ME->getInstanceReceiver()) { @@ -611,7 +612,7 @@ bool ento::shouldRegisterCallAndMessageChecker(const LangOptions &LO) { void ento::registerCallAndMessageUnInitRefArg(CheckerManager &mgr) { CallAndMessageChecker *Checker = mgr.getChecker<CallAndMessageChecker>(); Checker->Check_CallAndMessageUnInitRefArg = true; - Checker->CheckName_CallAndMessageUnInitRefArg = mgr.getCurrentCheckName(); + Checker->CheckName_CallAndMessageUnInitRefArg = mgr.getCurrentCheckerName(); } bool ento::shouldRegisterCallAndMessageUnInitRefArg(const LangOptions &LO) { diff --git a/lib/StaticAnalyzer/Checkers/CastSizeChecker.cpp b/lib/StaticAnalyzer/Checkers/CastSizeChecker.cpp index 05ece961467fb..51c1d4409929e 100644 --- a/lib/StaticAnalyzer/Checkers/CastSizeChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/CastSizeChecker.cpp @@ -132,7 +132,8 @@ void CastSizeChecker::checkPreStmt(const CastExpr *CE,CheckerContext &C) const { 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); + auto R = std::make_unique<PathSensitiveBugReport>(*BT, BT->getDescription(), + errorNode); R->addRange(CE->getSourceRange()); C.emitReport(std::move(R)); } diff --git a/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp b/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp index ff5d12c27c69f..cc1c9a66b90e2 100644 --- a/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp @@ -6,178 +6,429 @@ // //===----------------------------------------------------------------------===// // -// This defines CastValueChecker which models casts of custom RTTIs. +// This defines CastValueChecker which models casts of custom RTTIs. +// +// TODO list: +// - It only allows one succesful cast between two types however in the wild +// the object could be casted to multiple types. +// - It needs to check the most likely type information from the dynamic type +// map to increase precision of dynamic casting. // //===----------------------------------------------------------------------===// +#include "clang/AST/DeclTemplate.h" #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" #include "clang/StaticAnalyzer/Core/Checker.h" #include "clang/StaticAnalyzer/Core/CheckerManager.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicType.h" #include "llvm/ADT/Optional.h" +#include <utility> using namespace clang; using namespace ento; namespace { class CastValueChecker : public Checker<eval::Call> { + enum class CallKind { Function, Method, InstanceOf }; + using CastCheck = - std::function<void(const CastValueChecker *, const CallExpr *, + std::function<void(const CastValueChecker *, const CallEvent &Call, DefinedOrUnknownSVal, CheckerContext &)>; public: - // We have three cases to evaluate a cast: - // 1) The parameter is non-null, the return value is non-null - // 2) The parameter is non-null, the return value is null - // 3) The parameter is null, the return value is null - // + // We have five cases to evaluate a cast: + // 1) The parameter is non-null, the return value is non-null. + // 2) The parameter is non-null, the return value is null. + // 3) The parameter is null, the return value is null. // cast: 1; dyn_cast: 1, 2; cast_or_null: 1, 3; dyn_cast_or_null: 1, 2, 3. + // + // 4) castAs: Has no parameter, the return value is non-null. + // 5) getAs: Has no parameter, the return value is null or non-null. + // + // We have two cases to check the parameter is an instance of the given type. + // 1) isa: The parameter is non-null, returns boolean. + // 2) isa_and_nonnull: The parameter is null or non-null, returns boolean. bool evalCall(const CallEvent &Call, CheckerContext &C) const; private: - // These are known in the LLVM project. - const CallDescriptionMap<CastCheck> CDM = { - {{{"llvm", "cast"}, 1}, &CastValueChecker::evalCast}, - {{{"llvm", "dyn_cast"}, 1}, &CastValueChecker::evalDynCast}, - {{{"llvm", "cast_or_null"}, 1}, &CastValueChecker::evalCastOrNull}, + // These are known in the LLVM project. The pairs are in the following form: + // {{{namespace, call}, argument-count}, {callback, kind}} + const CallDescriptionMap<std::pair<CastCheck, CallKind>> CDM = { + {{{"llvm", "cast"}, 1}, + {&CastValueChecker::evalCast, CallKind::Function}}, + {{{"llvm", "dyn_cast"}, 1}, + {&CastValueChecker::evalDynCast, CallKind::Function}}, + {{{"llvm", "cast_or_null"}, 1}, + {&CastValueChecker::evalCastOrNull, CallKind::Function}}, {{{"llvm", "dyn_cast_or_null"}, 1}, - &CastValueChecker::evalDynCastOrNull}}; - - void evalCast(const CallExpr *CE, DefinedOrUnknownSVal ParamDV, + {&CastValueChecker::evalDynCastOrNull, CallKind::Function}}, + {{{"clang", "castAs"}, 0}, + {&CastValueChecker::evalCastAs, CallKind::Method}}, + {{{"clang", "getAs"}, 0}, + {&CastValueChecker::evalGetAs, CallKind::Method}}, + {{{"llvm", "isa"}, 1}, + {&CastValueChecker::evalIsa, CallKind::InstanceOf}}, + {{{"llvm", "isa_and_nonnull"}, 1}, + {&CastValueChecker::evalIsaAndNonNull, CallKind::InstanceOf}}}; + + void evalCast(const CallEvent &Call, DefinedOrUnknownSVal DV, CheckerContext &C) const; - void evalDynCast(const CallExpr *CE, DefinedOrUnknownSVal ParamDV, + void evalDynCast(const CallEvent &Call, DefinedOrUnknownSVal DV, CheckerContext &C) const; - void evalCastOrNull(const CallExpr *CE, DefinedOrUnknownSVal ParamDV, + void evalCastOrNull(const CallEvent &Call, DefinedOrUnknownSVal DV, CheckerContext &C) const; - void evalDynCastOrNull(const CallExpr *CE, DefinedOrUnknownSVal ParamDV, + void evalDynCastOrNull(const CallEvent &Call, DefinedOrUnknownSVal DV, + CheckerContext &C) const; + void evalCastAs(const CallEvent &Call, DefinedOrUnknownSVal DV, + CheckerContext &C) const; + void evalGetAs(const CallEvent &Call, DefinedOrUnknownSVal DV, + CheckerContext &C) const; + void evalIsa(const CallEvent &Call, DefinedOrUnknownSVal DV, + CheckerContext &C) const; + void evalIsaAndNonNull(const CallEvent &Call, DefinedOrUnknownSVal DV, CheckerContext &C) const; }; } // namespace -static std::string getCastName(const Expr *Cast) { - return Cast->getType()->getPointeeCXXRecordDecl()->getNameAsString(); +static bool isInfeasibleCast(const DynamicCastInfo *CastInfo, + bool CastSucceeds) { + if (!CastInfo) + return false; + + return CastSucceeds ? CastInfo->fails() : CastInfo->succeeds(); } -static void evalNonNullParamNonNullReturn(const CallExpr *CE, - DefinedOrUnknownSVal ParamDV, - CheckerContext &C) { - ProgramStateRef State = C.getState()->assume(ParamDV, true); - if (!State) - return; +static const NoteTag *getNoteTag(CheckerContext &C, + const DynamicCastInfo *CastInfo, + QualType CastToTy, const Expr *Object, + bool CastSucceeds, bool IsKnownCast) { + std::string CastToName = + CastInfo ? CastInfo->to()->getPointeeCXXRecordDecl()->getNameAsString() + : CastToTy->getPointeeCXXRecordDecl()->getNameAsString(); + Object = Object->IgnoreParenImpCasts(); + + return C.getNoteTag( + [=]() -> std::string { + SmallString<128> Msg; + llvm::raw_svector_ostream Out(Msg); - State = State->BindExpr(CE, C.getLocationContext(), ParamDV, false); + if (!IsKnownCast) + Out << "Assuming "; - std::string CastFromName = getCastName(CE->getArg(0)); - std::string CastToName = getCastName(CE); + if (const auto *DRE = dyn_cast<DeclRefExpr>(Object)) { + Out << '\'' << DRE->getDecl()->getNameAsString() << '\''; + } else if (const auto *ME = dyn_cast<MemberExpr>(Object)) { + Out << (IsKnownCast ? "Field '" : "field '") + << ME->getMemberDecl()->getNameAsString() << '\''; + } else { + Out << (IsKnownCast ? "The object" : "the object"); + } - const NoteTag *CastTag = C.getNoteTag( - [CastFromName, CastToName](BugReport &) -> std::string { - SmallString<128> Msg; - llvm::raw_svector_ostream Out(Msg); + Out << ' ' << (CastSucceeds ? "is a" : "is not a") << " '" << CastToName + << '\''; - Out << "Assuming dynamic cast from '" << CastFromName << "' to '" - << CastToName << "' succeeds"; return Out.str(); }, /*IsPrunable=*/true); +} - C.addTransition(State, CastTag); +//===----------------------------------------------------------------------===// +// Main logic to evaluate a cast. +//===----------------------------------------------------------------------===// + +static QualType alignReferenceTypes(QualType toAlign, QualType alignTowards, + ASTContext &ACtx) { + if (alignTowards->isLValueReferenceType() && + alignTowards.isConstQualified()) { + toAlign.addConst(); + return ACtx.getLValueReferenceType(toAlign); + } else if (alignTowards->isLValueReferenceType()) + return ACtx.getLValueReferenceType(toAlign); + else if (alignTowards->isRValueReferenceType()) + return ACtx.getRValueReferenceType(toAlign); + + llvm_unreachable("Must align towards a reference type!"); } -static void evalNonNullParamNullReturn(const CallExpr *CE, - DefinedOrUnknownSVal ParamDV, - CheckerContext &C) { - ProgramStateRef State = C.getState()->assume(ParamDV, true); +static void addCastTransition(const CallEvent &Call, DefinedOrUnknownSVal DV, + CheckerContext &C, bool IsNonNullParam, + bool IsNonNullReturn, + bool IsCheckedCast = false) { + ProgramStateRef State = C.getState()->assume(DV, IsNonNullParam); if (!State) return; - State = State->BindExpr(CE, C.getLocationContext(), - C.getSValBuilder().makeNull(), false); + const Expr *Object; + QualType CastFromTy; + QualType CastToTy = Call.getResultType(); + + if (Call.getNumArgs() > 0) { + Object = Call.getArgExpr(0); + CastFromTy = Call.parameters()[0]->getType(); + } else { + Object = cast<CXXInstanceCall>(&Call)->getCXXThisExpr(); + CastFromTy = Object->getType(); + if (CastToTy->isPointerType()) { + if (!CastFromTy->isPointerType()) + return; + } else { + if (!CastFromTy->isReferenceType()) + return; + + CastFromTy = alignReferenceTypes(CastFromTy, CastToTy, C.getASTContext()); + } + } + + const MemRegion *MR = DV.getAsRegion(); + const DynamicCastInfo *CastInfo = + getDynamicCastInfo(State, MR, CastFromTy, CastToTy); + + // We assume that every checked cast succeeds. + bool CastSucceeds = IsCheckedCast || CastFromTy == CastToTy; + if (!CastSucceeds) { + if (CastInfo) + CastSucceeds = IsNonNullReturn && CastInfo->succeeds(); + else + CastSucceeds = IsNonNullReturn; + } + + // Check for infeasible casts. + if (isInfeasibleCast(CastInfo, CastSucceeds)) { + C.generateSink(State, C.getPredecessor()); + return; + } + + // Store the type and the cast information. + bool IsKnownCast = CastInfo || IsCheckedCast || CastFromTy == CastToTy; + if (!IsKnownCast || IsCheckedCast) + State = setDynamicTypeAndCastInfo(State, MR, CastFromTy, CastToTy, + CastSucceeds); + + SVal V = CastSucceeds ? C.getSValBuilder().evalCast(DV, CastToTy, CastFromTy) + : C.getSValBuilder().makeNull(); + C.addTransition( + State->BindExpr(Call.getOriginExpr(), C.getLocationContext(), V, false), + getNoteTag(C, CastInfo, CastToTy, Object, CastSucceeds, IsKnownCast)); +} - std::string CastFromName = getCastName(CE->getArg(0)); - std::string CastToName = getCastName(CE); +static void addInstanceOfTransition(const CallEvent &Call, + DefinedOrUnknownSVal DV, + ProgramStateRef State, CheckerContext &C, + bool IsInstanceOf) { + const FunctionDecl *FD = Call.getDecl()->getAsFunction(); + QualType CastFromTy = Call.parameters()[0]->getType(); + QualType CastToTy = FD->getTemplateSpecializationArgs()->get(0).getAsType(); + if (CastFromTy->isPointerType()) + CastToTy = C.getASTContext().getPointerType(CastToTy); + else if (CastFromTy->isReferenceType()) + CastToTy = alignReferenceTypes(CastToTy, CastFromTy, C.getASTContext()); + else + return; - const NoteTag *CastTag = C.getNoteTag( - [CastFromName, CastToName](BugReport &) -> std::string { - SmallString<128> Msg; - llvm::raw_svector_ostream Out(Msg); + const MemRegion *MR = DV.getAsRegion(); + const DynamicCastInfo *CastInfo = + getDynamicCastInfo(State, MR, CastFromTy, CastToTy); - Out << "Assuming dynamic cast from '" << CastFromName << "' to '" - << CastToName << "' fails"; - return Out.str(); - }, - /*IsPrunable=*/true); + bool CastSucceeds; + if (CastInfo) + CastSucceeds = IsInstanceOf && CastInfo->succeeds(); + else + CastSucceeds = IsInstanceOf || CastFromTy == CastToTy; - C.addTransition(State, CastTag); + if (isInfeasibleCast(CastInfo, CastSucceeds)) { + C.generateSink(State, C.getPredecessor()); + return; + } + + // Store the type and the cast information. + bool IsKnownCast = CastInfo || CastFromTy == CastToTy; + if (!IsKnownCast) + State = setDynamicTypeAndCastInfo(State, MR, CastFromTy, CastToTy, + IsInstanceOf); + + C.addTransition( + State->BindExpr(Call.getOriginExpr(), C.getLocationContext(), + C.getSValBuilder().makeTruthVal(CastSucceeds)), + getNoteTag(C, CastInfo, CastToTy, Call.getArgExpr(0), CastSucceeds, + IsKnownCast)); } -static void evalNullParamNullReturn(const CallExpr *CE, - DefinedOrUnknownSVal ParamDV, - CheckerContext &C) { - ProgramStateRef State = C.getState()->assume(ParamDV, false); - if (!State) - return; +//===----------------------------------------------------------------------===// +// Evaluating cast, dyn_cast, cast_or_null, dyn_cast_or_null. +//===----------------------------------------------------------------------===// - State = State->BindExpr(CE, C.getLocationContext(), - C.getSValBuilder().makeNull(), false); +static void evalNonNullParamNonNullReturn(const CallEvent &Call, + DefinedOrUnknownSVal DV, + CheckerContext &C, + bool IsCheckedCast = false) { + addCastTransition(Call, DV, C, /*IsNonNullParam=*/true, + /*IsNonNullReturn=*/true, IsCheckedCast); +} - const NoteTag *CastTag = - C.getNoteTag("Assuming null pointer is passed into cast", - /*IsPrunable=*/true); +static void evalNonNullParamNullReturn(const CallEvent &Call, + DefinedOrUnknownSVal DV, + CheckerContext &C) { + addCastTransition(Call, DV, C, /*IsNonNullParam=*/true, + /*IsNonNullReturn=*/false); +} - C.addTransition(State, CastTag); +static void evalNullParamNullReturn(const CallEvent &Call, + DefinedOrUnknownSVal DV, + CheckerContext &C) { + if (ProgramStateRef State = C.getState()->assume(DV, false)) + C.addTransition(State->BindExpr(Call.getOriginExpr(), + C.getLocationContext(), + C.getSValBuilder().makeNull(), false), + C.getNoteTag("Assuming null pointer is passed into cast", + /*IsPrunable=*/true)); } -void CastValueChecker::evalCast(const CallExpr *CE, - DefinedOrUnknownSVal ParamDV, +void CastValueChecker::evalCast(const CallEvent &Call, DefinedOrUnknownSVal DV, CheckerContext &C) const { - evalNonNullParamNonNullReturn(CE, ParamDV, C); + evalNonNullParamNonNullReturn(Call, DV, C, /*IsCheckedCast=*/true); } -void CastValueChecker::evalDynCast(const CallExpr *CE, - DefinedOrUnknownSVal ParamDV, +void CastValueChecker::evalDynCast(const CallEvent &Call, + DefinedOrUnknownSVal DV, CheckerContext &C) const { - evalNonNullParamNonNullReturn(CE, ParamDV, C); - evalNonNullParamNullReturn(CE, ParamDV, C); + evalNonNullParamNonNullReturn(Call, DV, C); + evalNonNullParamNullReturn(Call, DV, C); } -void CastValueChecker::evalCastOrNull(const CallExpr *CE, - DefinedOrUnknownSVal ParamDV, +void CastValueChecker::evalCastOrNull(const CallEvent &Call, + DefinedOrUnknownSVal DV, CheckerContext &C) const { - evalNonNullParamNonNullReturn(CE, ParamDV, C); - evalNullParamNullReturn(CE, ParamDV, C); + evalNonNullParamNonNullReturn(Call, DV, C); + evalNullParamNullReturn(Call, DV, C); } -void CastValueChecker::evalDynCastOrNull(const CallExpr *CE, - DefinedOrUnknownSVal ParamDV, +void CastValueChecker::evalDynCastOrNull(const CallEvent &Call, + DefinedOrUnknownSVal DV, CheckerContext &C) const { - evalNonNullParamNonNullReturn(CE, ParamDV, C); - evalNonNullParamNullReturn(CE, ParamDV, C); - evalNullParamNullReturn(CE, ParamDV, C); + evalNonNullParamNonNullReturn(Call, DV, C); + evalNonNullParamNullReturn(Call, DV, C); + evalNullParamNullReturn(Call, DV, C); } -bool CastValueChecker::evalCall(const CallEvent &Call, - CheckerContext &C) const { - const CastCheck *Check = CDM.lookup(Call); - if (!Check) - return false; +//===----------------------------------------------------------------------===// +// Evaluating castAs, getAs. +//===----------------------------------------------------------------------===// - const auto *CE = cast<CallExpr>(Call.getOriginExpr()); - if (!CE) - return false; +static void evalZeroParamNonNullReturn(const CallEvent &Call, + DefinedOrUnknownSVal DV, + CheckerContext &C, + bool IsCheckedCast = false) { + addCastTransition(Call, DV, C, /*IsNonNullParam=*/true, + /*IsNonNullReturn=*/true, IsCheckedCast); +} + +static void evalZeroParamNullReturn(const CallEvent &Call, + DefinedOrUnknownSVal DV, + CheckerContext &C) { + addCastTransition(Call, DV, C, /*IsNonNullParam=*/true, + /*IsNonNullReturn=*/false); +} + +void CastValueChecker::evalCastAs(const CallEvent &Call, + DefinedOrUnknownSVal DV, + CheckerContext &C) const { + evalZeroParamNonNullReturn(Call, DV, C, /*IsCheckedCast=*/true); +} + +void CastValueChecker::evalGetAs(const CallEvent &Call, DefinedOrUnknownSVal DV, + CheckerContext &C) const { + evalZeroParamNonNullReturn(Call, DV, C); + evalZeroParamNullReturn(Call, DV, C); +} - // If we cannot obtain both of the classes we cannot be sure how to model it. - if (!CE->getType()->getPointeeCXXRecordDecl() || - !CE->getArg(0)->getType()->getPointeeCXXRecordDecl()) +//===----------------------------------------------------------------------===// +// Evaluating isa, isa_and_nonnull. +//===----------------------------------------------------------------------===// + +void CastValueChecker::evalIsa(const CallEvent &Call, DefinedOrUnknownSVal DV, + CheckerContext &C) const { + ProgramStateRef NonNullState, NullState; + std::tie(NonNullState, NullState) = C.getState()->assume(DV); + + if (NonNullState) { + addInstanceOfTransition(Call, DV, NonNullState, C, /*IsInstanceOf=*/true); + addInstanceOfTransition(Call, DV, NonNullState, C, /*IsInstanceOf=*/false); + } + + if (NullState) { + C.generateSink(NullState, C.getPredecessor()); + } +} + +void CastValueChecker::evalIsaAndNonNull(const CallEvent &Call, + DefinedOrUnknownSVal DV, + CheckerContext &C) const { + ProgramStateRef NonNullState, NullState; + std::tie(NonNullState, NullState) = C.getState()->assume(DV); + + if (NonNullState) { + addInstanceOfTransition(Call, DV, NonNullState, C, /*IsInstanceOf=*/true); + addInstanceOfTransition(Call, DV, NonNullState, C, /*IsInstanceOf=*/false); + } + + if (NullState) { + addInstanceOfTransition(Call, DV, NullState, C, /*IsInstanceOf=*/false); + } +} + +//===----------------------------------------------------------------------===// +// Main logic to evaluate a call. +//===----------------------------------------------------------------------===// + +bool CastValueChecker::evalCall(const CallEvent &Call, + CheckerContext &C) const { + const auto *Lookup = CDM.lookup(Call); + if (!Lookup) return false; - SVal ParamV = Call.getArgSVal(0); - auto ParamDV = ParamV.getAs<DefinedOrUnknownSVal>(); - if (!ParamDV) + const CastCheck &Check = Lookup->first; + CallKind Kind = Lookup->second; + + Optional<DefinedOrUnknownSVal> DV; + + switch (Kind) { + case CallKind::Function: { + // We only model casts from pointers to pointers or from references + // to references. Other casts are most likely specialized and we + // cannot model them. + QualType ParamT = Call.parameters()[0]->getType(); + QualType ResultT = Call.getResultType(); + if (!(ParamT->isPointerType() && ResultT->isPointerType()) && + !(ParamT->isReferenceType() && ResultT->isReferenceType())) + return false; + + DV = Call.getArgSVal(0).getAs<DefinedOrUnknownSVal>(); + break; + } + case CallKind::InstanceOf: { + // We need to obtain the only template argument to determinte the type. + const FunctionDecl *FD = Call.getDecl()->getAsFunction(); + if (!FD || !FD->getTemplateSpecializationArgs()) + return false; + + DV = Call.getArgSVal(0).getAs<DefinedOrUnknownSVal>(); + break; + } + case CallKind::Method: + const auto *InstanceCall = dyn_cast<CXXInstanceCall>(&Call); + if (!InstanceCall) + return false; + + DV = InstanceCall->getCXXThisVal().getAs<DefinedOrUnknownSVal>(); + break; + } + + if (!DV) return false; - (*Check)(this, CE, *ParamDV, C); + Check(this, Call, *DV, C); return true; } diff --git a/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp b/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp index a7ca814c8f967..50b872bd8682f 100644 --- a/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp +++ b/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp @@ -28,6 +28,7 @@ //===----------------------------------------------------------------------===// #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" +#include "clang/Analysis/PathDiagnostic.h" #include "clang/AST/Attr.h" #include "clang/AST/DeclObjC.h" #include "clang/AST/Expr.h" @@ -36,7 +37,6 @@ #include "clang/Basic/TargetInfo.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" -#include "clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h" #include "clang/StaticAnalyzer/Core/Checker.h" #include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" @@ -576,9 +576,8 @@ void ObjCDeallocChecker::diagnoseMissingReleases(CheckerContext &C) const { OS << " by a synthesized property but not released" " before '[super dealloc]'"; - std::unique_ptr<BugReport> BR( - new BugReport(*MissingReleaseBugType, OS.str(), ErrNode)); - + auto BR = std::make_unique<PathSensitiveBugReport>(*MissingReleaseBugType, + OS.str(), ErrNode); C.emitReport(std::move(BR)); } @@ -699,8 +698,8 @@ bool ObjCDeallocChecker::diagnoseExtraRelease(SymbolRef ReleasedValue, OS << " property but was released in 'dealloc'"; } - std::unique_ptr<BugReport> BR( - new BugReport(*ExtraReleaseBugType, OS.str(), ErrNode)); + auto BR = std::make_unique<PathSensitiveBugReport>(*ExtraReleaseBugType, + OS.str(), ErrNode); BR->addRange(M.getOriginExpr()->getSourceRange()); C.emitReport(std::move(BR)); @@ -741,8 +740,8 @@ bool ObjCDeallocChecker::diagnoseMistakenDealloc(SymbolRef DeallocedValue, OS << "'" << *PropImpl->getPropertyIvarDecl() << "' should be released rather than deallocated"; - std::unique_ptr<BugReport> BR( - new BugReport(*MistakenDeallocBugType, OS.str(), ErrNode)); + auto BR = std::make_unique<PathSensitiveBugReport>(*MistakenDeallocBugType, + OS.str(), ErrNode); BR->addRange(M.getOriginExpr()->getSourceRange()); C.emitReport(std::move(BR)); diff --git a/lib/StaticAnalyzer/Checkers/CheckObjCInstMethSignature.cpp b/lib/StaticAnalyzer/Checkers/CheckObjCInstMethSignature.cpp index a020d33bfd95f..1694c237cda42 100644 --- a/lib/StaticAnalyzer/Checkers/CheckObjCInstMethSignature.cpp +++ b/lib/StaticAnalyzer/Checkers/CheckObjCInstMethSignature.cpp @@ -13,11 +13,11 @@ //===----------------------------------------------------------------------===// #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" +#include "clang/Analysis/PathDiagnostic.h" #include "clang/AST/ASTContext.h" #include "clang/AST/DeclObjC.h" #include "clang/AST/Type.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h" -#include "clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h" #include "clang/StaticAnalyzer/Core/Checker.h" #include "llvm/ADT/DenseMap.h" #include "llvm/Support/raw_ostream.h" diff --git a/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp b/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp index 3f1c213a56478..260a2896e78c8 100644 --- a/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp +++ b/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp @@ -50,19 +50,19 @@ struct ChecksFilter { DefaultBool check_FloatLoopCounter; DefaultBool check_UncheckedReturn; - CheckName checkName_bcmp; - CheckName checkName_bcopy; - CheckName checkName_bzero; - CheckName checkName_gets; - CheckName checkName_getpw; - CheckName checkName_mktemp; - CheckName checkName_mkstemp; - CheckName checkName_strcpy; - CheckName checkName_DeprecatedOrUnsafeBufferHandling; - CheckName checkName_rand; - CheckName checkName_vfork; - CheckName checkName_FloatLoopCounter; - CheckName checkName_UncheckedReturn; + CheckerNameRef checkName_bcmp; + CheckerNameRef checkName_bcopy; + CheckerNameRef checkName_bzero; + CheckerNameRef checkName_gets; + CheckerNameRef checkName_getpw; + CheckerNameRef checkName_mktemp; + CheckerNameRef checkName_mkstemp; + CheckerNameRef checkName_strcpy; + CheckerNameRef checkName_DeprecatedOrUnsafeBufferHandling; + CheckerNameRef checkName_rand; + CheckerNameRef checkName_vfork; + CheckerNameRef checkName_FloatLoopCounter; + CheckerNameRef checkName_UncheckedReturn; }; class WalkAST : public StmtVisitor<WalkAST> { @@ -204,6 +204,8 @@ void WalkAST::VisitForStmt(ForStmt *FS) { // Implements: CERT security coding advisory FLP-30. //===----------------------------------------------------------------------===// +// Returns either 'x' or 'y', depending on which one of them is incremented +// in 'expr', or nullptr if none of them is incremented. static const DeclRefExpr* getIncrementedVar(const Expr *expr, const VarDecl *x, const VarDecl *y) { expr = expr->IgnoreParenCasts(); @@ -289,14 +291,15 @@ void WalkAST::checkLoopConditionForFloat(const ForStmt *FS) { // Does either variable appear in increment? const DeclRefExpr *drInc = getIncrementedVar(increment, vdLHS, vdRHS); - if (!drInc) return; + const VarDecl *vdInc = cast<VarDecl>(drInc->getDecl()); + assert(vdInc && (vdInc == vdLHS || vdInc == vdRHS)); + // Emit the error. First figure out which DeclRefExpr in the condition // referenced the compared variable. - assert(drInc->getDecl()); - const DeclRefExpr *drCond = vdLHS == drInc->getDecl() ? drLHS : drRHS; + const DeclRefExpr *drCond = vdLHS == vdInc ? drLHS : drRHS; SmallVector<SourceRange, 2> ranges; SmallString<256> sbuf; @@ -1012,14 +1015,12 @@ bool ento::shouldRegisterSecuritySyntaxChecker(const LangOptions &LO) { #define REGISTER_CHECKER(name) \ void ento::register##name(CheckerManager &mgr) { \ - SecuritySyntaxChecker *checker = mgr.getChecker<SecuritySyntaxChecker>(); \ + SecuritySyntaxChecker *checker = mgr.getChecker<SecuritySyntaxChecker>(); \ checker->filter.check_##name = true; \ - checker->filter.checkName_##name = mgr.getCurrentCheckName(); \ + checker->filter.checkName_##name = mgr.getCurrentCheckerName(); \ } \ \ - bool ento::shouldRegister##name(const LangOptions &LO) { \ - return true; \ - } + bool ento::shouldRegister##name(const LangOptions &LO) { return true; } REGISTER_CHECKER(bcmp) REGISTER_CHECKER(bcopy) diff --git a/lib/StaticAnalyzer/Checkers/ChrootChecker.cpp b/lib/StaticAnalyzer/Checkers/ChrootChecker.cpp index 9fffedfccd871..7a41a7b6b216e 100644 --- a/lib/StaticAnalyzer/Checkers/ChrootChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/ChrootChecker.cpp @@ -127,7 +127,7 @@ void ChrootChecker::checkPreCall(const CallEvent &Call, BT_BreakJail.reset(new BuiltinBug( this, "Break out of jail", "No call of chdir(\"/\") immediately " "after chroot")); - C.emitReport(llvm::make_unique<BugReport>( + C.emitReport(std::make_unique<PathSensitiveBugReport>( *BT_BreakJail, BT_BreakJail->getDescription(), N)); } } diff --git a/lib/StaticAnalyzer/Checkers/CloneChecker.cpp b/lib/StaticAnalyzer/Checkers/CloneChecker.cpp index 4fc225056d4c7..ce45b5be34c9f 100644 --- a/lib/StaticAnalyzer/Checkers/CloneChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/CloneChecker.cpp @@ -114,8 +114,8 @@ void CloneChecker::reportClones( for (const CloneDetector::CloneGroup &Group : CloneGroups) { // We group the clones by printing the first as a warning and all others // as a note. - auto R = llvm::make_unique<BugReport>(*BT_Exact, "Duplicate code detected", - makeLocation(Group.front(), Mgr)); + auto R = std::make_unique<BasicBugReport>( + *BT_Exact, "Duplicate code detected", makeLocation(Group.front(), Mgr)); R->addRange(Group.front().getSourceRange()); for (unsigned i = 1; i < Group.size(); ++i) @@ -169,7 +169,7 @@ void CloneChecker::reportSuspiciousClones( // which may confuse the user. // Think how to perform more accurate suggestions? - auto R = llvm::make_unique<BugReport>( + auto R = std::make_unique<BasicBugReport>( *BT_Suspicious, "Potential copy-paste error; did you really mean to use '" + Pair.FirstCloneInfo.Variable->getNameAsString() + "' here?", diff --git a/lib/StaticAnalyzer/Checkers/ConversionChecker.cpp b/lib/StaticAnalyzer/Checkers/ConversionChecker.cpp index 5058d101b8e59..8dd3132f07e2b 100644 --- a/lib/StaticAnalyzer/Checkers/ConversionChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/ConversionChecker.cpp @@ -121,7 +121,7 @@ void ConversionChecker::reportBug(ExplodedNode *N, CheckerContext &C, new BuiltinBug(this, "Conversion", "Possible loss of sign/precision.")); // Generate a report for this bug. - auto R = llvm::make_unique<BugReport>(*BT, Msg, N); + auto R = std::make_unique<PathSensitiveBugReport>(*BT, Msg, N); C.emitReport(std::move(R)); } diff --git a/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp b/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp index d5baa2bcba6fc..61441889fc649 100644 --- a/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp @@ -11,6 +11,7 @@ // //===----------------------------------------------------------------------===// +#include "clang/Lex/Lexer.h" #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" #include "clang/AST/ASTContext.h" #include "clang/AST/Attr.h" @@ -119,11 +120,20 @@ LookThroughTransitiveAssignmentsAndCommaOperators(const Expr *Ex) { } namespace { +class DeadStoresChecker : public Checker<check::ASTCodeBody> { +public: + bool ShowFixIts = false; + bool WarnForDeadNestedAssignments = true; + + void checkASTCodeBody(const Decl *D, AnalysisManager &Mgr, + BugReporter &BR) const; +}; + class DeadStoreObs : public LiveVariables::Observer { const CFG &cfg; ASTContext &Ctx; BugReporter& BR; - const CheckerBase *Checker; + const DeadStoresChecker *Checker; AnalysisDeclContext* AC; ParentMap& Parents; llvm::SmallPtrSet<const VarDecl*, 20> Escaped; @@ -135,9 +145,10 @@ class DeadStoreObs : public LiveVariables::Observer { public: DeadStoreObs(const CFG &cfg, ASTContext &ctx, BugReporter &br, - const CheckerBase *checker, AnalysisDeclContext *ac, + const DeadStoresChecker *checker, AnalysisDeclContext *ac, ParentMap &parents, - llvm::SmallPtrSet<const VarDecl *, 20> &escaped) + llvm::SmallPtrSet<const VarDecl *, 20> &escaped, + bool warnForDeadNestedAssignments) : cfg(cfg), Ctx(ctx), BR(br), Checker(checker), AC(ac), Parents(parents), Escaped(escaped), currentBlock(nullptr) {} @@ -202,12 +213,32 @@ public: llvm::raw_svector_ostream os(buf); const char *BugType = nullptr; + SmallVector<FixItHint, 1> Fixits; + switch (dsk) { - case DeadInit: + case DeadInit: { BugType = "Dead initialization"; os << "Value stored to '" << *V << "' during its initialization is never read"; + + ASTContext &ACtx = V->getASTContext(); + if (Checker->ShowFixIts) { + if (V->getInit()->HasSideEffects(ACtx, + /*IncludePossibleEffects=*/true)) { + break; + } + SourceManager &SM = ACtx.getSourceManager(); + const LangOptions &LO = ACtx.getLangOpts(); + SourceLocation L1 = + Lexer::findNextToken( + V->getTypeSourceInfo()->getTypeLoc().getEndLoc(), + SM, LO)->getEndLoc(); + SourceLocation L2 = + Lexer::getLocForEndOfToken(V->getInit()->getEndLoc(), 1, SM, LO); + Fixits.push_back(FixItHint::CreateRemoval({L1, L2})); + } break; + } case DeadIncrement: BugType = "Dead increment"; @@ -217,15 +248,20 @@ public: os << "Value stored to '" << *V << "' is never read"; break; + // eg.: f((x = foo())) case Enclosing: - // Don't report issues in this case, e.g.: "if (x = foo())", - // where 'x' is unused later. We have yet to see a case where - // this is a real bug. - return; + if (!Checker->WarnForDeadNestedAssignments) + return; + BugType = "Dead nested assignment"; + os << "Although the value stored to '" << *V + << "' is used in the enclosing expression, the value is never " + "actually read from '" + << *V << "'"; + break; } BR.EmitBasicReport(AC->getDecl(), Checker, BugType, "Dead store", os.str(), - L, R); + L, R, Fixits); } void CheckVarDecl(const VarDecl *VD, const Expr *Ex, const Expr *Val, @@ -471,35 +507,37 @@ public: // DeadStoresChecker //===----------------------------------------------------------------------===// -namespace { -class DeadStoresChecker : public Checker<check::ASTCodeBody> { -public: - void checkASTCodeBody(const Decl *D, AnalysisManager& mgr, - BugReporter &BR) const { - - // Don't do anything for template instantiations. - // Proving that code in a template instantiation is "dead" - // means proving that it is dead in all instantiations. - // This same problem exists with -Wunreachable-code. - if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) - if (FD->isTemplateInstantiation()) - return; +void DeadStoresChecker::checkASTCodeBody(const Decl *D, AnalysisManager &mgr, + BugReporter &BR) const { - if (LiveVariables *L = mgr.getAnalysis<LiveVariables>(D)) { - CFG &cfg = *mgr.getCFG(D); - AnalysisDeclContext *AC = mgr.getAnalysisDeclContext(D); - ParentMap &pmap = mgr.getParentMap(D); - FindEscaped FS; - cfg.VisitBlockStmts(FS); - DeadStoreObs A(cfg, BR.getContext(), BR, this, AC, pmap, FS.Escaped); - L->runOnAllBlocks(A); - } + // Don't do anything for template instantiations. + // Proving that code in a template instantiation is "dead" + // means proving that it is dead in all instantiations. + // This same problem exists with -Wunreachable-code. + if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) + if (FD->isTemplateInstantiation()) + return; + + if (LiveVariables *L = mgr.getAnalysis<LiveVariables>(D)) { + CFG &cfg = *mgr.getCFG(D); + AnalysisDeclContext *AC = mgr.getAnalysisDeclContext(D); + ParentMap &pmap = mgr.getParentMap(D); + FindEscaped FS; + cfg.VisitBlockStmts(FS); + DeadStoreObs A(cfg, BR.getContext(), BR, this, AC, pmap, FS.Escaped, + WarnForDeadNestedAssignments); + L->runOnAllBlocks(A); } -}; } -void ento::registerDeadStoresChecker(CheckerManager &mgr) { - mgr.registerChecker<DeadStoresChecker>(); +void ento::registerDeadStoresChecker(CheckerManager &Mgr) { + auto *Chk = Mgr.registerChecker<DeadStoresChecker>(); + + const AnalyzerOptions &AnOpts = Mgr.getAnalyzerOptions(); + Chk->WarnForDeadNestedAssignments = + AnOpts.getCheckerBooleanOption(Chk, "WarnForDeadNestedAssignments"); + Chk->ShowFixIts = + AnOpts.getCheckerBooleanOption(Chk, "ShowFixIts"); } bool ento::shouldRegisterDeadStoresChecker(const LangOptions &LO) { diff --git a/lib/StaticAnalyzer/Checkers/DebugCheckers.cpp b/lib/StaticAnalyzer/Checkers/DebugCheckers.cpp index bb9e8110b6479..0cb4be2c7fdc3 100644 --- a/lib/StaticAnalyzer/Checkers/DebugCheckers.cpp +++ b/lib/StaticAnalyzer/Checkers/DebugCheckers.cpp @@ -333,7 +333,8 @@ public: if (!Node) return; - auto Report = llvm::make_unique<BugReport>(BT_stmtLoc, "Statement", Node); + auto Report = + std::make_unique<PathSensitiveBugReport>(BT_stmtLoc, "Statement", Node); C.emitReport(std::move(Report)); } diff --git a/lib/StaticAnalyzer/Checkers/DeleteWithNonVirtualDtorChecker.cpp b/lib/StaticAnalyzer/Checkers/DeleteWithNonVirtualDtorChecker.cpp index 8bf77c109f8a6..45c1984c5e157 100644 --- a/lib/StaticAnalyzer/Checkers/DeleteWithNonVirtualDtorChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/DeleteWithNonVirtualDtorChecker.cpp @@ -27,7 +27,7 @@ #include "clang/StaticAnalyzer/Core/CheckerManager.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" -#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicTypeMap.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicType.h" #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h" using namespace clang; @@ -45,9 +45,9 @@ class DeleteWithNonVirtualDtorChecker static int X = 0; ID.AddPointer(&X); } - std::shared_ptr<PathDiagnosticPiece> VisitNode(const ExplodedNode *N, - BugReporterContext &BRC, - BugReport &BR) override; + PathDiagnosticPieceRef VisitNode(const ExplodedNode *N, + BugReporterContext &BRC, + PathSensitiveBugReport &BR) override; private: bool Satisfied; @@ -92,23 +92,23 @@ void DeleteWithNonVirtualDtorChecker::checkPreStmt(const CXXDeleteExpr *DE, "Logic error")); ExplodedNode *N = C.generateNonFatalErrorNode(); - auto R = llvm::make_unique<BugReport>(*BT, BT->getName(), N); + auto R = std::make_unique<PathSensitiveBugReport>(*BT, BT->getDescription(), N); // Mark region of problematic base class for later use in the BugVisitor. R->markInteresting(BaseClassRegion); - R->addVisitor(llvm::make_unique<DeleteBugVisitor>()); + R->addVisitor(std::make_unique<DeleteBugVisitor>()); C.emitReport(std::move(R)); } -std::shared_ptr<PathDiagnosticPiece> +PathDiagnosticPieceRef DeleteWithNonVirtualDtorChecker::DeleteBugVisitor::VisitNode( const ExplodedNode *N, BugReporterContext &BRC, - BugReport &BR) { + PathSensitiveBugReport &BR) { // Stop traversal after the first conversion was found on a path. if (Satisfied) return nullptr; - const Stmt *S = PathDiagnosticLocation::getStmt(N); + const Stmt *S = N->getStmtForDiagnostics(); if (!S) return nullptr; @@ -140,8 +140,7 @@ DeleteWithNonVirtualDtorChecker::DeleteBugVisitor::VisitNode( OS << "Conversion from derived to base happened here"; PathDiagnosticLocation Pos(S, BRC.getSourceManager(), N->getLocationContext()); - return std::make_shared<PathDiagnosticEventPiece>(Pos, OS.str(), true, - nullptr); + return std::make_shared<PathDiagnosticEventPiece>(Pos, OS.str(), true); } void ento::registerDeleteWithNonVirtualDtorChecker(CheckerManager &mgr) { diff --git a/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp b/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp index 2c264833f2a9f..e3de0b4f4a7ff 100644 --- a/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp @@ -179,7 +179,7 @@ void DereferenceChecker::reportBug(ProgramStateRef State, const Stmt *S, break; } - auto report = llvm::make_unique<BugReport>( + auto report = std::make_unique<PathSensitiveBugReport>( *BT_null, buf.empty() ? BT_null->getDescription() : StringRef(buf), N); bugreporter::trackExpressionValue(N, bugreporter::getDerefExpr(S), *report); @@ -200,8 +200,8 @@ void DereferenceChecker::checkLocation(SVal l, bool isLoad, const Stmt* S, BT_undef.reset( new BuiltinBug(this, "Dereference of undefined pointer value")); - auto report = - llvm::make_unique<BugReport>(*BT_undef, BT_undef->getDescription(), N); + auto report = std::make_unique<PathSensitiveBugReport>( + *BT_undef, BT_undef->getDescription(), N); bugreporter::trackExpressionValue(N, bugreporter::getDerefExpr(S), *report); C.emitReport(std::move(report)); } diff --git a/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp b/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp index 33e8fcd8af7bb..8798bde88dcd6 100644 --- a/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp @@ -47,7 +47,7 @@ void DivZeroChecker::reportBug( if (!BT) BT.reset(new BuiltinBug(this, "Division by zero")); - auto R = llvm::make_unique<BugReport>(*BT, Msg, N); + auto R = std::make_unique<PathSensitiveBugReport>(*BT, Msg, N); R->addVisitor(std::move(Visitor)); bugreporter::trackExpressionValue(N, getDenomExpr(N), *R); C.emitReport(std::move(R)); @@ -88,7 +88,7 @@ void DivZeroChecker::checkPreStmt(const BinaryOperator *B, bool TaintedD = isTainted(C.getState(), *DV); if ((stateNotZero && stateZero && TaintedD)) { reportBug("Division by a tainted value, possibly zero", stateZero, C, - llvm::make_unique<taint::TaintBugVisitor>(*DV)); + std::make_unique<taint::TaintBugVisitor>(*DV)); return; } diff --git a/lib/StaticAnalyzer/Checkers/DynamicTypeChecker.cpp b/lib/StaticAnalyzer/Checkers/DynamicTypeChecker.cpp index 4d979dc9f2400..8cc38f9735f33 100644 --- a/lib/StaticAnalyzer/Checkers/DynamicTypeChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/DynamicTypeChecker.cpp @@ -21,7 +21,7 @@ #include "clang/StaticAnalyzer/Core/Checker.h" #include "clang/StaticAnalyzer/Core/CheckerManager.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" -#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicTypeMap.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicType.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h" @@ -47,9 +47,9 @@ class DynamicTypeChecker : public Checker<check::PostStmt<ImplicitCastExpr>> { ID.AddPointer(Reg); } - std::shared_ptr<PathDiagnosticPiece> VisitNode(const ExplodedNode *N, - BugReporterContext &BRC, - BugReport &BR) override; + PathDiagnosticPieceRef VisitNode(const ExplodedNode *N, + BugReporterContext &BRC, + PathSensitiveBugReport &BR) override; private: // The tracked region. @@ -80,18 +80,16 @@ void DynamicTypeChecker::reportTypeError(QualType DynamicType, QualType::print(StaticType.getTypePtr(), Qualifiers(), OS, C.getLangOpts(), llvm::Twine()); OS << "'"; - std::unique_ptr<BugReport> R( - new BugReport(*BT, OS.str(), C.generateNonFatalErrorNode())); + auto R = std::make_unique<PathSensitiveBugReport>( + *BT, OS.str(), C.generateNonFatalErrorNode()); R->markInteresting(Reg); - R->addVisitor(llvm::make_unique<DynamicTypeBugVisitor>(Reg)); + R->addVisitor(std::make_unique<DynamicTypeBugVisitor>(Reg)); R->addRange(ReportedNode->getSourceRange()); C.emitReport(std::move(R)); } -std::shared_ptr<PathDiagnosticPiece> -DynamicTypeChecker::DynamicTypeBugVisitor::VisitNode(const ExplodedNode *N, - BugReporterContext &BRC, - BugReport &) { +PathDiagnosticPieceRef DynamicTypeChecker::DynamicTypeBugVisitor::VisitNode( + const ExplodedNode *N, BugReporterContext &BRC, PathSensitiveBugReport &) { ProgramStateRef State = N->getState(); ProgramStateRef StatePrev = N->getFirstPred()->getState(); @@ -105,7 +103,7 @@ DynamicTypeChecker::DynamicTypeBugVisitor::VisitNode(const ExplodedNode *N, return nullptr; // Retrieve the associated statement. - const Stmt *S = PathDiagnosticLocation::getStmt(N); + const Stmt *S = N->getStmtForDiagnostics(); if (!S) return nullptr; @@ -141,8 +139,7 @@ DynamicTypeChecker::DynamicTypeBugVisitor::VisitNode(const ExplodedNode *N, // Generate the extra diagnostic. PathDiagnosticLocation Pos(S, BRC.getSourceManager(), N->getLocationContext()); - return std::make_shared<PathDiagnosticEventPiece>(Pos, OS.str(), true, - nullptr); + return std::make_shared<PathDiagnosticEventPiece>(Pos, OS.str(), true); } static bool hasDefinition(const ObjCObjectPointerType *ObjPtr) { diff --git a/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp b/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp index 3cfe4dc82a100..cce3449b8873f 100644 --- a/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp +++ b/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp @@ -20,16 +20,16 @@ // //===----------------------------------------------------------------------===// -#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" #include "clang/AST/ParentMap.h" #include "clang/AST/RecursiveASTVisitor.h" #include "clang/Basic/Builtins.h" +#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" #include "clang/StaticAnalyzer/Core/Checker.h" #include "clang/StaticAnalyzer/Core/CheckerManager.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" -#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicTypeMap.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicType.h" #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h" using namespace clang; @@ -83,9 +83,9 @@ class DynamicTypePropagation: ID.AddPointer(Sym); } - std::shared_ptr<PathDiagnosticPiece> VisitNode(const ExplodedNode *N, - BugReporterContext &BRC, - BugReport &BR) override; + PathDiagnosticPieceRef VisitNode(const ExplodedNode *N, + BugReporterContext &BRC, + PathSensitiveBugReport &BR) override; private: // The tracked symbol. @@ -113,14 +113,7 @@ public: void DynamicTypePropagation::checkDeadSymbols(SymbolReaper &SR, CheckerContext &C) const { - ProgramStateRef State = C.getState(); - DynamicTypeMapTy TypeMap = State->get<DynamicTypeMap>(); - for (DynamicTypeMapTy::iterator I = TypeMap.begin(), E = TypeMap.end(); - I != E; ++I) { - if (!SR.isLiveRegion(I->first)) { - State = State->remove<DynamicTypeMap>(I->first); - } - } + ProgramStateRef State = removeDeadTypes(C.getState(), SR); MostSpecializedTypeArgsMapTy TyArgMap = State->get<MostSpecializedTypeArgsMap>(); @@ -401,11 +394,11 @@ static const ObjCObjectPointerType *getMostInformativeDerivedClassImpl( } const auto *SuperOfTo = - To->getObjectType()->getSuperClassType()->getAs<ObjCObjectType>(); + To->getObjectType()->getSuperClassType()->castAs<ObjCObjectType>(); assert(SuperOfTo); QualType SuperPtrOfToQual = C.getObjCObjectPointerType(QualType(SuperOfTo, 0)); - const auto *SuperPtrOfTo = SuperPtrOfToQual->getAs<ObjCObjectPointerType>(); + const auto *SuperPtrOfTo = SuperPtrOfToQual->castAs<ObjCObjectPointerType>(); if (To->isUnspecialized()) return getMostInformativeDerivedClassImpl(From, SuperPtrOfTo, SuperPtrOfTo, C); @@ -834,16 +827,15 @@ void DynamicTypePropagation::checkPostObjCMessage(const ObjCMethodCall &M, if (MessageExpr->getReceiverKind() == ObjCMessageExpr::Class && Sel.getAsString() == "class") { QualType ReceiverType = MessageExpr->getClassReceiver(); - const auto *ReceiverClassType = ReceiverType->getAs<ObjCObjectType>(); + const auto *ReceiverClassType = ReceiverType->castAs<ObjCObjectType>(); + if (!ReceiverClassType->isSpecialized()) + return; + QualType ReceiverClassPointerType = C.getASTContext().getObjCObjectPointerType( QualType(ReceiverClassType, 0)); - - if (!ReceiverClassType->isSpecialized()) - return; const auto *InferredType = - ReceiverClassPointerType->getAs<ObjCObjectPointerType>(); - assert(InferredType); + ReceiverClassPointerType->castAs<ObjCObjectPointerType>(); State = State->set<MostSpecializedTypeArgsMap>(RetSym, InferredType); C.addTransition(State); @@ -882,7 +874,7 @@ void DynamicTypePropagation::checkPostObjCMessage(const ObjCMethodCall &M, // When there is an entry available for the return symbol in DynamicTypeMap, // the call was inlined, and the information in the DynamicTypeMap is should // be precise. - if (RetRegion && !State->get<DynamicTypeMap>(RetRegion)) { + if (RetRegion && !getRawDynamicTypeInfo(State, RetRegion)) { // TODO: we have duplicated information in DynamicTypeMap and // MostSpecializedTypeArgsMap. We should only store anything in the later if // the stored data differs from the one stored in the former. @@ -919,19 +911,18 @@ void DynamicTypePropagation::reportGenericsBug( OS << "' to incompatible type '"; QualType::print(To, Qualifiers(), OS, C.getLangOpts(), llvm::Twine()); OS << "'"; - std::unique_ptr<BugReport> R( - new BugReport(*ObjCGenericsBugType, OS.str(), N)); + auto R = std::make_unique<PathSensitiveBugReport>(*ObjCGenericsBugType, + OS.str(), N); R->markInteresting(Sym); - R->addVisitor(llvm::make_unique<GenericsBugVisitor>(Sym)); + R->addVisitor(std::make_unique<GenericsBugVisitor>(Sym)); if (ReportedNode) R->addRange(ReportedNode->getSourceRange()); C.emitReport(std::move(R)); } -std::shared_ptr<PathDiagnosticPiece> -DynamicTypePropagation::GenericsBugVisitor::VisitNode(const ExplodedNode *N, - BugReporterContext &BRC, - BugReport &BR) { +PathDiagnosticPieceRef DynamicTypePropagation::GenericsBugVisitor::VisitNode( + const ExplodedNode *N, BugReporterContext &BRC, + PathSensitiveBugReport &BR) { ProgramStateRef state = N->getState(); ProgramStateRef statePrev = N->getFirstPred()->getState(); @@ -946,7 +937,7 @@ DynamicTypePropagation::GenericsBugVisitor::VisitNode(const ExplodedNode *N, return nullptr; // Retrieve the associated statement. - const Stmt *S = PathDiagnosticLocation::getStmt(N); + const Stmt *S = N->getStmtForDiagnostics(); if (!S) return nullptr; @@ -981,8 +972,7 @@ DynamicTypePropagation::GenericsBugVisitor::VisitNode(const ExplodedNode *N, // Generate the extra diagnostic. PathDiagnosticLocation Pos(S, BRC.getSourceManager(), N->getLocationContext()); - return std::make_shared<PathDiagnosticEventPiece>(Pos, OS.str(), true, - nullptr); + return std::make_shared<PathDiagnosticEventPiece>(Pos, OS.str(), true); } /// Register checkers. diff --git a/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp b/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp index 736d80ef9ec7e..481a5685a71f6 100644 --- a/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp @@ -83,7 +83,7 @@ void EnumCastOutOfRangeChecker::reportWarning(CheckerContext &C) const { new BuiltinBug(this, "Enum cast out of range", "The value provided to the cast expression is not in " "the valid range of values for the enum")); - C.emitReport(llvm::make_unique<BugReport>( + C.emitReport(std::make_unique<PathSensitiveBugReport>( *EnumValueCastOutOfRange, EnumValueCastOutOfRange->getDescription(), N)); } @@ -91,6 +91,22 @@ void EnumCastOutOfRangeChecker::reportWarning(CheckerContext &C) const { void EnumCastOutOfRangeChecker::checkPreStmt(const CastExpr *CE, CheckerContext &C) const { + + // Only perform enum range check on casts where such checks are valid. For + // all other cast kinds (where enum range checks are unnecessary or invalid), + // just return immediately. TODO: The set of casts whitelisted for enum + // range checking may be incomplete. Better to add a missing cast kind to + // enable a missing check than to generate false negatives and have to remove + // those later. + switch (CE->getCastKind()) { + case CK_IntegralCast: + break; + + default: + return; + break; + } + // Get the value of the expression to cast. const llvm::Optional<DefinedOrUnknownSVal> ValueToCast = C.getSVal(CE->getSubExpr()).getAs<DefinedOrUnknownSVal>(); diff --git a/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp b/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp index f23f784016d88..17c813962a234 100644 --- a/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp @@ -148,7 +148,7 @@ ExplodedNode *ExprInspectionChecker::reportBug(llvm::StringRef Msg, if (!BT) BT.reset(new BugType(this, "Checking analyzer assumptions", "debug")); - BR.emitReport(llvm::make_unique<BugReport>(*BT, Msg, N)); + BR.emitReport(std::make_unique<PathSensitiveBugReport>(*BT, Msg, N)); return N; } @@ -305,7 +305,7 @@ void ExprInspectionChecker::analyzerHashDump(const CallExpr *CE, const SourceManager &SM = C.getSourceManager(); FullSourceLoc FL(CE->getArg(0)->getBeginLoc(), SM); std::string HashContent = - GetIssueString(SM, FL, getCheckName().getName(), "Category", + GetIssueString(SM, FL, getCheckerName().getName(), "Category", C.getLocationContext()->getDecl(), Opts); reportBug(HashContent, C); diff --git a/lib/StaticAnalyzer/Checkers/FixedAddressChecker.cpp b/lib/StaticAnalyzer/Checkers/FixedAddressChecker.cpp index 94542be7dd7ca..b315a84522859 100644 --- a/lib/StaticAnalyzer/Checkers/FixedAddressChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/FixedAddressChecker.cpp @@ -55,7 +55,8 @@ void FixedAddressChecker::checkPreStmt(const BinaryOperator *B, "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); + auto R = + std::make_unique<PathSensitiveBugReport>(*BT, BT->getDescription(), N); R->addRange(B->getRHS()->getSourceRange()); C.emitReport(std::move(R)); } diff --git a/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp b/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp index d3ab980332365..d442b26b39594 100644 --- a/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp @@ -15,16 +15,18 @@ //===----------------------------------------------------------------------===// #include "Taint.h" -#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" +#include "Yaml.h" #include "clang/AST/Attr.h" #include "clang/Basic/Builtins.h" +#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" #include "clang/StaticAnalyzer/Core/Checker.h" #include "clang/StaticAnalyzer/Core/CheckerManager.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h" -#include <climits> -#include <initializer_list> +#include "llvm/ADT/StringMap.h" +#include "llvm/Support/YAMLTraits.h" +#include <limits> #include <utility> using namespace clang; @@ -44,14 +46,51 @@ public: void checkPreStmt(const CallExpr *CE, CheckerContext &C) const; - void printState(raw_ostream &Out, ProgramStateRef State, - const char *NL, const char *Sep) const override; + void printState(raw_ostream &Out, ProgramStateRef State, const char *NL, + const char *Sep) const override; -private: - static const unsigned InvalidArgIndex = UINT_MAX; + using ArgVector = SmallVector<unsigned, 2>; + using SignedArgVector = SmallVector<int, 2>; + + enum class VariadicType { None, Src, Dst }; + + /// Used to parse the configuration file. + struct TaintConfiguration { + using NameArgsPair = std::pair<std::string, ArgVector>; + + struct Propagation { + std::string Name; + ArgVector SrcArgs; + SignedArgVector DstArgs; + VariadicType VarType; + unsigned VarIndex; + }; + + std::vector<Propagation> Propagations; + std::vector<NameArgsPair> Filters; + std::vector<NameArgsPair> Sinks; + + TaintConfiguration() = default; + TaintConfiguration(const TaintConfiguration &) = default; + TaintConfiguration(TaintConfiguration &&) = default; + TaintConfiguration &operator=(const TaintConfiguration &) = default; + TaintConfiguration &operator=(TaintConfiguration &&) = default; + }; + + /// Convert SignedArgVector to ArgVector. + ArgVector convertToArgVector(CheckerManager &Mgr, const std::string &Option, + SignedArgVector Args); + + /// Parse the config. + void parseConfiguration(CheckerManager &Mgr, const std::string &Option, + TaintConfiguration &&Config); + + static const unsigned InvalidArgIndex{std::numeric_limits<unsigned>::max()}; /// Denotes the return vale. - static const unsigned ReturnValueIndex = UINT_MAX - 1; + static const unsigned ReturnValueIndex{std::numeric_limits<unsigned>::max() - + 1}; +private: mutable std::unique_ptr<BugType> BT; void initBugType() const { if (!BT) @@ -76,28 +115,43 @@ private: static Optional<SVal> getPointedToSVal(CheckerContext &C, const Expr *Arg); /// Check for CWE-134: Uncontrolled Format String. - static const char MsgUncontrolledFormatString[]; + static constexpr llvm::StringLiteral MsgUncontrolledFormatString = + "Untrusted data is used as a format string " + "(CWE-134: Uncontrolled Format String)"; bool checkUncontrolledFormatString(const CallExpr *CE, CheckerContext &C) const; /// Check for: /// CERT/STR02-C. "Sanitize data passed to complex subsystems" /// CWE-78, "Failure to Sanitize Data into an OS Command" - static const char MsgSanitizeSystemArgs[]; + static constexpr llvm::StringLiteral MsgSanitizeSystemArgs = + "Untrusted data is passed to a system call " + "(CERT/STR02-C. Sanitize data passed to complex subsystems)"; bool checkSystemCall(const CallExpr *CE, StringRef Name, CheckerContext &C) const; /// Check if tainted data is used as a buffer size ins strn.. functions, /// and allocators. - static const char MsgTaintedBufferSize[]; + static constexpr llvm::StringLiteral MsgTaintedBufferSize = + "Untrusted data is used to specify the buffer size " + "(CERT/STR31-C. Guarantee that storage for strings has sufficient space " + "for character data and the null terminator)"; bool checkTaintedBufferSize(const CallExpr *CE, const FunctionDecl *FDecl, CheckerContext &C) const; + /// Check if tainted data is used as a custom sink's parameter. + static constexpr llvm::StringLiteral MsgCustomSink = + "Untrusted data is passed to a user-defined sink"; + bool checkCustomSinks(const CallExpr *CE, StringRef Name, + CheckerContext &C) const; + /// Generate a report if the expression is tainted or points to tainted data. - bool generateReportIfTainted(const Expr *E, const char Msg[], + bool generateReportIfTainted(const Expr *E, StringRef Msg, CheckerContext &C) const; - using ArgVector = SmallVector<unsigned, 2>; + struct TaintPropagationRule; + using NameRuleMap = llvm::StringMap<TaintPropagationRule>; + using NameArgMap = llvm::StringMap<ArgVector>; /// A struct used to specify taint propagation rules for a function. /// @@ -109,8 +163,6 @@ private: /// ReturnValueIndex is added to the dst list, the return value will be /// tainted. struct TaintPropagationRule { - enum class VariadicType { None, Src, Dst }; - using PropagationFuncType = bool (*)(bool IsTainted, const CallExpr *, CheckerContext &C); @@ -131,8 +183,7 @@ private: : VariadicIndex(InvalidArgIndex), VarType(VariadicType::None), PropagationFunc(nullptr) {} - TaintPropagationRule(std::initializer_list<unsigned> &&Src, - std::initializer_list<unsigned> &&Dst, + TaintPropagationRule(ArgVector &&Src, ArgVector &&Dst, VariadicType Var = VariadicType::None, unsigned VarIndex = InvalidArgIndex, PropagationFuncType Func = nullptr) @@ -141,7 +192,8 @@ private: /// Get the propagation rule for a given function. static TaintPropagationRule - getTaintPropagationRule(const FunctionDecl *FDecl, StringRef Name, + getTaintPropagationRule(const NameRuleMap &CustomPropagations, + const FunctionDecl *FDecl, StringRef Name, CheckerContext &C); void addSrcArg(unsigned A) { SrcArgs.push_back(A); } @@ -176,25 +228,71 @@ private: static bool postSocket(bool IsTainted, const CallExpr *CE, CheckerContext &C); }; + + /// Defines a map between the propagation function's name and + /// TaintPropagationRule. + NameRuleMap CustomPropagations; + + /// Defines a map between the filter function's name and filtering args. + NameArgMap CustomFilters; + + /// Defines a map between the sink function's name and sinking args. + NameArgMap CustomSinks; }; const unsigned GenericTaintChecker::ReturnValueIndex; const unsigned GenericTaintChecker::InvalidArgIndex; -const char GenericTaintChecker::MsgUncontrolledFormatString[] = - "Untrusted data is used as a format string " - "(CWE-134: Uncontrolled Format String)"; +// FIXME: these lines can be removed in C++17 +constexpr llvm::StringLiteral GenericTaintChecker::MsgUncontrolledFormatString; +constexpr llvm::StringLiteral GenericTaintChecker::MsgSanitizeSystemArgs; +constexpr llvm::StringLiteral GenericTaintChecker::MsgTaintedBufferSize; +constexpr llvm::StringLiteral GenericTaintChecker::MsgCustomSink; +} // end of anonymous namespace -const char GenericTaintChecker::MsgSanitizeSystemArgs[] = - "Untrusted data is passed to a system call " - "(CERT/STR02-C. Sanitize data passed to complex subsystems)"; +using TaintConfig = GenericTaintChecker::TaintConfiguration; -const char GenericTaintChecker::MsgTaintedBufferSize[] = - "Untrusted data is used to specify the buffer size " - "(CERT/STR31-C. Guarantee that storage for strings has sufficient space " - "for character data and the null terminator)"; +LLVM_YAML_IS_SEQUENCE_VECTOR(TaintConfig::Propagation) +LLVM_YAML_IS_SEQUENCE_VECTOR(TaintConfig::NameArgsPair) -} // end of anonymous namespace +namespace llvm { +namespace yaml { +template <> struct MappingTraits<TaintConfig> { + static void mapping(IO &IO, TaintConfig &Config) { + IO.mapOptional("Propagations", Config.Propagations); + IO.mapOptional("Filters", Config.Filters); + IO.mapOptional("Sinks", Config.Sinks); + } +}; + +template <> struct MappingTraits<TaintConfig::Propagation> { + static void mapping(IO &IO, TaintConfig::Propagation &Propagation) { + IO.mapRequired("Name", Propagation.Name); + IO.mapOptional("SrcArgs", Propagation.SrcArgs); + IO.mapOptional("DstArgs", Propagation.DstArgs); + IO.mapOptional("VariadicType", Propagation.VarType, + GenericTaintChecker::VariadicType::None); + IO.mapOptional("VariadicIndex", Propagation.VarIndex, + GenericTaintChecker::InvalidArgIndex); + } +}; + +template <> struct ScalarEnumerationTraits<GenericTaintChecker::VariadicType> { + static void enumeration(IO &IO, GenericTaintChecker::VariadicType &Value) { + IO.enumCase(Value, "None", GenericTaintChecker::VariadicType::None); + IO.enumCase(Value, "Src", GenericTaintChecker::VariadicType::Src); + IO.enumCase(Value, "Dst", GenericTaintChecker::VariadicType::Dst); + } +}; + +template <> struct MappingTraits<TaintConfig::NameArgsPair> { + static void mapping(IO &IO, TaintConfig::NameArgsPair &NameArg) { + IO.mapRequired("Name", NameArg.first); + IO.mapRequired("Args", NameArg.second); + } +}; +} // namespace yaml +} // namespace llvm /// A set which is used to pass information from call pre-visit instruction /// to the call post-visit. The values are unsigned integers, which are either @@ -202,9 +300,46 @@ const char GenericTaintChecker::MsgTaintedBufferSize[] = /// points to data, which should be tainted on return. REGISTER_SET_WITH_PROGRAMSTATE(TaintArgsOnPostVisit, unsigned) +GenericTaintChecker::ArgVector GenericTaintChecker::convertToArgVector( + CheckerManager &Mgr, const std::string &Option, SignedArgVector Args) { + ArgVector Result; + for (int Arg : Args) { + if (Arg == -1) + Result.push_back(ReturnValueIndex); + else if (Arg < -1) { + Result.push_back(InvalidArgIndex); + Mgr.reportInvalidCheckerOptionValue( + this, Option, + "an argument number for propagation rules greater or equal to -1"); + } else + Result.push_back(static_cast<unsigned>(Arg)); + } + return Result; +} + +void GenericTaintChecker::parseConfiguration(CheckerManager &Mgr, + const std::string &Option, + TaintConfiguration &&Config) { + for (auto &P : Config.Propagations) { + GenericTaintChecker::CustomPropagations.try_emplace( + P.Name, std::move(P.SrcArgs), + convertToArgVector(Mgr, Option, P.DstArgs), P.VarType, P.VarIndex); + } + + for (auto &F : Config.Filters) { + GenericTaintChecker::CustomFilters.try_emplace(F.first, + std::move(F.second)); + } + + for (auto &S : Config.Sinks) { + GenericTaintChecker::CustomSinks.try_emplace(S.first, std::move(S.second)); + } +} + GenericTaintChecker::TaintPropagationRule GenericTaintChecker::TaintPropagationRule::getTaintPropagationRule( - const FunctionDecl *FDecl, StringRef Name, CheckerContext &C) { + const NameRuleMap &CustomPropagations, const FunctionDecl *FDecl, + StringRef Name, CheckerContext &C) { // TODO: Currently, we might lose precision here: we always mark a return // value as tainted even if it's just a pointer, pointing to tainted data. @@ -218,7 +353,8 @@ GenericTaintChecker::TaintPropagationRule::getTaintPropagationRule( .Case("freopen", TaintPropagationRule({}, {ReturnValueIndex})) .Case("getch", TaintPropagationRule({}, {ReturnValueIndex})) .Case("getchar", TaintPropagationRule({}, {ReturnValueIndex})) - .Case("getchar_unlocked", TaintPropagationRule({}, {ReturnValueIndex})) + .Case("getchar_unlocked", + TaintPropagationRule({}, {ReturnValueIndex})) .Case("getenv", TaintPropagationRule({}, {ReturnValueIndex})) .Case("gets", TaintPropagationRule({}, {0, ReturnValueIndex})) .Case("scanf", TaintPropagationRule({}, {}, VariadicType::Dst, 1)) @@ -297,6 +433,10 @@ GenericTaintChecker::TaintPropagationRule::getTaintPropagationRule( // or smart memory copy: // - memccpy - copying until hitting a special character. + auto It = CustomPropagations.find(Name); + if (It != CustomPropagations.end()) + return It->getValue(); + return TaintPropagationRule(); } @@ -336,8 +476,8 @@ void GenericTaintChecker::addSourcesPre(const CallExpr *CE, return; // First, try generating a propagation rule for this function. - TaintPropagationRule Rule = - TaintPropagationRule::getTaintPropagationRule(FDecl, Name, C); + TaintPropagationRule Rule = TaintPropagationRule::getTaintPropagationRule( + this->CustomPropagations, FDecl, Name, C); if (!Rule.isNull()) { State = Rule.process(CE, C); if (!State) @@ -409,6 +549,9 @@ bool GenericTaintChecker::checkPre(const CallExpr *CE, if (checkTaintedBufferSize(CE, FDecl, C)) return true; + if (checkCustomSinks(CE, Name, C)) + return true; + return false; } @@ -446,7 +589,8 @@ GenericTaintChecker::TaintPropagationRule::process(const CallExpr *CE, bool IsTainted = true; for (unsigned ArgNum : SrcArgs) { if (ArgNum >= CE->getNumArgs()) - return State; + continue; + if ((IsTainted = isTaintedOrPointsToTainted(CE->getArg(ArgNum), State, C))) break; } @@ -454,7 +598,7 @@ GenericTaintChecker::TaintPropagationRule::process(const CallExpr *CE, // Check for taint in variadic arguments. if (!IsTainted && VariadicType::Src == VarType) { // Check if any of the arguments is tainted - for (unsigned int i = VariadicIndex; i < CE->getNumArgs(); ++i) { + for (unsigned i = VariadicIndex; i < CE->getNumArgs(); ++i) { if ((IsTainted = isTaintedOrPointsToTainted(CE->getArg(i), State, C))) break; } @@ -474,8 +618,10 @@ GenericTaintChecker::TaintPropagationRule::process(const CallExpr *CE, continue; } + if (ArgNum >= CE->getNumArgs()) + continue; + // Mark the given argument. - assert(ArgNum < CE->getNumArgs()); State = State->add<TaintArgsOnPostVisit>(ArgNum); } @@ -485,7 +631,7 @@ GenericTaintChecker::TaintPropagationRule::process(const CallExpr *CE, // If they are not pointing to const data, mark data as tainted. // TODO: So far we are just going one level down; ideally we'd need to // recurse here. - for (unsigned int i = VariadicIndex; i < CE->getNumArgs(); ++i) { + for (unsigned i = VariadicIndex; i < CE->getNumArgs(); ++i) { const Expr *Arg = CE->getArg(i); // Process pointer argument. const Type *ArgTy = Arg->getType().getTypePtr(); @@ -550,7 +696,7 @@ bool GenericTaintChecker::isStdin(const Expr *E, CheckerContext &C) { static bool getPrintfFormatArgumentNum(const CallExpr *CE, const CheckerContext &C, - unsigned int &ArgNum) { + unsigned &ArgNum) { // Find if the function contains a format string argument. // Handles: fprintf, printf, sprintf, snprintf, vfprintf, vprintf, vsprintf, // vsnprintf, syslog, custom annotated functions. @@ -572,8 +718,7 @@ static bool getPrintfFormatArgumentNum(const CallExpr *CE, return false; } -bool GenericTaintChecker::generateReportIfTainted(const Expr *E, - const char Msg[], +bool GenericTaintChecker::generateReportIfTainted(const Expr *E, StringRef Msg, CheckerContext &C) const { assert(E); @@ -591,9 +736,9 @@ bool GenericTaintChecker::generateReportIfTainted(const Expr *E, // Generate diagnostic. if (ExplodedNode *N = C.generateNonFatalErrorNode()) { initBugType(); - auto report = llvm::make_unique<BugReport>(*BT, Msg, N); + auto report = std::make_unique<PathSensitiveBugReport>(*BT, Msg, N); report->addRange(E->getSourceRange()); - report->addVisitor(llvm::make_unique<TaintBugVisitor>(TaintedSVal)); + report->addVisitor(std::make_unique<TaintBugVisitor>(TaintedSVal)); C.emitReport(std::move(report)); return true; } @@ -603,7 +748,7 @@ bool GenericTaintChecker::generateReportIfTainted(const Expr *E, bool GenericTaintChecker::checkUncontrolledFormatString( const CallExpr *CE, CheckerContext &C) const { // Check if the function contains a format string argument. - unsigned int ArgNum = 0; + unsigned ArgNum = 0; if (!getPrintfFormatArgumentNum(CE, C, ArgNum)) return false; @@ -629,9 +774,9 @@ bool GenericTaintChecker::checkSystemCall(const CallExpr *CE, StringRef Name, .Case("execvP", 0) .Case("execve", 0) .Case("dlopen", 0) - .Default(UINT_MAX); + .Default(InvalidArgIndex); - if (ArgNum == UINT_MAX || CE->getNumArgs() < (ArgNum + 1)) + if (ArgNum == InvalidArgIndex || CE->getNumArgs() < (ArgNum + 1)) return false; return generateReportIfTainted(CE->getArg(ArgNum), MsgSanitizeSystemArgs, C); @@ -676,8 +821,33 @@ bool GenericTaintChecker::checkTaintedBufferSize(const CallExpr *CE, generateReportIfTainted(CE->getArg(ArgNum), MsgTaintedBufferSize, C); } -void ento::registerGenericTaintChecker(CheckerManager &mgr) { - mgr.registerChecker<GenericTaintChecker>(); +bool GenericTaintChecker::checkCustomSinks(const CallExpr *CE, StringRef Name, + CheckerContext &C) const { + auto It = CustomSinks.find(Name); + if (It == CustomSinks.end()) + return false; + + const GenericTaintChecker::ArgVector &Args = It->getValue(); + for (unsigned ArgNum : Args) { + if (ArgNum >= CE->getNumArgs()) + continue; + + if (generateReportIfTainted(CE->getArg(ArgNum), MsgCustomSink, C)) + return true; + } + + return false; +} + +void ento::registerGenericTaintChecker(CheckerManager &Mgr) { + auto *Checker = Mgr.registerChecker<GenericTaintChecker>(); + std::string Option{"Config"}; + StringRef ConfigFile = + Mgr.getAnalyzerOptions().getCheckerStringOption(Checker, Option); + llvm::Optional<TaintConfig> Config = + getConfiguration<TaintConfig>(Mgr, Checker, Option, ConfigFile); + if (Config) + Checker->parseConfiguration(Mgr, Option, std::move(Config.getValue())); } bool ento::shouldRegisterGenericTaintChecker(const LangOptions &LO) { diff --git a/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp b/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp index e3270f1f7be27..b0d101c88517f 100644 --- a/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp @@ -54,9 +54,9 @@ public: ID.AddPointer(getTag()); } - virtual std::shared_ptr<PathDiagnosticPiece> VisitNode(const ExplodedNode *N, - BugReporterContext &BRC, - BugReport &BR) override; + virtual PathDiagnosticPieceRef + VisitNode(const ExplodedNode *N, BugReporterContext &BRC, + PathSensitiveBugReport &BR) override; // FIXME: Scan the map once in the visitor's constructor and do a direct // lookup by region. @@ -261,7 +261,7 @@ namespace ento { namespace allocation_state { std::unique_ptr<BugReporterVisitor> getInnerPointerBRVisitor(SymbolRef Sym) { - return llvm::make_unique<InnerPointerChecker::InnerPointerBRVisitor>(Sym); + return std::make_unique<InnerPointerChecker::InnerPointerBRVisitor>(Sym); } const MemRegion *getContainerObjRegion(ProgramStateRef State, SymbolRef Sym) { @@ -278,15 +278,13 @@ const MemRegion *getContainerObjRegion(ProgramStateRef State, SymbolRef Sym) { } // end namespace ento } // end namespace clang -std::shared_ptr<PathDiagnosticPiece> -InnerPointerChecker::InnerPointerBRVisitor::VisitNode(const ExplodedNode *N, - BugReporterContext &BRC, - BugReport &) { +PathDiagnosticPieceRef InnerPointerChecker::InnerPointerBRVisitor::VisitNode( + const ExplodedNode *N, BugReporterContext &BRC, PathSensitiveBugReport &) { if (!isSymbolTracked(N->getState(), PtrToBuf) || isSymbolTracked(N->getFirstPred()->getState(), PtrToBuf)) return nullptr; - const Stmt *S = PathDiagnosticLocation::getStmt(N); + const Stmt *S = N->getStmtForDiagnostics(); if (!S) return nullptr; @@ -301,8 +299,7 @@ InnerPointerChecker::InnerPointerBRVisitor::VisitNode(const ExplodedNode *N, << "' obtained here"; PathDiagnosticLocation Pos(S, BRC.getSourceManager(), N->getLocationContext()); - return std::make_shared<PathDiagnosticEventPiece>(Pos, OS.str(), true, - nullptr); + return std::make_shared<PathDiagnosticEventPiece>(Pos, OS.str(), true); } void ento::registerInnerPointerChecker(CheckerManager &Mgr) { diff --git a/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp b/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp index 6f1060b5f26d9..97ace68569ef2 100644 --- a/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp @@ -71,7 +71,7 @@ #include "clang/StaticAnalyzer/Core/Checker.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" -#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicTypeMap.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicType.h" #include <utility> @@ -248,7 +248,7 @@ public: }; DefaultBool ChecksEnabled[CK_NumCheckKinds]; - CheckName CheckNames[CK_NumCheckKinds]; + CheckerNameRef CheckNames[CK_NumCheckKinds]; void checkPreCall(const CallEvent &Call, CheckerContext &C) const; void checkPostCall(const CallEvent &Call, CheckerContext &C) const; @@ -356,14 +356,12 @@ bool isZero(ProgramStateRef State, const NonLoc &Val); IteratorChecker::IteratorChecker() { OutOfRangeBugType.reset( - new BugType(this, "Iterator out of range", "Misuse of STL APIs", - /*SuppressOnSink=*/true)); + new BugType(this, "Iterator out of range", "Misuse of STL APIs")); MismatchedBugType.reset( new BugType(this, "Iterator(s) mismatched", "Misuse of STL APIs", /*SuppressOnSink=*/true)); InvalidatedBugType.reset( - new BugType(this, "Iterator invalidated", "Misuse of STL APIs", - /*SuppressOnSink=*/true)); + new BugType(this, "Iterator invalidated", "Misuse of STL APIs")); } void IteratorChecker::checkPreCall(const CallEvent &Call, @@ -406,13 +404,15 @@ void IteratorChecker::checkPreCall(const CallEvent &Call, } else if (isRandomIncrOrDecrOperator(Func->getOverloadedOperator())) { if (const auto *InstCall = dyn_cast<CXXInstanceCall>(&Call)) { // Check for out-of-range incrementions and decrementions - if (Call.getNumArgs() >= 1) { + if (Call.getNumArgs() >= 1 && + Call.getArgExpr(0)->getType()->isIntegralOrEnumerationType()) { verifyRandomIncrOrDecr(C, Func->getOverloadedOperator(), InstCall->getCXXThisVal(), Call.getArgSVal(0)); } } else { - if (Call.getNumArgs() >= 2) { + if (Call.getNumArgs() >= 2 && + Call.getArgExpr(1)->getType()->isIntegralOrEnumerationType()) { verifyRandomIncrOrDecr(C, Func->getOverloadedOperator(), Call.getArgSVal(0), Call.getArgSVal(1)); } @@ -565,7 +565,8 @@ void IteratorChecker::checkPostCall(const CallEvent &Call, if (Func->isOverloadedOperator()) { const auto Op = Func->getOverloadedOperator(); if (isAssignmentOperator(Op)) { - const auto *InstCall = dyn_cast<CXXInstanceCall>(&Call); + // Overloaded 'operator=' must be a non-static member function. + const auto *InstCall = cast<CXXInstanceCall>(&Call); if (cast<CXXMethodDecl>(Func)->isMoveAssignmentOperator()) { handleAssign(C, InstCall->getCXXThisVal(), Call.getOriginExpr(), Call.getArgSVal(0)); @@ -590,14 +591,16 @@ void IteratorChecker::checkPostCall(const CallEvent &Call, return; } else if (isRandomIncrOrDecrOperator(Func->getOverloadedOperator())) { if (const auto *InstCall = dyn_cast<CXXInstanceCall>(&Call)) { - if (Call.getNumArgs() >= 1) { + if (Call.getNumArgs() >= 1 && + Call.getArgExpr(0)->getType()->isIntegralOrEnumerationType()) { handleRandomIncrOrDecr(C, Func->getOverloadedOperator(), Call.getReturnValue(), InstCall->getCXXThisVal(), Call.getArgSVal(0)); return; } } else { - if (Call.getNumArgs() >= 2) { + if (Call.getNumArgs() >= 2 && + Call.getArgExpr(1)->getType()->isIntegralOrEnumerationType()) { handleRandomIncrOrDecr(C, Func->getOverloadedOperator(), Call.getReturnValue(), Call.getArgSVal(0), Call.getArgSVal(1)); @@ -923,7 +926,7 @@ void IteratorChecker::verifyDereference(CheckerContext &C, auto State = C.getState(); const auto *Pos = getIteratorPosition(State, Val); if (Pos && isPastTheEnd(State, *Pos)) { - auto *N = C.generateNonFatalErrorNode(State); + auto *N = C.generateErrorNode(State); if (!N) return; reportOutOfRangeBug("Past-the-end iterator dereferenced.", Val, C, N); @@ -935,7 +938,7 @@ void IteratorChecker::verifyAccess(CheckerContext &C, const SVal &Val) const { auto State = C.getState(); const auto *Pos = getIteratorPosition(State, Val); if (Pos && !Pos->isValid()) { - auto *N = C.generateNonFatalErrorNode(State); + auto *N = C.generateErrorNode(State); if (!N) { return; } @@ -1043,14 +1046,14 @@ void IteratorChecker::verifyRandomIncrOrDecr(CheckerContext &C, // The result may be the past-end iterator of the container, but any other // out of range position is undefined behaviour if (isAheadOfRange(State, advancePosition(C, Op, *Pos, Value))) { - auto *N = C.generateNonFatalErrorNode(State); + auto *N = C.generateErrorNode(State); if (!N) return; reportOutOfRangeBug("Iterator decremented ahead of its valid range.", LHS, C, N); } if (isBehindPastTheEnd(State, advancePosition(C, Op, *Pos, Value))) { - auto *N = C.generateNonFatalErrorNode(State); + auto *N = C.generateErrorNode(State); if (!N) return; reportOutOfRangeBug("Iterator incremented behind the past-the-end " @@ -1587,7 +1590,8 @@ IteratorPosition IteratorChecker::advancePosition(CheckerContext &C, void IteratorChecker::reportOutOfRangeBug(const StringRef &Message, const SVal &Val, CheckerContext &C, ExplodedNode *ErrNode) const { - auto R = llvm::make_unique<BugReport>(*OutOfRangeBugType, Message, ErrNode); + auto R = std::make_unique<PathSensitiveBugReport>(*OutOfRangeBugType, Message, + ErrNode); R->markInteresting(Val); C.emitReport(std::move(R)); } @@ -1596,7 +1600,8 @@ void IteratorChecker::reportMismatchedBug(const StringRef &Message, const SVal &Val1, const SVal &Val2, CheckerContext &C, ExplodedNode *ErrNode) const { - auto R = llvm::make_unique<BugReport>(*MismatchedBugType, Message, ErrNode); + auto R = std::make_unique<PathSensitiveBugReport>(*MismatchedBugType, Message, + ErrNode); R->markInteresting(Val1); R->markInteresting(Val2); C.emitReport(std::move(R)); @@ -1606,7 +1611,8 @@ void IteratorChecker::reportMismatchedBug(const StringRef &Message, const SVal &Val, const MemRegion *Reg, CheckerContext &C, ExplodedNode *ErrNode) const { - auto R = llvm::make_unique<BugReport>(*MismatchedBugType, Message, ErrNode); + auto R = std::make_unique<PathSensitiveBugReport>(*MismatchedBugType, Message, + ErrNode); R->markInteresting(Val); R->markInteresting(Reg); C.emitReport(std::move(R)); @@ -1615,7 +1621,8 @@ void IteratorChecker::reportMismatchedBug(const StringRef &Message, void IteratorChecker::reportInvalidatedBug(const StringRef &Message, const SVal &Val, CheckerContext &C, ExplodedNode *ErrNode) const { - auto R = llvm::make_unique<BugReport>(*InvalidatedBugType, Message, ErrNode); + auto R = std::make_unique<PathSensitiveBugReport>(*InvalidatedBugType, + Message, ErrNode); R->markInteresting(Val); C.emitReport(std::move(R)); } @@ -2373,12 +2380,10 @@ bool ento::shouldRegisterIteratorModeling(const LangOptions &LO) { auto *checker = Mgr.getChecker<IteratorChecker>(); \ checker->ChecksEnabled[IteratorChecker::CK_##name] = true; \ checker->CheckNames[IteratorChecker::CK_##name] = \ - Mgr.getCurrentCheckName(); \ + Mgr.getCurrentCheckerName(); \ } \ \ - bool ento::shouldRegister##name(const LangOptions &LO) { \ - return true; \ - } + bool ento::shouldRegister##name(const LangOptions &LO) { return true; } REGISTER_CHECKER(IteratorRangeChecker) REGISTER_CHECKER(MismatchedIteratorChecker) diff --git a/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp b/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp index 2b75f3acc9228..0d64fbd6f62ec 100644 --- a/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp @@ -48,8 +48,8 @@ struct ChecksFilter { /// Check that all ivars are invalidated. DefaultBool check_InstanceVariableInvalidation; - CheckName checkName_MissingInvalidationMethod; - CheckName checkName_InstanceVariableInvalidation; + CheckerNameRef checkName_MissingInvalidationMethod; + CheckerNameRef checkName_InstanceVariableInvalidation; }; class IvarInvalidationCheckerImpl { @@ -199,7 +199,7 @@ class IvarInvalidationCheckerImpl { const ObjCIvarDecl *IvarDecl, const IvarToPropMapTy &IvarToPopertyMap); - void reportNoInvalidationMethod(CheckName CheckName, + void reportNoInvalidationMethod(CheckerNameRef CheckName, const ObjCIvarDecl *FirstIvarDecl, const IvarToPropMapTy &IvarToPopertyMap, const ObjCInterfaceDecl *InterfaceD, @@ -526,7 +526,7 @@ visit(const ObjCImplementationDecl *ImplD) const { } void IvarInvalidationCheckerImpl::reportNoInvalidationMethod( - CheckName CheckName, const ObjCIvarDecl *FirstIvarDecl, + CheckerNameRef CheckName, const ObjCIvarDecl *FirstIvarDecl, const IvarToPropMapTy &IvarToPopertyMap, const ObjCInterfaceDecl *InterfaceD, bool MissingDeclaration) const { SmallString<128> sbuf; @@ -748,12 +748,10 @@ bool ento::shouldRegisterIvarInvalidationModeling(const LangOptions &LO) { IvarInvalidationChecker *checker = \ mgr.getChecker<IvarInvalidationChecker>(); \ checker->Filter.check_##name = true; \ - checker->Filter.checkName_##name = mgr.getCurrentCheckName(); \ + checker->Filter.checkName_##name = mgr.getCurrentCheckerName(); \ } \ \ - bool ento::shouldRegister##name(const LangOptions &LO) { \ - return true; \ - } + bool ento::shouldRegister##name(const LangOptions &LO) { return true; } REGISTER_CHECKER(InstanceVariableInvalidation) REGISTER_CHECKER(MissingInvalidationMethod) diff --git a/lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp b/lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp index 46067ecbca991..a81015b6e524b 100644 --- a/lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp @@ -120,12 +120,12 @@ class NonLocalizedStringBRVisitor final : public BugReporterVisitor { public: NonLocalizedStringBRVisitor(const MemRegion *NonLocalizedString) : NonLocalizedString(NonLocalizedString), Satisfied(false) { - assert(NonLocalizedString); + assert(NonLocalizedString); } - std::shared_ptr<PathDiagnosticPiece> VisitNode(const ExplodedNode *Succ, - BugReporterContext &BRC, - BugReport &BR) override; + PathDiagnosticPieceRef VisitNode(const ExplodedNode *Succ, + BugReporterContext &BRC, + PathSensitiveBugReport &BR) override; void Profile(llvm::FoldingSetNodeID &ID) const override { ID.Add(NonLocalizedString); @@ -761,8 +761,8 @@ void NonLocalizedStringChecker::reportLocalizationError( return; // Generate the bug report. - std::unique_ptr<BugReport> R(new BugReport( - *BT, "User-facing text should use localized string macro", ErrNode)); + auto R = std::make_unique<PathSensitiveBugReport>( + *BT, "User-facing text should use localized string macro", ErrNode); if (argumentNumber) { R->addRange(M.getArgExpr(argumentNumber - 1)->getSourceRange()); } else { @@ -772,7 +772,7 @@ void NonLocalizedStringChecker::reportLocalizationError( const MemRegion *StringRegion = S.getAsRegion(); if (StringRegion) - R->addVisitor(llvm::make_unique<NonLocalizedStringBRVisitor>(StringRegion)); + R->addVisitor(std::make_unique<NonLocalizedStringBRVisitor>(StringRegion)); C.emitReport(std::move(R)); } @@ -882,18 +882,17 @@ void NonLocalizedStringChecker::checkPreObjCMessage(const ObjCMethodCall &msg, void NonLocalizedStringChecker::checkPreCall(const CallEvent &Call, CheckerContext &C) const { - const Decl *D = Call.getDecl(); - if (D && isa<FunctionDecl>(D)) { - const FunctionDecl *FD = dyn_cast<FunctionDecl>(D); - auto formals = FD->parameters(); - for (unsigned i = 0, - ei = std::min(unsigned(formals.size()), Call.getNumArgs()); - i != ei; ++i) { - if (isAnnotatedAsTakingLocalized(formals[i])) { - auto actual = Call.getArgSVal(i); - if (hasNonLocalizedState(actual, C)) { - reportLocalizationError(actual, Call, C, i + 1); - } + const auto *FD = dyn_cast_or_null<FunctionDecl>(Call.getDecl()); + if (!FD) + return; + + auto formals = FD->parameters(); + for (unsigned i = 0, ei = std::min(static_cast<unsigned>(formals.size()), + Call.getNumArgs()); i != ei; ++i) { + if (isAnnotatedAsTakingLocalized(formals[i])) { + auto actual = Call.getArgSVal(i); + if (hasNonLocalizedState(actual, C)) { + reportLocalizationError(actual, Call, C, i + 1); } } } @@ -998,9 +997,10 @@ void NonLocalizedStringChecker::checkPostStmt(const ObjCStringLiteral *SL, setNonLocalizedState(sv, C); } -std::shared_ptr<PathDiagnosticPiece> +PathDiagnosticPieceRef NonLocalizedStringBRVisitor::VisitNode(const ExplodedNode *Succ, - BugReporterContext &BRC, BugReport &BR) { + BugReporterContext &BRC, + PathSensitiveBugReport &BR) { if (Satisfied) return nullptr; diff --git a/lib/StaticAnalyzer/Checkers/MIGChecker.cpp b/lib/StaticAnalyzer/Checkers/MIGChecker.cpp index 6e7776bb484e5..d8fd125f4003d 100644 --- a/lib/StaticAnalyzer/Checkers/MIGChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/MIGChecker.cpp @@ -274,7 +274,7 @@ void MIGChecker::checkReturnAux(const ReturnStmt *RS, CheckerContext &C) const { if (!N) return; - auto R = llvm::make_unique<BugReport>( + auto R = std::make_unique<PathSensitiveBugReport>( BT, "MIG callback fails with error after deallocating argument value. " "This is a use-after-free vulnerability because the caller will try to " @@ -282,7 +282,8 @@ void MIGChecker::checkReturnAux(const ReturnStmt *RS, CheckerContext &C) const { N); R->addRange(RS->getSourceRange()); - bugreporter::trackExpressionValue(N, RS->getRetValue(), *R, false); + bugreporter::trackExpressionValue(N, RS->getRetValue(), *R, + bugreporter::TrackingKind::Thorough, false); C.emitReport(std::move(R)); } diff --git a/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.cpp b/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.cpp index b250d3f8795e7..bbf2ddec57620 100644 --- a/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.cpp +++ b/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.cpp @@ -30,8 +30,8 @@ void MPIBugReporter::reportDoubleNonblocking( ErrorText = "Double nonblocking on request " + RequestRegion->getDescriptiveName() + ". "; - auto Report = llvm::make_unique<BugReport>(*DoubleNonblockingBugType, - ErrorText, ExplNode); + auto Report = std::make_unique<PathSensitiveBugReport>( + *DoubleNonblockingBugType, ErrorText, ExplNode); Report->addRange(MPICallEvent.getSourceRange()); SourceRange Range = RequestRegion->sourceRange(); @@ -39,7 +39,7 @@ void MPIBugReporter::reportDoubleNonblocking( if (Range.isValid()) Report->addRange(Range); - Report->addVisitor(llvm::make_unique<RequestNodeVisitor>( + Report->addVisitor(std::make_unique<RequestNodeVisitor>( RequestRegion, "Request is previously used by nonblocking call here. ")); Report->markInteresting(RequestRegion); @@ -53,13 +53,13 @@ void MPIBugReporter::reportMissingWait( std::string ErrorText{"Request " + RequestRegion->getDescriptiveName() + " has no matching wait. "}; - auto Report = - llvm::make_unique<BugReport>(*MissingWaitBugType, ErrorText, ExplNode); + auto Report = std::make_unique<PathSensitiveBugReport>(*MissingWaitBugType, + ErrorText, ExplNode); SourceRange Range = RequestRegion->sourceRange(); if (Range.isValid()) Report->addRange(Range); - Report->addVisitor(llvm::make_unique<RequestNodeVisitor>( + Report->addVisitor(std::make_unique<RequestNodeVisitor>( RequestRegion, "Request is previously used by nonblocking call here. ")); Report->markInteresting(RequestRegion); @@ -73,8 +73,8 @@ void MPIBugReporter::reportUnmatchedWait( std::string ErrorText{"Request " + RequestRegion->getDescriptiveName() + " has no matching nonblocking call. "}; - auto Report = - llvm::make_unique<BugReport>(*UnmatchedWaitBugType, ErrorText, ExplNode); + auto Report = std::make_unique<PathSensitiveBugReport>(*UnmatchedWaitBugType, + ErrorText, ExplNode); Report->addRange(CE.getSourceRange()); SourceRange Range = RequestRegion->sourceRange(); @@ -84,20 +84,22 @@ void MPIBugReporter::reportUnmatchedWait( BReporter.emitReport(std::move(Report)); } -std::shared_ptr<PathDiagnosticPiece> +PathDiagnosticPieceRef MPIBugReporter::RequestNodeVisitor::VisitNode(const ExplodedNode *N, BugReporterContext &BRC, - BugReport &BR) { + PathSensitiveBugReport &BR) { if (IsNodeFound) return nullptr; const Request *const Req = N->getState()->get<RequestMap>(RequestRegion); + assert(Req && "The region must be tracked and alive, given that we've " + "just emitted a report against it"); const Request *const PrevReq = N->getFirstPred()->getState()->get<RequestMap>(RequestRegion); // Check if request was previously unused or in a different state. - if ((Req && !PrevReq) || (Req->CurrentState != PrevReq->CurrentState)) { + if (!PrevReq || (Req->CurrentState != PrevReq->CurrentState)) { IsNodeFound = true; ProgramPoint P = N->getFirstPred()->getLocation(); diff --git a/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.h b/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.h index 6fbc30288655c..9871da026b042 100644 --- a/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.h +++ b/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.h @@ -89,9 +89,9 @@ private: ID.AddPointer(RequestRegion); } - std::shared_ptr<PathDiagnosticPiece> VisitNode(const ExplodedNode *N, - BugReporterContext &BRC, - BugReport &BR) override; + PathDiagnosticPieceRef VisitNode(const ExplodedNode *N, + BugReporterContext &BRC, + PathSensitiveBugReport &BR) override; private: const MemRegion *const RequestRegion; diff --git a/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp b/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp index 32ba9bc8e2ef5..e064ca6bd88f6 100644 --- a/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp @@ -113,11 +113,14 @@ private: const ExplodedNode *getAllocationNode(const ExplodedNode *N, SymbolRef Sym, CheckerContext &C) const; - std::unique_ptr<BugReport> generateAllocatedDataNotReleasedReport( - const AllocationPair &AP, ExplodedNode *N, CheckerContext &C) const; + std::unique_ptr<PathSensitiveBugReport> + generateAllocatedDataNotReleasedReport(const AllocationPair &AP, + ExplodedNode *N, + CheckerContext &C) const; /// Mark an AllocationPair interesting for diagnostic reporting. - void markInteresting(BugReport *R, const AllocationPair &AP) const { + void markInteresting(PathSensitiveBugReport *R, + const AllocationPair &AP) const { R->markInteresting(AP.first); R->markInteresting(AP.second->Region); } @@ -139,9 +142,9 @@ private: ID.AddPointer(Sym); } - std::shared_ptr<PathDiagnosticPiece> VisitNode(const ExplodedNode *N, - BugReporterContext &BRC, - BugReport &BR) override; + PathDiagnosticPieceRef VisitNode(const ExplodedNode *N, + BugReporterContext &BRC, + PathSensitiveBugReport &BR) override; }; }; } @@ -236,8 +239,8 @@ void MacOSKeychainAPIChecker:: os << "Deallocator doesn't match the allocator: '" << FunctionsToTrack[PDeallocIdx].Name << "' should be used."; - auto Report = llvm::make_unique<BugReport>(*BT, os.str(), N); - Report->addVisitor(llvm::make_unique<SecKeychainBugVisitor>(AP.first)); + auto Report = std::make_unique<PathSensitiveBugReport>(*BT, os.str(), N); + Report->addVisitor(std::make_unique<SecKeychainBugVisitor>(AP.first)); Report->addRange(ArgExpr->getSourceRange()); markInteresting(Report.get(), AP); C.emitReport(std::move(Report)); @@ -280,8 +283,9 @@ void MacOSKeychainAPIChecker::checkPreStmt(const CallExpr *CE, << "the allocator: missing a call to '" << FunctionsToTrack[DIdx].Name << "'."; - auto Report = llvm::make_unique<BugReport>(*BT, os.str(), N); - Report->addVisitor(llvm::make_unique<SecKeychainBugVisitor>(V)); + auto Report = + std::make_unique<PathSensitiveBugReport>(*BT, os.str(), N); + Report->addVisitor(std::make_unique<SecKeychainBugVisitor>(V)); Report->addRange(ArgExpr->getSourceRange()); Report->markInteresting(AS->Region); C.emitReport(std::move(Report)); @@ -334,7 +338,7 @@ void MacOSKeychainAPIChecker::checkPreStmt(const CallExpr *CE, if (!N) return; initBugType(); - auto Report = llvm::make_unique<BugReport>( + auto Report = std::make_unique<PathSensitiveBugReport>( *BT, "Trying to free data which has not been allocated.", N); Report->addRange(ArgExpr->getSourceRange()); if (AS) @@ -465,7 +469,7 @@ MacOSKeychainAPIChecker::getAllocationNode(const ExplodedNode *N, return AllocNode; } -std::unique_ptr<BugReport> +std::unique_ptr<PathSensitiveBugReport> MacOSKeychainAPIChecker::generateAllocatedDataNotReleasedReport( const AllocationPair &AP, ExplodedNode *N, CheckerContext &C) const { const ADFunctionInfo &FI = FunctionsToTrack[AP.second->AllocatorIdx]; @@ -480,18 +484,18 @@ MacOSKeychainAPIChecker::generateAllocatedDataNotReleasedReport( // allocated, and only report a single path. PathDiagnosticLocation LocUsedForUniqueing; const ExplodedNode *AllocNode = getAllocationNode(N, AP.first, C); - const Stmt *AllocStmt = PathDiagnosticLocation::getStmt(AllocNode); + const Stmt *AllocStmt = AllocNode->getStmtForDiagnostics(); if (AllocStmt) LocUsedForUniqueing = PathDiagnosticLocation::createBegin(AllocStmt, C.getSourceManager(), AllocNode->getLocationContext()); - auto Report = - llvm::make_unique<BugReport>(*BT, os.str(), N, LocUsedForUniqueing, - AllocNode->getLocationContext()->getDecl()); + auto Report = std::make_unique<PathSensitiveBugReport>( + *BT, os.str(), N, LocUsedForUniqueing, + AllocNode->getLocationContext()->getDecl()); - Report->addVisitor(llvm::make_unique<SecKeychainBugVisitor>(AP.first)); + Report->addVisitor(std::make_unique<SecKeychainBugVisitor>(AP.first)); markInteresting(Report.get(), AP); return Report; } @@ -613,9 +617,10 @@ ProgramStateRef MacOSKeychainAPIChecker::checkPointerEscape( return State; } -std::shared_ptr<PathDiagnosticPiece> +PathDiagnosticPieceRef MacOSKeychainAPIChecker::SecKeychainBugVisitor::VisitNode( - const ExplodedNode *N, BugReporterContext &BRC, BugReport &BR) { + const ExplodedNode *N, BugReporterContext &BRC, + PathSensitiveBugReport &BR) { const AllocationState *AS = N->getState()->get<AllocatedData>(Sym); if (!AS) return nullptr; diff --git a/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp b/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp index 1c52d20d09914..d964a1668eaaa 100644 --- a/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp @@ -139,7 +139,8 @@ void MacOSXAPIChecker::CheckDispatchOnce(CheckerContext &C, const CallExpr *CE, BT_dispatchOnce.reset(new BugType(this, "Improper use of 'dispatch_once'", "API Misuse (Apple)")); - auto report = llvm::make_unique<BugReport>(*BT_dispatchOnce, os.str(), N); + auto report = + std::make_unique<PathSensitiveBugReport>(*BT_dispatchOnce, os.str(), N); report->addRange(CE->getArg(0)->getSourceRange()); C.emitReport(std::move(report)); } diff --git a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp index a79b341890655..a824499518730 100644 --- a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -6,8 +6,41 @@ // //===----------------------------------------------------------------------===// // -// This file defines malloc/free checker, which checks for potential memory -// leaks, double free, and use-after-free problems. +// This file defines a variety of memory management related checkers, such as +// leak, double free, and use-after-free. +// +// The following checkers are defined here: +// +// * MallocChecker +// Despite its name, it models all sorts of memory allocations and +// de- or reallocation, including but not limited to malloc, free, +// relloc, new, delete. It also reports on a variety of memory misuse +// errors. +// Many other checkers interact very closely with this checker, in fact, +// most are merely options to this one. Other checkers may register +// MallocChecker, but do not enable MallocChecker's reports (more details +// to follow around its field, ChecksEnabled). +// It also has a boolean "Optimistic" checker option, which if set to true +// will cause the checker to model user defined memory management related +// functions annotated via the attribute ownership_takes, ownership_holds +// and ownership_returns. +// +// * NewDeleteChecker +// Enables the modeling of new, new[], delete, delete[] in MallocChecker, +// and checks for related double-free and use-after-free errors. +// +// * NewDeleteLeaksChecker +// Checks for leaks related to new, new[], delete, delete[]. +// Depends on NewDeleteChecker. +// +// * MismatchedDeallocatorChecker +// Enables checking whether memory is deallocated with the correspending +// allocation function in MallocChecker, such as malloc() allocated +// regions are only freed by free(), new by delete, new[] by delete[]. +// +// InnerPointerChecker interacts very closely with MallocChecker, but unlike +// the above checkers, it has it's own file, hence the many InnerPointerChecker +// related headers and non-static functions. // //===----------------------------------------------------------------------===// @@ -37,6 +70,10 @@ using namespace clang; using namespace ento; +//===----------------------------------------------------------------------===// +// The types of allocation we're modeling. +//===----------------------------------------------------------------------===// + namespace { // Used to check correspondence between allocators and deallocators. @@ -50,57 +87,88 @@ enum AllocationFamily { AF_InnerBuffer }; +struct MemFunctionInfoTy; + +} // end of anonymous namespace + +/// Determine family of a deallocation expression. +static AllocationFamily +getAllocationFamily(const MemFunctionInfoTy &MemFunctionInfo, CheckerContext &C, + const Stmt *S); + +/// Print names of allocators and deallocators. +/// +/// \returns true on success. +static bool printAllocDeallocName(raw_ostream &os, CheckerContext &C, + const Expr *E); + +/// Print expected name of an allocator based on the deallocator's +/// family derived from the DeallocExpr. +static void printExpectedAllocName(raw_ostream &os, + const MemFunctionInfoTy &MemFunctionInfo, + CheckerContext &C, const Expr *E); + +/// Print expected name of a deallocator based on the allocator's +/// family. +static void printExpectedDeallocName(raw_ostream &os, AllocationFamily Family); + +//===----------------------------------------------------------------------===// +// The state of a symbol, in terms of memory management. +//===----------------------------------------------------------------------===// + +namespace { + class RefState { - enum Kind { // Reference to allocated memory. - Allocated, - // Reference to zero-allocated memory. - AllocatedOfSizeZero, - // Reference to released/freed memory. - Released, - // The responsibility for freeing resources has transferred from - // this reference. A relinquished symbol should not be freed. - Relinquished, - // We are no longer guaranteed to have observed all manipulations - // of this pointer/memory. For example, it could have been - // passed as a parameter to an opaque function. - Escaped + enum Kind { + // Reference to allocated memory. + Allocated, + // Reference to zero-allocated memory. + AllocatedOfSizeZero, + // Reference to released/freed memory. + Released, + // The responsibility for freeing resources has transferred from + // this reference. A relinquished symbol should not be freed. + Relinquished, + // We are no longer guaranteed to have observed all manipulations + // of this pointer/memory. For example, it could have been + // passed as a parameter to an opaque function. + Escaped }; const Stmt *S; - unsigned K : 3; // Kind enum, but stored as a bitfield. - unsigned Family : 29; // Rest of 32-bit word, currently just an allocation - // family. - RefState(Kind k, const Stmt *s, unsigned family) - : S(s), K(k), Family(family) { + Kind K; + AllocationFamily Family; + + RefState(Kind k, const Stmt *s, AllocationFamily family) + : S(s), K(k), Family(family) { assert(family != AF_None); } + public: bool isAllocated() const { return K == Allocated; } bool isAllocatedOfSizeZero() const { return K == AllocatedOfSizeZero; } bool isReleased() const { return K == Released; } bool isRelinquished() const { return K == Relinquished; } bool isEscaped() const { return K == Escaped; } - AllocationFamily getAllocationFamily() const { - return (AllocationFamily)Family; - } + AllocationFamily getAllocationFamily() const { return Family; } const Stmt *getStmt() const { return S; } bool operator==(const RefState &X) const { return K == X.K && S == X.S && Family == X.Family; } - static RefState getAllocated(unsigned family, const Stmt *s) { + static RefState getAllocated(AllocationFamily family, const Stmt *s) { return RefState(Allocated, s, family); } static RefState getAllocatedOfSizeZero(const RefState *RS) { return RefState(AllocatedOfSizeZero, RS->getStmt(), RS->getAllocationFamily()); } - static RefState getReleased(unsigned family, const Stmt *s) { + static RefState getReleased(AllocationFamily family, const Stmt *s) { return RefState(Released, s, family); } - static RefState getRelinquished(unsigned family, const Stmt *s) { + static RefState getRelinquished(AllocationFamily family, const Stmt *s) { return RefState(Relinquished, s, family); } static RefState getEscaped(const RefState *RS) { @@ -113,8 +181,8 @@ public: ID.AddInteger(Family); } - void dump(raw_ostream &OS) const { - switch (static_cast<Kind>(K)) { + LLVM_DUMP_METHOD void dump(raw_ostream &OS) const { + switch (K) { #define CASE(ID) case ID: OS << #ID; break; CASE(Allocated) CASE(AllocatedOfSizeZero) @@ -127,24 +195,62 @@ public: LLVM_DUMP_METHOD void dump() const { dump(llvm::errs()); } }; -enum ReallocPairKind { - RPToBeFreedAfterFailure, - // The symbol has been freed when reallocation failed. - RPIsFreeOnFailure, - // The symbol does not need to be freed after reallocation fails. - RPDoNotTrackAfterFailure +} // end of anonymous namespace + +REGISTER_MAP_WITH_PROGRAMSTATE(RegionState, SymbolRef, RefState) + +/// Check if the memory associated with this symbol was released. +static bool isReleased(SymbolRef Sym, CheckerContext &C); + +/// Update the RefState to reflect the new memory allocation. +/// The optional \p RetVal parameter specifies the newly allocated pointer +/// value; if unspecified, the value of expression \p E is used. +static ProgramStateRef MallocUpdateRefState(CheckerContext &C, const Expr *E, + ProgramStateRef State, + AllocationFamily Family = AF_Malloc, + Optional<SVal> RetVal = None); + +//===----------------------------------------------------------------------===// +// The modeling of memory reallocation. +// +// The terminology 'toPtr' and 'fromPtr' will be used: +// toPtr = realloc(fromPtr, 20); +//===----------------------------------------------------------------------===// + +REGISTER_SET_WITH_PROGRAMSTATE(ReallocSizeZeroSymbols, SymbolRef) + +namespace { + +/// The state of 'fromPtr' after reallocation is known to have failed. +enum OwnershipAfterReallocKind { + // The symbol needs to be freed (e.g.: realloc) + OAR_ToBeFreedAfterFailure, + // The symbol has been freed (e.g.: reallocf) + OAR_FreeOnFailure, + // The symbol doesn't have to freed (e.g.: we aren't sure if, how and where + // 'fromPtr' was allocated: + // void Haha(int *ptr) { + // ptr = realloc(ptr, 67); + // // ... + // } + // ). + OAR_DoNotTrackAfterFailure }; -/// \class ReallocPair -/// Stores information about the symbol being reallocated by a call to -/// 'realloc' to allow modeling failed reallocation later in the path. +/// Stores information about the 'fromPtr' symbol after reallocation. +/// +/// This is important because realloc may fail, and that needs special modeling. +/// Whether reallocation failed or not will not be known until later, so we'll +/// store whether upon failure 'fromPtr' will be freed, or needs to be freed +/// later, etc. struct ReallocPair { - // The symbol which realloc reallocated. + + // The 'fromPtr'. SymbolRef ReallocatedSym; - ReallocPairKind Kind; + OwnershipAfterReallocKind Kind; - ReallocPair(SymbolRef S, ReallocPairKind K) : - ReallocatedSym(S), Kind(K) {} + ReallocPair(SymbolRef S, OwnershipAfterReallocKind K) + : ReallocatedSym(S), Kind(K) {} void Profile(llvm::FoldingSetNodeID &ID) const { ID.AddInteger(Kind); ID.AddPointer(ReallocatedSym); @@ -155,42 +261,88 @@ struct ReallocPair { } }; -typedef std::pair<const ExplodedNode*, const MemRegion*> LeakInfo; - -class MallocChecker : public Checker<check::DeadSymbols, - check::PointerEscape, - check::ConstPointerEscape, - check::PreStmt<ReturnStmt>, - check::EndFunction, - check::PreCall, - check::PostStmt<CallExpr>, - check::PostStmt<CXXNewExpr>, - check::NewAllocator, - check::PreStmt<CXXDeleteExpr>, - check::PostStmt<BlockExpr>, - check::PostObjCMessage, - check::Location, - eval::Assume> -{ -public: - MallocChecker() - : II_alloca(nullptr), II_win_alloca(nullptr), II_malloc(nullptr), - II_free(nullptr), II_realloc(nullptr), II_calloc(nullptr), - II_valloc(nullptr), II_reallocf(nullptr), II_strndup(nullptr), - II_strdup(nullptr), II_win_strdup(nullptr), II_kmalloc(nullptr), - II_kfree(nullptr), II_if_nameindex(nullptr), - II_if_freenameindex(nullptr), II_wcsdup(nullptr), - II_win_wcsdup(nullptr), II_g_malloc(nullptr), II_g_malloc0(nullptr), - II_g_realloc(nullptr), II_g_try_malloc(nullptr), - II_g_try_malloc0(nullptr), II_g_try_realloc(nullptr), - II_g_free(nullptr), II_g_memdup(nullptr), II_g_malloc_n(nullptr), - II_g_malloc0_n(nullptr), II_g_realloc_n(nullptr), - II_g_try_malloc_n(nullptr), II_g_try_malloc0_n(nullptr), - II_g_try_realloc_n(nullptr) {} +} // end of anonymous namespace + +REGISTER_MAP_WITH_PROGRAMSTATE(ReallocPairs, SymbolRef, ReallocPair) + +//===----------------------------------------------------------------------===// +// Kinds of memory operations, information about resource managing functions. +//===----------------------------------------------------------------------===// + +namespace { + +enum class MemoryOperationKind { MOK_Allocate, MOK_Free, MOK_Any }; +struct MemFunctionInfoTy { + /// The value of the MallocChecker:Optimistic is stored in this variable. + /// /// In pessimistic mode, the checker assumes that it does not know which /// functions might free the memory. + /// In optimistic mode, the checker assumes that all user-defined functions + /// which might free a pointer are annotated. + DefaultBool ShouldIncludeOwnershipAnnotatedFunctions; + + // TODO: Change these to CallDescription, and get rid of lazy initialization. + mutable IdentifierInfo *II_alloca = nullptr, *II_win_alloca = nullptr, + *II_malloc = nullptr, *II_free = nullptr, + *II_realloc = nullptr, *II_calloc = nullptr, + *II_valloc = nullptr, *II_reallocf = nullptr, + *II_strndup = nullptr, *II_strdup = nullptr, + *II_win_strdup = nullptr, *II_kmalloc = nullptr, + *II_if_nameindex = nullptr, + *II_if_freenameindex = nullptr, *II_wcsdup = nullptr, + *II_win_wcsdup = nullptr, *II_g_malloc = nullptr, + *II_g_malloc0 = nullptr, *II_g_realloc = nullptr, + *II_g_try_malloc = nullptr, + *II_g_try_malloc0 = nullptr, + *II_g_try_realloc = nullptr, *II_g_free = nullptr, + *II_g_memdup = nullptr, *II_g_malloc_n = nullptr, + *II_g_malloc0_n = nullptr, *II_g_realloc_n = nullptr, + *II_g_try_malloc_n = nullptr, + *II_g_try_malloc0_n = nullptr, *II_kfree = nullptr, + *II_g_try_realloc_n = nullptr; + + void initIdentifierInfo(ASTContext &C) const; + + ///@{ + /// Check if this is one of the functions which can allocate/reallocate + /// memory pointed to by one of its arguments. + bool isMemFunction(const FunctionDecl *FD, ASTContext &C) const; + bool isCMemFunction(const FunctionDecl *FD, ASTContext &C, + AllocationFamily Family, + MemoryOperationKind MemKind) const; + + /// Tells if the callee is one of the builtin new/delete operators, including + /// placement operators and other standard overloads. + bool isStandardNewDelete(const FunctionDecl *FD, ASTContext &C) const; + ///@} +}; + +} // end of anonymous namespace + +//===----------------------------------------------------------------------===// +// Definition of the MallocChecker class. +//===----------------------------------------------------------------------===// + +namespace { + +class MallocChecker + : public Checker<check::DeadSymbols, check::PointerEscape, + check::ConstPointerEscape, check::PreStmt<ReturnStmt>, + check::EndFunction, check::PreCall, + check::PostStmt<CallExpr>, check::PostStmt<CXXNewExpr>, + check::NewAllocator, check::PreStmt<CXXDeleteExpr>, + check::PostStmt<BlockExpr>, check::PostObjCMessage, + check::Location, eval::Assume> { +public: + MemFunctionInfoTy MemFunctionInfo; + + /// Many checkers are essentially built into this one, so enabling them will + /// make MallocChecker perform additional modeling and reporting. enum CheckKind { + /// When a subchecker is enabled but MallocChecker isn't, model memory + /// management but do not emit warnings emitted with MallocChecker only + /// enabled. CK_MallocChecker, CK_NewDeleteChecker, CK_NewDeleteLeaksChecker, @@ -199,16 +351,10 @@ public: CK_NumCheckKinds }; - enum class MemoryOperationKind { - MOK_Allocate, - MOK_Free, - MOK_Any - }; - - DefaultBool IsOptimistic; + using LeakInfo = std::pair<const ExplodedNode *, const MemRegion *>; DefaultBool ChecksEnabled[CK_NumCheckKinds]; - CheckName CheckNames[CK_NumCheckKinds]; + CheckerNameRef CheckNames[CK_NumCheckKinds]; void checkPreCall(const CallEvent &Call, CheckerContext &C) const; void checkPostStmt(const CallExpr *CE, CheckerContext &C) const; @@ -248,47 +394,9 @@ private: mutable std::unique_ptr<BugType> BT_MismatchedDealloc; mutable std::unique_ptr<BugType> BT_OffsetFree[CK_NumCheckKinds]; mutable std::unique_ptr<BugType> BT_UseZerroAllocated[CK_NumCheckKinds]; - mutable IdentifierInfo *II_alloca, *II_win_alloca, *II_malloc, *II_free, - *II_realloc, *II_calloc, *II_valloc, *II_reallocf, - *II_strndup, *II_strdup, *II_win_strdup, *II_kmalloc, - *II_kfree, *II_if_nameindex, *II_if_freenameindex, - *II_wcsdup, *II_win_wcsdup, *II_g_malloc, - *II_g_malloc0, *II_g_realloc, *II_g_try_malloc, - *II_g_try_malloc0, *II_g_try_realloc, *II_g_free, - *II_g_memdup, *II_g_malloc_n, *II_g_malloc0_n, - *II_g_realloc_n, *II_g_try_malloc_n, - *II_g_try_malloc0_n, *II_g_try_realloc_n; - mutable Optional<uint64_t> KernelZeroFlagVal; - - void initIdentifierInfo(ASTContext &C) const; - - /// Determine family of a deallocation expression. - AllocationFamily getAllocationFamily(CheckerContext &C, const Stmt *S) const; - - /// Print names of allocators and deallocators. - /// - /// \returns true on success. - bool printAllocDeallocName(raw_ostream &os, CheckerContext &C, - const Expr *E) const; - - /// Print expected name of an allocator based on the deallocator's - /// family derived from the DeallocExpr. - void printExpectedAllocName(raw_ostream &os, CheckerContext &C, - const Expr *DeallocExpr) const; - /// Print expected name of a deallocator based on the allocator's - /// family. - void printExpectedDeallocName(raw_ostream &os, AllocationFamily Family) const; - ///@{ - /// Check if this is one of the functions which can allocate/reallocate memory - /// pointed to by one of its arguments. - bool isMemFunction(const FunctionDecl *FD, ASTContext &C) const; - bool isCMemFunction(const FunctionDecl *FD, - ASTContext &C, - AllocationFamily Family, - MemoryOperationKind MemKind) const; - bool isStandardNewDelete(const FunctionDecl *FD, ASTContext &C) const; - ///@} + // TODO: Remove mutable by moving the initializtaion to the registry function. + mutable Optional<uint64_t> KernelZeroFlagVal; /// Process C++ operator new()'s allocation, which is the part of C++ /// new-expression that goes before the constructor. @@ -296,23 +404,64 @@ private: SVal Target) const; /// Perform a zero-allocation check. - /// The optional \p RetVal parameter specifies the newly allocated pointer - /// value; if unspecified, the value of expression \p E is used. - ProgramStateRef ProcessZeroAllocation(CheckerContext &C, const Expr *E, - const unsigned AllocationSizeArg, - ProgramStateRef State, - Optional<SVal> RetVal = None) const; - + /// + /// \param [in] E The expression that allocates memory. + /// \param [in] IndexOfSizeArg Index of the argument that specifies the size + /// of the memory that needs to be allocated. E.g. for malloc, this would be + /// 0. + /// \param [in] RetVal Specifies the newly allocated pointer value; + /// if unspecified, the value of expression \p E is used. + static ProgramStateRef ProcessZeroAllocCheck(CheckerContext &C, const Expr *E, + const unsigned IndexOfSizeArg, + ProgramStateRef State, + Optional<SVal> RetVal = None); + + /// Model functions with the ownership_returns attribute. + /// + /// User-defined function may have the ownership_returns attribute, which + /// annotates that the function returns with an object that was allocated on + /// the heap, and passes the ownertship to the callee. + /// + /// void __attribute((ownership_returns(malloc, 1))) *my_malloc(size_t); + /// + /// It has two parameters: + /// - first: name of the resource (e.g. 'malloc') + /// - (OPTIONAL) second: size of the allocated region + /// + /// \param [in] CE The expression that allocates memory. + /// \param [in] Att The ownership_returns attribute. + /// \param [in] State The \c ProgramState right before allocation. + /// \returns The ProgramState right after allocation. ProgramStateRef MallocMemReturnsAttr(CheckerContext &C, const CallExpr *CE, const OwnershipAttr* Att, ProgramStateRef State) const; + + /// Models memory allocation. + /// + /// \param [in] CE The expression that allocates memory. + /// \param [in] SizeEx Size of the memory that needs to be allocated. + /// \param [in] Init The value the allocated memory needs to be initialized. + /// with. For example, \c calloc initializes the allocated memory to 0, + /// malloc leaves it undefined. + /// \param [in] State The \c ProgramState right before allocation. + /// \returns The ProgramState right after allocation. static ProgramStateRef MallocMemAux(CheckerContext &C, const CallExpr *CE, const Expr *SizeEx, SVal Init, ProgramStateRef State, AllocationFamily Family = AF_Malloc); + + /// Models memory allocation. + /// + /// \param [in] CE The expression that allocates memory. + /// \param [in] Size Size of the memory that needs to be allocated. + /// \param [in] Init The value the allocated memory needs to be initialized. + /// with. For example, \c calloc initializes the allocated memory to 0, + /// malloc leaves it undefined. + /// \param [in] State The \c ProgramState right before allocation. + /// \returns The ProgramState right after allocation. static ProgramStateRef MallocMemAux(CheckerContext &C, const CallExpr *CE, - SVal SizeEx, SVal Init, + SVal Size, SVal Init, ProgramStateRef State, AllocationFamily Family = AF_Malloc); @@ -325,54 +474,125 @@ private: performKernelMalloc(const CallExpr *CE, CheckerContext &C, const ProgramStateRef &State) const; - /// Update the RefState to reflect the new memory allocation. - /// The optional \p RetVal parameter specifies the newly allocated pointer - /// value; if unspecified, the value of expression \p E is used. - static ProgramStateRef - MallocUpdateRefState(CheckerContext &C, const Expr *E, ProgramStateRef State, - AllocationFamily Family = AF_Malloc, - Optional<SVal> RetVal = None); - + /// Model functions with the ownership_takes and ownership_holds attributes. + /// + /// User-defined function may have the ownership_takes and/or ownership_holds + /// attributes, which annotates that the function frees the memory passed as a + /// parameter. + /// + /// void __attribute((ownership_takes(malloc, 1))) my_free(void *); + /// void __attribute((ownership_holds(malloc, 1))) my_hold(void *); + /// + /// They have two parameters: + /// - first: name of the resource (e.g. 'malloc') + /// - second: index of the parameter the attribute applies to + /// + /// \param [in] CE The expression that frees memory. + /// \param [in] Att The ownership_takes or ownership_holds attribute. + /// \param [in] State The \c ProgramState right before allocation. + /// \returns The ProgramState right after deallocation. ProgramStateRef FreeMemAttr(CheckerContext &C, const CallExpr *CE, const OwnershipAttr* Att, ProgramStateRef State) const; + + /// Models memory deallocation. + /// + /// \param [in] CE The expression that frees memory. + /// \param [in] State The \c ProgramState right before allocation. + /// \param [in] Num Index of the argument that needs to be freed. This is + /// normally 0, but for custom free functions it may be different. + /// \param [in] Hold Whether the parameter at \p Index has the ownership_holds + /// attribute. + /// \param [out] IsKnownToBeAllocated Whether the memory to be freed is known + /// to have been allocated, or in other words, the symbol to be freed was + /// registered as allocated by this checker. In the following case, \c ptr + /// isn't known to be allocated. + /// void Haha(int *ptr) { + /// ptr = realloc(ptr, 67); + /// // ... + /// } + /// \param [in] ReturnsNullOnFailure Whether the memory deallocation function + /// we're modeling returns with Null on failure. + /// \returns The ProgramState right after deallocation. ProgramStateRef FreeMemAux(CheckerContext &C, const CallExpr *CE, - ProgramStateRef state, unsigned Num, - bool Hold, - bool &ReleasedAllocated, + ProgramStateRef State, unsigned Num, bool Hold, + bool &IsKnownToBeAllocated, bool ReturnsNullOnFailure = false) const; - ProgramStateRef FreeMemAux(CheckerContext &C, const Expr *Arg, - const Expr *ParentExpr, - ProgramStateRef State, - bool Hold, - bool &ReleasedAllocated, + + /// Models memory deallocation. + /// + /// \param [in] ArgExpr The variable who's pointee needs to be freed. + /// \param [in] ParentExpr The expression that frees the memory. + /// \param [in] State The \c ProgramState right before allocation. + /// normally 0, but for custom free functions it may be different. + /// \param [in] Hold Whether the parameter at \p Index has the ownership_holds + /// attribute. + /// \param [out] IsKnownToBeAllocated Whether the memory to be freed is known + /// to have been allocated, or in other words, the symbol to be freed was + /// registered as allocated by this checker. In the following case, \c ptr + /// isn't known to be allocated. + /// void Haha(int *ptr) { + /// ptr = realloc(ptr, 67); + /// // ... + /// } + /// \param [in] ReturnsNullOnFailure Whether the memory deallocation function + /// we're modeling returns with Null on failure. + /// \returns The ProgramState right after deallocation. + ProgramStateRef FreeMemAux(CheckerContext &C, const Expr *ArgExpr, + const Expr *ParentExpr, ProgramStateRef State, + bool Hold, bool &IsKnownToBeAllocated, bool ReturnsNullOnFailure = false) const; + // TODO: Needs some refactoring, as all other deallocation modeling + // functions are suffering from out parameters and messy code due to how + // realloc is handled. + // + /// Models memory reallocation. + /// + /// \param [in] CE The expression that reallocated memory + /// \param [in] ShouldFreeOnFail Whether if reallocation fails, the supplied + /// memory should be freed. + /// \param [in] State The \c ProgramState right before reallocation. + /// \param [in] SuffixWithN Whether the reallocation function we're modeling + /// has an '_n' suffix, such as g_realloc_n. + /// \returns The ProgramState right after reallocation. ProgramStateRef ReallocMemAux(CheckerContext &C, const CallExpr *CE, - bool FreesMemOnFailure, - ProgramStateRef State, + bool ShouldFreeOnFail, ProgramStateRef State, bool SuffixWithN = false) const; + + /// Evaluates the buffer size that needs to be allocated. + /// + /// \param [in] Blocks The amount of blocks that needs to be allocated. + /// \param [in] BlockBytes The size of a block. + /// \returns The symbolic value of \p Blocks * \p BlockBytes. static SVal evalMulForBufferSize(CheckerContext &C, const Expr *Blocks, const Expr *BlockBytes); + + /// Models zero initialized array allocation. + /// + /// \param [in] CE The expression that reallocated memory + /// \param [in] State The \c ProgramState right before reallocation. + /// \returns The ProgramState right after allocation. static ProgramStateRef CallocMem(CheckerContext &C, const CallExpr *CE, ProgramStateRef State); - /// Check if the memory associated with this symbol was released. - bool isReleased(SymbolRef Sym, CheckerContext &C) const; - /// See if deallocation happens in a suspicious context. If so, escape the /// pointers that otherwise would have been deallocated and return true. bool suppressDeallocationsInSuspiciousContexts(const CallExpr *CE, CheckerContext &C) const; + /// If in \p S \p Sym is used, check whether \p Sym was already freed. bool checkUseAfterFree(SymbolRef Sym, CheckerContext &C, const Stmt *S) const; + /// If in \p S \p Sym is used, check whether \p Sym was allocated as a zero + /// sized memory region. void checkUseZeroAllocated(SymbolRef Sym, CheckerContext &C, const Stmt *S) const; + /// If in \p S \p Sym is being freed, check whether \p Sym was already freed. bool checkDoubleDelete(SymbolRef Sym, CheckerContext &C) const; - /// Check if the function is known free memory, or if it is + /// Check if the function is known to free memory, or if it is /// "interesting" and should be modeled explicitly. /// /// \param [out] EscapingSymbol A function might not free memory in general, @@ -386,12 +606,12 @@ private: ProgramStateRef State, SymbolRef &EscapingSymbol) const; - // Implementation of the checkPointerEscape callbacks. + /// Implementation of the checkPointerEscape callbacks. ProgramStateRef checkPointerEscapeAux(ProgramStateRef State, - const InvalidatedSymbols &Escaped, - const CallEvent *Call, - PointerEscapeKind Kind, - bool(*CheckRefState)(const RefState*)) const; + const InvalidatedSymbols &Escaped, + const CallEvent *Call, + PointerEscapeKind Kind, + bool IsConstPointerEscape) const; // Implementation of the checkPreStmt and checkEndFunction callbacks. void checkEscapeOnReturn(const ReturnStmt *S, CheckerContext &C) const; @@ -410,6 +630,7 @@ private: ///@} static bool SummarizeValue(raw_ostream &os, SVal V); static bool SummarizeRegion(raw_ostream &os, const MemRegion *MR); + void ReportBadFree(CheckerContext &C, SVal ArgVal, SourceRange Range, const Expr *DeallocExpr) const; void ReportFreeAlloca(CheckerContext &C, SVal ArgVal, @@ -435,143 +656,142 @@ private: /// Find the location of the allocation for Sym on the path leading to the /// exploded node N. - LeakInfo getAllocationSite(const ExplodedNode *N, SymbolRef Sym, - CheckerContext &C) const; + static LeakInfo getAllocationSite(const ExplodedNode *N, SymbolRef Sym, + CheckerContext &C); void reportLeak(SymbolRef Sym, ExplodedNode *N, CheckerContext &C) const; +}; - /// The bug visitor which allows us to print extra diagnostics along the - /// BugReport path. For example, showing the allocation site of the leaked - /// region. - class MallocBugVisitor final : public BugReporterVisitor { - protected: - enum NotificationMode { - Normal, - ReallocationFailed - }; - - // The allocated region symbol tracked by the main analysis. - SymbolRef Sym; - - // The mode we are in, i.e. what kind of diagnostics will be emitted. - NotificationMode Mode; - - // A symbol from when the primary region should have been reallocated. - SymbolRef FailedReallocSymbol; - - // A C++ destructor stack frame in which memory was released. Used for - // miscellaneous false positive suppression. - const StackFrameContext *ReleaseDestructorLC; - - bool IsLeak; - - public: - MallocBugVisitor(SymbolRef S, bool isLeak = false) - : Sym(S), Mode(Normal), FailedReallocSymbol(nullptr), - ReleaseDestructorLC(nullptr), IsLeak(isLeak) {} - - static void *getTag() { - static int Tag = 0; - return &Tag; - } +//===----------------------------------------------------------------------===// +// Definition of MallocBugVisitor. +//===----------------------------------------------------------------------===// - void Profile(llvm::FoldingSetNodeID &ID) const override { - ID.AddPointer(getTag()); - ID.AddPointer(Sym); - } +/// The bug visitor which allows us to print extra diagnostics along the +/// BugReport path. For example, showing the allocation site of the leaked +/// region. +class MallocBugVisitor final : public BugReporterVisitor { +protected: + enum NotificationMode { Normal, ReallocationFailed }; - inline bool isAllocated(const RefState *S, const RefState *SPrev, - const Stmt *Stmt) { - // Did not track -> allocated. Other state (released) -> allocated. - return (Stmt && (isa<CallExpr>(Stmt) || isa<CXXNewExpr>(Stmt)) && - (S && (S->isAllocated() || S->isAllocatedOfSizeZero())) && - (!SPrev || !(SPrev->isAllocated() || - SPrev->isAllocatedOfSizeZero()))); - } + // The allocated region symbol tracked by the main analysis. + SymbolRef Sym; - inline bool isReleased(const RefState *S, const RefState *SPrev, - const Stmt *Stmt) { - // Did not track -> released. Other state (allocated) -> released. - // The statement associated with the release might be missing. - bool IsReleased = (S && S->isReleased()) && - (!SPrev || !SPrev->isReleased()); - assert(!IsReleased || - (Stmt && (isa<CallExpr>(Stmt) || isa<CXXDeleteExpr>(Stmt))) || - (!Stmt && S->getAllocationFamily() == AF_InnerBuffer)); - return IsReleased; - } + // The mode we are in, i.e. what kind of diagnostics will be emitted. + NotificationMode Mode; - inline bool isRelinquished(const RefState *S, const RefState *SPrev, - const Stmt *Stmt) { - // Did not track -> relinquished. Other state (allocated) -> relinquished. - return (Stmt && (isa<CallExpr>(Stmt) || isa<ObjCMessageExpr>(Stmt) || - isa<ObjCPropertyRefExpr>(Stmt)) && - (S && S->isRelinquished()) && - (!SPrev || !SPrev->isRelinquished())); - } + // A symbol from when the primary region should have been reallocated. + SymbolRef FailedReallocSymbol; - inline bool isReallocFailedCheck(const RefState *S, const RefState *SPrev, - const Stmt *Stmt) { - // If the expression is not a call, and the state change is - // released -> allocated, it must be the realloc return value - // check. If we have to handle more cases here, it might be cleaner just - // to track this extra bit in the state itself. - return ((!Stmt || !isa<CallExpr>(Stmt)) && - (S && (S->isAllocated() || S->isAllocatedOfSizeZero())) && - (SPrev && !(SPrev->isAllocated() || - SPrev->isAllocatedOfSizeZero()))); - } + // A C++ destructor stack frame in which memory was released. Used for + // miscellaneous false positive suppression. + const StackFrameContext *ReleaseDestructorLC; - std::shared_ptr<PathDiagnosticPiece> VisitNode(const ExplodedNode *N, - BugReporterContext &BRC, - BugReport &BR) override; + bool IsLeak; - std::shared_ptr<PathDiagnosticPiece> - getEndPath(BugReporterContext &BRC, const ExplodedNode *EndPathNode, - BugReport &BR) override { - if (!IsLeak) - return nullptr; +public: + MallocBugVisitor(SymbolRef S, bool isLeak = false) + : Sym(S), Mode(Normal), FailedReallocSymbol(nullptr), + ReleaseDestructorLC(nullptr), IsLeak(isLeak) {} + + static void *getTag() { + static int Tag = 0; + return &Tag; + } + + void Profile(llvm::FoldingSetNodeID &ID) const override { + ID.AddPointer(getTag()); + ID.AddPointer(Sym); + } + + /// Did not track -> allocated. Other state (released) -> allocated. + static inline bool isAllocated(const RefState *RSCurr, const RefState *RSPrev, + const Stmt *Stmt) { + return (Stmt && (isa<CallExpr>(Stmt) || isa<CXXNewExpr>(Stmt)) && + (RSCurr && + (RSCurr->isAllocated() || RSCurr->isAllocatedOfSizeZero())) && + (!RSPrev || + !(RSPrev->isAllocated() || RSPrev->isAllocatedOfSizeZero()))); + } + + /// Did not track -> released. Other state (allocated) -> released. + /// The statement associated with the release might be missing. + static inline bool isReleased(const RefState *RSCurr, const RefState *RSPrev, + const Stmt *Stmt) { + bool IsReleased = + (RSCurr && RSCurr->isReleased()) && (!RSPrev || !RSPrev->isReleased()); + assert(!IsReleased || + (Stmt && (isa<CallExpr>(Stmt) || isa<CXXDeleteExpr>(Stmt))) || + (!Stmt && RSCurr->getAllocationFamily() == AF_InnerBuffer)); + return IsReleased; + } + + /// Did not track -> relinquished. Other state (allocated) -> relinquished. + static inline bool isRelinquished(const RefState *RSCurr, + const RefState *RSPrev, const Stmt *Stmt) { + return (Stmt && + (isa<CallExpr>(Stmt) || isa<ObjCMessageExpr>(Stmt) || + isa<ObjCPropertyRefExpr>(Stmt)) && + (RSCurr && RSCurr->isRelinquished()) && + (!RSPrev || !RSPrev->isRelinquished())); + } + + /// If the expression is not a call, and the state change is + /// released -> allocated, it must be the realloc return value + /// check. If we have to handle more cases here, it might be cleaner just + /// to track this extra bit in the state itself. + static inline bool hasReallocFailed(const RefState *RSCurr, + const RefState *RSPrev, + const Stmt *Stmt) { + return ((!Stmt || !isa<CallExpr>(Stmt)) && + (RSCurr && + (RSCurr->isAllocated() || RSCurr->isAllocatedOfSizeZero())) && + (RSPrev && + !(RSPrev->isAllocated() || RSPrev->isAllocatedOfSizeZero()))); + } + + PathDiagnosticPieceRef VisitNode(const ExplodedNode *N, + BugReporterContext &BRC, + PathSensitiveBugReport &BR) override; + + PathDiagnosticPieceRef getEndPath(BugReporterContext &BRC, + const ExplodedNode *EndPathNode, + PathSensitiveBugReport &BR) override { + if (!IsLeak) + return nullptr; - PathDiagnosticLocation L = - PathDiagnosticLocation::createEndOfPath(EndPathNode, - BRC.getSourceManager()); - // Do not add the statement itself as a range in case of leak. - return std::make_shared<PathDiagnosticEventPiece>(L, BR.getDescription(), - false); - } + PathDiagnosticLocation L = BR.getLocation(); + // Do not add the statement itself as a range in case of leak. + return std::make_shared<PathDiagnosticEventPiece>(L, BR.getDescription(), + false); + } - private: - class StackHintGeneratorForReallocationFailed - : public StackHintGeneratorForSymbol { - public: - StackHintGeneratorForReallocationFailed(SymbolRef S, StringRef M) +private: + class StackHintGeneratorForReallocationFailed + : public StackHintGeneratorForSymbol { + public: + StackHintGeneratorForReallocationFailed(SymbolRef S, StringRef M) : StackHintGeneratorForSymbol(S, M) {} - std::string getMessageForArg(const Expr *ArgE, - unsigned ArgIndex) override { - // Printed parameters start at 1, not 0. - ++ArgIndex; + std::string getMessageForArg(const Expr *ArgE, unsigned ArgIndex) override { + // Printed parameters start at 1, not 0. + ++ArgIndex; - SmallString<200> buf; - llvm::raw_svector_ostream os(buf); + SmallString<200> buf; + llvm::raw_svector_ostream os(buf); - os << "Reallocation of " << ArgIndex << llvm::getOrdinalSuffix(ArgIndex) - << " parameter failed"; + os << "Reallocation of " << ArgIndex << llvm::getOrdinalSuffix(ArgIndex) + << " parameter failed"; - return os.str(); - } + return os.str(); + } - std::string getMessageForReturn(const CallExpr *CallExpr) override { - return "Reallocation of returned value failed"; - } - }; + std::string getMessageForReturn(const CallExpr *CallExpr) override { + return "Reallocation of returned value failed"; + } }; }; -} // end anonymous namespace -REGISTER_MAP_WITH_PROGRAMSTATE(RegionState, SymbolRef, RefState) -REGISTER_MAP_WITH_PROGRAMSTATE(ReallocPairs, SymbolRef, ReallocPair) -REGISTER_SET_WITH_PROGRAMSTATE(ReallocSizeZeroSymbols, SymbolRef) +} // end anonymous namespace // A map from the freed symbol to the symbol representing the return value of // the free function. @@ -591,7 +811,11 @@ public: }; } // end anonymous namespace -void MallocChecker::initIdentifierInfo(ASTContext &Ctx) const { +//===----------------------------------------------------------------------===// +// Methods of MemFunctionInfoTy. +//===----------------------------------------------------------------------===// + +void MemFunctionInfoTy::initIdentifierInfo(ASTContext &Ctx) const { if (II_malloc) return; II_alloca = &Ctx.Idents.get("alloca"); @@ -631,7 +855,8 @@ void MallocChecker::initIdentifierInfo(ASTContext &Ctx) const { II_g_try_realloc_n = &Ctx.Idents.get("g_try_realloc_n"); } -bool MallocChecker::isMemFunction(const FunctionDecl *FD, ASTContext &C) const { +bool MemFunctionInfoTy::isMemFunction(const FunctionDecl *FD, + ASTContext &C) const { if (isCMemFunction(FD, C, AF_Malloc, MemoryOperationKind::MOK_Any)) return true; @@ -647,10 +872,9 @@ bool MallocChecker::isMemFunction(const FunctionDecl *FD, ASTContext &C) const { return false; } -bool MallocChecker::isCMemFunction(const FunctionDecl *FD, - ASTContext &C, - AllocationFamily Family, - MemoryOperationKind MemKind) const { +bool MemFunctionInfoTy::isCMemFunction(const FunctionDecl *FD, ASTContext &C, + AllocationFamily Family, + MemoryOperationKind MemKind) const { if (!FD) return false; @@ -703,7 +927,7 @@ bool MallocChecker::isCMemFunction(const FunctionDecl *FD, if (Family != AF_Malloc) return false; - if (IsOptimistic && FD->hasAttrs()) { + if (ShouldIncludeOwnershipAnnotatedFunctions && FD->hasAttrs()) { for (const auto *I : FD->specific_attrs<OwnershipAttr>()) { OwnershipAttr::OwnershipKind OwnKind = I->getOwnKind(); if(OwnKind == OwnershipAttr::Takes || OwnKind == OwnershipAttr::Holds) { @@ -718,11 +942,8 @@ bool MallocChecker::isCMemFunction(const FunctionDecl *FD, return false; } - -// Tells if the callee is one of the builtin new/delete operators, including -// placement operators and other standard overloads. -bool MallocChecker::isStandardNewDelete(const FunctionDecl *FD, - ASTContext &C) const { +bool MemFunctionInfoTy::isStandardNewDelete(const FunctionDecl *FD, + ASTContext &C) const { if (!FD) return false; @@ -738,6 +959,10 @@ bool MallocChecker::isStandardNewDelete(const FunctionDecl *FD, return !L.isValid() || C.getSourceManager().isInSystemHeader(L); } +//===----------------------------------------------------------------------===// +// Methods of MallocChecker and MallocBugVisitor. +//===----------------------------------------------------------------------===// + llvm::Optional<ProgramStateRef> MallocChecker::performKernelMalloc( const CallExpr *CE, CheckerContext &C, const ProgramStateRef &State) const { // 3-argument malloc(), as commonly used in {Free,Net,Open}BSD Kernels: @@ -836,28 +1061,35 @@ void MallocChecker::checkPostStmt(const CallExpr *CE, CheckerContext &C) const { return; ProgramStateRef State = C.getState(); - bool ReleasedAllocatedMemory = false; + bool IsKnownToBeAllocatedMemory = false; if (FD->getKind() == Decl::Function) { - initIdentifierInfo(C.getASTContext()); + MemFunctionInfo.initIdentifierInfo(C.getASTContext()); IdentifierInfo *FunI = FD->getIdentifier(); - if (FunI == II_malloc || FunI == II_g_malloc || FunI == II_g_try_malloc) { - if (CE->getNumArgs() < 1) + if (FunI == MemFunctionInfo.II_malloc || + FunI == MemFunctionInfo.II_g_malloc || + FunI == MemFunctionInfo.II_g_try_malloc) { + switch (CE->getNumArgs()) { + default: return; - if (CE->getNumArgs() < 3) { + case 1: + State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State); + State = ProcessZeroAllocCheck(C, CE, 0, State); + break; + case 2: State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State); - if (CE->getNumArgs() == 1) - State = ProcessZeroAllocation(C, CE, 0, State); - } else if (CE->getNumArgs() == 3) { + break; + case 3: llvm::Optional<ProgramStateRef> MaybeState = performKernelMalloc(CE, C, State); if (MaybeState.hasValue()) State = MaybeState.getValue(); else State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State); + break; } - } else if (FunI == II_kmalloc) { + } else if (FunI == MemFunctionInfo.II_kmalloc) { if (CE->getNumArgs() < 1) return; llvm::Optional<ProgramStateRef> MaybeState = @@ -866,100 +1098,116 @@ void MallocChecker::checkPostStmt(const CallExpr *CE, CheckerContext &C) const { State = MaybeState.getValue(); else State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State); - } else if (FunI == II_valloc) { + } else if (FunI == MemFunctionInfo.II_valloc) { if (CE->getNumArgs() < 1) return; State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State); - State = ProcessZeroAllocation(C, CE, 0, State); - } else if (FunI == II_realloc || FunI == II_g_realloc || - FunI == II_g_try_realloc) { - State = ReallocMemAux(C, CE, false, State); - State = ProcessZeroAllocation(C, CE, 1, State); - } else if (FunI == II_reallocf) { - State = ReallocMemAux(C, CE, true, State); - State = ProcessZeroAllocation(C, CE, 1, State); - } else if (FunI == II_calloc) { + State = ProcessZeroAllocCheck(C, CE, 0, State); + } else if (FunI == MemFunctionInfo.II_realloc || + FunI == MemFunctionInfo.II_g_realloc || + FunI == MemFunctionInfo.II_g_try_realloc) { + State = ReallocMemAux(C, CE, /*ShouldFreeOnFail*/ false, State); + State = ProcessZeroAllocCheck(C, CE, 1, State); + } else if (FunI == MemFunctionInfo.II_reallocf) { + State = ReallocMemAux(C, CE, /*ShouldFreeOnFail*/ true, State); + State = ProcessZeroAllocCheck(C, CE, 1, State); + } else if (FunI == MemFunctionInfo.II_calloc) { State = CallocMem(C, CE, State); - State = ProcessZeroAllocation(C, CE, 0, State); - State = ProcessZeroAllocation(C, CE, 1, State); - } else if (FunI == II_free || FunI == II_g_free || FunI == II_kfree) { + State = ProcessZeroAllocCheck(C, CE, 0, State); + State = ProcessZeroAllocCheck(C, CE, 1, State); + } else if (FunI == MemFunctionInfo.II_free || + FunI == MemFunctionInfo.II_g_free || + FunI == MemFunctionInfo.II_kfree) { if (suppressDeallocationsInSuspiciousContexts(CE, C)) return; - State = FreeMemAux(C, CE, State, 0, false, ReleasedAllocatedMemory); - } else if (FunI == II_strdup || FunI == II_win_strdup || - FunI == II_wcsdup || FunI == II_win_wcsdup) { + State = FreeMemAux(C, CE, State, 0, false, IsKnownToBeAllocatedMemory); + } else if (FunI == MemFunctionInfo.II_strdup || + FunI == MemFunctionInfo.II_win_strdup || + FunI == MemFunctionInfo.II_wcsdup || + FunI == MemFunctionInfo.II_win_wcsdup) { State = MallocUpdateRefState(C, CE, State); - } else if (FunI == II_strndup) { + } else if (FunI == MemFunctionInfo.II_strndup) { State = MallocUpdateRefState(C, CE, State); - } else if (FunI == II_alloca || FunI == II_win_alloca) { + } else if (FunI == MemFunctionInfo.II_alloca || + FunI == MemFunctionInfo.II_win_alloca) { if (CE->getNumArgs() < 1) return; State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State, AF_Alloca); - State = ProcessZeroAllocation(C, CE, 0, State); - } else if (isStandardNewDelete(FD, C.getASTContext())) { + State = ProcessZeroAllocCheck(C, CE, 0, State); + } else if (MemFunctionInfo.isStandardNewDelete(FD, C.getASTContext())) { // Process direct calls to operator new/new[]/delete/delete[] functions // as distinct from new/new[]/delete/delete[] expressions that are // processed by the checkPostStmt callbacks for CXXNewExpr and // CXXDeleteExpr. - OverloadedOperatorKind K = FD->getOverloadedOperator(); - if (K == OO_New) { + switch (FD->getOverloadedOperator()) { + case OO_New: State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State, AF_CXXNew); - State = ProcessZeroAllocation(C, CE, 0, State); - } - else if (K == OO_Array_New) { + State = ProcessZeroAllocCheck(C, CE, 0, State); + break; + case OO_Array_New: State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State, AF_CXXNewArray); - State = ProcessZeroAllocation(C, CE, 0, State); - } - else if (K == OO_Delete || K == OO_Array_Delete) - State = FreeMemAux(C, CE, State, 0, false, ReleasedAllocatedMemory); - else + State = ProcessZeroAllocCheck(C, CE, 0, State); + break; + case OO_Delete: + case OO_Array_Delete: + State = FreeMemAux(C, CE, State, 0, false, IsKnownToBeAllocatedMemory); + break; + default: llvm_unreachable("not a new/delete operator"); - } else if (FunI == II_if_nameindex) { + } + } else if (FunI == MemFunctionInfo.II_if_nameindex) { // Should we model this differently? We can allocate a fixed number of // elements with zeros in the last one. State = MallocMemAux(C, CE, UnknownVal(), UnknownVal(), State, AF_IfNameIndex); - } else if (FunI == II_if_freenameindex) { - State = FreeMemAux(C, CE, State, 0, false, ReleasedAllocatedMemory); - } else if (FunI == II_g_malloc0 || FunI == II_g_try_malloc0) { + } else if (FunI == MemFunctionInfo.II_if_freenameindex) { + State = FreeMemAux(C, CE, State, 0, false, IsKnownToBeAllocatedMemory); + } else if (FunI == MemFunctionInfo.II_g_malloc0 || + FunI == MemFunctionInfo.II_g_try_malloc0) { if (CE->getNumArgs() < 1) return; SValBuilder &svalBuilder = C.getSValBuilder(); SVal zeroVal = svalBuilder.makeZeroVal(svalBuilder.getContext().CharTy); State = MallocMemAux(C, CE, CE->getArg(0), zeroVal, State); - State = ProcessZeroAllocation(C, CE, 0, State); - } else if (FunI == II_g_memdup) { + State = ProcessZeroAllocCheck(C, CE, 0, State); + } else if (FunI == MemFunctionInfo.II_g_memdup) { if (CE->getNumArgs() < 2) return; State = MallocMemAux(C, CE, CE->getArg(1), UndefinedVal(), State); - State = ProcessZeroAllocation(C, CE, 1, State); - } else if (FunI == II_g_malloc_n || FunI == II_g_try_malloc_n || - FunI == II_g_malloc0_n || FunI == II_g_try_malloc0_n) { + State = ProcessZeroAllocCheck(C, CE, 1, State); + } else if (FunI == MemFunctionInfo.II_g_malloc_n || + FunI == MemFunctionInfo.II_g_try_malloc_n || + FunI == MemFunctionInfo.II_g_malloc0_n || + FunI == MemFunctionInfo.II_g_try_malloc0_n) { if (CE->getNumArgs() < 2) return; SVal Init = UndefinedVal(); - if (FunI == II_g_malloc0_n || FunI == II_g_try_malloc0_n) { + if (FunI == MemFunctionInfo.II_g_malloc0_n || + FunI == MemFunctionInfo.II_g_try_malloc0_n) { SValBuilder &SB = C.getSValBuilder(); Init = SB.makeZeroVal(SB.getContext().CharTy); } SVal TotalSize = evalMulForBufferSize(C, CE->getArg(0), CE->getArg(1)); State = MallocMemAux(C, CE, TotalSize, Init, State); - State = ProcessZeroAllocation(C, CE, 0, State); - State = ProcessZeroAllocation(C, CE, 1, State); - } else if (FunI == II_g_realloc_n || FunI == II_g_try_realloc_n) { + State = ProcessZeroAllocCheck(C, CE, 0, State); + State = ProcessZeroAllocCheck(C, CE, 1, State); + } else if (FunI == MemFunctionInfo.II_g_realloc_n || + FunI == MemFunctionInfo.II_g_try_realloc_n) { if (CE->getNumArgs() < 3) return; - State = ReallocMemAux(C, CE, false, State, true); - State = ProcessZeroAllocation(C, CE, 1, State); - State = ProcessZeroAllocation(C, CE, 2, State); + State = ReallocMemAux(C, CE, /*ShouldFreeOnFail*/ false, State, + /*SuffixWithN*/ true); + State = ProcessZeroAllocCheck(C, CE, 1, State); + State = ProcessZeroAllocCheck(C, CE, 2, State); } } - if (IsOptimistic || ChecksEnabled[CK_MismatchedDeallocatorChecker]) { + if (MemFunctionInfo.ShouldIncludeOwnershipAnnotatedFunctions || + ChecksEnabled[CK_MismatchedDeallocatorChecker]) { // Check all the attributes, if there are any. // There can be multiple of these attributes. if (FD->hasAttrs()) @@ -979,9 +1227,9 @@ void MallocChecker::checkPostStmt(const CallExpr *CE, CheckerContext &C) const { } // Performs a 0-sized allocations check. -ProgramStateRef MallocChecker::ProcessZeroAllocation( - CheckerContext &C, const Expr *E, const unsigned AllocationSizeArg, - ProgramStateRef State, Optional<SVal> RetVal) const { +ProgramStateRef MallocChecker::ProcessZeroAllocCheck( + CheckerContext &C, const Expr *E, const unsigned IndexOfSizeArg, + ProgramStateRef State, Optional<SVal> RetVal) { if (!State) return nullptr; @@ -991,7 +1239,7 @@ ProgramStateRef MallocChecker::ProcessZeroAllocation( const Expr *Arg = nullptr; if (const CallExpr *CE = dyn_cast<CallExpr>(E)) { - Arg = CE->getArg(AllocationSizeArg); + Arg = CE->getArg(IndexOfSizeArg); } else if (const CXXNewExpr *NE = dyn_cast<CXXNewExpr>(E)) { if (NE->isArray()) @@ -1053,7 +1301,9 @@ static QualType getDeepPointeeType(QualType T) { return Result; } -static bool treatUnusedNewEscaped(const CXXNewExpr *NE) { +/// \returns true if the constructor invoked by \p NE has an argument of a +/// pointer/reference to a record type. +static bool hasNonTrivialConstructorCall(const CXXNewExpr *NE) { const CXXConstructExpr *ConstructE = NE->getConstructExpr(); if (!ConstructE) @@ -1083,11 +1333,17 @@ static bool treatUnusedNewEscaped(const CXXNewExpr *NE) { void MallocChecker::processNewAllocation(const CXXNewExpr *NE, CheckerContext &C, SVal Target) const { - if (!isStandardNewDelete(NE->getOperatorNew(), C.getASTContext())) + if (!MemFunctionInfo.isStandardNewDelete(NE->getOperatorNew(), + C.getASTContext())) return; - ParentMap &PM = C.getLocationContext()->getParentMap(); - if (!PM.isConsumedExpr(NE) && treatUnusedNewEscaped(NE)) + const ParentMap &PM = C.getLocationContext()->getParentMap(); + + // Non-trivial constructors have a chance to escape 'this', but marking all + // invocations of trivial constructors as escaped would cause too great of + // reduction of true positives, so let's just do that for constructors that + // have an argument of a pointer-to-record type. + if (!PM.isConsumedExpr(NE) && hasNonTrivialConstructorCall(NE)) return; ProgramStateRef State = C.getState(); @@ -1098,7 +1354,7 @@ void MallocChecker::processNewAllocation(const CXXNewExpr *NE, State = MallocUpdateRefState(C, NE, State, NE->isArray() ? AF_CXXNewArray : AF_CXXNew, Target); State = addExtentSize(C, NE, State, Target); - State = ProcessZeroAllocation(C, NE, 0, State, Target); + State = ProcessZeroAllocCheck(C, NE, 0, State, Target); C.addTransition(State); } @@ -1132,14 +1388,13 @@ ProgramStateRef MallocChecker::addExtentSize(CheckerContext &C, // Store the extent size for the (symbolic)region // containing the elements. Region = Target.getAsRegion() - ->getAs<SubRegion>() + ->castAs<SubRegion>() ->StripCasts() - ->getAs<SubRegion>(); + ->castAs<SubRegion>(); } else { ElementCount = svalBuilder.makeIntVal(1, true); - Region = Target.getAsRegion()->getAs<SubRegion>(); + Region = Target.getAsRegion()->castAs<SubRegion>(); } - assert(Region); // Set the region's extent equal to the Size in Bytes. QualType ElementType = NE->getAllocatedType(); @@ -1167,13 +1422,14 @@ void MallocChecker::checkPreStmt(const CXXDeleteExpr *DE, if (SymbolRef Sym = C.getSVal(DE->getArgument()).getAsSymbol()) checkUseAfterFree(Sym, C, DE->getArgument()); - if (!isStandardNewDelete(DE->getOperatorDelete(), C.getASTContext())) + if (!MemFunctionInfo.isStandardNewDelete(DE->getOperatorDelete(), + C.getASTContext())) return; ProgramStateRef State = C.getState(); - bool ReleasedAllocated; + bool IsKnownToBeAllocated; State = FreeMemAux(C, DE->getArgument(), DE, State, - /*Hold*/false, ReleasedAllocated); + /*Hold*/ false, IsKnownToBeAllocated); C.addTransition(State); } @@ -1213,11 +1469,11 @@ void MallocChecker::checkPostObjCMessage(const ObjCMethodCall &Call, if (!*FreeWhenDone) return; - bool ReleasedAllocatedMemory; - ProgramStateRef State = FreeMemAux(C, Call.getArgExpr(0), - Call.getOriginExpr(), C.getState(), - /*Hold=*/true, ReleasedAllocatedMemory, - /*ReturnsNullOnFailure=*/true); + bool IsKnownToBeAllocatedMemory; + ProgramStateRef State = + FreeMemAux(C, Call.getArgExpr(0), Call.getOriginExpr(), C.getState(), + /*Hold=*/true, IsKnownToBeAllocatedMemory, + /*RetNullOnFailure=*/true); C.addTransition(State); } @@ -1229,7 +1485,7 @@ MallocChecker::MallocMemReturnsAttr(CheckerContext &C, const CallExpr *CE, if (!State) return nullptr; - if (Att->getModule() != II_malloc) + if (Att->getModule() != MemFunctionInfo.II_malloc) return nullptr; OwnershipAttr::args_iterator I = Att->args_begin(), E = Att->args_end(); @@ -1295,11 +1551,10 @@ ProgramStateRef MallocChecker::MallocMemAux(CheckerContext &C, return MallocUpdateRefState(C, CE, State, Family); } -ProgramStateRef MallocChecker::MallocUpdateRefState(CheckerContext &C, - const Expr *E, - ProgramStateRef State, - AllocationFamily Family, - Optional<SVal> RetVal) { +static ProgramStateRef MallocUpdateRefState(CheckerContext &C, const Expr *E, + ProgramStateRef State, + AllocationFamily Family, + Optional<SVal> RetVal) { if (!State) return nullptr; @@ -1327,27 +1582,24 @@ ProgramStateRef MallocChecker::FreeMemAttr(CheckerContext &C, if (!State) return nullptr; - if (Att->getModule() != II_malloc) + if (Att->getModule() != MemFunctionInfo.II_malloc) return nullptr; - bool ReleasedAllocated = false; + bool IsKnownToBeAllocated = false; for (const auto &Arg : Att->args()) { ProgramStateRef StateI = FreeMemAux( C, CE, State, Arg.getASTIndex(), - Att->getOwnKind() == OwnershipAttr::Holds, ReleasedAllocated); + Att->getOwnKind() == OwnershipAttr::Holds, IsKnownToBeAllocated); if (StateI) State = StateI; } return State; } -ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C, - const CallExpr *CE, - ProgramStateRef State, - unsigned Num, - bool Hold, - bool &ReleasedAllocated, +ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C, const CallExpr *CE, + ProgramStateRef State, unsigned Num, + bool Hold, bool &IsKnownToBeAllocated, bool ReturnsNullOnFailure) const { if (!State) return nullptr; @@ -1355,8 +1607,8 @@ ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C, if (CE->getNumArgs() < (Num + 1)) return nullptr; - return FreeMemAux(C, CE->getArg(Num), CE, State, Hold, - ReleasedAllocated, ReturnsNullOnFailure); + return FreeMemAux(C, CE->getArg(Num), CE, State, Hold, IsKnownToBeAllocated, + ReturnsNullOnFailure); } /// Checks if the previous call to free on the given symbol failed - if free @@ -1374,8 +1626,10 @@ static bool didPreviousFreeFail(ProgramStateRef State, return false; } -AllocationFamily MallocChecker::getAllocationFamily(CheckerContext &C, - const Stmt *S) const { +static AllocationFamily +getAllocationFamily(const MemFunctionInfoTy &MemFunctionInfo, CheckerContext &C, + const Stmt *S) { + if (!S) return AF_None; @@ -1387,10 +1641,11 @@ AllocationFamily MallocChecker::getAllocationFamily(CheckerContext &C, ASTContext &Ctx = C.getASTContext(); - if (isCMemFunction(FD, Ctx, AF_Malloc, MemoryOperationKind::MOK_Any)) + if (MemFunctionInfo.isCMemFunction(FD, Ctx, AF_Malloc, + MemoryOperationKind::MOK_Any)) return AF_Malloc; - if (isStandardNewDelete(FD, Ctx)) { + if (MemFunctionInfo.isStandardNewDelete(FD, Ctx)) { OverloadedOperatorKind Kind = FD->getOverloadedOperator(); if (Kind == OO_New || Kind == OO_Delete) return AF_CXXNew; @@ -1398,10 +1653,12 @@ AllocationFamily MallocChecker::getAllocationFamily(CheckerContext &C, return AF_CXXNewArray; } - if (isCMemFunction(FD, Ctx, AF_IfNameIndex, MemoryOperationKind::MOK_Any)) + if (MemFunctionInfo.isCMemFunction(FD, Ctx, AF_IfNameIndex, + MemoryOperationKind::MOK_Any)) return AF_IfNameIndex; - if (isCMemFunction(FD, Ctx, AF_Alloca, MemoryOperationKind::MOK_Any)) + if (MemFunctionInfo.isCMemFunction(FD, Ctx, AF_Alloca, + MemoryOperationKind::MOK_Any)) return AF_Alloca; return AF_None; @@ -1419,8 +1676,8 @@ AllocationFamily MallocChecker::getAllocationFamily(CheckerContext &C, return AF_None; } -bool MallocChecker::printAllocDeallocName(raw_ostream &os, CheckerContext &C, - const Expr *E) const { +static bool printAllocDeallocName(raw_ostream &os, CheckerContext &C, + const Expr *E) { if (const CallExpr *CE = dyn_cast<CallExpr>(E)) { // FIXME: This doesn't handle indirect calls. const FunctionDecl *FD = CE->getDirectCallee(); @@ -1459,9 +1716,10 @@ bool MallocChecker::printAllocDeallocName(raw_ostream &os, CheckerContext &C, return false; } -void MallocChecker::printExpectedAllocName(raw_ostream &os, CheckerContext &C, - const Expr *E) const { - AllocationFamily Family = getAllocationFamily(C, E); +static void printExpectedAllocName(raw_ostream &os, + const MemFunctionInfoTy &MemFunctionInfo, + CheckerContext &C, const Expr *E) { + AllocationFamily Family = getAllocationFamily(MemFunctionInfo, C, E); switch(Family) { case AF_Malloc: os << "malloc()"; return; @@ -1474,8 +1732,7 @@ void MallocChecker::printExpectedAllocName(raw_ostream &os, CheckerContext &C, } } -void MallocChecker::printExpectedDeallocName(raw_ostream &os, - AllocationFamily Family) const { +static void printExpectedDeallocName(raw_ostream &os, AllocationFamily Family) { switch(Family) { case AF_Malloc: os << "free()"; return; case AF_CXXNew: os << "'delete'"; return; @@ -1490,9 +1747,8 @@ void MallocChecker::printExpectedDeallocName(raw_ostream &os, ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C, const Expr *ArgExpr, const Expr *ParentExpr, - ProgramStateRef State, - bool Hold, - bool &ReleasedAllocated, + ProgramStateRef State, bool Hold, + bool &IsKnownToBeAllocated, bool ReturnsNullOnFailure) const { if (!State) @@ -1566,6 +1822,9 @@ ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C, const RefState *RsBase = State->get<RegionState>(SymBase); SymbolRef PreviousRetStatusSymbol = nullptr; + IsKnownToBeAllocated = + RsBase && (RsBase->isAllocated() || RsBase->isAllocatedOfSizeZero()); + if (RsBase) { // Memory returned by alloca() shouldn't be freed. @@ -1588,7 +1847,8 @@ ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C, // Check if an expected deallocation function matches the real one. bool DeallocMatchesAlloc = - RsBase->getAllocationFamily() == getAllocationFamily(C, ParentExpr); + RsBase->getAllocationFamily() == + getAllocationFamily(MemFunctionInfo, C, ParentExpr); if (!DeallocMatchesAlloc) { ReportMismatchedDealloc(C, ArgExpr->getSourceRange(), ParentExpr, RsBase, SymBase, Hold); @@ -1614,9 +1874,6 @@ ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C, return nullptr; } - ReleasedAllocated = (RsBase != nullptr) && (RsBase->isAllocated() || - RsBase->isAllocatedOfSizeZero()); - // Clean out the info on previous call to free return info. State = State->remove<FreeReturnValue>(SymBase); @@ -1631,8 +1888,9 @@ ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C, } } - AllocationFamily Family = RsBase ? RsBase->getAllocationFamily() - : getAllocationFamily(C, ParentExpr); + AllocationFamily Family = + RsBase ? RsBase->getAllocationFamily() + : getAllocationFamily(MemFunctionInfo, C, ParentExpr); // Normal free. if (Hold) return State->set<RegionState>(SymBase, @@ -1682,8 +1940,8 @@ Optional<MallocChecker::CheckKind> MallocChecker::getCheckIfTracked(CheckerContext &C, const Stmt *AllocDeallocStmt, bool IsALeakCheck) const { - return getCheckIfTracked(getAllocationFamily(C, AllocDeallocStmt), - IsALeakCheck); + return getCheckIfTracked( + getAllocationFamily(MemFunctionInfo, C, AllocDeallocStmt), IsALeakCheck); } Optional<MallocChecker::CheckKind> @@ -1821,9 +2079,10 @@ void MallocChecker::ReportBadFree(CheckerContext &C, SVal ArgVal, else os << "not memory allocated by "; - printExpectedAllocName(os, C, DeallocExpr); + printExpectedAllocName(os, MemFunctionInfo, C, DeallocExpr); - auto R = llvm::make_unique<BugReport>(*BT_BadFree[*CheckKind], os.str(), N); + auto R = std::make_unique<PathSensitiveBugReport>(*BT_BadFree[*CheckKind], + os.str(), N); R->markInteresting(MR); R->addRange(Range); C.emitReport(std::move(R)); @@ -1847,7 +2106,7 @@ void MallocChecker::ReportFreeAlloca(CheckerContext &C, SVal ArgVal, BT_FreeAlloca[*CheckKind].reset(new BugType( CheckNames[*CheckKind], "Free alloca()", categories::MemoryError)); - auto R = llvm::make_unique<BugReport>( + auto R = std::make_unique<PathSensitiveBugReport>( *BT_FreeAlloca[*CheckKind], "Memory allocated by alloca() should not be deallocated", N); R->markInteresting(ArgVal.getAsRegion()); @@ -1903,10 +2162,11 @@ void MallocChecker::ReportMismatchedDealloc(CheckerContext &C, os << ", not " << DeallocOs.str(); } - auto R = llvm::make_unique<BugReport>(*BT_MismatchedDealloc, os.str(), N); + auto R = std::make_unique<PathSensitiveBugReport>(*BT_MismatchedDealloc, + os.str(), N); R->markInteresting(Sym); R->addRange(Range); - R->addVisitor(llvm::make_unique<MallocBugVisitor>(Sym)); + R->addVisitor(std::make_unique<MallocBugVisitor>(Sym)); C.emitReport(std::move(R)); } } @@ -1962,7 +2222,8 @@ void MallocChecker::ReportOffsetFree(CheckerContext &C, SVal ArgVal, else os << "allocated memory"; - auto R = llvm::make_unique<BugReport>(*BT_OffsetFree[*CheckKind], os.str(), N); + auto R = std::make_unique<PathSensitiveBugReport>(*BT_OffsetFree[*CheckKind], + os.str(), N); R->markInteresting(MR->getBaseRegion()); R->addRange(Range); C.emitReport(std::move(R)); @@ -1988,15 +2249,16 @@ void MallocChecker::ReportUseAfterFree(CheckerContext &C, SourceRange Range, AllocationFamily AF = C.getState()->get<RegionState>(Sym)->getAllocationFamily(); - auto R = llvm::make_unique<BugReport>(*BT_UseFree[*CheckKind], + auto R = std::make_unique<PathSensitiveBugReport>( + *BT_UseFree[*CheckKind], AF == AF_InnerBuffer - ? "Inner pointer of container used after re/deallocation" - : "Use of memory after it is freed", + ? "Inner pointer of container used after re/deallocation" + : "Use of memory after it is freed", N); R->markInteresting(Sym); R->addRange(Range); - R->addVisitor(llvm::make_unique<MallocBugVisitor>(Sym)); + R->addVisitor(std::make_unique<MallocBugVisitor>(Sym)); if (AF == AF_InnerBuffer) R->addVisitor(allocation_state::getInnerPointerBRVisitor(Sym)); @@ -2022,7 +2284,7 @@ void MallocChecker::ReportDoubleFree(CheckerContext &C, SourceRange Range, BT_DoubleFree[*CheckKind].reset(new BugType( CheckNames[*CheckKind], "Double free", categories::MemoryError)); - auto R = llvm::make_unique<BugReport>( + auto R = std::make_unique<PathSensitiveBugReport>( *BT_DoubleFree[*CheckKind], (Released ? "Attempt to free released memory" : "Attempt to free non-owned memory"), @@ -2031,7 +2293,7 @@ void MallocChecker::ReportDoubleFree(CheckerContext &C, SourceRange Range, R->markInteresting(Sym); if (PrevSym) R->markInteresting(PrevSym); - R->addVisitor(llvm::make_unique<MallocBugVisitor>(Sym)); + R->addVisitor(std::make_unique<MallocBugVisitor>(Sym)); C.emitReport(std::move(R)); } } @@ -2051,11 +2313,11 @@ void MallocChecker::ReportDoubleDelete(CheckerContext &C, SymbolRef Sym) const { "Double delete", categories::MemoryError)); - auto R = llvm::make_unique<BugReport>( + auto R = std::make_unique<PathSensitiveBugReport>( *BT_DoubleDelete, "Attempt to delete released memory", N); R->markInteresting(Sym); - R->addVisitor(llvm::make_unique<MallocBugVisitor>(Sym)); + R->addVisitor(std::make_unique<MallocBugVisitor>(Sym)); C.emitReport(std::move(R)); } } @@ -2079,13 +2341,13 @@ void MallocChecker::ReportUseZeroAllocated(CheckerContext &C, new BugType(CheckNames[*CheckKind], "Use of zero allocated", categories::MemoryError)); - auto R = llvm::make_unique<BugReport>(*BT_UseZerroAllocated[*CheckKind], - "Use of zero-allocated memory", N); + auto R = std::make_unique<PathSensitiveBugReport>( + *BT_UseZerroAllocated[*CheckKind], "Use of zero-allocated memory", N); R->addRange(Range); if (Sym) { R->markInteresting(Sym); - R->addVisitor(llvm::make_unique<MallocBugVisitor>(Sym)); + R->addVisitor(std::make_unique<MallocBugVisitor>(Sym)); } C.emitReport(std::move(R)); } @@ -2119,7 +2381,8 @@ void MallocChecker::ReportFunctionPointerFree(CheckerContext &C, SVal ArgVal, Os << " is a function pointer"; - auto R = llvm::make_unique<BugReport>(*BT_BadFree[*CheckKind], Os.str(), N); + auto R = std::make_unique<PathSensitiveBugReport>(*BT_BadFree[*CheckKind], + Os.str(), N); R->markInteresting(MR); R->addRange(Range); C.emitReport(std::move(R)); @@ -2128,7 +2391,7 @@ void MallocChecker::ReportFunctionPointerFree(CheckerContext &C, SVal ArgVal, ProgramStateRef MallocChecker::ReallocMemAux(CheckerContext &C, const CallExpr *CE, - bool FreesOnFail, + bool ShouldFreeOnFail, ProgramStateRef State, bool SuffixWithN) const { if (!State) @@ -2193,33 +2456,32 @@ ProgramStateRef MallocChecker::ReallocMemAux(CheckerContext &C, if (!FromPtr || !ToPtr) return nullptr; - bool ReleasedAllocated = false; + bool IsKnownToBeAllocated = false; // If the size is 0, free the memory. if (SizeIsZero) - if (ProgramStateRef stateFree = FreeMemAux(C, CE, StateSizeIsZero, 0, - false, ReleasedAllocated)){ - // The semantics of the return value are: - // If size was equal to 0, either NULL or a pointer suitable to be passed - // to free() is returned. We just free the input pointer and do not add - // any constrains on the output pointer. + // The semantics of the return value are: + // If size was equal to 0, either NULL or a pointer suitable to be passed + // to free() is returned. We just free the input pointer and do not add + // any constrains on the output pointer. + if (ProgramStateRef stateFree = + FreeMemAux(C, CE, StateSizeIsZero, 0, false, IsKnownToBeAllocated)) return stateFree; - } // Default behavior. if (ProgramStateRef stateFree = - FreeMemAux(C, CE, State, 0, false, ReleasedAllocated)) { + FreeMemAux(C, CE, State, 0, false, IsKnownToBeAllocated)) { ProgramStateRef stateRealloc = MallocMemAux(C, CE, TotalSize, UnknownVal(), stateFree); if (!stateRealloc) return nullptr; - ReallocPairKind Kind = RPToBeFreedAfterFailure; - if (FreesOnFail) - Kind = RPIsFreeOnFailure; - else if (!ReleasedAllocated) - Kind = RPDoNotTrackAfterFailure; + OwnershipAfterReallocKind Kind = OAR_ToBeFreedAfterFailure; + if (ShouldFreeOnFail) + Kind = OAR_FreeOnFailure; + else if (!IsKnownToBeAllocated) + Kind = OAR_DoNotTrackAfterFailure; // Record the info about the reallocated symbol so that we could properly // process failed reallocation. @@ -2247,9 +2509,9 @@ ProgramStateRef MallocChecker::CallocMem(CheckerContext &C, const CallExpr *CE, return MallocMemAux(C, CE, TotalSize, zeroVal, State); } -LeakInfo -MallocChecker::getAllocationSite(const ExplodedNode *N, SymbolRef Sym, - CheckerContext &C) const { +MallocChecker::LeakInfo MallocChecker::getAllocationSite(const ExplodedNode *N, + SymbolRef Sym, + CheckerContext &C) { const LocationContext *LeakContext = N->getLocationContext(); // Walk the ExplodedGraph backwards and find the first node that referred to // the tracked symbol. @@ -2329,7 +2591,7 @@ void MallocChecker::reportLeak(SymbolRef Sym, ExplodedNode *N, const MemRegion *Region = nullptr; std::tie(AllocNode, Region) = getAllocationSite(N, Sym, C); - const Stmt *AllocationStmt = PathDiagnosticLocation::getStmt(AllocNode); + const Stmt *AllocationStmt = AllocNode->getStmtForDiagnostics(); if (AllocationStmt) LocUsedForUniqueing = PathDiagnosticLocation::createBegin(AllocationStmt, C.getSourceManager(), @@ -2344,11 +2606,11 @@ void MallocChecker::reportLeak(SymbolRef Sym, ExplodedNode *N, os << "Potential memory leak"; } - auto R = llvm::make_unique<BugReport>( + auto R = std::make_unique<PathSensitiveBugReport>( *BT_Leak[*CheckKind], os.str(), N, LocUsedForUniqueing, AllocNode->getLocationContext()->getDecl()); R->markInteresting(Sym); - R->addVisitor(llvm::make_unique<MallocBugVisitor>(Sym, true)); + R->addVisitor(std::make_unique<MallocBugVisitor>(Sym, true)); C.emitReport(std::move(R)); } @@ -2430,9 +2692,10 @@ void MallocChecker::checkPreCall(const CallEvent &Call, ASTContext &Ctx = C.getASTContext(); if (ChecksEnabled[CK_MallocChecker] && - (isCMemFunction(FD, Ctx, AF_Malloc, MemoryOperationKind::MOK_Free) || - isCMemFunction(FD, Ctx, AF_IfNameIndex, - MemoryOperationKind::MOK_Free))) + (MemFunctionInfo.isCMemFunction(FD, Ctx, AF_Malloc, + MemoryOperationKind::MOK_Free) || + MemFunctionInfo.isCMemFunction(FD, Ctx, AF_IfNameIndex, + MemoryOperationKind::MOK_Free))) return; } @@ -2535,7 +2798,7 @@ void MallocChecker::checkPostStmt(const BlockExpr *BE, C.addTransition(state); } -bool MallocChecker::isReleased(SymbolRef Sym, CheckerContext &C) const { +static bool isReleased(SymbolRef Sym, CheckerContext &C) { assert(Sym); const RefState *RS = C.getState()->get<RegionState>(Sym); return (RS && RS->isReleased()); @@ -2640,13 +2903,17 @@ ProgramStateRef MallocChecker::evalAssume(ProgramStateRef state, SymbolRef ReallocSym = I.getData().ReallocatedSym; if (const RefState *RS = state->get<RegionState>(ReallocSym)) { if (RS->isReleased()) { - if (I.getData().Kind == RPToBeFreedAfterFailure) + switch (I.getData().Kind) { + case OAR_ToBeFreedAfterFailure: state = state->set<RegionState>(ReallocSym, RefState::getAllocated(RS->getAllocationFamily(), RS->getStmt())); - else if (I.getData().Kind == RPDoNotTrackAfterFailure) + break; + case OAR_DoNotTrackAfterFailure: state = state->remove<RegionState>(ReallocSym); - else - assert(I.getData().Kind == RPIsFreeOnFailure); + break; + default: + assert(I.getData().Kind == OAR_FreeOnFailure); + } } } state = state->remove<ReallocPairs>(I.getKey()); @@ -2729,7 +2996,7 @@ bool MallocChecker::mayFreeAnyEscapedMemoryOrIsModeledExplicitly( // If it's one of the allocation functions we can reason about, we model // its behavior explicitly. - if (isMemFunction(FD, ASTC)) + if (MemFunctionInfo.isMemFunction(FD, ASTC)) return false; // If it's not a system call, assume it frees memory. @@ -2821,35 +3088,32 @@ bool MallocChecker::mayFreeAnyEscapedMemoryOrIsModeledExplicitly( return false; } -static bool retTrue(const RefState *RS) { - return true; -} - -static bool checkIfNewOrNewArrayFamily(const RefState *RS) { - return (RS->getAllocationFamily() == AF_CXXNewArray || - RS->getAllocationFamily() == AF_CXXNew); -} - ProgramStateRef MallocChecker::checkPointerEscape(ProgramStateRef State, const InvalidatedSymbols &Escaped, const CallEvent *Call, PointerEscapeKind Kind) const { - return checkPointerEscapeAux(State, Escaped, Call, Kind, &retTrue); + return checkPointerEscapeAux(State, Escaped, Call, Kind, + /*IsConstPointerEscape*/ false); } ProgramStateRef MallocChecker::checkConstPointerEscape(ProgramStateRef State, const InvalidatedSymbols &Escaped, const CallEvent *Call, PointerEscapeKind Kind) const { + // If a const pointer escapes, it may not be freed(), but it could be deleted. return checkPointerEscapeAux(State, Escaped, Call, Kind, - &checkIfNewOrNewArrayFamily); + /*IsConstPointerEscape*/ true); } -ProgramStateRef MallocChecker::checkPointerEscapeAux(ProgramStateRef State, - const InvalidatedSymbols &Escaped, - const CallEvent *Call, - PointerEscapeKind Kind, - bool(*CheckRefState)(const RefState*)) const { +static bool checkIfNewOrNewArrayFamily(const RefState *RS) { + return (RS->getAllocationFamily() == AF_CXXNewArray || + RS->getAllocationFamily() == AF_CXXNew); +} + +ProgramStateRef MallocChecker::checkPointerEscapeAux( + ProgramStateRef State, const InvalidatedSymbols &Escaped, + const CallEvent *Call, PointerEscapeKind Kind, + bool IsConstPointerEscape) const { // If we know that the call does not free memory, or we want to process the // call later, keep tracking the top level arguments. SymbolRef EscapingSymbol = nullptr; @@ -2868,12 +3132,10 @@ ProgramStateRef MallocChecker::checkPointerEscapeAux(ProgramStateRef State, if (EscapingSymbol && EscapingSymbol != sym) continue; - if (const RefState *RS = State->get<RegionState>(sym)) { - if ((RS->isAllocated() || RS->isAllocatedOfSizeZero()) && - CheckRefState(RS)) { - State = State->set<RegionState>(sym, RefState::getEscaped(RS)); - } - } + if (const RefState *RS = State->get<RegionState>(sym)) + if (RS->isAllocated() || RS->isAllocatedOfSizeZero()) + if (!IsConstPointerEscape || checkIfNewOrNewArrayFamily(RS)) + State = State->set<RegionState>(sym, RefState::getEscaped(RS)); } return State; } @@ -2883,9 +3145,8 @@ static SymbolRef findFailedReallocSymbol(ProgramStateRef currState, ReallocPairsTy currMap = currState->get<ReallocPairs>(); ReallocPairsTy prevMap = prevState->get<ReallocPairs>(); - for (ReallocPairsTy::iterator I = prevMap.begin(), E = prevMap.end(); - I != E; ++I) { - SymbolRef sym = I.getKey(); + for (const ReallocPairsTy::value_type &Pair : prevMap) { + SymbolRef sym = Pair.first; if (!currMap.lookup(sym)) return sym; } @@ -2906,19 +3167,19 @@ static bool isReferenceCountingPointerDestructor(const CXXDestructorDecl *DD) { return false; } -std::shared_ptr<PathDiagnosticPiece> MallocChecker::MallocBugVisitor::VisitNode( - const ExplodedNode *N, BugReporterContext &BRC, BugReport &BR) { - +PathDiagnosticPieceRef MallocBugVisitor::VisitNode(const ExplodedNode *N, + BugReporterContext &BRC, + PathSensitiveBugReport &BR) { ProgramStateRef state = N->getState(); ProgramStateRef statePrev = N->getFirstPred()->getState(); - const RefState *RS = state->get<RegionState>(Sym); + const RefState *RSCurr = state->get<RegionState>(Sym); const RefState *RSPrev = statePrev->get<RegionState>(Sym); - const Stmt *S = PathDiagnosticLocation::getStmt(N); + const Stmt *S = N->getStmtForDiagnostics(); // When dealing with containers, we sometimes want to give a note // even if the statement is missing. - if (!S && (!RS || RS->getAllocationFamily() != AF_InnerBuffer)) + if (!S && (!RSCurr || RSCurr->getAllocationFamily() != AF_InnerBuffer)) return nullptr; const LocationContext *CurrentLC = N->getLocationContext(); @@ -2948,17 +3209,17 @@ std::shared_ptr<PathDiagnosticPiece> MallocChecker::MallocBugVisitor::VisitNode( // Find out if this is an interesting point and what is the kind. StringRef Msg; - StackHintGeneratorForSymbol *StackHint = nullptr; + std::unique_ptr<StackHintGeneratorForSymbol> StackHint = nullptr; SmallString<256> Buf; llvm::raw_svector_ostream OS(Buf); if (Mode == Normal) { - if (isAllocated(RS, RSPrev, S)) { + if (isAllocated(RSCurr, RSPrev, S)) { Msg = "Memory is allocated"; - StackHint = new StackHintGeneratorForSymbol(Sym, - "Returned allocated memory"); - } else if (isReleased(RS, RSPrev, S)) { - const auto Family = RS->getAllocationFamily(); + StackHint = std::make_unique<StackHintGeneratorForSymbol>( + Sym, "Returned allocated memory"); + } else if (isReleased(RSCurr, RSPrev, S)) { + const auto Family = RSCurr->getAllocationFamily(); switch (Family) { case AF_Alloca: case AF_Malloc: @@ -2966,8 +3227,8 @@ std::shared_ptr<PathDiagnosticPiece> MallocChecker::MallocBugVisitor::VisitNode( case AF_CXXNewArray: case AF_IfNameIndex: Msg = "Memory is released"; - StackHint = new StackHintGeneratorForSymbol(Sym, - "Returning; memory was released"); + StackHint = std::make_unique<StackHintGeneratorForSymbol>( + Sym, "Returning; memory was released"); break; case AF_InnerBuffer: { const MemRegion *ObjRegion = @@ -2978,11 +3239,11 @@ std::shared_ptr<PathDiagnosticPiece> MallocChecker::MallocBugVisitor::VisitNode( if (N->getLocation().getKind() == ProgramPoint::PostImplicitCallKind) { OS << "deallocated by call to destructor"; - StackHint = new StackHintGeneratorForSymbol(Sym, - "Returning; inner buffer was deallocated"); + StackHint = std::make_unique<StackHintGeneratorForSymbol>( + Sym, "Returning; inner buffer was deallocated"); } else { OS << "reallocated by call to '"; - const Stmt *S = RS->getStmt(); + const Stmt *S = RSCurr->getStmt(); if (const auto *MemCallE = dyn_cast<CXXMemberCallExpr>(S)) { OS << MemCallE->getMethodDecl()->getNameAsString(); } else if (const auto *OpCallE = dyn_cast<CXXOperatorCallExpr>(S)) { @@ -2994,8 +3255,8 @@ std::shared_ptr<PathDiagnosticPiece> MallocChecker::MallocBugVisitor::VisitNode( OS << (D ? D->getNameAsString() : "unknown"); } OS << "'"; - StackHint = new StackHintGeneratorForSymbol(Sym, - "Returning; inner buffer was reallocated"); + StackHint = std::make_unique<StackHintGeneratorForSymbol>( + Sym, "Returning; inner buffer was reallocated"); } Msg = OS.str(); break; @@ -3033,14 +3294,14 @@ std::shared_ptr<PathDiagnosticPiece> MallocChecker::MallocBugVisitor::VisitNode( } } } - } else if (isRelinquished(RS, RSPrev, S)) { + } else if (isRelinquished(RSCurr, RSPrev, S)) { Msg = "Memory ownership is transferred"; - StackHint = new StackHintGeneratorForSymbol(Sym, ""); - } else if (isReallocFailedCheck(RS, RSPrev, S)) { + StackHint = std::make_unique<StackHintGeneratorForSymbol>(Sym, ""); + } else if (hasReallocFailed(RSCurr, RSPrev, S)) { Mode = ReallocationFailed; Msg = "Reallocation failed"; - StackHint = new StackHintGeneratorForReallocationFailed(Sym, - "Reallocation failed"); + StackHint = std::make_unique<StackHintGeneratorForReallocationFailed>( + Sym, "Reallocation failed"); if (SymbolRef sym = findFailedReallocSymbol(state, statePrev)) { // Is it possible to fail two reallocs WITHOUT testing in between? @@ -3059,21 +3320,24 @@ std::shared_ptr<PathDiagnosticPiece> MallocChecker::MallocBugVisitor::VisitNode( if (!statePrev->get<RegionState>(FailedReallocSymbol)) { // We're at the reallocation point. Msg = "Attempt to reallocate memory"; - StackHint = new StackHintGeneratorForSymbol(Sym, - "Returned reallocated memory"); + StackHint = std::make_unique<StackHintGeneratorForSymbol>( + Sym, "Returned reallocated memory"); FailedReallocSymbol = nullptr; Mode = Normal; } } - if (Msg.empty()) + if (Msg.empty()) { + assert(!StackHint); return nullptr; + } + assert(StackHint); // Generate the extra diagnostic. PathDiagnosticLocation Pos; if (!S) { - assert(RS->getAllocationFamily() == AF_InnerBuffer); + assert(RSCurr->getAllocationFamily() == AF_InnerBuffer); auto PostImplCall = N->getLocation().getAs<PostImplicitCall>(); if (!PostImplCall) return nullptr; @@ -3084,7 +3348,9 @@ std::shared_ptr<PathDiagnosticPiece> MallocChecker::MallocBugVisitor::VisitNode( N->getLocationContext()); } - return std::make_shared<PathDiagnosticEventPiece>(Pos, Msg, true, StackHint); + auto P = std::make_shared<PathDiagnosticEventPiece>(Pos, Msg, true); + BR.addCallStackHint(P, std::move(StackHint)); + return P; } void MallocChecker::printState(raw_ostream &Out, ProgramStateRef State, @@ -3131,13 +3397,13 @@ void ento::registerInnerPointerCheckerAux(CheckerManager &mgr) { MallocChecker *checker = mgr.getChecker<MallocChecker>(); checker->ChecksEnabled[MallocChecker::CK_InnerPointerChecker] = true; checker->CheckNames[MallocChecker::CK_InnerPointerChecker] = - mgr.getCurrentCheckName(); + mgr.getCurrentCheckerName(); } void ento::registerDynamicMemoryModeling(CheckerManager &mgr) { auto *checker = mgr.registerChecker<MallocChecker>(); - checker->IsOptimistic = mgr.getAnalyzerOptions().getCheckerBooleanOption( - checker, "Optimistic"); + checker->MemFunctionInfo.ShouldIncludeOwnershipAnnotatedFunctions = + mgr.getAnalyzerOptions().getCheckerBooleanOption(checker, "Optimistic"); } bool ento::shouldRegisterDynamicMemoryModeling(const LangOptions &LO) { @@ -3148,12 +3414,11 @@ bool ento::shouldRegisterDynamicMemoryModeling(const LangOptions &LO) { void ento::register##name(CheckerManager &mgr) { \ MallocChecker *checker = mgr.getChecker<MallocChecker>(); \ checker->ChecksEnabled[MallocChecker::CK_##name] = true; \ - checker->CheckNames[MallocChecker::CK_##name] = mgr.getCurrentCheckName(); \ + checker->CheckNames[MallocChecker::CK_##name] = \ + mgr.getCurrentCheckerName(); \ } \ \ - bool ento::shouldRegister##name(const LangOptions &LO) { \ - return true; \ - } + bool ento::shouldRegister##name(const LangOptions &LO) { return true; } REGISTER_CHECKER(MallocChecker) REGISTER_CHECKER(NewDeleteChecker) diff --git a/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp b/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp index 2eb4d7141e284..b5881a9e65338 100644 --- a/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp @@ -183,7 +183,7 @@ public: QualType CastedType = i->CastedExpr->getType(); if (!CastedType->isPointerType()) continue; - QualType PointeeType = CastedType->getAs<PointerType>()->getPointeeType(); + QualType PointeeType = CastedType->getPointeeType(); if (PointeeType->isVoidType()) continue; diff --git a/lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp b/lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp index 270efede83858..ceea62160545e 100644 --- a/lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp @@ -67,7 +67,7 @@ void MmapWriteExecChecker::checkPreCall(const CallEvent &Call, if (!N) return; - auto Report = llvm::make_unique<BugReport>( + auto Report = std::make_unique<PathSensitiveBugReport>( *BT, "Both PROT_WRITE and PROT_EXEC flags are set. This can " "lead to exploitable memory regions, which could be overwritten " "with malicious code", N); diff --git a/lib/StaticAnalyzer/Checkers/MoveChecker.cpp b/lib/StaticAnalyzer/Checkers/MoveChecker.cpp index d8a9af78536a0..1473c05d7e3fc 100644 --- a/lib/StaticAnalyzer/Checkers/MoveChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/MoveChecker.cpp @@ -169,9 +169,9 @@ private: // in the first place. } - std::shared_ptr<PathDiagnosticPiece> VisitNode(const ExplodedNode *N, - BugReporterContext &BRC, - BugReport &BR) override; + PathDiagnosticPieceRef VisitNode(const ExplodedNode *N, + BugReporterContext &BRC, + PathSensitiveBugReport &BR) override; private: const MoveChecker &Chk; @@ -270,9 +270,10 @@ static const MemRegion *unwrapRValueReferenceIndirection(const MemRegion *MR) { return MR; } -std::shared_ptr<PathDiagnosticPiece> +PathDiagnosticPieceRef MoveChecker::MovedBugVisitor::VisitNode(const ExplodedNode *N, - BugReporterContext &BRC, BugReport &BR) { + BugReporterContext &BRC, + PathSensitiveBugReport &BR) { // We need only the last move of the reported object's region. // The visitor walks the ExplodedGraph backwards. if (Found) @@ -288,7 +289,7 @@ MoveChecker::MovedBugVisitor::VisitNode(const ExplodedNode *N, return nullptr; // Retrieve the associated statement. - const Stmt *S = PathDiagnosticLocation::getStmt(N); + const Stmt *S = N->getStmtForDiagnostics(); if (!S) return nullptr; Found = true; @@ -400,7 +401,7 @@ ExplodedNode *MoveChecker::reportBug(const MemRegion *Region, PathDiagnosticLocation LocUsedForUniqueing; const ExplodedNode *MoveNode = getMoveLocation(N, Region, C); - if (const Stmt *MoveStmt = PathDiagnosticLocation::getStmt(MoveNode)) + if (const Stmt *MoveStmt = MoveNode->getStmtForDiagnostics()) LocUsedForUniqueing = PathDiagnosticLocation::createBegin( MoveStmt, C.getSourceManager(), MoveNode->getLocationContext()); @@ -428,10 +429,10 @@ ExplodedNode *MoveChecker::reportBug(const MemRegion *Region, break; } - auto R = - llvm::make_unique<BugReport>(*BT, OS.str(), N, LocUsedForUniqueing, - MoveNode->getLocationContext()->getDecl()); - R->addVisitor(llvm::make_unique<MovedBugVisitor>(*this, Region, RD, MK)); + auto R = std::make_unique<PathSensitiveBugReport>( + *BT, OS.str(), N, LocUsedForUniqueing, + MoveNode->getLocationContext()->getDecl()); + R->addVisitor(std::make_unique<MovedBugVisitor>(*this, Region, RD, MK)); C.emitReport(std::move(R)); return N; } diff --git a/lib/StaticAnalyzer/Checkers/NSAutoreleasePoolChecker.cpp b/lib/StaticAnalyzer/Checkers/NSAutoreleasePoolChecker.cpp index 6fc7c17bc42fb..41b7fe5e43b6f 100644 --- a/lib/StaticAnalyzer/Checkers/NSAutoreleasePoolChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/NSAutoreleasePoolChecker.cpp @@ -67,9 +67,11 @@ void NSAutoreleasePoolChecker::checkPreObjCMessage(const ObjCMethodCall &msg, return; } - auto Report = llvm::make_unique<BugReport>( - *BT, "Use -drain instead of -release when using NSAutoreleasePool and " - "garbage collection", N); + auto Report = std::make_unique<PathSensitiveBugReport>( + *BT, + "Use -drain instead of -release when using NSAutoreleasePool and " + "garbage collection", + N); Report->addRange(msg.getSourceRange()); C.emitReport(std::move(Report)); } diff --git a/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp b/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp index 5cec012258c15..85370bf133cd7 100644 --- a/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp @@ -273,7 +273,8 @@ void NSOrCFErrorDerefChecker::checkEvent(ImplicitNullDerefEvent event) const { CFBT.reset(new CFErrorDerefBug(this)); bug = CFBT.get(); } - BR.emitReport(llvm::make_unique<BugReport>(*bug, os.str(), event.SinkNode)); + BR.emitReport( + std::make_unique<PathSensitiveBugReport>(*bug, os.str(), event.SinkNode)); } static bool IsNSError(QualType T, IdentifierInfo *II) { diff --git a/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp b/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp index bf6b3e3e87cf8..6ffc89745365c 100644 --- a/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp @@ -35,9 +35,11 @@ public: void checkPreCall(const CallEvent &Call, CheckerContext &C) const; - std::unique_ptr<BugReport> - genReportNullAttrNonNull(const ExplodedNode *ErrorN, const Expr *ArgE) const; - std::unique_ptr<BugReport> + std::unique_ptr<PathSensitiveBugReport> + genReportNullAttrNonNull(const ExplodedNode *ErrorN, + const Expr *ArgE, + unsigned IdxOfArg) const; + std::unique_ptr<PathSensitiveBugReport> genReportReferenceToNullPointer(const ExplodedNode *ErrorN, const Expr *ArgE) const; }; @@ -143,7 +145,7 @@ void NonNullParamChecker::checkPreCall(const CallEvent &Call, std::unique_ptr<BugReport> R; if (haveAttrNonNull) - R = genReportNullAttrNonNull(errorNode, ArgE); + R = genReportNullAttrNonNull(errorNode, ArgE, idx + 1); else if (haveRefTypeParam) R = genReportReferenceToNullPointer(errorNode, ArgE); @@ -177,9 +179,10 @@ void NonNullParamChecker::checkPreCall(const CallEvent &Call, C.addTransition(state); } -std::unique_ptr<BugReport> +std::unique_ptr<PathSensitiveBugReport> NonNullParamChecker::genReportNullAttrNonNull(const ExplodedNode *ErrorNode, - const Expr *ArgE) const { + const Expr *ArgE, + unsigned IdxOfArg) const { // Lazily allocate the BugType object if it hasn't already been // created. Ownership is transferred to the BugReporter object once // the BugReport is passed to 'EmitWarning'. @@ -187,21 +190,27 @@ NonNullParamChecker::genReportNullAttrNonNull(const ExplodedNode *ErrorNode, BTAttrNonNull.reset(new BugType( this, "Argument with 'nonnull' attribute passed null", "API")); - auto R = llvm::make_unique<BugReport>( - *BTAttrNonNull, - "Null pointer passed as an argument to a 'nonnull' parameter", ErrorNode); + llvm::SmallString<256> SBuf; + llvm::raw_svector_ostream OS(SBuf); + OS << "Null pointer passed to " + << IdxOfArg << llvm::getOrdinalSuffix(IdxOfArg) + << " parameter expecting 'nonnull'"; + + auto R = + std::make_unique<PathSensitiveBugReport>(*BTAttrNonNull, SBuf, ErrorNode); if (ArgE) bugreporter::trackExpressionValue(ErrorNode, ArgE, *R); return R; } -std::unique_ptr<BugReport> NonNullParamChecker::genReportReferenceToNullPointer( +std::unique_ptr<PathSensitiveBugReport> +NonNullParamChecker::genReportReferenceToNullPointer( const ExplodedNode *ErrorNode, const Expr *ArgE) const { if (!BTNullRefArg) BTNullRefArg.reset(new BuiltinBug(this, "Dereference of null pointer")); - auto R = llvm::make_unique<BugReport>( + auto R = std::make_unique<PathSensitiveBugReport>( *BTNullRefArg, "Forming reference to null pointer", ErrorNode); if (ArgE) { const Expr *ArgEDeref = bugreporter::getDerefExpr(ArgE); diff --git a/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp b/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp index af21c84b995b4..4322ac207112c 100644 --- a/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp @@ -112,11 +112,11 @@ public: DefaultBool CheckNullablePassedToNonnull; DefaultBool CheckNullableReturnedFromNonnull; - CheckName CheckNameNullPassedToNonnull; - CheckName CheckNameNullReturnedFromNonnull; - CheckName CheckNameNullableDereferenced; - CheckName CheckNameNullablePassedToNonnull; - CheckName CheckNameNullableReturnedFromNonnull; + CheckerNameRef CheckNameNullPassedToNonnull; + CheckerNameRef CheckNameNullReturnedFromNonnull; + CheckerNameRef CheckNameNullableDereferenced; + CheckerNameRef CheckNameNullablePassedToNonnull; + CheckerNameRef CheckNameNullableReturnedFromNonnull; }; NullabilityChecksFilter Filter; @@ -137,9 +137,9 @@ private: ID.AddPointer(Region); } - std::shared_ptr<PathDiagnosticPiece> VisitNode(const ExplodedNode *N, - BugReporterContext &BRC, - BugReport &BR) override; + PathDiagnosticPieceRef VisitNode(const ExplodedNode *N, + BugReporterContext &BRC, + PathSensitiveBugReport &BR) override; private: // The tracked region. @@ -163,10 +163,10 @@ private: if (!BT) BT.reset(new BugType(this, "Nullability", categories::MemoryError)); - auto R = llvm::make_unique<BugReport>(*BT, Msg, N); + auto R = std::make_unique<PathSensitiveBugReport>(*BT, Msg, N); if (Region) { R->markInteresting(Region); - R->addVisitor(llvm::make_unique<NullabilityBugVisitor>(Region)); + R->addVisitor(std::make_unique<NullabilityBugVisitor>(Region)); } if (ValueExpr) { R->addRange(ValueExpr->getSourceRange()); @@ -290,10 +290,9 @@ NullabilityChecker::getTrackRegion(SVal Val, bool CheckSuperRegion) const { return dyn_cast<SymbolicRegion>(Region); } -std::shared_ptr<PathDiagnosticPiece> -NullabilityChecker::NullabilityBugVisitor::VisitNode(const ExplodedNode *N, - BugReporterContext &BRC, - BugReport &BR) { +PathDiagnosticPieceRef NullabilityChecker::NullabilityBugVisitor::VisitNode( + const ExplodedNode *N, BugReporterContext &BRC, + PathSensitiveBugReport &BR) { ProgramStateRef State = N->getState(); ProgramStateRef StatePrev = N->getFirstPred()->getState(); @@ -310,7 +309,7 @@ NullabilityChecker::NullabilityBugVisitor::VisitNode(const ExplodedNode *N, // Retrieve the associated statement. const Stmt *S = TrackedNullab->getNullabilitySource(); if (!S || S->getBeginLoc().isInvalid()) { - S = PathDiagnosticLocation::getStmt(N); + S = N->getStmtForDiagnostics(); } if (!S) @@ -324,8 +323,7 @@ NullabilityChecker::NullabilityBugVisitor::VisitNode(const ExplodedNode *N, // Generate the extra diagnostic. PathDiagnosticLocation Pos(S, BRC.getSourceManager(), N->getLocationContext()); - return std::make_shared<PathDiagnosticEventPiece>(Pos, InfoText, true, - nullptr); + return std::make_shared<PathDiagnosticEventPiece>(Pos, InfoText, true); } /// Returns true when the value stored at the given location has been @@ -1203,12 +1201,12 @@ bool ento::shouldRegisterNullabilityBase(const LangOptions &LO) { void ento::register##name##Checker(CheckerManager &mgr) { \ NullabilityChecker *checker = mgr.getChecker<NullabilityChecker>(); \ checker->Filter.Check##name = true; \ - checker->Filter.CheckName##name = mgr.getCurrentCheckName(); \ + checker->Filter.CheckName##name = mgr.getCurrentCheckerName(); \ checker->NeedTracking = checker->NeedTracking || trackingRequired; \ checker->NoDiagnoseCallsToSystemHeaders = \ checker->NoDiagnoseCallsToSystemHeaders || \ mgr.getAnalyzerOptions().getCheckerBooleanOption( \ - checker, "NoDiagnoseCallsToSystemHeaders", true); \ + checker, "NoDiagnoseCallsToSystemHeaders", true); \ } \ \ bool ento::shouldRegister##name##Checker(const LangOptions &LO) { \ diff --git a/lib/StaticAnalyzer/Checkers/ObjCAtSyncChecker.cpp b/lib/StaticAnalyzer/Checkers/ObjCAtSyncChecker.cpp index bd8cfb14680fe..0e25817c87932 100644 --- a/lib/StaticAnalyzer/Checkers/ObjCAtSyncChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/ObjCAtSyncChecker.cpp @@ -46,8 +46,8 @@ void ObjCAtSyncChecker::checkPreStmt(const ObjCAtSynchronizedStmt *S, 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); + auto report = std::make_unique<PathSensitiveBugReport>( + *BT_undef, BT_undef->getDescription(), N); bugreporter::trackExpressionValue(N, Ex, *report); C.emitReport(std::move(report)); } @@ -70,8 +70,8 @@ void ObjCAtSyncChecker::checkPreStmt(const ObjCAtSynchronizedStmt *S, 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); + auto report = std::make_unique<PathSensitiveBugReport>( + *BT_null, BT_null->getDescription(), N); bugreporter::trackExpressionValue(N, Ex, *report); C.emitReport(std::move(report)); diff --git a/lib/StaticAnalyzer/Checkers/ObjCContainersChecker.cpp b/lib/StaticAnalyzer/Checkers/ObjCContainersChecker.cpp index f69a3944a56ca..8abb926d4862f 100644 --- a/lib/StaticAnalyzer/Checkers/ObjCContainersChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/ObjCContainersChecker.cpp @@ -144,10 +144,11 @@ void ObjCContainersChecker::checkPreStmt(const CallExpr *CE, if (!N) return; initBugType(); - auto R = llvm::make_unique<BugReport>(*BT, "Index is out of bounds", N); + auto R = std::make_unique<PathSensitiveBugReport>( + *BT, "Index is out of bounds", N); R->addRange(IdxExpr->getSourceRange()); - bugreporter::trackExpressionValue(N, IdxExpr, *R, - /*EnableNullFPSuppression=*/false); + bugreporter::trackExpressionValue( + N, IdxExpr, *R, bugreporter::TrackingKind::Thorough, false); C.emitReport(std::move(R)); return; } diff --git a/lib/StaticAnalyzer/Checkers/ObjCMissingSuperCallChecker.cpp b/lib/StaticAnalyzer/Checkers/ObjCMissingSuperCallChecker.cpp index 33e4d2af000dd..1870c08432de3 100644 --- a/lib/StaticAnalyzer/Checkers/ObjCMissingSuperCallChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/ObjCMissingSuperCallChecker.cpp @@ -13,12 +13,12 @@ //===----------------------------------------------------------------------===// #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" +#include "clang/Analysis/PathDiagnostic.h" #include "clang/AST/DeclObjC.h" #include "clang/AST/Expr.h" #include "clang/AST/ExprObjC.h" #include "clang/AST/RecursiveASTVisitor.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h" -#include "clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h" #include "clang/StaticAnalyzer/Core/Checker.h" #include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h" #include "llvm/ADT/SmallSet.h" diff --git a/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp b/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp index 767b7bf4063c8..344285750f0e8 100644 --- a/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp @@ -159,7 +159,7 @@ void ObjCSelfInitChecker::checkForInvalidSelf(const Expr *E, CheckerContext &C, if (!BT) BT.reset(new BugType(this, "Missing \"self = [(super or self) init...]\"", categories::CoreFoundationObjectiveC)); - C.emitReport(llvm::make_unique<BugReport>(*BT, errorStr, N)); + C.emitReport(std::make_unique<PathSensitiveBugReport>(*BT, errorStr, N)); } void ObjCSelfInitChecker::checkPostObjCMessage(const ObjCMethodCall &Msg, diff --git a/lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp b/lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp index f435f00c08e79..0575be8453743 100644 --- a/lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp @@ -67,12 +67,11 @@ class SuperDeallocBRVisitor final : public BugReporterVisitor { public: SuperDeallocBRVisitor(SymbolRef ReceiverSymbol) - : ReceiverSymbol(ReceiverSymbol), - Satisfied(false) {} + : ReceiverSymbol(ReceiverSymbol), Satisfied(false) {} - std::shared_ptr<PathDiagnosticPiece> VisitNode(const ExplodedNode *Succ, - BugReporterContext &BRC, - BugReport &BR) override; + PathDiagnosticPieceRef VisitNode(const ExplodedNode *Succ, + BugReporterContext &BRC, + PathSensitiveBugReport &BR) override; void Profile(llvm::FoldingSetNodeID &ID) const override { ID.Add(ReceiverSymbol); @@ -188,10 +187,10 @@ void ObjCSuperDeallocChecker::reportUseAfterDealloc(SymbolRef Sym, Desc = "Use of 'self' after it has been deallocated"; // Generate the report. - std::unique_ptr<BugReport> BR( - new BugReport(*DoubleSuperDeallocBugType, Desc, ErrNode)); + auto BR = std::make_unique<PathSensitiveBugReport>(*DoubleSuperDeallocBugType, + Desc, ErrNode); BR->addRange(S->getSourceRange()); - BR->addVisitor(llvm::make_unique<SuperDeallocBRVisitor>(Sym)); + BR->addVisitor(std::make_unique<SuperDeallocBRVisitor>(Sym)); C.emitReport(std::move(BR)); } @@ -243,9 +242,10 @@ ObjCSuperDeallocChecker::isSuperDeallocMessage(const ObjCMethodCall &M) const { return M.getSelector() == SELdealloc; } -std::shared_ptr<PathDiagnosticPiece> +PathDiagnosticPieceRef SuperDeallocBRVisitor::VisitNode(const ExplodedNode *Succ, - BugReporterContext &BRC, BugReport &) { + BugReporterContext &BRC, + PathSensitiveBugReport &) { if (Satisfied) return nullptr; diff --git a/lib/StaticAnalyzer/Checkers/ObjCUnusedIVarsChecker.cpp b/lib/StaticAnalyzer/Checkers/ObjCUnusedIVarsChecker.cpp index 4b39a97c7e8d1..cb47704515723 100644 --- a/lib/StaticAnalyzer/Checkers/ObjCUnusedIVarsChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/ObjCUnusedIVarsChecker.cpp @@ -13,6 +13,7 @@ //===----------------------------------------------------------------------===// #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" +#include "clang/Analysis/PathDiagnostic.h" #include "clang/AST/Attr.h" #include "clang/AST/DeclObjC.h" #include "clang/AST/Expr.h" @@ -20,7 +21,6 @@ #include "clang/Basic/LangOptions.h" #include "clang/Basic/SourceManager.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h" -#include "clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h" #include "clang/StaticAnalyzer/Core/Checker.h" using namespace clang; @@ -94,7 +94,7 @@ static void Scan(IvarUsageMap& M, const ObjCContainerDecl *D) { } static void Scan(IvarUsageMap &M, const DeclContext *C, const FileID FID, - SourceManager &SM) { + const SourceManager &SM) { for (const auto *I : C->decls()) if (const auto *FD = dyn_cast<FunctionDecl>(I)) { SourceLocation L = FD->getBeginLoc(); @@ -148,7 +148,7 @@ static void checkObjCUnusedIvar(const ObjCImplementationDecl *D, // FIXME: In the future hopefully we can just use the lexical DeclContext // to go from the ObjCImplementationDecl to the lexically "nested" // C functions. - SourceManager &SM = BR.getSourceManager(); + const SourceManager &SM = BR.getSourceManager(); Scan(M, D->getDeclContext(), SM.getFileID(D->getLocation()), SM); // Find ivars that are unused. diff --git a/lib/StaticAnalyzer/Checkers/PaddingChecker.cpp b/lib/StaticAnalyzer/Checkers/PaddingChecker.cpp index 0aa410de15ff2..4a3c2b8cd40e2 100644 --- a/lib/StaticAnalyzer/Checkers/PaddingChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/PaddingChecker.cpp @@ -306,7 +306,7 @@ public: const SmallVector<const FieldDecl *, 20> &OptimalFieldsOrder) const { if (!PaddingBug) PaddingBug = - llvm::make_unique<BugType>(this, "Excessive Padding", "Performance"); + std::make_unique<BugType>(this, "Excessive Padding", "Performance"); SmallString<100> Buf; llvm::raw_svector_ostream Os(Buf); @@ -335,7 +335,8 @@ public: PathDiagnosticLocation CELoc = PathDiagnosticLocation::create(RD, BR->getSourceManager()); - auto Report = llvm::make_unique<BugReport>(*PaddingBug, Os.str(), CELoc); + auto Report = + std::make_unique<BasicBugReport>(*PaddingBug, Os.str(), CELoc); Report->setDeclWithIssue(RD); Report->addRange(RD->getSourceRange()); BR->emitReport(std::move(Report)); diff --git a/lib/StaticAnalyzer/Checkers/PointerArithChecker.cpp b/lib/StaticAnalyzer/Checkers/PointerArithChecker.cpp index 03c3f4dd23570..259f23abdc958 100644 --- a/lib/StaticAnalyzer/Checkers/PointerArithChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/PointerArithChecker.cpp @@ -119,12 +119,12 @@ const MemRegion *PointerArithChecker::getArrayRegion(const MemRegion *Region, AllocKind &AKind, CheckerContext &C) const { assert(Region); - while (Region->getKind() == MemRegion::Kind::CXXBaseObjectRegionKind) { - Region = Region->getAs<CXXBaseObjectRegion>()->getSuperRegion(); + while (const auto *BaseRegion = dyn_cast<CXXBaseObjectRegion>(Region)) { + Region = BaseRegion->getSuperRegion(); Polymorphic = true; } - if (Region->getKind() == MemRegion::Kind::ElementRegionKind) { - Region = Region->getAs<ElementRegion>()->getSuperRegion(); + if (const auto *ElemRegion = dyn_cast<ElementRegion>(Region)) { + Region = ElemRegion->getSuperRegion(); } ProgramStateRef State = C.getState(); @@ -137,7 +137,7 @@ const MemRegion *PointerArithChecker::getArrayRegion(const MemRegion *Region, } // When the region is symbolic and we do not have any information about it, // assume that this is an array to avoid false positives. - if (Region->getKind() == MemRegion::Kind::SymbolicRegionKind) + if (isa<SymbolicRegion>(Region)) return Region; // No AllocKind stored and not symbolic, assume that it points to a single @@ -173,8 +173,8 @@ void PointerArithChecker::reportPointerArithMisuse(const Expr *E, this, "Dangerous pointer arithmetic", "Pointer arithmetic on a pointer to base class is dangerous " "because derived and base class may have different size.")); - auto R = llvm::make_unique<BugReport>(*BT_polyArray, - BT_polyArray->getDescription(), N); + auto R = std::make_unique<PathSensitiveBugReport>( + *BT_polyArray, BT_polyArray->getDescription(), N); R->addRange(E->getSourceRange()); R->markInteresting(ArrayRegion); C.emitReport(std::move(R)); @@ -196,8 +196,8 @@ void PointerArithChecker::reportPointerArithMisuse(const Expr *E, "Pointer arithmetic on non-array " "variables relies on memory layout, " "which is dangerous.")); - auto R = llvm::make_unique<BugReport>(*BT_pointerArith, - BT_pointerArith->getDescription(), N); + auto R = std::make_unique<PathSensitiveBugReport>( + *BT_pointerArith, BT_pointerArith->getDescription(), N); R->addRange(SR); R->markInteresting(Region); C.emitReport(std::move(R)); diff --git a/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp b/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp index c9512f4fc42ff..88d0eb2ae7484 100644 --- a/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp @@ -63,7 +63,8 @@ void PointerSubChecker::checkPreStmt(const BinaryOperator *B, 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); + auto R = + std::make_unique<PathSensitiveBugReport>(*BT, BT->getDescription(), N); R->addRange(B->getSourceRange()); C.emitReport(std::move(R)); } diff --git a/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp b/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp index 33f677e1c2583..8649b8b96dd03 100644 --- a/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp @@ -240,7 +240,7 @@ void PthreadLockChecker::AcquireLock(CheckerContext &C, const CallExpr *CE, ExplodedNode *N = C.generateErrorNode(); if (!N) return; - auto report = llvm::make_unique<BugReport>( + auto report = std::make_unique<PathSensitiveBugReport>( *BT_doublelock, "This lock has already been acquired", N); report->addRange(CE->getArg(0)->getSourceRange()); C.emitReport(std::move(report)); @@ -305,7 +305,7 @@ void PthreadLockChecker::ReleaseLock(CheckerContext &C, const CallExpr *CE, ExplodedNode *N = C.generateErrorNode(); if (!N) return; - auto Report = llvm::make_unique<BugReport>( + auto Report = std::make_unique<PathSensitiveBugReport>( *BT_doubleunlock, "This lock has already been unlocked", N); Report->addRange(CE->getArg(0)->getSourceRange()); C.emitReport(std::move(Report)); @@ -328,7 +328,7 @@ void PthreadLockChecker::ReleaseLock(CheckerContext &C, const CallExpr *CE, ExplodedNode *N = C.generateErrorNode(); if (!N) return; - auto report = llvm::make_unique<BugReport>( + auto report = std::make_unique<PathSensitiveBugReport>( *BT_lor, "This was not the most recently acquired lock. Possible " "lock order reversal", N); report->addRange(CE->getArg(0)->getSourceRange()); @@ -399,7 +399,8 @@ void PthreadLockChecker::DestroyLock(CheckerContext &C, const CallExpr *CE, ExplodedNode *N = C.generateErrorNode(); if (!N) return; - auto Report = llvm::make_unique<BugReport>(*BT_destroylock, Message, N); + auto Report = + std::make_unique<PathSensitiveBugReport>(*BT_destroylock, Message, N); Report->addRange(CE->getArg(0)->getSourceRange()); C.emitReport(std::move(Report)); } @@ -438,7 +439,8 @@ void PthreadLockChecker::InitLock(CheckerContext &C, const CallExpr *CE, ExplodedNode *N = C.generateErrorNode(); if (!N) return; - auto Report = llvm::make_unique<BugReport>(*BT_initlock, Message, N); + auto Report = + std::make_unique<PathSensitiveBugReport>(*BT_initlock, Message, N); Report->addRange(CE->getArg(0)->getSourceRange()); C.emitReport(std::move(Report)); } @@ -451,7 +453,7 @@ void PthreadLockChecker::reportUseDestroyedBug(CheckerContext &C, ExplodedNode *N = C.generateErrorNode(); if (!N) return; - auto Report = llvm::make_unique<BugReport>( + auto Report = std::make_unique<PathSensitiveBugReport>( *BT_destroylock, "This lock has already been destroyed", N); Report->addRange(CE->getArg(0)->getSourceRange()); C.emitReport(std::move(Report)); diff --git a/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp b/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp index 4a3a8dae23a7f..6f8cb1432bb11 100644 --- a/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp @@ -875,7 +875,7 @@ void RetainCountChecker::processNonLeakError(ProgramStateRef St, if (!N) return; - auto report = llvm::make_unique<RefCountReport>( + auto report = std::make_unique<RefCountReport>( errorKindToBugKind(ErrorKind, Sym), C.getASTContext().getLangOpts(), N, Sym); report->addRange(ErrorRange); @@ -1095,7 +1095,7 @@ ExplodedNode * RetainCountChecker::checkReturnWithRetEffect(const ReturnStmt *S, if (N) { const LangOptions &LOpts = C.getASTContext().getLangOpts(); auto R = - llvm::make_unique<RefLeakReport>(leakAtReturn, LOpts, N, Sym, C); + std::make_unique<RefLeakReport>(leakAtReturn, LOpts, N, Sym, C); C.emitReport(std::move(R)); } return N; @@ -1119,7 +1119,7 @@ ExplodedNode * RetainCountChecker::checkReturnWithRetEffect(const ReturnStmt *S, ExplodedNode *N = C.addTransition(state, Pred, &ReturnNotOwnedTag); if (N) { - auto R = llvm::make_unique<RefCountReport>( + auto R = std::make_unique<RefCountReport>( returnNotOwnedForOwned, C.getASTContext().getLangOpts(), N, Sym); C.emitReport(std::move(R)); } @@ -1273,7 +1273,7 @@ RetainCountChecker::handleAutoreleaseCounts(ProgramStateRef state, os << "has a +" << V.getCount() << " retain count"; const LangOptions &LOpts = Ctx.getASTContext().getLangOpts(); - auto R = llvm::make_unique<RefCountReport>(overAutorelease, LOpts, N, Sym, + auto R = std::make_unique<RefCountReport>(overAutorelease, LOpts, N, Sym, os.str()); Ctx.emitReport(std::move(R)); } @@ -1321,7 +1321,7 @@ RetainCountChecker::processLeaks(ProgramStateRef state, if (N) { for (SymbolRef L : Leaked) { const RefCountBug &BT = Pred ? leakWithinFunction : leakAtReturn; - Ctx.emitReport(llvm::make_unique<RefLeakReport>(BT, LOpts, N, L, Ctx)); + Ctx.emitReport(std::make_unique<RefLeakReport>(BT, LOpts, N, L, Ctx)); } } diff --git a/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h b/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h index 124c0e5040b99..dd79bbef321c3 100644 --- a/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h +++ b/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h @@ -21,12 +21,12 @@ #include "clang/AST/DeclObjC.h" #include "clang/AST/ParentMap.h" #include "clang/Analysis/DomainSpecific/CocoaConventions.h" +#include "clang/Analysis/PathDiagnostic.h" #include "clang/Analysis/RetainSummaryManager.h" #include "clang/Basic/LangOptions.h" #include "clang/Basic/SourceManager.h" #include "clang/Analysis/SelectorExtras.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" -#include "clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h" #include "clang/StaticAnalyzer/Core/Checker.h" #include "clang/StaticAnalyzer/Core/CheckerManager.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" diff --git a/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp b/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp index 796fd882ffd5e..9853758f7f2c0 100644 --- a/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp +++ b/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp @@ -325,22 +325,22 @@ public: ID.AddPointer(Sym); } - std::shared_ptr<PathDiagnosticPiece> VisitNode(const ExplodedNode *N, - BugReporterContext &BRC, - BugReport &BR) override; + PathDiagnosticPieceRef VisitNode(const ExplodedNode *N, + BugReporterContext &BRC, + PathSensitiveBugReport &BR) override; - std::shared_ptr<PathDiagnosticPiece> getEndPath(BugReporterContext &BRC, - const ExplodedNode *N, - BugReport &BR) override; + PathDiagnosticPieceRef getEndPath(BugReporterContext &BRC, + const ExplodedNode *N, + PathSensitiveBugReport &BR) override; }; class RefLeakReportVisitor : public RefCountReportVisitor { public: RefLeakReportVisitor(SymbolRef sym) : RefCountReportVisitor(sym) {} - std::shared_ptr<PathDiagnosticPiece> getEndPath(BugReporterContext &BRC, - const ExplodedNode *N, - BugReport &BR) override; + PathDiagnosticPieceRef getEndPath(BugReporterContext &BRC, + const ExplodedNode *N, + PathSensitiveBugReport &BR) override; }; } // end namespace retaincountchecker @@ -448,9 +448,9 @@ annotateStartParameter(const ExplodedNode *N, SymbolRef Sym, return std::make_shared<PathDiagnosticEventPiece>(L, os.str()); } -std::shared_ptr<PathDiagnosticPiece> -RefCountReportVisitor::VisitNode(const ExplodedNode *N, - BugReporterContext &BRC, BugReport &BR) { +PathDiagnosticPieceRef +RefCountReportVisitor::VisitNode(const ExplodedNode *N, BugReporterContext &BRC, + PathSensitiveBugReport &BR) { const auto &BT = static_cast<const RefCountBug&>(BR.getBugType()); const auto *Checker = @@ -709,21 +709,22 @@ static AllocationInfo GetAllocationSite(ProgramStateManager &StateMgr, LeakContext) FirstBinding = nullptr; - return AllocationInfo(AllocationNodeInCurrentOrParentContext, - FirstBinding, + return AllocationInfo(AllocationNodeInCurrentOrParentContext, FirstBinding, InterestingMethodContext); } -std::shared_ptr<PathDiagnosticPiece> +PathDiagnosticPieceRef RefCountReportVisitor::getEndPath(BugReporterContext &BRC, - const ExplodedNode *EndN, BugReport &BR) { + const ExplodedNode *EndN, + PathSensitiveBugReport &BR) { BR.markInteresting(Sym); return BugReporterVisitor::getDefaultEndPath(BRC, EndN, BR); } -std::shared_ptr<PathDiagnosticPiece> +PathDiagnosticPieceRef RefLeakReportVisitor::getEndPath(BugReporterContext &BRC, - const ExplodedNode *EndN, BugReport &BR) { + const ExplodedNode *EndN, + PathSensitiveBugReport &BR) { // Tell the BugReporterContext to report cases when the tracked symbol is // assigned to different variables, etc. @@ -737,13 +738,7 @@ RefLeakReportVisitor::getEndPath(BugReporterContext &BRC, const MemRegion* FirstBinding = AllocI.R; BR.markInteresting(AllocI.InterestingMethodContext); - SourceManager& SM = BRC.getSourceManager(); - - // Compute an actual location for the leak. Sometimes a leak doesn't - // occur at an actual statement (e.g., transition between blocks; end - // of function) so we need to walk the graph and compute a real location. - const ExplodedNode *LeakN = EndN; - PathDiagnosticLocation L = PathDiagnosticLocation::createEndOfPath(LeakN, SM); + PathDiagnosticLocation L = cast<RefLeakReport>(BR).getEndOfPath(); std::string sbuf; llvm::raw_string_ostream os(sbuf); @@ -814,19 +809,19 @@ RefLeakReportVisitor::getEndPath(BugReporterContext &BRC, } RefCountReport::RefCountReport(const RefCountBug &D, const LangOptions &LOpts, - ExplodedNode *n, SymbolRef sym, - bool isLeak) - : BugReport(D, D.getDescription(), n), Sym(sym), isLeak(isLeak) { + ExplodedNode *n, SymbolRef sym, bool isLeak) + : PathSensitiveBugReport(D, D.getDescription(), n), Sym(sym), + isLeak(isLeak) { if (!isLeak) - addVisitor(llvm::make_unique<RefCountReportVisitor>(sym)); + addVisitor(std::make_unique<RefCountReportVisitor>(sym)); } RefCountReport::RefCountReport(const RefCountBug &D, const LangOptions &LOpts, ExplodedNode *n, SymbolRef sym, StringRef endText) - : BugReport(D, D.getDescription(), endText, n) { + : PathSensitiveBugReport(D, D.getDescription(), endText, n) { - addVisitor(llvm::make_unique<RefCountReportVisitor>(sym)); + addVisitor(std::make_unique<RefCountReportVisitor>(sym)); } void RefLeakReport::deriveParamLocation(CheckerContext &Ctx, SymbolRef sym) { @@ -873,7 +868,7 @@ void RefLeakReport::deriveAllocLocation(CheckerContext &Ctx, // FIXME: This will crash the analyzer if an allocation comes from an // implicit call (ex: a destructor call). // (Currently there are no such allocations in Cocoa, though.) - AllocStmt = PathDiagnosticLocation::getStmt(AllocNode); + AllocStmt = AllocNode->getStmtForDiagnostics(); if (!AllocStmt) { AllocBinding = nullptr; @@ -918,5 +913,5 @@ RefLeakReport::RefLeakReport(const RefCountBug &D, const LangOptions &LOpts, createDescription(Ctx); - addVisitor(llvm::make_unique<RefLeakReportVisitor>(sym)); + addVisitor(std::make_unique<RefLeakReportVisitor>(sym)); } diff --git a/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.h b/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.h index ef3c75f87af5d..e9e2777540548 100644 --- a/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.h +++ b/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.h @@ -14,10 +14,10 @@ #ifndef LLVM_CLANG_LIB_STATICANALYZER_CHECKERS_RETAINCOUNTCHECKER_DIAGNOSTICS_H #define LLVM_CLANG_LIB_STATICANALYZER_CHECKERS_RETAINCOUNTCHECKER_DIAGNOSTICS_H +#include "clang/Analysis/PathDiagnostic.h" #include "clang/Analysis/RetainSummaryManager.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h" -#include "clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" namespace clang { @@ -53,7 +53,7 @@ private: static StringRef bugTypeToName(RefCountBugType BT); }; -class RefCountReport : public BugReport { +class RefCountReport : public PathSensitiveBugReport { protected: SymbolRef Sym; bool isLeak = false; @@ -67,16 +67,17 @@ public: ExplodedNode *n, SymbolRef sym, StringRef endText); - llvm::iterator_range<ranges_iterator> getRanges() override { + ArrayRef<SourceRange> getRanges() const override { if (!isLeak) - return BugReport::getRanges(); - return llvm::make_range(ranges_iterator(), ranges_iterator()); + return PathSensitiveBugReport::getRanges(); + return {}; } }; class RefLeakReport : public RefCountReport { const MemRegion* AllocBinding; const Stmt *AllocStmt; + PathDiagnosticLocation Location; // Finds the function declaration where a leak warning for the parameter // 'sym' should be raised. @@ -89,11 +90,14 @@ class RefLeakReport : public RefCountReport { public: RefLeakReport(const RefCountBug &D, const LangOptions &LOpts, ExplodedNode *n, SymbolRef sym, CheckerContext &Ctx); - - PathDiagnosticLocation getLocation(const SourceManager &SM) const override { + PathDiagnosticLocation getLocation() const override { assert(Location.isValid()); return Location; } + + PathDiagnosticLocation getEndOfPath() const { + return PathSensitiveBugReport::getLocation(); + } }; } // end namespace retaincountchecker diff --git a/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp b/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp index 9eb47e0526dc5..abd1a074b4871 100644 --- a/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp @@ -79,7 +79,8 @@ void ReturnPointerRangeChecker::checkPreStmt(const ReturnStmt *RS, // reference is outside the range. // Generate a report for this bug. - auto report = llvm::make_unique<BugReport>(*BT, BT->getDescription(), N); + auto report = + std::make_unique<PathSensitiveBugReport>(*BT, BT->getDescription(), N); report->addRange(RetE->getSourceRange()); C.emitReport(std::move(report)); diff --git a/lib/StaticAnalyzer/Checkers/ReturnUndefChecker.cpp b/lib/StaticAnalyzer/Checkers/ReturnUndefChecker.cpp index f55c369da67eb..fbd15d864424e 100644 --- a/lib/StaticAnalyzer/Checkers/ReturnUndefChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/ReturnUndefChecker.cpp @@ -83,7 +83,8 @@ static void emitBug(CheckerContext &C, BuiltinBug &BT, const Expr *RetE, if (!N) return; - auto Report = llvm::make_unique<BugReport>(BT, BT.getDescription(), N); + auto Report = + std::make_unique<PathSensitiveBugReport>(BT, BT.getDescription(), N); Report->addRange(RetE->getSourceRange()); bugreporter::trackExpressionValue(N, TrackingE ? TrackingE : RetE, *Report); diff --git a/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp b/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp index ec5e9622c236f..8193bcbef4cdf 100644 --- a/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp @@ -206,8 +206,8 @@ void SimpleStreamChecker::reportDoubleClose(SymbolRef FileDescSym, return; // Generate the report. - auto R = llvm::make_unique<BugReport>(*DoubleCloseBugType, - "Closing a previously closed file stream", ErrNode); + auto R = std::make_unique<PathSensitiveBugReport>( + *DoubleCloseBugType, "Closing a previously closed file stream", ErrNode); R->addRange(Call.getSourceRange()); R->markInteresting(FileDescSym); C.emitReport(std::move(R)); @@ -219,8 +219,9 @@ void SimpleStreamChecker::reportLeaks(ArrayRef<SymbolRef> LeakedStreams, // Attach bug reports to the leak node. // TODO: Identify the leaked file descriptor. for (SymbolRef LeakedStream : LeakedStreams) { - auto R = llvm::make_unique<BugReport>(*LeakBugType, - "Opened file is never closed; potential resource leak", ErrNode); + auto R = std::make_unique<PathSensitiveBugReport>( + *LeakBugType, "Opened file is never closed; potential resource leak", + ErrNode); R->markInteresting(LeakedStream); C.emitReport(std::move(R)); } diff --git a/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp b/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp index b93bed5c30973..7285d27495a7c 100644 --- a/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp @@ -155,14 +155,15 @@ void StackAddrEscapeChecker::EmitStackError(CheckerContext &C, if (!N) return; if (!BT_returnstack) - BT_returnstack = llvm::make_unique<BuiltinBug>( + BT_returnstack = std::make_unique<BuiltinBug>( this, "Return of address to stack-allocated memory"); // Generate a report for this bug. SmallString<128> buf; llvm::raw_svector_ostream os(buf); SourceRange range = genName(os, R, C.getASTContext()); os << " returned to caller"; - auto report = llvm::make_unique<BugReport>(*BT_returnstack, os.str(), N); + auto report = + std::make_unique<PathSensitiveBugReport>(*BT_returnstack, os.str(), N); report->addRange(RetE->getSourceRange()); if (range.isValid()) report->addRange(range); @@ -193,14 +194,14 @@ void StackAddrEscapeChecker::checkAsyncExecutedBlockCaptures( if (!N) continue; if (!BT_capturedstackasync) - BT_capturedstackasync = llvm::make_unique<BuiltinBug>( + BT_capturedstackasync = std::make_unique<BuiltinBug>( this, "Address of stack-allocated memory is captured"); SmallString<128> Buf; llvm::raw_svector_ostream Out(Buf); SourceRange Range = genName(Out, Region, C.getASTContext()); Out << " is captured by an asynchronously-executed block"; - auto Report = - llvm::make_unique<BugReport>(*BT_capturedstackasync, Out.str(), N); + auto Report = std::make_unique<PathSensitiveBugReport>( + *BT_capturedstackasync, Out.str(), N); if (Range.isValid()) Report->addRange(Range); C.emitReport(std::move(Report)); @@ -216,14 +217,14 @@ void StackAddrEscapeChecker::checkReturnedBlockCaptures( if (!N) continue; if (!BT_capturedstackret) - BT_capturedstackret = llvm::make_unique<BuiltinBug>( + BT_capturedstackret = std::make_unique<BuiltinBug>( this, "Address of stack-allocated memory is captured"); SmallString<128> Buf; llvm::raw_svector_ostream Out(Buf); SourceRange Range = genName(Out, Region, C.getASTContext()); Out << " is captured by a returned block"; - auto Report = - llvm::make_unique<BugReport>(*BT_capturedstackret, Out.str(), N); + auto Report = std::make_unique<PathSensitiveBugReport>(*BT_capturedstackret, + Out.str(), N); if (Range.isValid()) Report->addRange(Range); C.emitReport(std::move(Report)); @@ -331,7 +332,7 @@ void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS, return; if (!BT_stackleak) - BT_stackleak = llvm::make_unique<BuiltinBug>( + BT_stackleak = std::make_unique<BuiltinBug>( this, "Stack address stored into global variable", "Stack address was saved into a global variable. " "This is dangerous because the address will become " @@ -351,7 +352,8 @@ void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS, const VarRegion *VR = cast<VarRegion>(P.first->getBaseRegion()); Out << *VR->getDecl() << "' upon returning to the caller. This will be a dangling reference"; - auto Report = llvm::make_unique<BugReport>(*BT_stackleak, Out.str(), N); + auto Report = + std::make_unique<PathSensitiveBugReport>(*BT_stackleak, Out.str(), N); if (Range.isValid()) Report->addRange(Range); diff --git a/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/lib/StaticAnalyzer/Checkers/StreamChecker.cpp index 1ea5e07695133..c254408351c81 100644 --- a/lib/StaticAnalyzer/Checkers/StreamChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/StreamChecker.cpp @@ -277,7 +277,7 @@ void StreamChecker::Fseek(CheckerContext &C, const CallExpr *CE) const { 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>( + C.emitReport(std::make_unique<PathSensitiveBugReport>( *BT_illegalwhence, BT_illegalwhence->getDescription(), N)); } } @@ -345,7 +345,7 @@ ProgramStateRef StreamChecker::CheckNullStream(SVal SV, ProgramStateRef state, if (!BT_nullfp) BT_nullfp.reset(new BuiltinBug(this, "NULL stream pointer", "Stream pointer might be NULL.")); - C.emitReport(llvm::make_unique<BugReport>( + C.emitReport(std::make_unique<PathSensitiveBugReport>( *BT_nullfp, BT_nullfp->getDescription(), N)); } return nullptr; @@ -375,7 +375,7 @@ ProgramStateRef StreamChecker::CheckDoubleClose(const CallExpr *CE, 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>( + C.emitReport(std::make_unique<PathSensitiveBugReport>( *BT_doubleclose, BT_doubleclose->getDescription(), N)); } return nullptr; @@ -405,7 +405,7 @@ void StreamChecker::checkDeadSymbols(SymbolReaper &SymReaper, BT_ResourceLeak.reset( new BuiltinBug(this, "Resource Leak", "Opened File never closed. Potential Resource leak.")); - C.emitReport(llvm::make_unique<BugReport>( + C.emitReport(std::make_unique<PathSensitiveBugReport>( *BT_ResourceLeak, BT_ResourceLeak->getDescription(), N)); } } diff --git a/lib/StaticAnalyzer/Checkers/Taint.cpp b/lib/StaticAnalyzer/Checkers/Taint.cpp index bc120949ee4f1..574d4ed1e600c 100644 --- a/lib/StaticAnalyzer/Checkers/Taint.cpp +++ b/lib/StaticAnalyzer/Checkers/Taint.cpp @@ -204,16 +204,16 @@ bool taint::isTainted(ProgramStateRef State, SymbolRef Sym, TaintTagType Kind) { return false; } -std::shared_ptr<PathDiagnosticPiece> -TaintBugVisitor::VisitNode(const ExplodedNode *N, BugReporterContext &BRC, - BugReport &BR) { +PathDiagnosticPieceRef TaintBugVisitor::VisitNode(const ExplodedNode *N, + BugReporterContext &BRC, + PathSensitiveBugReport &BR) { // Find the ExplodedNode where the taint was first introduced if (!isTainted(N->getState(), V) || isTainted(N->getFirstPred()->getState(), V)) return nullptr; - const Stmt *S = PathDiagnosticLocation::getStmt(N); + const Stmt *S = N->getStmtForDiagnostics(); if (!S) return nullptr; diff --git a/lib/StaticAnalyzer/Checkers/Taint.h b/lib/StaticAnalyzer/Checkers/Taint.h index 72cf6a79d52c4..8940916c19331 100644 --- a/lib/StaticAnalyzer/Checkers/Taint.h +++ b/lib/StaticAnalyzer/Checkers/Taint.h @@ -89,9 +89,9 @@ public: TaintBugVisitor(const SVal V) : V(V) {} void Profile(llvm::FoldingSetNodeID &ID) const override { ID.Add(V); } - std::shared_ptr<PathDiagnosticPiece> VisitNode(const ExplodedNode *N, - BugReporterContext &BRC, - BugReport &BR) override; + PathDiagnosticPieceRef VisitNode(const ExplodedNode *N, + BugReporterContext &BRC, + PathSensitiveBugReport &BR) override; }; } // namespace taint diff --git a/lib/StaticAnalyzer/Checkers/TaintTesterChecker.cpp b/lib/StaticAnalyzer/Checkers/TaintTesterChecker.cpp index 094762e2faf83..f81705304f3ab 100644 --- a/lib/StaticAnalyzer/Checkers/TaintTesterChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/TaintTesterChecker.cpp @@ -52,7 +52,7 @@ void TaintTesterChecker::checkPostStmt(const Expr *E, if (isTainted(State, E, C.getLocationContext())) { if (ExplodedNode *N = C.generateNonFatalErrorNode()) { initBugType(); - auto report = llvm::make_unique<BugReport>(*BT, "tainted",N); + auto report = std::make_unique<PathSensitiveBugReport>(*BT, "tainted", N); report->addRange(E->getSourceRange()); C.emitReport(std::move(report)); } diff --git a/lib/StaticAnalyzer/Checkers/TestAfterDivZeroChecker.cpp b/lib/StaticAnalyzer/Checkers/TestAfterDivZeroChecker.cpp index 7a33845a6a266..3663b09636924 100644 --- a/lib/StaticAnalyzer/Checkers/TestAfterDivZeroChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/TestAfterDivZeroChecker.cpp @@ -69,9 +69,9 @@ public: ID.Add(SFC); } - std::shared_ptr<PathDiagnosticPiece> VisitNode(const ExplodedNode *Succ, - BugReporterContext &BRC, - BugReport &BR) override; + PathDiagnosticPieceRef VisitNode(const ExplodedNode *Succ, + BugReporterContext &BRC, + PathSensitiveBugReport &BR) override; }; class TestAfterDivZeroChecker @@ -92,9 +92,9 @@ public: REGISTER_SET_WITH_PROGRAMSTATE(DivZeroMap, ZeroState) -std::shared_ptr<PathDiagnosticPiece> -DivisionBRVisitor::VisitNode(const ExplodedNode *Succ, - BugReporterContext &BRC, BugReport &BR) { +PathDiagnosticPieceRef +DivisionBRVisitor::VisitNode(const ExplodedNode *Succ, BugReporterContext &BRC, + PathSensitiveBugReport &BR) { if (Satisfied) return nullptr; @@ -167,12 +167,12 @@ void TestAfterDivZeroChecker::reportBug(SVal Val, CheckerContext &C) const { if (!DivZeroBug) DivZeroBug.reset(new BuiltinBug(this, "Division by zero")); - auto R = llvm::make_unique<BugReport>( + auto R = std::make_unique<PathSensitiveBugReport>( *DivZeroBug, "Value being compared against zero has already been used " "for division", N); - R->addVisitor(llvm::make_unique<DivisionBRVisitor>(Val.getAsSymbol(), + R->addVisitor(std::make_unique<DivisionBRVisitor>(Val.getAsSymbol(), C.getStackFrame())); C.emitReport(std::move(R)); } diff --git a/lib/StaticAnalyzer/Checkers/UndefBranchChecker.cpp b/lib/StaticAnalyzer/Checkers/UndefBranchChecker.cpp index 3a4a1dbf641bb..247cba7dc9333 100644 --- a/lib/StaticAnalyzer/Checkers/UndefBranchChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/UndefBranchChecker.cpp @@ -96,7 +96,8 @@ void UndefBranchChecker::checkBranchCondition(const Stmt *Condition, Ex = FindIt.FindExpr(Ex); // Emit the bug report. - auto R = llvm::make_unique<BugReport>(*BT, BT->getDescription(), N); + auto R = std::make_unique<PathSensitiveBugReport>( + *BT, BT->getDescription(), N); bugreporter::trackExpressionValue(N, Ex, *R); R->addRange(Ex->getSourceRange()); diff --git a/lib/StaticAnalyzer/Checkers/UndefCapturedBlockVarChecker.cpp b/lib/StaticAnalyzer/Checkers/UndefCapturedBlockVarChecker.cpp index c787ef58660ab..7b581bef39001 100644 --- a/lib/StaticAnalyzer/Checkers/UndefCapturedBlockVarChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/UndefCapturedBlockVarChecker.cpp @@ -83,11 +83,12 @@ UndefCapturedBlockVarChecker::checkPostStmt(const BlockExpr *BE, os << "Variable '" << VD->getName() << "' is uninitialized when captured by block"; - auto R = llvm::make_unique<BugReport>(*BT, os.str(), N); + auto R = std::make_unique<PathSensitiveBugReport>(*BT, os.str(), N); if (const Expr *Ex = FindBlockDeclRefExpr(BE->getBody(), VD)) R->addRange(Ex->getSourceRange()); - R->addVisitor(llvm::make_unique<FindLastStoreBRVisitor>( - *V, VR, /*EnableNullFPSuppression*/ false)); + R->addVisitor(std::make_unique<FindLastStoreBRVisitor>( + *V, VR, /*EnableNullFPSuppression*/ false, + bugreporter::TrackingKind::Thorough)); R->disablePathPruning(); // need location of block C.emitReport(std::move(R)); diff --git a/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp b/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp index 1ae287d39f11a..a2f3e0da13fb0 100644 --- a/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp @@ -170,7 +170,7 @@ void UndefResultChecker::checkPostStmt(const BinaryOperator *B, << "' expression is undefined"; } } - auto report = llvm::make_unique<BugReport>(*BT, OS.str(), N); + auto report = std::make_unique<PathSensitiveBugReport>(*BT, OS.str(), N); if (Ex) { report->addRange(Ex->getSourceRange()); bugreporter::trackExpressionValue(N, Ex, *report); diff --git a/lib/StaticAnalyzer/Checkers/UndefinedArraySubscriptChecker.cpp b/lib/StaticAnalyzer/Checkers/UndefinedArraySubscriptChecker.cpp index 4c517d6f05622..2f075eaeb03bf 100644 --- a/lib/StaticAnalyzer/Checkers/UndefinedArraySubscriptChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/UndefinedArraySubscriptChecker.cpp @@ -52,7 +52,7 @@ UndefinedArraySubscriptChecker::checkPreStmt(const ArraySubscriptExpr *A, 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); + auto R = std::make_unique<PathSensitiveBugReport>(*BT, BT->getDescription(), N); R->addRange(A->getIdx()->getSourceRange()); bugreporter::trackExpressionValue(N, A->getIdx(), *R); C.emitReport(std::move(R)); diff --git a/lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp b/lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp index d32d2a4042de7..277a8a1433282 100644 --- a/lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp @@ -85,7 +85,7 @@ void UndefinedAssignmentChecker::checkBind(SVal location, SVal val, } if (const DeclStmt *DS = dyn_cast<DeclStmt>(StoreE)) { - const VarDecl *VD = dyn_cast<VarDecl>(DS->getSingleDecl()); + const VarDecl *VD = cast<VarDecl>(DS->getSingleDecl()); ex = VD->getInit(); } @@ -108,7 +108,7 @@ void UndefinedAssignmentChecker::checkBind(SVal location, SVal val, if (OS.str().empty()) OS << DefaultMsg; - auto R = llvm::make_unique<BugReport>(*BT, OS.str(), N); + auto R = std::make_unique<PathSensitiveBugReport>(*BT, OS.str(), N); if (ex) { R->addRange(ex->getSourceRange()); bugreporter::trackExpressionValue(N, ex, *R); diff --git a/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp b/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp index 9d608c12d19be..020df8a1bb8c7 100644 --- a/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp @@ -24,7 +24,7 @@ #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" #include "clang/StaticAnalyzer/Core/Checker.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" -#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicTypeMap.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicType.h" using namespace clang; using namespace clang::ento; @@ -187,7 +187,7 @@ void UninitializedObjectChecker::checkEndFunction( if (Opts.ShouldConvertNotesToWarnings) { for (const auto &Pair : UninitFields) { - auto Report = llvm::make_unique<BugReport>( + auto Report = std::make_unique<PathSensitiveBugReport>( *BT_uninitField, Pair.second, Node, LocUsedForUniqueing, Node->getLocationContext()->getDecl()); Context.emitReport(std::move(Report)); @@ -201,7 +201,7 @@ void UninitializedObjectChecker::checkEndFunction( << (UninitFields.size() == 1 ? "" : "s") << " at the end of the constructor call"; - auto Report = llvm::make_unique<BugReport>( + auto Report = std::make_unique<PathSensitiveBugReport>( *BT_uninitField, WarningOS.str(), Node, LocUsedForUniqueing, Node->getLocationContext()->getDecl()); diff --git a/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp b/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp index a5dc250104f32..f0dd0bf813aff 100644 --- a/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp +++ b/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp @@ -18,7 +18,7 @@ #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" #include "clang/StaticAnalyzer/Core/Checker.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" -#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicTypeMap.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicType.h" using namespace clang; using namespace clang::ento; @@ -260,12 +260,13 @@ static llvm::Optional<DereferenceInfo> dereference(ProgramStateRef State, break; } - while (R->getAs<CXXBaseObjectRegion>()) { + while (isa<CXXBaseObjectRegion>(R)) { NeedsCastBack = true; - - if (!isa<TypedValueRegion>(R->getSuperRegion())) + const auto *SuperR = dyn_cast<TypedValueRegion>(R->getSuperRegion()); + if (!SuperR) break; - R = R->getSuperRegion()->getAs<TypedValueRegion>(); + + R = SuperR; } return DereferenceInfo{R, NeedsCastBack, /*IsCyclic*/ false}; diff --git a/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp b/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp index 2ccb519891f37..f4e225d836f38 100644 --- a/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp @@ -135,7 +135,7 @@ void UnixAPIMisuseChecker::ReportOpenBug(CheckerContext &C, LazyInitialize(this, BT_open, "Improper use of 'open'"); - auto Report = llvm::make_unique<BugReport>(*BT_open, Msg, N); + auto Report = std::make_unique<PathSensitiveBugReport>(*BT_open, Msg, N); Report->addRange(SR); C.emitReport(std::move(Report)); } @@ -304,7 +304,8 @@ void UnixAPIMisuseChecker::CheckPthreadOnce(CheckerContext &C, LazyInitialize(this, BT_pthreadOnce, "Improper use of 'pthread_once'"); - auto report = llvm::make_unique<BugReport>(*BT_pthreadOnce, os.str(), N); + auto report = + std::make_unique<PathSensitiveBugReport>(*BT_pthreadOnce, os.str(), N); report->addRange(CE->getArg(0)->getSourceRange()); C.emitReport(std::move(report)); } @@ -347,7 +348,8 @@ bool UnixAPIPortabilityChecker::ReportZeroByteAllocation( SmallString<256> S; llvm::raw_svector_ostream os(S); os << "Call to '" << fn_name << "' has an allocation size of 0 bytes"; - auto report = llvm::make_unique<BugReport>(*BT_mallocZero, os.str(), N); + auto report = + std::make_unique<PathSensitiveBugReport>(*BT_mallocZero, os.str(), N); report->addRange(arg->getSourceRange()); bugreporter::trackExpressionValue(N, arg, *report); diff --git a/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp b/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp index 0b0bf8465c9dd..65dd82675df9b 100644 --- a/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp @@ -55,7 +55,7 @@ void UnreachableCodeChecker::checkEndAnalysis(ExplodedGraph &G, const Decl *D = nullptr; CFG *C = nullptr; - ParentMap *PM = nullptr; + const ParentMap *PM = nullptr; const LocationContext *LC = nullptr; // Iterate over ExplodedGraph for (ExplodedGraph::node_iterator I = G.nodes_begin(), E = G.nodes_end(); diff --git a/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp b/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp index 1630896c3b607..b92757312dc65 100644 --- a/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp @@ -72,7 +72,7 @@ void VLASizeChecker::reportBug( break; } - auto report = llvm::make_unique<BugReport>(*BT, os.str(), N); + auto report = std::make_unique<PathSensitiveBugReport>(*BT, os.str(), N); report->addVisitor(std::move(Visitor)); report->addRange(SizeE->getSourceRange()); bugreporter::trackExpressionValue(N, SizeE, *report); @@ -110,7 +110,7 @@ void VLASizeChecker::checkPreStmt(const DeclStmt *DS, CheckerContext &C) const { // Check if the size is tainted. if (isTainted(state, sizeV)) { reportBug(VLA_Tainted, SE, nullptr, C, - llvm::make_unique<TaintBugVisitor>(sizeV)); + std::make_unique<TaintBugVisitor>(sizeV)); return; } diff --git a/lib/StaticAnalyzer/Checkers/ValistChecker.cpp b/lib/StaticAnalyzer/Checkers/ValistChecker.cpp index 13ad3d98e8ebb..a3610514a924b 100644 --- a/lib/StaticAnalyzer/Checkers/ValistChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/ValistChecker.cpp @@ -46,7 +46,7 @@ public: }; DefaultBool ChecksEnabled[CK_NumCheckKinds]; - CheckName CheckNames[CK_NumCheckKinds]; + CheckerNameRef CheckNames[CK_NumCheckKinds]; void checkPreStmt(const VAArgExpr *VAA, CheckerContext &C) const; void checkPreCall(const CallEvent &Call, CheckerContext &C) const; @@ -77,20 +77,20 @@ private: ID.AddPointer(&X); ID.AddPointer(Reg); } - std::shared_ptr<PathDiagnosticPiece> - getEndPath(BugReporterContext &BRC, const ExplodedNode *EndPathNode, - BugReport &BR) override { + PathDiagnosticPieceRef getEndPath(BugReporterContext &BRC, + const ExplodedNode *EndPathNode, + PathSensitiveBugReport &BR) override { if (!IsLeak) return nullptr; - PathDiagnosticLocation L = PathDiagnosticLocation::createEndOfPath( - EndPathNode, BRC.getSourceManager()); + PathDiagnosticLocation L = BR.getLocation(); // Do not add the statement itself as a range in case of leak. - return std::make_shared<PathDiagnosticEventPiece>(L, BR.getDescription(), false); + return std::make_shared<PathDiagnosticEventPiece>(L, BR.getDescription(), + false); } - std::shared_ptr<PathDiagnosticPiece> VisitNode(const ExplodedNode *N, - BugReporterContext &BRC, - BugReport &BR) override; + PathDiagnosticPieceRef VisitNode(const ExplodedNode *N, + BugReporterContext &BRC, + PathSensitiveBugReport &BR) override; private: const MemRegion *Reg; @@ -115,7 +115,9 @@ const SmallVector<ValistChecker::VAListAccepter, 15> // vswprintf is the wide version of vsnprintf, // vsprintf has no wide version {{"vswscanf", 3}, 2}}; -const CallDescription ValistChecker::VaStart("__builtin_va_start", 2), + +const CallDescription + ValistChecker::VaStart("__builtin_va_start", /*Args=*/2, /*Params=*/1), ValistChecker::VaCopy("__builtin_va_copy", 2), ValistChecker::VaEnd("__builtin_va_end", 1); } // end anonymous namespace @@ -253,9 +255,9 @@ void ValistChecker::reportUninitializedAccess(const MemRegion *VAList, BT_uninitaccess.reset(new BugType(CheckNames[CK_Uninitialized], "Uninitialized va_list", categories::MemoryError)); - auto R = llvm::make_unique<BugReport>(*BT_uninitaccess, Msg, N); + auto R = std::make_unique<PathSensitiveBugReport>(*BT_uninitaccess, Msg, N); R->markInteresting(VAList); - R->addVisitor(llvm::make_unique<ValistBugVisitor>(VAList)); + R->addVisitor(std::make_unique<ValistBugVisitor>(VAList)); C.emitReport(std::move(R)); } } @@ -282,7 +284,7 @@ void ValistChecker::reportLeakedVALists(const RegionVector &LeakedVALists, const ExplodedNode *StartNode = getStartCallSite(N, Reg); PathDiagnosticLocation LocUsedForUniqueing; - if (const Stmt *StartCallStmt = PathDiagnosticLocation::getStmt(StartNode)) + if (const Stmt *StartCallStmt = StartNode->getStmtForDiagnostics()) LocUsedForUniqueing = PathDiagnosticLocation::createBegin( StartCallStmt, C.getSourceManager(), StartNode->getLocationContext()); @@ -294,11 +296,11 @@ void ValistChecker::reportLeakedVALists(const RegionVector &LeakedVALists, OS << " " << VariableName; OS << Msg2; - auto R = llvm::make_unique<BugReport>( + auto R = std::make_unique<PathSensitiveBugReport>( *BT_leakedvalist, OS.str(), N, LocUsedForUniqueing, StartNode->getLocationContext()->getDecl()); R->markInteresting(Reg); - R->addVisitor(llvm::make_unique<ValistBugVisitor>(Reg, true)); + R->addVisitor(std::make_unique<ValistBugVisitor>(Reg, true)); C.emitReport(std::move(R)); } } @@ -373,13 +375,12 @@ void ValistChecker::checkVAListEndCall(const CallEvent &Call, C.addTransition(State); } -std::shared_ptr<PathDiagnosticPiece> ValistChecker::ValistBugVisitor::VisitNode( - const ExplodedNode *N, BugReporterContext &BRC, - BugReport &) { +PathDiagnosticPieceRef ValistChecker::ValistBugVisitor::VisitNode( + const ExplodedNode *N, BugReporterContext &BRC, PathSensitiveBugReport &) { ProgramStateRef State = N->getState(); ProgramStateRef StatePrev = N->getFirstPred()->getState(); - const Stmt *S = PathDiagnosticLocation::getStmt(N); + const Stmt *S = N->getStmtForDiagnostics(); if (!S) return nullptr; @@ -411,7 +412,8 @@ bool ento::shouldRegisterValistBase(const LangOptions &LO) { void ento::register##name##Checker(CheckerManager &mgr) { \ ValistChecker *checker = mgr.getChecker<ValistChecker>(); \ checker->ChecksEnabled[ValistChecker::CK_##name] = true; \ - checker->CheckNames[ValistChecker::CK_##name] = mgr.getCurrentCheckName(); \ + checker->CheckNames[ValistChecker::CK_##name] = \ + mgr.getCurrentCheckerName(); \ } \ \ bool ento::shouldRegister##name##Checker(const LangOptions &LO) { \ diff --git a/lib/StaticAnalyzer/Checkers/VforkChecker.cpp b/lib/StaticAnalyzer/Checkers/VforkChecker.cpp index 40d14aa5c7d44..6724eead50720 100644 --- a/lib/StaticAnalyzer/Checkers/VforkChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/VforkChecker.cpp @@ -132,7 +132,7 @@ void VforkChecker::reportBug(const char *What, CheckerContext &C, if (Details) os << "; " << Details; - auto Report = llvm::make_unique<BugReport>(*BT, os.str(), N); + auto Report = std::make_unique<PathSensitiveBugReport>(*BT, os.str(), N); // TODO: mark vfork call in BugReportVisitor C.emitReport(std::move(Report)); } diff --git a/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp b/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp index 762c9c1c8d7aa..12cee5f8d4f74 100644 --- a/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp @@ -6,7 +6,7 @@ // //===----------------------------------------------------------------------===// // -// This file defines a checker that checks virtual function calls during +// This file defines a checker that checks virtual method calls during // construction or destruction of C++ objects. // //===----------------------------------------------------------------------===// @@ -40,11 +40,10 @@ template <> struct FoldingSetTrait<ObjectState> { namespace { class VirtualCallChecker : public Checker<check::BeginFunction, check::EndFunction, check::PreCall> { - mutable std::unique_ptr<BugType> BT; - public: - // The flag to determine if pure virtual functions should be issued only. - DefaultBool IsPureOnly; + // These are going to be null if the respective check is disabled. + mutable std::unique_ptr<BugType> BT_Pure, BT_Impure; + bool ShowFixIts = false; void checkBeginFunction(CheckerContext &C) const; void checkEndFunction(const ReturnStmt *RS, CheckerContext &C) const; @@ -53,87 +52,13 @@ public: private: void registerCtorDtorCallInState(bool IsBeginFunction, CheckerContext &C) const; - void reportBug(StringRef Msg, bool PureError, const MemRegion *Reg, - CheckerContext &C) const; - - class VirtualBugVisitor : public BugReporterVisitor { - private: - const MemRegion *ObjectRegion; - bool Found; - - public: - VirtualBugVisitor(const MemRegion *R) : ObjectRegion(R), Found(false) {} - - void Profile(llvm::FoldingSetNodeID &ID) const override { - static int X = 0; - ID.AddPointer(&X); - ID.AddPointer(ObjectRegion); - } - - std::shared_ptr<PathDiagnosticPiece> VisitNode(const ExplodedNode *N, - BugReporterContext &BRC, - BugReport &BR) override; - }; }; } // end namespace // GDM (generic data map) to the memregion of this for the ctor and dtor. REGISTER_MAP_WITH_PROGRAMSTATE(CtorDtorMap, const MemRegion *, ObjectState) -std::shared_ptr<PathDiagnosticPiece> -VirtualCallChecker::VirtualBugVisitor::VisitNode(const ExplodedNode *N, - BugReporterContext &BRC, - BugReport &) { - // We need the last ctor/dtor which call the virtual function. - // The visitor walks the ExplodedGraph backwards. - if (Found) - return nullptr; - - ProgramStateRef State = N->getState(); - const LocationContext *LCtx = N->getLocationContext(); - const CXXConstructorDecl *CD = - dyn_cast_or_null<CXXConstructorDecl>(LCtx->getDecl()); - const CXXDestructorDecl *DD = - dyn_cast_or_null<CXXDestructorDecl>(LCtx->getDecl()); - - if (!CD && !DD) - return nullptr; - - ProgramStateManager &PSM = State->getStateManager(); - auto &SVB = PSM.getSValBuilder(); - const auto *MD = dyn_cast<CXXMethodDecl>(LCtx->getDecl()); - if (!MD) - return nullptr; - auto ThiSVal = - State->getSVal(SVB.getCXXThis(MD, LCtx->getStackFrame())); - const MemRegion *Reg = ThiSVal.castAs<loc::MemRegionVal>().getRegion(); - if (!Reg) - return nullptr; - if (Reg != ObjectRegion) - return nullptr; - - const Stmt *S = PathDiagnosticLocation::getStmt(N); - if (!S) - return nullptr; - Found = true; - - std::string InfoText; - if (CD) - InfoText = "This constructor of an object of type '" + - CD->getNameAsString() + - "' has not returned when the virtual method was called"; - else - InfoText = "This destructor of an object of type '" + - DD->getNameAsString() + - "' has not returned when the virtual method was called"; - - // Generate the extra diagnostic. - PathDiagnosticLocation Pos(S, BRC.getSourceManager(), - N->getLocationContext()); - return std::make_shared<PathDiagnosticEventPiece>(Pos, InfoText, true); -} - -// The function to check if a callexpr is a virtual function. +// The function to check if a callexpr is a virtual method call. static bool isVirtualCall(const CallExpr *CE) { bool CallIsNonVirtual = false; @@ -178,11 +103,10 @@ void VirtualCallChecker::checkPreCall(const CallEvent &Call, const CXXMethodDecl *MD = dyn_cast_or_null<CXXMethodDecl>(Call.getDecl()); if (!MD) return; - ProgramStateRef State = C.getState(); - const CallExpr *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr()); - if (IsPureOnly && !MD->isPure()) - return; + ProgramStateRef State = C.getState(); + // Member calls are always represented by a call-expression. + const auto *CE = cast<CallExpr>(Call.getOriginExpr()); if (!isVirtualCall(CE)) return; @@ -190,29 +114,51 @@ void VirtualCallChecker::checkPreCall(const CallEvent &Call, const ObjectState *ObState = State->get<CtorDtorMap>(Reg); if (!ObState) return; - // Check if a virtual method is called. - // The GDM of constructor and destructor should be true. - if (*ObState == ObjectState::CtorCalled) { - if (IsPureOnly && MD->isPure()) - reportBug("Call to pure virtual function during construction", true, Reg, - C); - else if (!MD->isPure()) - reportBug("Call to virtual function during construction", false, Reg, C); - else - reportBug("Call to pure virtual function during construction", false, Reg, - C); + + bool IsPure = MD->isPure(); + + // At this point we're sure that we're calling a virtual method + // during construction or destruction, so we'll emit a report. + SmallString<128> Msg; + llvm::raw_svector_ostream OS(Msg); + OS << "Call to "; + if (IsPure) + OS << "pure "; + OS << "virtual method '" << MD->getParent()->getNameAsString() + << "::" << MD->getNameAsString() << "' during "; + if (*ObState == ObjectState::CtorCalled) + OS << "construction "; + else + OS << "destruction "; + if (IsPure) + OS << "has undefined behavior"; + else + OS << "bypasses virtual dispatch"; + + ExplodedNode *N = + IsPure ? C.generateErrorNode() : C.generateNonFatalErrorNode(); + if (!N) + return; + + const std::unique_ptr<BugType> &BT = IsPure ? BT_Pure : BT_Impure; + if (!BT) { + // The respective check is disabled. + return; } - if (*ObState == ObjectState::DtorCalled) { - if (IsPureOnly && MD->isPure()) - reportBug("Call to pure virtual function during destruction", true, Reg, - C); - else if (!MD->isPure()) - reportBug("Call to virtual function during destruction", false, Reg, C); - else - reportBug("Call to pure virtual function during construction", false, Reg, - C); + auto Report = std::make_unique<PathSensitiveBugReport>(*BT, OS.str(), N); + + if (ShowFixIts && !IsPure) { + // FIXME: These hints are valid only when the virtual call is made + // directly from the constructor/destructor. Otherwise the dispatch + // will work just fine from other callees, and the fix may break + // the otherwise correct program. + FixItHint Fixit = FixItHint::CreateInsertion( + CE->getBeginLoc(), MD->getParent()->getNameAsString() + "::"); + Report->addFixItHint(Fixit); } + + C.emitReport(std::move(Report)); } void VirtualCallChecker::registerCtorDtorCallInState(bool IsBeginFunction, @@ -254,34 +200,37 @@ void VirtualCallChecker::registerCtorDtorCallInState(bool IsBeginFunction, } } -void VirtualCallChecker::reportBug(StringRef Msg, bool IsSink, - const MemRegion *Reg, - CheckerContext &C) const { - ExplodedNode *N; - if (IsSink) - N = C.generateErrorNode(); - else - N = C.generateNonFatalErrorNode(); +void ento::registerVirtualCallModeling(CheckerManager &Mgr) { + Mgr.registerChecker<VirtualCallChecker>(); +} - if (!N) - return; - if (!BT) - BT.reset(new BugType( - this, "Call to virtual function during construction or destruction", - "C++ Object Lifecycle")); - - auto Reporter = llvm::make_unique<BugReport>(*BT, Msg, N); - Reporter->addVisitor(llvm::make_unique<VirtualBugVisitor>(Reg)); - C.emitReport(std::move(Reporter)); +void ento::registerPureVirtualCallChecker(CheckerManager &Mgr) { + auto *Chk = Mgr.getChecker<VirtualCallChecker>(); + Chk->BT_Pure = std::make_unique<BugType>(Mgr.getCurrentCheckerName(), + "Pure virtual method call", + categories::CXXObjectLifecycle); } -void ento::registerVirtualCallChecker(CheckerManager &mgr) { - VirtualCallChecker *checker = mgr.registerChecker<VirtualCallChecker>(); +void ento::registerVirtualCallChecker(CheckerManager &Mgr) { + auto *Chk = Mgr.getChecker<VirtualCallChecker>(); + if (!Mgr.getAnalyzerOptions().getCheckerBooleanOption( + Mgr.getCurrentCheckerName(), "PureOnly")) { + Chk->BT_Impure = std::make_unique<BugType>( + Mgr.getCurrentCheckerName(), "Unexpected loss of virtual dispatch", + categories::CXXObjectLifecycle); + Chk->ShowFixIts = Mgr.getAnalyzerOptions().getCheckerBooleanOption( + Mgr.getCurrentCheckerName(), "ShowFixIts"); + } +} + +bool ento::shouldRegisterVirtualCallModeling(const LangOptions &LO) { + return LO.CPlusPlus; +} - checker->IsPureOnly = - mgr.getAnalyzerOptions().getCheckerBooleanOption(checker, "PureOnly"); +bool ento::shouldRegisterPureVirtualCallChecker(const LangOptions &LO) { + return LO.CPlusPlus; } bool ento::shouldRegisterVirtualCallChecker(const LangOptions &LO) { - return true; + return LO.CPlusPlus; } diff --git a/lib/StaticAnalyzer/Checkers/Yaml.h b/lib/StaticAnalyzer/Checkers/Yaml.h new file mode 100755 index 0000000000000..968c50e33f6d6 --- /dev/null +++ b/lib/StaticAnalyzer/Checkers/Yaml.h @@ -0,0 +1,59 @@ +//== Yaml.h ---------------------------------------------------- -*- C++ -*--=// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// +// +// This file defines convenience functions for handling YAML configuration files +// for checkers/packages. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_LIB_STATICANALYZER_CHECKER_YAML_H +#define LLVM_CLANG_LIB_STATICANALYZER_CHECKER_YAML_H + +#include "clang/StaticAnalyzer/Core/CheckerManager.h" +#include "llvm/Support/YAMLTraits.h" + +namespace clang { +namespace ento { + +/// Read the given file from the filesystem and parse it as a yaml file. The +/// template parameter must have a yaml MappingTraits. +/// Emit diagnostic error in case of any failure. +template <class T, class Checker> +llvm::Optional<T> getConfiguration(CheckerManager &Mgr, Checker *Chk, + StringRef Option, StringRef ConfigFile) { + if (ConfigFile.trim().empty()) + return None; + + llvm::vfs::FileSystem *FS = llvm::vfs::getRealFileSystem().get(); + llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> Buffer = + FS->getBufferForFile(ConfigFile.str()); + + if (std::error_code ec = Buffer.getError()) { + Mgr.reportInvalidCheckerOptionValue(Chk, Option, + "a valid filename instead of '" + + std::string(ConfigFile) + "'"); + return None; + } + + llvm::yaml::Input Input(Buffer.get()->getBuffer()); + T Config; + Input >> Config; + + if (std::error_code ec = Input.error()) { + Mgr.reportInvalidCheckerOptionValue(Chk, Option, + "a valid yaml file: " + ec.message()); + return None; + } + + return Config; +} + +} // namespace ento +} // namespace clang + +#endif // LLVM_CLANG_LIB_STATICANALYZER_CHECKERS_MOVE_H |