diff options
Diffstat (limited to 'lib/StaticAnalyzer/Checkers')
58 files changed, 2292 insertions, 967 deletions
diff --git a/lib/StaticAnalyzer/Checkers/AdjustedReturnValueChecker.cpp b/lib/StaticAnalyzer/Checkers/AdjustedReturnValueChecker.cpp deleted file mode 100644 index 84ea8c7..0000000 --- a/lib/StaticAnalyzer/Checkers/AdjustedReturnValueChecker.cpp +++ /dev/null @@ -1,92 +0,0 @@ -//== AdjustedReturnValueChecker.cpp -----------------------------*- C++ -*--==// -// -// The LLVM Compiler Infrastructure -// -// This file is distributed under the University of Illinois Open Source -// License. See LICENSE.TXT for details. -// -//===----------------------------------------------------------------------===// -// -// This file defines AdjustedReturnValueChecker, a simple check to see if the -// return value of a function call is different than the one the caller thinks -// it is. -// -//===----------------------------------------------------------------------===// - -#include "ClangSACheckers.h" -#include "clang/StaticAnalyzer/Core/Checker.h" -#include "clang/StaticAnalyzer/Core/CheckerManager.h" -#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" -#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h" - -using namespace clang; -using namespace ento; - -namespace { -class AdjustedReturnValueChecker : - public Checker< check::PostStmt<CallExpr> > { -public: - void checkPostStmt(const CallExpr *CE, CheckerContext &C) const; -}; -} - -void AdjustedReturnValueChecker::checkPostStmt(const CallExpr *CE, - CheckerContext &C) const { - - // Get the result type of the call. - QualType expectedResultTy = CE->getType(); - - // Fetch the signature of the called function. - ProgramStateRef state = C.getState(); - const LocationContext *LCtx = C.getLocationContext(); - - SVal V = state->getSVal(CE, LCtx); - - if (V.isUnknown()) - return; - - // Casting to void? Discard the value. - if (expectedResultTy->isVoidType()) { - C.addTransition(state->BindExpr(CE, LCtx, UnknownVal())); - return; - } - - const MemRegion *callee = state->getSVal(CE->getCallee(), LCtx).getAsRegion(); - if (!callee) - return; - - QualType actualResultTy; - - if (const FunctionTextRegion *FT = dyn_cast<FunctionTextRegion>(callee)) { - const FunctionDecl *FD = FT->getDecl(); - actualResultTy = FD->getResultType(); - } - else if (const BlockDataRegion *BD = dyn_cast<BlockDataRegion>(callee)) { - const BlockTextRegion *BR = BD->getCodeRegion(); - const BlockPointerType *BT=BR->getLocationType()->getAs<BlockPointerType>(); - const FunctionType *FT = BT->getPointeeType()->getAs<FunctionType>(); - actualResultTy = FT->getResultType(); - } - - // Can this happen? - if (actualResultTy.isNull()) - return; - - // For now, ignore references. - if (actualResultTy->getAs<ReferenceType>()) - return; - - - // Are they the same? - if (expectedResultTy != actualResultTy) { - // FIXME: Do more checking and actual emit an error. At least performing - // the cast avoids some assertion failures elsewhere. - SValBuilder &svalBuilder = C.getSValBuilder(); - V = svalBuilder.evalCast(V, expectedResultTy, actualResultTy); - C.addTransition(state->BindExpr(CE, LCtx, V)); - } -} - -void ento::registerAdjustedReturnValueChecker(CheckerManager &mgr) { - mgr.registerChecker<AdjustedReturnValueChecker>(); -} diff --git a/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp b/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp index b2ad184..535d8ee 100644 --- a/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp @@ -78,7 +78,7 @@ void ArrayBoundChecker::checkLocation(SVal l, bool isLoad, const Stmt* LoadS, new BugReport(*BT, BT->getDescription(), N); report->addRange(LoadS->getSourceRange()); - C.EmitReport(report); + C.emitReport(report); return; } diff --git a/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp b/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp index c6efe94..457c870 100644 --- a/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp +++ b/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp @@ -208,7 +208,7 @@ void ArrayBoundCheckerV2::reportOOB(CheckerContext &checkerContext, break; } - checkerContext.EmitReport(new BugReport(*BT, os.str(), errorNode)); + checkerContext.emitReport(new BugReport(*BT, os.str(), errorNode)); } void RegionRawOffsetV2::dump() const { diff --git a/lib/StaticAnalyzer/Checkers/AttrNonNullChecker.cpp b/lib/StaticAnalyzer/Checkers/AttrNonNullChecker.cpp index c582cfc..81e8dd8 100644 --- a/lib/StaticAnalyzer/Checkers/AttrNonNullChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/AttrNonNullChecker.cpp @@ -105,9 +105,9 @@ void AttrNonNullChecker::checkPreCall(const CallEvent &Call, // Highlight the range of the argument that was null. R->addRange(Call.getArgSourceRange(idx)); if (const Expr *ArgE = Call.getArgExpr(idx)) - bugreporter::addTrackNullOrUndefValueVisitor(errorNode, ArgE, R); + bugreporter::trackNullOrUndefValue(errorNode, ArgE, *R); // Emit the bug report. - C.EmitReport(R); + C.emitReport(R); } // Always return. Either we cached out or we just emitted an error. diff --git a/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp b/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp index 955e79a..eba534e 100644 --- a/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp +++ b/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp @@ -117,7 +117,7 @@ void NilArgChecker::WarnNilArg(CheckerContext &C, BugReport *R = new BugReport(*BT, os.str(), N); R->addRange(msg.getArgSourceRange(Arg)); - C.EmitReport(R); + C.emitReport(R); } } @@ -358,20 +358,20 @@ void CFNumberCreateChecker::checkPreStmt(const CallExpr *CE, BugReport *report = new BugReport(*BT, os.str(), N); report->addRange(CE->getArg(2)->getSourceRange()); - C.EmitReport(report); + C.emitReport(report); } } //===----------------------------------------------------------------------===// -// CFRetain/CFRelease checking for null arguments. +// CFRetain/CFRelease/CFMakeCollectable checking for null arguments. //===----------------------------------------------------------------------===// namespace { class CFRetainReleaseChecker : public Checker< check::PreStmt<CallExpr> > { mutable OwningPtr<APIMisuse> BT; - mutable IdentifierInfo *Retain, *Release; + mutable IdentifierInfo *Retain, *Release, *MakeCollectable; public: - CFRetainReleaseChecker(): Retain(0), Release(0) {} + CFRetainReleaseChecker(): Retain(0), Release(0), MakeCollectable(0) {} void checkPreStmt(const CallExpr *CE, CheckerContext &C) const; }; } // end anonymous namespace @@ -392,12 +392,14 @@ void CFRetainReleaseChecker::checkPreStmt(const CallExpr *CE, ASTContext &Ctx = C.getASTContext(); Retain = &Ctx.Idents.get("CFRetain"); Release = &Ctx.Idents.get("CFRelease"); - BT.reset(new APIMisuse("null passed to CFRetain/CFRelease")); + MakeCollectable = &Ctx.Idents.get("CFMakeCollectable"); + BT.reset( + new APIMisuse("null passed to CFRetain/CFRelease/CFMakeCollectable")); } - // Check if we called CFRetain/CFRelease. + // Check if we called CFRetain/CFRelease/CFMakeCollectable. const IdentifierInfo *FuncII = FD->getIdentifier(); - if (!(FuncII == Retain || FuncII == Release)) + if (!(FuncII == Retain || FuncII == Release || FuncII == MakeCollectable)) return; // FIXME: The rest of this just checks that the argument is non-null. @@ -426,14 +428,20 @@ void CFRetainReleaseChecker::checkPreStmt(const CallExpr *CE, if (!N) return; - const char *description = (FuncII == Retain) - ? "Null pointer argument in call to CFRetain" - : "Null pointer argument in call to CFRelease"; + const char *description; + if (FuncII == Retain) + description = "Null pointer argument in call to CFRetain"; + else if (FuncII == Release) + description = "Null pointer argument in call to CFRelease"; + else if (FuncII == MakeCollectable) + description = "Null pointer argument in call to CFMakeCollectable"; + else + llvm_unreachable("impossible case"); BugReport *report = new BugReport(*BT, description, N); report->addRange(Arg->getSourceRange()); - bugreporter::addTrackNullOrUndefValueVisitor(N, Arg, report); - C.EmitReport(report); + bugreporter::trackNullOrUndefValue(N, Arg, *report); + C.emitReport(report); return; } @@ -491,7 +499,7 @@ void ClassReleaseChecker::checkPreObjCMessage(const ObjCMethodCall &msg, BugReport *report = new BugReport(*BT, os.str(), N); report->addRange(msg.getSourceRange()); - C.EmitReport(report); + C.emitReport(report); } } @@ -644,7 +652,7 @@ void VariadicMethodTypeChecker::checkPreObjCMessage(const ObjCMethodCall &msg, BugReport *R = new BugReport(*BT, os.str(), errorNode.getValue()); R->addRange(msg.getArgSourceRange(I)); - C.EmitReport(R); + C.emitReport(R); } } @@ -716,6 +724,73 @@ void ObjCLoopChecker::checkPostStmt(const ObjCForCollectionStmt *FCS, C.addTransition(State); } +namespace { +/// \class ObjCNonNilReturnValueChecker +/// \brief The checker restricts the return values of APIs known to +/// never (or almost never) return 'nil'. +class ObjCNonNilReturnValueChecker + : public Checker<check::PostObjCMessage> { + mutable bool Initialized; + mutable Selector ObjectAtIndex; + mutable Selector ObjectAtIndexedSubscript; + +public: + ObjCNonNilReturnValueChecker() : Initialized(false) {} + void checkPostObjCMessage(const ObjCMethodCall &M, CheckerContext &C) const; +}; +} + +static ProgramStateRef assumeExprIsNonNull(const Expr *NonNullExpr, + ProgramStateRef State, + CheckerContext &C) { + SVal Val = State->getSVal(NonNullExpr, C.getLocationContext()); + if (DefinedOrUnknownSVal *DV = dyn_cast<DefinedOrUnknownSVal>(&Val)) + return State->assume(*DV, true); + return State; +} + +void ObjCNonNilReturnValueChecker::checkPostObjCMessage(const ObjCMethodCall &M, + CheckerContext &C) + const { + ProgramStateRef State = C.getState(); + + if (!Initialized) { + ASTContext &Ctx = C.getASTContext(); + ObjectAtIndex = GetUnarySelector("objectAtIndex", Ctx); + ObjectAtIndexedSubscript = GetUnarySelector("objectAtIndexedSubscript", Ctx); + } + + // Check the receiver type. + if (const ObjCInterfaceDecl *Interface = M.getReceiverInterface()) { + + // Assume that object returned from '[self init]' or '[super init]' is not + // 'nil' if we are processing an inlined function/method. + // + // A defensive callee will (and should) check if the object returned by + // '[super init]' is 'nil' before doing it's own initialization. However, + // since 'nil' is rarely returned in practice, we should not warn when the + // caller to the defensive constructor uses the object in contexts where + // 'nil' is not accepted. + if (!C.inTopFrame() && M.getDecl() && + M.getDecl()->getMethodFamily() == OMF_init && + M.isReceiverSelfOrSuper()) { + State = assumeExprIsNonNull(M.getOriginExpr(), State, C); + } + + // Objects returned from + // [NSArray|NSOrderedSet]::[ObjectAtIndex|ObjectAtIndexedSubscript] + // are never 'nil'. + FoundationClass Cl = findKnownClass(Interface); + if (Cl == FC_NSArray || Cl == FC_NSOrderedSet) { + Selector Sel = M.getSelector(); + if (Sel == ObjectAtIndex || Sel == ObjectAtIndexedSubscript) { + // Go ahead and assume the value is non-nil. + State = assumeExprIsNonNull(M.getOriginExpr(), State, C); + } + } + } + C.addTransition(State); +} //===----------------------------------------------------------------------===// // Check registration. @@ -744,3 +819,7 @@ void ento::registerVariadicMethodTypeChecker(CheckerManager &mgr) { void ento::registerObjCLoopChecker(CheckerManager &mgr) { mgr.registerChecker<ObjCLoopChecker>(); } + +void ento::registerObjCNonNilReturnValueChecker(CheckerManager &mgr) { + mgr.registerChecker<ObjCNonNilReturnValueChecker>(); +} diff --git a/lib/StaticAnalyzer/Checkers/BoolAssignmentChecker.cpp b/lib/StaticAnalyzer/Checkers/BoolAssignmentChecker.cpp index a4fc396..92edefe 100644 --- a/lib/StaticAnalyzer/Checkers/BoolAssignmentChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/BoolAssignmentChecker.cpp @@ -35,7 +35,7 @@ void BoolAssignmentChecker::emitReport(ProgramStateRef state, if (ExplodedNode *N = C.addTransition(state)) { if (!BT) BT.reset(new BuiltinBug("Assignment of a non-Boolean value")); - C.EmitReport(new BugReport(*BT, BT->getDescription(), N)); + C.emitReport(new BugReport(*BT, BT->getDescription(), N)); } } diff --git a/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp b/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp index 509bc79..6ef022b 100644 --- a/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp @@ -55,7 +55,7 @@ bool BuiltinFunctionChecker::evalCall(const CallExpr *CE, // FIXME: Refactor into StoreManager itself? MemRegionManager& RM = C.getStoreManager().getRegionManager(); const AllocaRegion* R = - RM.getAllocaRegion(CE, C.getCurrentBlockCount(), C.getLocationContext()); + RM.getAllocaRegion(CE, C.blockCount(), C.getLocationContext()); // Set the extent of the region in bytes. This enables us to use the // SVal of the argument directly. If we save the extent in bits, we diff --git a/lib/StaticAnalyzer/Checkers/CMakeLists.txt b/lib/StaticAnalyzer/Checkers/CMakeLists.txt index 7fe51d3..8e455de 100644 --- a/lib/StaticAnalyzer/Checkers/CMakeLists.txt +++ b/lib/StaticAnalyzer/Checkers/CMakeLists.txt @@ -4,7 +4,6 @@ clang_tablegen(Checkers.inc -gen-clang-sa-checkers TARGET ClangSACheckers) add_clang_library(clangStaticAnalyzerCheckers - AdjustedReturnValueChecker.cpp AnalyzerStatsChecker.cpp ArrayBoundChecker.cpp ArrayBoundCheckerV2.cpp @@ -28,12 +27,15 @@ add_clang_library(clangStaticAnalyzerCheckers DeadStoresChecker.cpp DebugCheckers.cpp DereferenceChecker.cpp + DirectIvarAssignment.cpp DivZeroChecker.cpp DynamicTypePropagation.cpp ExprInspectionChecker.cpp + SimpleStreamChecker.cpp FixedAddressChecker.cpp GenericTaintChecker.cpp IdempotentOperationChecker.cpp + IvarInvalidationChecker.cpp LLVMConventionsChecker.cpp MacOSKeychainAPIChecker.cpp MacOSXAPIChecker.cpp @@ -43,10 +45,10 @@ add_clang_library(clangStaticAnalyzerCheckers NSAutoreleasePoolChecker.cpp NSErrorChecker.cpp NoReturnFunctionChecker.cpp - OSAtomicChecker.cpp ObjCAtSyncChecker.cpp ObjCContainersASTChecker.cpp ObjCContainersChecker.cpp + ObjCMissingSuperCallChecker.cpp ObjCSelfInitChecker.cpp ObjCUnusedIVarsChecker.cpp PointerArithChecker.cpp diff --git a/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/lib/StaticAnalyzer/Checkers/CStringChecker.cpp index 483082a..eae9ddf 100644 --- a/lib/StaticAnalyzer/Checkers/CStringChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/CStringChecker.cpp @@ -188,21 +188,9 @@ public: NonLoc right) const; }; -class CStringLength { -public: - typedef llvm::ImmutableMap<const MemRegion *, SVal> EntryMap; -}; } //end anonymous namespace -namespace clang { -namespace ento { - template <> - struct ProgramStateTrait<CStringLength> - : public ProgramStatePartialTrait<CStringLength::EntryMap> { - static void *GDMIndex() { return CStringChecker::getTag(); } - }; -} -} +REGISTER_MAP_WITH_PROGRAMSTATE(CStringLength, const MemRegion *, SVal) //===----------------------------------------------------------------------===// // Individual checks and utility methods. @@ -252,8 +240,8 @@ ProgramStateRef CStringChecker::checkNonNull(CheckerContext &C, BugReport *report = new BugReport(*BT, os.str(), N); report->addRange(S->getSourceRange()); - bugreporter::addTrackNullOrUndefValueVisitor(N, S, report); - C.EmitReport(report); + bugreporter::trackNullOrUndefValue(N, S, *report); + C.emitReport(report); return NULL; } @@ -327,7 +315,7 @@ ProgramStateRef CStringChecker::CheckLocation(CheckerContext &C, // reference is outside the range. report->addRange(S->getSourceRange()); - C.EmitReport(report); + C.emitReport(report); return NULL; } @@ -544,7 +532,7 @@ void CStringChecker::emitOverlapBug(CheckerContext &C, ProgramStateRef state, report->addRange(First->getSourceRange()); report->addRange(Second->getSourceRange()); - C.EmitReport(report); + C.emitReport(report); } ProgramStateRef CStringChecker::checkAdditionOverflow(CheckerContext &C, @@ -607,7 +595,7 @@ ProgramStateRef CStringChecker::checkAdditionOverflow(CheckerContext &C, // Generate a report for this bug. BugReport *report = new BugReport(*BT_AdditionOverflow, warning, N); - C.EmitReport(report); + C.emitReport(report); return NULL; } @@ -673,11 +661,11 @@ SVal CStringChecker::getCStringLengthForRegion(CheckerContext &C, } // Otherwise, get a new symbol and update the state. - unsigned Count = C.getCurrentBlockCount(); SValBuilder &svalBuilder = C.getSValBuilder(); QualType sizeTy = svalBuilder.getContext().getSizeType(); SVal strLength = svalBuilder.getMetadataSymbolVal(CStringChecker::getTag(), - MR, Ex, sizeTy, Count); + MR, Ex, sizeTy, + C.blockCount()); if (!hypothetical) state = state->set<CStringLength>(MR, strLength); @@ -714,7 +702,7 @@ SVal CStringChecker::getCStringLength(CheckerContext &C, ProgramStateRef &state, os.str(), N); report->addRange(Ex->getSourceRange()); - C.EmitReport(report); + C.emitReport(report); } return UndefinedVal(); @@ -778,7 +766,7 @@ SVal CStringChecker::getCStringLength(CheckerContext &C, ProgramStateRef &state, os.str(), N); report->addRange(Ex->getSourceRange()); - C.EmitReport(report); + C.emitReport(report); } return UndefinedVal(); @@ -826,15 +814,14 @@ ProgramStateRef CStringChecker::InvalidateBuffer(CheckerContext &C, } // Invalidate this region. - unsigned Count = C.getCurrentBlockCount(); const LocationContext *LCtx = C.getPredecessor()->getLocationContext(); - return state->invalidateRegions(R, E, Count, LCtx); + return state->invalidateRegions(R, E, C.blockCount(), LCtx); } // If we have a non-region value by chance, just remove the binding. // FIXME: is this necessary or correct? This handles the non-Region // cases. Is it ever valid to store to these? - return state->unbindLoc(*L); + return state->killBinding(*L); } bool CStringChecker::SummarizeRegion(raw_ostream &os, ASTContext &Ctx, @@ -843,7 +830,7 @@ bool CStringChecker::SummarizeRegion(raw_ostream &os, ASTContext &Ctx, switch (MR->getKind()) { case MemRegion::FunctionTextRegionKind: { - const FunctionDecl *FD = cast<FunctionTextRegion>(MR)->getDecl(); + const NamedDecl *FD = cast<FunctionTextRegion>(MR)->getDecl(); if (FD) os << "the address of the function '" << *FD << '\''; else @@ -957,9 +944,8 @@ void CStringChecker::evalCopyCommon(CheckerContext &C, } else { // If we don't know how much we copied, we can at least // conjure a return value for later. - unsigned Count = C.getCurrentBlockCount(); - SVal result = - C.getSValBuilder().getConjuredSymbolVal(NULL, CE, LCtx, Count); + SVal result = C.getSValBuilder().conjureSymbolVal(0, CE, LCtx, + C.blockCount()); state = state->BindExpr(CE, LCtx, result); } @@ -1093,8 +1079,7 @@ void CStringChecker::evalMemcmp(CheckerContext &C, const CallExpr *CE) const { state = CheckBufferAccess(C, state, Size, Left, Right); if (state) { // The return value is the comparison result, which we don't know. - unsigned Count = C.getCurrentBlockCount(); - SVal CmpV = svalBuilder.getConjuredSymbolVal(NULL, CE, LCtx, Count); + SVal CmpV = svalBuilder.conjureSymbolVal(0, CE, LCtx, C.blockCount()); state = state->BindExpr(CE, LCtx, CmpV); C.addTransition(state); } @@ -1206,8 +1191,7 @@ void CStringChecker::evalstrLengthCommon(CheckerContext &C, const CallExpr *CE, // no guarantee the full string length will actually be returned. // All we know is the return value is the min of the string length // and the limit. This is better than nothing. - unsigned Count = C.getCurrentBlockCount(); - result = C.getSValBuilder().getConjuredSymbolVal(NULL, CE, LCtx, Count); + result = C.getSValBuilder().conjureSymbolVal(0, CE, LCtx, C.blockCount()); NonLoc *resultNL = cast<NonLoc>(&result); if (strLengthNL) { @@ -1234,8 +1218,7 @@ void CStringChecker::evalstrLengthCommon(CheckerContext &C, const CallExpr *CE, // If we don't know the length of the string, conjure a return // value, so it can be used in constraints, at least. if (result.isUnknown()) { - unsigned Count = C.getCurrentBlockCount(); - result = C.getSValBuilder().getConjuredSymbolVal(NULL, CE, LCtx, Count); + result = C.getSValBuilder().conjureSymbolVal(0, CE, LCtx, C.blockCount()); } } @@ -1612,8 +1595,7 @@ void CStringChecker::evalStrcpyCommon(CheckerContext &C, const CallExpr *CE, // If this is a stpcpy-style copy, but we were unable to check for a buffer // overflow, we still need a result. Conjure a return value. if (returnEnd && Result.isUnknown()) { - unsigned Count = C.getCurrentBlockCount(); - Result = svalBuilder.getConjuredSymbolVal(NULL, CE, LCtx, Count); + Result = svalBuilder.conjureSymbolVal(0, CE, LCtx, C.blockCount()); } // Set the return value. @@ -1770,8 +1752,7 @@ void CStringChecker::evalStrcmpCommon(CheckerContext &C, const CallExpr *CE, if (!canComputeResult) { // Conjure a symbolic value. It's the best we can do. - unsigned Count = C.getCurrentBlockCount(); - SVal resultVal = svalBuilder.getConjuredSymbolVal(NULL, CE, LCtx, Count); + SVal resultVal = svalBuilder.conjureSymbolVal(0, CE, LCtx, C.blockCount()); state = state->BindExpr(CE, LCtx, resultVal); } @@ -1885,7 +1866,7 @@ void CStringChecker::checkPreStmt(const DeclStmt *DS, CheckerContext &C) const { } bool CStringChecker::wantsRegionChangeUpdate(ProgramStateRef state) const { - CStringLength::EntryMap Entries = state->get<CStringLength>(); + CStringLengthTy Entries = state->get<CStringLength>(); return !Entries.isEmpty(); } @@ -1895,7 +1876,7 @@ CStringChecker::checkRegionChanges(ProgramStateRef state, ArrayRef<const MemRegion *> ExplicitRegions, ArrayRef<const MemRegion *> Regions, const CallEvent *Call) const { - CStringLength::EntryMap Entries = state->get<CStringLength>(); + CStringLengthTy Entries = state->get<CStringLength>(); if (Entries.isEmpty()) return state; @@ -1915,10 +1896,10 @@ CStringChecker::checkRegionChanges(ProgramStateRef state, } } - CStringLength::EntryMap::Factory &F = state->get_context<CStringLength>(); + CStringLengthTy::Factory &F = state->get_context<CStringLength>(); // Then loop over the entries in the current state. - for (CStringLength::EntryMap::iterator I = Entries.begin(), + for (CStringLengthTy::iterator I = Entries.begin(), E = Entries.end(); I != E; ++I) { const MemRegion *MR = I.getKey(); @@ -1945,9 +1926,9 @@ CStringChecker::checkRegionChanges(ProgramStateRef state, void CStringChecker::checkLiveSymbols(ProgramStateRef state, SymbolReaper &SR) const { // Mark all symbols in our string length map as valid. - CStringLength::EntryMap Entries = state->get<CStringLength>(); + CStringLengthTy Entries = state->get<CStringLength>(); - for (CStringLength::EntryMap::iterator I = Entries.begin(), E = Entries.end(); + for (CStringLengthTy::iterator I = Entries.begin(), E = Entries.end(); I != E; ++I) { SVal Len = I.getData(); @@ -1963,12 +1944,12 @@ void CStringChecker::checkDeadSymbols(SymbolReaper &SR, return; ProgramStateRef state = C.getState(); - CStringLength::EntryMap Entries = state->get<CStringLength>(); + CStringLengthTy Entries = state->get<CStringLength>(); if (Entries.isEmpty()) return; - CStringLength::EntryMap::Factory &F = state->get_context<CStringLength>(); - for (CStringLength::EntryMap::iterator I = Entries.begin(), E = Entries.end(); + CStringLengthTy::Factory &F = state->get_context<CStringLength>(); + for (CStringLengthTy::iterator I = Entries.begin(), E = Entries.end(); I != E; ++I) { SVal Len = I.getData(); if (SymbolRef Sym = Len.getAsSymbol()) { diff --git a/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp b/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp index befc935..f1a3aac 100644 --- a/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp @@ -33,7 +33,6 @@ namespace { class WalkAST: public StmtVisitor<WalkAST> { BugReporter &BR; AnalysisDeclContext* AC; - ASTContext &ASTC; /// Check if two expressions refer to the same declaration. inline bool sameDecl(const Expr *A1, const Expr *A2) { @@ -58,8 +57,8 @@ class WalkAST: public StmtVisitor<WalkAST> { const FunctionDecl *FD = CE->getDirectCallee(); if (!FD) return false; - return (CheckerContext::isCLibraryFunction(FD, "strlen", ASTC) - && sameDecl(CE->getArg(0), WithArg)); + return (CheckerContext::isCLibraryFunction(FD, "strlen") && + sameDecl(CE->getArg(0), WithArg)); } return false; } @@ -83,7 +82,7 @@ class WalkAST: public StmtVisitor<WalkAST> { public: WalkAST(BugReporter &br, AnalysisDeclContext* ac) : - BR(br), AC(ac), ASTC(AC->getASTContext()) { + BR(br), AC(ac) { } // Statement visitor methods. @@ -136,7 +135,7 @@ void WalkAST::VisitCallExpr(CallExpr *CE) { if (!FD) return; - if (CheckerContext::isCLibraryFunction(FD, "strncat", ASTC)) { + if (CheckerContext::isCLibraryFunction(FD, "strncat")) { if (containsBadStrncatPattern(CE)) { const Expr *DstArg = CE->getArg(0); const Expr *LenArg = CE->getArg(2); diff --git a/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp b/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp index 5edcf09..82bc136 100644 --- a/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp @@ -75,13 +75,13 @@ void CallAndMessageChecker::emitBadCall(BugType *BT, CheckerContext &C, BugReport *R = new BugReport(*BT, BT->getName(), N); if (BadE) { R->addRange(BadE->getSourceRange()); - bugreporter::addTrackNullOrUndefValueVisitor(N, BadE, R); + bugreporter::trackNullOrUndefValue(N, BadE, *R); } - C.EmitReport(R); + C.emitReport(R); } -StringRef describeUninitializedArgumentInCall(const CallEvent &Call, - bool IsFirstArgument) { +static StringRef describeUninitializedArgumentInCall(const CallEvent &Call, + bool IsFirstArgument) { switch (Call.getKind()) { case CE_ObjCMessage: { const ObjCMethodCall &Msg = cast<ObjCMethodCall>(Call); @@ -122,8 +122,8 @@ bool CallAndMessageChecker::PreVisitProcessArg(CheckerContext &C, BugReport *R = new BugReport(*BT, Desc, N); R->addRange(argRange); if (argEx) - bugreporter::addTrackNullOrUndefValueVisitor(N, argEx, R); - C.EmitReport(R); + bugreporter::trackNullOrUndefValue(N, argEx, *R); + C.emitReport(R); } return true; } @@ -207,7 +207,7 @@ bool CallAndMessageChecker::PreVisitProcessArg(CheckerContext &C, // FIXME: enhance track back for uninitialized value for arbitrary // memregions - C.EmitReport(R); + C.emitReport(R); } return true; } @@ -335,8 +335,8 @@ void CallAndMessageChecker::checkPreObjCMessage(const ObjCMethodCall &msg, // FIXME: getTrackNullOrUndefValueVisitor can't handle "super" yet. if (const Expr *ReceiverE = ME->getInstanceReceiver()) - bugreporter::addTrackNullOrUndefValueVisitor(N, ReceiverE, R); - C.EmitReport(R); + bugreporter::trackNullOrUndefValue(N, ReceiverE, *R); + C.emitReport(R); } return; } else { @@ -377,9 +377,9 @@ void CallAndMessageChecker::emitNilReceiverBug(CheckerContext &C, report->addRange(ME->getReceiverRange()); // FIXME: This won't track "self" in messages to super. if (const Expr *receiver = ME->getInstanceReceiver()) { - bugreporter::addTrackNullOrUndefValueVisitor(N, receiver, report); + bugreporter::trackNullOrUndefValue(N, receiver, *report); } - C.EmitReport(report); + C.emitReport(report); } static bool supportsNilWithFloatRet(const llvm::Triple &triple) { diff --git a/lib/StaticAnalyzer/Checkers/CastSizeChecker.cpp b/lib/StaticAnalyzer/Checkers/CastSizeChecker.cpp index 2e184fb..1cb8a8d 100644 --- a/lib/StaticAnalyzer/Checkers/CastSizeChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/CastSizeChecker.cpp @@ -75,7 +75,7 @@ void CastSizeChecker::checkPreStmt(const CastExpr *CE,CheckerContext &C) const { BugReport *R = new BugReport(*BT, BT->getDescription(), errorNode); R->addRange(CE->getSourceRange()); - C.EmitReport(R); + C.emitReport(R); } } } diff --git a/lib/StaticAnalyzer/Checkers/CastToStructChecker.cpp b/lib/StaticAnalyzer/Checkers/CastToStructChecker.cpp index 1407638..d6d0e3c 100644 --- a/lib/StaticAnalyzer/Checkers/CastToStructChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/CastToStructChecker.cpp @@ -64,7 +64,7 @@ void CastToStructChecker::checkPreStmt(const CastExpr *CE, "errors or data corruption.")); BugReport *R = new BugReport(*BT,BT->getDescription(), N); R->addRange(CE->getSourceRange()); - C.EmitReport(R); + C.emitReport(R); } } } diff --git a/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp b/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp index 7a25865..9087205 100644 --- a/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp +++ b/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp @@ -85,7 +85,7 @@ static bool scan_ivar_release(Stmt *S, ObjCIvarDecl *ID, Expr::NPC_ValueDependentIsNull)) { // This is only a 'release' if the property kind is not // 'assign'. - return PD->getSetterKind() != ObjCPropertyDecl::Assign;; + return PD->getSetterKind() != ObjCPropertyDecl::Assign; } // Recurse to children. diff --git a/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp b/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp index b8b7c36..5cd6194 100644 --- a/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp +++ b/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp @@ -270,6 +270,7 @@ void WalkAST::checkLoopConditionForFloat(const ForStmt *FS) { // Emit the error. First figure out which DeclRefExpr in the condition // referenced the compared variable. + assert(drInc->getDecl()); const DeclRefExpr *drCond = vdLHS == drInc->getDecl() ? drLHS : drRHS; SmallVector<SourceRange, 2> ranges; diff --git a/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp b/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp index 0e9efaa..efaec2b 100644 --- a/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp +++ b/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp @@ -25,6 +25,7 @@ using namespace ento; // All checkers should be placed into anonymous namespace. // We place the CheckerDocumentation inside ento namespace to make the // it visible in doxygen. +namespace clang { namespace ento { /// This checker documents the callback functions checkers can use to implement @@ -33,8 +34,8 @@ namespace ento { /// checking. /// /// \sa CheckerContext -class CheckerDocumentation : public Checker< check::PreStmt<DeclStmt>, - check::PostStmt<CallExpr>, +class CheckerDocumentation : public Checker< check::PreStmt<ReturnStmt>, + check::PostStmt<DeclStmt>, check::PreObjCMessage, check::PostObjCMessage, check::PreCall, @@ -64,8 +65,8 @@ public: /// See checkBranchCondition() callback for performing custom processing of /// the branching statements. /// - /// check::PreStmt<DeclStmt> - void checkPreStmt(const DeclStmt *DS, CheckerContext &C) const {} + /// check::PreStmt<ReturnStmt> + void checkPreStmt(const ReturnStmt *DS, CheckerContext &C) const {} /// \brief Post-visit the Statement. /// @@ -74,8 +75,8 @@ public: /// which does not include the control flow statements such as IfStmt. The /// callback can be specialized to be called with any subclass of Stmt. /// - /// check::PostStmt<CallExpr> - void checkPostStmt(const CallExpr *DS, CheckerContext &C) const; + /// check::PostStmt<DeclStmt> + void checkPostStmt(const DeclStmt *DS, CheckerContext &C) const; /// \brief Pre-visit the Objective C message. /// @@ -98,8 +99,8 @@ public: /// behavior for functions and methods no matter how they are being invoked. /// /// Note that this includes ALL cross-body invocations, so if you want to - /// limit your checks to, say, function calls, you can either test for that - /// or fall back to the explicit callback (i.e. check::PreStmt). + /// limit your checks to, say, function calls, you should test for that at the + /// beginning of your callback function. /// /// check::PreCall void checkPreCall(const CallEvent &Call, CheckerContext &C) const {} @@ -151,9 +152,8 @@ public: /// check::DeadSymbols void checkDeadSymbols(SymbolReaper &SR, CheckerContext &C) const {} - /// \brief Called when an end of path is reached in the ExplodedGraph. - /// - /// This callback should be used to check if the allocated resources are freed. + /// \brief Called when the analyzer core reaches the end of the top-level + /// function being analyzed. /// /// check::EndPath void checkEndPath(CheckerContext &Ctx) const {} @@ -213,21 +213,35 @@ public: /// check::LiveSymbols void checkLiveSymbols(ProgramStateRef State, SymbolReaper &SR) const {} - + /// \brief Called to determine if the checker currently needs to know if when + /// contents of any regions change. + /// + /// Since it is not necessarily cheap to compute which regions are being + /// changed, this allows the analyzer core to skip the more expensive + /// #checkRegionChanges when no checkers are tracking any state. bool wantsRegionChangeUpdate(ProgramStateRef St) const { return true; } - /// \brief Allows tracking regions which get invalidated. + /// \brief Called when the contents of one or more regions change. + /// + /// This can occur in many different ways: an explicit bind, a blanket + /// invalidation of the region contents, or by passing a region to a function + /// call whose behavior the analyzer cannot model perfectly. /// /// \param State The current program state. /// \param Invalidated A set of all symbols potentially touched by the change. /// \param ExplicitRegions The regions explicitly requested for invalidation. - /// For example, in the case of a function call, these would be arguments. - /// \param Regions The transitive closure of accessible regions, - /// i.e. all regions that may have been touched by this change. - /// \param Call The call expression wrapper if the regions are invalidated - /// by a call, 0 otherwise. - /// Note, in order to be notified, the checker should also implement the - /// wantsRegionChangeUpdate callback. + /// For a function call, this would be the arguments. For a bind, this + /// would be the region being bound to. + /// \param Regions The transitive closure of regions accessible from, + /// \p ExplicitRegions, i.e. all regions that may have been touched + /// by this change. For a simple bind, this list will be the same as + /// \p ExplicitRegions, since a bind does not affect the contents of + /// anything accessible through the base region. + /// \param Call The opaque call triggering this invalidation. Will be 0 if the + /// change was not triggered by a call. + /// + /// Note that this callback will not be invoked unless + /// #wantsRegionChangeUpdate returns \c true. /// /// check::RegionChanges ProgramStateRef @@ -256,9 +270,10 @@ public: }; -void CheckerDocumentation::checkPostStmt(const CallExpr *DS, +void CheckerDocumentation::checkPostStmt(const DeclStmt *DS, CheckerContext &C) const { return; } -} // end namespace +} // end namespace ento +} // end namespace clang diff --git a/lib/StaticAnalyzer/Checkers/Checkers.td b/lib/StaticAnalyzer/Checkers/Checkers.td index 8110bd0..235e633 100644 --- a/lib/StaticAnalyzer/Checkers/Checkers.td +++ b/lib/StaticAnalyzer/Checkers/Checkers.td @@ -13,33 +13,33 @@ include "clang/StaticAnalyzer/Checkers/CheckerBase.td" // Packages. //===----------------------------------------------------------------------===// -def Experimental : Package<"experimental">; +def Alpha : Package<"alpha">; def Core : Package<"core">; def CoreBuiltin : Package<"builtin">, InPackage<Core>; def CoreUninitialized : Package<"uninitialized">, InPackage<Core>; -def CoreExperimental : Package<"core">, InPackage<Experimental>, Hidden; +def CoreAlpha : Package<"core">, InPackage<Alpha>, Hidden; def Cplusplus : Package<"cplusplus">; -def CplusplusExperimental : Package<"cplusplus">, InPackage<Experimental>, Hidden; +def CplusplusAlpha : Package<"cplusplus">, InPackage<Alpha>, Hidden; def DeadCode : Package<"deadcode">; -def DeadCodeExperimental : Package<"deadcode">, InPackage<Experimental>, Hidden; +def DeadCodeAlpha : Package<"deadcode">, InPackage<Alpha>, Hidden; def Security : Package <"security">; def InsecureAPI : Package<"insecureAPI">, InPackage<Security>; -def SecurityExperimental : Package<"security">, InPackage<Experimental>, Hidden; -def Taint : Package<"taint">, InPackage<SecurityExperimental>, Hidden; +def SecurityAlpha : Package<"security">, InPackage<Alpha>, Hidden; +def Taint : Package<"taint">, InPackage<SecurityAlpha>, Hidden; def Unix : Package<"unix">; -def UnixExperimental : Package<"unix">, InPackage<Experimental>, Hidden; +def UnixAlpha : Package<"unix">, InPackage<Alpha>, Hidden; def CString : Package<"cstring">, InPackage<Unix>, Hidden; -def CStringExperimental : Package<"cstring">, InPackage<UnixExperimental>, Hidden; +def CStringAlpha : Package<"cstring">, InPackage<UnixAlpha>, Hidden; def OSX : Package<"osx">; -def OSXExperimental : Package<"osx">, InPackage<Experimental>, Hidden; +def OSXAlpha : Package<"osx">, InPackage<Alpha>, Hidden; def Cocoa : Package<"cocoa">, InPackage<OSX>; -def CocoaExperimental : Package<"cocoa">, InPackage<OSXExperimental>, Hidden; +def CocoaAlpha : Package<"cocoa">, InPackage<OSXAlpha>, Hidden; def CoreFoundation : Package<"coreFoundation">, InPackage<OSX>; def Containers : Package<"containers">, InPackage<CoreFoundation>; @@ -60,10 +60,6 @@ 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 AdjustedReturnValueChecker : Checker<"AdjustedReturnValue">, - HelpText<"Check to see if the return value of a function call is different than the caller expects (e.g., from calls through function pointers)">, - DescFile<"AdjustedReturnValueChecker.cpp">; - def AttrNonNullChecker : Checker<"AttributeNonNull">, HelpText<"Check for null pointers passed as arguments to a function whose arguments are marked with the 'nonnull' attribute">, DescFile<"AttrNonNullChecker.cpp">; @@ -90,7 +86,7 @@ def DynamicTypePropagation : Checker<"DynamicTypePropagation">, } // end "core" -let ParentPackage = CoreExperimental in { +let ParentPackage = CoreAlpha in { def BoolAssignmentChecker : Checker<"BoolAssignment">, HelpText<"Warn about assigning non-{0,1} values to Boolean variables">, @@ -120,7 +116,7 @@ def SizeofPointerChecker : Checker<"SizeofPtr">, HelpText<"Warn about unintended use of sizeof() on pointer expressions">, DescFile<"CheckSizeofPointer.cpp">; -} // end "core.experimental" +} // end "alpha.core" //===----------------------------------------------------------------------===// // Evaluate "builtin" functions. @@ -170,13 +166,13 @@ def ReturnUndefChecker : Checker<"UndefReturn">, // C++ checkers. //===----------------------------------------------------------------------===// -let ParentPackage = CplusplusExperimental in { +let ParentPackage = CplusplusAlpha in { def VirtualCallChecker : Checker<"VirtualCall">, HelpText<"Check virtual function calls during construction or destruction">, DescFile<"VirtualCallChecker.cpp">; -} // end: "cplusplus.experimental" +} // end: "alpha.cplusplus" //===----------------------------------------------------------------------===// // Deadcode checkers. @@ -189,7 +185,7 @@ def DeadStoresChecker : Checker<"DeadStores">, DescFile<"DeadStoresChecker.cpp">; } // end DeadCode -let ParentPackage = DeadCodeExperimental in { +let ParentPackage = DeadCodeAlpha in { def IdempotentOperationChecker : Checker<"IdempotentOperations">, HelpText<"Warn about idempotent operations">, @@ -199,7 +195,7 @@ def UnreachableCodeChecker : Checker<"UnreachableCode">, HelpText<"Check unreachable code">, DescFile<"UnreachableCodeChecker.cpp">; -} // end "deadcode.experimental" +} // end "alpha.deadcode" //===----------------------------------------------------------------------===// // Security checkers. @@ -237,7 +233,7 @@ let ParentPackage = Security in { DescFile<"CheckSecuritySyntaxOnly.cpp">; } -let ParentPackage = SecurityExperimental in { +let ParentPackage = SecurityAlpha in { def ArrayBoundChecker : Checker<"ArrayBound">, HelpText<"Warn about buffer overflows (older checker)">, @@ -255,7 +251,7 @@ def MallocOverflowSecurityChecker : Checker<"MallocOverflow">, HelpText<"Check for overflows in the arguments to malloc()">, DescFile<"MallocOverflowSecurityChecker.cpp">; -} // end "security.experimental" +} // end "alpha.security" //===----------------------------------------------------------------------===// // Taint checkers. @@ -267,7 +263,7 @@ def GenericTaintChecker : Checker<"TaintPropagation">, HelpText<"Generate taint information used by other checkers">, DescFile<"GenericTaintChecker.cpp">; -} // end "experimental.security.taint" +} // end "alpha.security.taint" //===----------------------------------------------------------------------===// // Unix API checkers. @@ -289,7 +285,7 @@ def MallocSizeofChecker : Checker<"MallocSizeof">, } // end "unix" -let ParentPackage = UnixExperimental in { +let ParentPackage = UnixAlpha in { def ChrootChecker : Checker<"Chroot">, HelpText<"Check improper use of chroot">, @@ -307,7 +303,11 @@ def StreamChecker : Checker<"Stream">, HelpText<"Check stream handling functions">, DescFile<"StreamChecker.cpp">; -} // end "unix.experimental" +def SimpleStreamChecker : Checker<"SimpleStream">, + HelpText<"Check for misuses of stream APIs">, + DescFile<"SimpleStreamChecker.cpp">; + +} // end "alpha.unix" let ParentPackage = CString in { @@ -320,7 +320,7 @@ def CStringSyntaxChecker : Checker<"BadSizeArg">, DescFile<"CStringSyntaxChecker.cpp">; } -let ParentPackage = CStringExperimental in { +let ParentPackage = CStringAlpha in { def CStringOutOfBounds : Checker<"OutOfBounds">, HelpText<"Check for out-of-bounds access in string functions">, @@ -346,11 +346,6 @@ def MacOSXAPIChecker : Checker<"API">, HelpText<"Check for proper uses of various Mac OS X APIs">, DescFile<"MacOSXAPIChecker.cpp">; -def OSAtomicChecker : Checker<"AtomicCAS">, - InPackage<OSX>, - HelpText<"Evaluate calls to OSAtomic functions">, - DescFile<"OSAtomicChecker.cpp">; - def MacOSKeychainAPIChecker : Checker<"SecKeychainAPI">, InPackage<OSX>, HelpText<"Check for proper uses of Secure Keychain APIs">, @@ -397,6 +392,10 @@ 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 NSErrorChecker : Checker<"NSError">, HelpText<"Check usage of NSError** parameters">, DescFile<"NSErrorChecker.cpp">; @@ -407,13 +406,25 @@ def RetainCountChecker : Checker<"RetainCount">, } // end "osx.cocoa" -let ParentPackage = CocoaExperimental in { +let ParentPackage = CocoaAlpha in { def ObjCDeallocChecker : Checker<"Dealloc">, HelpText<"Warn about Objective-C classes that lack a correct implementation of -dealloc">, DescFile<"CheckObjCDealloc.cpp">; -} // end "cocoa.experimental" +def IvarInvalidationChecker : Checker<"InstanceVariableInvalidation">, + HelpText<"Check that the invalidatable instance variables are invalidated in the methods annotated with objc_instance_variable_invalidator">, + DescFile<"IvarInvalidationChecker.cpp">; + +def DirectIvarAssignment : Checker<"DirectIvarAssignment">, + HelpText<"Check that the invalidatable instance variables are invalidated in the methods annotated with objc_instance_variable_invalidator">, + DescFile<"DirectIvarAssignment.cpp">; + +def ObjCSuperCallChecker : Checker<"MissingSuperCall">, + HelpText<"Warn about Objective-C methods that lack a necessary call to super">, + DescFile<"ObjCMissingSuperCallChecker.cpp">; + +} // end "alpha.osx.cocoa" let ParentPackage = CoreFoundation in { @@ -422,7 +433,7 @@ def CFNumberCreateChecker : Checker<"CFNumber">, DescFile<"BasicObjCFoundationChecks.cpp">; def CFRetainReleaseChecker : Checker<"CFRetainRelease">, - HelpText<"Check for null arguments to CFRetain/CFRelease">, + HelpText<"Check for null arguments to CFRetain/CFRelease/CFMakeCollectable">, DescFile<"BasicObjCFoundationChecks.cpp">; def CFErrorChecker : Checker<"CFError">, @@ -479,6 +490,10 @@ 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">; diff --git a/lib/StaticAnalyzer/Checkers/ChrootChecker.cpp b/lib/StaticAnalyzer/Checkers/ChrootChecker.cpp index 30d0609..c885616 100644 --- a/lib/StaticAnalyzer/Checkers/ChrootChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/ChrootChecker.cpp @@ -147,7 +147,7 @@ void ChrootChecker::checkPreStmt(const CallExpr *CE, CheckerContext &C) const { "after chroot")); BugReport *R = new BugReport(*BT_BreakJail, BT_BreakJail->getDescription(), N); - C.EmitReport(R); + C.emitReport(R); } return; diff --git a/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp b/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp index 510e8cd..59e03ec 100644 --- a/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp @@ -13,22 +13,54 @@ //===----------------------------------------------------------------------===// #include "ClangSACheckers.h" -#include "clang/StaticAnalyzer/Core/Checker.h" -#include "clang/Analysis/Analyses/LiveVariables.h" -#include "clang/Analysis/Visitors/CFGRecStmtVisitor.h" -#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h" -#include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h" -#include "clang/Analysis/Visitors/CFGRecStmtDeclVisitor.h" -#include "clang/Basic/Diagnostic.h" #include "clang/AST/ASTContext.h" #include "clang/AST/ParentMap.h" -#include "llvm/ADT/SmallPtrSet.h" +#include "clang/AST/RecursiveASTVisitor.h" +#include "clang/Analysis/Analyses/LiveVariables.h" +#include "clang/Analysis/Visitors/CFGRecStmtDeclVisitor.h" +#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h" +#include "clang/StaticAnalyzer/Core/Checker.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h" +#include "llvm/ADT/BitVector.h" #include "llvm/ADT/SmallString.h" +#include "llvm/Support/SaveAndRestore.h" using namespace clang; using namespace ento; -namespace { +namespace { + +/// A simple visitor to record what VarDecls occur in EH-handling code. +class EHCodeVisitor : public RecursiveASTVisitor<EHCodeVisitor> { +public: + bool inEH; + llvm::DenseSet<const VarDecl *> &S; + + bool TraverseObjCAtFinallyStmt(ObjCAtFinallyStmt *S) { + SaveAndRestore<bool> inFinally(inEH, true); + return ::RecursiveASTVisitor<EHCodeVisitor>::TraverseObjCAtFinallyStmt(S); + } + + bool TraverseObjCAtCatchStmt(ObjCAtCatchStmt *S) { + SaveAndRestore<bool> inCatch(inEH, true); + return ::RecursiveASTVisitor<EHCodeVisitor>::TraverseObjCAtCatchStmt(S); + } + + bool TraverseCXXCatchStmt(CXXCatchStmt *S) { + SaveAndRestore<bool> inCatch(inEH, true); + return TraverseStmt(S->getHandlerBlock()); + } + + bool VisitDeclRefExpr(DeclRefExpr *DR) { + if (inEH) + if (const VarDecl *D = dyn_cast<VarDecl>(DR->getDecl())) + S.insert(D); + return true; + } + + EHCodeVisitor(llvm::DenseSet<const VarDecl *> &S) : + inEH(false), S(S) {} +}; // FIXME: Eventually migrate into its own file, and have it managed by // AnalysisManager. @@ -93,6 +125,7 @@ class DeadStoreObs : public LiveVariables::Observer { llvm::SmallPtrSet<const VarDecl*, 20> Escaped; OwningPtr<ReachableCode> reachableCode; const CFGBlock *currentBlock; + llvm::OwningPtr<llvm::DenseSet<const VarDecl *> > InEH; enum DeadStoreKind { Standard, Enclosing, DeadIncrement, DeadInit }; @@ -105,6 +138,23 @@ public: virtual ~DeadStoreObs() {} + bool isLive(const LiveVariables::LivenessValues &Live, const VarDecl *D) { + if (Live.isLive(D)) + return true; + // Lazily construct the set that records which VarDecls are in + // EH code. + if (!InEH.get()) { + InEH.reset(new llvm::DenseSet<const VarDecl *>()); + EHCodeVisitor V(*InEH.get()); + V.TraverseStmt(AC->getBody()); + } + // Treat all VarDecls that occur in EH code as being "always live" + // when considering to suppress dead stores. Frequently stores + // are followed by reads in EH code, but we don't have the ability + // to analyze that yet. + return InEH->count(D); + } + void Report(const VarDecl *V, DeadStoreKind dsk, PathDiagnosticLocation L, SourceRange R) { if (Escaped.count(V)) @@ -159,7 +209,7 @@ public: if (VD->getType()->getAs<ReferenceType>()) return; - if (!Live.isLive(VD) && + if (!isLive(Live, VD) && !(VD->getAttr<UnusedAttr>() || VD->getAttr<BlocksAttr>())) { PathDiagnosticLocation ExLoc = @@ -285,7 +335,7 @@ public: // A dead initialization is a variable that is dead after it // is initialized. We don't flag warnings for those variables // marked 'unused'. - if (!Live.isLive(V) && V->getAttr<UnusedAttr>() == 0) { + if (!isLive(Live, V) && V->getAttr<UnusedAttr>() == 0) { // Special case: check for initializations with constants. // // e.g. : int x = 0; diff --git a/lib/StaticAnalyzer/Checkers/DebugCheckers.cpp b/lib/StaticAnalyzer/Checkers/DebugCheckers.cpp index 34053cd..7ad9c59 100644 --- a/lib/StaticAnalyzer/Checkers/DebugCheckers.cpp +++ b/lib/StaticAnalyzer/Checkers/DebugCheckers.cpp @@ -144,3 +144,38 @@ public: void ento::registerCallGraphDumper(CheckerManager &mgr) { mgr.registerChecker<CallGraphDumper>(); } + + +//===----------------------------------------------------------------------===// +// ConfigDumper +//===----------------------------------------------------------------------===// + +namespace { +class ConfigDumper : public Checker< check::EndOfTranslationUnit > { +public: + void checkEndOfTranslationUnit(const TranslationUnitDecl *TU, + AnalysisManager& mgr, + BugReporter &BR) const { + + const AnalyzerOptions::ConfigTable &Config = mgr.options.Config; + AnalyzerOptions::ConfigTable::const_iterator I = + Config.begin(), E = Config.end(); + + std::vector<StringRef> Keys; + for (; I != E ; ++I) { Keys.push_back(I->getKey()); } + sort(Keys.begin(), Keys.end()); + + llvm::errs() << "[config]\n"; + for (unsigned i = 0, n = Keys.size(); i < n ; ++i) { + StringRef Key = Keys[i]; + I = Config.find(Key); + llvm::errs() << Key << " = " << I->second << '\n'; + } + llvm::errs() << "[stats]\n" << "num-entries = " << Keys.size() << '\n'; + } +}; +} + +void ento::registerConfigDumper(CheckerManager &mgr) { + mgr.registerChecker<ConfigDumper>(); +} diff --git a/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp b/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp index e98c131..3ace4be 100644 --- a/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp @@ -39,7 +39,7 @@ public: CheckerContext &C) const; void checkBind(SVal L, SVal V, const Stmt *S, CheckerContext &C) const; - static const MemRegion *AddDerefSource(raw_ostream &os, + static void AddDerefSource(raw_ostream &os, SmallVectorImpl<SourceRange> &Ranges, const Expr *Ex, const ProgramState *state, const LocationContext *LCtx, @@ -47,7 +47,7 @@ public: }; } // end anonymous namespace -const MemRegion * +void DereferenceChecker::AddDerefSource(raw_ostream &os, SmallVectorImpl<SourceRange> &Ranges, const Expr *Ex, @@ -55,7 +55,6 @@ DereferenceChecker::AddDerefSource(raw_ostream &os, const LocationContext *LCtx, bool loadedFrom) { Ex = Ex->IgnoreParenLValueCasts(); - const MemRegion *sourceR = 0; switch (Ex->getStmtClass()) { default: break; @@ -65,7 +64,6 @@ DereferenceChecker::AddDerefSource(raw_ostream &os, os << " (" << (loadedFrom ? "loaded from" : "from") << " variable '" << VD->getName() << "')"; Ranges.push_back(DR->getSourceRange()); - sourceR = state->getLValue(VD, LCtx).getAsRegion(); } break; } @@ -78,7 +76,6 @@ DereferenceChecker::AddDerefSource(raw_ostream &os, break; } } - return sourceR; } void DereferenceChecker::reportBug(ProgramStateRef State, const Stmt *S, @@ -94,6 +91,8 @@ void DereferenceChecker::reportBug(ProgramStateRef State, const Stmt *S, BT_null.reset(new BuiltinBug("Dereference of null pointer")); SmallString<100> buf; + llvm::raw_svector_ostream os(buf); + SmallVector<SourceRange, 2> Ranges; // Walk through lvalue casts to get the original expression @@ -101,8 +100,6 @@ void DereferenceChecker::reportBug(ProgramStateRef State, const Stmt *S, if (const Expr *expr = dyn_cast<Expr>(S)) S = expr->IgnoreParenLValueCasts(); - const MemRegion *sourceR = 0; - if (IsBind) { if (const BinaryOperator *BO = dyn_cast<BinaryOperator>(S)) { if (BO->isAssignmentOp()) @@ -117,68 +114,55 @@ void DereferenceChecker::reportBug(ProgramStateRef State, const Stmt *S, switch (S->getStmtClass()) { case Stmt::ArraySubscriptExprClass: { - llvm::raw_svector_ostream os(buf); os << "Array access"; const ArraySubscriptExpr *AE = cast<ArraySubscriptExpr>(S); - sourceR = AddDerefSource(os, Ranges, AE->getBase()->IgnoreParenCasts(), - State.getPtr(), N->getLocationContext()); + AddDerefSource(os, Ranges, AE->getBase()->IgnoreParenCasts(), + State.getPtr(), N->getLocationContext()); os << " results in a null pointer dereference"; break; } case Stmt::UnaryOperatorClass: { - llvm::raw_svector_ostream os(buf); os << "Dereference of null pointer"; const UnaryOperator *U = cast<UnaryOperator>(S); - sourceR = AddDerefSource(os, Ranges, U->getSubExpr()->IgnoreParens(), - State.getPtr(), N->getLocationContext(), true); + AddDerefSource(os, Ranges, U->getSubExpr()->IgnoreParens(), + State.getPtr(), N->getLocationContext(), true); break; } case Stmt::MemberExprClass: { const MemberExpr *M = cast<MemberExpr>(S); - if (M->isArrow()) { - llvm::raw_svector_ostream os(buf); + if (M->isArrow() || bugreporter::isDeclRefExprToReference(M->getBase())) { os << "Access to field '" << M->getMemberNameInfo() << "' results in a dereference of a null pointer"; - sourceR = AddDerefSource(os, Ranges, M->getBase()->IgnoreParenCasts(), - State.getPtr(), N->getLocationContext(), true); + AddDerefSource(os, Ranges, M->getBase()->IgnoreParenCasts(), + State.getPtr(), N->getLocationContext(), true); } break; } case Stmt::ObjCIvarRefExprClass: { const ObjCIvarRefExpr *IV = cast<ObjCIvarRefExpr>(S); - if (const DeclRefExpr *DR = - dyn_cast<DeclRefExpr>(IV->getBase()->IgnoreParenCasts())) { - if (const VarDecl *VD = dyn_cast<VarDecl>(DR->getDecl())) { - llvm::raw_svector_ostream os(buf); - os << "Instance variable access (via '" << VD->getName() - << "') results in a null pointer dereference"; - } - } - Ranges.push_back(IV->getSourceRange()); + os << "Access to instance variable '" << *IV->getDecl() + << "' results in a dereference of a null pointer"; + AddDerefSource(os, Ranges, IV->getBase()->IgnoreParenCasts(), + State.getPtr(), N->getLocationContext(), true); break; } default: break; } + os.flush(); BugReport *report = new BugReport(*BT_null, buf.empty() ? BT_null->getDescription() : buf.str(), N); - bugreporter::addTrackNullOrUndefValueVisitor(N, bugreporter::GetDerefExpr(N), - report); + bugreporter::trackNullOrUndefValue(N, bugreporter::GetDerefExpr(N), *report); for (SmallVectorImpl<SourceRange>::iterator I = Ranges.begin(), E = Ranges.end(); I!=E; ++I) report->addRange(*I); - if (sourceR) { - report->markInteresting(sourceR); - report->markInteresting(State->getRawSVal(loc::MemRegionVal(sourceR))); - } - - C.EmitReport(report); + C.emitReport(report); } void DereferenceChecker::checkLocation(SVal l, bool isLoad, const Stmt* S, @@ -191,11 +175,9 @@ void DereferenceChecker::checkLocation(SVal l, bool isLoad, const Stmt* S, BugReport *report = new BugReport(*BT_undef, BT_undef->getDescription(), N); - bugreporter::addTrackNullOrUndefValueVisitor(N, - bugreporter::GetDerefExpr(N), - report); - report->disablePathPruning(); - C.EmitReport(report); + bugreporter::trackNullOrUndefValue(N, bugreporter::GetDerefExpr(N), + *report); + C.emitReport(report); } return; } diff --git a/lib/StaticAnalyzer/Checkers/DirectIvarAssignment.cpp b/lib/StaticAnalyzer/Checkers/DirectIvarAssignment.cpp new file mode 100644 index 0000000..dc90b67 --- /dev/null +++ b/lib/StaticAnalyzer/Checkers/DirectIvarAssignment.cpp @@ -0,0 +1,180 @@ +//=- DirectIvarAssignment.cpp - Check rules on ObjC properties -*- C++ ----*-==// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +// +// Check that Objective C properties follow the following rules: +// - The property should be set with the setter, not though a direct +// assignment. +// +//===----------------------------------------------------------------------===// + +#include "ClangSACheckers.h" +#include "clang/StaticAnalyzer/Core/Checker.h" +#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h" +#include "clang/AST/DeclObjC.h" +#include "clang/AST/StmtVisitor.h" +#include "llvm/ADT/DenseMap.h" + +using namespace clang; +using namespace ento; + +namespace { + +class DirectIvarAssignment : + public Checker<check::ASTDecl<ObjCImplementationDecl> > { + + typedef llvm::DenseMap<const ObjCIvarDecl*, + const ObjCPropertyDecl*> IvarToPropertyMapTy; + + /// A helper class, which walks the AST and locates all assignments to ivars + /// in the given function. + class MethodCrawler : public ConstStmtVisitor<MethodCrawler> { + const IvarToPropertyMapTy &IvarToPropMap; + const ObjCMethodDecl *MD; + const ObjCInterfaceDecl *InterfD; + BugReporter &BR; + LocationOrAnalysisDeclContext DCtx; + + public: + MethodCrawler(const IvarToPropertyMapTy &InMap, const ObjCMethodDecl *InMD, + const ObjCInterfaceDecl *InID, + BugReporter &InBR, AnalysisDeclContext *InDCtx) + : IvarToPropMap(InMap), MD(InMD), InterfD(InID), BR(InBR), DCtx(InDCtx) {} + + void VisitStmt(const Stmt *S) { VisitChildren(S); } + + void VisitBinaryOperator(const BinaryOperator *BO); + + void VisitChildren(const Stmt *S) { + for (Stmt::const_child_range I = S->children(); I; ++I) + if (*I) + this->Visit(*I); + } + }; + +public: + void checkASTDecl(const ObjCImplementationDecl *D, AnalysisManager& Mgr, + BugReporter &BR) const; +}; + +static const ObjCIvarDecl *findPropertyBackingIvar(const ObjCPropertyDecl *PD, + const ObjCInterfaceDecl *InterD, + ASTContext &Ctx) { + // Check for synthesized ivars. + ObjCIvarDecl *ID = PD->getPropertyIvarDecl(); + if (ID) + return ID; + + ObjCInterfaceDecl *NonConstInterD = const_cast<ObjCInterfaceDecl*>(InterD); + + // Check for existing "_PropName". + ID = NonConstInterD->lookupInstanceVariable(PD->getDefaultSynthIvarName(Ctx)); + if (ID) + return ID; + + // Check for existing "PropName". + IdentifierInfo *PropIdent = PD->getIdentifier(); + ID = NonConstInterD->lookupInstanceVariable(PropIdent); + + return ID; +} + +void DirectIvarAssignment::checkASTDecl(const ObjCImplementationDecl *D, + AnalysisManager& Mgr, + BugReporter &BR) const { + const ObjCInterfaceDecl *InterD = D->getClassInterface(); + + + IvarToPropertyMapTy IvarToPropMap; + + // Find all properties for this class. + for (ObjCInterfaceDecl::prop_iterator I = InterD->prop_begin(), + E = InterD->prop_end(); I != E; ++I) { + ObjCPropertyDecl *PD = *I; + + // Find the corresponding IVar. + const ObjCIvarDecl *ID = findPropertyBackingIvar(PD, InterD, + Mgr.getASTContext()); + + if (!ID) + continue; + + // Store the IVar to property mapping. + IvarToPropMap[ID] = PD; + } + + if (IvarToPropMap.empty()) + return; + + for (ObjCImplementationDecl::instmeth_iterator I = D->instmeth_begin(), + E = D->instmeth_end(); I != E; ++I) { + + ObjCMethodDecl *M = *I; + AnalysisDeclContext *DCtx = Mgr.getAnalysisDeclContext(M); + + // Skip the init, dealloc functions and any functions that might be doing + // initialization based on their name. + if (M->getMethodFamily() == OMF_init || + M->getMethodFamily() == OMF_dealloc || + M->getMethodFamily() == OMF_copy || + M->getMethodFamily() == OMF_mutableCopy || + M->getSelector().getNameForSlot(0).find("init") != StringRef::npos || + M->getSelector().getNameForSlot(0).find("Init") != StringRef::npos) + continue; + + const Stmt *Body = M->getBody(); + assert(Body); + + MethodCrawler MC(IvarToPropMap, M->getCanonicalDecl(), InterD, BR, DCtx); + MC.VisitStmt(Body); + } +} + +void DirectIvarAssignment::MethodCrawler::VisitBinaryOperator( + const BinaryOperator *BO) { + if (!BO->isAssignmentOp()) + return; + + const ObjCIvarRefExpr *IvarRef = + dyn_cast<ObjCIvarRefExpr>(BO->getLHS()->IgnoreParenCasts()); + + if (!IvarRef) + return; + + if (const ObjCIvarDecl *D = IvarRef->getDecl()) { + IvarToPropertyMapTy::const_iterator I = IvarToPropMap.find(D); + if (I != IvarToPropMap.end()) { + const ObjCPropertyDecl *PD = I->second; + + ObjCMethodDecl *GetterMethod = + InterfD->getInstanceMethod(PD->getGetterName()); + ObjCMethodDecl *SetterMethod = + InterfD->getInstanceMethod(PD->getSetterName()); + + if (SetterMethod && SetterMethod->getCanonicalDecl() == MD) + return; + + if (GetterMethod && GetterMethod->getCanonicalDecl() == MD) + return; + + BR.EmitBasicReport(MD, + "Property access", + categories::CoreFoundationObjectiveC, + "Direct assignment to an instance variable backing a property; " + "use the setter instead", PathDiagnosticLocation(IvarRef, + BR.getSourceManager(), + DCtx)); + } + } +} +} + +void ento::registerDirectIvarAssignment(CheckerManager &mgr) { + mgr.registerChecker<DirectIvarAssignment>(); +} diff --git a/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp b/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp index dcf6a86..76fb3f2 100644 --- a/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp @@ -39,13 +39,9 @@ void DivZeroChecker::reportBug(const char *Msg, if (!BT) BT.reset(new BuiltinBug("Division by zero")); - BugReport *R = - new BugReport(*BT, Msg, N); - - bugreporter::addTrackNullOrUndefValueVisitor(N, - bugreporter::GetDenomExpr(N), - R); - C.EmitReport(R); + BugReport *R = new BugReport(*BT, Msg, N); + bugreporter::trackNullOrUndefValue(N, bugreporter::GetDenomExpr(N), *R); + C.emitReport(R); } } diff --git a/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp b/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp index b636efb..b0a4bc6 100644 --- a/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp +++ b/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp @@ -83,14 +83,14 @@ void DynamicTypePropagation::checkPreCall(const CallEvent &Call, if (const CXXDestructorCall *Dtor = dyn_cast<CXXDestructorCall>(&Call)) { // C++11 [class.cdtor]p4 (see above) + if (!Dtor->isBaseDestructor()) + return; const MemRegion *Target = Dtor->getCXXThisVal().getAsRegion(); if (!Target) return; - // FIXME: getRuntimeDefinition() can be expensive. It would be better to do - // this when we are entering the stack frame for the destructor. - const Decl *D = Dtor->getRuntimeDefinition().getDecl(); + const Decl *D = Dtor->getDecl(); if (!D) return; @@ -105,8 +105,7 @@ void DynamicTypePropagation::checkPostCall(const CallEvent &Call, if (const ObjCMethodCall *Msg = dyn_cast<ObjCMethodCall>(&Call)) { // Get the returned value if it's a region. - SVal Result = C.getSVal(Call.getOriginExpr()); - const MemRegion *RetReg = Result.getAsRegion(); + const MemRegion *RetReg = Call.getReturnValue().getAsRegion(); if (!RetReg) return; diff --git a/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp b/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp index 7acf223..e7e3162 100644 --- a/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp @@ -93,7 +93,7 @@ void ExprInspectionChecker::analyzerEval(const CallExpr *CE, BT.reset(new BugType("Checking analyzer assumptions", "debug")); BugReport *R = new BugReport(*BT, getArgumentValueString(CE, C), N); - C.EmitReport(R); + C.emitReport(R); } void ExprInspectionChecker::analyzerCheckInlined(const CallExpr *CE, @@ -113,7 +113,7 @@ void ExprInspectionChecker::analyzerCheckInlined(const CallExpr *CE, BT.reset(new BugType("Checking analyzer assumptions", "debug")); BugReport *R = new BugReport(*BT, getArgumentValueString(CE, C), N); - C.EmitReport(R); + C.emitReport(R); } void ento::registerExprInspectionChecker(CheckerManager &Mgr) { diff --git a/lib/StaticAnalyzer/Checkers/FixedAddressChecker.cpp b/lib/StaticAnalyzer/Checkers/FixedAddressChecker.cpp index a1f2f3b..7fde689 100644 --- a/lib/StaticAnalyzer/Checkers/FixedAddressChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/FixedAddressChecker.cpp @@ -58,7 +58,7 @@ void FixedAddressChecker::checkPreStmt(const BinaryOperator *B, "environments or platforms.")); BugReport *R = new BugReport(*BT, BT->getDescription(), N); R->addRange(B->getRHS()->getSourceRange()); - C.EmitReport(R); + C.emitReport(R); } } diff --git a/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp b/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp index afb862c..a9e0217 100644 --- a/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp @@ -192,13 +192,7 @@ const char GenericTaintChecker::MsgTaintedBufferSize[] = /// to the call post-visit. The values are unsigned integers, which are either /// ReturnValueIndex, or indexes of the pointer/reference argument, which /// points to data, which should be tainted on return. -namespace { struct TaintArgsOnPostVisit{}; } -namespace clang { namespace ento { -template<> struct ProgramStateTrait<TaintArgsOnPostVisit> - : public ProgramStatePartialTrait<llvm::ImmutableSet<unsigned> > { - static void *GDMIndex() { return GenericTaintChecker::getTag(); } -}; -}} +REGISTER_SET_WITH_PROGRAMSTATE(TaintArgsOnPostVisit, unsigned) GenericTaintChecker::TaintPropagationRule GenericTaintChecker::TaintPropagationRule::getTaintPropagationRule( @@ -337,7 +331,7 @@ bool GenericTaintChecker::propagateFromPre(const CallExpr *CE, // Depending on what was tainted at pre-visit, we determined a set of // arguments which should be tainted after the function returns. These are // stored in the state as TaintArgsOnPostVisit set. - llvm::ImmutableSet<unsigned> TaintArgs = State->get<TaintArgsOnPostVisit>(); + TaintArgsOnPostVisitTy TaintArgs = State->get<TaintArgsOnPostVisit>(); if (TaintArgs.isEmpty()) return false; @@ -653,7 +647,7 @@ bool GenericTaintChecker::generateReportIfTainted(const Expr *E, initBugType(); BugReport *report = new BugReport(*BT, Msg, N); report->addRange(E->getSourceRange()); - C.EmitReport(report); + C.emitReport(report); return true; } return false; diff --git a/lib/StaticAnalyzer/Checkers/IdempotentOperationChecker.cpp b/lib/StaticAnalyzer/Checkers/IdempotentOperationChecker.cpp index 9d0b83f..ffbbb8b 100644 --- a/lib/StaticAnalyzer/Checkers/IdempotentOperationChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/IdempotentOperationChecker.cpp @@ -430,7 +430,7 @@ void IdempotentOperationChecker::checkEndAnalysis(ExplodedGraph &G, FindLastStoreBRVisitor::registerStatementVarDecls(*report, RHS); } - BR.EmitReport(report); + BR.emitReport(report); } } diff --git a/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp b/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp new file mode 100644 index 0000000..bf256cd --- /dev/null +++ b/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp @@ -0,0 +1,550 @@ +//=- IvarInvalidationChecker.cpp - -*- C++ -------------------------------*-==// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +// +// This checker implements annotation driven invalidation checking. If a class +// contains a method annotated with 'objc_instance_variable_invalidator', +// - (void) foo +// __attribute__((annotate("objc_instance_variable_invalidator"))); +// all the "ivalidatable" instance variables of this class should be +// invalidated. We call an instance variable ivalidatable if it is an object of +// a class which contains an invalidation method. There could be multiple +// methods annotated with such annotations per class, either one can be used +// to invalidate the ivar. An ivar or property are considered to be +// invalidated if they are being assigned 'nil' or an invalidation method has +// been called on them. An invalidation method should either invalidate all +// the ivars or call another invalidation method (on self). +// +//===----------------------------------------------------------------------===// + +#include "ClangSACheckers.h" +#include "clang/StaticAnalyzer/Core/Checker.h" +#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h" +#include "clang/AST/DeclObjC.h" +#include "clang/AST/StmtVisitor.h" +#include "llvm/ADT/DenseMap.h" +#include "llvm/ADT/SmallString.h" + +using namespace clang; +using namespace ento; + +namespace { +class IvarInvalidationChecker : + public Checker<check::ASTDecl<ObjCMethodDecl> > { + + typedef llvm::DenseSet<const ObjCMethodDecl*> MethodSet; + typedef llvm::DenseMap<const ObjCMethodDecl*, + const ObjCIvarDecl*> MethToIvarMapTy; + typedef llvm::DenseMap<const ObjCPropertyDecl*, + const ObjCIvarDecl*> PropToIvarMapTy; + typedef llvm::DenseMap<const ObjCIvarDecl*, + const ObjCPropertyDecl*> IvarToPropMapTy; + + + struct IvarInfo { + /// Has the ivar been invalidated? + bool IsInvalidated; + + /// The methods which can be used to invalidate the ivar. + MethodSet InvalidationMethods; + + IvarInfo() : IsInvalidated(false) {} + void addInvalidationMethod(const ObjCMethodDecl *MD) { + InvalidationMethods.insert(MD); + } + + bool needsInvalidation() const { + return !InvalidationMethods.empty(); + } + + void markInvalidated() { + IsInvalidated = true; + } + + bool markInvalidated(const ObjCMethodDecl *MD) { + if (IsInvalidated) + return true; + for (MethodSet::iterator I = InvalidationMethods.begin(), + E = InvalidationMethods.end(); I != E; ++I) { + if (*I == MD) { + IsInvalidated = true; + return true; + } + } + return false; + } + + bool isInvalidated() const { + return IsInvalidated; + } + }; + + typedef llvm::DenseMap<const ObjCIvarDecl*, IvarInfo> IvarSet; + + /// Statement visitor, which walks the method body and flags the ivars + /// referenced in it (either directly or via property). + class MethodCrawler : public ConstStmtVisitor<MethodCrawler> { + /// The set of Ivars which need to be invalidated. + IvarSet &IVars; + + /// Flag is set as the result of a message send to another + /// invalidation method. + bool &CalledAnotherInvalidationMethod; + + /// Property setter to ivar mapping. + const MethToIvarMapTy &PropertySetterToIvarMap; + + /// Property getter to ivar mapping. + const MethToIvarMapTy &PropertyGetterToIvarMap; + + /// Property to ivar mapping. + const PropToIvarMapTy &PropertyToIvarMap; + + /// The invalidation method being currently processed. + const ObjCMethodDecl *InvalidationMethod; + + ASTContext &Ctx; + + /// Peel off parens, casts, OpaqueValueExpr, and PseudoObjectExpr. + const Expr *peel(const Expr *E) const; + + /// Does this expression represent zero: '0'? + bool isZero(const Expr *E) const; + + /// Mark the given ivar as invalidated. + void markInvalidated(const ObjCIvarDecl *Iv); + + /// Checks if IvarRef refers to the tracked IVar, if yes, marks it as + /// invalidated. + void checkObjCIvarRefExpr(const ObjCIvarRefExpr *IvarRef); + + /// Checks if ObjCPropertyRefExpr refers to the tracked IVar, if yes, marks + /// it as invalidated. + void checkObjCPropertyRefExpr(const ObjCPropertyRefExpr *PA); + + /// Checks if ObjCMessageExpr refers to (is a getter for) the tracked IVar, + /// if yes, marks it as invalidated. + void checkObjCMessageExpr(const ObjCMessageExpr *ME); + + /// Checks if the Expr refers to an ivar, if yes, marks it as invalidated. + void check(const Expr *E); + + public: + MethodCrawler(IvarSet &InIVars, + bool &InCalledAnotherInvalidationMethod, + const MethToIvarMapTy &InPropertySetterToIvarMap, + const MethToIvarMapTy &InPropertyGetterToIvarMap, + const PropToIvarMapTy &InPropertyToIvarMap, + ASTContext &InCtx) + : IVars(InIVars), + CalledAnotherInvalidationMethod(InCalledAnotherInvalidationMethod), + PropertySetterToIvarMap(InPropertySetterToIvarMap), + PropertyGetterToIvarMap(InPropertyGetterToIvarMap), + PropertyToIvarMap(InPropertyToIvarMap), + InvalidationMethod(0), + Ctx(InCtx) {} + + void VisitStmt(const Stmt *S) { VisitChildren(S); } + + void VisitBinaryOperator(const BinaryOperator *BO); + + void VisitObjCMessageExpr(const ObjCMessageExpr *ME); + + void VisitChildren(const Stmt *S) { + for (Stmt::const_child_range I = S->children(); I; ++I) { + if (*I) + this->Visit(*I); + if (CalledAnotherInvalidationMethod) + return; + } + } + }; + + /// Check if the any of the methods inside the interface are annotated with + /// the invalidation annotation, update the IvarInfo accordingly. + static void containsInvalidationMethod(const ObjCContainerDecl *D, + IvarInfo &Out); + + /// Check if ivar should be tracked and add to TrackedIvars if positive. + /// Returns true if ivar should be tracked. + static bool trackIvar(const ObjCIvarDecl *Iv, IvarSet &TrackedIvars); + + /// Given the property declaration, and the list of tracked ivars, finds + /// the ivar backing the property when possible. Returns '0' when no such + /// ivar could be found. + static const ObjCIvarDecl *findPropertyBackingIvar( + const ObjCPropertyDecl *Prop, + const ObjCInterfaceDecl *InterfaceD, + IvarSet &TrackedIvars); + +public: + void checkASTDecl(const ObjCMethodDecl *D, AnalysisManager& Mgr, + BugReporter &BR) const; + + // TODO: We are currently ignoring the ivars coming from class extensions. +}; + +static bool isInvalidationMethod(const ObjCMethodDecl *M) { + for (specific_attr_iterator<AnnotateAttr> + AI = M->specific_attr_begin<AnnotateAttr>(), + AE = M->specific_attr_end<AnnotateAttr>(); AI != AE; ++AI) { + const AnnotateAttr *Ann = *AI; + if (Ann->getAnnotation() == "objc_instance_variable_invalidator") + return true; + } + return false; +} + +void IvarInvalidationChecker::containsInvalidationMethod( + const ObjCContainerDecl *D, IvarInfo &OutInfo) { + + // TODO: Cache the results. + + if (!D) + return; + + // Check all methods. + for (ObjCContainerDecl::method_iterator + I = D->meth_begin(), + E = D->meth_end(); I != E; ++I) { + const ObjCMethodDecl *MDI = *I; + if (isInvalidationMethod(MDI)) + OutInfo.addInvalidationMethod( + cast<ObjCMethodDecl>(MDI->getCanonicalDecl())); + } + + // If interface, check all parent protocols and super. + // TODO: Visit all categories in case the invalidation method is declared in + // a category. + if (const ObjCInterfaceDecl *InterfaceD = dyn_cast<ObjCInterfaceDecl>(D)) { + for (ObjCInterfaceDecl::protocol_iterator + I = InterfaceD->protocol_begin(), + E = InterfaceD->protocol_end(); I != E; ++I) { + containsInvalidationMethod(*I, OutInfo); + } + containsInvalidationMethod(InterfaceD->getSuperClass(), OutInfo); + return; + } + + // If protocol, check all parent protocols. + if (const ObjCProtocolDecl *ProtD = dyn_cast<ObjCProtocolDecl>(D)) { + for (ObjCInterfaceDecl::protocol_iterator + I = ProtD->protocol_begin(), + E = ProtD->protocol_end(); I != E; ++I) { + containsInvalidationMethod(*I, OutInfo); + } + return; + } + + llvm_unreachable("One of the casts above should have succeeded."); +} + +bool IvarInvalidationChecker::trackIvar(const ObjCIvarDecl *Iv, + IvarSet &TrackedIvars) { + QualType IvQTy = Iv->getType(); + const ObjCObjectPointerType *IvTy = IvQTy->getAs<ObjCObjectPointerType>(); + if (!IvTy) + return false; + const ObjCInterfaceDecl *IvInterf = IvTy->getInterfaceDecl(); + + IvarInfo Info; + containsInvalidationMethod(IvInterf, Info); + if (Info.needsInvalidation()) { + TrackedIvars[cast<ObjCIvarDecl>(Iv->getCanonicalDecl())] = Info; + return true; + } + return false; +} + +const ObjCIvarDecl *IvarInvalidationChecker::findPropertyBackingIvar( + const ObjCPropertyDecl *Prop, + const ObjCInterfaceDecl *InterfaceD, + IvarSet &TrackedIvars) { + const ObjCIvarDecl *IvarD = 0; + + // Lookup for the synthesized case. + IvarD = Prop->getPropertyIvarDecl(); + if (IvarD) { + if (TrackedIvars.count(IvarD)) { + return IvarD; + } + // If the ivar is synthesized we still want to track it. + if (trackIvar(IvarD, TrackedIvars)) + return IvarD; + } + + // Lookup IVars named "_PropName"or "PropName" among the tracked Ivars. + StringRef PropName = Prop->getIdentifier()->getName(); + for (IvarSet::const_iterator I = TrackedIvars.begin(), + E = TrackedIvars.end(); I != E; ++I) { + const ObjCIvarDecl *Iv = I->first; + StringRef IvarName = Iv->getName(); + + if (IvarName == PropName) + return Iv; + + SmallString<128> PropNameWithUnderscore; + { + llvm::raw_svector_ostream os(PropNameWithUnderscore); + os << '_' << PropName; + } + if (IvarName == PropNameWithUnderscore.str()) + return Iv; + } + + // Note, this is a possible source of false positives. We could look at the + // getter implementation to find the ivar when its name is not derived from + // the property name. + return 0; +} + +void IvarInvalidationChecker::checkASTDecl(const ObjCMethodDecl *D, + AnalysisManager& Mgr, + BugReporter &BR) const { + // We are only interested in checking the cleanup methods. + if (!D->hasBody() || !isInvalidationMethod(D)) + return; + + // Collect all ivars that need cleanup. + IvarSet Ivars; + const ObjCInterfaceDecl *InterfaceD = D->getClassInterface(); + + // Collect ivars declared in this class, its extensions and its implementation + ObjCInterfaceDecl *IDecl = const_cast<ObjCInterfaceDecl *>(InterfaceD); + for (const ObjCIvarDecl *Iv = IDecl->all_declared_ivar_begin(); Iv; + Iv= Iv->getNextIvar()) + trackIvar(Iv, Ivars); + + // Construct Property/Property Accessor to Ivar maps to assist checking if an + // ivar which is backing a property has been reset. + MethToIvarMapTy PropSetterToIvarMap; + MethToIvarMapTy PropGetterToIvarMap; + PropToIvarMapTy PropertyToIvarMap; + IvarToPropMapTy IvarToPopertyMap; + + ObjCInterfaceDecl::PropertyMap PropMap; + InterfaceD->collectPropertiesToImplement(PropMap); + + for (ObjCInterfaceDecl::PropertyMap::iterator + I = PropMap.begin(), E = PropMap.end(); I != E; ++I) { + const ObjCPropertyDecl *PD = I->second; + + const ObjCIvarDecl *ID = findPropertyBackingIvar(PD, InterfaceD, Ivars); + if (!ID) { + continue; + } + + // Store the mappings. + PD = cast<ObjCPropertyDecl>(PD->getCanonicalDecl()); + PropertyToIvarMap[PD] = ID; + IvarToPopertyMap[ID] = PD; + + // Find the setter and the getter. + const ObjCMethodDecl *SetterD = PD->getSetterMethodDecl(); + if (SetterD) { + SetterD = cast<ObjCMethodDecl>(SetterD->getCanonicalDecl()); + PropSetterToIvarMap[SetterD] = ID; + } + + const ObjCMethodDecl *GetterD = PD->getGetterMethodDecl(); + if (GetterD) { + GetterD = cast<ObjCMethodDecl>(GetterD->getCanonicalDecl()); + PropGetterToIvarMap[GetterD] = ID; + } + } + + + // Check which ivars have been invalidated in the method body. + bool CalledAnotherInvalidationMethod = false; + MethodCrawler(Ivars, + CalledAnotherInvalidationMethod, + PropSetterToIvarMap, + PropGetterToIvarMap, + PropertyToIvarMap, + BR.getContext()).VisitStmt(D->getBody()); + + if (CalledAnotherInvalidationMethod) + return; + + // Warn on the ivars that were not accessed by the method. + for (IvarSet::const_iterator I = Ivars.begin(), E = Ivars.end(); I != E; ++I){ + if (!I->second.isInvalidated()) { + const ObjCIvarDecl *IvarDecl = I->first; + + PathDiagnosticLocation IvarDecLocation = + PathDiagnosticLocation::createEnd(D->getBody(), BR.getSourceManager(), + Mgr.getAnalysisDeclContext(D)); + + SmallString<128> sbuf; + llvm::raw_svector_ostream os(sbuf); + + // Construct the warning message. + if (IvarDecl->getSynthesize()) { + const ObjCPropertyDecl *PD = IvarToPopertyMap[IvarDecl]; + assert(PD && + "Do we synthesize ivars for something other than properties?"); + os << "Property "<< PD->getName() << + " needs to be invalidated or set to nil"; + } else { + os << "Instance variable "<< IvarDecl->getName() + << " needs to be invalidated or set to nil"; + } + + BR.EmitBasicReport(D, + "Incomplete invalidation", + categories::CoreFoundationObjectiveC, os.str(), + IvarDecLocation); + } + } +} + +void IvarInvalidationChecker::MethodCrawler::markInvalidated( + const ObjCIvarDecl *Iv) { + IvarSet::iterator I = IVars.find(Iv); + if (I != IVars.end()) { + // 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) + I->second.markInvalidated(InvalidationMethod); + else + I->second.markInvalidated(); + } +} + +const Expr *IvarInvalidationChecker::MethodCrawler::peel(const Expr *E) const { + E = E->IgnoreParenCasts(); + if (const PseudoObjectExpr *POE = dyn_cast<PseudoObjectExpr>(E)) + E = POE->getSyntacticForm()->IgnoreParenCasts(); + if (const OpaqueValueExpr *OVE = dyn_cast<OpaqueValueExpr>(E)) + E = OVE->getSourceExpr()->IgnoreParenCasts(); + return E; +} + +void IvarInvalidationChecker::MethodCrawler::checkObjCIvarRefExpr( + const ObjCIvarRefExpr *IvarRef) { + if (const Decl *D = IvarRef->getDecl()) + markInvalidated(cast<ObjCIvarDecl>(D->getCanonicalDecl())); +} + +void IvarInvalidationChecker::MethodCrawler::checkObjCMessageExpr( + const ObjCMessageExpr *ME) { + const ObjCMethodDecl *MD = ME->getMethodDecl(); + if (MD) { + MD = cast<ObjCMethodDecl>(MD->getCanonicalDecl()); + MethToIvarMapTy::const_iterator IvI = PropertyGetterToIvarMap.find(MD); + if (IvI != PropertyGetterToIvarMap.end()) + markInvalidated(IvI->second); + } +} + +void IvarInvalidationChecker::MethodCrawler::checkObjCPropertyRefExpr( + const ObjCPropertyRefExpr *PA) { + + if (PA->isExplicitProperty()) { + const ObjCPropertyDecl *PD = PA->getExplicitProperty(); + if (PD) { + PD = cast<ObjCPropertyDecl>(PD->getCanonicalDecl()); + PropToIvarMapTy::const_iterator IvI = PropertyToIvarMap.find(PD); + if (IvI != PropertyToIvarMap.end()) + markInvalidated(IvI->second); + return; + } + } + + if (PA->isImplicitProperty()) { + const ObjCMethodDecl *MD = PA->getImplicitPropertySetter(); + if (MD) { + MD = cast<ObjCMethodDecl>(MD->getCanonicalDecl()); + MethToIvarMapTy::const_iterator IvI =PropertyGetterToIvarMap.find(MD); + if (IvI != PropertyGetterToIvarMap.end()) + markInvalidated(IvI->second); + return; + } + } +} + +bool IvarInvalidationChecker::MethodCrawler::isZero(const Expr *E) const { + E = peel(E); + + return (E->isNullPointerConstant(Ctx, Expr::NPC_ValueDependentIsNotNull) + != Expr::NPCK_NotNull); +} + +void IvarInvalidationChecker::MethodCrawler::check(const Expr *E) { + E = peel(E); + + if (const ObjCIvarRefExpr *IvarRef = dyn_cast<ObjCIvarRefExpr>(E)) { + checkObjCIvarRefExpr(IvarRef); + return; + } + + if (const ObjCPropertyRefExpr *PropRef = dyn_cast<ObjCPropertyRefExpr>(E)) { + checkObjCPropertyRefExpr(PropRef); + return; + } + + if (const ObjCMessageExpr *MsgExpr = dyn_cast<ObjCMessageExpr>(E)) { + checkObjCMessageExpr(MsgExpr); + return; + } +} + +void IvarInvalidationChecker::MethodCrawler::VisitBinaryOperator( + const BinaryOperator *BO) { + VisitStmt(BO); + + if (BO->getOpcode() != BO_Assign) + return; + + // Do we assign zero? + if (!isZero(BO->getRHS())) + return; + + // Check the variable we are assigning to. + check(BO->getLHS()); +} + +void IvarInvalidationChecker::MethodCrawler::VisitObjCMessageExpr( + const ObjCMessageExpr *ME) { + const ObjCMethodDecl *MD = ME->getMethodDecl(); + const Expr *Receiver = ME->getInstanceReceiver(); + + // Stop if we are calling '[self invalidate]'. + if (Receiver && isInvalidationMethod(MD)) + if (Receiver->isObjCSelfExpr()) { + CalledAnotherInvalidationMethod = true; + return; + } + + // Check if we call a setter and set the property to 'nil'. + if (MD && (ME->getNumArgs() == 1) && isZero(ME->getArg(0))) { + MD = cast<ObjCMethodDecl>(MD->getCanonicalDecl()); + MethToIvarMapTy::const_iterator IvI = PropertySetterToIvarMap.find(MD); + if (IvI != PropertySetterToIvarMap.end()) { + markInvalidated(IvI->second); + return; + } + } + + // Check if we call the 'invalidation' routine on the ivar. + if (Receiver) { + InvalidationMethod = MD; + check(Receiver->IgnoreParenCasts()); + InvalidationMethod = 0; + } + + VisitStmt(ME); +} +} + +// Register the checker. +void ento::registerIvarInvalidationChecker(CheckerManager &mgr) { + mgr.registerChecker<IvarInvalidationChecker>(); +} diff --git a/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp b/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp index 969f2dd..76f20b6 100644 --- a/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp @@ -158,16 +158,9 @@ private: /// ProgramState traits to store the currently allocated (and not yet freed) /// symbols. This is a map from the allocated content symbol to the /// corresponding AllocationState. -typedef llvm::ImmutableMap<SymbolRef, - MacOSKeychainAPIChecker::AllocationState> AllocatedSetTy; - -namespace { struct AllocatedData {}; } -namespace clang { namespace ento { -template<> struct ProgramStateTrait<AllocatedData> - : public ProgramStatePartialTrait<AllocatedSetTy > { - static void *GDMIndex() { static int index = 0; return &index; } -}; -}} +REGISTER_MAP_WITH_PROGRAMSTATE(AllocatedData, + SymbolRef, + MacOSKeychainAPIChecker::AllocationState) static bool isEnclosingFunctionParam(const Expr *E) { E = E->IgnoreParenCasts(); @@ -282,7 +275,7 @@ void MacOSKeychainAPIChecker:: Report->addVisitor(new SecKeychainBugVisitor(AP.first)); Report->addRange(ArgExpr->getSourceRange()); markInteresting(Report, AP); - C.EmitReport(Report); + C.emitReport(Report); } void MacOSKeychainAPIChecker::checkPreStmt(const CallExpr *CE, @@ -323,7 +316,7 @@ void MacOSKeychainAPIChecker::checkPreStmt(const CallExpr *CE, Report->addVisitor(new SecKeychainBugVisitor(V)); Report->addRange(ArgExpr->getSourceRange()); Report->markInteresting(AS->Region); - C.EmitReport(Report); + C.emitReport(Report); } } return; @@ -376,7 +369,7 @@ void MacOSKeychainAPIChecker::checkPreStmt(const CallExpr *CE, Report->addRange(ArgExpr->getSourceRange()); if (AS) Report->markInteresting(AS->Region); - C.EmitReport(Report); + C.emitReport(Report); return; } @@ -440,7 +433,7 @@ void MacOSKeychainAPIChecker::checkPreStmt(const CallExpr *CE, Report->addVisitor(new SecKeychainBugVisitor(ArgSM)); Report->addRange(ArgExpr->getSourceRange()); Report->markInteresting(AS->Region); - C.EmitReport(Report); + C.emitReport(Report); return; } @@ -571,13 +564,13 @@ BugReport *MacOSKeychainAPIChecker:: void MacOSKeychainAPIChecker::checkDeadSymbols(SymbolReaper &SR, CheckerContext &C) const { ProgramStateRef State = C.getState(); - AllocatedSetTy ASet = State->get<AllocatedData>(); + AllocatedDataTy ASet = State->get<AllocatedData>(); if (ASet.isEmpty()) return; bool Changed = false; AllocationPairVec Errors; - for (AllocatedSetTy::iterator I = ASet.begin(), E = ASet.end(); I != E; ++I) { + for (AllocatedDataTy::iterator I = ASet.begin(), E = ASet.end(); I != E; ++I) { if (SR.isLive(I->first)) continue; @@ -585,7 +578,9 @@ void MacOSKeychainAPIChecker::checkDeadSymbols(SymbolReaper &SR, State = State->remove<AllocatedData>(I->first); // If the allocated symbol is null or if the allocation call might have // returned an error, do not report. - if (State->getSymVal(I->first) || + ConstraintManager &CMgr = State->getConstraintManager(); + ConditionTruthVal AllocFailed = CMgr.isNull(State, I.getKey()); + if (AllocFailed.isConstrainedTrue() || definitelyReturnedError(I->second.Region, State, C.getSValBuilder())) continue; Errors.push_back(std::make_pair(I->first, &I->second)); @@ -602,7 +597,7 @@ void MacOSKeychainAPIChecker::checkDeadSymbols(SymbolReaper &SR, // Generate the error reports. for (AllocationPairVec::iterator I = Errors.begin(), E = Errors.end(); I != E; ++I) { - C.EmitReport(generateAllocatedDataNotReleasedReport(*I, N, C)); + C.emitReport(generateAllocatedDataNotReleasedReport(*I, N, C)); } // Generate the new, cleaned up state. @@ -617,7 +612,7 @@ void MacOSKeychainAPIChecker::checkEndPath(CheckerContext &C) const { if (C.getLocationContext()->getParent() != 0) return; - AllocatedSetTy AS = state->get<AllocatedData>(); + AllocatedDataTy AS = state->get<AllocatedData>(); if (AS.isEmpty()) return; @@ -625,12 +620,14 @@ void MacOSKeychainAPIChecker::checkEndPath(CheckerContext &C) const { // found here, so report it. bool Changed = false; AllocationPairVec Errors; - for (AllocatedSetTy::iterator I = AS.begin(), E = AS.end(); I != E; ++I ) { + for (AllocatedDataTy::iterator I = AS.begin(), E = AS.end(); I != E; ++I ) { Changed = true; state = state->remove<AllocatedData>(I->first); // If the allocated symbol is null or if error code was returned at // allocation, do not report. - if (state->getSymVal(I.getKey()) || + ConstraintManager &CMgr = state->getConstraintManager(); + ConditionTruthVal AllocFailed = CMgr.isNull(state, I.getKey()); + if (AllocFailed.isConstrainedTrue() || definitelyReturnedError(I->second.Region, state, C.getSValBuilder())) { continue; @@ -650,7 +647,7 @@ void MacOSKeychainAPIChecker::checkEndPath(CheckerContext &C) const { // Generate the error reports. for (AllocationPairVec::iterator I = Errors.begin(), E = Errors.end(); I != E; ++I) { - C.EmitReport(generateAllocatedDataNotReleasedReport(*I, N, C)); + C.emitReport(generateAllocatedDataNotReleasedReport(*I, N, C)); } C.addTransition(state, N); diff --git a/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp b/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp index cfdb55d..467b8b1 100644 --- a/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp @@ -70,6 +70,16 @@ void MacOSXAPIChecker::CheckDispatchOnce(CheckerContext &C, const CallExpr *CE, BT_dispatchOnce.reset(new BugType("Improper use of 'dispatch_once'", "Mac OS X API")); + // Handle _dispatch_once. In some versions of the OS X SDK we have the case + // that dispatch_once is a macro that wraps a call to _dispatch_once. + // _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("_"); + if (TrimmedFName != FName) + FName = TrimmedFName; + } + SmallString<256> S; llvm::raw_svector_ostream os(S); os << "Call to '" << FName << "' uses"; @@ -84,7 +94,7 @@ void MacOSXAPIChecker::CheckDispatchOnce(CheckerContext &C, const CallExpr *CE, BugReport *report = new BugReport(*BT_dispatchOnce, os.str(), N); report->addRange(CE->getArg(0)->getSourceRange()); - C.EmitReport(report); + C.emitReport(report); } //===----------------------------------------------------------------------===// @@ -99,7 +109,9 @@ void MacOSXAPIChecker::checkPreStmt(const CallExpr *CE, SubChecker SC = llvm::StringSwitch<SubChecker>(Name) - .Cases("dispatch_once", "dispatch_once_f", + .Cases("dispatch_once", + "_dispatch_once", + "dispatch_once_f", &MacOSXAPIChecker::CheckDispatchOnce) .Default(NULL); diff --git a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp index dfcedf6..caf70ca 100644 --- a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -26,6 +26,7 @@ #include "llvm/ADT/ImmutableMap.h" #include "llvm/ADT/SmallString.h" #include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/StringExtras.h" #include <climits> using namespace clang; @@ -70,17 +71,31 @@ public: } }; +enum ReallocPairKind { + RPToBeFreedAfterFailure, + // The symbol has been freed when reallocation failed. + RPIsFreeOnFailure, + // The symbol does not need to be freed after reallocation fails. + RPDoNotTrackAfterFailure +}; + +/// \class ReallocPair +/// \brief Stores information about the symbol being reallocated by a call to +/// 'realloc' to allow modeling failed reallocation later in the path. struct ReallocPair { + // \brief The symbol which realloc reallocated. SymbolRef ReallocatedSym; - bool IsFreeOnFailure; - ReallocPair(SymbolRef S, bool F) : ReallocatedSym(S), IsFreeOnFailure(F) {} + ReallocPairKind Kind; + + ReallocPair(SymbolRef S, ReallocPairKind K) : + ReallocatedSym(S), Kind(K) {} void Profile(llvm::FoldingSetNodeID &ID) const { - ID.AddInteger(IsFreeOnFailure); + ID.AddInteger(Kind); ID.AddPointer(ReallocatedSym); } bool operator==(const ReallocPair &X) const { return ReallocatedSym == X.ReallocatedSym && - IsFreeOnFailure == X.IsFreeOnFailure; + Kind == X.Kind; } }; @@ -92,7 +107,7 @@ class MallocChecker : public Checker<check::DeadSymbols, check::PreStmt<CallExpr>, check::PostStmt<CallExpr>, check::PostStmt<BlockExpr>, - check::PreObjCMessage, + check::PostObjCMessage, check::Location, check::Bind, eval::Assume, @@ -120,7 +135,7 @@ public: void checkPreStmt(const CallExpr *S, CheckerContext &C) const; void checkPostStmt(const CallExpr *CE, CheckerContext &C) const; - void checkPreObjCMessage(const ObjCMethodCall &Call, CheckerContext &C) const; + void checkPostObjCMessage(const ObjCMethodCall &Call, CheckerContext &C) const; void checkPostStmt(const BlockExpr *BE, CheckerContext &C) const; void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const; void checkEndPath(CheckerContext &C) const; @@ -177,11 +192,15 @@ private: const OwnershipAttr* Att) const; ProgramStateRef FreeMemAux(CheckerContext &C, const CallExpr *CE, ProgramStateRef state, unsigned Num, - bool Hold) const; + bool Hold, + bool &ReleasedAllocated, + bool ReturnsNullOnFailure = false) const; ProgramStateRef FreeMemAux(CheckerContext &C, const Expr *Arg, const Expr *ParentExpr, - ProgramStateRef state, - bool Hold) const; + ProgramStateRef State, + bool Hold, + bool &ReleasedAllocated, + bool ReturnsNullOnFailure = false) const; ProgramStateRef ReallocMem(CheckerContext &C, const CallExpr *CE, bool FreesMemOnFailure) const; @@ -301,13 +320,14 @@ private: : StackHintGeneratorForSymbol(S, M) {} virtual std::string getMessageForArg(const Expr *ArgE, unsigned ArgIndex) { + // Printed parameters start at 1, not 0. + ++ArgIndex; + SmallString<200> buf; llvm::raw_svector_ostream os(buf); - os << "Reallocation of "; - // Printed parameters start at 1, not 0. - printOrdinal(++ArgIndex, os); - os << " parameter failed"; + os << "Reallocation of " << ArgIndex << llvm::getOrdinalSuffix(ArgIndex) + << " parameter failed"; return os.str(); } @@ -320,25 +340,12 @@ private: }; } // end anonymous namespace -typedef llvm::ImmutableMap<SymbolRef, RefState> RegionStateTy; -typedef llvm::ImmutableMap<SymbolRef, ReallocPair > ReallocMap; -class RegionState {}; -class ReallocPairs {}; -namespace clang { -namespace ento { - template <> - struct ProgramStateTrait<RegionState> - : public ProgramStatePartialTrait<RegionStateTy> { - static void *GDMIndex() { static int x; return &x; } - }; +REGISTER_MAP_WITH_PROGRAMSTATE(RegionState, SymbolRef, RefState) +REGISTER_MAP_WITH_PROGRAMSTATE(ReallocPairs, SymbolRef, ReallocPair) - template <> - struct ProgramStateTrait<ReallocPairs> - : public ProgramStatePartialTrait<ReallocMap> { - static void *GDMIndex() { static int x; return &x; } - }; -} -} +// A map from the freed symbol to the symbol representing the return value of +// the free function. +REGISTER_MAP_WITH_PROGRAMSTATE(FreeReturnValue, SymbolRef, SymbolRef) namespace { class StopTrackingCallback : public SymbolVisitor { @@ -426,11 +433,15 @@ bool MallocChecker::isFreeFunction(const FunctionDecl *FD, ASTContext &C) const } void MallocChecker::checkPostStmt(const CallExpr *CE, CheckerContext &C) const { + if (C.wasInlined) + return; + const FunctionDecl *FD = C.getCalleeDecl(CE); if (!FD) return; ProgramStateRef State = C.getState(); + bool ReleasedAllocatedMemory = false; if (FD->getKind() == Decl::Function) { initIdentifierInfo(C.getASTContext()); @@ -447,7 +458,7 @@ void MallocChecker::checkPostStmt(const CallExpr *CE, CheckerContext &C) const { } else if (FunI == II_calloc) { State = CallocMem(C, CE); } else if (FunI == II_free) { - State = FreeMemAux(C, CE, State, 0, false); + State = FreeMemAux(C, CE, State, 0, false, ReleasedAllocatedMemory); } else if (FunI == II_strdup) { State = MallocUpdateRefState(C, CE, State); } else if (FunI == II_strndup) { @@ -487,21 +498,26 @@ static bool isFreeWhenDoneSetToZero(const ObjCMethodCall &Call) { return false; } -void MallocChecker::checkPreObjCMessage(const ObjCMethodCall &Call, - CheckerContext &C) const { +void MallocChecker::checkPostObjCMessage(const ObjCMethodCall &Call, + CheckerContext &C) const { // If the first selector is dataWithBytesNoCopy, assume that the memory will // be released with 'free' by the new object. // Ex: [NSData dataWithBytesNoCopy:bytes length:10]; // Unless 'freeWhenDone' param set to 0. // TODO: Check that the memory was allocated with malloc. + bool ReleasedAllocatedMemory = false; Selector S = Call.getSelector(); if ((S.getNameForSlot(0) == "dataWithBytesNoCopy" || S.getNameForSlot(0) == "initWithBytesNoCopy" || S.getNameForSlot(0) == "initWithCharactersNoCopy") && !isFreeWhenDoneSetToZero(Call)){ unsigned int argIdx = 0; - C.addTransition(FreeMemAux(C, Call.getArgExpr(argIdx), - Call.getOriginExpr(), C.getState(), true)); + ProgramStateRef State = FreeMemAux(C, Call.getArgExpr(argIdx), + Call.getOriginExpr(), C.getState(), true, + ReleasedAllocatedMemory, + /* RetNullOnFailure*/ true); + + C.addTransition(State); } } @@ -526,7 +542,7 @@ ProgramStateRef MallocChecker::MallocMemAux(CheckerContext &C, // Bind the return value to the symbolic value from the heap region. // TODO: We could rewrite post visit to eval call; 'malloc' does not have // side effects other than what we model here. - unsigned Count = C.getCurrentBlockCount(); + unsigned Count = C.blockCount(); SValBuilder &svalBuilder = C.getSValBuilder(); const LocationContext *LCtx = C.getPredecessor()->getLocationContext(); DefinedSVal RetVal = @@ -584,11 +600,13 @@ ProgramStateRef MallocChecker::FreeMemAttr(CheckerContext &C, return 0; ProgramStateRef State = C.getState(); + bool ReleasedAllocated = false; for (OwnershipAttr::args_iterator I = Att->args_begin(), E = Att->args_end(); I != E; ++I) { ProgramStateRef StateI = FreeMemAux(C, CE, State, *I, - Att->getOwnKind() == OwnershipAttr::Holds); + Att->getOwnKind() == OwnershipAttr::Holds, + ReleasedAllocated); if (StateI) State = StateI; } @@ -599,20 +617,40 @@ ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C, const CallExpr *CE, ProgramStateRef state, unsigned Num, - bool Hold) const { + bool Hold, + bool &ReleasedAllocated, + bool ReturnsNullOnFailure) const { if (CE->getNumArgs() < (Num + 1)) return 0; - return FreeMemAux(C, CE->getArg(Num), CE, state, Hold); + return FreeMemAux(C, CE->getArg(Num), CE, state, Hold, + ReleasedAllocated, ReturnsNullOnFailure); +} + +/// Checks if the previous call to free on the given symbol failed - if free +/// failed, returns true. Also, returns the corresponding return value symbol. +bool didPreviousFreeFail(ProgramStateRef State, + SymbolRef Sym, SymbolRef &RetStatusSymbol) { + const SymbolRef *Ret = State->get<FreeReturnValue>(Sym); + if (Ret) { + assert(*Ret && "We should not store the null return symbol"); + ConstraintManager &CMgr = State->getConstraintManager(); + ConditionTruthVal FreeFailed = CMgr.isNull(State, *Ret); + RetStatusSymbol = *Ret; + return FreeFailed.isConstrainedTrue(); + } + return false; } ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C, const Expr *ArgExpr, const Expr *ParentExpr, - ProgramStateRef state, - bool Hold) const { + ProgramStateRef State, + bool Hold, + bool &ReleasedAllocated, + bool ReturnsNullOnFailure) const { - SVal ArgVal = state->getSVal(ArgExpr, C.getLocationContext()); + SVal ArgVal = State->getSVal(ArgExpr, C.getLocationContext()); if (!isa<DefinedOrUnknownSVal>(ArgVal)) return 0; DefinedOrUnknownSVal location = cast<DefinedOrUnknownSVal>(ArgVal); @@ -623,7 +661,7 @@ ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C, // The explicit NULL case, no operation is performed. ProgramStateRef notNullState, nullState; - llvm::tie(notNullState, nullState) = state->assume(location); + llvm::tie(notNullState, nullState) = State->assume(location); if (nullState && !notNullState) return 0; @@ -672,10 +710,14 @@ ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C, return 0; SymbolRef Sym = SR->getSymbol(); - const RefState *RS = state->get<RegionState>(Sym); + const RefState *RS = State->get<RegionState>(Sym); + SymbolRef PreviousRetStatusSymbol = 0; // Check double free. - if (RS && (RS->isReleased() || RS->isRelinquished())) { + if (RS && + (RS->isReleased() || RS->isRelinquished()) && + !didPreviousFreeFail(State, Sym, PreviousRetStatusSymbol)) { + if (ExplodedNode *N = C.generateSink()) { if (!BT_DoubleFree) BT_DoubleFree.reset( @@ -685,16 +727,34 @@ ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C, "Attempt to free non-owned memory"), N); R->addRange(ArgExpr->getSourceRange()); R->markInteresting(Sym); + if (PreviousRetStatusSymbol) + R->markInteresting(PreviousRetStatusSymbol); R->addVisitor(new MallocBugVisitor(Sym)); - C.EmitReport(R); + C.emitReport(R); } return 0; } + ReleasedAllocated = (RS != 0); + + // Clean out the info on previous call to free return info. + State = State->remove<FreeReturnValue>(Sym); + + // Keep track of the return value. If it is NULL, we will know that free + // failed. + if (ReturnsNullOnFailure) { + SVal RetVal = C.getSVal(ParentExpr); + SymbolRef RetStatusSymbol = RetVal.getAsSymbol(); + if (RetStatusSymbol) { + C.getSymbolManager().addSymbolDependency(Sym, RetStatusSymbol); + State = State->set<FreeReturnValue>(Sym, RetStatusSymbol); + } + } + // Normal free. if (Hold) - return state->set<RegionState>(Sym, RefState::getRelinquished(ParentExpr)); - return state->set<RegionState>(Sym, RefState::getReleased(ParentExpr)); + return State->set<RegionState>(Sym, RefState::getRelinquished(ParentExpr)); + return State->set<RegionState>(Sym, RefState::getReleased(ParentExpr)); } bool MallocChecker::SummarizeValue(raw_ostream &os, SVal V) { @@ -714,7 +774,7 @@ bool MallocChecker::SummarizeRegion(raw_ostream &os, const MemRegion *MR) { switch (MR->getKind()) { case MemRegion::FunctionTextRegionKind: { - const FunctionDecl *FD = cast<FunctionTextRegion>(MR)->getDecl(); + const NamedDecl *FD = cast<FunctionTextRegion>(MR)->getDecl(); if (FD) os << "the address of the function '" << *FD << '\''; else @@ -819,7 +879,7 @@ void MallocChecker::ReportBadFree(CheckerContext &C, SVal ArgVal, BugReport *R = new BugReport(*BT_BadFree, os.str(), N); R->markInteresting(MR); R->addRange(range); - C.EmitReport(R); + C.emitReport(R); } } @@ -886,9 +946,12 @@ ProgramStateRef MallocChecker::ReallocMem(CheckerContext &C, if (!FromPtr || !ToPtr) return 0; + bool ReleasedAllocated = false; + // If the size is 0, free the memory. if (SizeIsZero) - if (ProgramStateRef stateFree = FreeMemAux(C, CE, StateSizeIsZero,0,false)){ + if (ProgramStateRef stateFree = FreeMemAux(C, CE, StateSizeIsZero, 0, + false, ReleasedAllocated)){ // The semantics of the return value are: // If size was equal to 0, either NULL or a pointer suitable to be passed // to free() is returned. We just free the input pointer and do not add @@ -897,14 +960,25 @@ ProgramStateRef MallocChecker::ReallocMem(CheckerContext &C, } // Default behavior. - if (ProgramStateRef stateFree = FreeMemAux(C, CE, state, 0, false)) { - // FIXME: We should copy the content of the original buffer. + if (ProgramStateRef stateFree = + FreeMemAux(C, CE, state, 0, false, ReleasedAllocated)) { + ProgramStateRef stateRealloc = MallocMemAux(C, CE, CE->getArg(1), UnknownVal(), stateFree); if (!stateRealloc) return 0; + + ReallocPairKind Kind = RPToBeFreedAfterFailure; + if (FreesOnFail) + Kind = RPIsFreeOnFailure; + else if (!ReleasedAllocated) + Kind = RPDoNotTrackAfterFailure; + + // Record the info about the reallocated symbol so that we could properly + // process failed reallocation. stateRealloc = stateRealloc->set<ReallocPairs>(ToPtr, - ReallocPair(FromPtr, FreesOnFail)); + ReallocPair(FromPtr, Kind)); + // The reallocated symbol should stay alive for as long as the new symbol. C.getSymbolManager().addSymbolDependency(ToPtr, FromPtr); return stateRealloc; } @@ -1004,7 +1078,7 @@ void MallocChecker::reportLeak(SymbolRef Sym, ExplodedNode *N, BugReport *R = new BugReport(*BT_Leak, os.str(), N, LocUsedForUniqueing); R->markInteresting(Sym); R->addVisitor(new MallocBugVisitor(Sym, true)); - C.EmitReport(R); + C.emitReport(R); } void MallocChecker::checkDeadSymbols(SymbolReaper &SymReaper, @@ -1017,14 +1091,11 @@ void MallocChecker::checkDeadSymbols(SymbolReaper &SymReaper, RegionStateTy RS = state->get<RegionState>(); RegionStateTy::Factory &F = state->get_context<RegionState>(); - bool generateReport = false; llvm::SmallVector<SymbolRef, 2> Errors; for (RegionStateTy::iterator I = RS.begin(), E = RS.end(); I != E; ++I) { if (SymReaper.isDead(I->first)) { - if (I->second.isAllocated()) { - generateReport = true; + if (I->second.isAllocated()) Errors.push_back(I->first); - } // Remove the dead symbol from the map. RS = F.remove(RS, I->first); @@ -1032,24 +1103,34 @@ void MallocChecker::checkDeadSymbols(SymbolReaper &SymReaper, } // Cleanup the Realloc Pairs Map. - ReallocMap RP = state->get<ReallocPairs>(); - for (ReallocMap::iterator I = RP.begin(), E = RP.end(); I != E; ++I) { + ReallocPairsTy RP = state->get<ReallocPairs>(); + for (ReallocPairsTy::iterator I = RP.begin(), E = RP.end(); I != E; ++I) { if (SymReaper.isDead(I->first) || SymReaper.isDead(I->second.ReallocatedSym)) { state = state->remove<ReallocPairs>(I->first); } } - // Generate leak node. - static SimpleProgramPointTag Tag("MallocChecker : DeadSymbolsLeak"); - ExplodedNode *N = C.addTransition(C.getState(), C.getPredecessor(), &Tag); + // Cleanup the FreeReturnValue Map. + FreeReturnValueTy FR = state->get<FreeReturnValue>(); + for (FreeReturnValueTy::iterator I = FR.begin(), E = FR.end(); I != E; ++I) { + if (SymReaper.isDead(I->first) || + SymReaper.isDead(I->second)) { + state = state->remove<FreeReturnValue>(I->first); + } + } - if (generateReport) { + // Generate leak node. + ExplodedNode *N = C.getPredecessor(); + if (!Errors.empty()) { + static SimpleProgramPointTag Tag("MallocChecker : DeadSymbolsLeak"); + N = C.addTransition(C.getState(), C.getPredecessor(), &Tag); for (llvm::SmallVector<SymbolRef, 2>::iterator - I = Errors.begin(), E = Errors.end(); I != E; ++I) { + I = Errors.begin(), E = Errors.end(); I != E; ++I) { reportLeak(*I, N, C); } } + C.addTransition(state->set<RegionState>(RS), N); } @@ -1182,7 +1263,7 @@ bool MallocChecker::checkUseAfterFree(SymbolRef Sym, CheckerContext &C, R->addRange(S->getSourceRange()); R->markInteresting(Sym); R->addVisitor(new MallocBugVisitor(Sym)); - C.EmitReport(R); + C.emitReport(R); return true; } } @@ -1249,28 +1330,36 @@ ProgramStateRef MallocChecker::evalAssume(ProgramStateRef state, bool Assumption) const { RegionStateTy RS = state->get<RegionState>(); for (RegionStateTy::iterator I = RS.begin(), E = RS.end(); I != E; ++I) { - // If the symbol is assumed to NULL or another constant, this will - // return an APSInt*. - if (state->getSymVal(I.getKey())) + // If the symbol is assumed to be NULL, remove it from consideration. + ConstraintManager &CMgr = state->getConstraintManager(); + ConditionTruthVal AllocFailed = CMgr.isNull(state, I.getKey()); + if (AllocFailed.isConstrainedTrue()) state = state->remove<RegionState>(I.getKey()); } // Realloc returns 0 when reallocation fails, which means that we should // restore the state of the pointer being reallocated. - ReallocMap RP = state->get<ReallocPairs>(); - for (ReallocMap::iterator I = RP.begin(), E = RP.end(); I != E; ++I) { - // If the symbol is assumed to NULL or another constant, this will - // return an APSInt*. - if (state->getSymVal(I.getKey())) { - SymbolRef ReallocSym = I.getData().ReallocatedSym; - const RefState *RS = state->get<RegionState>(ReallocSym); - if (RS) { - if (RS->isReleased() && ! I.getData().IsFreeOnFailure) + ReallocPairsTy RP = state->get<ReallocPairs>(); + for (ReallocPairsTy::iterator I = RP.begin(), E = RP.end(); I != E; ++I) { + // If the symbol is assumed to be NULL, remove it from consideration. + ConstraintManager &CMgr = state->getConstraintManager(); + ConditionTruthVal AllocFailed = CMgr.isNull(state, I.getKey()); + if (!AllocFailed.isConstrainedTrue()) + continue; + + SymbolRef ReallocSym = I.getData().ReallocatedSym; + if (const RefState *RS = state->get<RegionState>(ReallocSym)) { + if (RS->isReleased()) { + if (I.getData().Kind == RPToBeFreedAfterFailure) state = state->set<RegionState>(ReallocSym, - RefState::getAllocated(RS->getStmt())); + RefState::getAllocated(RS->getStmt())); + else if (I.getData().Kind == RPDoNotTrackAfterFailure) + state = state->remove<RegionState>(ReallocSym); + else + assert(I.getData().Kind == RPIsFreeOnFailure); } - state = state->remove<ReallocPairs>(I.getKey()); } + state = state->remove<ReallocPairs>(I.getKey()); } return state; @@ -1463,10 +1552,10 @@ MallocChecker::checkRegionChanges(ProgramStateRef State, static SymbolRef findFailedReallocSymbol(ProgramStateRef currState, ProgramStateRef prevState) { - ReallocMap currMap = currState->get<ReallocPairs>(); - ReallocMap prevMap = prevState->get<ReallocPairs>(); + ReallocPairsTy currMap = currState->get<ReallocPairs>(); + ReallocPairsTy prevMap = prevState->get<ReallocPairs>(); - for (ReallocMap::iterator I = prevMap.begin(), E = prevMap.end(); + for (ReallocPairsTy::iterator I = prevMap.begin(), E = prevMap.end(); I != E; ++I) { SymbolRef sym = I.getKey(); if (!currMap.lookup(sym)) diff --git a/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp b/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp index 6292a47..fb40f22 100644 --- a/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp @@ -146,9 +146,9 @@ static bool typesCompatible(ASTContext &C, QualType A, QualType B) { if (const PointerType *ptrA = A->getAs<PointerType>()) if (const PointerType *ptrB = B->getAs<PointerType>()) { - A = ptrA->getPointeeType(); - B = ptrB->getPointeeType(); - continue; + A = ptrA->getPointeeType(); + B = ptrB->getPointeeType(); + continue; } break; @@ -157,6 +157,18 @@ static bool typesCompatible(ASTContext &C, QualType A, QualType B) { return false; } +static bool compatibleWithArrayType(ASTContext &C, QualType PT, QualType T) { + // Ex: 'int a[10][2]' is compatible with 'int', 'int[2]', 'int[10][2]'. + while (const ArrayType *AT = T->getAsArrayTypeUnsafe()) { + QualType ElemType = AT->getElementType(); + if (typesCompatible(C, PT, AT->getElementType())) + return true; + T = ElemType; + } + + return false; +} + class MallocSizeofChecker : public Checker<check::ASTCodeBody> { public: void checkASTCodeBody(const Decl *D, AnalysisManager& mgr, @@ -184,38 +196,49 @@ public: continue; QualType SizeofType = SFinder.Sizeofs[0]->getTypeOfArgument(); - if (!typesCompatible(BR.getContext(), PointeeType, SizeofType)) { - const TypeSourceInfo *TSI = 0; - if (i->CastedExprParent.is<const VarDecl *>()) { - TSI = + + if (typesCompatible(BR.getContext(), PointeeType, SizeofType)) + continue; + + // If the argument to sizeof is an array, the result could be a + // pointer to any array element. + if (compatibleWithArrayType(BR.getContext(), PointeeType, SizeofType)) + continue; + + const TypeSourceInfo *TSI = 0; + if (i->CastedExprParent.is<const VarDecl *>()) { + TSI = i->CastedExprParent.get<const VarDecl *>()->getTypeSourceInfo(); - } else { - TSI = i->ExplicitCastType; - } - - SmallString<64> buf; - llvm::raw_svector_ostream OS(buf); - - OS << "Result of '" - << i->AllocCall->getDirectCallee()->getIdentifier()->getName() - << "' is converted to a pointer of type '" - << PointeeType.getAsString() << "', which is incompatible with " - << "sizeof operand type '" << SizeofType.getAsString() << "'"; - llvm::SmallVector<SourceRange, 4> Ranges; - Ranges.push_back(i->AllocCall->getCallee()->getSourceRange()); - Ranges.push_back(SFinder.Sizeofs[0]->getSourceRange()); - if (TSI) - Ranges.push_back(TSI->getTypeLoc().getSourceRange()); - - PathDiagnosticLocation L = + } else { + TSI = i->ExplicitCastType; + } + + SmallString<64> buf; + llvm::raw_svector_ostream OS(buf); + + OS << "Result of "; + const FunctionDecl *Callee = i->AllocCall->getDirectCallee(); + if (Callee && Callee->getIdentifier()) + OS << '\'' << Callee->getIdentifier()->getName() << '\''; + else + OS << "call"; + OS << " is converted to a pointer of type '" + << PointeeType.getAsString() << "', which is incompatible with " + << "sizeof operand type '" << SizeofType.getAsString() << "'"; + llvm::SmallVector<SourceRange, 4> Ranges; + Ranges.push_back(i->AllocCall->getCallee()->getSourceRange()); + Ranges.push_back(SFinder.Sizeofs[0]->getSourceRange()); + if (TSI) + Ranges.push_back(TSI->getTypeLoc().getSourceRange()); + + PathDiagnosticLocation L = PathDiagnosticLocation::createBegin(i->AllocCall->getCallee(), - BR.getSourceManager(), ADC); + BR.getSourceManager(), ADC); - BR.EmitBasicReport(D, "Allocator sizeof operand mismatch", - categories::UnixAPI, - OS.str(), - L, Ranges.data(), Ranges.size()); - } + BR.EmitBasicReport(D, "Allocator sizeof operand mismatch", + categories::UnixAPI, + OS.str(), + L, Ranges.data(), Ranges.size()); } } } diff --git a/lib/StaticAnalyzer/Checkers/NSAutoreleasePoolChecker.cpp b/lib/StaticAnalyzer/Checkers/NSAutoreleasePoolChecker.cpp index aad3b0f..3331bc8 100644 --- a/lib/StaticAnalyzer/Checkers/NSAutoreleasePoolChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/NSAutoreleasePoolChecker.cpp @@ -71,7 +71,7 @@ void NSAutoreleasePoolChecker::checkPreObjCMessage(const ObjCMethodCall &msg, BugReport *Report = new BugReport(*BT, "Use -drain instead of -release when " "using NSAutoreleasePool and garbage collection", N); Report->addRange(msg.getSourceRange()); - C.EmitReport(Report); + C.emitReport(Report); } void ento::registerNSAutoreleasePoolChecker(CheckerManager &mgr) { diff --git a/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp b/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp index f826573..7a66ec3 100644 --- a/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp @@ -163,23 +163,9 @@ public: }; } -namespace { struct NSErrorOut {}; } -namespace { struct CFErrorOut {}; } - typedef llvm::ImmutableMap<SymbolRef, unsigned> ErrorOutFlag; - -namespace clang { -namespace ento { - template <> - struct ProgramStateTrait<NSErrorOut> : public ProgramStatePartialTrait<ErrorOutFlag> { - static void *GDMIndex() { static int index = 0; return &index; } - }; - template <> - struct ProgramStateTrait<CFErrorOut> : public ProgramStatePartialTrait<ErrorOutFlag> { - static void *GDMIndex() { static int index = 0; return &index; } - }; -} -} +REGISTER_TRAIT_WITH_PROGRAMSTATE(NSErrorOut, ErrorOutFlag) +REGISTER_TRAIT_WITH_PROGRAMSTATE(CFErrorOut, ErrorOutFlag) template <typename T> static bool hasFlag(SVal val, ProgramStateRef state) { @@ -285,7 +271,7 @@ void NSOrCFErrorDerefChecker::checkEvent(ImplicitNullDerefEvent event) const { bug = new CFErrorDerefBug(); BugReport *report = new BugReport(*bug, os.str(), event.SinkNode); - BR.EmitReport(report); + BR.emitReport(report); } static bool IsNSError(QualType T, IdentifierInfo *II) { diff --git a/lib/StaticAnalyzer/Checkers/OSAtomicChecker.cpp b/lib/StaticAnalyzer/Checkers/OSAtomicChecker.cpp deleted file mode 100644 index 7b724d2..0000000 --- a/lib/StaticAnalyzer/Checkers/OSAtomicChecker.cpp +++ /dev/null @@ -1,218 +0,0 @@ -//=== OSAtomicChecker.cpp - OSAtomic functions evaluator --------*- C++ -*-===// -// -// The LLVM Compiler Infrastructure -// -// This file is distributed under the University of Illinois Open Source -// License. See LICENSE.TXT for details. -// -//===----------------------------------------------------------------------===// -// -// This checker evaluates OSAtomic functions. -// -//===----------------------------------------------------------------------===// - -#include "ClangSACheckers.h" -#include "clang/StaticAnalyzer/Core/Checker.h" -#include "clang/StaticAnalyzer/Core/CheckerManager.h" -#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" -#include "clang/Basic/Builtins.h" - -using namespace clang; -using namespace ento; - -namespace { - -class OSAtomicChecker : public Checker<eval::InlineCall> { -public: - bool inlineCall(const CallExpr *CE, ExprEngine &Eng, - ExplodedNode *Pred, ExplodedNodeSet &Dst) const; - -private: - bool evalOSAtomicCompareAndSwap(const CallExpr *CE, - ExprEngine &Eng, - ExplodedNode *Pred, - ExplodedNodeSet &Dst) const; -}; -} - -static StringRef getCalleeName(ProgramStateRef State, - const CallExpr *CE, - const LocationContext *LCtx) { - const Expr *Callee = CE->getCallee(); - SVal L = State->getSVal(Callee, LCtx); - const FunctionDecl *funDecl = L.getAsFunctionDecl(); - if (!funDecl) - return StringRef(); - IdentifierInfo *funI = funDecl->getIdentifier(); - if (!funI) - return StringRef(); - return funI->getName(); -} - -bool OSAtomicChecker::inlineCall(const CallExpr *CE, - ExprEngine &Eng, - ExplodedNode *Pred, - ExplodedNodeSet &Dst) const { - StringRef FName = getCalleeName(Pred->getState(), - CE, Pred->getLocationContext()); - if (FName.empty()) - return false; - - // Check for compare and swap. - if (FName.startswith("OSAtomicCompareAndSwap") || - FName.startswith("objc_atomicCompareAndSwap")) - return evalOSAtomicCompareAndSwap(CE, Eng, Pred, Dst); - - // FIXME: Other atomics. - return false; -} - -bool OSAtomicChecker::evalOSAtomicCompareAndSwap(const CallExpr *CE, - ExprEngine &Eng, - ExplodedNode *Pred, - ExplodedNodeSet &Dst) const { - // Not enough arguments to match OSAtomicCompareAndSwap? - if (CE->getNumArgs() != 3) - return false; - - ASTContext &Ctx = Eng.getContext(); - const Expr *oldValueExpr = CE->getArg(0); - QualType oldValueType = Ctx.getCanonicalType(oldValueExpr->getType()); - - const Expr *newValueExpr = CE->getArg(1); - QualType newValueType = Ctx.getCanonicalType(newValueExpr->getType()); - - // Do the types of 'oldValue' and 'newValue' match? - if (oldValueType != newValueType) - return false; - - const Expr *theValueExpr = CE->getArg(2); - const PointerType *theValueType=theValueExpr->getType()->getAs<PointerType>(); - - // theValueType not a pointer? - if (!theValueType) - return false; - - QualType theValueTypePointee = - Ctx.getCanonicalType(theValueType->getPointeeType()).getUnqualifiedType(); - - // The pointee must match newValueType and oldValueType. - if (theValueTypePointee != newValueType) - return false; - - static SimpleProgramPointTag OSAtomicLoadTag("OSAtomicChecker : Load"); - static SimpleProgramPointTag OSAtomicStoreTag("OSAtomicChecker : Store"); - - // Load 'theValue'. - ProgramStateRef state = Pred->getState(); - const LocationContext *LCtx = Pred->getLocationContext(); - ExplodedNodeSet Tmp; - SVal location = state->getSVal(theValueExpr, LCtx); - // Here we should use the value type of the region as the load type, because - // we are simulating the semantics of the function, not the semantics of - // passing argument. So the type of theValue expr is not we are loading. - // But usually the type of the varregion is not the type we want either, - // we still need to do a CastRetrievedVal in store manager. So actually this - // LoadTy specifying can be omitted. But we put it here to emphasize the - // semantics. - QualType LoadTy; - if (const TypedValueRegion *TR = - dyn_cast_or_null<TypedValueRegion>(location.getAsRegion())) { - LoadTy = TR->getValueType(); - } - Eng.evalLoad(Tmp, CE, theValueExpr, Pred, - state, location, &OSAtomicLoadTag, LoadTy); - - if (Tmp.empty()) { - // If no nodes were generated, other checkers must have generated sinks. - // We return an empty Dst. - return true; - } - - for (ExplodedNodeSet::iterator I = Tmp.begin(), E = Tmp.end(); - I != E; ++I) { - - ExplodedNode *N = *I; - ProgramStateRef stateLoad = N->getState(); - - // Use direct bindings from the environment since we are forcing a load - // from a location that the Environment would typically not be used - // to bind a value. - SVal theValueVal_untested = stateLoad->getSVal(theValueExpr, LCtx, true); - - SVal oldValueVal_untested = stateLoad->getSVal(oldValueExpr, LCtx); - - // FIXME: Issue an error. - if (theValueVal_untested.isUndef() || oldValueVal_untested.isUndef()) { - return false; - } - - DefinedOrUnknownSVal theValueVal = - cast<DefinedOrUnknownSVal>(theValueVal_untested); - DefinedOrUnknownSVal oldValueVal = - cast<DefinedOrUnknownSVal>(oldValueVal_untested); - - SValBuilder &svalBuilder = Eng.getSValBuilder(); - - // Perform the comparison. - DefinedOrUnknownSVal Cmp = - svalBuilder.evalEQ(stateLoad,theValueVal,oldValueVal); - - ProgramStateRef stateEqual = stateLoad->assume(Cmp, true); - - // Were they equal? - if (stateEqual) { - // Perform the store. - ExplodedNodeSet TmpStore; - SVal val = stateEqual->getSVal(newValueExpr, LCtx); - - // Handle implicit value casts. - if (const TypedValueRegion *R = - dyn_cast_or_null<TypedValueRegion>(location.getAsRegion())) { - val = svalBuilder.evalCast(val,R->getValueType(), newValueExpr->getType()); - } - - Eng.evalStore(TmpStore, CE, theValueExpr, N, - stateEqual, location, val, &OSAtomicStoreTag); - - if (TmpStore.empty()) { - // If no nodes were generated, other checkers must have generated sinks. - // We return an empty Dst. - return true; - } - - StmtNodeBuilder B(TmpStore, Dst, Eng.getBuilderContext()); - // Now bind the result of the comparison. - for (ExplodedNodeSet::iterator I2 = TmpStore.begin(), - E2 = TmpStore.end(); I2 != E2; ++I2) { - ExplodedNode *predNew = *I2; - ProgramStateRef stateNew = predNew->getState(); - // Check for 'void' return type if we have a bogus function prototype. - SVal Res = UnknownVal(); - QualType T = CE->getType(); - if (!T->isVoidType()) - Res = Eng.getSValBuilder().makeTruthVal(true, T); - B.generateNode(CE, predNew, stateNew->BindExpr(CE, LCtx, Res), - false, this); - } - } - - // Were they not equal? - if (ProgramStateRef stateNotEqual = stateLoad->assume(Cmp, false)) { - // Check for 'void' return type if we have a bogus function prototype. - SVal Res = UnknownVal(); - QualType T = CE->getType(); - if (!T->isVoidType()) - Res = Eng.getSValBuilder().makeTruthVal(false, CE->getType()); - StmtNodeBuilder B(N, Dst, Eng.getBuilderContext()); - B.generateNode(CE, N, stateNotEqual->BindExpr(CE, LCtx, Res), - false, this); - } - } - - return true; -} - -void ento::registerOSAtomicChecker(CheckerManager &mgr) { - mgr.registerChecker<OSAtomicChecker>(); -} diff --git a/lib/StaticAnalyzer/Checkers/ObjCAtSyncChecker.cpp b/lib/StaticAnalyzer/Checkers/ObjCAtSyncChecker.cpp index 4cc92ce..9d84f52 100644 --- a/lib/StaticAnalyzer/Checkers/ObjCAtSyncChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/ObjCAtSyncChecker.cpp @@ -18,7 +18,6 @@ #include "clang/StaticAnalyzer/Core/CheckerManager.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" -#include "clang/StaticAnalyzer/Checkers/DereferenceChecker.h" #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h" using namespace clang; @@ -50,8 +49,8 @@ void ObjCAtSyncChecker::checkPreStmt(const ObjCAtSynchronizedStmt *S, "for @synchronized")); BugReport *report = new BugReport(*BT_undef, BT_undef->getDescription(), N); - bugreporter::addTrackNullOrUndefValueVisitor(N, Ex, report); - C.EmitReport(report); + bugreporter::trackNullOrUndefValue(N, Ex, *report); + C.emitReport(report); } return; } @@ -73,9 +72,9 @@ void ObjCAtSyncChecker::checkPreStmt(const ObjCAtSynchronizedStmt *S, "(no synchronization will occur)")); BugReport *report = new BugReport(*BT_null, BT_null->getDescription(), N); - bugreporter::addTrackNullOrUndefValueVisitor(N, Ex, report); + bugreporter::trackNullOrUndefValue(N, Ex, *report); - C.EmitReport(report); + C.emitReport(report); return; } } diff --git a/lib/StaticAnalyzer/Checkers/ObjCContainersASTChecker.cpp b/lib/StaticAnalyzer/Checkers/ObjCContainersASTChecker.cpp index f2929c0..63a8480 100644 --- a/lib/StaticAnalyzer/Checkers/ObjCContainersASTChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/ObjCContainersASTChecker.cpp @@ -31,8 +31,6 @@ class WalkAST : public StmtVisitor<WalkAST> { ASTContext &ASTC; uint64_t PtrWidth; - static const unsigned InvalidArgIndex = UINT_MAX; - /// Check if the type has pointer size (very conservative). inline bool isPointerSize(const Type *T) { if (!T) @@ -102,16 +100,18 @@ void WalkAST::VisitCallExpr(CallExpr *CE) { return; const Expr *Arg = 0; - unsigned ArgNum = InvalidArgIndex; + unsigned ArgNum; if (Name.equals("CFArrayCreate") || Name.equals("CFSetCreate")) { + if (CE->getNumArgs() != 4) + return; ArgNum = 1; Arg = CE->getArg(ArgNum)->IgnoreParenCasts(); if (hasPointerToPointerSizedType(Arg)) return; - } - - if (Arg == 0 && Name.equals("CFDictionaryCreate")) { + } else if (Name.equals("CFDictionaryCreate")) { + if (CE->getNumArgs() != 6) + return; // Check first argument. ArgNum = 1; Arg = CE->getArg(ArgNum)->IgnoreParenCasts(); @@ -125,17 +125,18 @@ void WalkAST::VisitCallExpr(CallExpr *CE) { } } - if (ArgNum != InvalidArgIndex) { + if (Arg) { assert(ArgNum == 1 || ArgNum == 2); - SmallString<256> BufName; + SmallString<64> BufName; llvm::raw_svector_ostream OsName(BufName); - assert(ArgNum == 1 || ArgNum == 2); OsName << " Invalid use of '" << Name << "'" ; SmallString<256> Buf; llvm::raw_svector_ostream Os(Buf); - Os << " The "<< ((ArgNum == 1) ? "first" : "second") << " argument to '" + // Use "second" and "third" since users will expect 1-based indexing + // for parameter names when mentioned in prose. + Os << " The "<< ((ArgNum == 1) ? "second" : "third") << " argument to '" << Name << "' must be a C array of pointer-sized values, not '" << Arg->getType().getAsString() << "'"; diff --git a/lib/StaticAnalyzer/Checkers/ObjCContainersChecker.cpp b/lib/StaticAnalyzer/Checkers/ObjCContainersChecker.cpp index 2ab49ed..999c994 100644 --- a/lib/StaticAnalyzer/Checkers/ObjCContainersChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/ObjCContainersChecker.cpp @@ -55,16 +55,8 @@ public: }; } // end anonymous namespace -// ProgramState trait - a map from array symbol to it's state. -typedef llvm::ImmutableMap<SymbolRef, DefinedSVal> ArraySizeM; - -namespace { struct ArraySizeMap {}; } -namespace clang { namespace ento { -template<> struct ProgramStateTrait<ArraySizeMap> - : public ProgramStatePartialTrait<ArraySizeM > { - static void *GDMIndex() { return ObjCContainersChecker::getTag(); } -}; -}} +// ProgramState trait - a map from array symbol to its state. +REGISTER_MAP_WITH_PROGRAMSTATE(ArraySizeMap, SymbolRef, DefinedSVal) void ObjCContainersChecker::addSizeInfo(const Expr *Array, const Expr *Size, CheckerContext &C) const { @@ -146,7 +138,7 @@ void ObjCContainersChecker::checkPreStmt(const CallExpr *CE, initBugType(); BugReport *R = new BugReport(*BT, "Index is out of bounds", N); R->addRange(IdxExpr->getSourceRange()); - C.EmitReport(R); + C.emitReport(R); return; } } diff --git a/lib/StaticAnalyzer/Checkers/ObjCMissingSuperCallChecker.cpp b/lib/StaticAnalyzer/Checkers/ObjCMissingSuperCallChecker.cpp new file mode 100644 index 0000000..e906e8a --- /dev/null +++ b/lib/StaticAnalyzer/Checkers/ObjCMissingSuperCallChecker.cpp @@ -0,0 +1,203 @@ +//==- ObjCMissingSuperCallChecker.cpp - Check missing super-calls in ObjC --==// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +// +// This file defines a ObjCMissingSuperCallChecker, a checker that +// analyzes a UIViewController implementation to determine if it +// correctly calls super in the methods where this is mandatory. +// +//===----------------------------------------------------------------------===// + +#include "ClangSACheckers.h" +#include "clang/StaticAnalyzer/Core/Checker.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h" +#include "clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h" +#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h" +#include "clang/AST/ExprObjC.h" +#include "clang/AST/Expr.h" +#include "clang/AST/DeclObjC.h" +#include "clang/AST/RecursiveASTVisitor.h" +#include "llvm/ADT/SmallString.h" +#include "llvm/ADT/SmallSet.h" +#include "llvm/Support/raw_ostream.h" + +using namespace clang; +using namespace ento; + +static bool isUIViewControllerSubclass(ASTContext &Ctx, + const ObjCImplementationDecl *D) { + IdentifierInfo *ViewControllerII = &Ctx.Idents.get("UIViewController"); + const ObjCInterfaceDecl *ID = D->getClassInterface(); + + for ( ; ID; ID = ID->getSuperClass()) + if (ID->getIdentifier() == ViewControllerII) + return true; + return false; +} + +//===----------------------------------------------------------------------===// +// FindSuperCallVisitor - Identify specific calls to the superclass. +//===----------------------------------------------------------------------===// + +class FindSuperCallVisitor : public RecursiveASTVisitor<FindSuperCallVisitor> { +public: + explicit FindSuperCallVisitor(Selector S) : DoesCallSuper(false), Sel(S) {} + + bool VisitObjCMessageExpr(ObjCMessageExpr *E) { + if (E->getSelector() == Sel) + if (E->getReceiverKind() == ObjCMessageExpr::SuperInstance) + DoesCallSuper = true; + + // Recurse if we didn't find the super call yet. + return !DoesCallSuper; + } + + bool DoesCallSuper; + +private: + Selector Sel; +}; + +//===----------------------------------------------------------------------===// +// ObjCSuperCallChecker +//===----------------------------------------------------------------------===// + +namespace { +class ObjCSuperCallChecker : public Checker< + check::ASTDecl<ObjCImplementationDecl> > { +public: + void checkASTDecl(const ObjCImplementationDecl *D, AnalysisManager &Mgr, + BugReporter &BR) const; +}; +} + +void ObjCSuperCallChecker::checkASTDecl(const ObjCImplementationDecl *D, + AnalysisManager &Mgr, + BugReporter &BR) const { + ASTContext &Ctx = BR.getContext(); + + if (!isUIViewControllerSubclass(Ctx, D)) + return; + + const char *SelectorNames[] = + {"addChildViewController", "viewDidAppear", "viewDidDisappear", + "viewWillAppear", "viewWillDisappear", "removeFromParentViewController", + "didReceiveMemoryWarning", "viewDidUnload", "viewWillUnload", + "viewDidLoad"}; + const unsigned SelectorArgumentCounts[] = + {1, 1, 1, 1, 1, 0, 0, 0, 0, 0}; + const size_t SelectorCount = llvm::array_lengthof(SelectorNames); + assert(llvm::array_lengthof(SelectorArgumentCounts) == SelectorCount); + + // Fill the Selectors SmallSet with all selectors we want to check. + llvm::SmallSet<Selector, 16> Selectors; + for (size_t i = 0; i < SelectorCount; i++) { + unsigned ArgumentCount = SelectorArgumentCounts[i]; + const char *SelectorCString = SelectorNames[i]; + + // Get the selector. + IdentifierInfo *II = &Ctx.Idents.get(SelectorCString); + Selectors.insert(Ctx.Selectors.getSelector(ArgumentCount, &II)); + } + + // Iterate over all instance methods. + for (ObjCImplementationDecl::instmeth_iterator I = D->instmeth_begin(), + E = D->instmeth_end(); + I != E; ++I) { + Selector S = (*I)->getSelector(); + // Find out whether this is a selector that we want to check. + if (!Selectors.count(S)) + continue; + + ObjCMethodDecl *MD = *I; + + // Check if the method calls its superclass implementation. + if (MD->getBody()) + { + FindSuperCallVisitor Visitor(S); + Visitor.TraverseDecl(MD); + + // It doesn't call super, emit a diagnostic. + if (!Visitor.DoesCallSuper) { + PathDiagnosticLocation DLoc = + PathDiagnosticLocation::createEnd(MD->getBody(), + BR.getSourceManager(), + Mgr.getAnalysisDeclContext(D)); + + const char *Name = "Missing call to superclass"; + SmallString<256> Buf; + llvm::raw_svector_ostream os(Buf); + + os << "The '" << S.getAsString() + << "' instance method in UIViewController subclass '" << *D + << "' is missing a [super " << S.getAsString() << "] call"; + + BR.EmitBasicReport(MD, Name, categories::CoreFoundationObjectiveC, + os.str(), DLoc); + } + } + } +} + + +//===----------------------------------------------------------------------===// +// Check registration. +//===----------------------------------------------------------------------===// + +void ento::registerObjCSuperCallChecker(CheckerManager &Mgr) { + Mgr.registerChecker<ObjCSuperCallChecker>(); +} + + +/* + ToDo list for expanding this check in the future, the list is not exhaustive. + There are also cases where calling super is suggested but not "mandatory". + In addition to be able to check the classes and methods below, architectural + improvements like being able to allow for the super-call to be done in a called + method would be good too. + +*** trivial cases: +UIResponder subclasses +- resignFirstResponder + +NSResponder subclasses +- cursorUpdate + +*** more difficult cases: + +UIDocument subclasses +- finishedHandlingError:recovered: (is multi-arg) +- finishedHandlingError:recovered: (is multi-arg) + +UIViewController subclasses +- loadView (should *never* call super) +- transitionFromViewController:toViewController: + duration:options:animations:completion: (is multi-arg) + +UICollectionViewController subclasses +- loadView (take care because UIViewController subclasses should NOT call super + in loadView, but UICollectionViewController subclasses should) + +NSObject subclasses +- doesNotRecognizeSelector (it only has to call super if it doesn't throw) + +UIPopoverBackgroundView subclasses (some of those are class methods) +- arrowDirection (should *never* call super) +- arrowOffset (should *never* call super) +- arrowBase (should *never* call super) +- arrowHeight (should *never* call super) +- contentViewInsets (should *never* call super) + +UITextSelectionRect subclasses (some of those are properties) +- rect (should *never* call super) +- range (should *never* call super) +- writingDirection (should *never* call super) +- isVertical (should *never* call super) +- containsStart (should *never* call super) +- containsEnd (should *never* call super) +*/ diff --git a/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp b/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp index be45da1..98d2a85a 100644 --- a/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp @@ -72,6 +72,8 @@ public: void checkPreCall(const CallEvent &CE, CheckerContext &C) const; void checkPostCall(const CallEvent &CE, CheckerContext &C) const; + void printState(raw_ostream &Out, ProgramStateRef State, + const char *NL, const char *Sep) const; }; } // end anonymous namespace @@ -97,31 +99,14 @@ enum SelfFlagEnum { }; } -typedef llvm::ImmutableMap<SymbolRef, unsigned> SelfFlag; -namespace { struct CalledInit {}; } -namespace { struct PreCallSelfFlags {}; } - -namespace clang { -namespace ento { - template<> - struct ProgramStateTrait<SelfFlag> : public ProgramStatePartialTrait<SelfFlag> { - static void *GDMIndex() { static int index = 0; return &index; } - }; - template <> - struct ProgramStateTrait<CalledInit> : public ProgramStatePartialTrait<bool> { - static void *GDMIndex() { static int index = 0; return &index; } - }; - - /// \brief A call receiving a reference to 'self' invalidates the object that - /// 'self' contains. This keeps the "self flags" assigned to the 'self' - /// object before the call so we can assign them to the new object that 'self' - /// points to after the call. - template <> - struct ProgramStateTrait<PreCallSelfFlags> : public ProgramStatePartialTrait<unsigned> { - static void *GDMIndex() { static int index = 0; return &index; } - }; -} -} +REGISTER_MAP_WITH_PROGRAMSTATE(SelfFlag, SymbolRef, unsigned) +REGISTER_TRAIT_WITH_PROGRAMSTATE(CalledInit, bool) + +/// \brief A call receiving a reference to 'self' invalidates the object that +/// 'self' contains. This keeps the "self flags" assigned to the 'self' +/// object before the call so we can assign them to the new object that 'self' +/// points to after the call. +REGISTER_TRAIT_WITH_PROGRAMSTATE(PreCallSelfFlags, unsigned) static SelfFlagEnum getSelfFlags(SVal val, ProgramStateRef state) { if (SymbolRef sym = val.getAsSymbol()) @@ -138,7 +123,8 @@ static void addSelfFlag(ProgramStateRef state, SVal val, SelfFlagEnum flag, CheckerContext &C) { // We tag the symbol that the SVal wraps. if (SymbolRef sym = val.getAsSymbol()) - C.addTransition(state->set<SelfFlag>(sym, getSelfFlags(val, C) | flag)); + state = state->set<SelfFlag>(sym, getSelfFlags(val, state) | flag); + C.addTransition(state); } static bool hasSelfFlag(SVal val, SelfFlagEnum flag, CheckerContext &C) { @@ -176,7 +162,7 @@ static void checkForInvalidSelf(const Expr *E, CheckerContext &C, BugReport *report = new BugReport(*new InitSelfBug(), errorStr, N); - C.EmitReport(report); + C.emitReport(report); } void ObjCSelfInitChecker::checkPostObjCMessage(const ObjCMethodCall &Msg, @@ -305,13 +291,12 @@ void ObjCSelfInitChecker::checkPostCall(const CallEvent &CE, // returns 'self'. So assign the flags, which were set on 'self' to the // return value. // EX: self = performMoreInitialization(self) - const Expr *CallExpr = CE.getOriginExpr(); - if (CallExpr) - addSelfFlag(state, state->getSVal(CallExpr, C.getLocationContext()), - prevFlags, C); + addSelfFlag(state, CE.getReturnValue(), prevFlags, C); return; } } + + C.addTransition(state); } void ObjCSelfInitChecker::checkLocation(SVal location, bool isLoad, @@ -346,6 +331,53 @@ void ObjCSelfInitChecker::checkBind(SVal loc, SVal val, const Stmt *S, } } +void ObjCSelfInitChecker::printState(raw_ostream &Out, ProgramStateRef State, + const char *NL, const char *Sep) const { + SelfFlagTy FlagMap = State->get<SelfFlag>(); + bool DidCallInit = State->get<CalledInit>(); + SelfFlagEnum PreCallFlags = (SelfFlagEnum)State->get<PreCallSelfFlags>(); + + if (FlagMap.isEmpty() && !DidCallInit && !PreCallFlags) + return; + + Out << Sep << NL << "ObjCSelfInitChecker:" << NL; + + if (DidCallInit) + Out << " An init method has been called." << NL; + + if (PreCallFlags != SelfFlag_None) { + if (PreCallFlags & SelfFlag_Self) { + Out << " An argument of the current call came from the 'self' variable." + << NL; + } + if (PreCallFlags & SelfFlag_InitRes) { + Out << " An argument of the current call came from an init method." + << NL; + } + } + + Out << NL; + for (SelfFlagTy::iterator I = FlagMap.begin(), E = FlagMap.end(); + I != E; ++I) { + Out << I->first << " : "; + + if (I->second == SelfFlag_None) + Out << "none"; + + if (I->second & SelfFlag_Self) + Out << "self variable"; + + if (I->second & SelfFlag_InitRes) { + if (I->second != SelfFlag_InitRes) + Out << " | "; + Out << "result of init method"; + } + + Out << NL; + } +} + + // FIXME: A callback should disable checkers at the start of functions. static bool shouldRunOnFunctionOrMethod(const NamedDecl *ND) { if (!ND) diff --git a/lib/StaticAnalyzer/Checkers/PointerArithChecker.cpp b/lib/StaticAnalyzer/Checkers/PointerArithChecker.cpp index fe4845b..b5d9959 100644 --- a/lib/StaticAnalyzer/Checkers/PointerArithChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/PointerArithChecker.cpp @@ -59,7 +59,7 @@ void PointerArithChecker::checkPreStmt(const BinaryOperator *B, "dangerous.")); BugReport *R = new BugReport(*BT, BT->getDescription(), N); R->addRange(B->getSourceRange()); - C.EmitReport(R); + C.emitReport(R); } } } diff --git a/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp b/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp index fa5c6a3..47da87f 100644 --- a/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp @@ -67,7 +67,7 @@ void PointerSubChecker::checkPreStmt(const BinaryOperator *B, "the same memory chunk may cause incorrect result.")); BugReport *R = new BugReport(*BT, BT->getDescription(), N); R->addRange(B->getSourceRange()); - C.EmitReport(R); + C.emitReport(R); } } diff --git a/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp b/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp index 2d018ef..d9b6384 100644 --- a/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp @@ -43,15 +43,7 @@ public: } // end anonymous namespace // GDM Entry for tracking lock state. -namespace { class LockSet {}; } -namespace clang { -namespace ento { -template <> struct ProgramStateTrait<LockSet> : - public ProgramStatePartialTrait<llvm::ImmutableList<const MemRegion*> > { - static void *GDMIndex() { static int x = 0; return &x; } -}; -} // end of ento (ProgramState) namespace -} // end clang namespace +REGISTER_LIST_WITH_PROGRAMSTATE(LockSet, const MemRegion *) void PthreadLockChecker::checkPostStmt(const CallExpr *CE, @@ -118,7 +110,7 @@ void PthreadLockChecker::AcquireLock(CheckerContext &C, const CallExpr *CE, "This lock has already " "been acquired", N); report->addRange(CE->getArg(0)->getSourceRange()); - C.EmitReport(report); + C.emitReport(report); return; } @@ -163,7 +155,7 @@ void PthreadLockChecker::ReleaseLock(CheckerContext &C, const CallExpr *CE, return; ProgramStateRef state = C.getState(); - llvm::ImmutableList<const MemRegion*> LS = state->get<LockSet>(); + LockSetTy LS = state->get<LockSet>(); // FIXME: Better analysis requires IPA for wrappers. // FIXME: check for double unlocks @@ -183,7 +175,7 @@ void PthreadLockChecker::ReleaseLock(CheckerContext &C, const CallExpr *CE, "Possible lock order " "reversal", N); report->addRange(CE->getArg(0)->getSourceRange()); - C.EmitReport(report); + C.emitReport(report); return; } diff --git a/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp b/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp index 3c00d99..304051c 100644 --- a/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp @@ -40,24 +40,6 @@ using namespace clang; using namespace ento; using llvm::StrInStrNoCase; -namespace { -/// Wrapper around different kinds of node builder, so that helper functions -/// can have a common interface. -class GenericNodeBuilderRefCount { - CheckerContext *C; - const ProgramPointTag *tag; -public: - GenericNodeBuilderRefCount(CheckerContext &c, - const ProgramPointTag *t = 0) - : C(&c), tag(t){} - - ExplodedNode *MakeNode(ProgramStateRef state, ExplodedNode *Pred, - bool MarkAsSink = false) { - return C->addTransition(state, Pred, tag, MarkAsSink); - } -}; -} // end anonymous namespace - //===----------------------------------------------------------------------===// // Primitives used for constructing summaries for function/method calls. //===----------------------------------------------------------------------===// @@ -66,9 +48,23 @@ public: /// particular argument. enum ArgEffect { DoNothing, Autorelease, Dealloc, DecRef, DecRefMsg, DecRefBridgedTransfered, - DecRefAndStopTracking, DecRefMsgAndStopTracking, IncRefMsg, IncRef, MakeCollectable, MayEscape, - NewAutoreleasePool, StopTracking }; + NewAutoreleasePool, + + // Stop tracking the argument - the effect of the call is + // unknown. + StopTracking, + + // In some cases, we obtain a better summary for this checker + // by looking at the call site than by inlining the function. + // Signifies that we should stop tracking the symbol even if + // the function is inlined. + StopTrackingHard, + + // The function decrements the reference count and the checker + // should stop tracking the argument. + DecRefAndStopTrackingHard, DecRefMsgAndStopTrackingHard + }; namespace llvm { template <> struct FoldingSetTrait<ArgEffect> { @@ -90,7 +86,13 @@ class RetEffect { public: enum Kind { NoRet, OwnedSymbol, OwnedAllocatedSymbol, NotOwnedSymbol, GCNotOwnedSymbol, ARCNotOwnedSymbol, - OwnedWhenTrackedReceiver }; + OwnedWhenTrackedReceiver, + // Treat this function as returning a non-tracked symbol even if + // the function has been inlined. This is used where the call + // site summary is more presise than the summary indirectly produced + // by inlining the function + NoRetHard + }; enum ObjKind { CF, ObjC, AnyObj }; @@ -133,6 +135,9 @@ public: static RetEffect MakeNoRet() { return RetEffect(NoRet); } + static RetEffect MakeNoRetHard() { + return RetEffect(NoRetHard); + } void Profile(llvm::FoldingSetNodeID& ID) const { ID.AddInteger((unsigned) K); @@ -337,20 +342,7 @@ void RefVal::print(raw_ostream &Out) const { // RefBindings - State used to track object reference counts. //===----------------------------------------------------------------------===// -typedef llvm::ImmutableMap<SymbolRef, RefVal> RefBindings; - -namespace clang { -namespace ento { -template<> -struct ProgramStateTrait<RefBindings> - : public ProgramStatePartialTrait<RefBindings> { - static void *GDMIndex() { - static int RefBIndex = 0; - return &RefBIndex; - } -}; -} -} +REGISTER_MAP_WITH_PROGRAMSTATE(RefBindings, SymbolRef, RefVal) static inline const RefVal *getRefBinding(ProgramStateRef State, SymbolRef Sym) { @@ -893,7 +885,7 @@ static bool isMakeCollectable(const FunctionDecl *FD, StringRef FName) { return FName.find("MakeCollectable") != StringRef::npos; } -static ArgEffect getStopTrackingEquivalent(ArgEffect E) { +static ArgEffect getStopTrackingHardEquivalent(ArgEffect E) { switch (E) { case DoNothing: case Autorelease: @@ -904,13 +896,14 @@ static ArgEffect getStopTrackingEquivalent(ArgEffect E) { case MayEscape: case NewAutoreleasePool: case StopTracking: - return StopTracking; + case StopTrackingHard: + return StopTrackingHard; case DecRef: - case DecRefAndStopTracking: - return DecRefAndStopTracking; + case DecRefAndStopTrackingHard: + return DecRefAndStopTrackingHard; case DecRefMsg: - case DecRefMsgAndStopTracking: - return DecRefMsgAndStopTracking; + case DecRefMsgAndStopTrackingHard: + return DecRefMsgAndStopTrackingHard; case Dealloc: return Dealloc; } @@ -921,33 +914,65 @@ static ArgEffect getStopTrackingEquivalent(ArgEffect E) { void RetainSummaryManager::updateSummaryForCall(const RetainSummary *&S, const CallEvent &Call) { if (Call.hasNonZeroCallbackArg()) { - ArgEffect RecEffect = getStopTrackingEquivalent(S->getReceiverEffect()); - ArgEffect DefEffect = getStopTrackingEquivalent(S->getDefaultArgEffect()); + ArgEffect RecEffect = + getStopTrackingHardEquivalent(S->getReceiverEffect()); + ArgEffect DefEffect = + getStopTrackingHardEquivalent(S->getDefaultArgEffect()); ArgEffects CustomArgEffects = S->getArgEffects(); for (ArgEffects::iterator I = CustomArgEffects.begin(), E = CustomArgEffects.end(); I != E; ++I) { - ArgEffect Translated = getStopTrackingEquivalent(I->second); + ArgEffect Translated = getStopTrackingHardEquivalent(I->second); if (Translated != DefEffect) ScratchArgs = AF.add(ScratchArgs, I->first, Translated); } - RetEffect RE = RetEffect::MakeNoRet(); + RetEffect RE = RetEffect::MakeNoRetHard(); // Special cases where the callback argument CANNOT free the return value. // This can generally only happen if we know that the callback will only be // called when the return value is already being deallocated. if (const FunctionCall *FC = dyn_cast<FunctionCall>(&Call)) { - IdentifierInfo *Name = FC->getDecl()->getIdentifier(); - - // This callback frees the associated buffer. - if (Name->isStr("CGBitmapContextCreateWithData")) - RE = S->getRetEffect(); + if (IdentifierInfo *Name = FC->getDecl()->getIdentifier()) { + // When the CGBitmapContext is deallocated, the callback here will free + // the associated data buffer. + if (Name->isStr("CGBitmapContextCreateWithData")) + RE = S->getRetEffect(); + } } S = getPersistentSummary(RE, RecEffect, DefEffect); } + + // Special case '[super init];' and '[self init];' + // + // Even though calling '[super init]' without assigning the result to self + // and checking if the parent returns 'nil' is a bad pattern, it is common. + // Additionally, our Self Init checker already warns about it. To avoid + // overwhelming the user with messages from both checkers, we model the case + // of '[super init]' in cases when it is not consumed by another expression + // as if the call preserves the value of 'self'; essentially, assuming it can + // never fail and return 'nil'. + // Note, we don't want to just stop tracking the value since we want the + // RetainCount checker to report leaks and use-after-free if SelfInit checker + // is turned off. + if (const ObjCMethodCall *MC = dyn_cast<ObjCMethodCall>(&Call)) { + if (MC->getMethodFamily() == OMF_init && MC->isReceiverSelfOrSuper()) { + + // Check if the message is not consumed, we know it will not be used in + // an assignment, ex: "self = [super init]". + const Expr *ME = MC->getOriginExpr(); + const LocationContext *LCtx = MC->getLocationContext(); + ParentMap &PM = LCtx->getAnalysisDeclContext()->getParentMap(); + if (!PM.isConsumedExpr(ME)) { + RetainSummaryTemplate ModifiableSummaryTemplate(S, *this); + ModifiableSummaryTemplate->setReceiverEffect(DoNothing); + ModifiableSummaryTemplate->setRetEffect(RetEffect::MakeNoRet()); + } + } + + } } const RetainSummary * @@ -1036,6 +1061,8 @@ RetainSummaryManager::getFunctionSummary(const FunctionDecl *FD) { // The headers on OS X 10.8 use cf_consumed/ns_returns_retained, // but we can fully model NSMakeCollectable ourselves. AllowAnnotations = false; + } else if (FName == "CFPlugInInstanceCreate") { + S = getPersistentSummary(RetEffect::MakeNoRet()); } else if (FName == "IOBSDNameMatching" || FName == "IOServiceMatching" || FName == "IOServiceNameMatching" || @@ -1108,6 +1135,11 @@ RetainSummaryManager::getFunctionSummary(const FunctionDecl *FD) { break; if (RetTy->isPointerType()) { + if (FD->getAttr<CFAuditedTransferAttr>()) { + S = getCFCreateGetRuleSummary(FD); + break; + } + // For CoreFoundation ('CF') types. if (cocoa::isRefType(RetTy, "CF", FName)) { if (isRetain(FD, FName)) @@ -1347,22 +1379,6 @@ RetainSummaryManager::updateSummaryFromAnnotations(const RetainSummary *&Summ, const RetainSummary * RetainSummaryManager::getStandardMethodSummary(const ObjCMethodDecl *MD, Selector S, QualType RetTy) { - - if (MD) { - // Scan the method decl for 'void*' arguments. These should be treated - // as 'StopTracking' because they are often used with delegates. - // Delegates are a frequent form of false positives with the retain - // count checker. - unsigned i = 0; - for (ObjCMethodDecl::param_const_iterator I = MD->param_begin(), - E = MD->param_end(); I != E; ++I, ++i) - if (const ParmVarDecl *PD = *I) { - QualType Ty = Ctx.getCanonicalType(PD->getType()); - if (Ty.getLocalUnqualifiedType() == Ctx.VoidPtrTy) - ScratchArgs = AF.add(ScratchArgs, i, StopTracking); - } - } - // Any special effects? ArgEffect ReceiverEff = DoNothing; RetEffect ResultEff = RetEffect::MakeNoRet(); @@ -1441,9 +1457,9 @@ RetainSummaryManager::getStandardMethodSummary(const ObjCMethodDecl *MD, StringRef Slot = S.getNameForSlot(i); if (Slot.substr(Slot.size() - 8).equals_lower("delegate")) { if (ResultEff == ObjCInitRetE) - ResultEff = RetEffect::MakeNoRet(); + ResultEff = RetEffect::MakeNoRetHard(); else - ReceiverEff = StopTracking; + ReceiverEff = StopTrackingHard; } } } @@ -2174,6 +2190,7 @@ GetAllocationSite(ProgramStateManager& StateMgr, const ExplodedNode *N, // If allocation happened in a function different from the leak node context, // do not report the binding. + assert(N && "Could not find allocation node"); if (N->getLocationContext() != LeakContext) { FirstBinding = 0; } @@ -2229,27 +2246,36 @@ CFRefLeakReportVisitor::getEndPath(BugReporterContext &BRC, // Get the retain count. const RefVal* RV = getRefBinding(EndN->getState(), Sym); + assert(RV); if (RV->getKind() == RefVal::ErrorLeakReturned) { // FIXME: Per comments in rdar://6320065, "create" only applies to CF // objects. Only "copy", "alloc", "retain" and "new" transfer ownership // to the caller for NS objects. const Decl *D = &EndN->getCodeDecl(); - if (const ObjCMethodDecl *MD = dyn_cast<ObjCMethodDecl>(D)) { - os << " is returned from a method whose name ('" - << MD->getSelector().getAsString() - << "') does not start with 'copy', 'mutableCopy', 'alloc' or 'new'." - " This violates the naming convention rules" - " given in the Memory Management Guide for Cocoa"; - } + + os << (isa<ObjCMethodDecl>(D) ? " is returned from a method " + : " is returned from a function "); + + if (D->getAttr<CFReturnsNotRetainedAttr>()) + os << "that is annotated as CF_RETURNS_NOT_RETAINED"; + else if (D->getAttr<NSReturnsNotRetainedAttr>()) + os << "that is annotated as NS_RETURNS_NOT_RETAINED"; else { - const FunctionDecl *FD = cast<FunctionDecl>(D); - os << " is returned from a function whose name ('" - << *FD - << "') does not contain 'Copy' or 'Create'. This violates the naming" - " convention rules given in the Memory Management Guide for Core" - " Foundation"; - } + if (const ObjCMethodDecl *MD = dyn_cast<ObjCMethodDecl>(D)) { + os << "whose name ('" << MD->getSelector().getAsString() + << "') does not start with 'copy', 'mutableCopy', 'alloc' or 'new'." + " This violates the naming convention rules" + " given in the Memory Management Guide for Cocoa"; + } + else { + const FunctionDecl *FD = cast<FunctionDecl>(D); + os << "whose name ('" << *FD + << "') does not contain 'Copy' or 'Create'. This violates the naming" + " convention rules given in the Memory Management Guide for Core" + " Foundation"; + } + } } else if (RV->getKind() == RefVal::ErrorGCLeakReturned) { ObjCMethodDecl &MD = cast<ObjCMethodDecl>(EndN->getCodeDecl()); @@ -2474,6 +2500,10 @@ public: void checkSummary(const RetainSummary &Summ, const CallEvent &Call, CheckerContext &C) const; + void processSummaryOfInlined(const RetainSummary &Summ, + const CallEvent &Call, + CheckerContext &C) const; + bool evalCall(const CallExpr *CE, CheckerContext &C) const; ProgramStateRef evalAssume(ProgramStateRef state, SVal Cond, @@ -2499,8 +2529,8 @@ public: void checkEndPath(CheckerContext &C) const; ProgramStateRef updateSymbol(ProgramStateRef state, SymbolRef sym, - RefVal V, ArgEffect E, RefVal::Kind &hasErr, - CheckerContext &C) const; + RefVal V, ArgEffect E, RefVal::Kind &hasErr, + CheckerContext &C) const; void processNonLeakError(ProgramStateRef St, SourceRange ErrorRange, RefVal::Kind ErrorKind, SymbolRef Sym, @@ -2515,13 +2545,12 @@ public: SmallVectorImpl<SymbolRef> &Leaked) const; std::pair<ExplodedNode *, ProgramStateRef > - handleAutoreleaseCounts(ProgramStateRef state, - GenericNodeBuilderRefCount Bd, ExplodedNode *Pred, - CheckerContext &Ctx, SymbolRef Sym, RefVal V) const; + handleAutoreleaseCounts(ProgramStateRef state, ExplodedNode *Pred, + const ProgramPointTag *Tag, CheckerContext &Ctx, + SymbolRef Sym, RefVal V) const; ExplodedNode *processLeaks(ProgramStateRef state, SmallVectorImpl<SymbolRef> &Leaked, - GenericNodeBuilderRefCount &Builder, CheckerContext &Ctx, ExplodedNode *Pred = 0) const; }; @@ -2685,11 +2714,13 @@ void RetainCountChecker::checkPostStmt(const ObjCBoxedExpr *Ex, void RetainCountChecker::checkPostCall(const CallEvent &Call, CheckerContext &C) const { - if (C.wasInlined) - return; - RetainSummaryManager &Summaries = getSummaryManager(C); const RetainSummary *Summ = Summaries.getSummary(Call, C.getState()); + + if (C.wasInlined) { + processSummaryOfInlined(*Summ, Call, C); + return; + } checkSummary(*Summ, Call, C); } @@ -2721,6 +2752,45 @@ static QualType GetReturnType(const Expr *RetE, ASTContext &Ctx) { return RetTy; } +// We don't always get the exact modeling of the function with regards to the +// retain count checker even when the function is inlined. For example, we need +// to stop tracking the symbols which were marked with StopTrackingHard. +void RetainCountChecker::processSummaryOfInlined(const RetainSummary &Summ, + const CallEvent &CallOrMsg, + CheckerContext &C) const { + ProgramStateRef state = C.getState(); + + // Evaluate the effect of the arguments. + for (unsigned idx = 0, e = CallOrMsg.getNumArgs(); idx != e; ++idx) { + if (Summ.getArg(idx) == StopTrackingHard) { + SVal V = CallOrMsg.getArgSVal(idx); + if (SymbolRef Sym = V.getAsLocSymbol()) { + state = removeRefBinding(state, Sym); + } + } + } + + // Evaluate the effect on the message receiver. + const ObjCMethodCall *MsgInvocation = dyn_cast<ObjCMethodCall>(&CallOrMsg); + if (MsgInvocation) { + if (SymbolRef Sym = MsgInvocation->getReceiverSVal().getAsLocSymbol()) { + if (Summ.getReceiverEffect() == StopTrackingHard) { + state = removeRefBinding(state, Sym); + } + } + } + + // Consult the summary for the return value. + RetEffect RE = Summ.getRetEffect(); + if (RE.getKind() == RetEffect::NoRetHard) { + SymbolRef Sym = CallOrMsg.getReturnValue().getAsSymbol(); + if (Sym) + state = removeRefBinding(state, Sym); + } + + C.addTransition(state); +} + void RetainCountChecker::checkSummary(const RetainSummary &Summ, const CallEvent &CallOrMsg, CheckerContext &C) const { @@ -2755,7 +2825,7 @@ void RetainCountChecker::checkSummary(const RetainSummary &Summ, if (const RefVal *T = getRefBinding(state, Sym)) { ReceiverIsTracked = true; state = updateSymbol(state, Sym, *T, Summ.getReceiverEffect(), - hasErr, C); + hasErr, C); if (hasErr) { ErrorRange = MsgInvocation->getOriginExpr()->getReceiverRange(); ErrorSym = Sym; @@ -2786,13 +2856,13 @@ void RetainCountChecker::checkSummary(const RetainSummary &Summ, llvm_unreachable("Unhandled RetEffect."); case RetEffect::NoRet: + case RetEffect::NoRetHard: // No work necessary. break; case RetEffect::OwnedAllocatedSymbol: case RetEffect::OwnedSymbol: { - SymbolRef Sym = state->getSVal(CallOrMsg.getOriginExpr(), - C.getLocationContext()).getAsSymbol(); + SymbolRef Sym = CallOrMsg.getReturnValue().getAsSymbol(); if (!Sym) break; @@ -2811,10 +2881,10 @@ void RetainCountChecker::checkSummary(const RetainSummary &Summ, case RetEffect::ARCNotOwnedSymbol: case RetEffect::NotOwnedSymbol: { const Expr *Ex = CallOrMsg.getOriginExpr(); - SymbolRef Sym = state->getSVal(Ex, C.getLocationContext()).getAsSymbol(); + SymbolRef Sym = CallOrMsg.getReturnValue().getAsSymbol(); if (!Sym) break; - + assert(Ex); // Use GetReturnType in order to give [NSFoo alloc] the type NSFoo *. QualType ResultTy = GetReturnType(Ex, C.getASTContext()); state = setRefBinding(state, Sym, RefVal::makeNotOwned(RE.getObjKind(), @@ -2864,8 +2934,8 @@ RetainCountChecker::updateSymbol(ProgramStateRef state, SymbolRef sym, case DecRefMsg: E = IgnoreRetainMsg ? DoNothing : DecRef; break; - case DecRefMsgAndStopTracking: - E = IgnoreRetainMsg ? StopTracking : DecRefAndStopTracking; + case DecRefMsgAndStopTrackingHard: + E = IgnoreRetainMsg ? StopTracking : DecRefAndStopTrackingHard; break; case MakeCollectable: E = C.isObjCGCEnabled() ? DecRef : DoNothing; @@ -2886,7 +2956,7 @@ RetainCountChecker::updateSymbol(ProgramStateRef state, SymbolRef sym, case DecRefMsg: case IncRefMsg: case MakeCollectable: - case DecRefMsgAndStopTracking: + case DecRefMsgAndStopTrackingHard: llvm_unreachable("DecRefMsg/IncRefMsg/MakeCollectable already converted"); case Dealloc: @@ -2935,6 +3005,7 @@ RetainCountChecker::updateSymbol(ProgramStateRef state, SymbolRef sym, break; case StopTracking: + case StopTrackingHard: return removeRefBinding(state, sym); case IncRef: @@ -2955,7 +3026,7 @@ RetainCountChecker::updateSymbol(ProgramStateRef state, SymbolRef sym, case DecRef: case DecRefBridgedTransfered: - case DecRefAndStopTracking: + case DecRefAndStopTrackingHard: switch (V.getKind()) { default: // case 'RefVal::Released' handled above. @@ -2966,7 +3037,7 @@ RetainCountChecker::updateSymbol(ProgramStateRef state, SymbolRef sym, if (V.getCount() == 1) V = V ^ (E == DecRefBridgedTransfered ? RefVal::NotOwned : RefVal::Released); - else if (E == DecRefAndStopTracking) + else if (E == DecRefAndStopTrackingHard) return removeRefBinding(state, sym); V = V - 1; @@ -2974,7 +3045,7 @@ RetainCountChecker::updateSymbol(ProgramStateRef state, SymbolRef sym, case RefVal::NotOwned: if (V.getCount() > 0) { - if (E == DecRefAndStopTracking) + if (E == DecRefAndStopTrackingHard) return removeRefBinding(state, sym); V = V - 1; } else { @@ -3035,7 +3106,7 @@ void RetainCountChecker::processNonLeakError(ProgramStateRef St, C.isObjCGCEnabled(), SummaryLog, N, Sym); report->addRange(ErrorRange); - C.EmitReport(report); + C.emitReport(report); } //===----------------------------------------------------------------------===// @@ -3090,8 +3161,7 @@ bool RetainCountChecker::evalCall(const CallExpr *CE, CheckerContext &C) const { if (RetVal.isUnknown()) { // If the receiver is unknown, conjure a return value. SValBuilder &SVB = C.getSValBuilder(); - unsigned Count = C.getCurrentBlockCount(); - RetVal = SVB.getConjuredSymbolVal(0, CE, LCtx, ResultTy, Count); + RetVal = SVB.conjureSymbolVal(0, CE, LCtx, ResultTy, C.blockCount()); } state = state->BindExpr(CE, LCtx, RetVal, false); @@ -3105,8 +3175,7 @@ bool RetainCountChecker::evalCall(const CallExpr *CE, CheckerContext &C) const { Binding = getRefBinding(state, Sym); // Invalidate the argument region. - unsigned Count = C.getCurrentBlockCount(); - state = state->invalidateRegions(ArgRegion, CE, Count, LCtx); + state = state->invalidateRegions(ArgRegion, CE, C.blockCount(), LCtx); // Restore the refcount status of the argument. if (Binding) @@ -3121,12 +3190,6 @@ bool RetainCountChecker::evalCall(const CallExpr *CE, CheckerContext &C) const { // Handle return statements. //===----------------------------------------------------------------------===// -// Return true if the current LocationContext has no caller context. -static bool inTopFrame(CheckerContext &C) { - const LocationContext *LC = C.getLocationContext(); - return LC->getParent() == 0; -} - void RetainCountChecker::checkPreStmt(const ReturnStmt *S, CheckerContext &C) const { @@ -3135,7 +3198,7 @@ void RetainCountChecker::checkPreStmt(const ReturnStmt *S, // better checking even for inlined calls, and see if they match // with their expected semantics (e.g., the method should return a retained // object, etc.). - if (!inTopFrame(C)) + if (!C.inTopFrame()) return; const Expr *RetE = S->getRetValue(); @@ -3196,8 +3259,8 @@ void RetainCountChecker::checkPreStmt(const ReturnStmt *S, // Update the autorelease counts. static SimpleProgramPointTag AutoreleaseTag("RetainCountChecker : Autorelease"); - GenericNodeBuilderRefCount Bd(C, &AutoreleaseTag); - llvm::tie(Pred, state) = handleAutoreleaseCounts(state, Bd, Pred, C, Sym, X); + llvm::tie(Pred, state) = handleAutoreleaseCounts(state, Pred, &AutoreleaseTag, + C, Sym, X); // Did we cache out? if (!Pred) @@ -3267,7 +3330,7 @@ void RetainCountChecker::checkReturnWithRetEffect(const ReturnStmt *S, new CFRefLeakReport(*getLeakAtReturnBug(LOpts, GCEnabled), LOpts, GCEnabled, SummaryLog, N, Sym, C); - C.EmitReport(report); + C.emitReport(report); } } } @@ -3288,7 +3351,7 @@ void RetainCountChecker::checkReturnWithRetEffect(const ReturnStmt *S, new CFRefReport(*returnNotOwnedForOwned, C.getASTContext().getLangOpts(), C.isObjCGCEnabled(), SummaryLog, N, Sym); - C.EmitReport(report); + C.emitReport(report); } } } @@ -3354,18 +3417,19 @@ ProgramStateRef RetainCountChecker::evalAssume(ProgramStateRef state, // too bad since the number of symbols we will track in practice are // probably small and evalAssume is only called at branches and a few // other places. - RefBindings B = state->get<RefBindings>(); + RefBindingsTy B = state->get<RefBindings>(); if (B.isEmpty()) return state; bool changed = false; - RefBindings::Factory &RefBFactory = state->get_context<RefBindings>(); + RefBindingsTy::Factory &RefBFactory = state->get_context<RefBindings>(); - for (RefBindings::iterator I = B.begin(), E = B.end(); I != E; ++I) { - // Check if the symbol is null (or equal to any constant). - // If this is the case, stop tracking the symbol. - if (state->getSymVal(I.getKey())) { + for (RefBindingsTy::iterator I = B.begin(), E = B.end(); I != E; ++I) { + // Check if the symbol is null stop tracking the symbol. + ConstraintManager &CMgr = state->getConstraintManager(); + ConditionTruthVal AllocFailed = CMgr.isNull(state, I.getKey()); + if (AllocFailed.isConstrainedTrue()) { changed = true; B = RefBFactory.remove(B, I.getKey()); } @@ -3410,8 +3474,8 @@ RetainCountChecker::checkRegionChanges(ProgramStateRef state, std::pair<ExplodedNode *, ProgramStateRef > RetainCountChecker::handleAutoreleaseCounts(ProgramStateRef state, - GenericNodeBuilderRefCount Bd, ExplodedNode *Pred, + const ProgramPointTag *Tag, CheckerContext &Ctx, SymbolRef Sym, RefVal V) const { unsigned ACnt = V.getAutoreleaseCount(); @@ -3440,7 +3504,7 @@ RetainCountChecker::handleAutoreleaseCounts(ProgramStateRef state, V.setAutoreleaseCount(0); } state = setRefBinding(state, Sym, V); - ExplodedNode *N = Bd.MakeNode(state, Pred); + ExplodedNode *N = Ctx.addTransition(state, Pred, Tag); if (N == 0) state = 0; return std::make_pair(N, state); @@ -3451,7 +3515,8 @@ RetainCountChecker::handleAutoreleaseCounts(ProgramStateRef state, V = V ^ RefVal::ErrorOverAutorelease; state = setRefBinding(state, Sym, V); - if (ExplodedNode *N = Bd.MakeNode(state, Pred, true)) { + ExplodedNode *N = Ctx.generateSink(state, Pred, Tag); + if (N) { SmallString<128> sbuf; llvm::raw_svector_ostream os(sbuf); os << "Object over-autoreleased: object was sent -autorelease "; @@ -3466,7 +3531,7 @@ RetainCountChecker::handleAutoreleaseCounts(ProgramStateRef state, CFRefReport *report = new CFRefReport(*overAutorelease, LOpts, /* GCEnabled = */ false, SummaryLog, N, Sym, os.str()); - Ctx.EmitReport(report); + Ctx.emitReport(report); } return std::make_pair((ExplodedNode *)0, (ProgramStateRef )0); @@ -3492,14 +3557,13 @@ RetainCountChecker::handleSymbolDeath(ProgramStateRef state, ExplodedNode * RetainCountChecker::processLeaks(ProgramStateRef state, SmallVectorImpl<SymbolRef> &Leaked, - GenericNodeBuilderRefCount &Builder, CheckerContext &Ctx, ExplodedNode *Pred) const { if (Leaked.empty()) return Pred; // Generate an intermediate node representing the leak point. - ExplodedNode *N = Builder.MakeNode(state, Pred); + ExplodedNode *N = Ctx.addTransition(state, Pred); if (N) { for (SmallVectorImpl<SymbolRef>::iterator @@ -3513,7 +3577,7 @@ RetainCountChecker::processLeaks(ProgramStateRef state, CFRefLeakReport *report = new CFRefLeakReport(*BT, LOpts, GCEnabled, SummaryLog, N, *I, Ctx); - Ctx.EmitReport(report); + Ctx.emitReport(report); } } @@ -3522,13 +3586,12 @@ RetainCountChecker::processLeaks(ProgramStateRef state, void RetainCountChecker::checkEndPath(CheckerContext &Ctx) const { ProgramStateRef state = Ctx.getState(); - GenericNodeBuilderRefCount Bd(Ctx); - RefBindings B = state->get<RefBindings>(); + RefBindingsTy B = state->get<RefBindings>(); ExplodedNode *Pred = Ctx.getPredecessor(); - for (RefBindings::iterator I = B.begin(), E = B.end(); I != E; ++I) { - llvm::tie(Pred, state) = handleAutoreleaseCounts(state, Bd, Pred, Ctx, - I->first, I->second); + for (RefBindingsTy::iterator I = B.begin(), E = B.end(); I != E; ++I) { + llvm::tie(Pred, state) = handleAutoreleaseCounts(state, Pred, /*Tag=*/0, + Ctx, I->first, I->second); if (!state) return; } @@ -3543,10 +3606,10 @@ void RetainCountChecker::checkEndPath(CheckerContext &Ctx) const { B = state->get<RefBindings>(); SmallVector<SymbolRef, 10> Leaked; - for (RefBindings::iterator I = B.begin(), E = B.end(); I != E; ++I) + for (RefBindingsTy::iterator I = B.begin(), E = B.end(); I != E; ++I) state = handleSymbolDeath(state, I->first, I->second, Leaked); - processLeaks(state, Leaked, Bd, Ctx, Pred); + processLeaks(state, Leaked, Ctx, Pred); } const ProgramPointTag * @@ -3567,7 +3630,7 @@ void RetainCountChecker::checkDeadSymbols(SymbolReaper &SymReaper, ExplodedNode *Pred = C.getPredecessor(); ProgramStateRef state = C.getState(); - RefBindings B = state->get<RefBindings>(); + RefBindingsTy B = state->get<RefBindings>(); // Update counts from autorelease pools for (SymbolReaper::dead_iterator I = SymReaper.dead_begin(), @@ -3576,8 +3639,8 @@ void RetainCountChecker::checkDeadSymbols(SymbolReaper &SymReaper, if (const RefVal *T = B.lookup(Sym)){ // Use the symbol as the tag. // FIXME: This might not be as unique as we would like. - GenericNodeBuilderRefCount Bd(C, getDeadSymbolTag(Sym)); - llvm::tie(Pred, state) = handleAutoreleaseCounts(state, Bd, Pred, C, + const ProgramPointTag *Tag = getDeadSymbolTag(Sym); + llvm::tie(Pred, state) = handleAutoreleaseCounts(state, Pred, Tag, C, Sym, *T); if (!state) return; @@ -3593,17 +3656,14 @@ void RetainCountChecker::checkDeadSymbols(SymbolReaper &SymReaper, state = handleSymbolDeath(state, *I, *T, Leaked); } - { - GenericNodeBuilderRefCount Bd(C, this); - Pred = processLeaks(state, Leaked, Bd, C, Pred); - } + Pred = processLeaks(state, Leaked, C, Pred); // Did we cache out? if (!Pred) return; // Now generate a new node that nukes the old bindings. - RefBindings::Factory &F = state->get_context<RefBindings>(); + RefBindingsTy::Factory &F = state->get_context<RefBindings>(); for (SymbolReaper::dead_iterator I = SymReaper.dead_begin(), E = SymReaper.dead_end(); I != E; ++I) @@ -3616,12 +3676,12 @@ void RetainCountChecker::checkDeadSymbols(SymbolReaper &SymReaper, void RetainCountChecker::printState(raw_ostream &Out, ProgramStateRef State, const char *NL, const char *Sep) const { - RefBindings B = State->get<RefBindings>(); + RefBindingsTy B = State->get<RefBindings>(); if (!B.isEmpty()) Out << Sep << NL; - for (RefBindings::iterator I = B.begin(), E = B.end(); I != E; ++I) { + for (RefBindingsTy::iterator I = B.begin(), E = B.end(); I != E; ++I) { Out << I->first << " : "; I->second.print(Out); Out << NL; diff --git a/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp b/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp index 6e56593..f3560aa 100644 --- a/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp @@ -82,7 +82,7 @@ void ReturnPointerRangeChecker::checkPreStmt(const ReturnStmt *RS, new BugReport(*BT, BT->getDescription(), N); report->addRange(RetE->getSourceRange()); - C.EmitReport(report); + C.emitReport(report); } } diff --git a/lib/StaticAnalyzer/Checkers/ReturnUndefChecker.cpp b/lib/StaticAnalyzer/Checkers/ReturnUndefChecker.cpp index ca2a55d..37ec1aa 100644 --- a/lib/StaticAnalyzer/Checkers/ReturnUndefChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/ReturnUndefChecker.cpp @@ -16,6 +16,7 @@ #include "ClangSACheckers.h" #include "clang/StaticAnalyzer/Core/Checker.h" #include "clang/StaticAnalyzer/Core/CheckerManager.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" @@ -41,6 +42,19 @@ void ReturnUndefChecker::checkPreStmt(const ReturnStmt *RS, if (!C.getState()->getSVal(RetE, C.getLocationContext()).isUndef()) return; + // "return;" is modeled to evaluate to an UndefinedValue. Allow UndefinedValue + // to be returned in functions returning void to support the following pattern: + // void foo() { + // return; + // } + // void test() { + // return foo(); + // } + const StackFrameContext *SFC = C.getStackFrame(); + QualType RT = CallEvent::getDeclaredResultType(SFC->getDecl()); + if (!RT.isNull() && RT->isSpecificBuiltinType(BuiltinType::Void)) + return; + ExplodedNode *N = C.generateSink(); if (!N) @@ -53,11 +67,10 @@ void ReturnUndefChecker::checkPreStmt(const ReturnStmt *RS, BugReport *report = new BugReport(*BT, BT->getDescription(), N); - report->disablePathPruning(); report->addRange(RetE->getSourceRange()); - bugreporter::addTrackNullOrUndefValueVisitor(N, RetE, report); + bugreporter::trackNullOrUndefValue(N, RetE, *report); - C.EmitReport(report); + C.emitReport(report); } void ento::registerReturnUndefChecker(CheckerManager &mgr) { diff --git a/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp b/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp new file mode 100644 index 0000000..ee055ad --- /dev/null +++ b/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp @@ -0,0 +1,348 @@ +//===-- SimpleStreamChecker.cpp -----------------------------------------*- C++ -*--// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +// +// Defines a checker for proper use of fopen/fclose APIs. +// - If a file has been closed with fclose, it should not be accessed again. +// Accessing a closed file results in undefined behavior. +// - If a file was opened with fopen, it must be closed with fclose before +// the execution ends. Failing to do so results in a resource leak. +// +//===----------------------------------------------------------------------===// + +#include "ClangSACheckers.h" +#include "clang/StaticAnalyzer/Core/Checker.h" +#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" + +using namespace clang; +using namespace ento; + +namespace { +typedef llvm::SmallVector<SymbolRef, 2> SymbolVector; + +struct StreamState { +private: + enum Kind { Opened, Closed } K; + StreamState(Kind InK) : K(InK) { } + +public: + bool isOpened() const { return K == Opened; } + bool isClosed() const { return K == Closed; } + + static StreamState getOpened() { return StreamState(Opened); } + static StreamState getClosed() { return StreamState(Closed); } + + bool operator==(const StreamState &X) const { + return K == X.K; + } + void Profile(llvm::FoldingSetNodeID &ID) const { + ID.AddInteger(K); + } +}; + +class SimpleStreamChecker : public Checker<check::PostCall, + check::PreCall, + check::DeadSymbols, + check::Bind, + check::RegionChanges> { + + mutable IdentifierInfo *IIfopen, *IIfclose; + + OwningPtr<BugType> DoubleCloseBugType; + OwningPtr<BugType> LeakBugType; + + void initIdentifierInfo(ASTContext &Ctx) const; + + void reportDoubleClose(SymbolRef FileDescSym, + const CallEvent &Call, + CheckerContext &C) const; + + void reportLeaks(SymbolVector LeakedStreams, + CheckerContext &C, + ExplodedNode *ErrNode) const; + + bool guaranteedNotToCloseFile(const CallEvent &Call) const; + +public: + SimpleStreamChecker(); + + /// Process fopen. + void checkPostCall(const CallEvent &Call, CheckerContext &C) const; + /// Process fclose. + void checkPreCall(const CallEvent &Call, CheckerContext &C) const; + + void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const; + + /// Deal with symbol escape as a byproduct of a bind. + void checkBind(SVal location, SVal val, const Stmt*S, + CheckerContext &C) const; + + /// Deal with symbol escape as a byproduct of a region change. + ProgramStateRef + checkRegionChanges(ProgramStateRef state, + const StoreManager::InvalidatedSymbols *invalidated, + ArrayRef<const MemRegion *> ExplicitRegions, + ArrayRef<const MemRegion *> Regions, + const CallEvent *Call) const; + bool wantsRegionChangeUpdate(ProgramStateRef state) const { + return true; + } +}; + +} // end anonymous namespace + +/// The state of the checker is a map from tracked stream symbols to their +/// state. Let's store it in the ProgramState. +REGISTER_MAP_WITH_PROGRAMSTATE(StreamMap, SymbolRef, StreamState) + +namespace { +class StopTrackingCallback : public SymbolVisitor { + ProgramStateRef state; +public: + StopTrackingCallback(ProgramStateRef st) : state(st) {} + ProgramStateRef getState() const { return state; } + + bool VisitSymbol(SymbolRef sym) { + state = state->remove<StreamMap>(sym); + return true; + } +}; +} // end anonymous namespace + + +SimpleStreamChecker::SimpleStreamChecker() : IIfopen(0), IIfclose(0) { + // Initialize the bug types. + DoubleCloseBugType.reset(new BugType("Double fclose", + "Unix Stream API Error")); + + LeakBugType.reset(new BugType("Resource Leak", + "Unix Stream API Error")); + // Sinks are higher importance bugs as well as calls to assert() or exit(0). + LeakBugType->setSuppressOnSink(true); +} + +void SimpleStreamChecker::checkPostCall(const CallEvent &Call, + CheckerContext &C) const { + initIdentifierInfo(C.getASTContext()); + + if (!Call.isGlobalCFunction()) + return; + + if (Call.getCalleeIdentifier() != IIfopen) + return; + + // Get the symbolic value corresponding to the file handle. + SymbolRef FileDesc = Call.getReturnValue().getAsSymbol(); + if (!FileDesc) + return; + + // Generate the next transition (an edge in the exploded graph). + ProgramStateRef State = C.getState(); + State = State->set<StreamMap>(FileDesc, StreamState::getOpened()); + C.addTransition(State); +} + +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) + return; + + // Get the symbolic value corresponding to the file handle. + SymbolRef FileDesc = Call.getArgSVal(0).getAsSymbol(); + if (!FileDesc) + return; + + // Check if the stream has already been closed. + ProgramStateRef State = C.getState(); + const StreamState *SS = State->get<StreamMap>(FileDesc); + if (SS && SS->isClosed()) { + reportDoubleClose(FileDesc, Call, C); + return; + } + + // Generate the next transition, in which the stream is closed. + State = State->set<StreamMap>(FileDesc, StreamState::getClosed()); + C.addTransition(State); +} + +static bool isLeaked(SymbolRef Sym, const StreamState &SS, + bool IsSymDead, ProgramStateRef State) { + if (IsSymDead && SS.isOpened()) { + // If a symbol is NULL, assume that fopen failed on this path. + // A symbol should only be considered leaked if it is non-null. + ConstraintManager &CMgr = State->getConstraintManager(); + ConditionTruthVal OpenFailed = CMgr.isNull(State, Sym); + return !OpenFailed.isConstrainedTrue(); + } + return false; +} + +void SimpleStreamChecker::checkDeadSymbols(SymbolReaper &SymReaper, + CheckerContext &C) const { + ProgramStateRef State = C.getState(); + SymbolVector LeakedStreams; + StreamMapTy TrackedStreams = State->get<StreamMap>(); + for (StreamMapTy::iterator I = TrackedStreams.begin(), + E = TrackedStreams.end(); I != E; ++I) { + SymbolRef Sym = I->first; + bool IsSymDead = SymReaper.isDead(Sym); + + // Collect leaked symbols. + if (isLeaked(Sym, I->second, IsSymDead, State)) + LeakedStreams.push_back(Sym); + + // Remove the dead symbol from the streams map. + if (IsSymDead) + State = State->remove<StreamMap>(Sym); + } + + ExplodedNode *N = C.addTransition(State); + reportLeaks(LeakedStreams, C, N); +} + +void SimpleStreamChecker::reportDoubleClose(SymbolRef FileDescSym, + const CallEvent &Call, + CheckerContext &C) const { + // We reached a bug, stop exploring the path here by generating a sink. + ExplodedNode *ErrNode = C.generateSink(); + // If we've already reached this node on another path, return. + if (!ErrNode) + return; + + // Generate the report. + BugReport *R = new BugReport(*DoubleCloseBugType, + "Closing a previously closed file stream", ErrNode); + R->addRange(Call.getSourceRange()); + R->markInteresting(FileDescSym); + C.emitReport(R); +} + +void SimpleStreamChecker::reportLeaks(SymbolVector LeakedStreams, + CheckerContext &C, + ExplodedNode *ErrNode) const { + // Attach bug reports to the leak node. + // TODO: Identify the leaked file descriptor. + for (llvm::SmallVector<SymbolRef, 2>::iterator + I = LeakedStreams.begin(), E = LeakedStreams.end(); I != E; ++I) { + BugReport *R = new BugReport(*LeakBugType, + "Opened file is never closed; potential resource leak", ErrNode); + R->markInteresting(*I); + C.emitReport(R); + } +} + +// Check various ways a symbol can be invalidated. +// Stop tracking symbols when a value escapes as a result of checkBind. +// A value escapes in three possible cases: +// (1) We are binding to something that is not a memory region. +// (2) We are binding to a MemRegion that does not have stack storage +// (3) We are binding to a MemRegion with stack storage that the store +// does not understand. +void SimpleStreamChecker::checkBind(SVal loc, SVal val, const Stmt *S, + CheckerContext &C) const { + // Are we storing to something that causes the value to "escape"? + bool escapes = true; + ProgramStateRef state = C.getState(); + + if (loc::MemRegionVal *regionLoc = dyn_cast<loc::MemRegionVal>(&loc)) { + escapes = !regionLoc->getRegion()->hasStackStorage(); + + if (!escapes) { + // To test (3), generate a new state with the binding added. If it is + // the same state, then it escapes (since the store cannot represent + // the binding). Do this only if we know that the store is not supposed + // to generate the same state. + SVal StoredVal = state->getSVal(regionLoc->getRegion()); + if (StoredVal != val) + escapes = (state == (state->bindLoc(*regionLoc, val))); + } + } + + // If our store can represent the binding and we aren't storing to something + // that doesn't have local storage then just return the state and + // continue as is. + if (!escapes) + return; + + // Otherwise, find all symbols referenced by 'val' that we are tracking + // and stop tracking them. + state = state->scanReachableSymbols<StopTrackingCallback>(val).getState(); + C.addTransition(state); +} + +bool SimpleStreamChecker::guaranteedNotToCloseFile(const CallEvent &Call) const{ + // If it's not in a system header, assume it might close a file. + if (!Call.isInSystemHeader()) + return false; + + // Handle cases where we know a buffer's /address/ can escape. + if (Call.argumentsMayEscape()) + return false; + + // Note, even though fclose closes the file, we do not list it here + // since the checker is modeling the call. + + return true; +} + +// If the symbol we are tracking is invalidated, do not track the symbol as +// we cannot reason about it anymore. +ProgramStateRef +SimpleStreamChecker::checkRegionChanges(ProgramStateRef State, + const StoreManager::InvalidatedSymbols *invalidated, + ArrayRef<const MemRegion *> ExplicitRegions, + ArrayRef<const MemRegion *> Regions, + const CallEvent *Call) const { + + if (!invalidated || invalidated->empty()) + return State; + + // If it's a call which might close the file, we assume that all regions + // (explicit and implicit) escaped. Otherwise, whitelist explicit pointers + // (the parameters to the call); we still can track them. + llvm::SmallPtrSet<SymbolRef, 8> WhitelistedSymbols; + if (!Call || guaranteedNotToCloseFile(*Call)) { + for (ArrayRef<const MemRegion *>::iterator I = ExplicitRegions.begin(), + E = ExplicitRegions.end(); I != E; ++I) { + if (const SymbolicRegion *R = (*I)->StripCasts()->getAs<SymbolicRegion>()) + WhitelistedSymbols.insert(R->getSymbol()); + } + } + + for (StoreManager::InvalidatedSymbols::const_iterator I=invalidated->begin(), + E = invalidated->end(); I!=E; ++I) { + SymbolRef sym = *I; + if (WhitelistedSymbols.count(sym)) + continue; + // The symbol escaped. Optimistically, assume that the corresponding file + // handle will be closed somewhere else. + State = State->remove<StreamMap>(sym); + } + 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 54cf569..0c2f266 100644 --- a/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp @@ -109,7 +109,7 @@ void StackAddrEscapeChecker::EmitStackError(CheckerContext &C, const MemRegion * if (range.isValid()) report->addRange(range); - C.EmitReport(report); + C.emitReport(report); } void StackAddrEscapeChecker::checkPreStmt(const ReturnStmt *RS, @@ -118,8 +118,10 @@ void StackAddrEscapeChecker::checkPreStmt(const ReturnStmt *RS, const Expr *RetE = RS->getRetValue(); if (!RetE) return; - - SVal V = C.getState()->getSVal(RetE, C.getLocationContext()); + RetE = RetE->IgnoreParens(); + + const LocationContext *LCtx = C.getLocationContext(); + SVal V = C.getState()->getSVal(RetE, LCtx); const MemRegion *R = V.getAsRegion(); if (!R) @@ -132,8 +134,9 @@ void StackAddrEscapeChecker::checkPreStmt(const ReturnStmt *RS, return; // Return stack memory in an ancestor stack frame is fine. - const StackFrameContext *SFC = SS->getStackFrame(); - if (SFC != C.getLocationContext()->getCurrentStackFrame()) + const StackFrameContext *CurFrame = LCtx->getCurrentStackFrame(); + const StackFrameContext *MemFrame = SS->getStackFrame(); + if (MemFrame != CurFrame) return; // Automatic reference counting automatically copies blocks. @@ -141,6 +144,14 @@ void StackAddrEscapeChecker::checkPreStmt(const ReturnStmt *RS, isa<BlockDataRegion>(R)) return; + // Returning a record by value is fine. (In this case, the returned + // expression will be a copy-constructor, possibly wrapped in an + // ExprWithCleanups node.) + if (const ExprWithCleanups *Cleanup = dyn_cast<ExprWithCleanups>(RetE)) + RetE = Cleanup->getSubExpr(); + if (isa<CXXConstructExpr>(RetE) && RetE->getType()->isRecordType()) + return; + EmitStackError(C, R, RetE); } @@ -221,7 +232,7 @@ void StackAddrEscapeChecker::checkEndPath(CheckerContext &Ctx) const { if (range.isValid()) report->addRange(range); - Ctx.EmitReport(report); + Ctx.emitReport(report); } } diff --git a/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/lib/StaticAnalyzer/Checkers/StreamChecker.cpp index 731dd66..c06ba7c 100644 --- a/lib/StaticAnalyzer/Checkers/StreamChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/StreamChecker.cpp @@ -104,15 +104,8 @@ private: } // end anonymous namespace -namespace clang { -namespace ento { - template <> - struct ProgramStateTrait<StreamState> - : public ProgramStatePartialTrait<llvm::ImmutableMap<SymbolRef, StreamState> > { - static void *GDMIndex() { static int x; return &x; } - }; -} -} +REGISTER_MAP_WITH_PROGRAMSTATE(StreamMap, SymbolRef, StreamState) + bool StreamChecker::evalCall(const CallExpr *CE, CheckerContext &C) const { const FunctionDecl *FD = C.getCalleeDecl(CE); @@ -219,11 +212,11 @@ void StreamChecker::Tmpfile(CheckerContext &C, const CallExpr *CE) const { void StreamChecker::OpenFileAux(CheckerContext &C, const CallExpr *CE) const { ProgramStateRef state = C.getState(); - unsigned Count = C.getCurrentBlockCount(); SValBuilder &svalBuilder = C.getSValBuilder(); const LocationContext *LCtx = C.getPredecessor()->getLocationContext(); DefinedSVal RetVal = - cast<DefinedSVal>(svalBuilder.getConjuredSymbolVal(0, CE, LCtx, Count)); + cast<DefinedSVal>(svalBuilder.conjureSymbolVal(0, CE, LCtx, + C.blockCount())); state = state->BindExpr(CE, C.getLocationContext(), RetVal); ConstraintManager &CM = C.getConstraintManager(); @@ -235,9 +228,9 @@ void StreamChecker::OpenFileAux(CheckerContext &C, const CallExpr *CE) const { if (SymbolRef Sym = RetVal.getAsSymbol()) { // if RetVal is not NULL, set the symbol's state to Opened. stateNotNull = - stateNotNull->set<StreamState>(Sym,StreamState::getOpened(CE)); + stateNotNull->set<StreamMap>(Sym,StreamState::getOpened(CE)); stateNull = - stateNull->set<StreamState>(Sym, StreamState::getOpenFailed(CE)); + stateNull->set<StreamMap>(Sym, StreamState::getOpenFailed(CE)); C.addTransition(stateNotNull); C.addTransition(stateNull); @@ -287,7 +280,7 @@ void StreamChecker::Fseek(CheckerContext &C, const CallExpr *CE) const { "SEEK_SET, SEEK_END, or SEEK_CUR.")); BugReport *R = new BugReport(*BT_illegalwhence, BT_illegalwhence->getDescription(), N); - C.EmitReport(R); + C.emitReport(R); } } @@ -363,7 +356,7 @@ ProgramStateRef StreamChecker::CheckNullStream(SVal SV, ProgramStateRef state, BT_nullfp.reset(new BuiltinBug("NULL stream pointer", "Stream pointer might be NULL.")); BugReport *R =new BugReport(*BT_nullfp, BT_nullfp->getDescription(), N); - C.EmitReport(R); + C.emitReport(R); } return 0; } @@ -378,7 +371,7 @@ ProgramStateRef StreamChecker::CheckDoubleClose(const CallExpr *CE, if (!Sym) return state; - const StreamState *SS = state->get<StreamState>(Sym); + const StreamState *SS = state->get<StreamMap>(Sym); // If the file stream is not tracked, return. if (!SS) @@ -395,22 +388,24 @@ ProgramStateRef StreamChecker::CheckDoubleClose(const CallExpr *CE, " closed. Cause undefined behaviour.")); BugReport *R = new BugReport(*BT_doubleclose, BT_doubleclose->getDescription(), N); - C.EmitReport(R); + C.emitReport(R); } return NULL; } // Close the File Descriptor. - return state->set<StreamState>(Sym, StreamState::getClosed(CE)); + return state->set<StreamMap>(Sym, StreamState::getClosed(CE)); } void StreamChecker::checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const { + // TODO: Clean up the state. for (SymbolReaper::dead_iterator I = SymReaper.dead_begin(), E = SymReaper.dead_end(); I != E; ++I) { SymbolRef Sym = *I; ProgramStateRef state = C.getState(); - const StreamState *SS = state->get<StreamState>(Sym); + const StreamState *SS = state->get<StreamMap>(Sym); + // TODO: Shouldn't we have a continue here? if (!SS) return; @@ -422,7 +417,7 @@ void StreamChecker::checkDeadSymbols(SymbolReaper &SymReaper, "Opened File never closed. Potential Resource leak.")); BugReport *R = new BugReport(*BT_ResourceLeak, BT_ResourceLeak->getDescription(), N); - C.EmitReport(R); + C.emitReport(R); } } } @@ -430,10 +425,9 @@ void StreamChecker::checkDeadSymbols(SymbolReaper &SymReaper, void StreamChecker::checkEndPath(CheckerContext &Ctx) const { ProgramStateRef state = Ctx.getState(); - typedef llvm::ImmutableMap<SymbolRef, StreamState> SymMap; - SymMap M = state->get<StreamState>(); + StreamMapTy M = state->get<StreamMap>(); - for (SymMap::iterator I = M.begin(), E = M.end(); I != E; ++I) { + for (StreamMapTy::iterator I = M.begin(), E = M.end(); I != E; ++I) { StreamState SS = I->second; if (SS.isOpened()) { ExplodedNode *N = Ctx.addTransition(state); @@ -443,7 +437,7 @@ void StreamChecker::checkEndPath(CheckerContext &Ctx) const { "Opened File never closed. Potential Resource leak.")); BugReport *R = new BugReport(*BT_ResourceLeak, BT_ResourceLeak->getDescription(), N); - Ctx.EmitReport(R); + Ctx.emitReport(R); } } } @@ -460,12 +454,12 @@ void StreamChecker::checkPreStmt(const ReturnStmt *S, CheckerContext &C) const { if (!Sym) return; - const StreamState *SS = state->get<StreamState>(Sym); + const StreamState *SS = state->get<StreamMap>(Sym); if(!SS) return; if (SS->isOpened()) - state = state->set<StreamState>(Sym, StreamState::getEscaped(S)); + state = state->set<StreamMap>(Sym, StreamState::getEscaped(S)); C.addTransition(state); } diff --git a/lib/StaticAnalyzer/Checkers/TaintTesterChecker.cpp b/lib/StaticAnalyzer/Checkers/TaintTesterChecker.cpp index 1133682..382be84 100644 --- a/lib/StaticAnalyzer/Checkers/TaintTesterChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/TaintTesterChecker.cpp @@ -52,7 +52,7 @@ void TaintTesterChecker::checkPostStmt(const Expr *E, initBugType(); BugReport *report = new BugReport(*BT, "tainted",N); report->addRange(E->getSourceRange()); - C.EmitReport(report); + C.emitReport(report); } } } diff --git a/lib/StaticAnalyzer/Checkers/UndefBranchChecker.cpp b/lib/StaticAnalyzer/Checkers/UndefBranchChecker.cpp index 70a33c7..70e141e 100644 --- a/lib/StaticAnalyzer/Checkers/UndefBranchChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/UndefBranchChecker.cpp @@ -99,11 +99,10 @@ void UndefBranchChecker::checkBranchCondition(const Stmt *Condition, // Emit the bug report. BugReport *R = new BugReport(*BT, BT->getDescription(), N); - bugreporter::addTrackNullOrUndefValueVisitor(N, Ex, R); + bugreporter::trackNullOrUndefValue(N, Ex, *R); R->addRange(Ex->getSourceRange()); - R->disablePathPruning(); - Ctx.EmitReport(R); + Ctx.emitReport(R); } } } diff --git a/lib/StaticAnalyzer/Checkers/UndefCapturedBlockVarChecker.cpp b/lib/StaticAnalyzer/Checkers/UndefCapturedBlockVarChecker.cpp index 675b38a..30ccffa 100644 --- a/lib/StaticAnalyzer/Checkers/UndefCapturedBlockVarChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/UndefCapturedBlockVarChecker.cpp @@ -96,7 +96,7 @@ UndefCapturedBlockVarChecker::checkPostStmt(const BlockExpr *BE, R->addVisitor(new FindLastStoreBRVisitor(VRVal, VR)); R->disablePathPruning(); // need location of block - C.EmitReport(R); + C.emitReport(R); } } } diff --git a/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp b/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp index e220499..415bab5 100644 --- a/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp @@ -76,13 +76,12 @@ void UndefResultChecker::checkPostStmt(const BinaryOperator *B, BugReport *report = new BugReport(*BT, OS.str(), N); if (Ex) { report->addRange(Ex->getSourceRange()); - bugreporter::addTrackNullOrUndefValueVisitor(N, Ex, report); + bugreporter::trackNullOrUndefValue(N, Ex, *report); } else - bugreporter::addTrackNullOrUndefValueVisitor(N, B, report); + bugreporter::trackNullOrUndefValue(N, B, *report); - report->disablePathPruning(); - C.EmitReport(report); + C.emitReport(report); } } diff --git a/lib/StaticAnalyzer/Checkers/UndefinedArraySubscriptChecker.cpp b/lib/StaticAnalyzer/Checkers/UndefinedArraySubscriptChecker.cpp index 6ae3c18..b3a83e8 100644 --- a/lib/StaticAnalyzer/Checkers/UndefinedArraySubscriptChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/UndefinedArraySubscriptChecker.cpp @@ -42,8 +42,8 @@ UndefinedArraySubscriptChecker::checkPreStmt(const ArraySubscriptExpr *A, // Generate a report for this bug. BugReport *R = new BugReport(*BT, BT->getName(), N); R->addRange(A->getIdx()->getSourceRange()); - bugreporter::addTrackNullOrUndefValueVisitor(N, A->getIdx(), R); - C.EmitReport(R); + bugreporter::trackNullOrUndefValue(N, A->getIdx(), *R); + C.emitReport(R); } } } diff --git a/lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp b/lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp index 14a884e..410010a 100644 --- a/lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp @@ -78,10 +78,9 @@ void UndefinedAssignmentChecker::checkBind(SVal location, SVal val, BugReport *R = new BugReport(*BT, str, N); if (ex) { R->addRange(ex->getSourceRange()); - bugreporter::addTrackNullOrUndefValueVisitor(N, ex, R); + bugreporter::trackNullOrUndefValue(N, ex, *R); } - R->disablePathPruning(); - C.EmitReport(R); + C.emitReport(R); } void ento::registerUndefinedAssignmentChecker(CheckerManager &mgr) { diff --git a/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp b/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp index d35455c..171e15b 100644 --- a/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp @@ -41,6 +41,7 @@ public: void CheckCallocZero(CheckerContext &C, const CallExpr *CE) const; void CheckMallocZero(CheckerContext &C, const CallExpr *CE) const; void CheckReallocZero(CheckerContext &C, const CallExpr *CE) const; + void CheckReallocfZero(CheckerContext &C, const CallExpr *CE) const; void CheckAllocaZero(CheckerContext &C, const CallExpr *CE) const; void CheckVallocZero(CheckerContext &C, const CallExpr *CE) const; @@ -138,7 +139,7 @@ void UnixAPIChecker::CheckOpen(CheckerContext &C, const CallExpr *CE) const { "Call to 'open' requires a third argument when " "the 'O_CREAT' flag is set", N); report->addRange(oflagsEx->getSourceRange()); - C.EmitReport(report); + C.emitReport(report); } } @@ -183,11 +184,12 @@ void UnixAPIChecker::CheckPthreadOnce(CheckerContext &C, BugReport *report = new BugReport(*BT_pthreadOnce, os.str(), N); report->addRange(CE->getArg(0)->getSourceRange()); - C.EmitReport(report); + C.emitReport(report); } //===----------------------------------------------------------------------===// -// "calloc", "malloc", "realloc", "alloca" and "valloc" with allocation size 0 +// "calloc", "malloc", "realloc", "reallocf", "alloca" and "valloc" +// with allocation size 0 //===----------------------------------------------------------------------===// // FIXME: Eventually these should be rolled into the MallocChecker, but right now // they're more basic and valuable for widespread use. @@ -224,8 +226,8 @@ bool UnixAPIChecker::ReportZeroByteAllocation(CheckerContext &C, BugReport *report = new BugReport(*BT_mallocZero, os.str(), N); report->addRange(arg->getSourceRange()); - bugreporter::addTrackNullOrUndefValueVisitor(N, arg, report); - C.EmitReport(report); + bugreporter::trackNullOrUndefValue(N, arg, *report); + C.emitReport(report); return true; } @@ -307,6 +309,11 @@ void UnixAPIChecker::CheckReallocZero(CheckerContext &C, BasicAllocationCheck(C, CE, 2, 1, "realloc"); } +void UnixAPIChecker::CheckReallocfZero(CheckerContext &C, + const CallExpr *CE) const { + BasicAllocationCheck(C, CE, 2, 1, "reallocf"); +} + void UnixAPIChecker::CheckAllocaZero(CheckerContext &C, const CallExpr *CE) const { BasicAllocationCheck(C, CE, 1, 0, "alloca"); @@ -339,6 +346,7 @@ void UnixAPIChecker::checkPreStmt(const CallExpr *CE, .Case("calloc", &UnixAPIChecker::CheckCallocZero) .Case("malloc", &UnixAPIChecker::CheckMallocZero) .Case("realloc", &UnixAPIChecker::CheckReallocZero) + .Case("reallocf", &UnixAPIChecker::CheckReallocfZero) .Cases("alloca", "__builtin_alloca", &UnixAPIChecker::CheckAllocaZero) .Case("valloc", &UnixAPIChecker::CheckVallocZero) .Default(NULL); diff --git a/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp b/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp index fab4adf..58f9ec0 100644 --- a/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp @@ -69,8 +69,8 @@ void VLASizeChecker::reportBug(VLASize_Kind Kind, BugReport *report = new BugReport(*BT, os.str(), N); report->addRange(SizeE->getSourceRange()); - bugreporter::addTrackNullOrUndefValueVisitor(N, SizeE, report); - C.EmitReport(report); + bugreporter::trackNullOrUndefValue(N, SizeE, *report); + C.emitReport(report); return; } |
