diff options
Diffstat (limited to 'lib/StaticAnalyzer')
69 files changed, 3675 insertions, 1394 deletions
diff --git a/lib/StaticAnalyzer/Checkers/AnalyzerStatsChecker.cpp b/lib/StaticAnalyzer/Checkers/AnalyzerStatsChecker.cpp index a052d83f5afa1..64c30e7a82c1e 100644 --- a/lib/StaticAnalyzer/Checkers/AnalyzerStatsChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/AnalyzerStatsChecker.cpp @@ -43,7 +43,7 @@ void AnalyzerStatsChecker::checkEndAnalysis(ExplodedGraph &G, ExprEngine &Eng) const { const CFG *C = nullptr; const SourceManager &SM = B.getSourceManager(); - llvm::SmallPtrSet<const CFGBlock*, 256> reachable; + llvm::SmallPtrSet<const CFGBlock*, 32> reachable; // Root node should have the location context of the top most function. const ExplodedNode *GraphRoot = *G.roots_begin(); diff --git a/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp b/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp index f4de733bd7944..13f0f655b89c4 100644 --- a/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp +++ b/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp @@ -211,7 +211,7 @@ void ArrayBoundCheckerV2::reportOOB(CheckerContext &checkerContext, llvm::make_unique<BugReport>(*BT, os.str(), errorNode)); } -void RegionRawOffsetV2::dump() const { +LLVM_DUMP_METHOD void RegionRawOffsetV2::dump() const { dumpToStream(llvm::errs()); } diff --git a/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp b/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp index 26d42ba59c223..6239c5507a4be 100644 --- a/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp +++ b/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp @@ -132,7 +132,7 @@ namespace { void checkPostStmt(const ObjCArrayLiteral *AL, CheckerContext &C) const; }; -} +} // end anonymous namespace void NilArgChecker::warnIfNilExpr(const Expr *E, const char *Msg, @@ -143,7 +143,6 @@ void NilArgChecker::warnIfNilExpr(const Expr *E, if (ExplodedNode *N = C.generateErrorNode()) { generateBugReport(N, Msg, E->getSourceRange(), E, C); } - } } @@ -530,6 +529,7 @@ namespace { class CFRetainReleaseChecker : public Checker< check::PreStmt<CallExpr> > { mutable std::unique_ptr<APIMisuse> BT; mutable IdentifierInfo *Retain, *Release, *MakeCollectable, *Autorelease; + public: CFRetainReleaseChecker() : Retain(nullptr), Release(nullptr), MakeCollectable(nullptr), @@ -538,7 +538,6 @@ public: }; } // end anonymous namespace - void CFRetainReleaseChecker::checkPreStmt(const CallExpr *CE, CheckerContext &C) const { // If the CallExpr doesn't have exactly 1 argument just give up checking. @@ -631,11 +630,10 @@ class ClassReleaseChecker : public Checker<check::PreObjCMessage> { public: void checkPreObjCMessage(const ObjCMethodCall &msg, CheckerContext &C) const; }; -} +} // end anonymous namespace void ClassReleaseChecker::checkPreObjCMessage(const ObjCMethodCall &msg, CheckerContext &C) const { - if (!BT) { BT.reset(new APIMisuse( this, "message incorrectly sent to class instead of class instance")); @@ -692,7 +690,7 @@ class VariadicMethodTypeChecker : public Checker<check::PreObjCMessage> { public: void checkPreObjCMessage(const ObjCMethodCall &msg, CheckerContext &C) const; }; -} +} // end anonymous namespace /// isVariadicMessage - Returns whether the given message is a variadic message, /// where all arguments must be Objective-C types. @@ -855,7 +853,7 @@ public: const CallEvent *Call, PointerEscapeKind Kind) const; }; -} +} // end anonymous namespace static bool isKnownNonNilCollectionType(QualType T) { const ObjCObjectPointerType *PT = T->getAs<ObjCObjectPointerType>(); @@ -983,7 +981,6 @@ assumeCollectionNonEmpty(CheckerContext &C, ProgramStateRef State, return assumeCollectionNonEmpty(C, State, CollectionS, Assumption); } - /// If the fist block edge is a back edge, we are reentering the loop. static bool alreadyExecutedAtLeastOneLoopIteration(const ExplodedNode *N, const ObjCForCollectionStmt *FCS) { @@ -1080,7 +1077,6 @@ void ObjCLoopChecker::checkPostObjCMessage(const ObjCMethodCall &M, C.addTransition(State); } - return; } static SymbolRef getMethodReceiverIfKnownImmutable(const CallEvent *Call) { @@ -1203,7 +1199,7 @@ public: void checkPostObjCMessage(const ObjCMethodCall &M, CheckerContext &C) const; }; -} +} // end anonymous namespace ProgramStateRef ObjCNonNilReturnValueChecker::assumeExprIsNonNull(const Expr *NonNullExpr, diff --git a/lib/StaticAnalyzer/Checkers/CMakeLists.txt b/lib/StaticAnalyzer/Checkers/CMakeLists.txt index 58ff48d6a22a0..62ccc3cb4970b 100644 --- a/lib/StaticAnalyzer/Checkers/CMakeLists.txt +++ b/lib/StaticAnalyzer/Checkers/CMakeLists.txt @@ -1,8 +1,3 @@ -clang_tablegen(Checkers.inc -gen-clang-sa-checkers - -I ${CMAKE_CURRENT_SOURCE_DIR}/../../../include - SOURCE Checkers.td - TARGET ClangSACheckers) - set(LLVM_LINK_COMPONENTS Support ) @@ -46,6 +41,9 @@ add_clang_library(clangStaticAnalyzerCheckers MallocChecker.cpp MallocOverflowSecurityChecker.cpp MallocSizeofChecker.cpp + MPI-Checker/MPIBugReporter.cpp + MPI-Checker/MPIChecker.cpp + MPI-Checker/MPIFunctionClassifier.cpp NSAutoreleasePoolChecker.cpp NSErrorChecker.cpp NoReturnFunctionChecker.cpp @@ -56,6 +54,7 @@ add_clang_library(clangStaticAnalyzerCheckers ObjCContainersChecker.cpp ObjCMissingSuperCallChecker.cpp ObjCSelfInitChecker.cpp + ObjCSuperDeallocChecker.cpp ObjCUnusedIVarsChecker.cpp PaddingChecker.cpp PointerArithChecker.cpp diff --git a/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/lib/StaticAnalyzer/Checkers/CStringChecker.cpp index 17537445d66c5..e9512977fa6d1 100644 --- a/lib/StaticAnalyzer/Checkers/CStringChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/CStringChecker.cpp @@ -118,6 +118,10 @@ public: void evalStrsep(CheckerContext &C, const CallExpr *CE) const; + void evalStdCopy(CheckerContext &C, const CallExpr *CE) const; + void evalStdCopyBackward(CheckerContext &C, const CallExpr *CE) const; + void evalStdCopyCommon(CheckerContext &C, const CallExpr *CE) const; + // Utility methods std::pair<ProgramStateRef , ProgramStateRef > static assumeZero(CheckerContext &C, @@ -916,7 +920,7 @@ ProgramStateRef CStringChecker::InvalidateBuffer(CheckerContext &C, // Invalidate and escape only indirect regions accessible through the source // buffer. if (IsSourceBuffer) { - ITraits.setTrait(R, + ITraits.setTrait(R->getBaseRegion(), RegionAndSymbolInvalidationTraits::TK_PreserveContents); ITraits.setTrait(R, RegionAndSymbolInvalidationTraits::TK_SuppressEscape); CausesPointerEscape = true; @@ -1833,6 +1837,8 @@ void CStringChecker::evalStrcmpCommon(CheckerContext &C, const CallExpr *CE, const StringLiteral *s1StrLiteral = getCStringLiteral(C, state, s1, s1Val); const StringLiteral *s2StrLiteral = getCStringLiteral(C, state, s2, s2Val); bool canComputeResult = false; + SVal resultVal = svalBuilder.conjureSymbolVal(nullptr, CE, LCtx, + C.blockCount()); if (s1StrLiteral && s2StrLiteral) { StringRef s1StrRef = s1StrLiteral->getString(); @@ -1866,28 +1872,29 @@ void CStringChecker::evalStrcmpCommon(CheckerContext &C, const CallExpr *CE, s2StrRef = s2StrRef.substr(0, s2Term); // Use StringRef's comparison methods to compute the actual result. - int result; + int compareRes = ignoreCase ? s1StrRef.compare_lower(s2StrRef) + : s1StrRef.compare(s2StrRef); - if (ignoreCase) { - // Compare string 1 to string 2 the same way strcasecmp() does. - result = s1StrRef.compare_lower(s2StrRef); - } else { - // Compare string 1 to string 2 the same way strcmp() does. - result = s1StrRef.compare(s2StrRef); + // The strcmp function returns an integer greater than, equal to, or less + // than zero, [c11, p7.24.4.2]. + if (compareRes == 0) { + resultVal = svalBuilder.makeIntVal(compareRes, CE->getType()); + } + else { + DefinedSVal zeroVal = svalBuilder.makeIntVal(0, CE->getType()); + // Constrain strcmp's result range based on the result of StringRef's + // comparison methods. + BinaryOperatorKind op = (compareRes == 1) ? BO_GT : BO_LT; + SVal compareWithZero = + svalBuilder.evalBinOp(state, op, resultVal, zeroVal, + svalBuilder.getConditionType()); + DefinedSVal compareWithZeroVal = compareWithZero.castAs<DefinedSVal>(); + state = state->assume(compareWithZeroVal, true); } - - // Build the SVal of the comparison and bind the return value. - SVal resultVal = svalBuilder.makeIntVal(result, CE->getType()); - state = state->BindExpr(CE, LCtx, resultVal); } } - if (!canComputeResult) { - // Conjure a symbolic value. It's the best we can do. - SVal resultVal = svalBuilder.conjureSymbolVal(nullptr, CE, LCtx, - C.blockCount()); - state = state->BindExpr(CE, LCtx, resultVal); - } + state = state->BindExpr(CE, LCtx, resultVal); // Record this as a possible path. C.addTransition(state); @@ -1950,7 +1957,57 @@ void CStringChecker::evalStrsep(CheckerContext &C, const CallExpr *CE) const { C.addTransition(State); } +// These should probably be moved into a C++ standard library checker. +void CStringChecker::evalStdCopy(CheckerContext &C, const CallExpr *CE) const { + evalStdCopyCommon(C, CE); +} + +void CStringChecker::evalStdCopyBackward(CheckerContext &C, + const CallExpr *CE) const { + evalStdCopyCommon(C, CE); +} + +void CStringChecker::evalStdCopyCommon(CheckerContext &C, + const CallExpr *CE) const { + if (CE->getNumArgs() < 3) + return; + + ProgramStateRef State = C.getState(); + + const LocationContext *LCtx = C.getLocationContext(); + // template <class _InputIterator, class _OutputIterator> + // _OutputIterator + // copy(_InputIterator __first, _InputIterator __last, + // _OutputIterator __result) + + // Invalidate the destination buffer + const Expr *Dst = CE->getArg(2); + SVal DstVal = State->getSVal(Dst, LCtx); + State = InvalidateBuffer(C, State, Dst, DstVal, /*IsSource=*/false, + /*Size=*/nullptr); + + SValBuilder &SVB = C.getSValBuilder(); + + SVal ResultVal = SVB.conjureSymbolVal(nullptr, CE, LCtx, C.blockCount()); + State = State->BindExpr(CE, LCtx, ResultVal); + + C.addTransition(State); +} + +static bool isCPPStdLibraryFunction(const FunctionDecl *FD, StringRef Name) { + IdentifierInfo *II = FD->getIdentifier(); + if (!II) + return false; + + if (!AnalysisDeclContext::isInStdNamespace(FD)) + return false; + + if (II->getName().equals(Name)) + return true; + + return false; +} //===----------------------------------------------------------------------===// // The driver method, and other Checker callbacks. //===----------------------------------------------------------------------===// @@ -1999,6 +2056,10 @@ bool CStringChecker::evalCall(const CallExpr *CE, CheckerContext &C) const { evalFunction = &CStringChecker::evalBcopy; else if (C.isCLibraryFunction(FDecl, "bcmp")) evalFunction = &CStringChecker::evalMemcmp; + else if (isCPPStdLibraryFunction(FDecl, "copy")) + evalFunction = &CStringChecker::evalStdCopy; + else if (isCPPStdLibraryFunction(FDecl, "copy_backward")) + evalFunction = &CStringChecker::evalStdCopyBackward; // If the callee isn't a string function, let another checker handle it. if (!evalFunction) diff --git a/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp b/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp index 1459083769968..5126716fcded1 100644 --- a/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp @@ -313,7 +313,7 @@ void CallAndMessageChecker::checkPreStmt(const CallExpr *CE, if (L.isUndef()) { if (!BT_call_undef) BT_call_undef.reset(new BuiltinBug( - this, "Called function pointer is an uninitalized pointer value")); + this, "Called function pointer is an uninitialized pointer value")); emitBadCall(BT_call_undef.get(), C, Callee); return; } diff --git a/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp b/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp index 25caa00025981..9e863e79e41fc 100644 --- a/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp +++ b/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp @@ -7,9 +7,24 @@ // //===----------------------------------------------------------------------===// // -// This file defines a CheckObjCDealloc, a checker that -// analyzes an Objective-C class's implementation to determine if it -// correctly implements -dealloc. +// This checker analyzes Objective-C -dealloc methods and their callees +// to warn about improper releasing of instance variables that back synthesized +// properties. It warns about missing releases in the following cases: +// - When a class has a synthesized instance variable for a 'retain' or 'copy' +// property and lacks a -dealloc method in its implementation. +// - When a class has a synthesized instance variable for a 'retain'/'copy' +// property but the ivar is not released in -dealloc by either -release +// or by nilling out the property. +// +// It warns about extra releases in -dealloc (but not in callees) when a +// synthesized instance variable is released in the following cases: +// - When the property is 'assign' and is not 'readonly'. +// - When the property is 'weak'. +// +// This checker only warns for instance variables synthesized to back +// properties. Handling the more general case would require inferring whether +// an instance variable is stored retained or not. For synthesized properties, +// this is specified in the property declaration itself. // //===----------------------------------------------------------------------===// @@ -20,229 +35,1035 @@ #include "clang/AST/ExprObjC.h" #include "clang/Basic/LangOptions.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" +#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h" #include "llvm/Support/raw_ostream.h" using namespace clang; using namespace ento; -static bool scan_ivar_release(Stmt *S, ObjCIvarDecl *ID, - const ObjCPropertyDecl *PD, - Selector Release, - IdentifierInfo* SelfII, - ASTContext &Ctx) { - - // [mMyIvar release] - if (ObjCMessageExpr *ME = dyn_cast<ObjCMessageExpr>(S)) - if (ME->getSelector() == Release) - if (ME->getInstanceReceiver()) - if (Expr *Receiver = ME->getInstanceReceiver()->IgnoreParenCasts()) - if (ObjCIvarRefExpr *E = dyn_cast<ObjCIvarRefExpr>(Receiver)) - if (E->getDecl() == ID) - return true; - - // [self setMyIvar:nil]; - if (ObjCMessageExpr *ME = dyn_cast<ObjCMessageExpr>(S)) - if (ME->getInstanceReceiver()) - if (Expr *Receiver = ME->getInstanceReceiver()->IgnoreParenCasts()) - if (DeclRefExpr *E = dyn_cast<DeclRefExpr>(Receiver)) - if (E->getDecl()->getIdentifier() == SelfII) - if (ME->getMethodDecl() == PD->getSetterMethodDecl() && - ME->getNumArgs() == 1 && - ME->getArg(0)->isNullPointerConstant(Ctx, - Expr::NPC_ValueDependentIsNull)) - return true; - - // self.myIvar = nil; - if (BinaryOperator* BO = dyn_cast<BinaryOperator>(S)) - if (BO->isAssignmentOp()) - if (ObjCPropertyRefExpr *PRE = - dyn_cast<ObjCPropertyRefExpr>(BO->getLHS()->IgnoreParenCasts())) - if (PRE->isExplicitProperty() && PRE->getExplicitProperty() == PD) - if (BO->getRHS()->isNullPointerConstant(Ctx, - Expr::NPC_ValueDependentIsNull)) { - // This is only a 'release' if the property kind is not - // 'assign'. - return PD->getSetterKind() != ObjCPropertyDecl::Assign; - } - - // Recurse to children. - for (Stmt *SubStmt : S->children()) - if (SubStmt && scan_ivar_release(SubStmt, ID, PD, Release, SelfII, Ctx)) - return true; +/// Indicates whether an instance variable is required to be released in +/// -dealloc. +enum class ReleaseRequirement { + /// The instance variable must be released, either by calling + /// -release on it directly or by nilling it out with a property setter. + MustRelease, - return false; + /// The instance variable must not be directly released with -release. + MustNotReleaseDirectly, + + /// The requirement for the instance variable could not be determined. + Unknown +}; + +/// Returns true if the property implementation is synthesized and the +/// type of the property is retainable. +static bool isSynthesizedRetainableProperty(const ObjCPropertyImplDecl *I, + const ObjCIvarDecl **ID, + const ObjCPropertyDecl **PD) { + + if (I->getPropertyImplementation() != ObjCPropertyImplDecl::Synthesize) + return false; + + (*ID) = I->getPropertyIvarDecl(); + if (!(*ID)) + return false; + + QualType T = (*ID)->getType(); + if (!T->isObjCRetainableType()) + return false; + + (*PD) = I->getPropertyDecl(); + // Shouldn't be able to synthesize a property that doesn't exist. + assert(*PD); + + return true; } -static void checkObjCDealloc(const CheckerBase *Checker, - const ObjCImplementationDecl *D, - const LangOptions &LOpts, BugReporter &BR) { +namespace { - assert (LOpts.getGC() != LangOptions::GCOnly); +class ObjCDeallocChecker + : public Checker<check::ASTDecl<ObjCImplementationDecl>, + check::PreObjCMessage, check::PostObjCMessage, + check::PreCall, + check::BeginFunction, check::EndFunction, + eval::Assume, + check::PointerEscape, + check::PreStmt<ReturnStmt>> { - ASTContext &Ctx = BR.getContext(); - const ObjCInterfaceDecl *ID = D->getClassInterface(); + mutable IdentifierInfo *NSObjectII, *SenTestCaseII, *XCTestCaseII, + *Block_releaseII, *CIFilterII; - // Does the class contain any ivars that are pointers (or id<...>)? - // If not, skip the check entirely. - // NOTE: This is motivated by PR 2517: - // http://llvm.org/bugs/show_bug.cgi?id=2517 + mutable Selector DeallocSel, ReleaseSel; - bool containsPointerIvar = false; + std::unique_ptr<BugType> MissingReleaseBugType; + std::unique_ptr<BugType> ExtraReleaseBugType; + std::unique_ptr<BugType> MistakenDeallocBugType; - for (const auto *Ivar : ID->ivars()) { - QualType T = Ivar->getType(); +public: + ObjCDeallocChecker(); - if (!T->isObjCObjectPointerType() || - Ivar->hasAttr<IBOutletAttr>() || // Skip IBOutlets. - Ivar->hasAttr<IBOutletCollectionAttr>()) // Skip IBOutletCollections. - continue; + void checkASTDecl(const ObjCImplementationDecl *D, AnalysisManager& Mgr, + BugReporter &BR) const; + void checkBeginFunction(CheckerContext &Ctx) const; + void checkPreObjCMessage(const ObjCMethodCall &M, CheckerContext &C) const; + void checkPreCall(const CallEvent &Call, CheckerContext &C) const; + void checkPostObjCMessage(const ObjCMethodCall &M, CheckerContext &C) const; - containsPointerIvar = true; - break; - } + ProgramStateRef evalAssume(ProgramStateRef State, SVal Cond, + bool Assumption) const; - if (!containsPointerIvar) - return; + ProgramStateRef checkPointerEscape(ProgramStateRef State, + const InvalidatedSymbols &Escaped, + const CallEvent *Call, + PointerEscapeKind Kind) const; + void checkPreStmt(const ReturnStmt *RS, CheckerContext &C) const; + void checkEndFunction(CheckerContext &Ctx) const; - // Determine if the class subclasses NSObject. - IdentifierInfo* NSObjectII = &Ctx.Idents.get("NSObject"); - IdentifierInfo* SenTestCaseII = &Ctx.Idents.get("SenTestCase"); +private: + void diagnoseMissingReleases(CheckerContext &C) const; + bool diagnoseExtraRelease(SymbolRef ReleasedValue, const ObjCMethodCall &M, + CheckerContext &C) const; - for ( ; ID ; ID = ID->getSuperClass()) { - IdentifierInfo *II = ID->getIdentifier(); + bool diagnoseMistakenDealloc(SymbolRef DeallocedValue, + const ObjCMethodCall &M, + CheckerContext &C) const; - if (II == NSObjectII) - break; + SymbolRef getValueReleasedByNillingOut(const ObjCMethodCall &M, + CheckerContext &C) const; - // FIXME: For now, ignore classes that subclass SenTestCase, as these don't - // need to implement -dealloc. They implement tear down in another way, - // which we should try and catch later. - // http://llvm.org/bugs/show_bug.cgi?id=3187 - if (II == SenTestCaseII) - return; + const ObjCIvarRegion *getIvarRegionForIvarSymbol(SymbolRef IvarSym) const; + SymbolRef getInstanceSymbolFromIvarSymbol(SymbolRef IvarSym) const; + + const ObjCPropertyImplDecl* + findPropertyOnDeallocatingInstance(SymbolRef IvarSym, + CheckerContext &C) const; + + ReleaseRequirement + getDeallocReleaseRequirement(const ObjCPropertyImplDecl *PropImpl) const; + + bool isInInstanceDealloc(const CheckerContext &C, SVal &SelfValOut) const; + bool isInInstanceDealloc(const CheckerContext &C, const LocationContext *LCtx, + SVal &SelfValOut) const; + bool instanceDeallocIsOnStack(const CheckerContext &C, + SVal &InstanceValOut) const; + + bool isSuperDeallocMessage(const ObjCMethodCall &M) const; + + const ObjCImplDecl *getContainingObjCImpl(const LocationContext *LCtx) const; + + const ObjCPropertyDecl * + findShadowedPropertyDecl(const ObjCPropertyImplDecl *PropImpl) const; + + void transitionToReleaseValue(CheckerContext &C, SymbolRef Value) const; + ProgramStateRef removeValueRequiringRelease(ProgramStateRef State, + SymbolRef InstanceSym, + SymbolRef ValueSym) const; + + void initIdentifierInfoAndSelectors(ASTContext &Ctx) const; + + bool classHasSeparateTeardown(const ObjCInterfaceDecl *ID) const; + + bool isReleasedByCIFilterDealloc(const ObjCPropertyImplDecl *PropImpl) const; +}; +} // End anonymous namespace. + +typedef llvm::ImmutableSet<SymbolRef> SymbolSet; + +/// Maps from the symbol for a class instance to the set of +/// symbols remaining that must be released in -dealloc. +REGISTER_MAP_WITH_PROGRAMSTATE(UnreleasedIvarMap, SymbolRef, SymbolSet) + +namespace clang { +namespace ento { +template<> struct ProgramStateTrait<SymbolSet> +: public ProgramStatePartialTrait<SymbolSet> { + static void *GDMIndex() { static int index = 0; return &index; } +}; +} +} + +/// An AST check that diagnose when the class requires a -dealloc method and +/// is missing one. +void ObjCDeallocChecker::checkASTDecl(const ObjCImplementationDecl *D, + AnalysisManager &Mgr, + BugReporter &BR) const { + assert(Mgr.getLangOpts().getGC() != LangOptions::GCOnly); + assert(!Mgr.getLangOpts().ObjCAutoRefCount); + initIdentifierInfoAndSelectors(Mgr.getASTContext()); + + const ObjCInterfaceDecl *ID = D->getClassInterface(); + // If the class is known to have a lifecycle with a separate teardown method + // then it may not require a -dealloc method. + if (classHasSeparateTeardown(ID)) + return; + + // Does the class contain any synthesized properties that are retainable? + // If not, skip the check entirely. + const ObjCPropertyImplDecl *PropImplRequiringRelease = nullptr; + bool HasOthers = false; + for (const auto *I : D->property_impls()) { + if (getDeallocReleaseRequirement(I) == ReleaseRequirement::MustRelease) { + if (!PropImplRequiringRelease) + PropImplRequiringRelease = I; + else { + HasOthers = true; + break; + } + } } - if (!ID) + if (!PropImplRequiringRelease) return; - // Get the "dealloc" selector. - IdentifierInfo* II = &Ctx.Idents.get("dealloc"); - Selector S = Ctx.Selectors.getSelector(0, &II); const ObjCMethodDecl *MD = nullptr; // Scan the instance methods for "dealloc". for (const auto *I : D->instance_methods()) { - if (I->getSelector() == S) { + if (I->getSelector() == DeallocSel) { MD = I; break; } } - PathDiagnosticLocation DLoc = - PathDiagnosticLocation::createBegin(D, BR.getSourceManager()); - if (!MD) { // No dealloc found. + const char* Name = "Missing -dealloc"; - const char* name = LOpts.getGC() == LangOptions::NonGC - ? "missing -dealloc" - : "missing -dealloc (Hybrid MM, non-GC)"; + std::string Buf; + llvm::raw_string_ostream OS(Buf); + OS << "'" << *D << "' lacks a 'dealloc' instance method but " + << "must release '" << *PropImplRequiringRelease->getPropertyIvarDecl() + << "'"; - std::string buf; - llvm::raw_string_ostream os(buf); - os << "Objective-C class '" << *D << "' lacks a 'dealloc' instance method"; + if (HasOthers) + OS << " and others"; + PathDiagnosticLocation DLoc = + PathDiagnosticLocation::createBegin(D, BR.getSourceManager()); - BR.EmitBasicReport(D, Checker, name, categories::CoreFoundationObjectiveC, - os.str(), DLoc); + BR.EmitBasicReport(D, this, Name, categories::CoreFoundationObjectiveC, + OS.str(), DLoc); return; } +} - // Get the "release" selector. - IdentifierInfo* RII = &Ctx.Idents.get("release"); - Selector RS = Ctx.Selectors.getSelector(0, &RII); +/// If this is the beginning of -dealloc, mark the values initially stored in +/// instance variables that must be released by the end of -dealloc +/// as unreleased in the state. +void ObjCDeallocChecker::checkBeginFunction( + CheckerContext &C) const { + initIdentifierInfoAndSelectors(C.getASTContext()); - // Get the "self" identifier - IdentifierInfo* SelfII = &Ctx.Idents.get("self"); + // Only do this if the current method is -dealloc. + SVal SelfVal; + if (!isInInstanceDealloc(C, SelfVal)) + return; - // Scan for missing and extra releases of ivars used by implementations - // of synthesized properties - for (const auto *I : D->property_impls()) { - // We can only check the synthesized properties - if (I->getPropertyImplementation() != ObjCPropertyImplDecl::Synthesize) + SymbolRef SelfSymbol = SelfVal.getAsSymbol(); + + const LocationContext *LCtx = C.getLocationContext(); + ProgramStateRef InitialState = C.getState(); + + ProgramStateRef State = InitialState; + + SymbolSet::Factory &F = State->getStateManager().get_context<SymbolSet>(); + + // Symbols that must be released by the end of the -dealloc; + SymbolSet RequiredReleases = F.getEmptySet(); + + // If we're an inlined -dealloc, we should add our symbols to the existing + // set from our subclass. + if (const SymbolSet *CurrSet = State->get<UnreleasedIvarMap>(SelfSymbol)) + RequiredReleases = *CurrSet; + + for (auto *PropImpl : getContainingObjCImpl(LCtx)->property_impls()) { + ReleaseRequirement Requirement = getDeallocReleaseRequirement(PropImpl); + if (Requirement != ReleaseRequirement::MustRelease) continue; - ObjCIvarDecl *ID = I->getPropertyIvarDecl(); - if (!ID) + SVal LVal = State->getLValue(PropImpl->getPropertyIvarDecl(), SelfVal); + Optional<Loc> LValLoc = LVal.getAs<Loc>(); + if (!LValLoc) continue; - QualType T = ID->getType(); - if (!T->isObjCObjectPointerType()) // Skip non-pointer ivars + SVal InitialVal = State->getSVal(LValLoc.getValue()); + SymbolRef Symbol = InitialVal.getAsSymbol(); + if (!Symbol || !isa<SymbolRegionValue>(Symbol)) continue; - const ObjCPropertyDecl *PD = I->getPropertyDecl(); - if (!PD) + // Mark the value as requiring a release. + RequiredReleases = F.add(RequiredReleases, Symbol); + } + + if (!RequiredReleases.isEmpty()) { + State = State->set<UnreleasedIvarMap>(SelfSymbol, RequiredReleases); + } + + if (State != InitialState) { + C.addTransition(State); + } +} + +/// Given a symbol for an ivar, return the ivar region it was loaded from. +/// Returns nullptr if the instance symbol cannot be found. +const ObjCIvarRegion * +ObjCDeallocChecker::getIvarRegionForIvarSymbol(SymbolRef IvarSym) const { + return dyn_cast_or_null<ObjCIvarRegion>(IvarSym->getOriginRegion()); +} + +/// Given a symbol for an ivar, return a symbol for the instance containing +/// the ivar. Returns nullptr if the instance symbol cannot be found. +SymbolRef +ObjCDeallocChecker::getInstanceSymbolFromIvarSymbol(SymbolRef IvarSym) const { + + const ObjCIvarRegion *IvarRegion = getIvarRegionForIvarSymbol(IvarSym); + if (!IvarRegion) + return nullptr; + + return IvarRegion->getSymbolicBase()->getSymbol(); +} + +/// If we are in -dealloc or -dealloc is on the stack, handle the call if it is +/// a release or a nilling-out property setter. +void ObjCDeallocChecker::checkPreObjCMessage( + const ObjCMethodCall &M, CheckerContext &C) const { + // Only run if -dealloc is on the stack. + SVal DeallocedInstance; + if (!instanceDeallocIsOnStack(C, DeallocedInstance)) + return; + + SymbolRef ReleasedValue = nullptr; + + if (M.getSelector() == ReleaseSel) { + ReleasedValue = M.getReceiverSVal().getAsSymbol(); + } else if (M.getSelector() == DeallocSel && !M.isReceiverSelfOrSuper()) { + if (diagnoseMistakenDealloc(M.getReceiverSVal().getAsSymbol(), M, C)) + return; + } + + if (ReleasedValue) { + // An instance variable symbol was released with -release: + // [_property release]; + if (diagnoseExtraRelease(ReleasedValue,M, C)) + return; + } else { + // An instance variable symbol was released nilling out its property: + // self.property = nil; + ReleasedValue = getValueReleasedByNillingOut(M, C); + } + + if (!ReleasedValue) + return; + + transitionToReleaseValue(C, ReleasedValue); +} + +/// If we are in -dealloc or -dealloc is on the stack, handle the call if it is +/// call to Block_release(). +void ObjCDeallocChecker::checkPreCall(const CallEvent &Call, + CheckerContext &C) const { + const IdentifierInfo *II = Call.getCalleeIdentifier(); + if (II != Block_releaseII) + return; + + if (Call.getNumArgs() != 1) + return; + + SymbolRef ReleasedValue = Call.getArgSVal(0).getAsSymbol(); + if (!ReleasedValue) + return; + + transitionToReleaseValue(C, ReleasedValue); +} +/// If the message was a call to '[super dealloc]', diagnose any missing +/// releases. +void ObjCDeallocChecker::checkPostObjCMessage( + const ObjCMethodCall &M, CheckerContext &C) const { + // We perform this check post-message so that if the super -dealloc + // calls a helper method and that this class overrides, any ivars released in + // the helper method will be recorded before checking. + if (isSuperDeallocMessage(M)) + diagnoseMissingReleases(C); +} + +/// Check for missing releases even when -dealloc does not call +/// '[super dealloc]'. +void ObjCDeallocChecker::checkEndFunction( + CheckerContext &C) const { + diagnoseMissingReleases(C); +} + +/// Check for missing releases on early return. +void ObjCDeallocChecker::checkPreStmt( + const ReturnStmt *RS, CheckerContext &C) const { + diagnoseMissingReleases(C); +} + +/// When a symbol is assumed to be nil, remove it from the set of symbols +/// require to be nil. +ProgramStateRef ObjCDeallocChecker::evalAssume(ProgramStateRef State, SVal Cond, + bool Assumption) const { + if (State->get<UnreleasedIvarMap>().isEmpty()) + return State; + + auto *CondBSE = dyn_cast_or_null<BinarySymExpr>(Cond.getAsSymExpr()); + if (!CondBSE) + return State; + + BinaryOperator::Opcode OpCode = CondBSE->getOpcode(); + if (Assumption) { + if (OpCode != BO_EQ) + return State; + } else { + if (OpCode != BO_NE) + return State; + } + + SymbolRef NullSymbol = nullptr; + if (auto *SIE = dyn_cast<SymIntExpr>(CondBSE)) { + const llvm::APInt &RHS = SIE->getRHS(); + if (RHS != 0) + return State; + NullSymbol = SIE->getLHS(); + } else if (auto *SIE = dyn_cast<IntSymExpr>(CondBSE)) { + const llvm::APInt &LHS = SIE->getLHS(); + if (LHS != 0) + return State; + NullSymbol = SIE->getRHS(); + } else { + return State; + } + + SymbolRef InstanceSymbol = getInstanceSymbolFromIvarSymbol(NullSymbol); + if (!InstanceSymbol) + return State; + + State = removeValueRequiringRelease(State, InstanceSymbol, NullSymbol); + + return State; +} + +/// If a symbol escapes conservatively assume unseen code released it. +ProgramStateRef ObjCDeallocChecker::checkPointerEscape( + ProgramStateRef State, const InvalidatedSymbols &Escaped, + const CallEvent *Call, PointerEscapeKind Kind) const { + + if (State->get<UnreleasedIvarMap>().isEmpty()) + return State; + + // Don't treat calls to '[super dealloc]' as escaping for the purposes + // of this checker. Because the checker diagnoses missing releases in the + // post-message handler for '[super dealloc], escaping here would cause + // the checker to never warn. + auto *OMC = dyn_cast_or_null<ObjCMethodCall>(Call); + if (OMC && isSuperDeallocMessage(*OMC)) + return State; + + for (const auto &Sym : Escaped) { + if (!Call || (Call && !Call->isInSystemHeader())) { + // If Sym is a symbol for an object with instance variables that + // must be released, remove these obligations when the object escapes + // unless via a call to a system function. System functions are + // very unlikely to release instance variables on objects passed to them, + // and are frequently called on 'self' in -dealloc (e.g., to remove + // observers) -- we want to avoid false negatives from escaping on + // them. + State = State->remove<UnreleasedIvarMap>(Sym); + } + + + SymbolRef InstanceSymbol = getInstanceSymbolFromIvarSymbol(Sym); + if (!InstanceSymbol) + continue; + + State = removeValueRequiringRelease(State, InstanceSymbol, Sym); + } + + return State; +} + +/// Report any unreleased instance variables for the current instance being +/// dealloced. +void ObjCDeallocChecker::diagnoseMissingReleases(CheckerContext &C) const { + ProgramStateRef State = C.getState(); + + SVal SelfVal; + if (!isInInstanceDealloc(C, SelfVal)) + return; + + const MemRegion *SelfRegion = SelfVal.castAs<loc::MemRegionVal>().getRegion(); + const LocationContext *LCtx = C.getLocationContext(); + + ExplodedNode *ErrNode = nullptr; + + SymbolRef SelfSym = SelfVal.getAsSymbol(); + if (!SelfSym) + return; + + const SymbolSet *OldUnreleased = State->get<UnreleasedIvarMap>(SelfSym); + if (!OldUnreleased) + return; + + SymbolSet NewUnreleased = *OldUnreleased; + SymbolSet::Factory &F = State->getStateManager().get_context<SymbolSet>(); + + ProgramStateRef InitialState = State; + + for (auto *IvarSymbol : *OldUnreleased) { + const TypedValueRegion *TVR = + cast<SymbolRegionValue>(IvarSymbol)->getRegion(); + const ObjCIvarRegion *IvarRegion = cast<ObjCIvarRegion>(TVR); + + // Don't warn if the ivar is not for this instance. + if (SelfRegion != IvarRegion->getSuperRegion()) continue; - // ivars cannot be set via read-only properties, so we'll skip them - if (PD->isReadOnly()) + const ObjCIvarDecl *IvarDecl = IvarRegion->getDecl(); + // Prevent an inlined call to -dealloc in a super class from warning + // about the values the subclass's -dealloc should release. + if (IvarDecl->getContainingInterface() != + cast<ObjCMethodDecl>(LCtx->getDecl())->getClassInterface()) continue; - // ivar must be released if and only if the kind of setter was not 'assign' - bool requiresRelease = PD->getSetterKind() != ObjCPropertyDecl::Assign; - if (scan_ivar_release(MD->getBody(), ID, PD, RS, SelfII, Ctx) - != requiresRelease) { - const char *name = nullptr; - std::string buf; - llvm::raw_string_ostream os(buf); - - if (requiresRelease) { - name = LOpts.getGC() == LangOptions::NonGC - ? "missing ivar release (leak)" - : "missing ivar release (Hybrid MM, non-GC)"; - - os << "The '" << *ID - << "' instance variable was retained by a synthesized property but " - "wasn't released in 'dealloc'"; - } else { - name = LOpts.getGC() == LangOptions::NonGC - ? "extra ivar release (use-after-release)" - : "extra ivar release (Hybrid MM, non-GC)"; - - os << "The '" << *ID - << "' instance variable was not retained by a synthesized property " - "but was released in 'dealloc'"; - } + // Prevents diagnosing multiple times for the same instance variable + // at, for example, both a return and at the end of of the function. + NewUnreleased = F.remove(NewUnreleased, IvarSymbol); + + if (State->getStateManager() + .getConstraintManager() + .isNull(State, IvarSymbol) + .isConstrainedTrue()) { + continue; + } + + // A missing release manifests as a leak, so treat as a non-fatal error. + if (!ErrNode) + ErrNode = C.generateNonFatalErrorNode(); + // If we've already reached this node on another path, return without + // diagnosing. + if (!ErrNode) + return; + + std::string Buf; + llvm::raw_string_ostream OS(Buf); + + const ObjCInterfaceDecl *Interface = IvarDecl->getContainingInterface(); + // If the class is known to have a lifecycle with teardown that is + // separate from -dealloc, do not warn about missing releases. We + // suppress here (rather than not tracking for instance variables in + // such classes) because these classes are rare. + if (classHasSeparateTeardown(Interface)) + return; + + ObjCImplDecl *ImplDecl = Interface->getImplementation(); + + const ObjCPropertyImplDecl *PropImpl = + ImplDecl->FindPropertyImplIvarDecl(IvarDecl->getIdentifier()); - PathDiagnosticLocation SDLoc = - PathDiagnosticLocation::createBegin(I, BR.getSourceManager()); + const ObjCPropertyDecl *PropDecl = PropImpl->getPropertyDecl(); - BR.EmitBasicReport(MD, Checker, name, - categories::CoreFoundationObjectiveC, os.str(), SDLoc); + assert(PropDecl->getSetterKind() == ObjCPropertyDecl::Copy || + PropDecl->getSetterKind() == ObjCPropertyDecl::Retain); + + OS << "The '" << *IvarDecl << "' ivar in '" << *ImplDecl + << "' was "; + + if (PropDecl->getSetterKind() == ObjCPropertyDecl::Retain) + OS << "retained"; + else + OS << "copied"; + + OS << " by a synthesized property but not released" + " before '[super dealloc]'"; + + std::unique_ptr<BugReport> BR( + new BugReport(*MissingReleaseBugType, OS.str(), ErrNode)); + + C.emitReport(std::move(BR)); + } + + if (NewUnreleased.isEmpty()) { + State = State->remove<UnreleasedIvarMap>(SelfSym); + } else { + State = State->set<UnreleasedIvarMap>(SelfSym, NewUnreleased); + } + + if (ErrNode) { + C.addTransition(State, ErrNode); + } else if (State != InitialState) { + C.addTransition(State); + } + + // Make sure that after checking in the top-most frame the list of + // tracked ivars is empty. This is intended to detect accidental leaks in + // the UnreleasedIvarMap program state. + assert(!LCtx->inTopFrame() || State->get<UnreleasedIvarMap>().isEmpty()); +} + +/// Given a symbol, determine whether the symbol refers to an ivar on +/// the top-most deallocating instance. If so, find the property for that +/// ivar, if one exists. Otherwise return null. +const ObjCPropertyImplDecl * +ObjCDeallocChecker::findPropertyOnDeallocatingInstance( + SymbolRef IvarSym, CheckerContext &C) const { + SVal DeallocedInstance; + if (!isInInstanceDealloc(C, DeallocedInstance)) + return nullptr; + + // Try to get the region from which the ivar value was loaded. + auto *IvarRegion = getIvarRegionForIvarSymbol(IvarSym); + if (!IvarRegion) + return nullptr; + + // Don't try to find the property if the ivar was not loaded from the + // given instance. + if (DeallocedInstance.castAs<loc::MemRegionVal>().getRegion() != + IvarRegion->getSuperRegion()) + return nullptr; + + const LocationContext *LCtx = C.getLocationContext(); + const ObjCIvarDecl *IvarDecl = IvarRegion->getDecl(); + + const ObjCImplDecl *Container = getContainingObjCImpl(LCtx); + const ObjCPropertyImplDecl *PropImpl = + Container->FindPropertyImplIvarDecl(IvarDecl->getIdentifier()); + return PropImpl; +} + +/// Emits a warning if the current context is -dealloc and ReleasedValue +/// must not be directly released in a -dealloc. Returns true if a diagnostic +/// was emitted. +bool ObjCDeallocChecker::diagnoseExtraRelease(SymbolRef ReleasedValue, + const ObjCMethodCall &M, + CheckerContext &C) const { + // Try to get the region from which the the released value was loaded. + // Note that, unlike diagnosing for missing releases, here we don't track + // values that must not be released in the state. This is because even if + // these values escape, it is still an error under the rules of MRR to + // release them in -dealloc. + const ObjCPropertyImplDecl *PropImpl = + findPropertyOnDeallocatingInstance(ReleasedValue, C); + + if (!PropImpl) + return false; + + // If the ivar belongs to a property that must not be released directly + // in dealloc, emit a warning. + if (getDeallocReleaseRequirement(PropImpl) != + ReleaseRequirement::MustNotReleaseDirectly) { + return false; + } + + // If the property is readwrite but it shadows a read-only property in its + // external interface, treat the property a read-only. If the outside + // world cannot write to a property then the internal implementation is free + // to make its own convention about whether the value is stored retained + // or not. We look up the shadow here rather than in + // getDeallocReleaseRequirement() because doing so can be expensive. + const ObjCPropertyDecl *PropDecl = findShadowedPropertyDecl(PropImpl); + if (PropDecl) { + if (PropDecl->isReadOnly()) + return false; + } else { + PropDecl = PropImpl->getPropertyDecl(); + } + + ExplodedNode *ErrNode = C.generateNonFatalErrorNode(); + if (!ErrNode) + return false; + + std::string Buf; + llvm::raw_string_ostream OS(Buf); + + assert(PropDecl->getSetterKind() == ObjCPropertyDecl::Weak || + (PropDecl->getSetterKind() == ObjCPropertyDecl::Assign && + !PropDecl->isReadOnly()) || + isReleasedByCIFilterDealloc(PropImpl) + ); + + const ObjCImplDecl *Container = getContainingObjCImpl(C.getLocationContext()); + OS << "The '" << *PropImpl->getPropertyIvarDecl() + << "' ivar in '" << *Container; + + + if (isReleasedByCIFilterDealloc(PropImpl)) { + OS << "' will be released by '-[CIFilter dealloc]' but also released here"; + } else { + OS << "' was synthesized for "; + + if (PropDecl->getSetterKind() == ObjCPropertyDecl::Weak) + OS << "a weak"; + else + OS << "an assign, readwrite"; + + OS << " property but was released in 'dealloc'"; + } + + std::unique_ptr<BugReport> BR( + new BugReport(*ExtraReleaseBugType, OS.str(), ErrNode)); + BR->addRange(M.getOriginExpr()->getSourceRange()); + + C.emitReport(std::move(BR)); + + return true; +} + +/// Emits a warning if the current context is -dealloc and DeallocedValue +/// must not be directly dealloced in a -dealloc. Returns true if a diagnostic +/// was emitted. +bool ObjCDeallocChecker::diagnoseMistakenDealloc(SymbolRef DeallocedValue, + const ObjCMethodCall &M, + CheckerContext &C) const { + + // Find the property backing the instance variable that M + // is dealloc'ing. + const ObjCPropertyImplDecl *PropImpl = + findPropertyOnDeallocatingInstance(DeallocedValue, C); + if (!PropImpl) + return false; + + if (getDeallocReleaseRequirement(PropImpl) != + ReleaseRequirement::MustRelease) { + return false; + } + + ExplodedNode *ErrNode = C.generateErrorNode(); + if (!ErrNode) + return false; + + std::string Buf; + llvm::raw_string_ostream OS(Buf); + + OS << "'" << *PropImpl->getPropertyIvarDecl() + << "' should be released rather than deallocated"; + + std::unique_ptr<BugReport> BR( + new BugReport(*MistakenDeallocBugType, OS.str(), ErrNode)); + BR->addRange(M.getOriginExpr()->getSourceRange()); + + C.emitReport(std::move(BR)); + + return true; +} + +ObjCDeallocChecker::ObjCDeallocChecker() + : NSObjectII(nullptr), SenTestCaseII(nullptr), XCTestCaseII(nullptr), + CIFilterII(nullptr) { + + MissingReleaseBugType.reset( + new BugType(this, "Missing ivar release (leak)", + categories::MemoryCoreFoundationObjectiveC)); + + ExtraReleaseBugType.reset( + new BugType(this, "Extra ivar release", + categories::MemoryCoreFoundationObjectiveC)); + + MistakenDeallocBugType.reset( + new BugType(this, "Mistaken dealloc", + categories::MemoryCoreFoundationObjectiveC)); +} + +void ObjCDeallocChecker::initIdentifierInfoAndSelectors( + ASTContext &Ctx) const { + if (NSObjectII) + return; + + NSObjectII = &Ctx.Idents.get("NSObject"); + SenTestCaseII = &Ctx.Idents.get("SenTestCase"); + XCTestCaseII = &Ctx.Idents.get("XCTestCase"); + Block_releaseII = &Ctx.Idents.get("_Block_release"); + CIFilterII = &Ctx.Idents.get("CIFilter"); + + IdentifierInfo *DeallocII = &Ctx.Idents.get("dealloc"); + IdentifierInfo *ReleaseII = &Ctx.Idents.get("release"); + DeallocSel = Ctx.Selectors.getSelector(0, &DeallocII); + ReleaseSel = Ctx.Selectors.getSelector(0, &ReleaseII); +} + +/// Returns true if M is a call to '[super dealloc]'. +bool ObjCDeallocChecker::isSuperDeallocMessage( + const ObjCMethodCall &M) const { + if (M.getOriginExpr()->getReceiverKind() != ObjCMessageExpr::SuperInstance) + return false; + + return M.getSelector() == DeallocSel; +} + +/// Returns the ObjCImplDecl containing the method declaration in LCtx. +const ObjCImplDecl * +ObjCDeallocChecker::getContainingObjCImpl(const LocationContext *LCtx) const { + auto *MD = cast<ObjCMethodDecl>(LCtx->getDecl()); + return cast<ObjCImplDecl>(MD->getDeclContext()); +} + +/// Returns the property that shadowed by PropImpl if one exists and +/// nullptr otherwise. +const ObjCPropertyDecl *ObjCDeallocChecker::findShadowedPropertyDecl( + const ObjCPropertyImplDecl *PropImpl) const { + const ObjCPropertyDecl *PropDecl = PropImpl->getPropertyDecl(); + + // Only readwrite properties can shadow. + if (PropDecl->isReadOnly()) + return nullptr; + + auto *CatDecl = dyn_cast<ObjCCategoryDecl>(PropDecl->getDeclContext()); + + // Only class extensions can contain shadowing properties. + if (!CatDecl || !CatDecl->IsClassExtension()) + return nullptr; + + IdentifierInfo *ID = PropDecl->getIdentifier(); + DeclContext::lookup_result R = CatDecl->getClassInterface()->lookup(ID); + for (DeclContext::lookup_iterator I = R.begin(), E = R.end(); I != E; ++I) { + auto *ShadowedPropDecl = dyn_cast<ObjCPropertyDecl>(*I); + if (!ShadowedPropDecl) + continue; + + if (ShadowedPropDecl->isInstanceProperty()) { + assert(ShadowedPropDecl->isReadOnly()); + return ShadowedPropDecl; } } + + return nullptr; } -//===----------------------------------------------------------------------===// -// ObjCDeallocChecker -//===----------------------------------------------------------------------===// +/// Add a transition noting the release of the given value. +void ObjCDeallocChecker::transitionToReleaseValue(CheckerContext &C, + SymbolRef Value) const { + assert(Value); + SymbolRef InstanceSym = getInstanceSymbolFromIvarSymbol(Value); + if (!InstanceSym) + return; + ProgramStateRef InitialState = C.getState(); -namespace { -class ObjCDeallocChecker : public Checker< - check::ASTDecl<ObjCImplementationDecl> > { -public: - void checkASTDecl(const ObjCImplementationDecl *D, AnalysisManager& mgr, - BugReporter &BR) const { - if (mgr.getLangOpts().getGC() == LangOptions::GCOnly) - return; - checkObjCDealloc(this, cast<ObjCImplementationDecl>(D), mgr.getLangOpts(), - BR); + ProgramStateRef ReleasedState = + removeValueRequiringRelease(InitialState, InstanceSym, Value); + + if (ReleasedState != InitialState) { + C.addTransition(ReleasedState); } -}; } -void ento::registerObjCDeallocChecker(CheckerManager &mgr) { - mgr.registerChecker<ObjCDeallocChecker>(); +/// Remove the Value requiring a release from the tracked set for +/// Instance and return the resultant state. +ProgramStateRef ObjCDeallocChecker::removeValueRequiringRelease( + ProgramStateRef State, SymbolRef Instance, SymbolRef Value) const { + assert(Instance); + assert(Value); + const ObjCIvarRegion *RemovedRegion = getIvarRegionForIvarSymbol(Value); + if (!RemovedRegion) + return State; + + const SymbolSet *Unreleased = State->get<UnreleasedIvarMap>(Instance); + if (!Unreleased) + return State; + + // Mark the value as no longer requiring a release. + SymbolSet::Factory &F = State->getStateManager().get_context<SymbolSet>(); + SymbolSet NewUnreleased = *Unreleased; + for (auto &Sym : *Unreleased) { + const ObjCIvarRegion *UnreleasedRegion = getIvarRegionForIvarSymbol(Sym); + assert(UnreleasedRegion); + if (RemovedRegion->getDecl() == UnreleasedRegion->getDecl()) { + NewUnreleased = F.remove(NewUnreleased, Sym); + } + } + + if (NewUnreleased.isEmpty()) { + return State->remove<UnreleasedIvarMap>(Instance); + } + + return State->set<UnreleasedIvarMap>(Instance, NewUnreleased); +} + +/// Determines whether the instance variable for \p PropImpl must or must not be +/// released in -dealloc or whether it cannot be determined. +ReleaseRequirement ObjCDeallocChecker::getDeallocReleaseRequirement( + const ObjCPropertyImplDecl *PropImpl) const { + const ObjCIvarDecl *IvarDecl; + const ObjCPropertyDecl *PropDecl; + if (!isSynthesizedRetainableProperty(PropImpl, &IvarDecl, &PropDecl)) + return ReleaseRequirement::Unknown; + + ObjCPropertyDecl::SetterKind SK = PropDecl->getSetterKind(); + + switch (SK) { + // Retain and copy setters retain/copy their values before storing and so + // the value in their instance variables must be released in -dealloc. + case ObjCPropertyDecl::Retain: + case ObjCPropertyDecl::Copy: + if (isReleasedByCIFilterDealloc(PropImpl)) + return ReleaseRequirement::MustNotReleaseDirectly; + + return ReleaseRequirement::MustRelease; + + case ObjCPropertyDecl::Weak: + return ReleaseRequirement::MustNotReleaseDirectly; + + case ObjCPropertyDecl::Assign: + // It is common for the ivars for read-only assign properties to + // always be stored retained, so their release requirement cannot be + // be determined. + if (PropDecl->isReadOnly()) + return ReleaseRequirement::Unknown; + + return ReleaseRequirement::MustNotReleaseDirectly; + } + llvm_unreachable("Unrecognized setter kind"); +} + +/// Returns the released value if M is a call a setter that releases +/// and nils out its underlying instance variable. +SymbolRef +ObjCDeallocChecker::getValueReleasedByNillingOut(const ObjCMethodCall &M, + CheckerContext &C) const { + SVal ReceiverVal = M.getReceiverSVal(); + if (!ReceiverVal.isValid()) + return nullptr; + + if (M.getNumArgs() == 0) + return nullptr; + + if (!M.getArgExpr(0)->getType()->isObjCRetainableType()) + return nullptr; + + // Is the first argument nil? + SVal Arg = M.getArgSVal(0); + ProgramStateRef notNilState, nilState; + std::tie(notNilState, nilState) = + M.getState()->assume(Arg.castAs<DefinedOrUnknownSVal>()); + if (!(nilState && !notNilState)) + return nullptr; + + const ObjCPropertyDecl *Prop = M.getAccessedProperty(); + if (!Prop) + return nullptr; + + ObjCIvarDecl *PropIvarDecl = Prop->getPropertyIvarDecl(); + if (!PropIvarDecl) + return nullptr; + + ProgramStateRef State = C.getState(); + + SVal LVal = State->getLValue(PropIvarDecl, ReceiverVal); + Optional<Loc> LValLoc = LVal.getAs<Loc>(); + if (!LValLoc) + return nullptr; + + SVal CurrentValInIvar = State->getSVal(LValLoc.getValue()); + return CurrentValInIvar.getAsSymbol(); +} + +/// Returns true if the current context is a call to -dealloc and false +/// otherwise. If true, it also sets SelfValOut to the value of +/// 'self'. +bool ObjCDeallocChecker::isInInstanceDealloc(const CheckerContext &C, + SVal &SelfValOut) const { + return isInInstanceDealloc(C, C.getLocationContext(), SelfValOut); +} + +/// Returns true if LCtx is a call to -dealloc and false +/// otherwise. If true, it also sets SelfValOut to the value of +/// 'self'. +bool ObjCDeallocChecker::isInInstanceDealloc(const CheckerContext &C, + const LocationContext *LCtx, + SVal &SelfValOut) const { + auto *MD = dyn_cast<ObjCMethodDecl>(LCtx->getDecl()); + if (!MD || !MD->isInstanceMethod() || MD->getSelector() != DeallocSel) + return false; + + const ImplicitParamDecl *SelfDecl = LCtx->getSelfDecl(); + assert(SelfDecl && "No self in -dealloc?"); + + ProgramStateRef State = C.getState(); + SelfValOut = State->getSVal(State->getRegion(SelfDecl, LCtx)); + return true; +} + +/// Returns true if there is a call to -dealloc anywhere on the stack and false +/// otherwise. If true, it also sets InstanceValOut to the value of +/// 'self' in the frame for -dealloc. +bool ObjCDeallocChecker::instanceDeallocIsOnStack(const CheckerContext &C, + SVal &InstanceValOut) const { + const LocationContext *LCtx = C.getLocationContext(); + + while (LCtx) { + if (isInInstanceDealloc(C, LCtx, InstanceValOut)) + return true; + + LCtx = LCtx->getParent(); + } + + return false; +} + +/// Returns true if the ID is a class in which which is known to have +/// a separate teardown lifecycle. In this case, -dealloc warnings +/// about missing releases should be suppressed. +bool ObjCDeallocChecker::classHasSeparateTeardown( + const ObjCInterfaceDecl *ID) const { + // Suppress if the class is not a subclass of NSObject. + for ( ; ID ; ID = ID->getSuperClass()) { + IdentifierInfo *II = ID->getIdentifier(); + + if (II == NSObjectII) + return false; + + // FIXME: For now, ignore classes that subclass SenTestCase and XCTestCase, + // as these don't need to implement -dealloc. They implement tear down in + // another way, which we should try and catch later. + // http://llvm.org/bugs/show_bug.cgi?id=3187 + if (II == XCTestCaseII || II == SenTestCaseII) + return true; + } + + return true; +} + +/// The -dealloc method in CIFilter highly unusual in that is will release +/// instance variables belonging to its *subclasses* if the variable name +/// starts with "input" or backs a property whose name starts with "input". +/// Subclasses should not release these ivars in their own -dealloc method -- +/// doing so could result in an over release. +/// +/// This method returns true if the property will be released by +/// -[CIFilter dealloc]. +bool ObjCDeallocChecker::isReleasedByCIFilterDealloc( + const ObjCPropertyImplDecl *PropImpl) const { + assert(PropImpl->getPropertyIvarDecl()); + StringRef PropName = PropImpl->getPropertyDecl()->getName(); + StringRef IvarName = PropImpl->getPropertyIvarDecl()->getName(); + + const char *ReleasePrefix = "input"; + if (!(PropName.startswith(ReleasePrefix) || + IvarName.startswith(ReleasePrefix))) { + return false; + } + + const ObjCInterfaceDecl *ID = + PropImpl->getPropertyIvarDecl()->getContainingInterface(); + for ( ; ID ; ID = ID->getSuperClass()) { + IdentifierInfo *II = ID->getIdentifier(); + if (II == CIFilterII) + return true; + } + + return false; +} + +void ento::registerObjCDeallocChecker(CheckerManager &Mgr) { + const LangOptions &LangOpts = Mgr.getLangOpts(); + // These checker only makes sense under MRR. + if (LangOpts.getGC() == LangOptions::GCOnly || LangOpts.ObjCAutoRefCount) + return; + + Mgr.registerChecker<ObjCDeallocChecker>(); } diff --git a/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp b/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp index 37b84480f8929..74d05e27e8eb0 100644 --- a/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp +++ b/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp @@ -1,4 +1,4 @@ -//= CheckerDocumentation.cpp - Documentation checker ---------------*- C++ -*-// +//===- CheckerDocumentation.cpp - Documentation checker ---------*- C++ -*-===// // // The LLVM Compiler Infrastructure // @@ -57,7 +57,6 @@ class CheckerDocumentation : public Checker< check::PreStmt<ReturnStmt>, check::Event<ImplicitNullDerefEvent>, check::ASTDecl<FunctionDecl> > { public: - /// \brief Pre-visit the Statement. /// /// The method will be called before the analyzer core processes the @@ -147,7 +146,6 @@ public: /// check::Bind void checkBind(SVal Loc, SVal Val, const Stmt *S, CheckerContext &) const {} - /// \brief Called whenever a symbol becomes dead. /// /// This callback should be used by the checkers to aggressively clean @@ -164,8 +162,16 @@ public: /// check::DeadSymbols void checkDeadSymbols(SymbolReaper &SR, CheckerContext &C) const {} + + /// \brief Called when the analyzer core starts analyzing a function, + /// regardless of whether it is analyzed at the top level or is inlined. + /// + /// check::BeginFunction + void checkBeginFunction(CheckerContext &Ctx) const {} + /// \brief Called when the analyzer core reaches the end of a - /// function being analyzed. + /// function being analyzed regardless of whether it is analyzed at the top + /// level or is inlined. /// /// check::EndFunction void checkEndFunction(CheckerContext &Ctx) const {} @@ -190,7 +196,6 @@ public: AnalysisManager &Mgr, BugReporter &BR) const {} - /// \brief Evaluates function call. /// /// The analysis core threats all function calls in the same way. However, some @@ -310,12 +315,10 @@ public: void checkASTDecl(const FunctionDecl *D, AnalysisManager &Mgr, BugReporter &BR) const {} - }; void CheckerDocumentation::checkPostStmt(const DeclStmt *DS, CheckerContext &C) const { - return; } } // end namespace ento diff --git a/lib/StaticAnalyzer/Checkers/Checkers.td b/lib/StaticAnalyzer/Checkers/Checkers.td deleted file mode 100644 index 8133d290d8860..0000000000000 --- a/lib/StaticAnalyzer/Checkers/Checkers.td +++ /dev/null @@ -1,647 +0,0 @@ -//===--- Checkers.td - Static Analyzer Checkers -===-----------------------===// -// -// The LLVM Compiler Infrastructure -// -// This file is distributed under the University of Illinois Open Source -// License. See LICENSE.TXT for details. -// -//===----------------------------------------------------------------------===// - -include "clang/StaticAnalyzer/Checkers/CheckerBase.td" - -//===----------------------------------------------------------------------===// -// Packages. -//===----------------------------------------------------------------------===// - -// The Alpha package is for checkers that have too many false positives to be -// turned on by default. The hierarchy under Alpha should be organized in the -// hierarchy checkers would have had if they were truly at the top level. -// (For example, a Cocoa-specific checker that is alpha should be in -// alpha.osx.cocoa). -def Alpha : Package<"alpha">; - -def Core : Package<"core">; -def CoreBuiltin : Package<"builtin">, InPackage<Core>; -def CoreUninitialized : Package<"uninitialized">, InPackage<Core>; -def CoreAlpha : Package<"core">, InPackage<Alpha>, Hidden; - -// The OptIn package is for checkers that are not alpha and that would normally -// be on by default but where the driver does not have enough information to -// determine when they are applicable. For example, localizability checkers fit -// this criterion because the driver cannot determine whether a project is -// localized or not -- this is best determined at the IDE or build-system level. -// -// The checker hierarchy under OptIn should mirror that in Alpha: checkers -// should be organized as if they were at the top level. -// -// Note: OptIn is *not* intended for checkers that are too noisy to be on by -// default. Such checkers belong in the alpha package. -def OptIn : Package<"optin">; - -def Nullability : Package<"nullability">; - -def Cplusplus : Package<"cplusplus">; -def CplusplusAlpha : Package<"cplusplus">, InPackage<Alpha>, Hidden; - -def DeadCode : Package<"deadcode">; -def DeadCodeAlpha : Package<"deadcode">, InPackage<Alpha>, Hidden; - -def Performance : Package<"performance">, InPackage<OptIn>; - -def Security : Package <"security">; -def InsecureAPI : Package<"insecureAPI">, InPackage<Security>; -def SecurityAlpha : Package<"security">, InPackage<Alpha>, Hidden; -def Taint : Package<"taint">, InPackage<SecurityAlpha>, Hidden; - -def Unix : Package<"unix">; -def UnixAlpha : Package<"unix">, InPackage<Alpha>, Hidden; -def CString : Package<"cstring">, InPackage<Unix>, Hidden; -def CStringAlpha : Package<"cstring">, InPackage<UnixAlpha>, Hidden; - -def OSX : Package<"osx">; -def OSXAlpha : Package<"osx">, InPackage<Alpha>, Hidden; -def OSXOptIn : Package<"osx">, InPackage<OptIn>; - -def Cocoa : Package<"cocoa">, InPackage<OSX>; -def CocoaAlpha : Package<"cocoa">, InPackage<OSXAlpha>, Hidden; -def CocoaOptIn : Package<"cocoa">, InPackage<OSXOptIn>; - -def CoreFoundation : Package<"coreFoundation">, InPackage<OSX>; -def Containers : Package<"containers">, InPackage<CoreFoundation>; - -def LocalizabilityAlpha : Package<"localizability">, InPackage<CocoaAlpha>; -def LocalizabilityOptIn : Package<"localizability">, InPackage<CocoaOptIn>; - -def LLVM : Package<"llvm">; -def Debug : Package<"debug">; - -//===----------------------------------------------------------------------===// -// Core Checkers. -//===----------------------------------------------------------------------===// - -let ParentPackage = Core in { - -def DereferenceChecker : Checker<"NullDereference">, - HelpText<"Check for dereferences of null pointers">, - DescFile<"DereferenceChecker.cpp">; - -def CallAndMessageChecker : Checker<"CallAndMessage">, - HelpText<"Check for logical errors for function calls and Objective-C message expressions (e.g., uninitialized arguments, null function pointers)">, - DescFile<"CallAndMessageChecker.cpp">; - -def NonNullParamChecker : Checker<"NonNullParamChecker">, - HelpText<"Check for null pointers passed as arguments to a function whose arguments are references or marked with the 'nonnull' attribute">, - DescFile<"NonNullParamChecker.cpp">; - -def VLASizeChecker : Checker<"VLASize">, - HelpText<"Check for declarations of VLA of undefined or zero size">, - DescFile<"VLASizeChecker.cpp">; - -def DivZeroChecker : Checker<"DivideZero">, - HelpText<"Check for division by zero">, - DescFile<"DivZeroChecker.cpp">; - -def UndefResultChecker : Checker<"UndefinedBinaryOperatorResult">, - HelpText<"Check for undefined results of binary operators">, - DescFile<"UndefResultChecker.cpp">; - -def StackAddrEscapeChecker : Checker<"StackAddressEscape">, - HelpText<"Check that addresses to stack memory do not escape the function">, - DescFile<"StackAddrEscapeChecker.cpp">; - -def DynamicTypePropagation : Checker<"DynamicTypePropagation">, - HelpText<"Generate dynamic type information">, - DescFile<"DynamicTypePropagation.cpp">; - -} // end "core" - -let ParentPackage = CoreAlpha in { - -def BoolAssignmentChecker : Checker<"BoolAssignment">, - HelpText<"Warn about assigning non-{0,1} values to Boolean variables">, - DescFile<"BoolAssignmentChecker.cpp">; - -def CastSizeChecker : Checker<"CastSize">, - HelpText<"Check when casting a malloc'ed type T, whether the size is a multiple of the size of T">, - DescFile<"CastSizeChecker.cpp">; - -def CastToStructChecker : Checker<"CastToStruct">, - HelpText<"Check for cast from non-struct pointer to struct pointer">, - DescFile<"CastToStructChecker.cpp">; - -def IdenticalExprChecker : Checker<"IdenticalExpr">, - HelpText<"Warn about unintended use of identical expressions in operators">, - DescFile<"IdenticalExprChecker.cpp">; - -def FixedAddressChecker : Checker<"FixedAddr">, - HelpText<"Check for assignment of a fixed address to a pointer">, - DescFile<"FixedAddressChecker.cpp">; - -def PointerArithChecker : Checker<"PointerArithm">, - HelpText<"Check for pointer arithmetic on locations other than array elements">, - DescFile<"PointerArithChecker">; - -def PointerSubChecker : Checker<"PointerSub">, - HelpText<"Check for pointer subtractions on two pointers pointing to different memory chunks">, - DescFile<"PointerSubChecker">; - -def SizeofPointerChecker : Checker<"SizeofPtr">, - HelpText<"Warn about unintended use of sizeof() on pointer expressions">, - DescFile<"CheckSizeofPointer.cpp">; - -def CallAndMessageUnInitRefArg : Checker<"CallAndMessageUnInitRefArg">, - HelpText<"Check for logical errors for function calls and Objective-C message expressions (e.g., uninitialized arguments, null function pointers, and pointer to undefined variables)">, - DescFile<"CallAndMessageChecker.cpp">; - -def TestAfterDivZeroChecker : Checker<"TestAfterDivZero">, - HelpText<"Check for division by variable that is later compared against 0. Either the comparison is useless or there is division by zero.">, - DescFile<"TestAfterDivZeroChecker.cpp">; - -def DynamicTypeChecker : Checker<"DynamicTypeChecker">, - HelpText<"Check for cases where the dynamic and the static type of an object are unrelated.">, - DescFile<"DynamicTypeChecker.cpp">; - -} // end "alpha.core" - -let ParentPackage = Nullability in { - -def NullPassedToNonnullChecker : Checker<"NullPassedToNonnull">, - HelpText<"Warns when a null pointer is passed to a pointer which has a _Nonnull type.">, - DescFile<"NullabilityChecker.cpp">; - -def NullReturnedFromNonnullChecker : Checker<"NullReturnedFromNonnull">, - HelpText<"Warns when a null pointer is returned from a function that has _Nonnull return type.">, - DescFile<"NullabilityChecker.cpp">; - -def NullableDereferencedChecker : Checker<"NullableDereferenced">, - HelpText<"Warns when a nullable pointer is dereferenced.">, - DescFile<"NullabilityChecker.cpp">; - -def NullablePassedToNonnullChecker : Checker<"NullablePassedToNonnull">, - HelpText<"Warns when a nullable pointer is passed to a pointer which has a _Nonnull type.">, - DescFile<"NullabilityChecker.cpp">; - -def NullableReturnedFromNonnullChecker : Checker<"NullablePassedToNonnull">, - HelpText<"Warns when a nullable pointer is returned from a function that has _Nonnull return type.">, - DescFile<"NullabilityChecker.cpp">; - -} // end "nullability" - -//===----------------------------------------------------------------------===// -// Evaluate "builtin" functions. -//===----------------------------------------------------------------------===// - -let ParentPackage = CoreBuiltin in { - -def NoReturnFunctionChecker : Checker<"NoReturnFunctions">, - HelpText<"Evaluate \"panic\" functions that are known to not return to the caller">, - DescFile<"NoReturnFunctionChecker.cpp">; - -def BuiltinFunctionChecker : Checker<"BuiltinFunctions">, - HelpText<"Evaluate compiler builtin functions (e.g., alloca())">, - DescFile<"BuiltinFunctionChecker.cpp">; - -} // end "core.builtin" - -//===----------------------------------------------------------------------===// -// Uninitialized values checkers. -//===----------------------------------------------------------------------===// - -let ParentPackage = CoreUninitialized in { - -def UndefinedArraySubscriptChecker : Checker<"ArraySubscript">, - HelpText<"Check for uninitialized values used as array subscripts">, - DescFile<"UndefinedArraySubscriptChecker.cpp">; - -def UndefinedAssignmentChecker : Checker<"Assign">, - HelpText<"Check for assigning uninitialized values">, - DescFile<"UndefinedAssignmentChecker.cpp">; - -def UndefBranchChecker : Checker<"Branch">, - HelpText<"Check for uninitialized values used as branch conditions">, - DescFile<"UndefBranchChecker.cpp">; - -def UndefCapturedBlockVarChecker : Checker<"CapturedBlockVariable">, - HelpText<"Check for blocks that capture uninitialized values">, - DescFile<"UndefCapturedBlockVarChecker.cpp">; - -def ReturnUndefChecker : Checker<"UndefReturn">, - HelpText<"Check for uninitialized values being returned to the caller">, - DescFile<"ReturnUndefChecker.cpp">; - -} // end "core.uninitialized" - -//===----------------------------------------------------------------------===// -// C++ checkers. -//===----------------------------------------------------------------------===// - -let ParentPackage = Cplusplus in { - -def NewDeleteChecker : Checker<"NewDelete">, - HelpText<"Check for double-free and use-after-free problems. Traces memory managed by new/delete.">, - DescFile<"MallocChecker.cpp">; - -def NewDeleteLeaksChecker : Checker<"NewDeleteLeaks">, - HelpText<"Check for memory leaks. Traces memory managed by new/delete.">, - DescFile<"MallocChecker.cpp">; - -} // end: "cplusplus" - -let ParentPackage = CplusplusAlpha in { - -def VirtualCallChecker : Checker<"VirtualCall">, - HelpText<"Check virtual function calls during construction or destruction">, - DescFile<"VirtualCallChecker.cpp">; - -} // end: "alpha.cplusplus" - -//===----------------------------------------------------------------------===// -// Deadcode checkers. -//===----------------------------------------------------------------------===// - -let ParentPackage = DeadCode in { - -def DeadStoresChecker : Checker<"DeadStores">, - HelpText<"Check for values stored to variables that are never read afterwards">, - DescFile<"DeadStoresChecker.cpp">; -} // end DeadCode - -let ParentPackage = DeadCodeAlpha in { - -def UnreachableCodeChecker : Checker<"UnreachableCode">, - HelpText<"Check unreachable code">, - DescFile<"UnreachableCodeChecker.cpp">; - -} // end "alpha.deadcode" - -//===----------------------------------------------------------------------===// -// Performance checkers. -//===----------------------------------------------------------------------===// - -let ParentPackage = Performance in { - -def PaddingChecker : Checker<"Padding">, - HelpText<"Check for excessively padded structs.">, - DescFile<"PaddingChecker.cpp">; - -} // end: "padding" - -//===----------------------------------------------------------------------===// -// Security checkers. -//===----------------------------------------------------------------------===// - -let ParentPackage = InsecureAPI in { - def gets : Checker<"gets">, - HelpText<"Warn on uses of the 'gets' function">, - DescFile<"CheckSecuritySyntaxOnly.cpp">; - def getpw : Checker<"getpw">, - HelpText<"Warn on uses of the 'getpw' function">, - DescFile<"CheckSecuritySyntaxOnly.cpp">; - def mktemp : Checker<"mktemp">, - HelpText<"Warn on uses of the 'mktemp' function">, - DescFile<"CheckSecuritySyntaxOnly.cpp">; - def mkstemp : Checker<"mkstemp">, - HelpText<"Warn when 'mkstemp' is passed fewer than 6 X's in the format string">, - DescFile<"CheckSecuritySyntaxOnly.cpp">; - def rand : Checker<"rand">, - HelpText<"Warn on uses of the 'rand', 'random', and related functions">, - DescFile<"CheckSecuritySyntaxOnly.cpp">; - def strcpy : Checker<"strcpy">, - HelpText<"Warn on uses of the 'strcpy' and 'strcat' functions">, - DescFile<"CheckSecuritySyntaxOnly.cpp">; - def vfork : Checker<"vfork">, - HelpText<"Warn on uses of the 'vfork' function">, - DescFile<"CheckSecuritySyntaxOnly.cpp">; - def UncheckedReturn : Checker<"UncheckedReturn">, - HelpText<"Warn on uses of functions whose return values must be always checked">, - DescFile<"CheckSecuritySyntaxOnly.cpp">; -} -let ParentPackage = Security in { - def FloatLoopCounter : Checker<"FloatLoopCounter">, - HelpText<"Warn on using a floating point value as a loop counter (CERT: FLP30-C, FLP30-CPP)">, - DescFile<"CheckSecuritySyntaxOnly.cpp">; -} - -let ParentPackage = SecurityAlpha in { - -def ArrayBoundChecker : Checker<"ArrayBound">, - HelpText<"Warn about buffer overflows (older checker)">, - DescFile<"ArrayBoundChecker.cpp">; - -def ArrayBoundCheckerV2 : Checker<"ArrayBoundV2">, - HelpText<"Warn about buffer overflows (newer checker)">, - DescFile<"ArrayBoundCheckerV2.cpp">; - -def ReturnPointerRangeChecker : Checker<"ReturnPtrRange">, - HelpText<"Check for an out-of-bound pointer being returned to callers">, - DescFile<"ReturnPointerRangeChecker.cpp">; - -def MallocOverflowSecurityChecker : Checker<"MallocOverflow">, - HelpText<"Check for overflows in the arguments to malloc()">, - DescFile<"MallocOverflowSecurityChecker.cpp">; - -} // end "alpha.security" - -//===----------------------------------------------------------------------===// -// Taint checkers. -//===----------------------------------------------------------------------===// - -let ParentPackage = Taint in { - -def GenericTaintChecker : Checker<"TaintPropagation">, - HelpText<"Generate taint information used by other checkers">, - DescFile<"GenericTaintChecker.cpp">; - -} // end "alpha.security.taint" - -//===----------------------------------------------------------------------===// -// Unix API checkers. -//===----------------------------------------------------------------------===// - -let ParentPackage = Unix in { - -def UnixAPIChecker : Checker<"API">, - HelpText<"Check calls to various UNIX/Posix functions">, - DescFile<"UnixAPIChecker.cpp">; - -def MallocChecker: Checker<"Malloc">, - HelpText<"Check for memory leaks, double free, and use-after-free problems. Traces memory managed by malloc()/free().">, - DescFile<"MallocChecker.cpp">; - -def MallocSizeofChecker : Checker<"MallocSizeof">, - HelpText<"Check for dubious malloc arguments involving sizeof">, - DescFile<"MallocSizeofChecker.cpp">; - -def MismatchedDeallocatorChecker : Checker<"MismatchedDeallocator">, - HelpText<"Check for mismatched deallocators.">, - DescFile<"MallocChecker.cpp">; - -def VforkChecker : Checker<"Vfork">, - HelpText<"Check for proper usage of vfork">, - DescFile<"VforkChecker.cpp">; - -} // end "unix" - -let ParentPackage = UnixAlpha in { - -def ChrootChecker : Checker<"Chroot">, - HelpText<"Check improper use of chroot">, - DescFile<"ChrootChecker.cpp">; - -def PthreadLockChecker : Checker<"PthreadLock">, - HelpText<"Simple lock -> unlock checker">, - DescFile<"PthreadLockChecker.cpp">; - -def StreamChecker : Checker<"Stream">, - HelpText<"Check stream handling functions">, - DescFile<"StreamChecker.cpp">; - -def SimpleStreamChecker : Checker<"SimpleStream">, - HelpText<"Check for misuses of stream APIs">, - DescFile<"SimpleStreamChecker.cpp">; - -} // end "alpha.unix" - -let ParentPackage = CString in { - -def CStringNullArg : Checker<"NullArg">, - HelpText<"Check for null pointers being passed as arguments to C string functions">, - DescFile<"CStringChecker.cpp">; - -def CStringSyntaxChecker : Checker<"BadSizeArg">, - HelpText<"Check the size argument passed into C string functions for common erroneous patterns">, - DescFile<"CStringSyntaxChecker.cpp">; -} - -let ParentPackage = CStringAlpha in { - -def CStringOutOfBounds : Checker<"OutOfBounds">, - HelpText<"Check for out-of-bounds access in string functions">, - DescFile<"CStringChecker.cpp">; - -def CStringBufferOverlap : Checker<"BufferOverlap">, - HelpText<"Checks for overlap in two buffer arguments">, - DescFile<"CStringChecker.cpp">; - -def CStringNotNullTerm : Checker<"NotNullTerminated">, - HelpText<"Check for arguments which are not null-terminating strings">, - DescFile<"CStringChecker.cpp">; -} - -//===----------------------------------------------------------------------===// -// Mac OS X, Cocoa, and Core Foundation checkers. -//===----------------------------------------------------------------------===// - -let ParentPackage = OSX in { - -def MacOSXAPIChecker : Checker<"API">, - InPackage<OSX>, - HelpText<"Check for proper uses of various Apple APIs">, - DescFile<"MacOSXAPIChecker.cpp">; - -def MacOSKeychainAPIChecker : Checker<"SecKeychainAPI">, - InPackage<OSX>, - HelpText<"Check for proper uses of Secure Keychain APIs">, - DescFile<"MacOSKeychainAPIChecker.cpp">; - -} // end "osx" - -let ParentPackage = Cocoa in { - -def ObjCAtSyncChecker : Checker<"AtSync">, - HelpText<"Check for nil pointers used as mutexes for @synchronized">, - DescFile<"ObjCAtSyncChecker.cpp">; - -def NilArgChecker : Checker<"NilArg">, - HelpText<"Check for prohibited nil arguments to ObjC method calls">, - DescFile<"BasicObjCFoundationChecks.cpp">; - -def ClassReleaseChecker : Checker<"ClassRelease">, - HelpText<"Check for sending 'retain', 'release', or 'autorelease' directly to a Class">, - DescFile<"BasicObjCFoundationChecks.cpp">; - -def VariadicMethodTypeChecker : Checker<"VariadicMethodTypes">, - HelpText<"Check for passing non-Objective-C types to variadic collection " - "initialization methods that expect only Objective-C types">, - DescFile<"BasicObjCFoundationChecks.cpp">; - -def NSAutoreleasePoolChecker : Checker<"NSAutoreleasePool">, - HelpText<"Warn for suboptimal uses of NSAutoreleasePool in Objective-C GC mode">, - DescFile<"NSAutoreleasePoolChecker.cpp">; - -def ObjCMethSigsChecker : Checker<"IncompatibleMethodTypes">, - HelpText<"Warn about Objective-C method signatures with type incompatibilities">, - DescFile<"CheckObjCInstMethSignature.cpp">; - -def ObjCUnusedIvarsChecker : Checker<"UnusedIvars">, - HelpText<"Warn about private ivars that are never used">, - DescFile<"ObjCUnusedIVarsChecker.cpp">; - -def ObjCSelfInitChecker : Checker<"SelfInit">, - HelpText<"Check that 'self' is properly initialized inside an initializer method">, - DescFile<"ObjCSelfInitChecker.cpp">; - -def ObjCLoopChecker : Checker<"Loops">, - HelpText<"Improved modeling of loops using Cocoa collection types">, - DescFile<"BasicObjCFoundationChecks.cpp">; - -def ObjCNonNilReturnValueChecker : Checker<"NonNilReturnValue">, - HelpText<"Model the APIs that are guaranteed to return a non-nil value">, - DescFile<"BasicObjCFoundationChecks.cpp">; - -def ObjCSuperCallChecker : Checker<"MissingSuperCall">, - HelpText<"Warn about Objective-C methods that lack a necessary call to super">, - DescFile<"ObjCMissingSuperCallChecker.cpp">; - -def NSErrorChecker : Checker<"NSError">, - HelpText<"Check usage of NSError** parameters">, - DescFile<"NSErrorChecker.cpp">; - -def RetainCountChecker : Checker<"RetainCount">, - HelpText<"Check for leaks and improper reference count management">, - DescFile<"RetainCountChecker.cpp">; - -def ObjCGenericsChecker : Checker<"ObjCGenerics">, - HelpText<"Check for type errors when using Objective-C generics">, - DescFile<"DynamicTypePropagation.cpp">; - -} // end "osx.cocoa" - -let ParentPackage = CocoaAlpha in { - -def ObjCDeallocChecker : Checker<"Dealloc">, - HelpText<"Warn about Objective-C classes that lack a correct implementation of -dealloc">, - DescFile<"CheckObjCDealloc.cpp">; - -def InstanceVariableInvalidation : Checker<"InstanceVariableInvalidation">, - HelpText<"Check that the invalidatable instance variables are invalidated in the methods annotated with objc_instance_variable_invalidator">, - DescFile<"IvarInvalidationChecker.cpp">; - -def MissingInvalidationMethod : Checker<"MissingInvalidationMethod">, - HelpText<"Check that the invalidation methods are present in classes that contain invalidatable instance variables">, - DescFile<"IvarInvalidationChecker.cpp">; - -def DirectIvarAssignment : Checker<"DirectIvarAssignment">, - HelpText<"Check for direct assignments to instance variables">, - DescFile<"DirectIvarAssignment.cpp">; - -def DirectIvarAssignmentForAnnotatedFunctions : Checker<"DirectIvarAssignmentForAnnotatedFunctions">, - HelpText<"Check for direct assignments to instance variables in the methods annotated with objc_no_direct_instance_variable_assignment">, - DescFile<"DirectIvarAssignment.cpp">; - -} // end "alpha.osx.cocoa" - -let ParentPackage = CoreFoundation in { - -def CFNumberCreateChecker : Checker<"CFNumber">, - HelpText<"Check for proper uses of CFNumberCreate">, - DescFile<"BasicObjCFoundationChecks.cpp">; - -def CFRetainReleaseChecker : Checker<"CFRetainRelease">, - HelpText<"Check for null arguments to CFRetain/CFRelease/CFMakeCollectable">, - DescFile<"BasicObjCFoundationChecks.cpp">; - -def CFErrorChecker : Checker<"CFError">, - HelpText<"Check usage of CFErrorRef* parameters">, - DescFile<"NSErrorChecker.cpp">; -} - -let ParentPackage = Containers in { -def ObjCContainersASTChecker : Checker<"PointerSizedValues">, - HelpText<"Warns if 'CFArray', 'CFDictionary', 'CFSet' are created with non-pointer-size values">, - DescFile<"ObjCContainersASTChecker.cpp">; - -def ObjCContainersChecker : Checker<"OutOfBounds">, - HelpText<"Checks for index out-of-bounds when using 'CFArray' API">, - DescFile<"ObjCContainersChecker.cpp">; - -} - -let ParentPackage = LocalizabilityOptIn in { -def NonLocalizedStringChecker : Checker<"NonLocalizedStringChecker">, - HelpText<"Warns about uses of non-localized NSStrings passed to UI methods expecting localized NSStrings">, - DescFile<"LocalizationChecker.cpp">; - -def EmptyLocalizationContextChecker : Checker<"EmptyLocalizationContextChecker">, - HelpText<"Check that NSLocalizedString macros include a comment for context">, - DescFile<"LocalizationChecker.cpp">; -} - -let ParentPackage = LocalizabilityAlpha in { -def PluralMisuseChecker : Checker<"PluralMisuseChecker">, - HelpText<"Warns against using one vs. many plural pattern in code when generating localized strings.">, - DescFile<"LocalizationChecker.cpp">; -} - -//===----------------------------------------------------------------------===// -// Checkers for LLVM development. -//===----------------------------------------------------------------------===// - -def LLVMConventionsChecker : Checker<"Conventions">, - InPackage<LLVM>, - HelpText<"Check code for LLVM codebase conventions">, - DescFile<"LLVMConventionsChecker.cpp">; - -//===----------------------------------------------------------------------===// -// Debugging checkers (for analyzer development). -//===----------------------------------------------------------------------===// - -let ParentPackage = Debug in { - -def DominatorsTreeDumper : Checker<"DumpDominators">, - HelpText<"Print the dominance tree for a given CFG">, - DescFile<"DebugCheckers.cpp">; - -def LiveVariablesDumper : Checker<"DumpLiveVars">, - HelpText<"Print results of live variable analysis">, - DescFile<"DebugCheckers.cpp">; - -def CFGViewer : Checker<"ViewCFG">, - HelpText<"View Control-Flow Graphs using GraphViz">, - DescFile<"DebugCheckers.cpp">; - -def CFGDumper : Checker<"DumpCFG">, - HelpText<"Display Control-Flow Graphs">, - DescFile<"DebugCheckers.cpp">; - -def CallGraphViewer : Checker<"ViewCallGraph">, - HelpText<"View Call Graph using GraphViz">, - DescFile<"DebugCheckers.cpp">; - -def CallGraphDumper : Checker<"DumpCallGraph">, - HelpText<"Display Call Graph">, - DescFile<"DebugCheckers.cpp">; - -def ConfigDumper : Checker<"ConfigDumper">, - HelpText<"Dump config table">, - DescFile<"DebugCheckers.cpp">; - -def TraversalDumper : Checker<"DumpTraversal">, - HelpText<"Print branch conditions as they are traversed by the engine">, - DescFile<"TraversalChecker.cpp">; - -def CallDumper : Checker<"DumpCalls">, - HelpText<"Print calls as they are traversed by the engine">, - DescFile<"TraversalChecker.cpp">; - -def AnalyzerStatsChecker : Checker<"Stats">, - HelpText<"Emit warnings with analyzer statistics">, - DescFile<"AnalyzerStatsChecker.cpp">; - -def TaintTesterChecker : Checker<"TaintTest">, - HelpText<"Mark tainted symbols as such.">, - DescFile<"TaintTesterChecker.cpp">; - -def ExprInspectionChecker : Checker<"ExprInspection">, - HelpText<"Check the analyzer's understanding of expressions">, - DescFile<"ExprInspectionChecker.cpp">; - -def ExplodedGraphViewer : Checker<"ViewExplodedGraph">, - HelpText<"View Exploded Graphs using GraphViz">, - DescFile<"DebugCheckers.cpp">; - -def BugHashDumper : Checker<"DumpBugHash">, - HelpText<"Dump the bug hash for all statements.">, - DescFile<"DebugCheckers.cpp">; - -} // end "debug" diff --git a/lib/StaticAnalyzer/Checkers/ChrootChecker.cpp b/lib/StaticAnalyzer/Checkers/ChrootChecker.cpp index 3ad1996db8933..14587fb5163b3 100644 --- a/lib/StaticAnalyzer/Checkers/ChrootChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/ChrootChecker.cpp @@ -1,4 +1,4 @@ -//===- Chrootchecker.cpp -------- Basic security checks ----------*- C++ -*-==// +//===- Chrootchecker.cpp -------- Basic security checks ---------*- C++ -*-===// // // The LLVM Compiler Infrastructure // @@ -20,6 +20,7 @@ #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h" #include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h" #include "llvm/ADT/ImmutableMap.h" + using namespace clang; using namespace ento; @@ -148,8 +149,6 @@ void ChrootChecker::checkPreStmt(const CallExpr *CE, CheckerContext &C) const { C.emitReport(llvm::make_unique<BugReport>( *BT_BreakJail, BT_BreakJail->getDescription(), N)); } - - return; } void ento::registerChrootChecker(CheckerManager &mgr) { diff --git a/lib/StaticAnalyzer/Checkers/ClangCheckers.cpp b/lib/StaticAnalyzer/Checkers/ClangCheckers.cpp index 77a5a72264532..fb9e366c3de00 100644 --- a/lib/StaticAnalyzer/Checkers/ClangCheckers.cpp +++ b/lib/StaticAnalyzer/Checkers/ClangCheckers.cpp @@ -27,6 +27,6 @@ void ento::registerBuiltinCheckers(CheckerRegistry ®istry) { #define GET_CHECKERS #define CHECKER(FULLNAME,CLASS,DESCFILE,HELPTEXT,GROUPINDEX,HIDDEN) \ registry.addChecker(register##CLASS, FULLNAME, HELPTEXT); -#include "Checkers.inc" +#include "clang/StaticAnalyzer/Checkers/Checkers.inc" #undef GET_CHECKERS } diff --git a/lib/StaticAnalyzer/Checkers/ClangSACheckers.h b/lib/StaticAnalyzer/Checkers/ClangSACheckers.h index 05b4a61c5af12..d6e96f27a75e6 100644 --- a/lib/StaticAnalyzer/Checkers/ClangSACheckers.h +++ b/lib/StaticAnalyzer/Checkers/ClangSACheckers.h @@ -26,7 +26,7 @@ class CheckerRegistry; #define GET_CHECKERS #define CHECKER(FULLNAME,CLASS,CXXFILE,HELPTEXT,GROUPINDEX,HIDDEN) \ void register##CLASS(CheckerManager &mgr); -#include "Checkers.inc" +#include "clang/StaticAnalyzer/Checkers/Checkers.inc" #undef CHECKER #undef GET_CHECKERS diff --git a/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp b/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp index f2a269a3335c5..8ca2a24cffe74 100644 --- a/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp @@ -278,6 +278,8 @@ public: RHS = RHS->IgnoreParenCasts(); QualType T = VD->getType(); + if (T.isVolatileQualified()) + return; if (T->isPointerType() || T->isObjCObjectPointerType()) { if (RHS->isNullPointerConstant(Ctx, Expr::NPC_ValueDependentIsNull)) return; diff --git a/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp b/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp index f216f696ef65a..152b937bb03f4 100644 --- a/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp @@ -230,7 +230,7 @@ void DereferenceChecker::checkLocation(SVal l, bool isLoad, const Stmt* S, // dereference. if (ExplodedNode *N = C.generateSink(nullState, C.getPredecessor())) { ImplicitNullDerefEvent event = {l, isLoad, N, &C.getBugReporter(), - /*IsDirectDereference=*/false}; + /*IsDirectDereference=*/true}; dispatchEvent(event); } } @@ -272,7 +272,7 @@ void DereferenceChecker::checkBind(SVal L, SVal V, const Stmt *S, if (ExplodedNode *N = C.generateSink(StNull, C.getPredecessor())) { ImplicitNullDerefEvent event = {V, /*isLoad=*/true, N, &C.getBugReporter(), - /*IsDirectDereference=*/false}; + /*IsDirectDereference=*/true}; dispatchEvent(event); } } diff --git a/lib/StaticAnalyzer/Checkers/DirectIvarAssignment.cpp b/lib/StaticAnalyzer/Checkers/DirectIvarAssignment.cpp index ad478cbf7829f..5efb9096f2ff2 100644 --- a/lib/StaticAnalyzer/Checkers/DirectIvarAssignment.cpp +++ b/lib/StaticAnalyzer/Checkers/DirectIvarAssignment.cpp @@ -123,7 +123,7 @@ void DirectIvarAssignment::checkASTDecl(const ObjCImplementationDecl *D, IvarToPropertyMapTy IvarToPropMap; // Find all properties for this class. - for (const auto *PD : InterD->properties()) { + for (const auto *PD : InterD->instance_properties()) { // Find the corresponding IVar. const ObjCIvarDecl *ID = findPropertyBackingIvar(PD, InterD, Mgr.getASTContext()); diff --git a/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp b/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp index 30f629830c613..b8e43325da04c 100644 --- a/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp +++ b/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp @@ -1,4 +1,4 @@ -//== DynamicTypePropagation.cpp -------------------------------- -*- C++ -*--=// +//===- DynamicTypePropagation.cpp ------------------------------*- C++ -*--===// // // The LLVM Compiler Infrastructure // @@ -97,6 +97,7 @@ class DynamicTypePropagation: const ObjCObjectPointerType *To, ExplodedNode *N, SymbolRef Sym, CheckerContext &C, const Stmt *ReportedNode = nullptr) const; + public: void checkPreCall(const CallEvent &Call, CheckerContext &C) const; void checkPostCall(const CallEvent &Call, CheckerContext &C) const; @@ -109,7 +110,7 @@ public: /// This value is set to true, when the Generics checker is turned on. DefaultBool CheckGenerics; }; -} +} // end anonymous namespace void DynamicTypePropagation::checkDeadSymbols(SymbolReaper &SR, CheckerContext &C) const { @@ -151,7 +152,6 @@ static void recordFixedType(const MemRegion *Region, const CXXMethodDecl *MD, ProgramStateRef State = C.getState(); State = setDynamicTypeInfo(State, Region, Ty, /*CanBeSubclass=*/false); C.addTransition(State); - return; } void DynamicTypePropagation::checkPreCall(const CallEvent &Call, @@ -387,6 +387,14 @@ static const ObjCObjectPointerType *getMostInformativeDerivedClassImpl( } return From; } + + if (To->getObjectType()->getSuperClassType().isNull()) { + // If To has no super class and From and To aren't the same then + // To was not actually a descendent of From. In this case the best we can + // do is 'From'. + return From; + } + const auto *SuperOfTo = To->getObjectType()->getSuperClassType()->getAs<ObjCObjectType>(); assert(SuperOfTo); @@ -444,6 +452,23 @@ storeWhenMoreInformative(ProgramStateRef &State, SymbolRef Sym, const ObjCObjectPointerType *StaticLowerBound, const ObjCObjectPointerType *StaticUpperBound, ASTContext &C) { + // TODO: The above 4 cases are not exhaustive. In particular, it is possible + // for Current to be incomparable with StaticLowerBound, StaticUpperBound, + // or both. + // + // For example, suppose Foo<T> and Bar<T> are unrelated types. + // + // Foo<T> *f = ... + // Bar<T> *b = ... + // + // id t1 = b; + // f = t1; + // id t2 = f; // StaticLowerBound is Foo<T>, Current is Bar<T> + // + // We should either constrain the callers of this function so that the stated + // preconditions hold (and assert it) or rewrite the function to expicitly + // handle the additional cases. + // Precondition assert(StaticUpperBound->isSpecialized() || StaticLowerBound->isSpecialized()); @@ -772,7 +797,6 @@ void DynamicTypePropagation::checkPostObjCMessage(const ObjCMethodCall &M, // class. This method is provided by the runtime and available on all classes. if (MessageExpr->getReceiverKind() == ObjCMessageExpr::Class && Sel.getAsString() == "class") { - QualType ReceiverType = MessageExpr->getClassReceiver(); const auto *ReceiverClassType = ReceiverType->getAs<ObjCObjectType>(); QualType ReceiverClassPointerType = diff --git a/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp b/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp index 8f6c20ab19061..31e9150cc15b3 100644 --- a/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp @@ -11,6 +11,7 @@ #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" #include "clang/StaticAnalyzer/Core/Checker.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" +#include "clang/StaticAnalyzer/Checkers/SValExplainer.h" #include "llvm/ADT/StringSwitch.h" using namespace clang; @@ -25,17 +26,21 @@ class ExprInspectionChecker : public Checker<eval::Call, check::DeadSymbols> { void analyzerWarnIfReached(const CallExpr *CE, CheckerContext &C) const; void analyzerCrash(const CallExpr *CE, CheckerContext &C) const; void analyzerWarnOnDeadSymbol(const CallExpr *CE, CheckerContext &C) const; + void analyzerExplain(const CallExpr *CE, CheckerContext &C) const; + void analyzerGetExtent(const CallExpr *CE, CheckerContext &C) const; typedef void (ExprInspectionChecker::*FnCheck)(const CallExpr *, CheckerContext &C) const; + void reportBug(llvm::StringRef Msg, CheckerContext &C) const; + public: bool evalCall(const CallExpr *CE, CheckerContext &C) const; void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const; }; } -REGISTER_SET_WITH_PROGRAMSTATE(MarkedSymbols, const void *) +REGISTER_SET_WITH_PROGRAMSTATE(MarkedSymbols, SymbolRef) bool ExprInspectionChecker::evalCall(const CallExpr *CE, CheckerContext &C) const { @@ -50,6 +55,8 @@ bool ExprInspectionChecker::evalCall(const CallExpr *CE, &ExprInspectionChecker::analyzerWarnIfReached) .Case("clang_analyzer_warnOnDeadSymbol", &ExprInspectionChecker::analyzerWarnOnDeadSymbol) + .Case("clang_analyzer_explain", &ExprInspectionChecker::analyzerExplain) + .Case("clang_analyzer_getExtent", &ExprInspectionChecker::analyzerGetExtent) .Default(nullptr); if (!Handler) @@ -91,6 +98,18 @@ static const char *getArgumentValueString(const CallExpr *CE, } } +void ExprInspectionChecker::reportBug(llvm::StringRef Msg, + CheckerContext &C) const { + if (!BT) + BT.reset(new BugType(this, "Checking analyzer assumptions", "debug")); + + ExplodedNode *N = C.generateNonFatalErrorNode(); + if (!N) + return; + + C.emitReport(llvm::make_unique<BugReport>(*BT, Msg, N)); +} + void ExprInspectionChecker::analyzerEval(const CallExpr *CE, CheckerContext &C) const { const LocationContext *LC = C.getPredecessor()->getLocationContext(); @@ -100,26 +119,12 @@ void ExprInspectionChecker::analyzerEval(const CallExpr *CE, if (LC->getCurrentStackFrame()->getParent() != nullptr) return; - if (!BT) - BT.reset(new BugType(this, "Checking analyzer assumptions", "debug")); - - ExplodedNode *N = C.generateNonFatalErrorNode(); - if (!N) - return; - C.emitReport( - llvm::make_unique<BugReport>(*BT, getArgumentValueString(CE, C), N)); + reportBug(getArgumentValueString(CE, C), C); } void ExprInspectionChecker::analyzerWarnIfReached(const CallExpr *CE, CheckerContext &C) const { - - if (!BT) - BT.reset(new BugType(this, "Checking analyzer assumptions", "debug")); - - ExplodedNode *N = C.generateNonFatalErrorNode(); - if (!N) - return; - C.emitReport(llvm::make_unique<BugReport>(*BT, "REACHABLE", N)); + reportBug("REACHABLE", C); } void ExprInspectionChecker::analyzerCheckInlined(const CallExpr *CE, @@ -134,14 +139,32 @@ void ExprInspectionChecker::analyzerCheckInlined(const CallExpr *CE, if (LC->getCurrentStackFrame()->getParent() == nullptr) return; - if (!BT) - BT.reset(new BugType(this, "Checking analyzer assumptions", "debug")); + reportBug(getArgumentValueString(CE, C), C); +} - ExplodedNode *N = C.generateNonFatalErrorNode(); - if (!N) - return; - C.emitReport( - llvm::make_unique<BugReport>(*BT, getArgumentValueString(CE, C), N)); +void ExprInspectionChecker::analyzerExplain(const CallExpr *CE, + CheckerContext &C) const { + if (CE->getNumArgs() == 0) + reportBug("Missing argument for explaining", C); + + SVal V = C.getSVal(CE->getArg(0)); + SValExplainer Ex(C.getASTContext()); + reportBug(Ex.Visit(V), C); +} + +void ExprInspectionChecker::analyzerGetExtent(const CallExpr *CE, + CheckerContext &C) const { + if (CE->getNumArgs() == 0) + reportBug("Missing region for obtaining extent", C); + + auto MR = dyn_cast_or_null<SubRegion>(C.getSVal(CE->getArg(0)).getAsRegion()); + if (!MR) + reportBug("Obtaining extent of a non-region", C); + + ProgramStateRef State = C.getState(); + State = State->BindExpr(CE, C.getLocationContext(), + MR->getExtent(C.getSValBuilder())); + C.addTransition(State); } void ExprInspectionChecker::analyzerWarnOnDeadSymbol(const CallExpr *CE, @@ -163,20 +186,14 @@ void ExprInspectionChecker::checkDeadSymbols(SymbolReaper &SymReaper, ProgramStateRef State = C.getState(); const MarkedSymbolsTy &Syms = State->get<MarkedSymbols>(); for (auto I = Syms.begin(), E = Syms.end(); I != E; ++I) { - SymbolRef Sym = static_cast<SymbolRef>(*I); + SymbolRef Sym = *I; if (!SymReaper.isDead(Sym)) continue; - if (!BT) - BT.reset(new BugType(this, "Checking analyzer assumptions", "debug")); - - ExplodedNode *N = C.generateNonFatalErrorNode(); - if (!N) - return; - - C.emitReport(llvm::make_unique<BugReport>(*BT, "SYMBOL DEAD", N)); - C.addTransition(State->remove<MarkedSymbols>(Sym), N); + reportBug("SYMBOL DEAD", C); + State = State->remove<MarkedSymbols>(Sym); } + C.addTransition(State); } void ExprInspectionChecker::analyzerCrash(const CallExpr *CE, diff --git a/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp b/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp index dffff38c91a2b..8076ca09591f8 100644 --- a/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp @@ -1,4 +1,4 @@ -//=- IvarInvalidationChecker.cpp - -*- C++ -------------------------------*-==// +//===- IvarInvalidationChecker.cpp ------------------------------*- C++ -*-===// // // The LLVM Compiler Infrastructure // @@ -43,7 +43,6 @@ using namespace clang; using namespace ento; namespace { - struct ChecksFilter { /// Check for missing invalidation method declarations. DefaultBool check_MissingInvalidationMethod; @@ -55,7 +54,6 @@ struct ChecksFilter { }; class IvarInvalidationCheckerImpl { - typedef llvm::SmallSetVector<const ObjCMethodDecl*, 2> MethodSet; typedef llvm::DenseMap<const ObjCMethodDecl*, const ObjCIvarDecl*> MethToIvarMapTy; @@ -64,7 +62,6 @@ class IvarInvalidationCheckerImpl { typedef llvm::DenseMap<const ObjCIvarDecl*, const ObjCPropertyDecl*> IvarToPropMapTy; - struct InvalidationInfo { /// Has the ivar been invalidated? bool IsInvalidated; @@ -167,7 +164,7 @@ class IvarInvalidationCheckerImpl { void VisitObjCMessageExpr(const ObjCMessageExpr *ME); void VisitChildren(const Stmt *S) { - for (const Stmt *Child : S->children()) { + for (const auto *Child : S->children()) { if (Child) this->Visit(Child); if (CalledAnotherInvalidationMethod) @@ -208,6 +205,7 @@ class IvarInvalidationCheckerImpl { const IvarToPropMapTy &IvarToPopertyMap, const ObjCInterfaceDecl *InterfaceD, bool MissingDeclaration) const; + void reportIvarNeedsInvalidation(const ObjCIvarDecl *IvarD, const IvarToPropMapTy &IvarToPopertyMap, const ObjCMethodDecl *MethodD) const; @@ -276,8 +274,6 @@ void IvarInvalidationCheckerImpl::containsInvalidationMethod( } return; } - - return; } bool IvarInvalidationCheckerImpl::trackIvar(const ObjCIvarDecl *Iv, @@ -390,6 +386,8 @@ visit(const ObjCImplementationDecl *ImplD) const { for (ObjCInterfaceDecl::PropertyMap::iterator I = PropMap.begin(), E = PropMap.end(); I != E; ++I) { const ObjCPropertyDecl *PD = I->second; + if (PD->isClassProperty()) + continue; const ObjCIvarDecl *ID = findPropertyBackingIvar(PD, InterfaceD, Ivars, &FirstIvarDecl); @@ -584,8 +582,7 @@ void IvarInvalidationCheckerImpl::MethodCrawler::markInvalidated( // If InvalidationMethod is present, we are processing the message send and // should ensure we are invalidating with the appropriate method, // otherwise, we are processing setting to 'nil'. - if (!InvalidationMethod || - (InvalidationMethod && I->second.hasMethod(InvalidationMethod))) + if (!InvalidationMethod || I->second.hasMethod(InvalidationMethod)) IVars.erase(I); } } @@ -722,11 +719,10 @@ void IvarInvalidationCheckerImpl::MethodCrawler::VisitObjCMessageExpr( VisitStmt(ME); } -} +} // end anonymous namespace // Register the checkers. namespace { - class IvarInvalidationChecker : public Checker<check::ASTDecl<ObjCImplementationDecl> > { public: @@ -738,7 +734,7 @@ public: Walker.visit(D); } }; -} +} // end anonymous namespace #define REGISTER_CHECKER(name) \ void ento::register##name(CheckerManager &mgr) { \ @@ -750,4 +746,3 @@ public: REGISTER_CHECKER(InstanceVariableInvalidation) REGISTER_CHECKER(MissingInvalidationMethod) - diff --git a/lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp b/lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp index 56346cd4f7069..7be2f574f0e94 100644 --- a/lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp @@ -111,6 +111,30 @@ NonLocalizedStringChecker::NonLocalizedStringChecker() { "Localizability Issue (Apple)")); } +namespace { +class NonLocalizedStringBRVisitor final + : public BugReporterVisitorImpl<NonLocalizedStringBRVisitor> { + + const MemRegion *NonLocalizedString; + bool Satisfied; + +public: + NonLocalizedStringBRVisitor(const MemRegion *NonLocalizedString) + : NonLocalizedString(NonLocalizedString), Satisfied(false) { + assert(NonLocalizedString); + } + + PathDiagnosticPiece *VisitNode(const ExplodedNode *Succ, + const ExplodedNode *Pred, + BugReporterContext &BRC, + BugReport &BR) override; + + void Profile(llvm::FoldingSetNodeID &ID) const override { + ID.Add(NonLocalizedString); + } +}; +} // End anonymous namespace. + #define NEW_RECEIVER(receiver) \ llvm::DenseMap<Selector, uint8_t> &receiver##M = \ UIMethods.insert({&Ctx.Idents.get(#receiver), \ @@ -619,11 +643,46 @@ void NonLocalizedStringChecker::setNonLocalizedState(const SVal S, } } + +static bool isDebuggingName(std::string name) { + return StringRef(name).lower().find("debug") != StringRef::npos; +} + +/// Returns true when, heuristically, the analyzer may be analyzing debugging +/// code. We use this to suppress localization diagnostics in un-localized user +/// interfaces that are only used for debugging and are therefore not user +/// facing. +static bool isDebuggingContext(CheckerContext &C) { + const Decl *D = C.getCurrentAnalysisDeclContext()->getDecl(); + if (!D) + return false; + + if (auto *ND = dyn_cast<NamedDecl>(D)) { + if (isDebuggingName(ND->getNameAsString())) + return true; + } + + const DeclContext *DC = D->getDeclContext(); + + if (auto *CD = dyn_cast<ObjCContainerDecl>(DC)) { + if (isDebuggingName(CD->getNameAsString())) + return true; + } + + return false; +} + + /// Reports a localization error for the passed in method call and SVal void NonLocalizedStringChecker::reportLocalizationError( SVal S, const ObjCMethodCall &M, CheckerContext &C, int argumentNumber) const { + // Don't warn about localization errors in classes and methods that + // may be debug code. + if (isDebuggingContext(C)) + return; + ExplodedNode *ErrNode = C.getPredecessor(); static CheckerProgramPointTag Tag("NonLocalizedStringChecker", "UnlocalizedString"); @@ -641,6 +700,11 @@ void NonLocalizedStringChecker::reportLocalizationError( R->addRange(M.getSourceRange()); } R->markInteresting(S); + + const MemRegion *StringRegion = S.getAsRegion(); + if (StringRegion) + R->addVisitor(llvm::make_unique<NonLocalizedStringBRVisitor>(StringRegion)); + C.emitReport(std::move(R)); } @@ -831,6 +895,41 @@ void NonLocalizedStringChecker::checkPostStmt(const ObjCStringLiteral *SL, setNonLocalizedState(sv, C); } +PathDiagnosticPiece * +NonLocalizedStringBRVisitor::VisitNode(const ExplodedNode *Succ, + const ExplodedNode *Pred, + BugReporterContext &BRC, BugReport &BR) { + if (Satisfied) + return nullptr; + + Optional<StmtPoint> Point = Succ->getLocation().getAs<StmtPoint>(); + if (!Point.hasValue()) + return nullptr; + + auto *LiteralExpr = dyn_cast<ObjCStringLiteral>(Point->getStmt()); + if (!LiteralExpr) + return nullptr; + + ProgramStateRef State = Succ->getState(); + SVal LiteralSVal = State->getSVal(LiteralExpr, Succ->getLocationContext()); + if (LiteralSVal.getAsRegion() != NonLocalizedString) + return nullptr; + + Satisfied = true; + + PathDiagnosticLocation L = + PathDiagnosticLocation::create(*Point, BRC.getSourceManager()); + + if (!L.isValid() || !L.asLocation().isValid()) + return nullptr; + + auto *Piece = new PathDiagnosticEventPiece(L, + "Non-localized string literal here"); + Piece->addRange(LiteralExpr->getSourceRange()); + + return Piece; +} + namespace { class EmptyLocalizationContextChecker : public Checker<check::ASTDecl<ObjCImplementationDecl>> { @@ -965,7 +1064,7 @@ void EmptyLocalizationContextChecker::MethodCrawler::VisitObjCMessageExpr( return; StringRef Comment = - StringRef(Result.getLiteralData(), Result.getLength()).trim("\""); + StringRef(Result.getLiteralData(), Result.getLength()).trim('"'); if ((Comment.trim().size() == 0 && Comment.size() > 0) || // Is Whitespace Comment.empty()) { diff --git a/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.cpp b/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.cpp new file mode 100644 index 0000000000000..d56ea6d689d34 --- /dev/null +++ b/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.cpp @@ -0,0 +1,115 @@ +//===-- MPIBugReporter.cpp - bug reporter -----------------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +/// +/// \file +/// This file defines prefabricated reports which are emitted in +/// case of MPI related bugs, detected by path-sensitive analysis. +/// +//===----------------------------------------------------------------------===// + +#include "MPIBugReporter.h" +#include "MPIChecker.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" + +namespace clang { +namespace ento { +namespace mpi { + +void MPIBugReporter::reportDoubleNonblocking( + const CallEvent &MPICallEvent, const ento::mpi::Request &Req, + const MemRegion *const RequestRegion, + const ExplodedNode *const ExplNode, + BugReporter &BReporter) const { + + std::string ErrorText; + ErrorText = "Double nonblocking on request " + + RequestRegion->getDescriptiveName() + ". "; + + auto Report = llvm::make_unique<BugReport>(*DoubleNonblockingBugType, + ErrorText, ExplNode); + + Report->addRange(MPICallEvent.getSourceRange()); + SourceRange Range = RequestRegion->sourceRange(); + + if (Range.isValid()) + Report->addRange(Range); + + Report->addVisitor(llvm::make_unique<RequestNodeVisitor>( + RequestRegion, "Request is previously used by nonblocking call here. ")); + Report->markInteresting(RequestRegion); + + BReporter.emitReport(std::move(Report)); +} + +void MPIBugReporter::reportMissingWait( + const ento::mpi::Request &Req, const MemRegion *const RequestRegion, + const ExplodedNode *const ExplNode, + BugReporter &BReporter) const { + std::string ErrorText{"Request " + RequestRegion->getDescriptiveName() + + " has no matching wait. "}; + + auto Report = + llvm::make_unique<BugReport>(*MissingWaitBugType, ErrorText, ExplNode); + + SourceRange Range = RequestRegion->sourceRange(); + if (Range.isValid()) + Report->addRange(Range); + Report->addVisitor(llvm::make_unique<RequestNodeVisitor>( + RequestRegion, "Request is previously used by nonblocking call here. ")); + Report->markInteresting(RequestRegion); + + BReporter.emitReport(std::move(Report)); +} + +void MPIBugReporter::reportUnmatchedWait( + const CallEvent &CE, const clang::ento::MemRegion *const RequestRegion, + const ExplodedNode *const ExplNode, + BugReporter &BReporter) const { + std::string ErrorText{"Request " + RequestRegion->getDescriptiveName() + + " has no matching nonblocking call. "}; + + auto Report = + llvm::make_unique<BugReport>(*UnmatchedWaitBugType, ErrorText, ExplNode); + + Report->addRange(CE.getSourceRange()); + SourceRange Range = RequestRegion->sourceRange(); + if (Range.isValid()) + Report->addRange(Range); + + BReporter.emitReport(std::move(Report)); +} + +PathDiagnosticPiece *MPIBugReporter::RequestNodeVisitor::VisitNode( + const ExplodedNode *N, const ExplodedNode *PrevN, BugReporterContext &BRC, + BugReport &BR) { + + if (IsNodeFound) + return nullptr; + + const Request *const Req = N->getState()->get<RequestMap>(RequestRegion); + const Request *const PrevReq = + PrevN->getState()->get<RequestMap>(RequestRegion); + + // Check if request was previously unused or in a different state. + if ((Req && !PrevReq) || (Req->CurrentState != PrevReq->CurrentState)) { + IsNodeFound = true; + + ProgramPoint P = PrevN->getLocation(); + PathDiagnosticLocation L = + PathDiagnosticLocation::create(P, BRC.getSourceManager()); + + return new PathDiagnosticEventPiece(L, ErrorText); + } + + return nullptr; +} + +} // end of namespace: mpi +} // end of namespace: ento +} // end of namespace: clang diff --git a/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.h b/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.h new file mode 100644 index 0000000000000..22fbf4c5b303d --- /dev/null +++ b/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.h @@ -0,0 +1,111 @@ +//===-- MPIBugReporter.h - bug reporter -----------------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +/// +/// \file +/// This file defines prefabricated reports which are emitted in +/// case of MPI related bugs, detected by path-sensitive analysis. +/// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_LIB_STATICANALYZER_CHECKERS_MPICHECKER_MPIBUGREPORTER_H +#define LLVM_CLANG_LIB_STATICANALYZER_CHECKERS_MPICHECKER_MPIBUGREPORTER_H + +#include "MPITypes.h" +#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" + +namespace clang { +namespace ento { +namespace mpi { + +class MPIBugReporter { +public: + MPIBugReporter(const CheckerBase &CB) { + UnmatchedWaitBugType.reset(new BugType(&CB, "Unmatched wait", MPIError)); + DoubleNonblockingBugType.reset( + new BugType(&CB, "Double nonblocking", MPIError)); + MissingWaitBugType.reset(new BugType(&CB, "Missing wait", MPIError)); + } + + /// Report duplicate request use by nonblocking calls without intermediate + /// wait. + /// + /// \param MPICallEvent MPI call that caused the double nonblocking + /// \param Req request that was used by two nonblocking calls in sequence + /// \param RequestRegion memory region of the request + /// \param ExplNode node in the graph the bug appeared at + /// \param BReporter bug reporter for current context + void reportDoubleNonblocking(const CallEvent &MPICallEvent, + const Request &Req, + const MemRegion *const RequestRegion, + const ExplodedNode *const ExplNode, + BugReporter &BReporter) const; + + /// Report a missing wait for a nonblocking call. A missing wait report + /// is emitted if a nonblocking call is not matched in the scope of a + /// function. + /// + /// \param Req request that is not matched by a wait + /// \param RequestRegion memory region of the request + /// \param ExplNode node in the graph the bug appeared at + /// \param BReporter bug reporter for current context + void reportMissingWait(const Request &Req, + const MemRegion *const RequestRegion, + const ExplodedNode *const ExplNode, + BugReporter &BReporter) const; + + /// Report a wait on a request that has not been used at all before. + /// + /// \param CE wait call that uses the request + /// \param RequestRegion memory region of the request + /// \param ExplNode node in the graph the bug appeared at + /// \param BReporter bug reporter for current context + void reportUnmatchedWait(const CallEvent &CE, + const MemRegion *const RequestRegion, + const ExplodedNode *const ExplNode, + BugReporter &BReporter) const; + +private: + const std::string MPIError = "MPI Error"; + + // path-sensitive bug types + std::unique_ptr<BugType> UnmatchedWaitBugType; + std::unique_ptr<BugType> MissingWaitBugType; + std::unique_ptr<BugType> DoubleNonblockingBugType; + + /// Bug visitor class to find the node where the request region was previously + /// used in order to include it into the BugReport path. + class RequestNodeVisitor : public BugReporterVisitorImpl<RequestNodeVisitor> { + public: + RequestNodeVisitor(const MemRegion *const MemoryRegion, + const std::string &ErrText) + : RequestRegion(MemoryRegion), ErrorText(ErrText) {} + + void Profile(llvm::FoldingSetNodeID &ID) const override { + static int X = 0; + ID.AddPointer(&X); + ID.AddPointer(RequestRegion); + } + + PathDiagnosticPiece *VisitNode(const ExplodedNode *N, + const ExplodedNode *PrevN, + BugReporterContext &BRC, + BugReport &BR) override; + + private: + const MemRegion *const RequestRegion; + bool IsNodeFound = false; + std::string ErrorText; + }; +}; + +} // end of namespace: mpi +} // end of namespace: ento +} // end of namespace: clang + +#endif diff --git a/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp b/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp new file mode 100644 index 0000000000000..c3d0f8f2a1298 --- /dev/null +++ b/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp @@ -0,0 +1,190 @@ +//===-- MPIChecker.cpp - Checker Entry Point Class --------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +/// +/// \file +/// This file defines the main class of MPI-Checker which serves as an entry +/// point. It is created once for each translation unit analysed. +/// The checker defines path-sensitive checks, to verify correct usage of the +/// MPI API. +/// +//===----------------------------------------------------------------------===// + +#include "MPIChecker.h" +#include "../ClangSACheckers.h" + +namespace clang { +namespace ento { +namespace mpi { + +void MPIChecker::checkDoubleNonblocking(const CallEvent &PreCallEvent, + CheckerContext &Ctx) const { + if (!FuncClassifier->isNonBlockingType(PreCallEvent.getCalleeIdentifier())) { + return; + } + const MemRegion *const MR = + PreCallEvent.getArgSVal(PreCallEvent.getNumArgs() - 1).getAsRegion(); + if (!MR) + return; + const ElementRegion *const ER = dyn_cast<ElementRegion>(MR); + + // The region must be typed, in order to reason about it. + if (!isa<TypedRegion>(MR) || (ER && !isa<TypedRegion>(ER->getSuperRegion()))) + return; + + ProgramStateRef State = Ctx.getState(); + const Request *const Req = State->get<RequestMap>(MR); + + // double nonblocking detected + if (Req && Req->CurrentState == Request::State::Nonblocking) { + ExplodedNode *ErrorNode = Ctx.generateNonFatalErrorNode(); + BReporter.reportDoubleNonblocking(PreCallEvent, *Req, MR, ErrorNode, Ctx.getBugReporter()); + Ctx.addTransition(ErrorNode->getState(), ErrorNode); + } + // no error + else { + State = State->set<RequestMap>(MR, Request::State::Nonblocking); + Ctx.addTransition(State); + } +} + +void MPIChecker::checkUnmatchedWaits(const CallEvent &PreCallEvent, + CheckerContext &Ctx) const { + if (!FuncClassifier->isWaitType(PreCallEvent.getCalleeIdentifier())) + return; + const MemRegion *const MR = topRegionUsedByWait(PreCallEvent); + if (!MR) + return; + const ElementRegion *const ER = dyn_cast<ElementRegion>(MR); + + // The region must be typed, in order to reason about it. + if (!isa<TypedRegion>(MR) || (ER && !isa<TypedRegion>(ER->getSuperRegion()))) + return; + + llvm::SmallVector<const MemRegion *, 2> ReqRegions; + allRegionsUsedByWait(ReqRegions, MR, PreCallEvent, Ctx); + if (ReqRegions.empty()) + return; + + ProgramStateRef State = Ctx.getState(); + static CheckerProgramPointTag Tag("MPI-Checker", "UnmatchedWait"); + ExplodedNode *ErrorNode{nullptr}; + + // Check all request regions used by the wait function. + for (const auto &ReqRegion : ReqRegions) { + const Request *const Req = State->get<RequestMap>(ReqRegion); + State = State->set<RequestMap>(ReqRegion, Request::State::Wait); + if (!Req) { + if (!ErrorNode) { + ErrorNode = Ctx.generateNonFatalErrorNode(State, &Tag); + State = ErrorNode->getState(); + } + // A wait has no matching nonblocking call. + BReporter.reportUnmatchedWait(PreCallEvent, ReqRegion, ErrorNode, Ctx.getBugReporter()); + } + } + + if (!ErrorNode) { + Ctx.addTransition(State); + } else { + Ctx.addTransition(State, ErrorNode); + } +} + +void MPIChecker::checkMissingWaits(SymbolReaper &SymReaper, + CheckerContext &Ctx) const { + if (!SymReaper.hasDeadSymbols()) + return; + + ProgramStateRef State = Ctx.getState(); + const auto &Requests = State->get<RequestMap>(); + if (Requests.isEmpty()) + return; + + static CheckerProgramPointTag Tag("MPI-Checker", "MissingWait"); + ExplodedNode *ErrorNode{nullptr}; + + auto ReqMap = State->get<RequestMap>(); + for (const auto &Req : ReqMap) { + if (!SymReaper.isLiveRegion(Req.first)) { + if (Req.second.CurrentState == Request::State::Nonblocking) { + + if (!ErrorNode) { + ErrorNode = Ctx.generateNonFatalErrorNode(State, &Tag); + State = ErrorNode->getState(); + } + BReporter.reportMissingWait(Req.second, Req.first, ErrorNode, Ctx.getBugReporter()); + } + State = State->remove<RequestMap>(Req.first); + } + } + + // Transition to update the state regarding removed requests. + if (!ErrorNode) { + Ctx.addTransition(State); + } else { + Ctx.addTransition(State, ErrorNode); + } +} + +const MemRegion *MPIChecker::topRegionUsedByWait(const CallEvent &CE) const { + + if (FuncClassifier->isMPI_Wait(CE.getCalleeIdentifier())) { + return CE.getArgSVal(0).getAsRegion(); + } else if (FuncClassifier->isMPI_Waitall(CE.getCalleeIdentifier())) { + return CE.getArgSVal(1).getAsRegion(); + } else { + return (const MemRegion *)nullptr; + } +} + +void MPIChecker::allRegionsUsedByWait( + llvm::SmallVector<const MemRegion *, 2> &ReqRegions, + const MemRegion *const MR, const CallEvent &CE, CheckerContext &Ctx) const { + + MemRegionManager *const RegionManager = MR->getMemRegionManager(); + + if (FuncClassifier->isMPI_Waitall(CE.getCalleeIdentifier())) { + const MemRegion *SuperRegion{nullptr}; + if (const ElementRegion *const ER = MR->getAs<ElementRegion>()) { + SuperRegion = ER->getSuperRegion(); + } + + // A single request is passed to MPI_Waitall. + if (!SuperRegion) { + ReqRegions.push_back(MR); + return; + } + + const auto &Size = Ctx.getStoreManager().getSizeInElements( + Ctx.getState(), SuperRegion, + CE.getArgExpr(1)->getType()->getPointeeType()); + const llvm::APSInt &ArrSize = Size.getAs<nonloc::ConcreteInt>()->getValue(); + + for (size_t i = 0; i < ArrSize; ++i) { + const NonLoc Idx = Ctx.getSValBuilder().makeArrayIndex(i); + + const ElementRegion *const ER = RegionManager->getElementRegion( + CE.getArgExpr(1)->getType()->getPointeeType(), Idx, SuperRegion, + Ctx.getASTContext()); + + ReqRegions.push_back(ER->getAs<MemRegion>()); + } + } else if (FuncClassifier->isMPI_Wait(CE.getCalleeIdentifier())) { + ReqRegions.push_back(MR); + } +} + +} // end of namespace: mpi +} // end of namespace: ento +} // end of namespace: clang + +// Registers the checker for static analysis. +void clang::ento::registerMPIChecker(CheckerManager &MGR) { + MGR.registerChecker<clang::ento::mpi::MPIChecker>(); +} diff --git a/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.h b/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.h new file mode 100644 index 0000000000000..20c60ad076a21 --- /dev/null +++ b/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.h @@ -0,0 +1,107 @@ +//===-- MPIChecker.h - Verify MPI API usage- --------------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +/// +/// \file +/// This file defines the main class of MPI-Checker which serves as an entry +/// point. It is created once for each translation unit analysed. +/// The checker defines path-sensitive checks, to verify correct usage of the +/// MPI API. +/// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_LIB_STATICANALYZER_CHECKERS_MPICHECKER_MPICHECKER_H +#define LLVM_CLANG_LIB_STATICANALYZER_CHECKERS_MPICHECKER_MPICHECKER_H + +#include "MPIBugReporter.h" +#include "MPIFunctionClassifier.h" +#include "MPITypes.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" + +namespace clang { +namespace ento { +namespace mpi { + +class MPIChecker : public Checker<check::PreCall, check::DeadSymbols> { +public: + MPIChecker() : BReporter(*this) { } + + // path-sensitive callbacks + void checkPreCall(const CallEvent &CE, CheckerContext &Ctx) const { + dynamicInit(Ctx); + checkUnmatchedWaits(CE, Ctx); + checkDoubleNonblocking(CE, Ctx); + } + + void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &Ctx) const { + dynamicInit(Ctx); + checkMissingWaits(SymReaper, Ctx); + } + + void dynamicInit(CheckerContext &Ctx) const { + if (FuncClassifier) + return; + const_cast<std::unique_ptr<MPIFunctionClassifier> &>(FuncClassifier) + .reset(new MPIFunctionClassifier{Ctx.getASTContext()}); + + } + + /// Checks if a request is used by nonblocking calls multiple times + /// in sequence without intermediate wait. The check contains a guard, + /// in order to only inspect nonblocking functions. + /// + /// \param PreCallEvent MPI call to verify + void checkDoubleNonblocking(const clang::ento::CallEvent &PreCallEvent, + clang::ento::CheckerContext &Ctx) const; + + /// Checks if a request is used by a wait multiple times in sequence without + /// intermediate nonblocking call or if the request used by the wait + /// function was not used at all before. The check contains a guard, + /// in order to only inspect wait functions. + /// + /// \param PreCallEvent MPI call to verify + void checkUnmatchedWaits(const clang::ento::CallEvent &PreCallEvent, + clang::ento::CheckerContext &Ctx) const; + + /// Check if a nonblocking call is not matched by a wait. + /// If a memory region is not alive and the last function using the + /// request was a nonblocking call, this is rated as a missing wait. + void checkMissingWaits(clang::ento::SymbolReaper &SymReaper, + clang::ento::CheckerContext &Ctx) const; + +private: + /// Collects all memory regions of a request(array) used by a wait + /// function. If the wait function uses a single request, this is a single + /// region. For wait functions using multiple requests, multiple regions + /// representing elements in the array are collected. + /// + /// \param ReqRegions vector the regions get pushed into + /// \param MR top most region to iterate + /// \param CE MPI wait call using the request(s) + void allRegionsUsedByWait( + llvm::SmallVector<const clang::ento::MemRegion *, 2> &ReqRegions, + const clang::ento::MemRegion *const MR, const clang::ento::CallEvent &CE, + clang::ento::CheckerContext &Ctx) const; + + /// Returns the memory region used by a wait function. + /// Distinguishes between MPI_Wait and MPI_Waitall. + /// + /// \param CE MPI wait call + const clang::ento::MemRegion * + topRegionUsedByWait(const clang::ento::CallEvent &CE) const; + + const std::unique_ptr<MPIFunctionClassifier> FuncClassifier; + MPIBugReporter BReporter; +}; + +} // end of namespace: mpi +} // end of namespace: ento +} // end of namespace: clang + +#endif diff --git a/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIFunctionClassifier.cpp b/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIFunctionClassifier.cpp new file mode 100644 index 0000000000000..ad937f683d30e --- /dev/null +++ b/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIFunctionClassifier.cpp @@ -0,0 +1,284 @@ +//===-- MPIFunctionClassifier.cpp - classifies MPI functions ----*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +/// +/// \file +/// This file defines functionality to identify and classify MPI functions. +/// +//===----------------------------------------------------------------------===// + +#include "MPIFunctionClassifier.h" +#include "llvm/ADT/STLExtras.h" + +namespace clang { +namespace ento { +namespace mpi { + +void MPIFunctionClassifier::identifierInit(ASTContext &ASTCtx) { + // Initialize function identifiers. + initPointToPointIdentifiers(ASTCtx); + initCollectiveIdentifiers(ASTCtx); + initAdditionalIdentifiers(ASTCtx); +} + +void MPIFunctionClassifier::initPointToPointIdentifiers(ASTContext &ASTCtx) { + // Copy identifiers into the correct classification containers. + IdentInfo_MPI_Send = &ASTCtx.Idents.get("MPI_Send"); + MPIPointToPointTypes.push_back(IdentInfo_MPI_Send); + MPIType.push_back(IdentInfo_MPI_Send); + assert(IdentInfo_MPI_Send); + + IdentInfo_MPI_Isend = &ASTCtx.Idents.get("MPI_Isend"); + MPIPointToPointTypes.push_back(IdentInfo_MPI_Isend); + MPINonBlockingTypes.push_back(IdentInfo_MPI_Isend); + MPIType.push_back(IdentInfo_MPI_Isend); + assert(IdentInfo_MPI_Isend); + + IdentInfo_MPI_Ssend = &ASTCtx.Idents.get("MPI_Ssend"); + MPIPointToPointTypes.push_back(IdentInfo_MPI_Ssend); + MPIType.push_back(IdentInfo_MPI_Ssend); + assert(IdentInfo_MPI_Ssend); + + IdentInfo_MPI_Issend = &ASTCtx.Idents.get("MPI_Issend"); + MPIPointToPointTypes.push_back(IdentInfo_MPI_Issend); + MPINonBlockingTypes.push_back(IdentInfo_MPI_Issend); + MPIType.push_back(IdentInfo_MPI_Issend); + assert(IdentInfo_MPI_Issend); + + IdentInfo_MPI_Bsend = &ASTCtx.Idents.get("MPI_Bsend"); + MPIPointToPointTypes.push_back(IdentInfo_MPI_Bsend); + MPIType.push_back(IdentInfo_MPI_Bsend); + assert(IdentInfo_MPI_Bsend); + + IdentInfo_MPI_Ibsend = &ASTCtx.Idents.get("MPI_Ibsend"); + MPIPointToPointTypes.push_back(IdentInfo_MPI_Ibsend); + MPINonBlockingTypes.push_back(IdentInfo_MPI_Ibsend); + MPIType.push_back(IdentInfo_MPI_Ibsend); + assert(IdentInfo_MPI_Ibsend); + + IdentInfo_MPI_Rsend = &ASTCtx.Idents.get("MPI_Rsend"); + MPIPointToPointTypes.push_back(IdentInfo_MPI_Rsend); + MPIType.push_back(IdentInfo_MPI_Rsend); + assert(IdentInfo_MPI_Rsend); + + IdentInfo_MPI_Irsend = &ASTCtx.Idents.get("MPI_Irsend"); + MPIPointToPointTypes.push_back(IdentInfo_MPI_Irsend); + MPIType.push_back(IdentInfo_MPI_Irsend); + assert(IdentInfo_MPI_Irsend); + + IdentInfo_MPI_Recv = &ASTCtx.Idents.get("MPI_Recv"); + MPIPointToPointTypes.push_back(IdentInfo_MPI_Recv); + MPIType.push_back(IdentInfo_MPI_Recv); + assert(IdentInfo_MPI_Recv); + + IdentInfo_MPI_Irecv = &ASTCtx.Idents.get("MPI_Irecv"); + MPIPointToPointTypes.push_back(IdentInfo_MPI_Irecv); + MPINonBlockingTypes.push_back(IdentInfo_MPI_Irecv); + MPIType.push_back(IdentInfo_MPI_Irecv); + assert(IdentInfo_MPI_Irecv); +} + +void MPIFunctionClassifier::initCollectiveIdentifiers(ASTContext &ASTCtx) { + // Copy identifiers into the correct classification containers. + IdentInfo_MPI_Scatter = &ASTCtx.Idents.get("MPI_Scatter"); + MPICollectiveTypes.push_back(IdentInfo_MPI_Scatter); + MPIPointToCollTypes.push_back(IdentInfo_MPI_Scatter); + MPIType.push_back(IdentInfo_MPI_Scatter); + assert(IdentInfo_MPI_Scatter); + + IdentInfo_MPI_Iscatter = &ASTCtx.Idents.get("MPI_Iscatter"); + MPICollectiveTypes.push_back(IdentInfo_MPI_Iscatter); + MPIPointToCollTypes.push_back(IdentInfo_MPI_Iscatter); + MPINonBlockingTypes.push_back(IdentInfo_MPI_Iscatter); + MPIType.push_back(IdentInfo_MPI_Iscatter); + assert(IdentInfo_MPI_Iscatter); + + IdentInfo_MPI_Gather = &ASTCtx.Idents.get("MPI_Gather"); + MPICollectiveTypes.push_back(IdentInfo_MPI_Gather); + MPICollToPointTypes.push_back(IdentInfo_MPI_Gather); + MPIType.push_back(IdentInfo_MPI_Gather); + assert(IdentInfo_MPI_Gather); + + IdentInfo_MPI_Igather = &ASTCtx.Idents.get("MPI_Igather"); + MPICollectiveTypes.push_back(IdentInfo_MPI_Igather); + MPICollToPointTypes.push_back(IdentInfo_MPI_Igather); + MPINonBlockingTypes.push_back(IdentInfo_MPI_Igather); + MPIType.push_back(IdentInfo_MPI_Igather); + assert(IdentInfo_MPI_Igather); + + IdentInfo_MPI_Allgather = &ASTCtx.Idents.get("MPI_Allgather"); + MPICollectiveTypes.push_back(IdentInfo_MPI_Allgather); + MPICollToCollTypes.push_back(IdentInfo_MPI_Allgather); + MPIType.push_back(IdentInfo_MPI_Allgather); + assert(IdentInfo_MPI_Allgather); + + IdentInfo_MPI_Iallgather = &ASTCtx.Idents.get("MPI_Iallgather"); + MPICollectiveTypes.push_back(IdentInfo_MPI_Iallgather); + MPICollToCollTypes.push_back(IdentInfo_MPI_Iallgather); + MPINonBlockingTypes.push_back(IdentInfo_MPI_Iallgather); + MPIType.push_back(IdentInfo_MPI_Iallgather); + assert(IdentInfo_MPI_Iallgather); + + IdentInfo_MPI_Bcast = &ASTCtx.Idents.get("MPI_Bcast"); + MPICollectiveTypes.push_back(IdentInfo_MPI_Bcast); + MPIPointToCollTypes.push_back(IdentInfo_MPI_Bcast); + MPIType.push_back(IdentInfo_MPI_Bcast); + assert(IdentInfo_MPI_Bcast); + + IdentInfo_MPI_Ibcast = &ASTCtx.Idents.get("MPI_Ibcast"); + MPICollectiveTypes.push_back(IdentInfo_MPI_Ibcast); + MPIPointToCollTypes.push_back(IdentInfo_MPI_Ibcast); + MPINonBlockingTypes.push_back(IdentInfo_MPI_Ibcast); + MPIType.push_back(IdentInfo_MPI_Ibcast); + assert(IdentInfo_MPI_Ibcast); + + IdentInfo_MPI_Reduce = &ASTCtx.Idents.get("MPI_Reduce"); + MPICollectiveTypes.push_back(IdentInfo_MPI_Reduce); + MPICollToPointTypes.push_back(IdentInfo_MPI_Reduce); + MPIType.push_back(IdentInfo_MPI_Reduce); + assert(IdentInfo_MPI_Reduce); + + IdentInfo_MPI_Ireduce = &ASTCtx.Idents.get("MPI_Ireduce"); + MPICollectiveTypes.push_back(IdentInfo_MPI_Ireduce); + MPICollToPointTypes.push_back(IdentInfo_MPI_Ireduce); + MPINonBlockingTypes.push_back(IdentInfo_MPI_Ireduce); + MPIType.push_back(IdentInfo_MPI_Ireduce); + assert(IdentInfo_MPI_Ireduce); + + IdentInfo_MPI_Allreduce = &ASTCtx.Idents.get("MPI_Allreduce"); + MPICollectiveTypes.push_back(IdentInfo_MPI_Allreduce); + MPICollToCollTypes.push_back(IdentInfo_MPI_Allreduce); + MPIType.push_back(IdentInfo_MPI_Allreduce); + assert(IdentInfo_MPI_Allreduce); + + IdentInfo_MPI_Iallreduce = &ASTCtx.Idents.get("MPI_Iallreduce"); + MPICollectiveTypes.push_back(IdentInfo_MPI_Iallreduce); + MPICollToCollTypes.push_back(IdentInfo_MPI_Iallreduce); + MPINonBlockingTypes.push_back(IdentInfo_MPI_Iallreduce); + MPIType.push_back(IdentInfo_MPI_Iallreduce); + assert(IdentInfo_MPI_Iallreduce); + + IdentInfo_MPI_Alltoall = &ASTCtx.Idents.get("MPI_Alltoall"); + MPICollectiveTypes.push_back(IdentInfo_MPI_Alltoall); + MPICollToCollTypes.push_back(IdentInfo_MPI_Alltoall); + MPIType.push_back(IdentInfo_MPI_Alltoall); + assert(IdentInfo_MPI_Alltoall); + + IdentInfo_MPI_Ialltoall = &ASTCtx.Idents.get("MPI_Ialltoall"); + MPICollectiveTypes.push_back(IdentInfo_MPI_Ialltoall); + MPICollToCollTypes.push_back(IdentInfo_MPI_Ialltoall); + MPINonBlockingTypes.push_back(IdentInfo_MPI_Ialltoall); + MPIType.push_back(IdentInfo_MPI_Ialltoall); + assert(IdentInfo_MPI_Ialltoall); +} + +void MPIFunctionClassifier::initAdditionalIdentifiers(ASTContext &ASTCtx) { + IdentInfo_MPI_Comm_rank = &ASTCtx.Idents.get("MPI_Comm_rank"); + MPIType.push_back(IdentInfo_MPI_Comm_rank); + assert(IdentInfo_MPI_Comm_rank); + + IdentInfo_MPI_Comm_size = &ASTCtx.Idents.get("MPI_Comm_size"); + MPIType.push_back(IdentInfo_MPI_Comm_size); + assert(IdentInfo_MPI_Comm_size); + + IdentInfo_MPI_Wait = &ASTCtx.Idents.get("MPI_Wait"); + MPIType.push_back(IdentInfo_MPI_Wait); + assert(IdentInfo_MPI_Wait); + + IdentInfo_MPI_Waitall = &ASTCtx.Idents.get("MPI_Waitall"); + MPIType.push_back(IdentInfo_MPI_Waitall); + assert(IdentInfo_MPI_Waitall); + + IdentInfo_MPI_Barrier = &ASTCtx.Idents.get("MPI_Barrier"); + MPICollectiveTypes.push_back(IdentInfo_MPI_Barrier); + MPIType.push_back(IdentInfo_MPI_Barrier); + assert(IdentInfo_MPI_Barrier); +} + +// general identifiers +bool MPIFunctionClassifier::isMPIType(const IdentifierInfo *IdentInfo) const { + return llvm::is_contained(MPIType, IdentInfo); +} + +bool MPIFunctionClassifier::isNonBlockingType( + const IdentifierInfo *IdentInfo) const { + return llvm::is_contained(MPINonBlockingTypes, IdentInfo); +} + +// point-to-point identifiers +bool MPIFunctionClassifier::isPointToPointType( + const IdentifierInfo *IdentInfo) const { + return llvm::is_contained(MPIPointToPointTypes, IdentInfo); +} + +// collective identifiers +bool MPIFunctionClassifier::isCollectiveType( + const IdentifierInfo *IdentInfo) const { + return llvm::is_contained(MPICollectiveTypes, IdentInfo); +} + +bool MPIFunctionClassifier::isCollToColl( + const IdentifierInfo *IdentInfo) const { + return llvm::is_contained(MPICollToCollTypes, IdentInfo); +} + +bool MPIFunctionClassifier::isScatterType( + const IdentifierInfo *IdentInfo) const { + return IdentInfo == IdentInfo_MPI_Scatter || + IdentInfo == IdentInfo_MPI_Iscatter; +} + +bool MPIFunctionClassifier::isGatherType( + const IdentifierInfo *IdentInfo) const { + return IdentInfo == IdentInfo_MPI_Gather || + IdentInfo == IdentInfo_MPI_Igather || + IdentInfo == IdentInfo_MPI_Allgather || + IdentInfo == IdentInfo_MPI_Iallgather; +} + +bool MPIFunctionClassifier::isAllgatherType( + const IdentifierInfo *IdentInfo) const { + return IdentInfo == IdentInfo_MPI_Allgather || + IdentInfo == IdentInfo_MPI_Iallgather; +} + +bool MPIFunctionClassifier::isAlltoallType( + const IdentifierInfo *IdentInfo) const { + return IdentInfo == IdentInfo_MPI_Alltoall || + IdentInfo == IdentInfo_MPI_Ialltoall; +} + +bool MPIFunctionClassifier::isBcastType(const IdentifierInfo *IdentInfo) const { + return IdentInfo == IdentInfo_MPI_Bcast || IdentInfo == IdentInfo_MPI_Ibcast; +} + +bool MPIFunctionClassifier::isReduceType( + const IdentifierInfo *IdentInfo) const { + return IdentInfo == IdentInfo_MPI_Reduce || + IdentInfo == IdentInfo_MPI_Ireduce || + IdentInfo == IdentInfo_MPI_Allreduce || + IdentInfo == IdentInfo_MPI_Iallreduce; +} + +// additional identifiers +bool MPIFunctionClassifier::isMPI_Wait(const IdentifierInfo *IdentInfo) const { + return IdentInfo == IdentInfo_MPI_Wait; +} + +bool MPIFunctionClassifier::isMPI_Waitall( + const IdentifierInfo *IdentInfo) const { + return IdentInfo == IdentInfo_MPI_Waitall; +} + +bool MPIFunctionClassifier::isWaitType(const IdentifierInfo *IdentInfo) const { + return IdentInfo == IdentInfo_MPI_Wait || IdentInfo == IdentInfo_MPI_Waitall; +} + +} // end of namespace: mpi +} // end of namespace: ento +} // end of namespace: clang diff --git a/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIFunctionClassifier.h b/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIFunctionClassifier.h new file mode 100644 index 0000000000000..65e908912c548 --- /dev/null +++ b/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIFunctionClassifier.h @@ -0,0 +1,97 @@ +//===-- MPIFunctionClassifier.h - classifies MPI functions ----*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +/// +/// \file +/// This file defines functionality to identify and classify MPI functions. +/// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_LIB_STATICANALYZER_CHECKERS_MPICHECKER_MPIFUNCTIONCLASSIFIER_H +#define LLVM_CLANG_LIB_STATICANALYZER_CHECKERS_MPICHECKER_MPIFUNCTIONCLASSIFIER_H + +#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" + +namespace clang { +namespace ento { +namespace mpi { + +class MPIFunctionClassifier { +public: + MPIFunctionClassifier(ASTContext &ASTCtx) { identifierInit(ASTCtx); } + + // general identifiers + bool isMPIType(const IdentifierInfo *const IdentInfo) const; + bool isNonBlockingType(const IdentifierInfo *const IdentInfo) const; + + // point-to-point identifiers + bool isPointToPointType(const IdentifierInfo *const IdentInfo) const; + + // collective identifiers + bool isCollectiveType(const IdentifierInfo *const IdentInfo) const; + bool isCollToColl(const IdentifierInfo *const IdentInfo) const; + bool isScatterType(const IdentifierInfo *const IdentInfo) const; + bool isGatherType(const IdentifierInfo *const IdentInfo) const; + bool isAllgatherType(const IdentifierInfo *const IdentInfo) const; + bool isAlltoallType(const IdentifierInfo *const IdentInfo) const; + bool isReduceType(const IdentifierInfo *const IdentInfo) const; + bool isBcastType(const IdentifierInfo *const IdentInfo) const; + + // additional identifiers + bool isMPI_Wait(const IdentifierInfo *const IdentInfo) const; + bool isMPI_Waitall(const IdentifierInfo *const IdentInfo) const; + bool isWaitType(const IdentifierInfo *const IdentInfo) const; + +private: + // Initializes function identifiers, to recognize them during analysis. + void identifierInit(ASTContext &ASTCtx); + void initPointToPointIdentifiers(ASTContext &ASTCtx); + void initCollectiveIdentifiers(ASTContext &ASTCtx); + void initAdditionalIdentifiers(ASTContext &ASTCtx); + + // The containers are used, to enable classification of MPI-functions during + // analysis. + llvm::SmallVector<IdentifierInfo *, 12> MPINonBlockingTypes; + + llvm::SmallVector<IdentifierInfo *, 10> MPIPointToPointTypes; + llvm::SmallVector<IdentifierInfo *, 16> MPICollectiveTypes; + + llvm::SmallVector<IdentifierInfo *, 4> MPIPointToCollTypes; + llvm::SmallVector<IdentifierInfo *, 4> MPICollToPointTypes; + llvm::SmallVector<IdentifierInfo *, 6> MPICollToCollTypes; + + llvm::SmallVector<IdentifierInfo *, 32> MPIType; + + // point-to-point functions + IdentifierInfo *IdentInfo_MPI_Send = nullptr, *IdentInfo_MPI_Isend = nullptr, + *IdentInfo_MPI_Ssend = nullptr, *IdentInfo_MPI_Issend = nullptr, + *IdentInfo_MPI_Bsend = nullptr, *IdentInfo_MPI_Ibsend = nullptr, + *IdentInfo_MPI_Rsend = nullptr, *IdentInfo_MPI_Irsend = nullptr, + *IdentInfo_MPI_Recv = nullptr, *IdentInfo_MPI_Irecv = nullptr; + + // collective functions + IdentifierInfo *IdentInfo_MPI_Scatter = nullptr, + *IdentInfo_MPI_Iscatter = nullptr, *IdentInfo_MPI_Gather = nullptr, + *IdentInfo_MPI_Igather = nullptr, *IdentInfo_MPI_Allgather = nullptr, + *IdentInfo_MPI_Iallgather = nullptr, *IdentInfo_MPI_Bcast = nullptr, + *IdentInfo_MPI_Ibcast = nullptr, *IdentInfo_MPI_Reduce = nullptr, + *IdentInfo_MPI_Ireduce = nullptr, *IdentInfo_MPI_Allreduce = nullptr, + *IdentInfo_MPI_Iallreduce = nullptr, *IdentInfo_MPI_Alltoall = nullptr, + *IdentInfo_MPI_Ialltoall = nullptr, *IdentInfo_MPI_Barrier = nullptr; + + // additional functions + IdentifierInfo *IdentInfo_MPI_Comm_rank = nullptr, + *IdentInfo_MPI_Comm_size = nullptr, *IdentInfo_MPI_Wait = nullptr, + *IdentInfo_MPI_Waitall = nullptr; +}; + +} // end of namespace: mpi +} // end of namespace: ento +} // end of namespace: clang + +#endif diff --git a/lib/StaticAnalyzer/Checkers/MPI-Checker/MPITypes.h b/lib/StaticAnalyzer/Checkers/MPI-Checker/MPITypes.h new file mode 100644 index 0000000000000..27ec950d31eb9 --- /dev/null +++ b/lib/StaticAnalyzer/Checkers/MPI-Checker/MPITypes.h @@ -0,0 +1,68 @@ +//===-- MPITypes.h - Functionality to model MPI concepts --------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +/// +/// \file +/// This file provides definitions to model concepts of MPI. The mpi::Request +/// class defines a wrapper class, in order to make MPI requests trackable for +/// path-sensitive analysis. +/// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_LIB_STATICANALYZER_CHECKERS_MPICHECKER_MPITYPES_H +#define LLVM_CLANG_LIB_STATICANALYZER_CHECKERS_MPICHECKER_MPITYPES_H + +#include "MPIFunctionClassifier.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" +#include "llvm/ADT/SmallSet.h" + +namespace clang { +namespace ento { +namespace mpi { + +class Request { +public: + enum State : unsigned char { Nonblocking, Wait }; + + Request(State S) : CurrentState{S} {} + + void Profile(llvm::FoldingSetNodeID &Id) const { + Id.AddInteger(CurrentState); + } + + bool operator==(const Request &ToCompare) const { + return CurrentState == ToCompare.CurrentState; + } + + const State CurrentState; +}; + +// The RequestMap stores MPI requests which are identified by their memory +// region. Requests are used in MPI to complete nonblocking operations with wait +// operations. A custom map implementation is used, in order to make it +// available in an arbitrary amount of translation units. +struct RequestMap {}; +typedef llvm::ImmutableMap<const clang::ento::MemRegion *, + clang::ento::mpi::Request> + RequestMapImpl; + +} // end of namespace: mpi + + +template <> +struct ProgramStateTrait<mpi::RequestMap> + : public ProgramStatePartialTrait<mpi::RequestMapImpl> { + static void *GDMIndex() { + static int index = 0; + return &index; + } +}; + +} // end of namespace: ento +} // end of namespace: clang +#endif diff --git a/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp b/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp index 4cbe97b26075c..c038a2649e15d 100644 --- a/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp @@ -75,7 +75,7 @@ void MacOSXAPIChecker::CheckDispatchOnce(CheckerContext &C, const CallExpr *CE, // _dispatch_once is then a function which then calls the real dispatch_once. // Users do not care; they just want the warning at the top-level call. if (CE->getLocStart().isMacroID()) { - StringRef TrimmedFName = FName.ltrim("_"); + StringRef TrimmedFName = FName.ltrim('_'); if (TrimmedFName != FName) FName = TrimmedFName; } diff --git a/lib/StaticAnalyzer/Checkers/Makefile b/lib/StaticAnalyzer/Checkers/Makefile deleted file mode 100644 index 7c8f7bf1bc4ed..0000000000000 --- a/lib/StaticAnalyzer/Checkers/Makefile +++ /dev/null @@ -1,24 +0,0 @@ -##===- clang/lib/Checker/Makefile --------------------------*- Makefile -*-===## -# -# The LLVM Compiler Infrastructure -# -# This file is distributed under the University of Illinois Open Source -# License. See LICENSE.TXT for details. -# -##===----------------------------------------------------------------------===## -# -# This implements analyses built on top of source-level CFGs. -# -##===----------------------------------------------------------------------===## - -CLANG_LEVEL := ../../.. -LIBRARYNAME := clangStaticAnalyzerCheckers - -BUILT_SOURCES = Checkers.inc -TABLEGEN_INC_FILES_COMMON = 1 - -include $(CLANG_LEVEL)/Makefile - -$(ObjDir)/Checkers.inc.tmp : Checkers.td $(PROJ_SRC_DIR)/$(CLANG_LEVEL)/include/clang/StaticAnalyzer/Checkers/CheckerBase.td $(CLANG_TBLGEN) $(ObjDir)/.dir - $(Echo) "Building Clang SA Checkers tables with tblgen" - $(Verb) $(ClangTableGen) -gen-clang-sa-checkers -I $(PROJ_SRC_DIR)/$(CLANG_LEVEL)/include -o $(call SYSPATH, $@) $< diff --git a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp index fee030feb6d20..e06662b169342 100644 --- a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -31,6 +31,7 @@ #include "llvm/ADT/SmallString.h" #include "llvm/ADT/StringExtras.h" #include <climits> +#include <utility> using namespace clang; using namespace ento; @@ -169,11 +170,12 @@ class MallocChecker : public Checker<check::DeadSymbols, { public: MallocChecker() - : II_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_kmalloc(nullptr), II_if_nameindex(nullptr), - II_if_freenameindex(nullptr) {} + : 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) {} /// In pessimistic mode, the checker assumes that it does not know which /// functions might free the memory. @@ -231,10 +233,11 @@ 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_malloc, *II_free, *II_realloc, - *II_calloc, *II_valloc, *II_reallocf, *II_strndup, - *II_strdup, *II_kmalloc, *II_if_nameindex, - *II_if_freenameindex; + 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_if_nameindex, *II_if_freenameindex, *II_wcsdup, + *II_win_wcsdup; mutable Optional<uint64_t> KernelZeroFlagVal; void initIdentifierInfo(ASTContext &C) const; @@ -518,7 +521,7 @@ namespace { class StopTrackingCallback final : public SymbolVisitor { ProgramStateRef state; public: - StopTrackingCallback(ProgramStateRef st) : state(st) {} + StopTrackingCallback(ProgramStateRef st) : state(std::move(st)) {} ProgramStateRef getState() const { return state; } bool VisitSymbol(SymbolRef sym) override { @@ -540,9 +543,15 @@ void MallocChecker::initIdentifierInfo(ASTContext &Ctx) const { II_valloc = &Ctx.Idents.get("valloc"); II_strdup = &Ctx.Idents.get("strdup"); II_strndup = &Ctx.Idents.get("strndup"); + II_wcsdup = &Ctx.Idents.get("wcsdup"); II_kmalloc = &Ctx.Idents.get("kmalloc"); II_if_nameindex = &Ctx.Idents.get("if_nameindex"); II_if_freenameindex = &Ctx.Idents.get("if_freenameindex"); + + //MSVC uses `_`-prefixed instead, so we check for them too. + II_win_strdup = &Ctx.Idents.get("_strdup"); + II_win_wcsdup = &Ctx.Idents.get("_wcsdup"); + II_win_alloca = &Ctx.Idents.get("_alloca"); } bool MallocChecker::isMemFunction(const FunctionDecl *FD, ASTContext &C) const { @@ -585,7 +594,8 @@ bool MallocChecker::isCMemFunction(const FunctionDecl *FD, if (Family == AF_Malloc && CheckAlloc) { if (FunI == II_malloc || FunI == II_realloc || FunI == II_reallocf || FunI == II_calloc || FunI == II_valloc || FunI == II_strdup || - FunI == II_strndup || FunI == II_kmalloc) + FunI == II_win_strdup || FunI == II_strndup || FunI == II_wcsdup || + FunI == II_win_wcsdup || FunI == II_kmalloc) return true; } @@ -600,7 +610,7 @@ bool MallocChecker::isCMemFunction(const FunctionDecl *FD, } if (Family == AF_Alloca && CheckAlloc) { - if (FunI == II_alloca) + if (FunI == II_alloca || FunI == II_win_alloca) return true; } } @@ -789,11 +799,12 @@ void MallocChecker::checkPostStmt(const CallExpr *CE, CheckerContext &C) const { State = ProcessZeroAllocation(C, CE, 1, State); } else if (FunI == II_free) { State = FreeMemAux(C, CE, State, 0, false, ReleasedAllocatedMemory); - } else if (FunI == II_strdup) { + } else if (FunI == II_strdup || FunI == II_win_strdup || + FunI == II_wcsdup || FunI == II_win_wcsdup) { State = MallocUpdateRefState(C, CE, State); } else if (FunI == II_strndup) { State = MallocUpdateRefState(C, CE, State); - } else if (FunI == II_alloca) { + } else if (FunI == II_alloca || FunI == II_win_alloca) { State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State, AF_Alloca); State = ProcessZeroAllocation(C, CE, 0, State); @@ -933,7 +944,7 @@ static bool treatUnusedNewEscaped(const CXXNewExpr *NE) { const CXXConstructorDecl *CtorD = ConstructE->getConstructor(); // Iterate over the constructor parameters. - for (const auto *CtorParam : CtorD->params()) { + for (const auto *CtorParam : CtorD->parameters()) { QualType CtorParamPointeeT = CtorParam->getType()->getPointeeType(); if (CtorParamPointeeT.isNull()) diff --git a/lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp b/lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp index 99ba90d7a2d93..fc2ab1d6e3f7d 100644 --- a/lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp @@ -25,10 +25,10 @@ #include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h" #include "llvm/ADT/APSInt.h" #include "llvm/ADT/SmallVector.h" +#include <utility> using namespace clang; using namespace ento; -using llvm::APInt; using llvm::APSInt; namespace { @@ -38,7 +38,7 @@ struct MallocOverflowCheck { APSInt maxVal; MallocOverflowCheck(const BinaryOperator *m, const Expr *v, APSInt val) - : mulop(m), variable(v), maxVal(val) {} + : mulop(m), variable(v), maxVal(std::move(val)) {} }; class MallocOverflowSecurityChecker : public Checker<check::ASTCodeBody> { @@ -141,25 +141,25 @@ private: return false; } - const Decl *getDecl(const DeclRefExpr *DR) { return DR->getDecl(); } - - const Decl *getDecl(const MemberExpr *ME) { return ME->getMemberDecl(); } + static const Decl *getDecl(const DeclRefExpr *DR) { return DR->getDecl(); } + static const Decl *getDecl(const MemberExpr *ME) { + return ME->getMemberDecl(); + } template <typename T1> - void Erase(const T1 *DR, std::function<bool(theVecType::iterator)> pred) { - theVecType::iterator i = toScanFor.end(); - theVecType::iterator e = toScanFor.begin(); - while (i != e) { - --i; - if (const T1 *DR_i = dyn_cast<T1>(i->variable)) { - if ((getDecl(DR_i) == getDecl(DR)) && pred(i)) - i = toScanFor.erase(i); - } - } + void Erase(const T1 *DR, + llvm::function_ref<bool(const MallocOverflowCheck &)> Pred) { + auto P = [DR, Pred](const MallocOverflowCheck &Check) { + if (const auto *CheckDR = dyn_cast<T1>(Check.variable)) + return getDecl(CheckDR) == getDecl(DR) && Pred(Check); + return false; + }; + toScanFor.erase(std::remove_if(toScanFor.begin(), toScanFor.end(), P), + toScanFor.end()); } void CheckExpr(const Expr *E_p) { - auto PredTrue = [](theVecType::iterator) -> bool { return true; }; + auto PredTrue = [](const MallocOverflowCheck &) { return true; }; const Expr *E = E_p->IgnoreParenImpCasts(); if (const DeclRefExpr *DR = dyn_cast<DeclRefExpr>(E)) Erase<DeclRefExpr>(DR, PredTrue); @@ -210,9 +210,9 @@ private: const Expr *E = lhs->IgnoreParenImpCasts(); auto pred = [assignKnown, numeratorKnown, - denomExtVal](theVecType::iterator i) { + denomExtVal](const MallocOverflowCheck &Check) { return assignKnown || - (numeratorKnown && (denomExtVal >= i->maxVal.getExtValue())); + (numeratorKnown && (denomExtVal >= Check.maxVal.getExtValue())); }; if (const DeclRefExpr *DR = dyn_cast<DeclRefExpr>(E)) diff --git a/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp b/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp index dab068b27e80c..559c75d7a5b06 100644 --- a/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp @@ -61,7 +61,7 @@ void NSErrorMethodChecker::checkASTDecl(const ObjCMethodDecl *D, II = &D->getASTContext().Idents.get("NSError"); bool hasNSError = false; - for (const auto *I : D->params()) { + for (const auto *I : D->parameters()) { if (IsNSError(I->getType(), II)) { hasNSError = true; break; @@ -108,7 +108,7 @@ void CFErrorFunctionChecker::checkASTDecl(const FunctionDecl *D, II = &D->getASTContext().Idents.get("CFErrorRef"); bool hasCFError = false; - for (auto I : D->params()) { + for (auto I : D->parameters()) { if (IsCFError(I->getType(), II)) { hasCFError = true; break; diff --git a/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp b/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp index bb86ea401df50..d7ec6b10c6f76 100644 --- a/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp @@ -26,13 +26,16 @@ //===----------------------------------------------------------------------===// #include "ClangSACheckers.h" -#include "llvm/Support/Path.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/CallEvent.h" +#include "llvm/ADT/StringExtras.h" +#include "llvm/Support/Path.h" + using namespace clang; using namespace ento; @@ -89,18 +92,6 @@ enum class ErrorKind : int { NullablePassedToNonnull }; -const char *const ErrorMessages[] = { - "Null is assigned to a pointer which is expected to have non-null value", - "Null passed to a callee that requires a non-null argument", - "Null is returned from a function that is expected to return a non-null " - "value", - "Nullable pointer is assigned to a pointer which is expected to have " - "non-null value", - "Nullable pointer is returned from a function that is expected to return a " - "non-null value", - "Nullable pointer is dereferenced", - "Nullable pointer is passed to a callee that requires a non-null argument"}; - class NullabilityChecker : public Checker<check::Bind, check::PreCall, check::PreStmt<ReturnStmt>, check::PostCall, check::PostStmt<ExplicitCastExpr>, @@ -109,6 +100,14 @@ class NullabilityChecker mutable std::unique_ptr<BugType> BT; public: + // If true, the checker will not diagnose nullabilility issues for calls + // to system headers. This option is motivated by the observation that large + // projects may have many nullability warnings. These projects may + // find warnings about nullability annotations that they have explicitly + // added themselves higher priority to fix than warnings on calls to system + // libraries. + DefaultBool NoDiagnoseCallsToSystemHeaders; + void checkBind(SVal L, SVal V, const Stmt *S, CheckerContext &C) const; void checkPostStmt(const ExplicitCastExpr *CE, CheckerContext &C) const; void checkPreStmt(const ReturnStmt *S, CheckerContext &C) const; @@ -169,17 +168,19 @@ private: /// /// When \p SuppressPath is set to true, no more bugs will be reported on this /// path by this checker. - void reportBugIfPreconditionHolds(ErrorKind Error, ExplodedNode *N, - const MemRegion *Region, CheckerContext &C, - const Stmt *ValueExpr = nullptr, - bool SuppressPath = false) const; - - void reportBug(ErrorKind Error, ExplodedNode *N, const MemRegion *Region, - BugReporter &BR, const Stmt *ValueExpr = nullptr) const { + void reportBugIfInvariantHolds(StringRef Msg, ErrorKind Error, + ExplodedNode *N, const MemRegion *Region, + CheckerContext &C, + const Stmt *ValueExpr = nullptr, + bool SuppressPath = false) const; + + void reportBug(StringRef Msg, ErrorKind Error, ExplodedNode *N, + const MemRegion *Region, BugReporter &BR, + const Stmt *ValueExpr = nullptr) const { if (!BT) BT.reset(new BugType(this, "Nullability", "Memory error")); - const char *Msg = ErrorMessages[static_cast<int>(Error)]; - std::unique_ptr<BugReport> R(new BugReport(*BT, Msg, N)); + + auto R = llvm::make_unique<BugReport>(*BT, Msg, N); if (Region) { R->markInteresting(Region); R->addVisitor(llvm::make_unique<NullabilityBugVisitor>(Region)); @@ -198,6 +199,15 @@ private: /// to the wrapped region. Otherwise it will return a nullptr. const SymbolicRegion *getTrackRegion(SVal Val, bool CheckSuperRegion = false) const; + + /// Returns true if the call is diagnosable in the currrent analyzer + /// configuration. + bool isDiagnosableCall(const CallEvent &Call) const { + if (NoDiagnoseCallsToSystemHeaders && Call.isInSystemHeader()) + return false; + + return true; + } }; class NullabilityState { @@ -237,12 +247,31 @@ bool operator==(NullabilityState Lhs, NullabilityState Rhs) { REGISTER_MAP_WITH_PROGRAMSTATE(NullabilityMap, const MemRegion *, NullabilityState) -// If the nullability precondition of a function is violated, we should not -// report nullability related issues on that path. For this reason once a -// precondition is not met on a path, this checker will be esentially turned off -// for the rest of the analysis. We do not want to generate a sink node however, -// so this checker would not lead to reduced coverage. -REGISTER_TRAIT_WITH_PROGRAMSTATE(PreconditionViolated, bool) +// We say "the nullability type invariant is violated" when a location with a +// non-null type contains NULL or a function with a non-null return type returns +// NULL. Violations of the nullability type invariant can be detected either +// directly (for example, when NULL is passed as an argument to a nonnull +// parameter) or indirectly (for example, when, inside a function, the +// programmer defensively checks whether a nonnull parameter contains NULL and +// finds that it does). +// +// As a matter of policy, the nullability checker typically warns on direct +// violations of the nullability invariant (although it uses various +// heuristics to suppress warnings in some cases) but will not warn if the +// invariant has already been violated along the path (either directly or +// indirectly). As a practical matter, this prevents the analyzer from +// (1) warning on defensive code paths where a nullability precondition is +// determined to have been violated, (2) warning additional times after an +// initial direct violation has been discovered, and (3) warning after a direct +// violation that has been implicitly or explicitly suppressed (for +// example, with a cast of NULL to _Nonnull). In essence, once an invariant +// violation is detected on a path, this checker will be esentially turned off +// for the rest of the analysis +// +// The analyzer takes this approach (rather than generating a sink node) to +// ensure coverage of defensive paths, which may be important for backwards +// compatibility in codebases that were developed without nullability in mind. +REGISTER_TRAIT_WITH_PROGRAMSTATE(InvariantViolated, bool) enum class NullConstraint { IsNull, IsNotNull, Unknown }; @@ -327,38 +356,79 @@ static Nullability getNullabilityAnnotation(QualType Type) { return Nullability::Unspecified; } -template <typename ParamVarDeclRange> +/// Returns true when the value stored at the given location is null +/// and the passed in type is nonnnull. +static bool checkValueAtLValForInvariantViolation(ProgramStateRef State, + SVal LV, QualType T) { + if (getNullabilityAnnotation(T) != Nullability::Nonnull) + return false; + + auto RegionVal = LV.getAs<loc::MemRegionVal>(); + if (!RegionVal) + return false; + + auto StoredVal = + State->getSVal(RegionVal->getRegion()).getAs<DefinedOrUnknownSVal>(); + if (!StoredVal) + return false; + + if (getNullConstraint(*StoredVal, State) == NullConstraint::IsNull) + return true; + + return false; +} + static bool -checkParamsForPreconditionViolation(const ParamVarDeclRange &Params, +checkParamsForPreconditionViolation(ArrayRef<ParmVarDecl *> Params, ProgramStateRef State, const LocationContext *LocCtxt) { for (const auto *ParamDecl : Params) { if (ParamDecl->isParameterPack()) break; - if (getNullabilityAnnotation(ParamDecl->getType()) != Nullability::Nonnull) - continue; + SVal LV = State->getLValue(ParamDecl, LocCtxt); + if (checkValueAtLValForInvariantViolation(State, LV, + ParamDecl->getType())) { + return true; + } + } + return false; +} - auto RegVal = State->getLValue(ParamDecl, LocCtxt) - .template getAs<loc::MemRegionVal>(); - if (!RegVal) - continue; +static bool +checkSelfIvarsForInvariantViolation(ProgramStateRef State, + const LocationContext *LocCtxt) { + auto *MD = dyn_cast<ObjCMethodDecl>(LocCtxt->getDecl()); + if (!MD || !MD->isInstanceMethod()) + return false; - auto ParamValue = State->getSVal(RegVal->getRegion()) - .template getAs<DefinedOrUnknownSVal>(); - if (!ParamValue) - continue; + const ImplicitParamDecl *SelfDecl = LocCtxt->getSelfDecl(); + if (!SelfDecl) + return false; - if (getNullConstraint(*ParamValue, State) == NullConstraint::IsNull) { + SVal SelfVal = State->getSVal(State->getRegion(SelfDecl, LocCtxt)); + + const ObjCObjectPointerType *SelfType = + dyn_cast<ObjCObjectPointerType>(SelfDecl->getType()); + if (!SelfType) + return false; + + const ObjCInterfaceDecl *ID = SelfType->getInterfaceDecl(); + if (!ID) + return false; + + for (const auto *IvarDecl : ID->ivars()) { + SVal LV = State->getLValue(IvarDecl, SelfVal); + if (checkValueAtLValForInvariantViolation(State, LV, IvarDecl->getType())) { return true; } } return false; } -static bool checkPreconditionViolation(ProgramStateRef State, ExplodedNode *N, - CheckerContext &C) { - if (State->get<PreconditionViolated>()) +static bool checkInvariantViolation(ProgramStateRef State, ExplodedNode *N, + CheckerContext &C) { + if (State->get<InvariantViolated>()) return true; const LocationContext *LocCtxt = C.getLocationContext(); @@ -366,41 +436,38 @@ static bool checkPreconditionViolation(ProgramStateRef State, ExplodedNode *N, if (!D) return false; - if (const auto *BlockD = dyn_cast<BlockDecl>(D)) { - if (checkParamsForPreconditionViolation(BlockD->parameters(), State, - LocCtxt)) { - if (!N->isSink()) - C.addTransition(State->set<PreconditionViolated>(true), N); - return true; - } + ArrayRef<ParmVarDecl*> Params; + if (const auto *BD = dyn_cast<BlockDecl>(D)) + Params = BD->parameters(); + else if (const auto *FD = dyn_cast<FunctionDecl>(D)) + Params = FD->parameters(); + else if (const auto *MD = dyn_cast<ObjCMethodDecl>(D)) + Params = MD->parameters(); + else return false; - } - if (const auto *FuncDecl = dyn_cast<FunctionDecl>(D)) { - if (checkParamsForPreconditionViolation(FuncDecl->parameters(), State, - LocCtxt)) { - if (!N->isSink()) - C.addTransition(State->set<PreconditionViolated>(true), N); - return true; - } - return false; + if (checkParamsForPreconditionViolation(Params, State, LocCtxt) || + checkSelfIvarsForInvariantViolation(State, LocCtxt)) { + if (!N->isSink()) + C.addTransition(State->set<InvariantViolated>(true), N); + return true; } return false; } -void NullabilityChecker::reportBugIfPreconditionHolds( +void NullabilityChecker::reportBugIfInvariantHolds(StringRef Msg, ErrorKind Error, ExplodedNode *N, const MemRegion *Region, CheckerContext &C, const Stmt *ValueExpr, bool SuppressPath) const { ProgramStateRef OriginalState = N->getState(); - if (checkPreconditionViolation(OriginalState, N, C)) + if (checkInvariantViolation(OriginalState, N, C)) return; if (SuppressPath) { - OriginalState = OriginalState->set<PreconditionViolated>(true); + OriginalState = OriginalState->set<InvariantViolated>(true); N = C.addTransition(OriginalState, N); } - reportBug(Error, N, Region, C.getBugReporter(), ValueExpr); + reportBug(Msg, Error, N, Region, C.getBugReporter(), ValueExpr); } /// Cleaning up the program state. @@ -424,7 +491,7 @@ void NullabilityChecker::checkDeadSymbols(SymbolReaper &SR, // preconditions are violated. It is not enough to check this only when we // actually report an error, because at that time interesting symbols might be // reaped. - if (checkPreconditionViolation(State, C.getPredecessor(), C)) + if (checkInvariantViolation(State, C.getPredecessor(), C)) return; C.addTransition(State); } @@ -433,7 +500,7 @@ void NullabilityChecker::checkDeadSymbols(SymbolReaper &SR, /// not know anything about the value of that pointer. When that pointer is /// nullable, this code emits a warning. void NullabilityChecker::checkEvent(ImplicitNullDerefEvent Event) const { - if (Event.SinkNode->getState()->get<PreconditionViolated>()) + if (Event.SinkNode->getState()->get<InvariantViolated>()) return; const MemRegion *Region = @@ -454,18 +521,32 @@ void NullabilityChecker::checkEvent(ImplicitNullDerefEvent Event) const { // Do not suppress errors on defensive code paths, because dereferencing // a nullable pointer is always an error. if (Event.IsDirectDereference) - reportBug(ErrorKind::NullableDereferenced, Event.SinkNode, Region, BR); - else - reportBug(ErrorKind::NullablePassedToNonnull, Event.SinkNode, Region, BR); + reportBug("Nullable pointer is dereferenced", + ErrorKind::NullableDereferenced, Event.SinkNode, Region, BR); + else { + reportBug("Nullable pointer is passed to a callee that requires a " + "non-null", ErrorKind::NullablePassedToNonnull, + Event.SinkNode, Region, BR); + } } } +/// Find the outermost subexpression of E that is not an implicit cast. +/// This looks through the implicit casts to _Nonnull that ARC adds to +/// return expressions of ObjC types when the return type of the function or +/// method is non-null but the express is not. +static const Expr *lookThroughImplicitCasts(const Expr *E) { + assert(E); + + while (auto *ICE = dyn_cast<ImplicitCastExpr>(E)) { + E = ICE->getSubExpr(); + } + + return E; +} + /// This method check when nullable pointer or null value is returned from a /// function that has nonnull return type. -/// -/// TODO: when nullability preconditons are violated, it is ok to violate the -/// nullability postconditons (i.e.: when one of the nonnull parameters are null -/// this check should not report any nullability related issue). void NullabilityChecker::checkPreStmt(const ReturnStmt *S, CheckerContext &C) const { auto RetExpr = S->getRetValue(); @@ -476,7 +557,7 @@ void NullabilityChecker::checkPreStmt(const ReturnStmt *S, return; ProgramStateRef State = C.getState(); - if (State->get<PreconditionViolated>()) + if (State->get<InvariantViolated>()) return; auto RetSVal = @@ -484,16 +565,31 @@ void NullabilityChecker::checkPreStmt(const ReturnStmt *S, if (!RetSVal) return; + bool InSuppressedMethodFamily = false; + + QualType RequiredRetType; AnalysisDeclContext *DeclCtxt = C.getLocationContext()->getAnalysisDeclContext(); - const FunctionType *FuncType = DeclCtxt->getDecl()->getFunctionType(); - if (!FuncType) + const Decl *D = DeclCtxt->getDecl(); + if (auto *MD = dyn_cast<ObjCMethodDecl>(D)) { + // HACK: This is a big hammer to avoid warning when there are defensive + // nil checks in -init and -copy methods. We should add more sophisticated + // logic here to suppress on common defensive idioms but still + // warn when there is a likely problem. + ObjCMethodFamily Family = MD->getMethodFamily(); + if (OMF_init == Family || OMF_copy == Family || OMF_mutableCopy == Family) + InSuppressedMethodFamily = true; + + RequiredRetType = MD->getReturnType(); + } else if (auto *FD = dyn_cast<FunctionDecl>(D)) { + RequiredRetType = FD->getReturnType(); + } else { return; + } NullConstraint Nullness = getNullConstraint(*RetSVal, State); - Nullability RequiredNullability = - getNullabilityAnnotation(FuncType->getReturnType()); + Nullability RequiredNullability = getNullabilityAnnotation(RequiredRetType); // If the returned value is null but the type of the expression // generating it is nonnull then we will suppress the diagnostic. @@ -501,18 +597,36 @@ void NullabilityChecker::checkPreStmt(const ReturnStmt *S, // function with a _Nonnull return type: // return (NSString * _Nonnull)0; Nullability RetExprTypeLevelNullability = - getNullabilityAnnotation(RetExpr->getType()); + getNullabilityAnnotation(lookThroughImplicitCasts(RetExpr)->getType()); + bool NullReturnedFromNonNull = (RequiredNullability == Nullability::Nonnull && + Nullness == NullConstraint::IsNull); if (Filter.CheckNullReturnedFromNonnull && - Nullness == NullConstraint::IsNull && + NullReturnedFromNonNull && RetExprTypeLevelNullability != Nullability::Nonnull && - RequiredNullability == Nullability::Nonnull) { + !InSuppressedMethodFamily && + C.getLocationContext()->inTopFrame()) { static CheckerProgramPointTag Tag(this, "NullReturnedFromNonnull"); ExplodedNode *N = C.generateErrorNode(State, &Tag); if (!N) return; - reportBugIfPreconditionHolds(ErrorKind::NilReturnedToNonnull, N, nullptr, C, - RetExpr); + + SmallString<256> SBuf; + llvm::raw_svector_ostream OS(SBuf); + OS << "Null is returned from a " << C.getDeclDescription(D) << + " that is expected to return a non-null value"; + + reportBugIfInvariantHolds(OS.str(), + ErrorKind::NilReturnedToNonnull, N, nullptr, C, + RetExpr); + return; + } + + // If null was returned from a non-null function, mark the nullability + // invariant as violated even if the diagnostic was suppressed. + if (NullReturnedFromNonNull) { + State = State->set<InvariantViolated>(true); + C.addTransition(State); return; } @@ -530,8 +644,15 @@ void NullabilityChecker::checkPreStmt(const ReturnStmt *S, RequiredNullability == Nullability::Nonnull) { static CheckerProgramPointTag Tag(this, "NullableReturnedFromNonnull"); ExplodedNode *N = C.addTransition(State, C.getPredecessor(), &Tag); - reportBugIfPreconditionHolds(ErrorKind::NullableReturnedToNonnull, N, - Region, C); + + SmallString<256> SBuf; + llvm::raw_svector_ostream OS(SBuf); + OS << "Nullable pointer is returned from a " << C.getDeclDescription(D) << + " that is expected to return a non-null value"; + + reportBugIfInvariantHolds(OS.str(), + ErrorKind::NullableReturnedToNonnull, N, + Region, C); } return; } @@ -551,7 +672,7 @@ void NullabilityChecker::checkPreCall(const CallEvent &Call, return; ProgramStateRef State = C.getState(); - if (State->get<PreconditionViolated>()) + if (State->get<InvariantViolated>()) return; ProgramStateRef OrigState = State; @@ -579,14 +700,22 @@ void NullabilityChecker::checkPreCall(const CallEvent &Call, Nullability ArgExprTypeLevelNullability = getNullabilityAnnotation(ArgExpr->getType()); + unsigned ParamIdx = Param->getFunctionScopeIndex() + 1; + if (Filter.CheckNullPassedToNonnull && Nullness == NullConstraint::IsNull && ArgExprTypeLevelNullability != Nullability::Nonnull && - RequiredNullability == Nullability::Nonnull) { + RequiredNullability == Nullability::Nonnull && + isDiagnosableCall(Call)) { ExplodedNode *N = C.generateErrorNode(State); if (!N) return; - reportBugIfPreconditionHolds(ErrorKind::NilPassedToNonnull, N, nullptr, C, - ArgExpr); + SmallString<256> SBuf; + llvm::raw_svector_ostream OS(SBuf); + OS << "Null passed to a callee that requires a non-null " << ParamIdx + << llvm::getOrdinalSuffix(ParamIdx) << " parameter"; + reportBugIfInvariantHolds(OS.str(), ErrorKind::NilPassedToNonnull, N, + nullptr, C, + ArgExpr, /*SuppressPath=*/false); return; } @@ -603,17 +732,24 @@ void NullabilityChecker::checkPreCall(const CallEvent &Call, continue; if (Filter.CheckNullablePassedToNonnull && - RequiredNullability == Nullability::Nonnull) { + RequiredNullability == Nullability::Nonnull && + isDiagnosableCall(Call)) { ExplodedNode *N = C.addTransition(State); - reportBugIfPreconditionHolds(ErrorKind::NullablePassedToNonnull, N, - Region, C, ArgExpr, /*SuppressPath=*/true); + SmallString<256> SBuf; + llvm::raw_svector_ostream OS(SBuf); + OS << "Nullable pointer is passed to a callee that requires a non-null " + << ParamIdx << llvm::getOrdinalSuffix(ParamIdx) << " parameter"; + reportBugIfInvariantHolds(OS.str(), + ErrorKind::NullablePassedToNonnull, N, + Region, C, ArgExpr, /*SuppressPath=*/true); return; } if (Filter.CheckNullableDereferenced && Param->getType()->isReferenceType()) { ExplodedNode *N = C.addTransition(State); - reportBugIfPreconditionHolds(ErrorKind::NullableDereferenced, N, Region, - C, ArgExpr, /*SuppressPath=*/true); + reportBugIfInvariantHolds("Nullable pointer is dereferenced", + ErrorKind::NullableDereferenced, N, Region, + C, ArgExpr, /*SuppressPath=*/true); return; } continue; @@ -644,7 +780,7 @@ void NullabilityChecker::checkPostCall(const CallEvent &Call, if (!ReturnType->isAnyPointerType()) return; ProgramStateRef State = C.getState(); - if (State->get<PreconditionViolated>()) + if (State->get<InvariantViolated>()) return; const MemRegion *Region = getTrackRegion(Call.getReturnValue()); @@ -713,7 +849,7 @@ void NullabilityChecker::checkPostObjCMessage(const ObjCMethodCall &M, return; ProgramStateRef State = C.getState(); - if (State->get<PreconditionViolated>()) + if (State->get<InvariantViolated>()) return; const MemRegion *ReturnRegion = getTrackRegion(M.getReturnValue()); @@ -828,7 +964,7 @@ void NullabilityChecker::checkPostStmt(const ExplicitCastExpr *CE, return; ProgramStateRef State = C.getState(); - if (State->get<PreconditionViolated>()) + if (State->get<InvariantViolated>()) return; Nullability DestNullability = getNullabilityAnnotation(DestType); @@ -953,7 +1089,7 @@ void NullabilityChecker::checkBind(SVal L, SVal V, const Stmt *S, return; ProgramStateRef State = C.getState(); - if (State->get<PreconditionViolated>()) + if (State->get<InvariantViolated>()) return; auto ValDefOrUnknown = V.getAs<DefinedOrUnknownSVal>(); @@ -967,24 +1103,48 @@ void NullabilityChecker::checkBind(SVal L, SVal V, const Stmt *S, ValNullability = getNullabilityAnnotation(Sym->getType()); Nullability LocNullability = getNullabilityAnnotation(LocType); + + // If the type of the RHS expression is nonnull, don't warn. This + // enables explicit suppression with a cast to nonnull. + Nullability ValueExprTypeLevelNullability = Nullability::Unspecified; + const Expr *ValueExpr = matchValueExprForBind(S); + if (ValueExpr) { + ValueExprTypeLevelNullability = + getNullabilityAnnotation(lookThroughImplicitCasts(ValueExpr)->getType()); + } + + bool NullAssignedToNonNull = (LocNullability == Nullability::Nonnull && + RhsNullness == NullConstraint::IsNull); if (Filter.CheckNullPassedToNonnull && - RhsNullness == NullConstraint::IsNull && + NullAssignedToNonNull && ValNullability != Nullability::Nonnull && - LocNullability == Nullability::Nonnull && + ValueExprTypeLevelNullability != Nullability::Nonnull && !isARCNilInitializedLocal(C, S)) { static CheckerProgramPointTag Tag(this, "NullPassedToNonnull"); ExplodedNode *N = C.generateErrorNode(State, &Tag); if (!N) return; - const Stmt *ValueExpr = matchValueExprForBind(S); - if (!ValueExpr) - ValueExpr = S; - reportBugIfPreconditionHolds(ErrorKind::NilAssignedToNonnull, N, nullptr, C, - ValueExpr); + const Stmt *ValueStmt = S; + if (ValueExpr) + ValueStmt = ValueExpr; + + reportBugIfInvariantHolds("Null is assigned to a pointer which is " + "expected to have non-null value", + ErrorKind::NilAssignedToNonnull, N, nullptr, C, + ValueStmt); return; } + + // If null was returned from a non-null function, mark the nullability + // invariant as violated even if the diagnostic was suppressed. + if (NullAssignedToNonNull) { + State = State->set<InvariantViolated>(true); + C.addTransition(State); + return; + } + // Intentionally missing case: '0' is bound to a reference. It is handled by // the DereferenceChecker. @@ -1003,8 +1163,10 @@ void NullabilityChecker::checkBind(SVal L, SVal V, const Stmt *S, LocNullability == Nullability::Nonnull) { static CheckerProgramPointTag Tag(this, "NullablePassedToNonnull"); ExplodedNode *N = C.addTransition(State, C.getPredecessor(), &Tag); - reportBugIfPreconditionHolds(ErrorKind::NullableAssignedToNonnull, N, - ValueRegion, C); + reportBugIfInvariantHolds("Nullable pointer is assigned to a pointer " + "which is expected to have non-null value", + ErrorKind::NullableAssignedToNonnull, N, + ValueRegion, C); } return; } @@ -1052,6 +1214,10 @@ void NullabilityChecker::printState(raw_ostream &Out, ProgramStateRef State, checker->Filter.Check##name = true; \ checker->Filter.CheckName##name = mgr.getCurrentCheckName(); \ checker->NeedTracking = checker->NeedTracking || trackingRequired; \ + checker->NoDiagnoseCallsToSystemHeaders = \ + checker->NoDiagnoseCallsToSystemHeaders || \ + mgr.getAnalyzerOptions().getBooleanOption( \ + "NoDiagnoseCallsToSystemHeaders", false, checker, true); \ } // The checks are likely to be turned on by default and it is possible to do diff --git a/lib/StaticAnalyzer/Checkers/ObjCContainersChecker.cpp b/lib/StaticAnalyzer/Checkers/ObjCContainersChecker.cpp index 0203d79cd00e4..58ebf72660b6b 100644 --- a/lib/StaticAnalyzer/Checkers/ObjCContainersChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/ObjCContainersChecker.cpp @@ -79,7 +79,6 @@ void ObjCContainersChecker::addSizeInfo(const Expr *Array, const Expr *Size, C.addTransition( State->set<ArraySizeMap>(ArraySym, SizeV.castAs<DefinedSVal>())); - return; } void ObjCContainersChecker::checkPostStmt(const CallExpr *CE, @@ -156,10 +155,7 @@ ObjCContainersChecker::checkPointerEscape(ProgramStateRef State, const InvalidatedSymbols &Escaped, const CallEvent *Call, PointerEscapeKind Kind) const { - for (InvalidatedSymbols::const_iterator I = Escaped.begin(), - E = Escaped.end(); - I != E; ++I) { - SymbolRef Sym = *I; + for (const auto &Sym : Escaped) { // When a symbol for a mutable array escapes, we can't reason precisely // about its size any more -- so remove it from the map. // Note that we aren't notified here when a CFMutableArrayRef escapes as a @@ -169,6 +165,7 @@ ObjCContainersChecker::checkPointerEscape(ProgramStateRef State, } return State; } + /// Register checker. void ento::registerObjCContainersChecker(CheckerManager &mgr) { mgr.registerChecker<ObjCContainersChecker>(); diff --git a/lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp b/lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp new file mode 100644 index 0000000000000..15980c5c53870 --- /dev/null +++ b/lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp @@ -0,0 +1,294 @@ +//===- ObjCSuperDeallocChecker.cpp - Check correct use of [super dealloc] -===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +// +// This defines ObjCSuperDeallocChecker, a builtin check that warns when +// self is used after a call to [super dealloc] in MRR mode. +// +//===----------------------------------------------------------------------===// + +#include "ClangSACheckers.h" +#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" +#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/ProgramState.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h" + +using namespace clang; +using namespace ento; + +namespace { +class ObjCSuperDeallocChecker + : public Checker<check::PostObjCMessage, check::PreObjCMessage, + check::PreCall, check::Location> { + + mutable IdentifierInfo *IIdealloc, *IINSObject; + mutable Selector SELdealloc; + + std::unique_ptr<BugType> DoubleSuperDeallocBugType; + + void initIdentifierInfoAndSelectors(ASTContext &Ctx) const; + + bool isSuperDeallocMessage(const ObjCMethodCall &M) const; + +public: + ObjCSuperDeallocChecker(); + void checkPostObjCMessage(const ObjCMethodCall &M, CheckerContext &C) const; + void checkPreObjCMessage(const ObjCMethodCall &M, CheckerContext &C) const; + + void checkPreCall(const CallEvent &Call, CheckerContext &C) const; + + void checkLocation(SVal l, bool isLoad, const Stmt *S, + CheckerContext &C) const; + +private: + + void diagnoseCallArguments(const CallEvent &CE, CheckerContext &C) const; + + void reportUseAfterDealloc(SymbolRef Sym, StringRef Desc, const Stmt *S, + CheckerContext &C) const; +}; + +} // End anonymous namespace. + +// Remember whether [super dealloc] has previously been called on the +// SymbolRef for the receiver. +REGISTER_SET_WITH_PROGRAMSTATE(CalledSuperDealloc, SymbolRef) + +namespace { +class SuperDeallocBRVisitor final + : public BugReporterVisitorImpl<SuperDeallocBRVisitor> { + + SymbolRef ReceiverSymbol; + bool Satisfied; + +public: + SuperDeallocBRVisitor(SymbolRef ReceiverSymbol) + : ReceiverSymbol(ReceiverSymbol), + Satisfied(false) {} + + PathDiagnosticPiece *VisitNode(const ExplodedNode *Succ, + const ExplodedNode *Pred, + BugReporterContext &BRC, + BugReport &BR) override; + + void Profile(llvm::FoldingSetNodeID &ID) const override { + ID.Add(ReceiverSymbol); + } +}; +} // End anonymous namespace. + +void ObjCSuperDeallocChecker::checkPreObjCMessage(const ObjCMethodCall &M, + CheckerContext &C) const { + + ProgramStateRef State = C.getState(); + SymbolRef ReceiverSymbol = M.getReceiverSVal().getAsSymbol(); + if (!ReceiverSymbol) { + diagnoseCallArguments(M, C); + return; + } + + bool AlreadyCalled = State->contains<CalledSuperDealloc>(ReceiverSymbol); + if (!AlreadyCalled) + return; + + StringRef Desc; + + if (isSuperDeallocMessage(M)) { + Desc = "[super dealloc] should not be called multiple times"; + } else { + Desc = StringRef(); + } + + reportUseAfterDealloc(ReceiverSymbol, Desc, M.getOriginExpr(), C); + + return; +} + +void ObjCSuperDeallocChecker::checkPreCall(const CallEvent &Call, + CheckerContext &C) const { + diagnoseCallArguments(Call, C); +} + +void ObjCSuperDeallocChecker::checkPostObjCMessage(const ObjCMethodCall &M, + CheckerContext &C) const { + // Check for [super dealloc] method call. + if (!isSuperDeallocMessage(M)) + return; + + ProgramStateRef State = C.getState(); + SymbolRef ReceiverSymbol = M.getSelfSVal().getAsSymbol(); + assert(ReceiverSymbol && "No receiver symbol at call to [super dealloc]?"); + + // We add this transition in checkPostObjCMessage to avoid warning when + // we inline a call to [super dealloc] where the inlined call itself + // calls [super dealloc]. + State = State->add<CalledSuperDealloc>(ReceiverSymbol); + C.addTransition(State); +} + +void ObjCSuperDeallocChecker::checkLocation(SVal L, bool IsLoad, const Stmt *S, + CheckerContext &C) const { + SymbolRef BaseSym = L.getLocSymbolInBase(); + if (!BaseSym) + return; + + ProgramStateRef State = C.getState(); + + if (!State->contains<CalledSuperDealloc>(BaseSym)) + return; + + const MemRegion *R = L.getAsRegion(); + if (!R) + return; + + // Climb the super regions to find the base symbol while recording + // the second-to-last region for error reporting. + const MemRegion *PriorSubRegion = nullptr; + while (const SubRegion *SR = dyn_cast<SubRegion>(R)) { + if (const SymbolicRegion *SymR = dyn_cast<SymbolicRegion>(SR)) { + BaseSym = SymR->getSymbol(); + break; + } else { + R = SR->getSuperRegion(); + PriorSubRegion = SR; + } + } + + StringRef Desc = StringRef(); + auto *IvarRegion = dyn_cast_or_null<ObjCIvarRegion>(PriorSubRegion); + + std::string Buf; + llvm::raw_string_ostream OS(Buf); + if (IvarRegion) { + OS << "Use of instance variable '" << *IvarRegion->getDecl() << + "' after 'self' has been deallocated"; + Desc = OS.str(); + } + + reportUseAfterDealloc(BaseSym, Desc, S, C); +} + +/// Report a use-after-dealloc on Sym. If not empty, +/// Desc will be used to describe the error; otherwise, +/// a default warning will be used. +void ObjCSuperDeallocChecker::reportUseAfterDealloc(SymbolRef Sym, + StringRef Desc, + const Stmt *S, + CheckerContext &C) const { + // We have a use of self after free. + // This likely causes a crash, so stop exploring the + // path by generating a sink. + ExplodedNode *ErrNode = C.generateErrorNode(); + // If we've already reached this node on another path, return. + if (!ErrNode) + return; + + if (Desc.empty()) + Desc = "use of 'self' after it has been deallocated"; + + // Generate the report. + std::unique_ptr<BugReport> BR( + new BugReport(*DoubleSuperDeallocBugType, Desc, ErrNode)); + BR->addRange(S->getSourceRange()); + BR->addVisitor(llvm::make_unique<SuperDeallocBRVisitor>(Sym)); + C.emitReport(std::move(BR)); +} + +/// Diagnose if any of the arguments to CE have already been +/// dealloc'd. +void ObjCSuperDeallocChecker::diagnoseCallArguments(const CallEvent &CE, + CheckerContext &C) const { + ProgramStateRef State = C.getState(); + unsigned ArgCount = CE.getNumArgs(); + for (unsigned I = 0; I < ArgCount; I++) { + SymbolRef Sym = CE.getArgSVal(I).getAsSymbol(); + if (!Sym) + continue; + + if (State->contains<CalledSuperDealloc>(Sym)) { + reportUseAfterDealloc(Sym, StringRef(), CE.getArgExpr(I), C); + return; + } + } +} + +ObjCSuperDeallocChecker::ObjCSuperDeallocChecker() + : IIdealloc(nullptr), IINSObject(nullptr) { + + DoubleSuperDeallocBugType.reset( + new BugType(this, "[super dealloc] should not be called more than once", + categories::CoreFoundationObjectiveC)); +} + +void +ObjCSuperDeallocChecker::initIdentifierInfoAndSelectors(ASTContext &Ctx) const { + if (IIdealloc) + return; + + IIdealloc = &Ctx.Idents.get("dealloc"); + IINSObject = &Ctx.Idents.get("NSObject"); + + SELdealloc = Ctx.Selectors.getSelector(0, &IIdealloc); +} + +bool +ObjCSuperDeallocChecker::isSuperDeallocMessage(const ObjCMethodCall &M) const { + if (M.getOriginExpr()->getReceiverKind() != ObjCMessageExpr::SuperInstance) + return false; + + ASTContext &Ctx = M.getState()->getStateManager().getContext(); + initIdentifierInfoAndSelectors(Ctx); + + return M.getSelector() == SELdealloc; +} + +PathDiagnosticPiece *SuperDeallocBRVisitor::VisitNode(const ExplodedNode *Succ, + const ExplodedNode *Pred, + BugReporterContext &BRC, + BugReport &BR) { + if (Satisfied) + return nullptr; + + ProgramStateRef State = Succ->getState(); + + bool CalledNow = + Succ->getState()->contains<CalledSuperDealloc>(ReceiverSymbol); + bool CalledBefore = + Pred->getState()->contains<CalledSuperDealloc>(ReceiverSymbol); + + // Is Succ the node on which the analyzer noted that [super dealloc] was + // called on ReceiverSymbol? + if (CalledNow && !CalledBefore) { + Satisfied = true; + + ProgramPoint P = Succ->getLocation(); + PathDiagnosticLocation L = + PathDiagnosticLocation::create(P, BRC.getSourceManager()); + + if (!L.isValid() || !L.asLocation().isValid()) + return nullptr; + + return new PathDiagnosticEventPiece( + L, "[super dealloc] called here"); + } + + return nullptr; +} + +//===----------------------------------------------------------------------===// +// Checker Registration. +//===----------------------------------------------------------------------===// + +void ento::registerObjCSuperDeallocChecker(CheckerManager &Mgr) { + const LangOptions &LangOpts = Mgr.getLangOpts(); + if (LangOpts.getGC() == LangOptions::GCOnly || LangOpts.ObjCAutoRefCount) + return; + Mgr.registerChecker<ObjCSuperDeallocChecker>(); +} diff --git a/lib/StaticAnalyzer/Checkers/PaddingChecker.cpp b/lib/StaticAnalyzer/Checkers/PaddingChecker.cpp index 8ce37357fe1fe..0640d2f49f43a 100644 --- a/lib/StaticAnalyzer/Checkers/PaddingChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/PaddingChecker.cpp @@ -168,7 +168,7 @@ public: const ASTRecordLayout &RL) { CharUnits PaddingSum; CharUnits Offset = ASTContext.toCharUnitsFromBits(RL.getFieldOffset(0)); - for (const auto &FD : RD->fields()) { + for (const FieldDecl *FD : RD->fields()) { // This checker only cares about the padded size of the // field, and not the data size. If the field is a record // with tail padding, then we won't put that number in our @@ -260,13 +260,13 @@ public: // We are poorly aligned, and we need to pad in order to layout another // field. Round up to at least the smallest field alignment that we // currently have. - CharUnits NextOffset = NewOffset.RoundUpToAlignment(Fields[0].Align); + CharUnits NextOffset = NewOffset.alignTo(Fields[0].Align); NewPad += NextOffset - NewOffset; NewOffset = NextOffset; } } // Calculate tail padding. - CharUnits NewSize = NewOffset.RoundUpToAlignment(RL.getAlignment()); + CharUnits NewSize = NewOffset.alignTo(RL.getAlignment()); NewPad += NewSize - NewOffset; return NewPad; } diff --git a/lib/StaticAnalyzer/Checkers/PointerArithChecker.cpp b/lib/StaticAnalyzer/Checkers/PointerArithChecker.cpp index e3369677af72d..df5118806bff4 100644 --- a/lib/StaticAnalyzer/Checkers/PointerArithChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/PointerArithChecker.cpp @@ -13,55 +13,329 @@ //===----------------------------------------------------------------------===// #include "ClangSACheckers.h" +#include "clang/AST/DeclCXX.h" +#include "clang/AST/ExprCXX.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 "llvm/ADT/SmallVector.h" using namespace clang; using namespace ento; namespace { +enum class AllocKind { + SingleObject, + Array, + Unknown, + Reinterpreted // Single object interpreted as an array. +}; +} // end namespace + +namespace llvm { +template <> struct FoldingSetTrait<AllocKind> { + static inline void Profile(AllocKind X, FoldingSetNodeID &ID) { + ID.AddInteger(static_cast<int>(X)); + } +}; +} // end namespace llvm + +namespace { class PointerArithChecker - : public Checker< check::PreStmt<BinaryOperator> > { - mutable std::unique_ptr<BuiltinBug> BT; + : public Checker< + check::PreStmt<BinaryOperator>, check::PreStmt<UnaryOperator>, + check::PreStmt<ArraySubscriptExpr>, check::PreStmt<CastExpr>, + check::PostStmt<CastExpr>, check::PostStmt<CXXNewExpr>, + check::PostStmt<CallExpr>, check::DeadSymbols> { + AllocKind getKindOfNewOp(const CXXNewExpr *NE, const FunctionDecl *FD) const; + const MemRegion *getArrayRegion(const MemRegion *Region, bool &Polymorphic, + AllocKind &AKind, CheckerContext &C) const; + const MemRegion *getPointedRegion(const MemRegion *Region, + CheckerContext &C) const; + void reportPointerArithMisuse(const Expr *E, CheckerContext &C, + bool PointedNeeded = false) const; + void initAllocIdentifiers(ASTContext &C) const; + + mutable std::unique_ptr<BuiltinBug> BT_pointerArith; + mutable std::unique_ptr<BuiltinBug> BT_polyArray; + mutable llvm::SmallSet<IdentifierInfo *, 8> AllocFunctions; public: - void checkPreStmt(const BinaryOperator *B, CheckerContext &C) const; + void checkPreStmt(const UnaryOperator *UOp, CheckerContext &C) const; + void checkPreStmt(const BinaryOperator *BOp, CheckerContext &C) const; + void checkPreStmt(const ArraySubscriptExpr *SubExpr, CheckerContext &C) const; + void checkPreStmt(const CastExpr *CE, CheckerContext &C) const; + void checkPostStmt(const CastExpr *CE, CheckerContext &C) const; + void checkPostStmt(const CXXNewExpr *NE, CheckerContext &C) const; + void checkPostStmt(const CallExpr *CE, CheckerContext &C) const; + void checkDeadSymbols(SymbolReaper &SR, CheckerContext &C) const; }; +} // end namespace + +REGISTER_MAP_WITH_PROGRAMSTATE(RegionState, const MemRegion *, AllocKind) + +void PointerArithChecker::checkDeadSymbols(SymbolReaper &SR, + CheckerContext &C) const { + // TODO: intentional leak. Some information is garbage collected too early, + // see http://reviews.llvm.org/D14203 for further information. + /*ProgramStateRef State = C.getState(); + RegionStateTy RegionStates = State->get<RegionState>(); + for (RegionStateTy::iterator I = RegionStates.begin(), E = RegionStates.end(); + I != E; ++I) { + if (!SR.isLiveRegion(I->first)) + State = State->remove<RegionState>(I->first); + } + C.addTransition(State);*/ } -void PointerArithChecker::checkPreStmt(const BinaryOperator *B, - CheckerContext &C) const { - if (B->getOpcode() != BO_Sub && B->getOpcode() != BO_Add) - return; +AllocKind PointerArithChecker::getKindOfNewOp(const CXXNewExpr *NE, + const FunctionDecl *FD) const { + // This checker try not to assume anything about placement and overloaded + // new to avoid false positives. + if (isa<CXXMethodDecl>(FD)) + return AllocKind::Unknown; + if (FD->getNumParams() != 1 || FD->isVariadic()) + return AllocKind::Unknown; + if (NE->isArray()) + return AllocKind::Array; + + return AllocKind::SingleObject; +} + +const MemRegion * +PointerArithChecker::getPointedRegion(const MemRegion *Region, + CheckerContext &C) const { + assert(Region); + ProgramStateRef State = C.getState(); + SVal S = State->getSVal(Region); + return S.getAsRegion(); +} - ProgramStateRef state = C.getState(); - const LocationContext *LCtx = C.getLocationContext(); - SVal LV = state->getSVal(B->getLHS(), LCtx); - SVal RV = state->getSVal(B->getRHS(), LCtx); +/// Checks whether a region is the part of an array. +/// In case there is a dericed to base cast above the array element, the +/// Polymorphic output value is set to true. AKind output value is set to the +/// allocation kind of the inspected region. +const MemRegion *PointerArithChecker::getArrayRegion(const MemRegion *Region, + bool &Polymorphic, + AllocKind &AKind, + CheckerContext &C) const { + assert(Region); + while (Region->getKind() == MemRegion::Kind::CXXBaseObjectRegionKind) { + Region = Region->getAs<CXXBaseObjectRegion>()->getSuperRegion(); + Polymorphic = true; + } + if (Region->getKind() == MemRegion::Kind::ElementRegionKind) { + Region = Region->getAs<ElementRegion>()->getSuperRegion(); + } - const MemRegion *LR = LV.getAsRegion(); + ProgramStateRef State = C.getState(); + if (const AllocKind *Kind = State->get<RegionState>(Region)) { + AKind = *Kind; + if (*Kind == AllocKind::Array) + return Region; + else + return nullptr; + } + // 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) + return Region; - if (!LR || !RV.isConstant()) + // No AllocKind stored and not symbolic, assume that it points to a single + // object. + return nullptr; +} + +void PointerArithChecker::reportPointerArithMisuse(const Expr *E, + CheckerContext &C, + bool PointedNeeded) const { + SourceRange SR = E->getSourceRange(); + if (SR.isInvalid()) return; - // If pointer arithmetic is done on variables of non-array type, this often - // means behavior rely on memory organization, which is dangerous. - if (isa<VarRegion>(LR) || isa<CodeTextRegion>(LR) || - isa<CompoundLiteralRegion>(LR)) { + ProgramStateRef State = C.getState(); + const MemRegion *Region = + State->getSVal(E, C.getLocationContext()).getAsRegion(); + if (!Region) + return; + if (PointedNeeded) + Region = getPointedRegion(Region, C); + if (!Region) + return; + bool IsPolymorphic = false; + AllocKind Kind = AllocKind::Unknown; + if (const MemRegion *ArrayRegion = + getArrayRegion(Region, IsPolymorphic, Kind, C)) { + if (!IsPolymorphic) + return; if (ExplodedNode *N = C.generateNonFatalErrorNode()) { - if (!BT) - BT.reset( - new BuiltinBug(this, "Dangerous pointer arithmetic", - "Pointer arithmetic done on non-array variables " - "means reliance on memory layout, which is " - "dangerous.")); - auto R = llvm::make_unique<BugReport>(*BT, BT->getDescription(), N); - R->addRange(B->getSourceRange()); + if (!BT_polyArray) + BT_polyArray.reset(new BuiltinBug( + 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); + R->addRange(E->getSourceRange()); + R->markInteresting(ArrayRegion); C.emitReport(std::move(R)); } + return; + } + + if (Kind == AllocKind::Reinterpreted) + return; + + // We might not have enough information about symbolic regions. + if (Kind != AllocKind::SingleObject && + Region->getKind() == MemRegion::Kind::SymbolicRegionKind) + return; + + if (ExplodedNode *N = C.generateNonFatalErrorNode()) { + if (!BT_pointerArith) + BT_pointerArith.reset(new BuiltinBug(this, "Dangerous pointer arithmetic", + "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); + R->addRange(SR); + R->markInteresting(Region); + C.emitReport(std::move(R)); + } +} + +void PointerArithChecker::initAllocIdentifiers(ASTContext &C) const { + if (!AllocFunctions.empty()) + return; + AllocFunctions.insert(&C.Idents.get("alloca")); + AllocFunctions.insert(&C.Idents.get("malloc")); + AllocFunctions.insert(&C.Idents.get("realloc")); + AllocFunctions.insert(&C.Idents.get("calloc")); + AllocFunctions.insert(&C.Idents.get("valloc")); +} + +void PointerArithChecker::checkPostStmt(const CallExpr *CE, + CheckerContext &C) const { + ProgramStateRef State = C.getState(); + const FunctionDecl *FD = C.getCalleeDecl(CE); + if (!FD) + return; + IdentifierInfo *FunI = FD->getIdentifier(); + initAllocIdentifiers(C.getASTContext()); + if (AllocFunctions.count(FunI) == 0) + return; + + SVal SV = State->getSVal(CE, C.getLocationContext()); + const MemRegion *Region = SV.getAsRegion(); + if (!Region) + return; + // Assume that C allocation functions allocate arrays to avoid false + // positives. + // TODO: Add heuristics to distinguish alloc calls that allocates single + // objecs. + State = State->set<RegionState>(Region, AllocKind::Array); + C.addTransition(State); +} + +void PointerArithChecker::checkPostStmt(const CXXNewExpr *NE, + CheckerContext &C) const { + const FunctionDecl *FD = NE->getOperatorNew(); + if (!FD) + return; + + AllocKind Kind = getKindOfNewOp(NE, FD); + + ProgramStateRef State = C.getState(); + SVal AllocedVal = State->getSVal(NE, C.getLocationContext()); + const MemRegion *Region = AllocedVal.getAsRegion(); + if (!Region) + return; + State = State->set<RegionState>(Region, Kind); + C.addTransition(State); +} + +void PointerArithChecker::checkPostStmt(const CastExpr *CE, + CheckerContext &C) const { + if (CE->getCastKind() != CastKind::CK_BitCast) + return; + + const Expr *CastedExpr = CE->getSubExpr(); + ProgramStateRef State = C.getState(); + SVal CastedVal = State->getSVal(CastedExpr, C.getLocationContext()); + + const MemRegion *Region = CastedVal.getAsRegion(); + if (!Region) + return; + + // Suppress reinterpret casted hits. + State = State->set<RegionState>(Region, AllocKind::Reinterpreted); + C.addTransition(State); +} + +void PointerArithChecker::checkPreStmt(const CastExpr *CE, + CheckerContext &C) const { + if (CE->getCastKind() != CastKind::CK_ArrayToPointerDecay) + return; + + const Expr *CastedExpr = CE->getSubExpr(); + ProgramStateRef State = C.getState(); + SVal CastedVal = State->getSVal(CastedExpr, C.getLocationContext()); + + const MemRegion *Region = CastedVal.getAsRegion(); + if (!Region) + return; + + if (const AllocKind *Kind = State->get<RegionState>(Region)) { + if (*Kind == AllocKind::Array || *Kind == AllocKind::Reinterpreted) + return; + } + State = State->set<RegionState>(Region, AllocKind::Array); + C.addTransition(State); +} + +void PointerArithChecker::checkPreStmt(const UnaryOperator *UOp, + CheckerContext &C) const { + if (!UOp->isIncrementDecrementOp() || !UOp->getType()->isPointerType()) + return; + reportPointerArithMisuse(UOp->getSubExpr(), C, true); +} + +void PointerArithChecker::checkPreStmt(const ArraySubscriptExpr *SubsExpr, + CheckerContext &C) const { + ProgramStateRef State = C.getState(); + SVal Idx = State->getSVal(SubsExpr->getIdx(), C.getLocationContext()); + + // Indexing with 0 is OK. + if (Idx.isZeroConstant()) + return; + reportPointerArithMisuse(SubsExpr->getBase(), C); +} + +void PointerArithChecker::checkPreStmt(const BinaryOperator *BOp, + CheckerContext &C) const { + BinaryOperatorKind OpKind = BOp->getOpcode(); + if (!BOp->isAdditiveOp() && OpKind != BO_AddAssign && OpKind != BO_SubAssign) + return; + + const Expr *Lhs = BOp->getLHS(); + const Expr *Rhs = BOp->getRHS(); + ProgramStateRef State = C.getState(); + + if (Rhs->getType()->isIntegerType() && Lhs->getType()->isPointerType()) { + SVal RHSVal = State->getSVal(Rhs, C.getLocationContext()); + if (State->isNull(RHSVal).isConstrainedTrue()) + return; + reportPointerArithMisuse(Lhs, C, !BOp->isAdditiveOp()); + } + // The int += ptr; case is not valid C++. + if (Lhs->getType()->isIntegerType() && Rhs->getType()->isPointerType()) { + SVal LHSVal = State->getSVal(Lhs, C.getLocationContext()); + if (State->isNull(LHSVal).isConstrainedTrue()) + return; + reportPointerArithMisuse(Rhs, C); } } diff --git a/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp b/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp index f983c30856351..b646127cfae7d 100644 --- a/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp @@ -12,8 +12,8 @@ // //===----------------------------------------------------------------------===// -#include "ClangSACheckers.h" #include "AllocationDiagnostics.h" +#include "ClangSACheckers.h" #include "SelectorExtras.h" #include "clang/AST/Attr.h" #include "clang/AST/DeclCXX.h" @@ -39,6 +39,7 @@ #include "llvm/ADT/SmallString.h" #include "llvm/ADT/StringExtras.h" #include <cstdarg> +#include <utility> using namespace clang; using namespace ento; @@ -2683,7 +2684,7 @@ namespace { class StopTrackingCallback final : public SymbolVisitor { ProgramStateRef state; public: - StopTrackingCallback(ProgramStateRef st) : state(st) {} + StopTrackingCallback(ProgramStateRef st) : state(std::move(st)) {} ProgramStateRef getState() const { return state; } bool VisitSymbol(SymbolRef sym) override { @@ -2832,14 +2833,6 @@ void RetainCountChecker::checkPostStmt(const ObjCBoxedExpr *Ex, C.addTransition(State); } -static bool wasLoadedFromIvar(SymbolRef Sym) { - if (auto DerivedVal = dyn_cast<SymbolDerived>(Sym)) - return isa<ObjCIvarRegion>(DerivedVal->getRegion()); - if (auto RegionVal = dyn_cast<SymbolRegionValue>(Sym)) - return isa<ObjCIvarRegion>(RegionVal->getRegion()); - return false; -} - void RetainCountChecker::checkPostStmt(const ObjCIvarRefExpr *IRE, CheckerContext &C) const { Optional<Loc> IVarLoc = C.getSVal(IRE).getAs<Loc>(); @@ -2848,7 +2841,7 @@ void RetainCountChecker::checkPostStmt(const ObjCIvarRefExpr *IRE, ProgramStateRef State = C.getState(); SymbolRef Sym = State->getSVal(*IVarLoc).getAsSymbol(); - if (!Sym || !wasLoadedFromIvar(Sym)) + if (!Sym || !dyn_cast_or_null<ObjCIvarRegion>(Sym->getOriginRegion())) return; // Accessing an ivar directly is unusual. If we've done that, be more diff --git a/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp b/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp index 7026a2ec16a1d..ab4b4d3bd91be 100644 --- a/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp @@ -20,6 +20,7 @@ #include "clang/StaticAnalyzer/Core/Checker.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" +#include <utility> using namespace clang; using namespace ento; @@ -51,14 +52,11 @@ class SimpleStreamChecker : public Checker<check::PostCall, check::PreCall, check::DeadSymbols, check::PointerEscape> { - - mutable IdentifierInfo *IIfopen, *IIfclose; + CallDescription OpenFn, CloseFn; std::unique_ptr<BugType> DoubleCloseBugType; std::unique_ptr<BugType> LeakBugType; - void initIdentifierInfo(ASTContext &Ctx) const; - void reportDoubleClose(SymbolRef FileDescSym, const CallEvent &Call, CheckerContext &C) const; @@ -95,7 +93,7 @@ namespace { class StopTrackingCallback final : public SymbolVisitor { ProgramStateRef state; public: - StopTrackingCallback(ProgramStateRef st) : state(st) {} + StopTrackingCallback(ProgramStateRef st) : state(std::move(st)) {} ProgramStateRef getState() const { return state; } bool VisitSymbol(SymbolRef sym) override { @@ -106,7 +104,7 @@ public: } // end anonymous namespace SimpleStreamChecker::SimpleStreamChecker() - : IIfopen(nullptr), IIfclose(nullptr) { + : OpenFn("fopen"), CloseFn("fclose", 1) { // Initialize the bug types. DoubleCloseBugType.reset( new BugType(this, "Double fclose", "Unix Stream API Error")); @@ -119,12 +117,10 @@ SimpleStreamChecker::SimpleStreamChecker() void SimpleStreamChecker::checkPostCall(const CallEvent &Call, CheckerContext &C) const { - initIdentifierInfo(C.getASTContext()); - if (!Call.isGlobalCFunction()) return; - if (Call.getCalleeIdentifier() != IIfopen) + if (!Call.isCalled(OpenFn)) return; // Get the symbolic value corresponding to the file handle. @@ -140,15 +136,10 @@ void SimpleStreamChecker::checkPostCall(const CallEvent &Call, void SimpleStreamChecker::checkPreCall(const CallEvent &Call, CheckerContext &C) const { - initIdentifierInfo(C.getASTContext()); - if (!Call.isGlobalCFunction()) return; - if (Call.getCalleeIdentifier() != IIfclose) - return; - - if (Call.getNumArgs() != 1) + if (!Call.isCalled(CloseFn)) return; // Get the symbolic value corresponding to the file handle. @@ -275,13 +266,6 @@ SimpleStreamChecker::checkPointerEscape(ProgramStateRef State, return State; } -void SimpleStreamChecker::initIdentifierInfo(ASTContext &Ctx) const { - if (IIfopen) - return; - IIfopen = &Ctx.Idents.get("fopen"); - IIfclose = &Ctx.Idents.get("fclose"); -} - void ento::registerSimpleStreamChecker(CheckerManager &mgr) { mgr.registerChecker<SimpleStreamChecker>(); } diff --git a/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp b/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp index 79fc701d6d58c..556274d0edb65 100644 --- a/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp @@ -236,7 +236,12 @@ void StackAddrEscapeChecker::checkEndFunction(CheckerContext &Ctx) const { SmallString<512> buf; llvm::raw_svector_ostream os(buf); SourceRange range = genName(os, cb.V[i].second, Ctx.getASTContext()); - os << " is still referred to by the global variable '"; + os << " is still referred to by the "; + if (isa<StaticGlobalSpaceRegion>(cb.V[i].first->getMemorySpace())) + os << "static"; + else + os << "global"; + os << " variable '"; const VarRegion *VR = cast<VarRegion>(cb.V[i].first->getBaseRegion()); os << *VR->getDecl() << "' upon returning to the caller. This will be a dangling reference"; diff --git a/lib/StaticAnalyzer/Checkers/TraversalChecker.cpp b/lib/StaticAnalyzer/Checkers/TraversalChecker.cpp index d02d2df1c5070..8ad962875b060 100644 --- a/lib/StaticAnalyzer/Checkers/TraversalChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/TraversalChecker.cpp @@ -25,9 +25,11 @@ using namespace ento; namespace { class TraversalDumper : public Checker< check::BranchCondition, + check::BeginFunction, check::EndFunction > { public: void checkBranchCondition(const Stmt *Condition, CheckerContext &C) const; + void checkBeginFunction(CheckerContext &C) const; void checkEndFunction(CheckerContext &C) const; }; } @@ -50,6 +52,10 @@ void TraversalDumper::checkBranchCondition(const Stmt *Condition, << Parent->getStmtClassName() << "\n"; } +void TraversalDumper::checkBeginFunction(CheckerContext &C) const { + llvm::outs() << "--BEGIN FUNCTION--\n"; +} + void TraversalDumper::checkEndFunction(CheckerContext &C) const { llvm::outs() << "--END FUNCTION--\n"; } diff --git a/lib/StaticAnalyzer/Checkers/UndefBranchChecker.cpp b/lib/StaticAnalyzer/Checkers/UndefBranchChecker.cpp index ed17610e41169..0a274292aa39f 100644 --- a/lib/StaticAnalyzer/Checkers/UndefBranchChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/UndefBranchChecker.cpp @@ -17,6 +17,7 @@ #include "clang/StaticAnalyzer/Core/Checker.h" #include "clang/StaticAnalyzer/Core/CheckerManager.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" +#include <utility> using namespace clang; using namespace ento; @@ -31,7 +32,7 @@ class UndefBranchChecker : public Checker<check::BranchCondition> { const LocationContext *LCtx; FindUndefExpr(ProgramStateRef S, const LocationContext *L) - : St(S), LCtx(L) {} + : St(std::move(S)), LCtx(L) {} const Expr *FindExpr(const Expr *Ex) { if (!MatchesCriteria(Ex)) diff --git a/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp b/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp index a03abce9626b5..892e713d241f0 100644 --- a/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp @@ -26,10 +26,6 @@ #include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h" #include "llvm/ADT/SmallSet.h" -// The number of CFGBlock pointers we want to reserve memory for. This is used -// once for each function we analyze. -#define DEFAULT_CFGBLOCKS 256 - using namespace clang; using namespace ento; @@ -39,7 +35,7 @@ public: void checkEndAnalysis(ExplodedGraph &G, BugReporter &B, ExprEngine &Eng) const; private: - typedef llvm::SmallSet<unsigned, DEFAULT_CFGBLOCKS> CFGBlocksSet; + typedef llvm::SmallSet<unsigned, 32> CFGBlocksSet; static inline const Stmt *getUnreachableStmt(const CFGBlock *CB); static void FindUnreachableEntryPoints(const CFGBlock *CB, diff --git a/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp b/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp index e3b2ed2223639..40217bdee892b 100644 --- a/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp @@ -76,7 +76,6 @@ void VLASizeChecker::reportBug(VLASize_Kind Kind, report->addRange(SizeE->getSourceRange()); bugreporter::trackNullOrUndefValue(N, SizeE, *report); C.emitReport(std::move(report)); - return; } void VLASizeChecker::checkPreStmt(const DeclStmt *DS, CheckerContext &C) const { diff --git a/lib/StaticAnalyzer/Checkers/VforkChecker.cpp b/lib/StaticAnalyzer/Checkers/VforkChecker.cpp index 26ffee827cff2..75aefc0e83840 100644 --- a/lib/StaticAnalyzer/Checkers/VforkChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/VforkChecker.cpp @@ -54,10 +54,10 @@ class VforkChecker : public Checker<check::PreCall, check::PostCall, bool isCallWhitelisted(const IdentifierInfo *II, CheckerContext &C) const; void reportBug(const char *What, CheckerContext &C, - const char *Details = 0) const; + const char *Details = nullptr) const; public: - VforkChecker() : II_vfork(0) {} + VforkChecker() : II_vfork(nullptr) {} void checkPreCall(const CallEvent &Call, CheckerContext &C) const; void checkPostCall(const CallEvent &Call, CheckerContext &C) const; @@ -107,7 +107,7 @@ bool VforkChecker::isCallWhitelisted(const IdentifierInfo *II, "execv", "execvp", "execvpe", - 0, + nullptr }; ASTContext &AC = C.getASTContext(); diff --git a/lib/StaticAnalyzer/Core/BugReporter.cpp b/lib/StaticAnalyzer/Core/BugReporter.cpp index 11be764633cf8..488126b0088a1 100644 --- a/lib/StaticAnalyzer/Core/BugReporter.cpp +++ b/lib/StaticAnalyzer/Core/BugReporter.cpp @@ -2922,7 +2922,7 @@ bool TrimmedGraph::popNextReportGraph(ReportGraph &GraphWrapper) { while (true) { // Create the equivalent node in the new graph with the same state // and location. - ExplodedNode *NewN = GNew->getNode(OrigN->getLocation(), OrigN->getState(), + ExplodedNode *NewN = GNew->createUncachedNode(OrigN->getLocation(), OrigN->getState(), OrigN->isSink()); // Store the mapping to the original node. @@ -3487,7 +3487,7 @@ LLVM_DUMP_METHOD void PathPieces::dump() const { } } -void PathDiagnosticCallPiece::dump() const { +LLVM_DUMP_METHOD void PathDiagnosticCallPiece::dump() const { llvm::errs() << "CALL\n--------------\n"; if (const Stmt *SLoc = getLocStmt(getLocation())) @@ -3498,26 +3498,26 @@ void PathDiagnosticCallPiece::dump() const { getLocation().dump(); } -void PathDiagnosticEventPiece::dump() const { +LLVM_DUMP_METHOD void PathDiagnosticEventPiece::dump() const { llvm::errs() << "EVENT\n--------------\n"; llvm::errs() << getString() << "\n"; llvm::errs() << " ---- at ----\n"; getLocation().dump(); } -void PathDiagnosticControlFlowPiece::dump() const { +LLVM_DUMP_METHOD void PathDiagnosticControlFlowPiece::dump() const { llvm::errs() << "CONTROL\n--------------\n"; getStartLocation().dump(); llvm::errs() << " ---- to ----\n"; getEndLocation().dump(); } -void PathDiagnosticMacroPiece::dump() const { +LLVM_DUMP_METHOD void PathDiagnosticMacroPiece::dump() const { llvm::errs() << "MACRO\n--------------\n"; // FIXME: Print which macro is being invoked. } -void PathDiagnosticLocation::dump() const { +LLVM_DUMP_METHOD void PathDiagnosticLocation::dump() const { if (!isValid()) { llvm::errs() << "<INVALID>\n"; return; diff --git a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp index cf1e0a6a656cf..0e505463bb5e4 100644 --- a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp +++ b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp @@ -14,6 +14,7 @@ #include "clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h" #include "clang/AST/Expr.h" #include "clang/AST/ExprObjC.h" +#include "clang/Analysis/CFGStmtMap.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h" #include "clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" @@ -323,6 +324,9 @@ public: } PathDiagnosticLocation L(Ret, BRC.getSourceManager(), StackFrame); + if (!L.isValid() || !L.asLocation().isValid()) + return nullptr; + return new PathDiagnosticEventPiece(L, Out.str()); } @@ -828,8 +832,53 @@ SuppressInlineDefensiveChecksVisitor::VisitNode(const ExplodedNode *Succ, // Check if this is inlined defensive checks. const LocationContext *CurLC =Succ->getLocationContext(); const LocationContext *ReportLC = BR.getErrorNode()->getLocationContext(); - if (CurLC != ReportLC && !CurLC->isParentOf(ReportLC)) + if (CurLC != ReportLC && !CurLC->isParentOf(ReportLC)) { BR.markInvalid("Suppress IDC", CurLC); + return nullptr; + } + + // Treat defensive checks in function-like macros as if they were an inlined + // defensive check. If the bug location is not in a macro and the + // terminator for the current location is in a macro then suppress the + // warning. + auto BugPoint = BR.getErrorNode()->getLocation().getAs<StmtPoint>(); + + if (!BugPoint) + return nullptr; + + SourceLocation BugLoc = BugPoint->getStmt()->getLocStart(); + if (BugLoc.isMacroID()) + return nullptr; + + ProgramPoint CurPoint = Succ->getLocation(); + const Stmt *CurTerminatorStmt = nullptr; + if (auto BE = CurPoint.getAs<BlockEdge>()) { + CurTerminatorStmt = BE->getSrc()->getTerminator().getStmt(); + } else if (auto SP = CurPoint.getAs<StmtPoint>()) { + const Stmt *CurStmt = SP->getStmt(); + if (!CurStmt->getLocStart().isMacroID()) + return nullptr; + + CFGStmtMap *Map = CurLC->getAnalysisDeclContext()->getCFGStmtMap(); + CurTerminatorStmt = Map->getBlock(CurStmt)->getTerminator(); + } else { + return nullptr; + } + + if (!CurTerminatorStmt) + return nullptr; + + SourceLocation TerminatorLoc = CurTerminatorStmt->getLocStart(); + if (TerminatorLoc.isMacroID()) { + const SourceManager &SMgr = BRC.getSourceManager(); + std::pair<FileID, unsigned> TLInfo = SMgr.getDecomposedLoc(TerminatorLoc); + SrcMgr::SLocEntry SE = SMgr.getSLocEntry(TLInfo.first); + const SrcMgr::ExpansionInfo &EInfo = SE.getExpansion(); + if (EInfo.isFunctionMacroExpansion()) { + BR.markInvalid("Suppress Macro IDC", CurLC); + return nullptr; + } + } } return nullptr; } @@ -862,6 +911,15 @@ static const Expr *peelOffOuterExpr(const Expr *Ex, return peelOffOuterExpr(EWC->getSubExpr(), N); if (const OpaqueValueExpr *OVE = dyn_cast<OpaqueValueExpr>(Ex)) return peelOffOuterExpr(OVE->getSourceExpr(), N); + if (auto *POE = dyn_cast<PseudoObjectExpr>(Ex)) { + auto *PropRef = dyn_cast<ObjCPropertyRefExpr>(POE->getSyntacticForm()); + if (PropRef && PropRef->isMessagingGetter()) { + const Expr *GetterMessageSend = + POE->getSemanticExpr(POE->getNumSemanticExprs() - 1); + assert(isa<ObjCMessageExpr>(GetterMessageSend)); + return peelOffOuterExpr(GetterMessageSend, N); + } + } // Peel off the ternary operator. if (const ConditionalOperator *CO = dyn_cast<ConditionalOperator>(Ex)) { @@ -1494,20 +1552,6 @@ ConditionBRVisitor::VisitTrueTest(const Expr *Cond, return event; } - -// FIXME: Copied from ExprEngineCallAndReturn.cpp. -static bool isInStdNamespace(const Decl *D) { - const DeclContext *DC = D->getDeclContext()->getEnclosingNamespaceContext(); - const NamespaceDecl *ND = dyn_cast<NamespaceDecl>(DC); - if (!ND) - return false; - - while (const NamespaceDecl *Parent = dyn_cast<NamespaceDecl>(ND->getParent())) - ND = Parent; - - return ND->isStdNamespace(); -} - std::unique_ptr<PathDiagnosticPiece> LikelyFalsePositiveSuppressionBRVisitor::getEndPath(BugReporterContext &BRC, const ExplodedNode *N, @@ -1518,7 +1562,7 @@ LikelyFalsePositiveSuppressionBRVisitor::getEndPath(BugReporterContext &BRC, AnalyzerOptions &Options = Eng.getAnalysisManager().options; const Decl *D = N->getLocationContext()->getDecl(); - if (isInStdNamespace(D)) { + if (AnalysisDeclContext::isInStdNamespace(D)) { // Skip reports within the 'std' namespace. Although these can sometimes be // the user's fault, we currently don't report them very well, and // Note that this will not help for any other data structure libraries, like @@ -1552,12 +1596,6 @@ LikelyFalsePositiveSuppressionBRVisitor::getEndPath(BugReporterContext &BRC, } } - // The analyzer issues a false positive on - // std::basic_string<uint8_t> v; v.push_back(1); - // and - // std::u16string s; s += u'a'; - // because we cannot reason about the internal invariants of the - // datastructure. for (const LocationContext *LCtx = N->getLocationContext(); LCtx; LCtx = LCtx->getParent()) { const CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(LCtx->getDecl()); @@ -1565,10 +1603,24 @@ LikelyFalsePositiveSuppressionBRVisitor::getEndPath(BugReporterContext &BRC, continue; const CXXRecordDecl *CD = MD->getParent(); + // The analyzer issues a false positive on + // std::basic_string<uint8_t> v; v.push_back(1); + // and + // std::u16string s; s += u'a'; + // because we cannot reason about the internal invariants of the + // datastructure. if (CD->getName() == "basic_string") { BR.markInvalid(getTag(), nullptr); return nullptr; } + + // The analyzer issues a false positive on + // std::shared_ptr<int> p(new int(1)); p = nullptr; + // because it does not reason properly about temporary destructors. + if (CD->getName() == "shared_ptr") { + BR.markInvalid(getTag(), nullptr); + return nullptr; + } } } } diff --git a/lib/StaticAnalyzer/Core/CallEvent.cpp b/lib/StaticAnalyzer/Core/CallEvent.cpp index 69af09b25b6e2..52613186677ac 100644 --- a/lib/StaticAnalyzer/Core/CallEvent.cpp +++ b/lib/StaticAnalyzer/Core/CallEvent.cpp @@ -177,7 +177,7 @@ ProgramStateRef CallEvent::invalidateRegions(unsigned BlockCount, // below for efficiency. if (PreserveArgs.count(Idx)) if (const MemRegion *MR = getArgSVal(Idx).getAsRegion()) - ETraits.setTrait(MR->StripCasts(), + ETraits.setTrait(MR->getBaseRegion(), RegionAndSymbolInvalidationTraits::TK_PreserveContents); // TODO: Factor this out + handle the lower level const pointers. @@ -210,6 +210,16 @@ ProgramPoint CallEvent::getProgramPoint(bool IsPreVisit, return PostImplicitCall(D, Loc, getLocationContext(), Tag); } +bool CallEvent::isCalled(const CallDescription &CD) const { + assert(getKind() != CE_ObjCMessage && "Obj-C methods are not supported"); + if (!CD.II) + CD.II = &getState()->getStateManager().getContext().Idents.get(CD.FuncName); + if (getCalleeIdentifier() != CD.II) + return false; + return (CD.RequiredArgs == CallDescription::NoArgRequirement || + CD.RequiredArgs == getNumArgs()); +} + SVal CallEvent::getArgSVal(unsigned Index) const { const Expr *ArgE = getArgExpr(Index); if (!ArgE) @@ -668,9 +678,26 @@ ArrayRef<ParmVarDecl*> ObjCMethodCall::parameters() const { return D->parameters(); } -void -ObjCMethodCall::getExtraInvalidatedValues(ValueList &Values, - RegionAndSymbolInvalidationTraits *ETraits) const { +void ObjCMethodCall::getExtraInvalidatedValues( + ValueList &Values, RegionAndSymbolInvalidationTraits *ETraits) const { + + // If the method call is a setter for property known to be backed by + // an instance variable, don't invalidate the entire receiver, just + // the storage for that instance variable. + if (const ObjCPropertyDecl *PropDecl = getAccessedProperty()) { + if (const ObjCIvarDecl *PropIvar = PropDecl->getPropertyIvarDecl()) { + SVal IvarLVal = getState()->getLValue(PropIvar, getReceiverSVal()); + const MemRegion *IvarRegion = IvarLVal.getAsRegion(); + ETraits->setTrait( + IvarRegion, + RegionAndSymbolInvalidationTraits::TK_DoNotInvalidateSuperRegion); + ETraits->setTrait(IvarRegion, + RegionAndSymbolInvalidationTraits::TK_SuppressEscape); + Values.push_back(IvarLVal); + return; + } + } + Values.push_back(getReceiverSVal()); } @@ -730,6 +757,18 @@ const PseudoObjectExpr *ObjCMethodCall::getContainingPseudoObjectExpr() const { return ObjCMessageDataTy::getFromOpaqueValue(Data).getPointer(); } +static const Expr * +getSyntacticFromForPseudoObjectExpr(const PseudoObjectExpr *POE) { + const Expr *Syntactic = POE->getSyntacticForm(); + + // This handles the funny case of assigning to the result of a getter. + // This can happen if the getter returns a non-const reference. + if (const BinaryOperator *BO = dyn_cast<BinaryOperator>(Syntactic)) + Syntactic = BO->getLHS(); + + return Syntactic; +} + ObjCMessageKind ObjCMethodCall::getMessageKind() const { if (!Data) { @@ -739,12 +778,7 @@ ObjCMessageKind ObjCMethodCall::getMessageKind() const { // Check if parent is a PseudoObjectExpr. if (const PseudoObjectExpr *POE = dyn_cast_or_null<PseudoObjectExpr>(S)) { - const Expr *Syntactic = POE->getSyntacticForm(); - - // This handles the funny case of assigning to the result of a getter. - // This can happen if the getter returns a non-const reference. - if (const BinaryOperator *BO = dyn_cast<BinaryOperator>(Syntactic)) - Syntactic = BO->getLHS(); + const Expr *Syntactic = getSyntacticFromForPseudoObjectExpr(POE); ObjCMessageKind K; switch (Syntactic->getStmtClass()) { @@ -780,6 +814,27 @@ ObjCMessageKind ObjCMethodCall::getMessageKind() const { return static_cast<ObjCMessageKind>(Info.getInt()); } +const ObjCPropertyDecl *ObjCMethodCall::getAccessedProperty() const { + // Look for properties accessed with property syntax (foo.bar = ...) + if ( getMessageKind() == OCM_PropertyAccess) { + const PseudoObjectExpr *POE = getContainingPseudoObjectExpr(); + assert(POE && "Property access without PseudoObjectExpr?"); + + const Expr *Syntactic = getSyntacticFromForPseudoObjectExpr(POE); + auto *RefExpr = cast<ObjCPropertyRefExpr>(Syntactic); + + if (RefExpr->isExplicitProperty()) + return RefExpr->getExplicitProperty(); + } + + // Look for properties accessed with method syntax ([foo setBar:...]). + const ObjCMethodDecl *MD = getDecl(); + if (!MD || !MD->isPropertyAccessor()) + return nullptr; + + // Note: This is potentially quite slow. + return MD->findPropertyDecl(); +} bool ObjCMethodCall::canBeOverridenInSubclass(ObjCInterfaceDecl *IDecl, Selector Sel) const { @@ -903,8 +958,30 @@ RuntimeDefinition ObjCMethodCall::getRuntimeDefinition() const { // even if we don't actually have an implementation. if (!*Val) if (const ObjCMethodDecl *CompileTimeMD = E->getMethodDecl()) - if (CompileTimeMD->isPropertyAccessor()) - Val = IDecl->lookupInstanceMethod(Sel); + if (CompileTimeMD->isPropertyAccessor()) { + if (!CompileTimeMD->getSelfDecl() && + isa<ObjCCategoryDecl>(CompileTimeMD->getDeclContext())) { + // If the method is an accessor in a category, and it doesn't + // have a self declaration, first + // try to find the method in a class extension. This + // works around a bug in Sema where multiple accessors + // are synthesized for properties in class + // extensions that are redeclared in a category and the + // the implicit parameters are not filled in for + // the method on the category. + // This ensures we find the accessor in the extension, which + // has the implicit parameters filled in. + auto *ID = CompileTimeMD->getClassInterface(); + for (auto *CatDecl : ID->visible_extensions()) { + Val = CatDecl->getMethod(Sel, + CompileTimeMD->isInstanceMethod()); + if (*Val) + break; + } + } + if (!*Val) + Val = IDecl->lookupInstanceMethod(Sel); + } } const ObjCMethodDecl *MD = Val.getValue(); diff --git a/lib/StaticAnalyzer/Core/CheckerContext.cpp b/lib/StaticAnalyzer/Core/CheckerContext.cpp index 5ec8bfa800744..548b06ef91fce 100644 --- a/lib/StaticAnalyzer/Core/CheckerContext.cpp +++ b/lib/StaticAnalyzer/Core/CheckerContext.cpp @@ -35,6 +35,13 @@ StringRef CheckerContext::getCalleeName(const FunctionDecl *FunDecl) const { return funI->getName(); } +StringRef CheckerContext::getDeclDescription(const Decl *D) { + if (isa<ObjCMethodDecl>(D) || isa<CXXMethodDecl>(D)) + return "method"; + if (isa<BlockDecl>(D)) + return "anonymous block"; + return "function"; +} bool CheckerContext::isCLibraryFunction(const FunctionDecl *FD, StringRef Name) { diff --git a/lib/StaticAnalyzer/Core/CheckerHelpers.cpp b/lib/StaticAnalyzer/Core/CheckerHelpers.cpp index d6aeceb1457db..ed41914ebd054 100644 --- a/lib/StaticAnalyzer/Core/CheckerHelpers.cpp +++ b/lib/StaticAnalyzer/Core/CheckerHelpers.cpp @@ -75,8 +75,8 @@ bool clang::ento::containsBuiltinOffsetOf(const Stmt *S) { // Extract lhs and rhs from assignment statement std::pair<const clang::VarDecl *, const clang::Expr *> clang::ento::parseAssignment(const Stmt *S) { - const VarDecl *VD = 0; - const Expr *RHS = 0; + const VarDecl *VD = nullptr; + const Expr *RHS = nullptr; if (auto Assign = dyn_cast_or_null<BinaryOperator>(S)) { if (Assign->isAssignmentOp()) { diff --git a/lib/StaticAnalyzer/Core/CheckerManager.cpp b/lib/StaticAnalyzer/Core/CheckerManager.cpp index 008e8ef31cda7..d8382e88691a5 100644 --- a/lib/StaticAnalyzer/Core/CheckerManager.cpp +++ b/lib/StaticAnalyzer/Core/CheckerManager.cpp @@ -377,6 +377,40 @@ void CheckerManager::runCheckersForEndAnalysis(ExplodedGraph &G, EndAnalysisCheckers[i](G, BR, Eng); } +namespace { +struct CheckBeginFunctionContext { + typedef std::vector<CheckerManager::CheckBeginFunctionFunc> CheckersTy; + const CheckersTy &Checkers; + ExprEngine &Eng; + const ProgramPoint &PP; + + CheckersTy::const_iterator checkers_begin() { return Checkers.begin(); } + CheckersTy::const_iterator checkers_end() { return Checkers.end(); } + + CheckBeginFunctionContext(const CheckersTy &Checkers, ExprEngine &Eng, + const ProgramPoint &PP) + : Checkers(Checkers), Eng(Eng), PP(PP) {} + + void runChecker(CheckerManager::CheckBeginFunctionFunc checkFn, + NodeBuilder &Bldr, ExplodedNode *Pred) { + const ProgramPoint &L = PP.withTag(checkFn.Checker); + CheckerContext C(Bldr, Eng, Pred, L); + + checkFn(C); + } +}; +} + +void CheckerManager::runCheckersForBeginFunction(ExplodedNodeSet &Dst, + const BlockEdge &L, + ExplodedNode *Pred, + ExprEngine &Eng) { + ExplodedNodeSet Src; + Src.insert(Pred); + CheckBeginFunctionContext C(BeginFunctionCheckers, Eng, L); + expandGraphWithCheckers(C, Dst, Src); +} + /// \brief Run checkers for end of path. // Note, We do not chain the checker output (like in expandGraphWithCheckers) // for this callback since end of path nodes are expected to be final. @@ -671,6 +705,10 @@ void CheckerManager::_registerForEndAnalysis(CheckEndAnalysisFunc checkfn) { EndAnalysisCheckers.push_back(checkfn); } +void CheckerManager::_registerForBeginFunction(CheckBeginFunctionFunc checkfn) { + BeginFunctionCheckers.push_back(checkfn); +} + void CheckerManager::_registerForEndFunction(CheckEndFunctionFunc checkfn) { EndFunctionCheckers.push_back(checkfn); } diff --git a/lib/StaticAnalyzer/Core/CheckerRegistry.cpp b/lib/StaticAnalyzer/Core/CheckerRegistry.cpp index a15e1573e228b..ba03e2f8a3c18 100644 --- a/lib/StaticAnalyzer/Core/CheckerRegistry.cpp +++ b/lib/StaticAnalyzer/Core/CheckerRegistry.cpp @@ -49,12 +49,12 @@ static void collectCheckers(const CheckerRegistry::CheckerInfoList &checkers, CheckerOptInfo &opt, CheckerInfoSet &collected) { // Use a binary search to find the possible start of the package. CheckerRegistry::CheckerInfo packageInfo(nullptr, opt.getName(), ""); - CheckerRegistry::CheckerInfoList::const_iterator e = checkers.end(); + auto end = checkers.cend(); CheckerRegistry::CheckerInfoList::const_iterator i = - std::lower_bound(checkers.begin(), e, packageInfo, checkerNameLT); + std::lower_bound(checkers.cbegin(), end, packageInfo, checkerNameLT); // If we didn't even find a possible package, give up. - if (i == e) + if (i == end) return; // If what we found doesn't actually start the package, give up. @@ -73,7 +73,7 @@ static void collectCheckers(const CheckerRegistry::CheckerInfoList &checkers, size = packageSize->getValue(); // Step through all the checkers in the package. - for (e = i+size; i != e; ++i) { + for (auto checkEnd = i+size; i != checkEnd; ++i) { if (opt.isEnabled()) collected.insert(&*i); else diff --git a/lib/StaticAnalyzer/Core/CoreEngine.cpp b/lib/StaticAnalyzer/Core/CoreEngine.cpp index 39cf7e771755d..da608f6c75588 100644 --- a/lib/StaticAnalyzer/Core/CoreEngine.cpp +++ b/lib/StaticAnalyzer/Core/CoreEngine.cpp @@ -192,14 +192,27 @@ bool CoreEngine::ExecuteWorkList(const LocationContext *L, unsigned Steps, WList->setBlockCounter(BCounterFactory.GetEmptyCounter()); if (!InitState) - // Generate the root. - generateNode(StartLoc, SubEng.getInitialState(L), nullptr); - else - generateNode(StartLoc, InitState, nullptr); + InitState = SubEng.getInitialState(L); + + bool IsNew; + ExplodedNode *Node = G.getNode(StartLoc, InitState, false, &IsNew); + assert (IsNew); + G.addRoot(Node); + + NodeBuilderContext BuilderCtx(*this, StartLoc.getDst(), Node); + ExplodedNodeSet DstBegin; + SubEng.processBeginOfFunction(BuilderCtx, Node, DstBegin, StartLoc); + + enqueue(DstBegin); } // Check if we have a steps limit bool UnlimitedSteps = Steps == 0; + // Cap our pre-reservation in the event that the user specifies + // a very large number of maximum steps. + const unsigned PreReservationCap = 4000000; + if(!UnlimitedSteps) + G.reserve(std::min(Steps,PreReservationCap)); while (WList->hasWork()) { if (!UnlimitedSteps) { @@ -243,8 +256,7 @@ void CoreEngine::dispatchWorkItem(ExplodedNode* Pred, ProgramPoint Loc, break; case ProgramPoint::CallEnterKind: { - CallEnter CEnter = Loc.castAs<CallEnter>(); - SubEng.processCallEnter(CEnter, Pred); + HandleCallEnter(Loc.castAs<CallEnter>(), Pred); break; } @@ -456,6 +468,11 @@ void CoreEngine::HandleBlockExit(const CFGBlock * B, ExplodedNode *Pred) { Pred->State, Pred); } +void CoreEngine::HandleCallEnter(const CallEnter &CE, ExplodedNode *Pred) { + NodeBuilderContext BuilderCtx(*this, CE.getEntry(), Pred); + SubEng.processCallEnter(BuilderCtx, CE, Pred); +} + void CoreEngine::HandleBranch(const Stmt *Cond, const Stmt *Term, const CFGBlock * B, ExplodedNode *Pred) { assert(B->succ_size() == 2); diff --git a/lib/StaticAnalyzer/Core/ExplodedGraph.cpp b/lib/StaticAnalyzer/Core/ExplodedGraph.cpp index 8a09720b2a196..02d382cc4885e 100644 --- a/lib/StaticAnalyzer/Core/ExplodedGraph.cpp +++ b/lib/StaticAnalyzer/Core/ExplodedGraph.cpp @@ -336,6 +336,14 @@ ExplodedNode *ExplodedGraph::getNode(const ProgramPoint &L, return V; } +ExplodedNode *ExplodedGraph::createUncachedNode(const ProgramPoint &L, + ProgramStateRef State, + bool IsSink) { + NodeTy *V = (NodeTy *) getAllocator().Allocate<NodeTy>(); + new (V) NodeTy(L, State, IsSink); + return V; +} + std::unique_ptr<ExplodedGraph> ExplodedGraph::trim(ArrayRef<const NodeTy *> Sinks, InterExplodedGraphMap *ForwardMap, @@ -395,8 +403,7 @@ ExplodedGraph::trim(ArrayRef<const NodeTy *> Sinks, // Create the corresponding node in the new graph and record the mapping // from the old node to the new node. - ExplodedNode *NewN = G->getNode(N->getLocation(), N->State, N->isSink(), - nullptr); + ExplodedNode *NewN = G->createUncachedNode(N->getLocation(), N->State, N->isSink()); Pass2[N] = NewN; // Also record the reverse mapping from the new node to the old node. diff --git a/lib/StaticAnalyzer/Core/ExprEngine.cpp b/lib/StaticAnalyzer/Core/ExprEngine.cpp index 662b0a2dd7989..405aecdee0320 100644 --- a/lib/StaticAnalyzer/Core/ExprEngine.cpp +++ b/lib/StaticAnalyzer/Core/ExprEngine.cpp @@ -30,6 +30,7 @@ #include "llvm/ADT/ImmutableList.h" #include "llvm/ADT/Statistic.h" #include "llvm/Support/raw_ostream.h" +#include "llvm/Support/SaveAndRestore.h" #ifndef NDEBUG #include "llvm/Support/GraphWriter.h" @@ -754,6 +755,7 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred, // C++ and ARC stuff we don't support yet. case Expr::ObjCIndirectCopyRestoreExprClass: case Stmt::CXXDependentScopeMemberExprClass: + case Stmt::CXXInheritedCtorInitExprClass: case Stmt::CXXTryStmtClass: case Stmt::CXXTypeidExprClass: case Stmt::CXXUuidofExprClass: @@ -830,12 +832,21 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred, case Stmt::OMPAtomicDirectiveClass: case Stmt::OMPTargetDirectiveClass: case Stmt::OMPTargetDataDirectiveClass: + case Stmt::OMPTargetEnterDataDirectiveClass: + case Stmt::OMPTargetExitDataDirectiveClass: + case Stmt::OMPTargetParallelDirectiveClass: + case Stmt::OMPTargetParallelForDirectiveClass: + case Stmt::OMPTargetUpdateDirectiveClass: case Stmt::OMPTeamsDirectiveClass: case Stmt::OMPCancellationPointDirectiveClass: case Stmt::OMPCancelDirectiveClass: case Stmt::OMPTaskLoopDirectiveClass: case Stmt::OMPTaskLoopSimdDirectiveClass: case Stmt::OMPDistributeDirectiveClass: + case Stmt::OMPDistributeParallelForDirectiveClass: + case Stmt::OMPDistributeParallelForSimdDirectiveClass: + case Stmt::OMPDistributeSimdDirectiveClass: + case Stmt::OMPTargetParallelForSimdDirectiveClass: llvm_unreachable("Stmt should not be in analyzer evaluation loop"); case Stmt::ObjCSubscriptRefExprClass: @@ -892,7 +903,6 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred, case Stmt::CUDAKernelCallExprClass: case Stmt::OpaqueValueExprClass: case Stmt::AsTypeExprClass: - case Stmt::AtomicExprClass: // Fall through. // Cases we intentionally don't evaluate, since they don't need @@ -906,6 +916,7 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred, case Stmt::CXXScalarValueInitExprClass: case Stmt::CXXBoolLiteralExprClass: case Stmt::ObjCBoolLiteralExprClass: + case Stmt::ObjCAvailabilityCheckExprClass: case Stmt::FloatingLiteralClass: case Stmt::NoInitExprClass: case Stmt::SizeOfPackExprClass: @@ -1237,6 +1248,12 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred, Bldr.addNodes(Dst); break; + case Stmt::AtomicExprClass: + Bldr.takeNodes(Pred); + VisitAtomicExpr(cast<AtomicExpr>(S), Pred, Dst); + Bldr.addNodes(Dst); + break; + case Stmt::ObjCIvarRefExprClass: Bldr.takeNodes(Pred); VisitLvalObjCIvarRefExpr(cast<ObjCIvarRefExpr>(S), Pred, Dst); @@ -1745,6 +1762,14 @@ static bool stackFrameDoesNotContainInitializedTemporaries(ExplodedNode &Pred) { } #endif +void ExprEngine::processBeginOfFunction(NodeBuilderContext &BC, + ExplodedNode *Pred, + ExplodedNodeSet &Dst, + const BlockEdge &L) { + SaveAndRestore<const NodeBuilderContext *> NodeContextRAII(currBldrCtx, &BC); + getCheckerManager().runCheckersForBeginFunction(Dst, L, Pred, *this); +} + /// ProcessEndPath - Called by CoreEngine. Used to generate end-of-path /// nodes when the control reaches the end of a function. void ExprEngine::processEndOfFunction(NodeBuilderContext& BC, @@ -2052,6 +2077,44 @@ void ExprEngine::VisitMemberExpr(const MemberExpr *M, ExplodedNode *Pred, getCheckerManager().runCheckersForPostStmt(Dst, EvalSet, M, *this); } +void ExprEngine::VisitAtomicExpr(const AtomicExpr *AE, ExplodedNode *Pred, + ExplodedNodeSet &Dst) { + ExplodedNodeSet AfterPreSet; + getCheckerManager().runCheckersForPreStmt(AfterPreSet, Pred, AE, *this); + + // For now, treat all the arguments to C11 atomics as escaping. + // FIXME: Ideally we should model the behavior of the atomics precisely here. + + ExplodedNodeSet AfterInvalidateSet; + StmtNodeBuilder Bldr(AfterPreSet, AfterInvalidateSet, *currBldrCtx); + + for (ExplodedNodeSet::iterator I = AfterPreSet.begin(), E = AfterPreSet.end(); + I != E; ++I) { + ProgramStateRef State = (*I)->getState(); + const LocationContext *LCtx = (*I)->getLocationContext(); + + SmallVector<SVal, 8> ValuesToInvalidate; + for (unsigned SI = 0, Count = AE->getNumSubExprs(); SI != Count; SI++) { + const Expr *SubExpr = AE->getSubExprs()[SI]; + SVal SubExprVal = State->getSVal(SubExpr, LCtx); + ValuesToInvalidate.push_back(SubExprVal); + } + + State = State->invalidateRegions(ValuesToInvalidate, AE, + currBldrCtx->blockCount(), + LCtx, + /*CausedByPointerEscape*/true, + /*Symbols=*/nullptr); + + SVal ResultVal = UnknownVal(); + State = State->BindExpr(AE, LCtx, ResultVal); + Bldr.generateNode(AE, *I, State, nullptr, + ProgramPoint::PostStmtKind); + } + + getCheckerManager().runCheckersForPostStmt(Dst, AfterInvalidateSet, AE, *this); +} + namespace { class CollectReachableSymbolsCallback final : public SymbolVisitor { InvalidatedSymbols Symbols; diff --git a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp index 74cc8d2ccbc5b..39d88bfda1486 100644 --- a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp +++ b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp @@ -37,13 +37,12 @@ STATISTIC(NumInlinedCalls, STATISTIC(NumReachedInlineCountMax, "The # of times we reached inline count maximum"); -void ExprEngine::processCallEnter(CallEnter CE, ExplodedNode *Pred) { +void ExprEngine::processCallEnter(NodeBuilderContext& BC, CallEnter CE, + ExplodedNode *Pred) { // Get the entry block in the CFG of the callee. const StackFrameContext *calleeCtx = CE.getCalleeContext(); PrettyStackTraceLocationContext CrashInfo(calleeCtx); - - const CFG *CalleeCFG = calleeCtx->getCFG(); - const CFGBlock *Entry = &(CalleeCFG->getEntry()); + const CFGBlock *Entry = CE.getEntry(); // Validate the CFG. assert(Entry->empty()); @@ -57,12 +56,16 @@ void ExprEngine::processCallEnter(CallEnter CE, ExplodedNode *Pred) { ProgramStateRef state = Pred->getState(); - // Construct a new node and add it to the worklist. + // Construct a new node, notify checkers that analysis of the function has + // begun, and add the resultant nodes to the worklist. bool isNew; ExplodedNode *Node = G.getNode(Loc, state, false, &isNew); Node->addPredecessor(Pred, G); - if (isNew) - Engine.getWorkList()->enqueue(Node); + if (isNew) { + ExplodedNodeSet DstBegin; + processBeginOfFunction(BC, Node, DstBegin, Loc); + Engine.enqueue(DstBegin); + } } // Find the last statement on the path to the exploded node and the @@ -379,22 +382,6 @@ void ExprEngine::examineStackFrames(const Decl *D, const LocationContext *LCtx, } LCtx = LCtx->getParent(); } - -} - -static bool IsInStdNamespace(const FunctionDecl *FD) { - const DeclContext *DC = FD->getEnclosingNamespaceContext(); - const NamespaceDecl *ND = dyn_cast<NamespaceDecl>(DC); - if (!ND) - return false; - - while (const DeclContext *Parent = ND->getParent()) { - if (!isa<NamespaceDecl>(Parent)) - break; - ND = cast<NamespaceDecl>(Parent); - } - - return ND->isStdNamespace(); } // The GDM component containing the dynamic dispatch bifurcation info. When @@ -408,7 +395,8 @@ namespace { DynamicDispatchModeInlined = 1, DynamicDispatchModeConservative }; -} +} // end anonymous namespace + REGISTER_TRAIT_WITH_PROGRAMSTATE(DynamicDispatchBifurcationMap, CLANG_ENTO_PROGRAMSTATE_MAP(const MemRegion *, unsigned)) @@ -441,7 +429,6 @@ bool ExprEngine::inlineCall(const CallEvent &Call, const Decl *D, currBldrCtx->getBlock(), currStmtIdx); - CallEnter Loc(CallE, CalleeSFC, CurLC); // Construct a new state which contains the mapping from actual to @@ -761,7 +748,7 @@ static bool mayInlineDecl(AnalysisDeclContext *CalleeADC, // Conditionally control the inlining of C++ standard library functions. if (!Opts.mayInlineCXXStandardLibrary()) if (Ctx.getSourceManager().isInSystemHeader(FD->getLocation())) - if (IsInStdNamespace(FD)) + if (AnalysisDeclContext::isInStdNamespace(FD)) return false; // Conditionally control the inlining of methods on objects that look @@ -778,7 +765,6 @@ static bool mayInlineDecl(AnalysisDeclContext *CalleeADC, if (!Opts.mayInlineCXXSharedPtrDtor()) if (isCXXSharedPtrDtor(FD)) return false; - } } @@ -988,13 +974,10 @@ void ExprEngine::BifurcateCall(const MemRegion *BifurReg, conservativeEvalCall(Call, Bldr, Pred, NoIState); NumOfDynamicDispatchPathSplits++; - return; } - void ExprEngine::VisitReturnStmt(const ReturnStmt *RS, ExplodedNode *Pred, ExplodedNodeSet &Dst) { - ExplodedNodeSet dstPreVisit; getCheckerManager().runCheckersForPreStmt(dstPreVisit, Pred, RS, *this); diff --git a/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp b/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp index b3edb8569bd6c..3a18956e41393 100644 --- a/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp +++ b/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp @@ -412,13 +412,13 @@ void HTMLDiagnostics::HandlePiece(Rewriter& R, FileID BugFileID, // Output a maximum size. if (!isa<PathDiagnosticMacroPiece>(P)) { // Get the string and determining its maximum substring. - const std::string& Msg = P.getString(); + const auto &Msg = P.getString(); unsigned max_token = 0; unsigned cnt = 0; unsigned len = Msg.size(); - for (std::string::const_iterator I=Msg.begin(), E=Msg.end(); I!=E; ++I) - switch (*I) { + for (char C : Msg) + switch (C) { default: ++cnt; continue; diff --git a/lib/StaticAnalyzer/Core/IssueHash.cpp b/lib/StaticAnalyzer/Core/IssueHash.cpp index 0a3af3dcc7e94..bd5c81179adc9 100644 --- a/lib/StaticAnalyzer/Core/IssueHash.cpp +++ b/lib/StaticAnalyzer/Core/IssueHash.cpp @@ -132,8 +132,11 @@ static std::string NormalizeLine(const SourceManager &SM, FullSourceLoc &L, StringRef Str = GetNthLineOfFile(SM.getBuffer(L.getFileID(), L), L.getExpansionLineNumber()); - unsigned col = Str.find_first_not_of(Whitespaces); - col++; + StringRef::size_type col = Str.find_first_not_of(Whitespaces); + if (col == StringRef::npos) + col = 1; // The line only contains whitespace. + else + col++; SourceLocation StartOfLine = SM.translateLineCol(SM.getFileID(L), L.getExpansionLineNumber(), col); llvm::MemoryBuffer *Buffer = @@ -180,7 +183,7 @@ std::string clang::GetIssueString(const SourceManager &SM, return (llvm::Twine(CheckerName) + Delimiter + GetEnclosingDeclContextSignature(D) + Delimiter + - llvm::utostr(IssueLoc.getExpansionColumnNumber()) + Delimiter + + Twine(IssueLoc.getExpansionColumnNumber()) + Delimiter + NormalizeLine(SM, IssueLoc, LangOpts) + Delimiter + BugType) .str(); } diff --git a/lib/StaticAnalyzer/Core/Makefile b/lib/StaticAnalyzer/Core/Makefile deleted file mode 100644 index c3e00fa36825e..0000000000000 --- a/lib/StaticAnalyzer/Core/Makefile +++ /dev/null @@ -1,17 +0,0 @@ -##===- clang/lib/StaticAnalyzer/Core/Makefile --------------*- Makefile -*-===## -# -# The LLVM Compiler Infrastructure -# -# This file is distributed under the University of Illinois Open Source -# License. See LICENSE.TXT for details. -# -##===----------------------------------------------------------------------===## -# -# This implements analyses built on top of source-level CFGs. -# -##===----------------------------------------------------------------------===## - -CLANG_LEVEL := ../../.. -LIBRARYNAME := clangStaticAnalyzerCore - -include $(CLANG_LEVEL)/Makefile diff --git a/lib/StaticAnalyzer/Core/MemRegion.cpp b/lib/StaticAnalyzer/Core/MemRegion.cpp index 30052ccacee44..b7b6f42b29102 100644 --- a/lib/StaticAnalyzer/Core/MemRegion.cpp +++ b/lib/StaticAnalyzer/Core/MemRegion.cpp @@ -35,7 +35,6 @@ template<typename RegionTy> struct MemRegionManagerTrait; template <typename RegionTy, typename A1> RegionTy* MemRegionManager::getRegion(const A1 a1) { - const typename MemRegionManagerTrait<RegionTy>::SuperRegionTy *superRegion = MemRegionManagerTrait<RegionTy>::getSuperRegion(*this, a1); @@ -46,7 +45,7 @@ RegionTy* MemRegionManager::getRegion(const A1 a1) { InsertPos)); if (!R) { - R = (RegionTy*) A.Allocate<RegionTy>(); + R = A.Allocate<RegionTy>(); new (R) RegionTy(a1, superRegion); Regions.InsertNode(R, InsertPos); } @@ -64,7 +63,7 @@ RegionTy* MemRegionManager::getSubRegion(const A1 a1, InsertPos)); if (!R) { - R = (RegionTy*) A.Allocate<RegionTy>(); + R = A.Allocate<RegionTy>(); new (R) RegionTy(a1, superRegion); Regions.InsertNode(R, InsertPos); } @@ -74,7 +73,6 @@ RegionTy* MemRegionManager::getSubRegion(const A1 a1, template <typename RegionTy, typename A1, typename A2> RegionTy* MemRegionManager::getRegion(const A1 a1, const A2 a2) { - const typename MemRegionManagerTrait<RegionTy>::SuperRegionTy *superRegion = MemRegionManagerTrait<RegionTy>::getSuperRegion(*this, a1, a2); @@ -85,7 +83,7 @@ RegionTy* MemRegionManager::getRegion(const A1 a1, const A2 a2) { InsertPos)); if (!R) { - R = (RegionTy*) A.Allocate<RegionTy>(); + R = A.Allocate<RegionTy>(); new (R) RegionTy(a1, a2, superRegion); Regions.InsertNode(R, InsertPos); } @@ -96,7 +94,6 @@ RegionTy* MemRegionManager::getRegion(const A1 a1, const A2 a2) { template <typename RegionTy, typename A1, typename A2> RegionTy* MemRegionManager::getSubRegion(const A1 a1, const A2 a2, const MemRegion *superRegion) { - llvm::FoldingSetNodeID ID; RegionTy::ProfileRegion(ID, a1, a2, superRegion); void *InsertPos; @@ -104,7 +101,7 @@ RegionTy* MemRegionManager::getSubRegion(const A1 a1, const A2 a2, InsertPos)); if (!R) { - R = (RegionTy*) A.Allocate<RegionTy>(); + R = A.Allocate<RegionTy>(); new (R) RegionTy(a1, a2, superRegion); Regions.InsertNode(R, InsertPos); } @@ -115,7 +112,6 @@ RegionTy* MemRegionManager::getSubRegion(const A1 a1, const A2 a2, template <typename RegionTy, typename A1, typename A2, typename A3> RegionTy* MemRegionManager::getSubRegion(const A1 a1, const A2 a2, const A3 a3, const MemRegion *superRegion) { - llvm::FoldingSetNodeID ID; RegionTy::ProfileRegion(ID, a1, a2, a3, superRegion); void *InsertPos; @@ -123,7 +119,7 @@ RegionTy* MemRegionManager::getSubRegion(const A1 a1, const A2 a2, const A3 a3, InsertPos)); if (!R) { - R = (RegionTy*) A.Allocate<RegionTy>(); + R = A.Allocate<RegionTy>(); new (R) RegionTy(a1, a2, a3, superRegion); Regions.InsertNode(R, InsertPos); } @@ -246,23 +242,23 @@ QualType CXXBaseObjectRegion::getValueType() const { //===----------------------------------------------------------------------===// void MemSpaceRegion::Profile(llvm::FoldingSetNodeID &ID) const { - ID.AddInteger((unsigned)getKind()); + ID.AddInteger(static_cast<unsigned>(getKind())); } void StackSpaceRegion::Profile(llvm::FoldingSetNodeID &ID) const { - ID.AddInteger((unsigned)getKind()); + ID.AddInteger(static_cast<unsigned>(getKind())); ID.AddPointer(getStackFrame()); } void StaticGlobalSpaceRegion::Profile(llvm::FoldingSetNodeID &ID) const { - ID.AddInteger((unsigned)getKind()); + ID.AddInteger(static_cast<unsigned>(getKind())); ID.AddPointer(getCodeRegion()); } void StringRegion::ProfileRegion(llvm::FoldingSetNodeID& ID, const StringLiteral* Str, const MemRegion* superRegion) { - ID.AddInteger((unsigned) StringRegionKind); + ID.AddInteger(static_cast<unsigned>(StringRegionKind)); ID.AddPointer(Str); ID.AddPointer(superRegion); } @@ -270,7 +266,7 @@ void StringRegion::ProfileRegion(llvm::FoldingSetNodeID& ID, void ObjCStringRegion::ProfileRegion(llvm::FoldingSetNodeID& ID, const ObjCStringLiteral* Str, const MemRegion* superRegion) { - ID.AddInteger((unsigned) ObjCStringRegionKind); + ID.AddInteger(static_cast<unsigned>(ObjCStringRegionKind)); ID.AddPointer(Str); ID.AddPointer(superRegion); } @@ -278,7 +274,7 @@ void ObjCStringRegion::ProfileRegion(llvm::FoldingSetNodeID& ID, void AllocaRegion::ProfileRegion(llvm::FoldingSetNodeID& ID, const Expr *Ex, unsigned cnt, const MemRegion *superRegion) { - ID.AddInteger((unsigned) AllocaRegionKind); + ID.AddInteger(static_cast<unsigned>(AllocaRegionKind)); ID.AddPointer(Ex); ID.AddInteger(cnt); ID.AddPointer(superRegion); @@ -295,7 +291,7 @@ void CompoundLiteralRegion::Profile(llvm::FoldingSetNodeID& ID) const { void CompoundLiteralRegion::ProfileRegion(llvm::FoldingSetNodeID& ID, const CompoundLiteralExpr *CL, const MemRegion* superRegion) { - ID.AddInteger((unsigned) CompoundLiteralRegionKind); + ID.AddInteger(static_cast<unsigned>(CompoundLiteralRegionKind)); ID.AddPointer(CL); ID.AddPointer(superRegion); } @@ -303,7 +299,7 @@ void CompoundLiteralRegion::ProfileRegion(llvm::FoldingSetNodeID& ID, void CXXThisRegion::ProfileRegion(llvm::FoldingSetNodeID &ID, const PointerType *PT, const MemRegion *sRegion) { - ID.AddInteger((unsigned) CXXThisRegionKind); + ID.AddInteger(static_cast<unsigned>(CXXThisRegionKind)); ID.AddPointer(PT); ID.AddPointer(sRegion); } @@ -320,7 +316,7 @@ void ObjCIvarRegion::ProfileRegion(llvm::FoldingSetNodeID& ID, void DeclRegion::ProfileRegion(llvm::FoldingSetNodeID& ID, const Decl *D, const MemRegion* superRegion, Kind k) { - ID.AddInteger((unsigned) k); + ID.AddInteger(static_cast<unsigned>(k)); ID.AddPointer(D); ID.AddPointer(superRegion); } @@ -335,7 +331,7 @@ void VarRegion::Profile(llvm::FoldingSetNodeID &ID) const { void SymbolicRegion::ProfileRegion(llvm::FoldingSetNodeID& ID, SymbolRef sym, const MemRegion *sreg) { - ID.AddInteger((unsigned) MemRegion::SymbolicRegionKind); + ID.AddInteger(static_cast<unsigned>(MemRegion::SymbolicRegionKind)); ID.Add(sym); ID.AddPointer(sreg); } @@ -438,7 +434,7 @@ void SubRegion::anchor() { } // Region pretty-printing. //===----------------------------------------------------------------------===// -void MemRegion::dump() const { +LLVM_DUMP_METHOD void MemRegion::dump() const { dumpToStream(llvm::errs()); } @@ -454,7 +450,7 @@ void MemRegion::dumpToStream(raw_ostream &os) const { } void AllocaRegion::dumpToStream(raw_ostream &os) const { - os << "alloca{" << (const void*) Ex << ',' << Cnt << '}'; + os << "alloca{" << static_cast<const void*>(Ex) << ',' << Cnt << '}'; } void FunctionCodeRegion::dumpToStream(raw_ostream &os) const { @@ -462,7 +458,7 @@ void FunctionCodeRegion::dumpToStream(raw_ostream &os) const { } void BlockCodeRegion::dumpToStream(raw_ostream &os) const { - os << "block_code{" << (const void*) this << '}'; + os << "block_code{" << static_cast<const void*>(this) << '}'; } void BlockDataRegion::dumpToStream(raw_ostream &os) const { @@ -478,12 +474,12 @@ void BlockDataRegion::dumpToStream(raw_ostream &os) const { void CompoundLiteralRegion::dumpToStream(raw_ostream &os) const { // FIXME: More elaborate pretty-printing. - os << "{ " << (const void*) CL << " }"; + os << "{ " << static_cast<const void*>(CL) << " }"; } void CXXTempObjectRegion::dumpToStream(raw_ostream &os) const { os << "temp_object{" << getValueType().getAsString() << ',' - << (const void*) Ex << '}'; + << static_cast<const void*>(Ex) << '}'; } void CXXBaseObjectRegion::dumpToStream(raw_ostream &os) const { @@ -525,7 +521,7 @@ void VarRegion::dumpToStream(raw_ostream &os) const { os << *cast<VarDecl>(D); } -void RegionRawOffset::dump() const { +LLVM_DUMP_METHOD void RegionRawOffset::dump() const { dumpToStream(llvm::errs()); } @@ -582,12 +578,10 @@ void MemRegion::printPretty(raw_ostream &os) const { os << "'"; printPrettyAsExpr(os); os << "'"; - return; } void MemRegion::printPrettyAsExpr(raw_ostream &os) const { llvm_unreachable("This region cannot be printed pretty."); - return; } bool VarRegion::canPrintPrettyAsExpr() const { @@ -628,7 +622,6 @@ void FieldRegion::printPretty(raw_ostream &os) const { } else { os << "field " << "\'" << getDecl()->getName() << "'"; } - return; } bool CXXBaseObjectRegion::canPrintPrettyAsExpr() const { @@ -639,6 +632,65 @@ void CXXBaseObjectRegion::printPrettyAsExpr(raw_ostream &os) const { superRegion->printPrettyAsExpr(os); } +std::string MemRegion::getDescriptiveName(bool UseQuotes) const { + std::string VariableName; + std::string ArrayIndices; + const MemRegion *R = this; + SmallString<50> buf; + llvm::raw_svector_ostream os(buf); + + // Obtain array indices to add them to the variable name. + const ElementRegion *ER = nullptr; + while ((ER = R->getAs<ElementRegion>())) { + // Index is a ConcreteInt. + if (auto CI = ER->getIndex().getAs<nonloc::ConcreteInt>()) { + llvm::SmallString<2> Idx; + CI->getValue().toString(Idx); + ArrayIndices = (llvm::Twine("[") + Idx.str() + "]" + ArrayIndices).str(); + } + // If not a ConcreteInt, try to obtain the variable + // name by calling 'getDescriptiveName' recursively. + else { + std::string Idx = ER->getDescriptiveName(false); + if (!Idx.empty()) { + ArrayIndices = (llvm::Twine("[") + Idx + "]" + ArrayIndices).str(); + } + } + R = ER->getSuperRegion(); + } + + // Get variable name. + if (R && R->canPrintPrettyAsExpr()) { + R->printPrettyAsExpr(os); + if (UseQuotes) { + return (llvm::Twine("'") + os.str() + ArrayIndices + "'").str(); + } else { + return (llvm::Twine(os.str()) + ArrayIndices).str(); + } + } + + return VariableName; +} + +SourceRange MemRegion::sourceRange() const { + const VarRegion *const VR = dyn_cast<VarRegion>(this->getBaseRegion()); + const FieldRegion *const FR = dyn_cast<FieldRegion>(this); + + // Check for more specific regions first. + // FieldRegion + if (FR) { + return FR->getDecl()->getSourceRange(); + } + // VarRegion + else if (VR) { + return VR->getDecl()->getSourceRange(); + } + // Return invalid source range (can be checked by client). + else { + return SourceRange{}; + } +} + //===----------------------------------------------------------------------===// // MemRegionManager methods. //===----------------------------------------------------------------------===// @@ -646,7 +698,7 @@ void CXXBaseObjectRegion::printPrettyAsExpr(raw_ostream &os) const { template <typename REG> const REG *MemRegionManager::LazyAllocate(REG*& region) { if (!region) { - region = (REG*) A.Allocate<REG>(); + region = A.Allocate<REG>(); new (region) REG(this); } @@ -656,7 +708,7 @@ const REG *MemRegionManager::LazyAllocate(REG*& region) { template <typename REG, typename ARG> const REG *MemRegionManager::LazyAllocate(REG*& region, ARG a) { if (!region) { - region = (REG*) A.Allocate<REG>(); + region = A.Allocate<REG>(); new (region) REG(this, a); } @@ -892,7 +944,6 @@ MemRegionManager::getCXXStaticTempObjectRegion(const Expr *Ex) { const CompoundLiteralRegion* MemRegionManager::getCompoundLiteralRegion(const CompoundLiteralExpr *CL, const LocationContext *LC) { - const MemRegion *sReg = nullptr; if (CL->isFileScope()) @@ -910,7 +961,6 @@ const ElementRegion* MemRegionManager::getElementRegion(QualType elementType, NonLoc Idx, const MemRegion* superRegion, ASTContext &Ctx){ - QualType T = Ctx.getCanonicalType(elementType).getUnqualifiedType(); llvm::FoldingSetNodeID ID; @@ -921,7 +971,7 @@ MemRegionManager::getElementRegion(QualType elementType, NonLoc Idx, ElementRegion* R = cast_or_null<ElementRegion>(data); if (!R) { - R = (ElementRegion*) A.Allocate<ElementRegion>(); + R = A.Allocate<ElementRegion>(); new (R) ElementRegion(T, Idx, superRegion); Regions.InsertNode(R, InsertPos); } @@ -1342,10 +1392,10 @@ RegionOffset MemRegion::getAsOffset() const { // Get the field number. unsigned idx = 0; for (RecordDecl::field_iterator FI = RD->field_begin(), - FE = RD->field_end(); FI != FE; ++FI, ++idx) + FE = RD->field_end(); FI != FE; ++FI, ++idx) { if (FR->getDecl() == *FI) break; - + } const ASTRecordLayout &Layout = getContext().getASTRecordLayout(RD); // This is offset in bits. Offset += Layout.getFieldOffset(idx); @@ -1406,9 +1456,9 @@ void BlockDataRegion::LazyInitializeReferencedVars() { BumpVectorContext BC(A); typedef BumpVector<const MemRegion*> VarVec; - VarVec *BV = (VarVec*) A.Allocate<VarVec>(); + VarVec *BV = A.Allocate<VarVec>(); new (BV) VarVec(BC, NumBlockVars); - VarVec *BVOriginal = (VarVec*) A.Allocate<VarVec>(); + VarVec *BVOriginal = A.Allocate<VarVec>(); new (BVOriginal) VarVec(BC, NumBlockVars); for (const VarDecl *VD : ReferencedBlockVars) { @@ -1488,7 +1538,7 @@ void RegionAndSymbolInvalidationTraits::setTrait(const MemRegion *MR, } bool RegionAndSymbolInvalidationTraits::hasTrait(SymbolRef Sym, - InvalidationKinds IK) { + InvalidationKinds IK) const { const_symbol_iterator I = SymTraitsMap.find(Sym); if (I != SymTraitsMap.end()) return I->second & IK; @@ -1497,7 +1547,7 @@ bool RegionAndSymbolInvalidationTraits::hasTrait(SymbolRef Sym, } bool RegionAndSymbolInvalidationTraits::hasTrait(const MemRegion *MR, - InvalidationKinds IK) { + InvalidationKinds IK) const { if (!MR) return false; diff --git a/lib/StaticAnalyzer/Core/PathDiagnostic.cpp b/lib/StaticAnalyzer/Core/PathDiagnostic.cpp index 504df30de834c..217d628a129c7 100644 --- a/lib/StaticAnalyzer/Core/PathDiagnostic.cpp +++ b/lib/StaticAnalyzer/Core/PathDiagnostic.cpp @@ -61,7 +61,6 @@ PathDiagnosticCallPiece::~PathDiagnosticCallPiece() {} PathDiagnosticControlFlowPiece::~PathDiagnosticControlFlowPiece() {} PathDiagnosticMacroPiece::~PathDiagnosticMacroPiece() {} - void PathPieces::flattenTo(PathPieces &Primary, PathPieces &Current, bool ShouldFlattenMacros) const { for (PathPieces::const_iterator I = begin(), E = end(); I != E; ++I) { @@ -102,7 +101,6 @@ void PathPieces::flattenTo(PathPieces &Primary, PathPieces &Current, } } - PathDiagnostic::~PathDiagnostic() {} PathDiagnostic::PathDiagnostic(StringRef CheckName, const Decl *declWithIssue, @@ -278,6 +276,7 @@ void PathDiagnosticConsumer::HandlePathDiagnostic( } static Optional<bool> comparePath(const PathPieces &X, const PathPieces &Y); + static Optional<bool> compareControlFlow(const PathDiagnosticControlFlowPiece &X, const PathDiagnosticControlFlowPiece &Y) { @@ -505,7 +504,6 @@ static SourceLocation getValidSourceLocation(const Stmt* S, // S might be a temporary statement that does not have a location in the // source code, so find an enclosing statement and use its location. if (!L.isValid()) { - AnalysisDeclContext *ADC; if (LAC.is<const LocationContext*>()) ADC = LAC.get<const LocationContext*>()->getAnalysisDeclContext(); @@ -578,22 +576,20 @@ getLocationForCaller(const StackFrameContext *SFC, llvm_unreachable("Unknown CFGElement kind"); } - PathDiagnosticLocation - PathDiagnosticLocation::createBegin(const Decl *D, - const SourceManager &SM) { +PathDiagnosticLocation::createBegin(const Decl *D, + const SourceManager &SM) { return PathDiagnosticLocation(D->getLocStart(), SM, SingleLocK); } PathDiagnosticLocation - PathDiagnosticLocation::createBegin(const Stmt *S, - const SourceManager &SM, - LocationOrAnalysisDeclContext LAC) { +PathDiagnosticLocation::createBegin(const Stmt *S, + const SourceManager &SM, + LocationOrAnalysisDeclContext LAC) { return PathDiagnosticLocation(getValidSourceLocation(S, LAC), SM, SingleLocK); } - PathDiagnosticLocation PathDiagnosticLocation::createEnd(const Stmt *S, const SourceManager &SM, @@ -605,13 +601,13 @@ PathDiagnosticLocation::createEnd(const Stmt *S, } PathDiagnosticLocation - PathDiagnosticLocation::createOperatorLoc(const BinaryOperator *BO, - const SourceManager &SM) { +PathDiagnosticLocation::createOperatorLoc(const BinaryOperator *BO, + const SourceManager &SM) { return PathDiagnosticLocation(BO->getOperatorLoc(), SM, SingleLocK); } PathDiagnosticLocation - PathDiagnosticLocation::createConditionalColonLoc( +PathDiagnosticLocation::createConditionalColonLoc( const ConditionalOperator *CO, const SourceManager &SM) { return PathDiagnosticLocation(CO->getColonLoc(), SM, SingleLocK); @@ -619,28 +615,28 @@ PathDiagnosticLocation PathDiagnosticLocation - PathDiagnosticLocation::createMemberLoc(const MemberExpr *ME, - const SourceManager &SM) { +PathDiagnosticLocation::createMemberLoc(const MemberExpr *ME, + const SourceManager &SM) { return PathDiagnosticLocation(ME->getMemberLoc(), SM, SingleLocK); } PathDiagnosticLocation - PathDiagnosticLocation::createBeginBrace(const CompoundStmt *CS, - const SourceManager &SM) { +PathDiagnosticLocation::createBeginBrace(const CompoundStmt *CS, + const SourceManager &SM) { SourceLocation L = CS->getLBracLoc(); return PathDiagnosticLocation(L, SM, SingleLocK); } PathDiagnosticLocation - PathDiagnosticLocation::createEndBrace(const CompoundStmt *CS, - const SourceManager &SM) { +PathDiagnosticLocation::createEndBrace(const CompoundStmt *CS, + const SourceManager &SM) { SourceLocation L = CS->getRBracLoc(); return PathDiagnosticLocation(L, SM, SingleLocK); } PathDiagnosticLocation - PathDiagnosticLocation::createDeclBegin(const LocationContext *LC, - const SourceManager &SM) { +PathDiagnosticLocation::createDeclBegin(const LocationContext *LC, + const SourceManager &SM) { // FIXME: Should handle CXXTryStmt if analyser starts supporting C++. if (const CompoundStmt *CS = dyn_cast_or_null<CompoundStmt>(LC->getDecl()->getBody())) @@ -653,16 +649,15 @@ PathDiagnosticLocation } PathDiagnosticLocation - PathDiagnosticLocation::createDeclEnd(const LocationContext *LC, - const SourceManager &SM) { +PathDiagnosticLocation::createDeclEnd(const LocationContext *LC, + const SourceManager &SM) { SourceLocation L = LC->getDecl()->getBodyRBrace(); return PathDiagnosticLocation(L, SM, SingleLocK); } PathDiagnosticLocation - PathDiagnosticLocation::create(const ProgramPoint& P, - const SourceManager &SMng) { - +PathDiagnosticLocation::create(const ProgramPoint& P, + const SourceManager &SMng) { const Stmt* S = nullptr; if (Optional<BlockEdge> BE = P.getAs<BlockEdge>()) { const CFGBlock *BSrc = BE->getSrc(); @@ -1062,7 +1057,6 @@ void PathDiagnosticLocation::Profile(llvm::FoldingSetNodeID &ID) const { ID.AddInteger(Range.getBegin().getRawEncoding()); ID.AddInteger(Range.getEnd().getRawEncoding()); ID.AddInteger(Loc.getRawEncoding()); - return; } void PathDiagnosticPiece::Profile(llvm::FoldingSetNodeID &ID) const { diff --git a/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp b/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp index 55e1222e0ac6c..8ad931acdf7f5 100644 --- a/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp +++ b/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp @@ -124,7 +124,7 @@ static void ReportControlFlow(raw_ostream &o, --indent; // Output any helper text. - const std::string& s = P.getString(); + const auto &s = P.getString(); if (!s.empty()) { Indent(o, indent) << "<key>alternate</key>"; EmitString(o, s) << '\n'; diff --git a/lib/StaticAnalyzer/Core/ProgramState.cpp b/lib/StaticAnalyzer/Core/ProgramState.cpp index 100fa75c5f421..adda7af08db8e 100644 --- a/lib/StaticAnalyzer/Core/ProgramState.cpp +++ b/lib/StaticAnalyzer/Core/ProgramState.cpp @@ -439,7 +439,7 @@ void ProgramState::printDOT(raw_ostream &Out) const { print(Out, "\\l", "\\|"); } -void ProgramState::dump() const { +LLVM_DUMP_METHOD void ProgramState::dump() const { print(llvm::errs()); } diff --git a/lib/StaticAnalyzer/Core/RegionStore.cpp b/lib/StaticAnalyzer/Core/RegionStore.cpp index a63f6e4962723..0d173c4644816 100644 --- a/lib/StaticAnalyzer/Core/RegionStore.cpp +++ b/lib/StaticAnalyzer/Core/RegionStore.cpp @@ -14,6 +14,7 @@ // parameters are created lazily. // //===----------------------------------------------------------------------===// + #include "clang/AST/Attr.h" #include "clang/AST/CharUnits.h" #include "clang/Analysis/Analyses/LiveVariables.h" @@ -29,6 +30,7 @@ #include "llvm/ADT/ImmutableMap.h" #include "llvm/ADT/Optional.h" #include "llvm/Support/raw_ostream.h" +#include <utility> using namespace clang; using namespace ento; @@ -665,10 +667,9 @@ protected: public: ClusterAnalysis(RegionStoreManager &rm, ProgramStateManager &StateMgr, - RegionBindingsRef b ) - : RM(rm), Ctx(StateMgr.getContext()), - svalBuilder(StateMgr.getSValBuilder()), - B(b) {} + RegionBindingsRef b) + : RM(rm), Ctx(StateMgr.getContext()), + svalBuilder(StateMgr.getSValBuilder()), B(std::move(b)) {} RegionBindingsRef getRegionBindings() const { return B; } @@ -1130,11 +1131,10 @@ void invalidateRegionsWorker::VisitCluster(const MemRegion *baseR, // Check offset is not symbolic and within array's boundaries. // Handles arrays of 0 elements and of 0-sized elements as well. if (!ROffset || - (ROffset && - ((*ROffset >= LowerOffset && *ROffset < UpperOffset) || - (UpperOverflow && - (*ROffset >= LowerOffset || *ROffset < UpperOffset)) || - (LowerOffset == UpperOffset && *ROffset == LowerOffset)))) { + ((*ROffset >= LowerOffset && *ROffset < UpperOffset) || + (UpperOverflow && + (*ROffset >= LowerOffset || *ROffset < UpperOffset)) || + (LowerOffset == UpperOffset && *ROffset == LowerOffset))) { B = B.removeBinding(I.getKey()); // Bound symbolic regions need to be invalidated for dead symbol // detection. diff --git a/lib/StaticAnalyzer/Core/SValBuilder.cpp b/lib/StaticAnalyzer/Core/SValBuilder.cpp index 18315225a99d5..72bcdd9ecb06b 100644 --- a/lib/StaticAnalyzer/Core/SValBuilder.cpp +++ b/lib/StaticAnalyzer/Core/SValBuilder.cpp @@ -367,6 +367,11 @@ SVal SValBuilder::evalBinOp(ProgramStateRef state, BinaryOperator::Opcode op, if (lhs.isUnknown() || rhs.isUnknown()) return UnknownVal(); + if (lhs.getAs<nonloc::LazyCompoundVal>() || + rhs.getAs<nonloc::LazyCompoundVal>()) { + return UnknownVal(); + } + if (Optional<Loc> LV = lhs.getAs<Loc>()) { if (Optional<Loc> RV = rhs.getAs<Loc>()) return evalBinOpLL(state, op, *LV, *RV, type); @@ -451,7 +456,7 @@ SVal SValBuilder::evalIntegralCast(ProgramStateRef state, SVal val, NonLoc FromVal = val.castAs<NonLoc>(); QualType CmpTy = getConditionType(); NonLoc CompVal = - evalBinOpNN(state, BO_LT, FromVal, ToTypeMaxVal, CmpTy).castAs<NonLoc>(); + evalBinOpNN(state, BO_LE, FromVal, ToTypeMaxVal, CmpTy).castAs<NonLoc>(); ProgramStateRef IsNotTruncated, IsTruncated; std::tie(IsNotTruncated, IsTruncated) = state->assume(CompVal); if (!IsNotTruncated && IsTruncated) { diff --git a/lib/StaticAnalyzer/Core/SVals.cpp b/lib/StaticAnalyzer/Core/SVals.cpp index dffee6c8c57be..a30beed688b7a 100644 --- a/lib/StaticAnalyzer/Core/SVals.cpp +++ b/lib/StaticAnalyzer/Core/SVals.cpp @@ -236,7 +236,7 @@ SVal loc::ConcreteInt::evalBinOp(BasicValueFactory& BasicVals, // Pretty-Printing. //===----------------------------------------------------------------------===// -void SVal::dump() const { dumpToStream(llvm::errs()); } +LLVM_DUMP_METHOD void SVal::dump() const { dumpToStream(llvm::errs()); } void SVal::dumpToStream(raw_ostream &os) const { switch (getBaseKind()) { diff --git a/lib/StaticAnalyzer/Core/SymbolManager.cpp b/lib/StaticAnalyzer/Core/SymbolManager.cpp index 2dd252c223fda..b8b4af1179e52 100644 --- a/lib/StaticAnalyzer/Core/SymbolManager.cpp +++ b/lib/StaticAnalyzer/Core/SymbolManager.cpp @@ -23,7 +23,7 @@ using namespace ento; void SymExpr::anchor() { } -void SymExpr::dump() const { +LLVM_DUMP_METHOD void SymExpr::dump() const { dumpToStream(llvm::errs()); } diff --git a/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp b/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp index d1446855e01f3..8ac229fc65837 100644 --- a/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp +++ b/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp @@ -14,11 +14,11 @@ #include "clang/StaticAnalyzer/Frontend/AnalysisConsumer.h" #include "ModelInjector.h" #include "clang/AST/ASTConsumer.h" -#include "clang/AST/RecursiveASTVisitor.h" #include "clang/AST/Decl.h" #include "clang/AST/DeclCXX.h" #include "clang/AST/DeclObjC.h" #include "clang/AST/ParentMap.h" +#include "clang/AST/RecursiveASTVisitor.h" #include "clang/Analysis/Analyses/LiveVariables.h" #include "clang/Analysis/CFG.h" #include "clang/Analysis/CallGraph.h" @@ -47,10 +47,10 @@ #include "llvm/Support/raw_ostream.h" #include <memory> #include <queue> +#include <utility> using namespace clang; using namespace ento; -using llvm::SmallPtrSet; #define DEBUG_TYPE "AnalysisConsumer" @@ -185,13 +185,12 @@ public: /// translation unit. FunctionSummariesTy FunctionSummaries; - AnalysisConsumer(const Preprocessor& pp, - const std::string& outdir, - AnalyzerOptionsRef opts, - ArrayRef<std::string> plugins, + AnalysisConsumer(const Preprocessor &pp, const std::string &outdir, + AnalyzerOptionsRef opts, ArrayRef<std::string> plugins, CodeInjector *injector) - : RecVisitorMode(0), RecVisitorBR(nullptr), Ctx(nullptr), PP(pp), - OutDir(outdir), Opts(opts), Plugins(plugins), Injector(injector) { + : RecVisitorMode(0), RecVisitorBR(nullptr), Ctx(nullptr), PP(pp), + OutDir(outdir), Opts(std::move(opts)), Plugins(plugins), + Injector(injector) { DigestAnalyzerOptions(); if (Opts->PrintStats) { llvm::EnableStatistics(); @@ -274,7 +273,7 @@ public: llvm::errs() << ": " << Loc.getFilename(); if (isa<FunctionDecl>(D) || isa<ObjCMethodDecl>(D)) { const NamedDecl *ND = cast<NamedDecl>(D); - llvm::errs() << ' ' << *ND << '\n'; + llvm::errs() << ' ' << ND->getQualifiedNameAsString() << '\n'; } else if (isa<BlockDecl>(D)) { llvm::errs() << ' ' << "block(line:" << Loc.getLine() << ",col:" @@ -799,10 +798,7 @@ UbigraphViz::~UbigraphViz() { std::string Ubiviz; if (auto Path = llvm::sys::findProgramByName("ubiviz")) Ubiviz = *Path; - std::vector<const char*> args; - args.push_back(Ubiviz.c_str()); - args.push_back(Filename.c_str()); - args.push_back(nullptr); + const char *args[] = {Ubiviz.c_str(), Filename.c_str(), nullptr}; if (llvm::sys::ExecuteAndWait(Ubiviz, &args[0], nullptr, nullptr, 0, 0, &ErrMsg)) { diff --git a/lib/StaticAnalyzer/Frontend/Makefile b/lib/StaticAnalyzer/Frontend/Makefile deleted file mode 100644 index 3f15988bfddbb..0000000000000 --- a/lib/StaticAnalyzer/Frontend/Makefile +++ /dev/null @@ -1,19 +0,0 @@ -##===- clang/lib/StaticAnalyzer/Frontend/Makefile ----------*- Makefile -*-===## -# -# The LLVM Compiler Infrastructure -# -# This file is distributed under the University of Illinois Open Source -# License. See LICENSE.TXT for details. -# -##===----------------------------------------------------------------------===## -# -# Starting point into the static analyzer land for the driver. -# -##===----------------------------------------------------------------------===## - -CLANG_LEVEL := ../../.. -LIBRARYNAME := clangStaticAnalyzerFrontend - -CPP.Flags += -I${PROJ_OBJ_DIR}/../Checkers - -include $(CLANG_LEVEL)/Makefile diff --git a/lib/StaticAnalyzer/Makefile b/lib/StaticAnalyzer/Makefile deleted file mode 100644 index c166f063f9259..0000000000000 --- a/lib/StaticAnalyzer/Makefile +++ /dev/null @@ -1,18 +0,0 @@ -##===- clang/lib/StaticAnalyzer/Makefile -------------------*- Makefile -*-===## -# -# The LLVM Compiler Infrastructure -# -# This file is distributed under the University of Illinois Open Source -# License. See LICENSE.TXT for details. -# -##===----------------------------------------------------------------------===## -# -# This implements analyses built on top of source-level CFGs. -# -##===----------------------------------------------------------------------===## - -CLANG_LEVEL := ../.. -DIRS := Checkers Frontend -PARALLEL_DIRS := Core - -include $(CLANG_LEVEL)/Makefile |