diff options
Diffstat (limited to 'lib/StaticAnalyzer/Checkers')
37 files changed, 1969 insertions, 1656 deletions
diff --git a/lib/StaticAnalyzer/Checkers/AttrNonNullChecker.cpp b/lib/StaticAnalyzer/Checkers/AttrNonNullChecker.cpp index ab66e98..c582cfc 100644 --- a/lib/StaticAnalyzer/Checkers/AttrNonNullChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/AttrNonNullChecker.cpp @@ -15,6 +15,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" @@ -23,40 +24,32 @@ using namespace ento; namespace { class AttrNonNullChecker - : public Checker< check::PreStmt<CallExpr> > { + : public Checker< check::PreCall > { mutable OwningPtr<BugType> BT; public: - void checkPreStmt(const CallExpr *CE, CheckerContext &C) const; + void checkPreCall(const CallEvent &Call, CheckerContext &C) const; }; } // end anonymous namespace -void AttrNonNullChecker::checkPreStmt(const CallExpr *CE, +void AttrNonNullChecker::checkPreCall(const CallEvent &Call, CheckerContext &C) const { - ProgramStateRef state = C.getState(); - const LocationContext *LCtx = C.getLocationContext(); - - // Check if the callee has a 'nonnull' attribute. - SVal X = state->getSVal(CE->getCallee(), LCtx); - - const FunctionDecl *FD = X.getAsFunctionDecl(); + const Decl *FD = Call.getDecl(); if (!FD) return; - const NonNullAttr* Att = FD->getAttr<NonNullAttr>(); + const NonNullAttr *Att = FD->getAttr<NonNullAttr>(); if (!Att) return; - // Iterate through the arguments of CE and check them for null. - unsigned idx = 0; - - for (CallExpr::const_arg_iterator I=CE->arg_begin(), E=CE->arg_end(); I!=E; - ++I, ++idx) { + ProgramStateRef state = C.getState(); + // Iterate through the arguments of CE and check them for null. + for (unsigned idx = 0, count = Call.getNumArgs(); idx != count; ++idx) { if (!Att->isNonNull(idx)) continue; - SVal V = state->getSVal(*I, LCtx); + SVal V = Call.getArgSVal(idx); DefinedSVal *DV = dyn_cast<DefinedSVal>(&V); // If the value is unknown or undefined, we can't perform this check. @@ -65,11 +58,16 @@ void AttrNonNullChecker::checkPreStmt(const CallExpr *CE, if (!isa<Loc>(*DV)) { // If the argument is a union type, we want to handle a potential - // transparent_unoin GCC extension. - QualType T = (*I)->getType(); + // transparent_union GCC extension. + const Expr *ArgE = Call.getArgExpr(idx); + if (!ArgE) + continue; + + QualType T = ArgE->getType(); const RecordType *UT = T->getAsUnionType(); if (!UT || !UT->getDecl()->hasAttr<TransparentUnionAttr>()) continue; + if (nonloc::CompoundVal *CSV = dyn_cast<nonloc::CompoundVal>(DV)) { nonloc::CompoundVal::iterator CSV_I = CSV->begin(); assert(CSV_I != CSV->end()); @@ -78,8 +76,7 @@ void AttrNonNullChecker::checkPreStmt(const CallExpr *CE, assert(++CSV_I == CSV->end()); if (!DV) continue; - } - else { + } else { // FIXME: Handle LazyCompoundVals? continue; } @@ -106,10 +103,9 @@ void AttrNonNullChecker::checkPreStmt(const CallExpr *CE, "'nonnull' parameter", errorNode); // Highlight the range of the argument that was null. - const Expr *arg = *I; - R->addRange(arg->getSourceRange()); - R->addVisitor(bugreporter::getTrackNullOrUndefValueVisitor(errorNode, - arg, R)); + R->addRange(Call.getArgSourceRange(idx)); + if (const Expr *ArgE = Call.getArgExpr(idx)) + bugreporter::addTrackNullOrUndefValueVisitor(errorNode, ArgE, R); // Emit the bug report. C.EmitReport(R); } diff --git a/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp b/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp index 6dd0a8c..955e79a 100644 --- a/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp +++ b/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp @@ -17,18 +17,20 @@ #include "clang/Analysis/DomainSpecific/CocoaConventions.h" #include "clang/StaticAnalyzer/Core/Checker.h" #include "clang/StaticAnalyzer/Core/CheckerManager.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" #include "clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h" #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h" -#include "clang/StaticAnalyzer/Core/PathSensitive/ObjCMessage.h" #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" #include "clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h" #include "clang/AST/DeclObjC.h" #include "clang/AST/Expr.h" #include "clang/AST/ExprObjC.h" +#include "clang/AST/StmtObjC.h" #include "clang/AST/ASTContext.h" #include "llvm/ADT/SmallString.h" +#include "llvm/ADT/StringMap.h" using namespace clang; using namespace ento; @@ -44,21 +46,40 @@ public: // Utility functions. //===----------------------------------------------------------------------===// -static const char* GetReceiverNameType(const ObjCMessage &msg) { +static StringRef GetReceiverInterfaceName(const ObjCMethodCall &msg) { if (const ObjCInterfaceDecl *ID = msg.getReceiverInterface()) - return ID->getIdentifier()->getNameStart(); - return 0; + return ID->getIdentifier()->getName(); + return StringRef(); } -static bool isReceiverClassOrSuperclass(const ObjCInterfaceDecl *ID, - StringRef ClassName) { - if (ID->getIdentifier()->getName() == ClassName) - return true; +enum FoundationClass { + FC_None, + FC_NSArray, + FC_NSDictionary, + FC_NSEnumerator, + FC_NSOrderedSet, + FC_NSSet, + FC_NSString +}; + +static FoundationClass findKnownClass(const ObjCInterfaceDecl *ID) { + static llvm::StringMap<FoundationClass> Classes; + if (Classes.empty()) { + Classes["NSArray"] = FC_NSArray; + Classes["NSDictionary"] = FC_NSDictionary; + Classes["NSEnumerator"] = FC_NSEnumerator; + Classes["NSOrderedSet"] = FC_NSOrderedSet; + Classes["NSSet"] = FC_NSSet; + Classes["NSString"] = FC_NSString; + } - if (const ObjCInterfaceDecl *Super = ID->getSuperClass()) - return isReceiverClassOrSuperclass(Super, ClassName); + // FIXME: Should we cache this at all? + FoundationClass result = Classes.lookup(ID->getIdentifier()->getName()); + if (result == FC_None) + if (const ObjCInterfaceDecl *Super = ID->getSuperClass()) + return findKnownClass(Super); - return false; + return result; } static inline bool isNil(SVal X) { @@ -74,15 +95,15 @@ namespace { mutable OwningPtr<APIMisuse> BT; void WarnNilArg(CheckerContext &C, - const ObjCMessage &msg, unsigned Arg) const; + const ObjCMethodCall &msg, unsigned Arg) const; public: - void checkPreObjCMessage(ObjCMessage msg, CheckerContext &C) const; + void checkPreObjCMessage(const ObjCMethodCall &M, CheckerContext &C) const; }; } void NilArgChecker::WarnNilArg(CheckerContext &C, - const ObjCMessage &msg, + const ObjCMethodCall &msg, unsigned int Arg) const { if (!BT) @@ -91,7 +112,7 @@ void NilArgChecker::WarnNilArg(CheckerContext &C, if (ExplodedNode *N = C.generateSink()) { SmallString<128> sbuf; llvm::raw_svector_ostream os(sbuf); - os << "Argument to '" << GetReceiverNameType(msg) << "' method '" + os << "Argument to '" << GetReceiverInterfaceName(msg) << "' method '" << msg.getSelector().getAsString() << "' cannot be nil"; BugReport *R = new BugReport(*BT, os.str(), N); @@ -100,13 +121,13 @@ void NilArgChecker::WarnNilArg(CheckerContext &C, } } -void NilArgChecker::checkPreObjCMessage(ObjCMessage msg, +void NilArgChecker::checkPreObjCMessage(const ObjCMethodCall &msg, CheckerContext &C) const { const ObjCInterfaceDecl *ID = msg.getReceiverInterface(); if (!ID) return; - if (isReceiverClassOrSuperclass(ID, "NSString")) { + if (findKnownClass(ID) == FC_NSString) { Selector S = msg.getSelector(); if (S.isUnarySelector()) @@ -130,7 +151,7 @@ void NilArgChecker::checkPreObjCMessage(ObjCMessage msg, Name == "compare:options:range:locale:" || Name == "componentsSeparatedByCharactersInSet:" || Name == "initWithFormat:") { - if (isNil(msg.getArgSVal(0, C.getLocationContext(), C.getState()))) + if (isNil(msg.getArgSVal(0))) WarnNilArg(C, msg, 0); } } @@ -411,8 +432,7 @@ void CFRetainReleaseChecker::checkPreStmt(const CallExpr *CE, BugReport *report = new BugReport(*BT, description, N); report->addRange(Arg->getSourceRange()); - report->addVisitor(bugreporter::getTrackNullOrUndefValueVisitor(N, Arg, - report)); + bugreporter::addTrackNullOrUndefValueVisitor(N, Arg, report); C.EmitReport(report); return; } @@ -434,11 +454,11 @@ class ClassReleaseChecker : public Checker<check::PreObjCMessage> { mutable OwningPtr<BugType> BT; public: - void checkPreObjCMessage(ObjCMessage msg, CheckerContext &C) const; + void checkPreObjCMessage(const ObjCMethodCall &msg, CheckerContext &C) const; }; } -void ClassReleaseChecker::checkPreObjCMessage(ObjCMessage msg, +void ClassReleaseChecker::checkPreObjCMessage(const ObjCMethodCall &msg, CheckerContext &C) const { if (!BT) { @@ -490,18 +510,18 @@ class VariadicMethodTypeChecker : public Checker<check::PreObjCMessage> { mutable Selector initWithObjectsAndKeysS; mutable OwningPtr<BugType> BT; - bool isVariadicMessage(const ObjCMessage &msg) const; + bool isVariadicMessage(const ObjCMethodCall &msg) const; public: - void checkPreObjCMessage(ObjCMessage msg, CheckerContext &C) const; + void checkPreObjCMessage(const ObjCMethodCall &msg, CheckerContext &C) const; }; } /// isVariadicMessage - Returns whether the given message is a variadic message, /// where all arguments must be Objective-C types. bool -VariadicMethodTypeChecker::isVariadicMessage(const ObjCMessage &msg) const { - const ObjCMethodDecl *MD = msg.getMethodDecl(); +VariadicMethodTypeChecker::isVariadicMessage(const ObjCMethodCall &msg) const { + const ObjCMethodDecl *MD = msg.getDecl(); if (!MD || !MD->isVariadic() || isa<ObjCProtocolDecl>(MD->getDeclContext())) return false; @@ -517,53 +537,35 @@ VariadicMethodTypeChecker::isVariadicMessage(const ObjCMessage &msg) const { // gains that this analysis gives. const ObjCInterfaceDecl *Class = MD->getClassInterface(); - // -[NSArray initWithObjects:] - if (isReceiverClassOrSuperclass(Class, "NSArray") && - S == initWithObjectsS) - return true; - - // -[NSDictionary initWithObjectsAndKeys:] - if (isReceiverClassOrSuperclass(Class, "NSDictionary") && - S == initWithObjectsAndKeysS) - return true; - - // -[NSSet initWithObjects:] - if (isReceiverClassOrSuperclass(Class, "NSSet") && - S == initWithObjectsS) - return true; - - // -[NSOrderedSet initWithObjects:] - if (isReceiverClassOrSuperclass(Class, "NSOrderedSet") && - S == initWithObjectsS) - return true; + switch (findKnownClass(Class)) { + case FC_NSArray: + case FC_NSOrderedSet: + case FC_NSSet: + return S == initWithObjectsS; + case FC_NSDictionary: + return S == initWithObjectsAndKeysS; + default: + return false; + } } else { const ObjCInterfaceDecl *Class = msg.getReceiverInterface(); - // -[NSArray arrayWithObjects:] - if (isReceiverClassOrSuperclass(Class, "NSArray") && - S == arrayWithObjectsS) - return true; - - // -[NSDictionary dictionaryWithObjectsAndKeys:] - if (isReceiverClassOrSuperclass(Class, "NSDictionary") && - S == dictionaryWithObjectsAndKeysS) - return true; - - // -[NSSet setWithObjects:] - if (isReceiverClassOrSuperclass(Class, "NSSet") && - S == setWithObjectsS) - return true; - - // -[NSOrderedSet orderedSetWithObjects:] - if (isReceiverClassOrSuperclass(Class, "NSOrderedSet") && - S == orderedSetWithObjectsS) - return true; + switch (findKnownClass(Class)) { + case FC_NSArray: + return S == arrayWithObjectsS; + case FC_NSOrderedSet: + return S == orderedSetWithObjectsS; + case FC_NSSet: + return S == setWithObjectsS; + case FC_NSDictionary: + return S == dictionaryWithObjectsAndKeysS; + default: + return false; + } } - - return false; } -void VariadicMethodTypeChecker::checkPreObjCMessage(ObjCMessage msg, +void VariadicMethodTypeChecker::checkPreObjCMessage(const ObjCMethodCall &msg, CheckerContext &C) const { if (!BT) { BT.reset(new APIMisuse("Arguments passed to variadic method aren't all " @@ -599,7 +601,7 @@ void VariadicMethodTypeChecker::checkPreObjCMessage(ObjCMessage msg, ProgramStateRef state = C.getState(); for (unsigned I = variadicArgsBegin; I != variadicArgsEnd; ++I) { - QualType ArgTy = msg.getArgType(I); + QualType ArgTy = msg.getArgExpr(I)->getType(); if (ArgTy->isObjCObjectPointerType()) continue; @@ -608,8 +610,7 @@ void VariadicMethodTypeChecker::checkPreObjCMessage(ObjCMessage msg, continue; // Ignore pointer constants. - if (isa<loc::ConcreteInt>(msg.getArgSVal(I, C.getLocationContext(), - state))) + if (isa<loc::ConcreteInt>(msg.getArgSVal(I))) continue; // Ignore pointer types annotated with 'NSObject' attribute. @@ -621,9 +622,8 @@ void VariadicMethodTypeChecker::checkPreObjCMessage(ObjCMessage msg, continue; // Generate only one error node to use for all bug reports. - if (!errorNode.hasValue()) { + if (!errorNode.hasValue()) errorNode = C.addTransition(); - } if (!errorNode.getValue()) continue; @@ -631,23 +631,93 @@ void VariadicMethodTypeChecker::checkPreObjCMessage(ObjCMessage msg, SmallString<128> sbuf; llvm::raw_svector_ostream os(sbuf); - if (const char *TypeName = GetReceiverNameType(msg)) + StringRef TypeName = GetReceiverInterfaceName(msg); + if (!TypeName.empty()) os << "Argument to '" << TypeName << "' method '"; else os << "Argument to method '"; os << msg.getSelector().getAsString() - << "' should be an Objective-C pointer type, not '" - << ArgTy.getAsString() << "'"; + << "' should be an Objective-C pointer type, not '"; + ArgTy.print(os, C.getLangOpts()); + os << "'"; - BugReport *R = new BugReport(*BT, os.str(), - errorNode.getValue()); + BugReport *R = new BugReport(*BT, os.str(), errorNode.getValue()); R->addRange(msg.getArgSourceRange(I)); C.EmitReport(R); } } //===----------------------------------------------------------------------===// +// Improves the modeling of loops over Cocoa collections. +//===----------------------------------------------------------------------===// + +namespace { +class ObjCLoopChecker + : public Checker<check::PostStmt<ObjCForCollectionStmt> > { + +public: + void checkPostStmt(const ObjCForCollectionStmt *FCS, CheckerContext &C) const; +}; +} + +static bool isKnownNonNilCollectionType(QualType T) { + const ObjCObjectPointerType *PT = T->getAs<ObjCObjectPointerType>(); + if (!PT) + return false; + + const ObjCInterfaceDecl *ID = PT->getInterfaceDecl(); + if (!ID) + return false; + + switch (findKnownClass(ID)) { + case FC_NSArray: + case FC_NSDictionary: + case FC_NSEnumerator: + case FC_NSOrderedSet: + case FC_NSSet: + return true; + default: + return false; + } +} + +void ObjCLoopChecker::checkPostStmt(const ObjCForCollectionStmt *FCS, + CheckerContext &C) const { + ProgramStateRef State = C.getState(); + + // Check if this is the branch for the end of the loop. + SVal CollectionSentinel = State->getSVal(FCS, C.getLocationContext()); + if (CollectionSentinel.isZeroConstant()) + return; + + // See if the collection is one where we /know/ the elements are non-nil. + const Expr *Collection = FCS->getCollection(); + if (!isKnownNonNilCollectionType(Collection->getType())) + return; + + // FIXME: Copied from ExprEngineObjC. + const Stmt *Element = FCS->getElement(); + SVal ElementVar; + if (const DeclStmt *DS = dyn_cast<DeclStmt>(Element)) { + const VarDecl *ElemDecl = cast<VarDecl>(DS->getSingleDecl()); + assert(ElemDecl->getInit() == 0); + ElementVar = State->getLValue(ElemDecl, C.getLocationContext()); + } else { + ElementVar = State->getSVal(Element, C.getLocationContext()); + } + + if (!isa<Loc>(ElementVar)) + return; + + // Go ahead and assume the value is non-nil. + SVal Val = State->getSVal(cast<Loc>(ElementVar)); + State = State->assume(cast<DefinedOrUnknownSVal>(Val), true); + C.addTransition(State); +} + + +//===----------------------------------------------------------------------===// // Check registration. //===----------------------------------------------------------------------===// @@ -670,3 +740,7 @@ void ento::registerClassReleaseChecker(CheckerManager &mgr) { void ento::registerVariadicMethodTypeChecker(CheckerManager &mgr) { mgr.registerChecker<VariadicMethodTypeChecker>(); } + +void ento::registerObjCLoopChecker(CheckerManager &mgr) { + mgr.registerChecker<ObjCLoopChecker>(); +} diff --git a/lib/StaticAnalyzer/Checkers/CMakeLists.txt b/lib/StaticAnalyzer/Checkers/CMakeLists.txt index a377ca9..7fe51d3 100644 --- a/lib/StaticAnalyzer/Checkers/CMakeLists.txt +++ b/lib/StaticAnalyzer/Checkers/CMakeLists.txt @@ -3,8 +3,6 @@ clang_tablegen(Checkers.inc -gen-clang-sa-checkers SOURCE Checkers.td TARGET ClangSACheckers) -set(LLVM_USED_LIBS clangBasic clangAST clangStaticAnalyzerCore) - add_clang_library(clangStaticAnalyzerCheckers AdjustedReturnValueChecker.cpp AnalyzerStatsChecker.cpp @@ -31,10 +29,11 @@ add_clang_library(clangStaticAnalyzerCheckers DebugCheckers.cpp DereferenceChecker.cpp DivZeroChecker.cpp + DynamicTypePropagation.cpp + ExprInspectionChecker.cpp FixedAddressChecker.cpp GenericTaintChecker.cpp IdempotentOperationChecker.cpp - IteratorsChecker.cpp LLVMConventionsChecker.cpp MacOSKeychainAPIChecker.cpp MacOSXAPIChecker.cpp @@ -59,6 +58,7 @@ add_clang_library(clangStaticAnalyzerCheckers StackAddrEscapeChecker.cpp StreamChecker.cpp TaintTesterChecker.cpp + TraversalChecker.cpp UndefBranchChecker.cpp UndefCapturedBlockVarChecker.cpp UndefResultChecker.cpp @@ -74,7 +74,15 @@ add_dependencies(clangStaticAnalyzerCheckers clangStaticAnalyzerCore ClangAttrClasses ClangAttrList + ClangCommentNodes ClangDeclNodes + ClangDiagnosticCommon ClangStmtNodes ClangSACheckers ) + +target_link_libraries(clangStaticAnalyzerCheckers + clangBasic + clangAST + clangStaticAnalyzerCore + ) diff --git a/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/lib/StaticAnalyzer/Checkers/CStringChecker.cpp index 9eb7edf..483082a 100644 --- a/lib/StaticAnalyzer/Checkers/CStringChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/CStringChecker.cpp @@ -66,7 +66,7 @@ public: const StoreManager::InvalidatedSymbols *, ArrayRef<const MemRegion *> ExplicitRegions, ArrayRef<const MemRegion *> Regions, - const CallOrObjCMessage *Call) const; + const CallEvent *Call) const; typedef void (CStringChecker::*FnCheck)(CheckerContext &, const CallExpr *) const; @@ -252,8 +252,7 @@ ProgramStateRef CStringChecker::checkNonNull(CheckerContext &C, BugReport *report = new BugReport(*BT, os.str(), N); report->addRange(S->getSourceRange()); - report->addVisitor(bugreporter::getTrackNullOrUndefValueVisitor(N, S, - report)); + bugreporter::addTrackNullOrUndefValueVisitor(N, S, report); C.EmitReport(report); return NULL; } @@ -901,9 +900,10 @@ void CStringChecker::evalCopyCommon(CheckerContext &C, // If the size is zero, there won't be any actual memory access, so // just bind the return value to the destination buffer and return. - if (stateZeroSize) { + if (stateZeroSize && !stateNonZeroSize) { stateZeroSize = stateZeroSize->BindExpr(CE, LCtx, destVal); C.addTransition(stateZeroSize); + return; } // If the size can be nonzero, we have to check the other arguments. @@ -1403,6 +1403,24 @@ void CStringChecker::evalStrcpyCommon(CheckerContext &C, const CallExpr *CE, // For strncpy, this is just checking that lenVal <= sizeof(dst) // (Yes, strncpy and strncat differ in how they treat termination. // strncat ALWAYS terminates, but strncpy doesn't.) + + // We need a special case for when the copy size is zero, in which + // case strncpy will do no work at all. Our bounds check uses n-1 + // as the last element accessed, so n == 0 is problematic. + ProgramStateRef StateZeroSize, StateNonZeroSize; + llvm::tie(StateZeroSize, StateNonZeroSize) = + assumeZero(C, state, *lenValNL, sizeTy); + + // If the size is known to be zero, we're done. + if (StateZeroSize && !StateNonZeroSize) { + StateZeroSize = StateZeroSize->BindExpr(CE, LCtx, DstVal); + C.addTransition(StateZeroSize); + return; + } + + // Otherwise, go ahead and figure out the last element we'll touch. + // We don't record the non-zero assumption here because we can't + // be sure. We won't warn on a possible zero. NonLoc one = cast<NonLoc>(svalBuilder.makeIntVal(1, sizeTy)); maxLastElementIndex = svalBuilder.evalBinOpNN(state, BO_Sub, *lenValNL, one, sizeTy); @@ -1876,7 +1894,7 @@ CStringChecker::checkRegionChanges(ProgramStateRef state, const StoreManager::InvalidatedSymbols *, ArrayRef<const MemRegion *> ExplicitRegions, ArrayRef<const MemRegion *> Regions, - const CallOrObjCMessage *Call) const { + const CallEvent *Call) const { CStringLength::EntryMap Entries = state->get<CStringLength>(); if (Entries.isEmpty()) return state; diff --git a/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp b/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp index f601431..30f45c7 100644 --- a/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp @@ -15,8 +15,8 @@ #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/PathSensitive/ObjCMessage.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" #include "clang/AST/ParentMap.h" #include "clang/Basic/TargetInfo.h" @@ -27,35 +27,37 @@ using namespace ento; namespace { class CallAndMessageChecker - : public Checker< check::PreStmt<CallExpr>, check::PreObjCMessage > { + : public Checker< check::PreStmt<CallExpr>, check::PreObjCMessage, + check::PreCall > { mutable OwningPtr<BugType> BT_call_null; mutable OwningPtr<BugType> BT_call_undef; + mutable OwningPtr<BugType> BT_cxx_call_null; + mutable OwningPtr<BugType> BT_cxx_call_undef; mutable OwningPtr<BugType> BT_call_arg; mutable OwningPtr<BugType> BT_msg_undef; mutable OwningPtr<BugType> BT_objc_prop_undef; + mutable OwningPtr<BugType> BT_objc_subscript_undef; mutable OwningPtr<BugType> BT_msg_arg; mutable OwningPtr<BugType> BT_msg_ret; public: void checkPreStmt(const CallExpr *CE, CheckerContext &C) const; - void checkPreObjCMessage(ObjCMessage msg, CheckerContext &C) const; + void checkPreObjCMessage(const ObjCMethodCall &msg, CheckerContext &C) const; + void checkPreCall(const CallEvent &Call, CheckerContext &C) const; private: - static void PreVisitProcessArgs(CheckerContext &C,CallOrObjCMessage callOrMsg, - const char *BT_desc, OwningPtr<BugType> &BT); - static bool PreVisitProcessArg(CheckerContext &C, SVal V,SourceRange argRange, - const Expr *argEx, - const bool checkUninitFields, - const char *BT_desc, - OwningPtr<BugType> &BT); - - static void EmitBadCall(BugType *BT, CheckerContext &C, const CallExpr *CE); - void emitNilReceiverBug(CheckerContext &C, const ObjCMessage &msg, + static bool PreVisitProcessArg(CheckerContext &C, SVal V, + SourceRange argRange, const Expr *argEx, + bool IsFirstArgument, bool checkUninitFields, + const CallEvent &Call, OwningPtr<BugType> &BT); + + static void emitBadCall(BugType *BT, CheckerContext &C, const Expr *BadE); + void emitNilReceiverBug(CheckerContext &C, const ObjCMethodCall &msg, ExplodedNode *N) const; void HandleNilReceiver(CheckerContext &C, ProgramStateRef state, - ObjCMessage msg) const; + const ObjCMethodCall &msg) const; static void LazyInit_BT(const char *desc, OwningPtr<BugType> &BT) { if (!BT) @@ -64,55 +66,63 @@ private: }; } // end anonymous namespace -void CallAndMessageChecker::EmitBadCall(BugType *BT, CheckerContext &C, - const CallExpr *CE) { +void CallAndMessageChecker::emitBadCall(BugType *BT, CheckerContext &C, + const Expr *BadE) { ExplodedNode *N = C.generateSink(); if (!N) return; BugReport *R = new BugReport(*BT, BT->getName(), N); - R->addVisitor(bugreporter::getTrackNullOrUndefValueVisitor(N, - bugreporter::GetCalleeExpr(N), R)); + if (BadE) { + R->addRange(BadE->getSourceRange()); + bugreporter::addTrackNullOrUndefValueVisitor(N, BadE, R); + } C.EmitReport(R); } -void CallAndMessageChecker::PreVisitProcessArgs(CheckerContext &C, - CallOrObjCMessage callOrMsg, - const char *BT_desc, - OwningPtr<BugType> &BT) { - // Don't check for uninitialized field values in arguments if the - // caller has a body that is available and we have the chance to inline it. - // This is a hack, but is a reasonable compromise betweens sometimes warning - // and sometimes not depending on if we decide to inline a function. - const Decl *D = callOrMsg.getDecl(); - const bool checkUninitFields = - !(C.getAnalysisManager().shouldInlineCall() && - (D && D->getBody())); - - for (unsigned i = 0, e = callOrMsg.getNumArgs(); i != e; ++i) - if (PreVisitProcessArg(C, callOrMsg.getArgSVal(i), - callOrMsg.getArgSourceRange(i), callOrMsg.getArg(i), - checkUninitFields, - BT_desc, BT)) - return; +StringRef describeUninitializedArgumentInCall(const CallEvent &Call, + bool IsFirstArgument) { + switch (Call.getKind()) { + case CE_ObjCMessage: { + const ObjCMethodCall &Msg = cast<ObjCMethodCall>(Call); + switch (Msg.getMessageKind()) { + case OCM_Message: + return "Argument in message expression is an uninitialized value"; + case OCM_PropertyAccess: + assert(Msg.isSetter() && "Getters have no args"); + return "Argument for property setter is an uninitialized value"; + case OCM_Subscript: + if (Msg.isSetter() && IsFirstArgument) + return "Argument for subscript setter is an uninitialized value"; + return "Subscript index is an uninitialized value"; + } + llvm_unreachable("Unknown message kind."); + } + case CE_Block: + return "Block call argument is an uninitialized value"; + default: + return "Function call argument is an uninitialized value"; + } } bool CallAndMessageChecker::PreVisitProcessArg(CheckerContext &C, SVal V, SourceRange argRange, const Expr *argEx, - const bool checkUninitFields, - const char *BT_desc, + bool IsFirstArgument, + bool checkUninitFields, + const CallEvent &Call, OwningPtr<BugType> &BT) { if (V.isUndef()) { if (ExplodedNode *N = C.generateSink()) { - LazyInit_BT(BT_desc, BT); + LazyInit_BT("Uninitialized argument value", BT); // Generate a report for this bug. - BugReport *R = new BugReport(*BT, BT->getName(), N); + StringRef Desc = describeUninitializedArgumentInCall(Call, + IsFirstArgument); + BugReport *R = new BugReport(*BT, Desc, N); R->addRange(argRange); if (argEx) - R->addVisitor(bugreporter::getTrackNullOrUndefValueVisitor(N, argEx, - R)); + bugreporter::addTrackNullOrUndefValueVisitor(N, argEx, R); C.EmitReport(R); } return true; @@ -128,14 +138,13 @@ bool CallAndMessageChecker::PreVisitProcessArg(CheckerContext &C, public: SmallVector<const FieldDecl *, 10> FieldChain; private: - ASTContext &C; StoreManager &StoreMgr; MemRegionManager &MrMgr; Store store; public: - FindUninitializedField(ASTContext &c, StoreManager &storeMgr, + FindUninitializedField(StoreManager &storeMgr, MemRegionManager &mrMgr, Store s) - : C(c), StoreMgr(storeMgr), MrMgr(mrMgr), store(s) {} + : StoreMgr(storeMgr), MrMgr(mrMgr), store(s) {} bool Find(const TypedValueRegion *R) { QualType T = R->getValueType(); @@ -146,7 +155,7 @@ bool CallAndMessageChecker::PreVisitProcessArg(CheckerContext &C, RD->field_begin(), E = RD->field_end(); I!=E; ++I) { const FieldRegion *FR = MrMgr.getFieldRegion(*I, R); FieldChain.push_back(*I); - T = (*I)->getType(); + T = I->getType(); if (T->getAsStructureType()) { if (Find(FR)) return true; @@ -165,14 +174,13 @@ bool CallAndMessageChecker::PreVisitProcessArg(CheckerContext &C, }; const LazyCompoundValData *D = LV->getCVData(); - FindUninitializedField F(C.getASTContext(), - C.getState()->getStateManager().getStoreManager(), + FindUninitializedField F(C.getState()->getStateManager().getStoreManager(), C.getSValBuilder().getRegionManager(), D->getStore()); if (F.Find(D->getRegion())) { if (ExplodedNode *N = C.generateSink()) { - LazyInit_BT(BT_desc, BT); + LazyInit_BT("Uninitialized argument value", BT); SmallString<512> Str; llvm::raw_svector_ostream os(Str); os << "Passed-by-value struct argument contains uninitialized data"; @@ -212,87 +220,137 @@ void CallAndMessageChecker::checkPreStmt(const CallExpr *CE, CheckerContext &C) const{ const Expr *Callee = CE->getCallee()->IgnoreParens(); + ProgramStateRef State = C.getState(); const LocationContext *LCtx = C.getLocationContext(); - SVal L = C.getState()->getSVal(Callee, LCtx); + SVal L = State->getSVal(Callee, LCtx); if (L.isUndef()) { if (!BT_call_undef) BT_call_undef.reset(new BuiltinBug("Called function pointer is an " "uninitalized pointer value")); - EmitBadCall(BT_call_undef.get(), C, CE); + emitBadCall(BT_call_undef.get(), C, Callee); return; } - if (isa<loc::ConcreteInt>(L)) { + ProgramStateRef StNonNull, StNull; + llvm::tie(StNonNull, StNull) = State->assume(cast<DefinedOrUnknownSVal>(L)); + + // FIXME: Do we want to record the non-null assumption here? + if (StNull && !StNonNull) { if (!BT_call_null) BT_call_null.reset( new BuiltinBug("Called function pointer is null (null dereference)")); - EmitBadCall(BT_call_null.get(), C, CE); + emitBadCall(BT_call_null.get(), C, Callee); + } +} + +void CallAndMessageChecker::checkPreCall(const CallEvent &Call, + CheckerContext &C) const { + // If this is a call to a C++ method, check if the callee is null or + // undefined. + if (const CXXInstanceCall *CC = dyn_cast<CXXInstanceCall>(&Call)) { + SVal V = CC->getCXXThisVal(); + if (V.isUndef()) { + if (!BT_cxx_call_undef) + BT_cxx_call_undef.reset(new BuiltinBug("Called C++ object pointer is " + "uninitialized")); + emitBadCall(BT_cxx_call_undef.get(), C, CC->getCXXThisExpr()); + return; + } + + ProgramStateRef State = C.getState(); + ProgramStateRef StNonNull, StNull; + llvm::tie(StNonNull, StNull) = State->assume(cast<DefinedOrUnknownSVal>(V)); + + // FIXME: Do we want to record the non-null assumption here? + if (StNull && !StNonNull) { + if (!BT_cxx_call_null) + BT_cxx_call_null.reset(new BuiltinBug("Called C++ object pointer " + "is null")); + emitBadCall(BT_cxx_call_null.get(), C, CC->getCXXThisExpr()); + return; + } } - PreVisitProcessArgs(C, CallOrObjCMessage(CE, C.getState(), LCtx), - "Function call argument is an uninitialized value", - BT_call_arg); + // Don't check for uninitialized field values in arguments if the + // caller has a body that is available and we have the chance to inline it. + // This is a hack, but is a reasonable compromise betweens sometimes warning + // and sometimes not depending on if we decide to inline a function. + const Decl *D = Call.getDecl(); + const bool checkUninitFields = + !(C.getAnalysisManager().shouldInlineCall() && (D && D->getBody())); + + OwningPtr<BugType> *BT; + if (isa<ObjCMethodCall>(Call)) + BT = &BT_msg_arg; + else + BT = &BT_call_arg; + + for (unsigned i = 0, e = Call.getNumArgs(); i != e; ++i) + if (PreVisitProcessArg(C, Call.getArgSVal(i), Call.getArgSourceRange(i), + Call.getArgExpr(i), /*IsFirstArgument=*/i == 0, + checkUninitFields, Call, *BT)) + return; } -void CallAndMessageChecker::checkPreObjCMessage(ObjCMessage msg, +void CallAndMessageChecker::checkPreObjCMessage(const ObjCMethodCall &msg, CheckerContext &C) const { + SVal recVal = msg.getReceiverSVal(); + if (recVal.isUndef()) { + if (ExplodedNode *N = C.generateSink()) { + BugType *BT = 0; + switch (msg.getMessageKind()) { + case OCM_Message: + if (!BT_msg_undef) + BT_msg_undef.reset(new BuiltinBug("Receiver in message expression " + "is an uninitialized value")); + BT = BT_msg_undef.get(); + break; + case OCM_PropertyAccess: + if (!BT_objc_prop_undef) + BT_objc_prop_undef.reset(new BuiltinBug("Property access on an " + "uninitialized object " + "pointer")); + BT = BT_objc_prop_undef.get(); + break; + case OCM_Subscript: + if (!BT_objc_subscript_undef) + BT_objc_subscript_undef.reset(new BuiltinBug("Subscript access on an " + "uninitialized object " + "pointer")); + BT = BT_objc_subscript_undef.get(); + break; + } + assert(BT && "Unknown message kind."); - ProgramStateRef state = C.getState(); - const LocationContext *LCtx = C.getLocationContext(); + BugReport *R = new BugReport(*BT, BT->getName(), N); + const ObjCMessageExpr *ME = msg.getOriginExpr(); + R->addRange(ME->getReceiverRange()); - // FIXME: Handle 'super'? - if (const Expr *receiver = msg.getInstanceReceiver()) { - SVal recVal = state->getSVal(receiver, LCtx); - if (recVal.isUndef()) { - if (ExplodedNode *N = C.generateSink()) { - BugType *BT = 0; - if (msg.isPureMessageExpr()) { - if (!BT_msg_undef) - BT_msg_undef.reset(new BuiltinBug("Receiver in message expression " - "is an uninitialized value")); - BT = BT_msg_undef.get(); - } - else { - if (!BT_objc_prop_undef) - BT_objc_prop_undef.reset(new BuiltinBug("Property access on an " - "uninitialized object pointer")); - BT = BT_objc_prop_undef.get(); - } - BugReport *R = - new BugReport(*BT, BT->getName(), N); - R->addRange(receiver->getSourceRange()); - R->addVisitor(bugreporter::getTrackNullOrUndefValueVisitor(N, - receiver, - R)); - C.EmitReport(R); - } + // FIXME: getTrackNullOrUndefValueVisitor can't handle "super" yet. + if (const Expr *ReceiverE = ME->getInstanceReceiver()) + bugreporter::addTrackNullOrUndefValueVisitor(N, ReceiverE, R); + C.EmitReport(R); + } + return; + } else { + // Bifurcate the state into nil and non-nil ones. + DefinedOrUnknownSVal receiverVal = cast<DefinedOrUnknownSVal>(recVal); + + ProgramStateRef state = C.getState(); + ProgramStateRef notNilState, nilState; + llvm::tie(notNilState, nilState) = state->assume(receiverVal); + + // Handle receiver must be nil. + if (nilState && !notNilState) { + HandleNilReceiver(C, state, msg); return; - } else { - // Bifurcate the state into nil and non-nil ones. - DefinedOrUnknownSVal receiverVal = cast<DefinedOrUnknownSVal>(recVal); - - ProgramStateRef notNilState, nilState; - llvm::tie(notNilState, nilState) = state->assume(receiverVal); - - // Handle receiver must be nil. - if (nilState && !notNilState) { - HandleNilReceiver(C, state, msg); - return; - } } } - - const char *bugDesc = msg.isPropertySetter() ? - "Argument for property setter is an uninitialized value" - : "Argument in message expression is an uninitialized value"; - // Check for any arguments that are uninitialized/undefined. - PreVisitProcessArgs(C, CallOrObjCMessage(msg, state, LCtx), - bugDesc, BT_msg_arg); } void CallAndMessageChecker::emitNilReceiverBug(CheckerContext &C, - const ObjCMessage &msg, + const ObjCMethodCall &msg, ExplodedNode *N) const { if (!BT_msg_ret) @@ -300,18 +358,20 @@ void CallAndMessageChecker::emitNilReceiverBug(CheckerContext &C, new BuiltinBug("Receiver in message expression is " "'nil' and returns a garbage value")); + const ObjCMessageExpr *ME = msg.getOriginExpr(); + SmallString<200> buf; llvm::raw_svector_ostream os(buf); - os << "The receiver of message '" << msg.getSelector().getAsString() - << "' is nil and returns a value of type '" - << msg.getType(C.getASTContext()).getAsString() << "' that will be garbage"; + os << "The receiver of message '" << ME->getSelector().getAsString() + << "' is nil and returns a value of type '"; + msg.getResultType().print(os, C.getLangOpts()); + os << "' that will be garbage"; BugReport *report = new BugReport(*BT_msg_ret, os.str(), N); - if (const Expr *receiver = msg.getInstanceReceiver()) { - report->addRange(receiver->getSourceRange()); - report->addVisitor(bugreporter::getTrackNullOrUndefValueVisitor(N, - receiver, - report)); + 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); } C.EmitReport(report); } @@ -324,25 +384,25 @@ static bool supportsNilWithFloatRet(const llvm::Triple &triple) { void CallAndMessageChecker::HandleNilReceiver(CheckerContext &C, ProgramStateRef state, - ObjCMessage msg) const { + const ObjCMethodCall &Msg) const { ASTContext &Ctx = C.getASTContext(); // Check the return type of the message expression. A message to nil will // return different values depending on the return type and the architecture. - QualType RetTy = msg.getType(Ctx); + QualType RetTy = Msg.getResultType(); CanQualType CanRetTy = Ctx.getCanonicalType(RetTy); const LocationContext *LCtx = C.getLocationContext(); if (CanRetTy->isStructureOrClassType()) { // Structure returns are safe since the compiler zeroes them out. - SVal V = C.getSValBuilder().makeZeroVal(msg.getType(Ctx)); - C.addTransition(state->BindExpr(msg.getMessageExpr(), LCtx, V)); + SVal V = C.getSValBuilder().makeZeroVal(RetTy); + C.addTransition(state->BindExpr(Msg.getOriginExpr(), LCtx, V)); return; } // Other cases: check if sizeof(return type) > sizeof(void*) if (CanRetTy != Ctx.VoidTy && C.getLocationContext()->getParentMap() - .isConsumedExpr(msg.getMessageExpr())) { + .isConsumedExpr(Msg.getOriginExpr())) { // Compute: sizeof(void *) and sizeof(return type) const uint64_t voidPtrSize = Ctx.getTypeSize(Ctx.VoidPtrTy); const uint64_t returnTypeSize = Ctx.getTypeSize(CanRetTy); @@ -355,7 +415,7 @@ void CallAndMessageChecker::HandleNilReceiver(CheckerContext &C, Ctx.LongLongTy == CanRetTy || Ctx.UnsignedLongLongTy == CanRetTy))) { if (ExplodedNode *N = C.generateSink(state)) - emitNilReceiverBug(C, msg, N); + emitNilReceiverBug(C, Msg, N); return; } @@ -372,8 +432,8 @@ void CallAndMessageChecker::HandleNilReceiver(CheckerContext &C, // it most likely isn't nil. We should assume the semantics // of this case unless we have *a lot* more knowledge. // - SVal V = C.getSValBuilder().makeZeroVal(msg.getType(Ctx)); - C.addTransition(state->BindExpr(msg.getMessageExpr(), LCtx, V)); + SVal V = C.getSValBuilder().makeZeroVal(RetTy); + C.addTransition(state->BindExpr(Msg.getOriginExpr(), LCtx, V)); return; } diff --git a/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp b/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp index 133204a..7a25865 100644 --- a/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp +++ b/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp @@ -215,10 +215,10 @@ static void checkObjCDealloc(const ObjCImplementationDecl *D, E = D->propimpl_end(); I!=E; ++I) { // We can only check the synthesized properties - if ((*I)->getPropertyImplementation() != ObjCPropertyImplDecl::Synthesize) + if (I->getPropertyImplementation() != ObjCPropertyImplDecl::Synthesize) continue; - ObjCIvarDecl *ID = (*I)->getPropertyIvarDecl(); + ObjCIvarDecl *ID = I->getPropertyIvarDecl(); if (!ID) continue; @@ -226,7 +226,7 @@ static void checkObjCDealloc(const ObjCImplementationDecl *D, if (!T->isObjCObjectPointerType()) // Skip non-pointer ivars continue; - const ObjCPropertyDecl *PD = (*I)->getPropertyDecl(); + const ObjCPropertyDecl *PD = I->getPropertyDecl(); if (!PD) continue; @@ -261,7 +261,7 @@ static void checkObjCDealloc(const ObjCImplementationDecl *D, } PathDiagnosticLocation SDLoc = - PathDiagnosticLocation::createBegin((*I), BR.getSourceManager()); + PathDiagnosticLocation::createBegin(*I, BR.getSourceManager()); BR.EmitBasicReport(MD, name, categories::CoreFoundationObjectiveC, os.str(), SDLoc); diff --git a/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp b/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp index dde9071..b8b7c36 100644 --- a/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp +++ b/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp @@ -31,6 +31,7 @@ static bool isArc4RandomAvailable(const ASTContext &Ctx) { T.getOS() == llvm::Triple::FreeBSD || T.getOS() == llvm::Triple::NetBSD || T.getOS() == llvm::Triple::OpenBSD || + T.getOS() == llvm::Triple::Bitrig || T.getOS() == llvm::Triple::DragonFly; } diff --git a/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp b/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp index 843502f..0e9efaa 100644 --- a/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp +++ b/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp @@ -37,6 +37,8 @@ class CheckerDocumentation : public Checker< check::PreStmt<DeclStmt>, check::PostStmt<CallExpr>, check::PreObjCMessage, check::PostObjCMessage, + check::PreCall, + check::PostCall, check::BranchCondition, check::Location, check::Bind, @@ -72,14 +74,41 @@ 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<DeclStmt> + /// check::PostStmt<CallExpr> void checkPostStmt(const CallExpr *DS, CheckerContext &C) const; - /// \brief Pre-visit the Objective C messages. - void checkPreObjCMessage(const ObjCMessage &Msg, CheckerContext &C) const {} + /// \brief Pre-visit the Objective C message. + /// + /// This will be called before the analyzer core processes the method call. + /// This is called for any action which produces an Objective-C message send, + /// including explicit message syntax and property access. + /// + /// check::PreObjCMessage + void checkPreObjCMessage(const ObjCMethodCall &M, CheckerContext &C) const {} + + /// \brief Post-visit the Objective C message. + /// \sa checkPreObjCMessage() + /// + /// check::PostObjCMessage + void checkPostObjCMessage(const ObjCMethodCall &M, CheckerContext &C) const {} - /// \brief Post-visit the Objective C messages. - void checkPostObjCMessage(const ObjCMessage &Msg, CheckerContext &C) const {} + /// \brief Pre-visit an abstract "call" event. + /// + /// This is used for checkers that want to check arguments or attributed + /// 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). + /// + /// check::PreCall + void checkPreCall(const CallEvent &Call, CheckerContext &C) const {} + + /// \brief Post-visit an abstract "call" event. + /// \sa checkPreObjCMessage() + /// + /// check::PostCall + void checkPostCall(const CallEvent &Call, CheckerContext &C) const {} /// \brief Pre-visit of the condition statement of a branch (such as IfStmt). void checkBranchCondition(const Stmt *Condition, CheckerContext &Ctx) const {} @@ -94,7 +123,7 @@ public: /// /// check::Location void checkLocation(SVal Loc, bool IsLoad, const Stmt *S, - CheckerContext &C) const {} + CheckerContext &) const {} /// \brief Called on binding of a value to a location. /// @@ -103,7 +132,7 @@ public: /// \param S The bind is performed while processing the statement S. /// /// check::Bind - void checkBind(SVal Loc, SVal Val, const Stmt *S, CheckerContext &C) const {} + void checkBind(SVal Loc, SVal Val, const Stmt *S, CheckerContext &) const {} /// \brief Called whenever a symbol becomes dead. @@ -187,24 +216,26 @@ public: bool wantsRegionChangeUpdate(ProgramStateRef St) const { return true; } - /// check::RegionChanges - /// Allows tracking regions which get invalidated. - /// \param state The current program state. - /// \param invalidated A set of all symbols potentially touched by the change. + /// \brief Allows tracking regions which get invalidated. + /// + /// \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 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 + /// \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. + /// + /// check::RegionChanges ProgramStateRef checkRegionChanges(ProgramStateRef State, - const StoreManager::InvalidatedSymbols *, + const StoreManager::InvalidatedSymbols *Invalidated, ArrayRef<const MemRegion *> ExplicitRegions, ArrayRef<const MemRegion *> Regions, - const CallOrObjCMessage *Call) const { + const CallEvent *Call) const { return State; } diff --git a/lib/StaticAnalyzer/Checkers/Checkers.td b/lib/StaticAnalyzer/Checkers/Checkers.td index 96a8d26..8110bd0 100644 --- a/lib/StaticAnalyzer/Checkers/Checkers.td +++ b/lib/StaticAnalyzer/Checkers/Checkers.td @@ -84,6 +84,10 @@ def StackAddrEscapeChecker : Checker<"StackAddressEscape">, HelpText<"Check that addresses to stack memory do not escape the function">, DescFile<"StackAddrEscapeChecker.cpp">; +def DynamicTypePropagation : Checker<"DynamicTypePropagation">, + HelpText<"Generate dynamic type information">, + DescFile<"DynamicTypePropagation.cpp">; + } // end "core" let ParentPackage = CoreExperimental in { @@ -168,10 +172,6 @@ def ReturnUndefChecker : Checker<"UndefReturn">, let ParentPackage = CplusplusExperimental in { -def IteratorsChecker : Checker<"Iterators">, - HelpText<"Check improper uses of STL vector iterators">, - DescFile<"IteratorsChecker.cpp">; - def VirtualCallChecker : Checker<"VirtualCall">, HelpText<"Check virtual function calls during construction or destruction">, DescFile<"VirtualCallChecker.cpp">; @@ -283,6 +283,10 @@ def MallocPessimistic : Checker<"Malloc">, HelpText<"Check for memory leaks, double free, and use-after-free problems.">, DescFile<"MallocChecker.cpp">; +def MallocSizeofChecker : Checker<"MallocSizeof">, + HelpText<"Check for dubious malloc arguments involving sizeof">, + DescFile<"MallocSizeofChecker.cpp">; + } // end "unix" let ParentPackage = UnixExperimental in { @@ -295,10 +299,6 @@ def MallocOptimistic : Checker<"MallocWithAnnotations">, HelpText<"Check for memory leaks, double free, and use-after-free problems. Assumes that all user-defined functions which might free a pointer are annotated.">, DescFile<"MallocChecker.cpp">; -def MallocSizeofChecker : Checker<"MallocSizeof">, - HelpText<"Check for dubious malloc arguments involving sizeof">, - DescFile<"MallocSizeofChecker.cpp">; - def PthreadLockChecker : Checker<"PthreadLock">, HelpText<"Simple lock -> unlock checker">, DescFile<"PthreadLockChecker.cpp">; @@ -361,7 +361,7 @@ def MacOSKeychainAPIChecker : Checker<"SecKeychainAPI">, let ParentPackage = Cocoa in { def ObjCAtSyncChecker : Checker<"AtSync">, - HelpText<"Check for null pointers used as mutexes for @synchronized">, + HelpText<"Check for nil pointers used as mutexes for @synchronized">, DescFile<"ObjCAtSyncChecker.cpp">; def NilArgChecker : Checker<"NilArg">, @@ -373,8 +373,8 @@ def ClassReleaseChecker : Checker<"ClassRelease">, DescFile<"BasicObjCFoundationChecks.cpp">; def VariadicMethodTypeChecker : Checker<"VariadicMethodTypes">, - HelpText<"Check for passing non-Objective-C types to variadic methods that expect " - "only Objective-C types">, + HelpText<"Check for passing non-Objective-C types to variadic collection " + "initialization methods that expect only Objective-C types">, DescFile<"BasicObjCFoundationChecks.cpp">; def NSAutoreleasePoolChecker : Checker<"NSAutoreleasePool">, @@ -393,6 +393,10 @@ def ObjCSelfInitChecker : Checker<"SelfInit">, HelpText<"Check that 'self' is properly initialized inside an initializer method">, DescFile<"ObjCSelfInitChecker.cpp">; +def ObjCLoopChecker : Checker<"Loops">, + HelpText<"Improved modeling of loops using Cocoa collection types">, + DescFile<"BasicObjCFoundationChecks.cpp">; + def NSErrorChecker : Checker<"NSError">, HelpText<"Check usage of NSError** parameters">, DescFile<"NSErrorChecker.cpp">; @@ -401,7 +405,7 @@ def RetainCountChecker : Checker<"RetainCount">, HelpText<"Check for leaks and improper reference count management">, DescFile<"RetainCountChecker.cpp">; -} // end "cocoa" +} // end "osx.cocoa" let ParentPackage = CocoaExperimental in { @@ -475,6 +479,14 @@ def CallGraphDumper : Checker<"DumpCallGraph">, HelpText<"Display Call Graph">, DescFile<"DebugCheckers.cpp">; +def TraversalDumper : Checker<"DumpTraversal">, + HelpText<"Print branch conditions as they are traversed by the engine">, + DescFile<"TraversalChecker.cpp">; + +def CallDumper : Checker<"DumpCalls">, + HelpText<"Print calls as they are traversed by the engine">, + DescFile<"TraversalChecker.cpp">; + def AnalyzerStatsChecker : Checker<"Stats">, HelpText<"Emit warnings with analyzer statistics">, DescFile<"AnalyzerStatsChecker.cpp">; @@ -483,5 +495,9 @@ def TaintTesterChecker : Checker<"TaintTest">, HelpText<"Mark tainted symbols as such.">, DescFile<"TaintTesterChecker.cpp">; +def ExprInspectionChecker : Checker<"ExprInspection">, + HelpText<"Check the analyzer's understanding of expressions">, + DescFile<"ExprInspectionChecker.cpp">; + } // end "debug" diff --git a/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp b/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp index 81a2745..e98c131 100644 --- a/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp @@ -26,13 +26,18 @@ using namespace ento; namespace { class DereferenceChecker : public Checker< check::Location, - EventDispatcher<ImplicitNullDerefEvent> > { + check::Bind, + EventDispatcher<ImplicitNullDerefEvent> > { mutable OwningPtr<BuiltinBug> BT_null; mutable OwningPtr<BuiltinBug> BT_undef; + void reportBug(ProgramStateRef State, const Stmt *S, CheckerContext &C, + bool IsBind = false) const; + public: void checkLocation(SVal location, bool isLoad, const Stmt* S, CheckerContext &C) const; + void checkBind(SVal L, SVal V, const Stmt *S, CheckerContext &C) const; static const MemRegion *AddDerefSource(raw_ostream &os, SmallVectorImpl<SourceRange> &Ranges, @@ -76,6 +81,106 @@ DereferenceChecker::AddDerefSource(raw_ostream &os, return sourceR; } +void DereferenceChecker::reportBug(ProgramStateRef State, const Stmt *S, + CheckerContext &C, bool IsBind) const { + // Generate an error node. + ExplodedNode *N = C.generateSink(State); + if (!N) + return; + + // We know that 'location' cannot be non-null. This is what + // we call an "explicit" null dereference. + if (!BT_null) + BT_null.reset(new BuiltinBug("Dereference of null pointer")); + + SmallString<100> buf; + SmallVector<SourceRange, 2> Ranges; + + // Walk through lvalue casts to get the original expression + // that syntactically caused the load. + 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()) + S = BO->getRHS(); + } else if (const DeclStmt *DS = dyn_cast<DeclStmt>(S)) { + assert(DS->isSingleDecl() && "We process decls one by one"); + if (const VarDecl *VD = dyn_cast<VarDecl>(DS->getSingleDecl())) + if (const Expr *Init = VD->getAnyInitializer()) + S = Init; + } + } + + 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()); + 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); + break; + } + case Stmt::MemberExprClass: { + const MemberExpr *M = cast<MemberExpr>(S); + if (M->isArrow()) { + llvm::raw_svector_ostream os(buf); + 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); + } + 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()); + break; + } + default: + break; + } + + BugReport *report = + new BugReport(*BT_null, + buf.empty() ? BT_null->getDescription() : buf.str(), + N); + + bugreporter::addTrackNullOrUndefValueVisitor(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); +} + void DereferenceChecker::checkLocation(SVal l, bool isLoad, const Stmt* S, CheckerContext &C) const { // Check for dereference of an undefined value. @@ -86,8 +191,10 @@ void DereferenceChecker::checkLocation(SVal l, bool isLoad, const Stmt* S, BugReport *report = new BugReport(*BT_undef, BT_undef->getDescription(), N); - report->addVisitor(bugreporter::getTrackNullOrUndefValueVisitor(N, - bugreporter::GetDerefExpr(N), report)); + bugreporter::addTrackNullOrUndefValueVisitor(N, + bugreporter::GetDerefExpr(N), + report); + report->disablePathPruning(); C.EmitReport(report); } return; @@ -100,115 +207,81 @@ void DereferenceChecker::checkLocation(SVal l, bool isLoad, const Stmt* S, return; ProgramStateRef state = C.getState(); - const LocationContext *LCtx = C.getLocationContext(); + ProgramStateRef notNullState, nullState; llvm::tie(notNullState, nullState) = state->assume(location); // The explicit NULL case. if (nullState) { if (!notNullState) { - // Generate an error node. - ExplodedNode *N = C.generateSink(nullState); - if (!N) - return; - - // We know that 'location' cannot be non-null. This is what - // we call an "explicit" null dereference. - if (!BT_null) - BT_null.reset(new BuiltinBug("Dereference of null pointer")); - - SmallString<100> buf; - SmallVector<SourceRange, 2> Ranges; - - // Walk through lvalue casts to get the original expression - // that syntactically caused the load. - if (const Expr *expr = dyn_cast<Expr>(S)) - S = expr->IgnoreParenLValueCasts(); - - const MemRegion *sourceR = 0; - - 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(), LCtx); - 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(), LCtx, true); - break; - } - case Stmt::MemberExprClass: { - const MemberExpr *M = cast<MemberExpr>(S); - if (M->isArrow()) { - llvm::raw_svector_ostream os(buf); - os << "Access to field '" << M->getMemberNameInfo() - << "' results in a dereference of a null pointer"; - sourceR = - AddDerefSource(os, Ranges, M->getBase()->IgnoreParenCasts(), - state.getPtr(), LCtx, 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()); - break; - } - default: - break; - } + reportBug(nullState, S, C); + return; + } - BugReport *report = - new BugReport(*BT_null, - buf.empty() ? BT_null->getDescription():buf.str(), - N); + // Otherwise, we have the case where the location could either be + // null or not-null. Record the error node as an "implicit" null + // dereference. + if (ExplodedNode *N = C.generateSink(nullState)) { + ImplicitNullDerefEvent event = { l, isLoad, N, &C.getBugReporter() }; + dispatchEvent(event); + } + } - report->addVisitor(bugreporter::getTrackNullOrUndefValueVisitor(N, - bugreporter::GetDerefExpr(N), report)); + // From this point forward, we know that the location is not null. + C.addTransition(notNullState); +} - for (SmallVectorImpl<SourceRange>::iterator - I = Ranges.begin(), E = Ranges.end(); I!=E; ++I) - report->addRange(*I); +void DereferenceChecker::checkBind(SVal L, SVal V, const Stmt *S, + CheckerContext &C) const { + // If we're binding to a reference, check if the value is known to be null. + if (V.isUndef()) + return; - if (sourceR) { - report->markInteresting(sourceR); - report->markInteresting(state->getRawSVal(loc::MemRegionVal(sourceR))); - } + const MemRegion *MR = L.getAsRegion(); + const TypedValueRegion *TVR = dyn_cast_or_null<TypedValueRegion>(MR); + if (!TVR) + return; - C.EmitReport(report); + if (!TVR->getValueType()->isReferenceType()) + return; + + ProgramStateRef State = C.getState(); + + ProgramStateRef StNonNull, StNull; + llvm::tie(StNonNull, StNull) = State->assume(cast<DefinedOrUnknownSVal>(V)); + + if (StNull) { + if (!StNonNull) { + reportBug(StNull, S, C, /*isBind=*/true); return; } - else { - // Otherwise, we have the case where the location could either be - // null or not-null. Record the error node as an "implicit" null - // dereference. - if (ExplodedNode *N = C.generateSink(nullState)) { - ImplicitNullDerefEvent event = { l, isLoad, N, &C.getBugReporter() }; - dispatchEvent(event); - } + + // At this point the value could be either null or non-null. + // Record this as an "implicit" null dereference. + if (ExplodedNode *N = C.generateSink(StNull)) { + ImplicitNullDerefEvent event = { V, /*isLoad=*/true, N, + &C.getBugReporter() }; + dispatchEvent(event); } } - // From this point forward, we know that the location is not null. - C.addTransition(notNullState); + // Unlike a regular null dereference, initializing a reference with a + // dereferenced null pointer does not actually cause a runtime exception in + // Clang's implementation of references. + // + // int &r = *p; // safe?? + // if (p != NULL) return; // uh-oh + // r = 5; // trap here + // + // The standard says this is invalid as soon as we try to create a "null + // reference" (there is no such thing), but turning this into an assumption + // that 'p' is never null will not match our actual runtime behavior. + // So we do not record this assumption, allowing us to warn on the last line + // of this example. + // + // We do need to add a transition because we may have generated a sink for + // the "implicit" null dereference. + C.addTransition(State, this); } void ento::registerDereferenceChecker(CheckerManager &mgr) { diff --git a/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp b/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp index 2627f0c..dcf6a86 100644 --- a/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp @@ -42,8 +42,9 @@ void DivZeroChecker::reportBug(const char *Msg, BugReport *R = new BugReport(*BT, Msg, N); - R->addVisitor(bugreporter::getTrackNullOrUndefValueVisitor(N, - bugreporter::GetDenomExpr(N), R)); + bugreporter::addTrackNullOrUndefValueVisitor(N, + bugreporter::GetDenomExpr(N), + R); C.EmitReport(R); } } @@ -57,8 +58,7 @@ void DivZeroChecker::checkPreStmt(const BinaryOperator *B, Op != BO_RemAssign) return; - if (!B->getRHS()->getType()->isIntegerType() || - !B->getRHS()->getType()->isScalarType()) + if (!B->getRHS()->getType()->isScalarType()) return; SVal Denom = C.getState()->getSVal(B->getRHS(), C.getLocationContext()); diff --git a/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp b/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp new file mode 100644 index 0000000..fea5733 --- /dev/null +++ b/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp @@ -0,0 +1,179 @@ +//== DynamicTypePropagation.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 defines the rules for dynamic type gathering and propagation. +// +//===----------------------------------------------------------------------===// + +#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/PathSensitive/ProgramStateTrait.h" +#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" +#include "clang/Basic/Builtins.h" + +using namespace clang; +using namespace ento; + +namespace { +class DynamicTypePropagation: + public Checker< check::PostCall, + check::PostStmt<ImplicitCastExpr> > { + const ObjCObjectType *getObjectTypeForAllocAndNew(const ObjCMessageExpr *MsgE, + CheckerContext &C) const; + + /// \brief Return a better dynamic type if one can be derived from the cast. + const ObjCObjectPointerType *getBetterObjCType(const Expr *CastE, + CheckerContext &C) const; +public: + void checkPostCall(const CallEvent &Call, CheckerContext &C) const; + void checkPostStmt(const ImplicitCastExpr *CastE, CheckerContext &C) const; +}; +} + +void DynamicTypePropagation::checkPostCall(const CallEvent &Call, + CheckerContext &C) const { + // We can obtain perfect type info for return values from some calls. + 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(); + if (!RetReg) + return; + + ProgramStateRef State = C.getState(); + + switch (Msg->getMethodFamily()) { + default: + break; + + // We assume that the type of the object returned by alloc and new are the + // pointer to the object of the class specified in the receiver of the + // message. + case OMF_alloc: + case OMF_new: { + // Get the type of object that will get created. + const ObjCMessageExpr *MsgE = Msg->getOriginExpr(); + const ObjCObjectType *ObjTy = getObjectTypeForAllocAndNew(MsgE, C); + if (!ObjTy) + return; + QualType DynResTy = + C.getASTContext().getObjCObjectPointerType(QualType(ObjTy, 0)); + C.addTransition(State->setDynamicTypeInfo(RetReg, DynResTy, false)); + break; + } + case OMF_init: { + // Assume, the result of the init method has the same dynamic type as + // the receiver and propagate the dynamic type info. + const MemRegion *RecReg = Msg->getReceiverSVal().getAsRegion(); + if (!RecReg) + return; + DynamicTypeInfo RecDynType = State->getDynamicTypeInfo(RecReg); + C.addTransition(State->setDynamicTypeInfo(RetReg, RecDynType)); + break; + } + } + } +} + +void DynamicTypePropagation::checkPostStmt(const ImplicitCastExpr *CastE, + CheckerContext &C) const { + // We only track dynamic type info for regions. + const MemRegion *ToR = C.getSVal(CastE).getAsRegion(); + if (!ToR) + return; + + switch (CastE->getCastKind()) { + default: + break; + case CK_BitCast: + // Only handle ObjCObjects for now. + if (const Type *NewTy = getBetterObjCType(CastE, C)) + C.addTransition(C.getState()->setDynamicTypeInfo(ToR, QualType(NewTy,0))); + break; + } + return; +} + +const ObjCObjectType * +DynamicTypePropagation::getObjectTypeForAllocAndNew(const ObjCMessageExpr *MsgE, + CheckerContext &C) const { + if (MsgE->getReceiverKind() == ObjCMessageExpr::Class) { + if (const ObjCObjectType *ObjTy + = MsgE->getClassReceiver()->getAs<ObjCObjectType>()) + return ObjTy; + } + + if (MsgE->getReceiverKind() == ObjCMessageExpr::SuperClass) { + if (const ObjCObjectType *ObjTy + = MsgE->getSuperType()->getAs<ObjCObjectType>()) + return ObjTy; + } + + const Expr *RecE = MsgE->getInstanceReceiver(); + if (!RecE) + return 0; + + RecE= RecE->IgnoreParenImpCasts(); + if (const DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(RecE)) { + const StackFrameContext *SFCtx = C.getStackFrame(); + // Are we calling [self alloc]? If this is self, get the type of the + // enclosing ObjC class. + if (DRE->getDecl() == SFCtx->getSelfDecl()) { + if (const ObjCMethodDecl *MD = dyn_cast<ObjCMethodDecl>(SFCtx->getDecl())) + if (const ObjCObjectType *ObjTy = + dyn_cast<ObjCObjectType>(MD->getClassInterface()->getTypeForDecl())) + return ObjTy; + } + } + return 0; +} + +// Return a better dynamic type if one can be derived from the cast. +// Compare the current dynamic type of the region and the new type to which we +// are casting. If the new type is lower in the inheritance hierarchy, pick it. +const ObjCObjectPointerType * +DynamicTypePropagation::getBetterObjCType(const Expr *CastE, + CheckerContext &C) const { + const MemRegion *ToR = C.getSVal(CastE).getAsRegion(); + assert(ToR); + + // Get the old and new types. + const ObjCObjectPointerType *NewTy = + CastE->getType()->getAs<ObjCObjectPointerType>(); + if (!NewTy) + return 0; + QualType OldDTy = C.getState()->getDynamicTypeInfo(ToR).getType(); + if (OldDTy.isNull()) { + return NewTy; + } + const ObjCObjectPointerType *OldTy = + OldDTy->getAs<ObjCObjectPointerType>(); + if (!OldTy) + return 0; + + // Id the old type is 'id', the new one is more precise. + if (OldTy->isObjCIdType() && !NewTy->isObjCIdType()) + return NewTy; + + // Return new if it's a subclass of old. + const ObjCInterfaceDecl *ToI = NewTy->getInterfaceDecl(); + const ObjCInterfaceDecl *FromI = OldTy->getInterfaceDecl(); + if (ToI && FromI && FromI->isSuperClassOf(ToI)) + return NewTy; + + return 0; +} + +void ento::registerDynamicTypePropagation(CheckerManager &mgr) { + mgr.registerChecker<DynamicTypePropagation>(); +} diff --git a/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp b/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp new file mode 100644 index 0000000..7acf223 --- /dev/null +++ b/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp @@ -0,0 +1,122 @@ +//==- ExprInspectionChecker.cpp - Used for regression tests ------*- C++ -*-==// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "ClangSACheckers.h" +#include "clang/StaticAnalyzer/Core/Checker.h" +#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" + +using namespace clang; +using namespace ento; + +namespace { +class ExprInspectionChecker : public Checker< eval::Call > { + mutable OwningPtr<BugType> BT; + + void analyzerEval(const CallExpr *CE, CheckerContext &C) const; + void analyzerCheckInlined(const CallExpr *CE, CheckerContext &C) const; + + typedef void (ExprInspectionChecker::*FnCheck)(const CallExpr *, + CheckerContext &C) const; + +public: + bool evalCall(const CallExpr *CE, CheckerContext &C) const; +}; +} + +bool ExprInspectionChecker::evalCall(const CallExpr *CE, + CheckerContext &C) const { + // These checks should have no effect on the surrounding environment + // (globals should not be invalidated, etc), hence the use of evalCall. + FnCheck Handler = llvm::StringSwitch<FnCheck>(C.getCalleeName(CE)) + .Case("clang_analyzer_eval", &ExprInspectionChecker::analyzerEval) + .Case("clang_analyzer_checkInlined", + &ExprInspectionChecker::analyzerCheckInlined) + .Default(0); + + if (!Handler) + return false; + + (this->*Handler)(CE, C); + return true; +} + +static const char *getArgumentValueString(const CallExpr *CE, + CheckerContext &C) { + if (CE->getNumArgs() == 0) + return "Missing assertion argument"; + + ExplodedNode *N = C.getPredecessor(); + const LocationContext *LC = N->getLocationContext(); + ProgramStateRef State = N->getState(); + + const Expr *Assertion = CE->getArg(0); + SVal AssertionVal = State->getSVal(Assertion, LC); + + if (AssertionVal.isUndef()) + return "UNDEFINED"; + + ProgramStateRef StTrue, StFalse; + llvm::tie(StTrue, StFalse) = + State->assume(cast<DefinedOrUnknownSVal>(AssertionVal)); + + if (StTrue) { + if (StFalse) + return "UNKNOWN"; + else + return "TRUE"; + } else { + if (StFalse) + return "FALSE"; + else + llvm_unreachable("Invalid constraint; neither true or false."); + } +} + +void ExprInspectionChecker::analyzerEval(const CallExpr *CE, + CheckerContext &C) const { + ExplodedNode *N = C.getPredecessor(); + const LocationContext *LC = N->getLocationContext(); + + // A specific instantiation of an inlined function may have more constrained + // values than can generally be assumed. Skip the check. + if (LC->getCurrentStackFrame()->getParent() != 0) + return; + + if (!BT) + BT.reset(new BugType("Checking analyzer assumptions", "debug")); + + BugReport *R = new BugReport(*BT, getArgumentValueString(CE, C), N); + C.EmitReport(R); +} + +void ExprInspectionChecker::analyzerCheckInlined(const CallExpr *CE, + CheckerContext &C) const { + ExplodedNode *N = C.getPredecessor(); + const LocationContext *LC = N->getLocationContext(); + + // An inlined function could conceivably also be analyzed as a top-level + // function. We ignore this case and only emit a message (TRUE or FALSE) + // when we are analyzing it as an inlined function. This means that + // clang_analyzer_checkInlined(true) should always print TRUE, but + // clang_analyzer_checkInlined(false) should never actually print anything. + if (LC->getCurrentStackFrame()->getParent() == 0) + return; + + if (!BT) + BT.reset(new BugType("Checking analyzer assumptions", "debug")); + + BugReport *R = new BugReport(*BT, getArgumentValueString(CE, C), N); + C.EmitReport(R); +} + +void ento::registerExprInspectionChecker(CheckerManager &Mgr) { + Mgr.registerChecker<ExprInspectionChecker>(); +} + diff --git a/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp b/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp index 135b81d..afb862c 100644 --- a/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp @@ -273,7 +273,7 @@ GenericTaintChecker::TaintPropagationRule::getTaintPropagationRule( // Skipping the following functions, since they might be used for cleansing // or smart memory copy: - // - memccpy - copying untill hitting a special character. + // - memccpy - copying until hitting a special character. return TaintPropagationRule(); } @@ -299,6 +299,9 @@ void GenericTaintChecker::addSourcesPre(const CallExpr *CE, CheckerContext &C) const { ProgramStateRef State = 0; const FunctionDecl *FDecl = C.getCalleeDecl(CE); + if (!FDecl || FDecl->getKind() != Decl::Function) + return; + StringRef Name = C.getCalleeName(FDecl); if (Name.empty()) return; @@ -372,7 +375,11 @@ void GenericTaintChecker::addSourcesPost(const CallExpr *CE, CheckerContext &C) const { // Define the attack surface. // Set the evaluation function by switching on the callee name. - StringRef Name = C.getCalleeName(CE); + const FunctionDecl *FDecl = C.getCalleeDecl(CE); + if (!FDecl || FDecl->getKind() != Decl::Function) + return; + + StringRef Name = C.getCalleeName(FDecl); if (Name.empty()) return; FnCheck evalFunction = llvm::StringSwitch<FnCheck>(Name) @@ -406,6 +413,9 @@ bool GenericTaintChecker::checkPre(const CallExpr *CE, CheckerContext &C) const{ return true; const FunctionDecl *FDecl = C.getCalleeDecl(CE); + if (!FDecl || FDecl->getKind() != Decl::Function) + return false; + StringRef Name = C.getCalleeName(FDecl); if (Name.empty()) return false; @@ -549,7 +559,6 @@ ProgramStateRef GenericTaintChecker::postScanf(const CallExpr *CE, if (CE->getNumArgs() < 2) return State; - SVal x = State->getSVal(CE->getArg(1), C.getLocationContext()); // All arguments except for the very first one should get taint. for (unsigned int i = 1; i < CE->getNumArgs(); ++i) { // The arguments are pointer arguments. The data they are pointing at is diff --git a/lib/StaticAnalyzer/Checkers/IdempotentOperationChecker.cpp b/lib/StaticAnalyzer/Checkers/IdempotentOperationChecker.cpp index c08f163..9d0b83f 100644 --- a/lib/StaticAnalyzer/Checkers/IdempotentOperationChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/IdempotentOperationChecker.cpp @@ -106,6 +106,7 @@ private: typedef llvm::DenseMap<const BinaryOperator *, BinaryOperatorData> AssumptionMap; mutable AssumptionMap hash; + mutable OwningPtr<BugType> BT; }; } @@ -343,7 +344,9 @@ void IdempotentOperationChecker::checkPostStmt(const BinaryOperator *B, void IdempotentOperationChecker::checkEndAnalysis(ExplodedGraph &G, BugReporter &BR, ExprEngine &Eng) const { - BugType *BT = new BugType("Idempotent operation", "Dead code"); + if (!BT) + BT.reset(new BugType("Idempotent operation", "Dead code")); + // Iterate over the hash to see if we have any paths with definite // idempotent operations. for (AssumptionMap::const_iterator i = hash.begin(); i != hash.end(); ++i) { diff --git a/lib/StaticAnalyzer/Checkers/IteratorsChecker.cpp b/lib/StaticAnalyzer/Checkers/IteratorsChecker.cpp deleted file mode 100644 index b0bac33..0000000 --- a/lib/StaticAnalyzer/Checkers/IteratorsChecker.cpp +++ /dev/null @@ -1,603 +0,0 @@ -//=== IteratorsChecker.cpp - Check for Invalidated Iterators ------*- C++ -*---- -// -// The LLVM Compiler Infrastructure -// -// This file is distributed under the University of Illinois Open Source -// License. See LICENSE.TXT for details. -// -//===----------------------------------------------------------------------===// -// -// This defines IteratorsChecker, a number of small checks for conditions -// leading to invalid iterators being used. -// FIXME: Currently only supports 'vector' and 'deque' -// -//===----------------------------------------------------------------------===// - -#include "clang/AST/DeclTemplate.h" -#include "clang/Basic/SourceManager.h" -#include "ClangSACheckers.h" -#include "clang/StaticAnalyzer/Core/Checker.h" -#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" -#include "clang/StaticAnalyzer/Core/CheckerManager.h" -#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" -#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h" -#include "clang/AST/DeclCXX.h" -#include "clang/AST/ExprCXX.h" -#include "clang/AST/Type.h" -#include "clang/AST/PrettyPrinter.h" -#include "llvm/ADT/SmallPtrSet.h" -#include "llvm/ADT/StringSwitch.h" - - -using namespace clang; -using namespace ento; - -// This is the state associated with each iterator which includes both the -// kind of state and the instance used to initialize it. -// FIXME: add location where invalidated for better error reporting. -namespace { -class RefState { - enum Kind { BeginValid, EndValid, Invalid, Undefined, Unknown } K; - const void *VR; - -public: - RefState(Kind k, const void *vr) : K(k), VR(vr) {} - - bool isValid() const { return K == BeginValid || K == EndValid; } - bool isInvalid() const { return K == Invalid; } - bool isUndefined() const { return K == Undefined; } - bool isUnknown() const { return K == Unknown; } - const MemRegion *getMemRegion() const { - if (K == BeginValid || K == EndValid) - return(const MemRegion *)VR; - return 0; - } - const MemberExpr *getMemberExpr() const { - if (K == Invalid) - return(const MemberExpr *)VR; - return 0; - } - - bool operator==(const RefState &X) const { - return K == X.K && VR == X.VR; - } - - static RefState getBeginValid(const MemRegion *vr) { - assert(vr); - return RefState(BeginValid, vr); - } - static RefState getEndValid(const MemRegion *vr) { - assert(vr); - return RefState(EndValid, vr); - } - static RefState getInvalid( const MemberExpr *ME ) { - return RefState(Invalid, ME); - } - static RefState getUndefined( void ) { - return RefState(Undefined, 0); - } - static RefState getUnknown( void ) { - return RefState(Unknown, 0); - } - - void Profile(llvm::FoldingSetNodeID &ID) const { - ID.AddInteger(K); - ID.AddPointer(VR); - } -}; - -enum RefKind { NoKind, VectorKind, VectorIteratorKind }; - -class IteratorsChecker : - public Checker<check::PreStmt<CXXOperatorCallExpr>, - check::PreStmt<DeclStmt>, - check::PreStmt<CXXMemberCallExpr>, - check::PreStmt<CallExpr> > - { - // Used when parsing iterators and vectors and deques. - BuiltinBug *BT_Invalid, *BT_Undefined, *BT_Incompatible; - -public: - IteratorsChecker() : - BT_Invalid(0), BT_Undefined(0), BT_Incompatible(0) - {} - static void *getTag() { static int tag; return &tag; } - - // Checker entry points. - void checkPreStmt(const CXXOperatorCallExpr *OCE, - CheckerContext &C) const; - - void checkPreStmt(const DeclStmt *DS, - CheckerContext &C) const; - - void checkPreStmt(const CXXMemberCallExpr *MCE, - CheckerContext &C) const; - - void checkPreStmt(const CallExpr *CE, - CheckerContext &C) const; - -private: - ProgramStateRef handleAssign(ProgramStateRef state, - const Expr *lexp, - const Expr *rexp, - const LocationContext *LC) const; - - ProgramStateRef handleAssign(ProgramStateRef state, - const MemRegion *MR, - const Expr *rexp, - const LocationContext *LC) const; - - ProgramStateRef invalidateIterators(ProgramStateRef state, - const MemRegion *MR, - const MemberExpr *ME) const; - - void checkExpr(CheckerContext &C, const Expr *E) const; - - void checkArgs(CheckerContext &C, const CallExpr *CE) const; - - const MemRegion *getRegion(ProgramStateRef state, - const Expr *E, - const LocationContext *LC) const; - - const DeclRefExpr *getDeclRefExpr(const Expr *E) const; -}; - -class IteratorState { -public: - typedef llvm::ImmutableMap<const MemRegion *, RefState> EntryMap; -}; -} //end anonymous namespace - -namespace clang { - namespace ento { - template <> - struct ProgramStateTrait<IteratorState> - : public ProgramStatePartialTrait<IteratorState::EntryMap> { - static void *GDMIndex() { return IteratorsChecker::getTag(); } - }; - } -} - -void ento::registerIteratorsChecker(CheckerManager &mgr) { - mgr.registerChecker<IteratorsChecker>(); -} - -// =============================================== -// Utility functions used by visitor functions -// =============================================== - -// check a templated type for std::vector or std::deque -static RefKind getTemplateKind(const NamedDecl *td) { - const DeclContext *dc = td->getDeclContext(); - const NamespaceDecl *nameSpace = dyn_cast<NamespaceDecl>(dc); - if (!nameSpace || !isa<TranslationUnitDecl>(nameSpace->getDeclContext()) - || nameSpace->getName() != "std") - return NoKind; - - StringRef name = td->getName(); - return llvm::StringSwitch<RefKind>(name) - .Cases("vector", "deque", VectorKind) - .Default(NoKind); -} - -static RefKind getTemplateKind(const DeclContext *dc) { - if (const ClassTemplateSpecializationDecl *td = - dyn_cast<ClassTemplateSpecializationDecl>(dc)) - return getTemplateKind(cast<NamedDecl>(td)); - return NoKind; -} - -static RefKind getTemplateKind(const TypedefType *tdt) { - const TypedefNameDecl *td = tdt->getDecl(); - RefKind parentKind = getTemplateKind(td->getDeclContext()); - if (parentKind == VectorKind) { - return llvm::StringSwitch<RefKind>(td->getName()) - .Cases("iterator", - "const_iterator", - "reverse_iterator", VectorIteratorKind) - .Default(NoKind); - } - return NoKind; -} - -static RefKind getTemplateKind(const TemplateSpecializationType *tsp) { - const TemplateName &tname = tsp->getTemplateName(); - TemplateDecl *td = tname.getAsTemplateDecl(); - if (!td) - return NoKind; - return getTemplateKind(td); -} - -static RefKind getTemplateKind(QualType T) { - if (const TemplateSpecializationType *tsp = - T->getAs<TemplateSpecializationType>()) { - return getTemplateKind(tsp); - } - if (const ElaboratedType *ET = dyn_cast<ElaboratedType>(T)) { - QualType namedType = ET->getNamedType(); - if (const TypedefType *tdt = namedType->getAs<TypedefType>()) - return getTemplateKind(tdt); - if (const TemplateSpecializationType *tsp = - namedType->getAs<TemplateSpecializationType>()) { - return getTemplateKind(tsp); - } - } - return NoKind; -} - -// Iterate through our map and invalidate any iterators that were -// initialized fromt the specified instance MemRegion. -ProgramStateRef IteratorsChecker::invalidateIterators(ProgramStateRef state, - const MemRegion *MR, const MemberExpr *ME) const { - IteratorState::EntryMap Map = state->get<IteratorState>(); - if (Map.isEmpty()) - return state; - - // Loop over the entries in the current state. - // The key doesn't change, so the map iterators won't change. - for (IteratorState::EntryMap::iterator I = Map.begin(), E = Map.end(); - I != E; ++I) { - RefState RS = I.getData(); - if (RS.getMemRegion() == MR) - state = state->set<IteratorState>(I.getKey(), RefState::getInvalid(ME)); - } - - return state; -} - -// Handle assigning to an iterator where we don't have the LValue MemRegion. -ProgramStateRef IteratorsChecker::handleAssign(ProgramStateRef state, - const Expr *lexp, const Expr *rexp, const LocationContext *LC) const { - // Skip the cast if present. - if (const MaterializeTemporaryExpr *M - = dyn_cast<MaterializeTemporaryExpr>(lexp)) - lexp = M->GetTemporaryExpr(); - if (const ImplicitCastExpr *ICE = dyn_cast<ImplicitCastExpr>(lexp)) - lexp = ICE->getSubExpr(); - SVal sv = state->getSVal(lexp, LC); - const MemRegion *MR = sv.getAsRegion(); - if (!MR) - return state; - RefKind kind = getTemplateKind(lexp->getType()); - - // If assigning to a vector, invalidate any iterators currently associated. - if (kind == VectorKind) - return invalidateIterators(state, MR, 0); - - // Make sure that we are assigning to an iterator. - if (getTemplateKind(lexp->getType()) != VectorIteratorKind) - return state; - return handleAssign(state, MR, rexp, LC); -} - -// handle assigning to an iterator -ProgramStateRef IteratorsChecker::handleAssign(ProgramStateRef state, - const MemRegion *MR, const Expr *rexp, const LocationContext *LC) const { - // Assume unknown until we find something definite. - state = state->set<IteratorState>(MR, RefState::getUnknown()); - if (const MaterializeTemporaryExpr *M - = dyn_cast<MaterializeTemporaryExpr>(rexp)) - rexp = M->GetTemporaryExpr(); - if (const ImplicitCastExpr *ICE = dyn_cast<ImplicitCastExpr>(rexp)) - rexp = ICE->getSubExpr(); - // Need to handle three cases: MemberCall, copy, copy with addition. - if (const CallExpr *CE = dyn_cast<CallExpr>(rexp)) { - // Handle MemberCall. - if (const MemberExpr *ME = dyn_cast<MemberExpr>(CE->getCallee())) { - const DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(ME->getBase()); - if (!DRE) - return state; - // Verify that the type is std::vector<T>. - if (getTemplateKind(DRE->getType()) != VectorKind) - return state; - // Now get the MemRegion associated with the instance. - const VarDecl *VD = dyn_cast<VarDecl>(DRE->getDecl()); - if (!VD) - return state; - const MemRegion *IMR = state->getRegion(VD, LC); - if (!IMR) - return state; - // Finally, see if it is one of the calls that will create - // a valid iterator and mark it if so, else mark as Unknown. - StringRef mName = ME->getMemberDecl()->getName(); - - if (llvm::StringSwitch<bool>(mName) - .Cases("begin", "insert", "erase", true).Default(false)) { - return state->set<IteratorState>(MR, RefState::getBeginValid(IMR)); - } - if (mName == "end") - return state->set<IteratorState>(MR, RefState::getEndValid(IMR)); - - return state->set<IteratorState>(MR, RefState::getUnknown()); - } - } - // Handle straight copy from another iterator. - if (const DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(rexp)) { - if (getTemplateKind(DRE->getType()) != VectorIteratorKind) - return state; - // Now get the MemRegion associated with the instance. - const VarDecl *VD = dyn_cast<VarDecl>(DRE->getDecl()); - if (!VD) - return state; - const MemRegion *IMR = state->getRegion(VD, LC); - if (!IMR) - return state; - // Get the RefState of the iterator being copied. - const RefState *RS = state->get<IteratorState>(IMR); - if (!RS) - return state; - // Use it to set the state of the LValue. - return state->set<IteratorState>(MR, *RS); - } - // If we have operator+ or operator- ... - if (const CXXOperatorCallExpr *OCE = dyn_cast<CXXOperatorCallExpr>(rexp)) { - OverloadedOperatorKind Kind = OCE->getOperator(); - if (Kind == OO_Plus || Kind == OO_Minus) { - // Check left side of tree for a valid value. - state = handleAssign( state, MR, OCE->getArg(0), LC); - const RefState *RS = state->get<IteratorState>(MR); - // If found, return it. - if (!RS->isUnknown()) - return state; - // Otherwise return what we find in the right side. - return handleAssign(state, MR, OCE->getArg(1), LC); - } - } - // Fall through if nothing matched. - return state; -} - -// Iterate through the arguments looking for an Invalid or Undefined iterator. -void IteratorsChecker::checkArgs(CheckerContext &C, const CallExpr *CE) const { - for (CallExpr::const_arg_iterator I = CE->arg_begin(), E = CE->arg_end(); - I != E; ++I) { - checkExpr(C, *I); - } -} - -// Get the DeclRefExpr associated with the expression. -const DeclRefExpr *IteratorsChecker::getDeclRefExpr(const Expr *E) const { - // If it is a CXXConstructExpr, need to get the subexpression. - if (const CXXConstructExpr *CE = dyn_cast<CXXConstructExpr>(E)) { - if (CE->getNumArgs()== 1) { - CXXConstructorDecl *CD = CE->getConstructor(); - if (CD->isTrivial()) - E = CE->getArg(0); - } - } - if (const MaterializeTemporaryExpr *M = dyn_cast<MaterializeTemporaryExpr>(E)) - E = M->GetTemporaryExpr(); - if (const ImplicitCastExpr *ICE = dyn_cast<ImplicitCastExpr>(E)) - E = ICE->getSubExpr(); - // If it isn't one of our types, don't do anything. - if (getTemplateKind(E->getType()) != VectorIteratorKind) - return NULL; - return dyn_cast<DeclRefExpr>(E); -} - -// Get the MemRegion associated with the expresssion. -const MemRegion *IteratorsChecker::getRegion(ProgramStateRef state, - const Expr *E, const LocationContext *LC) const { - const DeclRefExpr *DRE = getDeclRefExpr(E); - if (!DRE) - return NULL; - const VarDecl *VD = dyn_cast<VarDecl>(DRE->getDecl()); - if (!VD) - return NULL; - // return the MemRegion associated with the iterator - return state->getRegion(VD, LC); -} - -// Check the expression and if it is an iterator, generate a diagnostic -// if the iterator is not valid. -// FIXME: this method can generate new nodes, and subsequent logic should -// use those nodes. We also cannot create multiple nodes at one ProgramPoint -// with the same tag. -void IteratorsChecker::checkExpr(CheckerContext &C, const Expr *E) const { - ProgramStateRef state = C.getState(); - const MemRegion *MR = getRegion(state, E, C.getLocationContext()); - if (!MR) - return; - - // Get the state associated with the iterator. - const RefState *RS = state->get<IteratorState>(MR); - if (!RS) - return; - if (RS->isInvalid()) { - if (ExplodedNode *N = C.addTransition()) { - if (!BT_Invalid) - // FIXME: We are eluding constness here. - const_cast<IteratorsChecker*>(this)->BT_Invalid = new BuiltinBug(""); - - std::string msg; - const MemberExpr *ME = RS->getMemberExpr(); - if (ME) { - std::string name = ME->getMemberNameInfo().getAsString(); - msg = "Attempt to use an iterator made invalid by call to '" + - name + "'"; - } - else { - msg = "Attempt to use an iterator made invalid by copying another " - "container to its container"; - } - - BugReport *R = new BugReport(*BT_Invalid, msg, N); - R->addRange(getDeclRefExpr(E)->getSourceRange()); - C.EmitReport(R); - } - } - else if (RS->isUndefined()) { - if (ExplodedNode *N = C.addTransition()) { - if (!BT_Undefined) - // FIXME: We are eluding constness here. - const_cast<IteratorsChecker*>(this)->BT_Undefined = - new BuiltinBug("Use of iterator that is not defined"); - - BugReport *R = new BugReport(*BT_Undefined, - BT_Undefined->getDescription(), N); - R->addRange(getDeclRefExpr(E)->getSourceRange()); - C.EmitReport(R); - } - } -} - -// =============================================== -// Path analysis visitor functions -// =============================================== - -// For a generic Call, just check the args for bad iterators. -void IteratorsChecker::checkPreStmt(const CallExpr *CE, - CheckerContext &C) const{ - - // FIXME: These checks are to currently work around a bug - // in CheckerManager. - if (isa<CXXOperatorCallExpr>(CE)) - return; - if (isa<CXXMemberCallExpr>(CE)) - return; - - checkArgs(C, CE); -} - -// Handle operator calls. First, if it is operator=, check the argument, -// and handle assigning and set target state appropriately. Otherwise, for -// other operators, check the args for bad iterators and handle comparisons. -void IteratorsChecker::checkPreStmt(const CXXOperatorCallExpr *OCE, - CheckerContext &C) const -{ - const LocationContext *LC = C.getLocationContext(); - ProgramStateRef state = C.getState(); - OverloadedOperatorKind Kind = OCE->getOperator(); - if (Kind == OO_Equal) { - checkExpr(C, OCE->getArg(1)); - state = handleAssign(state, OCE->getArg(0), OCE->getArg(1), LC); - C.addTransition(state); - return; - } - else { - checkArgs(C, OCE); - // If it is a compare and both are iterators, ensure that they are for - // the same container. - if (Kind == OO_EqualEqual || Kind == OO_ExclaimEqual || - Kind == OO_Less || Kind == OO_LessEqual || - Kind == OO_Greater || Kind == OO_GreaterEqual) { - const MemRegion *MR0, *MR1; - MR0 = getRegion(state, OCE->getArg(0), LC); - if (!MR0) - return; - MR1 = getRegion(state, OCE->getArg(1), LC); - if (!MR1) - return; - const RefState *RS0, *RS1; - RS0 = state->get<IteratorState>(MR0); - if (!RS0) - return; - RS1 = state->get<IteratorState>(MR1); - if (!RS1) - return; - if (RS0->getMemRegion() != RS1->getMemRegion()) { - if (ExplodedNode *N = C.addTransition()) { - if (!BT_Incompatible) - const_cast<IteratorsChecker*>(this)->BT_Incompatible = - new BuiltinBug( - "Cannot compare iterators from different containers"); - - BugReport *R = new BugReport(*BT_Incompatible, - BT_Incompatible->getDescription(), N); - R->addRange(OCE->getSourceRange()); - C.EmitReport(R); - } - } - } - } -} - -// Need to handle DeclStmts to pick up initializing of iterators and to mark -// uninitialized ones as Undefined. -void IteratorsChecker::checkPreStmt(const DeclStmt *DS, - CheckerContext &C) const { - const Decl *D = *DS->decl_begin(); - const VarDecl *VD = dyn_cast<VarDecl>(D); - // Only care about iterators. - if (getTemplateKind(VD->getType()) != VectorIteratorKind) - return; - - // Get the MemRegion associated with the iterator and mark it as Undefined. - ProgramStateRef state = C.getState(); - Loc VarLoc = state->getLValue(VD, C.getLocationContext()); - const MemRegion *MR = VarLoc.getAsRegion(); - if (!MR) - return; - state = state->set<IteratorState>(MR, RefState::getUndefined()); - - // if there is an initializer, handle marking Valid if a proper initializer - const Expr *InitEx = VD->getInit(); - if (InitEx) { - // FIXME: This is too syntactic. Since 'InitEx' will be analyzed first - // it should resolve to an SVal that we can check for validity - // *semantically* instead of walking through the AST. - if (const CXXConstructExpr *CE = dyn_cast<CXXConstructExpr>(InitEx)) { - if (CE->getNumArgs() == 1) { - const Expr *E = CE->getArg(0); - if (const MaterializeTemporaryExpr *M - = dyn_cast<MaterializeTemporaryExpr>(E)) - E = M->GetTemporaryExpr(); - if (const ImplicitCastExpr *ICE = dyn_cast<ImplicitCastExpr>(E)) - InitEx = ICE->getSubExpr(); - state = handleAssign(state, MR, InitEx, C.getLocationContext()); - } - } - } - C.addTransition(state); -} - - -namespace { struct CalledReserved {}; } -namespace clang { namespace ento { -template<> struct ProgramStateTrait<CalledReserved> - : public ProgramStatePartialTrait<llvm::ImmutableSet<const MemRegion*> > { - static void *GDMIndex() { static int index = 0; return &index; } -}; -}} - -// on a member call, first check the args for any bad iterators -// then, check to see if it is a call to a function that will invalidate -// the iterators -void IteratorsChecker::checkPreStmt(const CXXMemberCallExpr *MCE, - CheckerContext &C) const { - // Check the arguments. - checkArgs(C, MCE); - const MemberExpr *ME = dyn_cast<MemberExpr>(MCE->getCallee()); - if (!ME) - return; - // Make sure we have the right kind of container. - const DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(ME->getBase()); - if (!DRE || getTemplateKind(DRE->getType()) != VectorKind) - return; - SVal tsv = C.getState()->getSVal(DRE, C.getLocationContext()); - // Get the MemRegion associated with the container instance. - const MemRegion *MR = tsv.getAsRegion(); - if (!MR) - return; - // If we are calling a function that invalidates iterators, mark them - // appropriately by finding matching instances. - ProgramStateRef state = C.getState(); - StringRef mName = ME->getMemberDecl()->getName(); - if (llvm::StringSwitch<bool>(mName) - .Cases("insert", "reserve", "push_back", true) - .Cases("erase", "pop_back", "clear", "resize", true) - .Default(false)) { - // If there was a 'reserve' call, assume iterators are good. - if (!state->contains<CalledReserved>(MR)) - state = invalidateIterators(state, MR, ME); - } - // Keep track of instances that have called 'reserve' - // note: do this after we invalidate any iterators by calling - // 'reserve' itself. - if (mName == "reserve") - state = state->add<CalledReserved>(MR); - - if (state != C.getState()) - C.addTransition(state); -} - diff --git a/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp b/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp index cb976e0..969f2dd 100644 --- a/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp @@ -290,7 +290,11 @@ void MacOSKeychainAPIChecker::checkPreStmt(const CallExpr *CE, unsigned idx = InvalidIdx; ProgramStateRef State = C.getState(); - StringRef funName = C.getCalleeName(CE); + const FunctionDecl *FD = C.getCalleeDecl(CE); + if (!FD || FD->getKind() != Decl::Function) + return; + + StringRef funName = C.getCalleeName(FD); if (funName.empty()) return; @@ -446,7 +450,11 @@ void MacOSKeychainAPIChecker::checkPreStmt(const CallExpr *CE, void MacOSKeychainAPIChecker::checkPostStmt(const CallExpr *CE, CheckerContext &C) const { ProgramStateRef State = C.getState(); - StringRef funName = C.getCalleeName(CE); + const FunctionDecl *FD = C.getCalleeDecl(CE); + if (!FD || FD->getKind() != Decl::Function) + return; + + StringRef funName = C.getCalleeName(FD); // If a value has been allocated, add it to the set for tracking. unsigned idx = getTrackedFunctionIndex(funName, true); @@ -528,9 +536,11 @@ MacOSKeychainAPIChecker::getAllocationSite(const ExplodedNode *N, } ProgramPoint P = AllocNode->getLocation(); - if (!isa<StmtPoint>(P)) - return 0; - return cast<clang::PostStmt>(P).getStmt(); + if (CallExitEnd *Exit = dyn_cast<CallExitEnd>(&P)) + return Exit->getCalleeContext()->getCallSite(); + if (clang::PostStmt *PS = dyn_cast<clang::PostStmt>(&P)) + return PS->getStmt(); + return 0; } BugReport *MacOSKeychainAPIChecker:: diff --git a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp index 8bce88a..dfcedf6 100644 --- a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -18,7 +18,7 @@ #include "clang/StaticAnalyzer/Core/CheckerManager.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" -#include "clang/StaticAnalyzer/Core/PathSensitive/ObjCMessage.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h" #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h" #include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h" @@ -34,15 +34,21 @@ using namespace ento; namespace { class RefState { - enum Kind { AllocateUnchecked, AllocateFailed, Released, Escaped, + enum Kind { // Reference to allocated memory. + Allocated, + // Reference to released/freed memory. + Released, + // The responsibility for freeing resources has transfered from + // this reference. A relinquished symbol should not be freed. Relinquished } K; const Stmt *S; public: RefState(Kind k, const Stmt *s) : K(k), S(s) {} - bool isAllocated() const { return K == AllocateUnchecked; } + bool isAllocated() const { return K == Allocated; } bool isReleased() const { return K == Released; } + bool isRelinquished() const { return K == Relinquished; } const Stmt *getStmt() const { return S; } @@ -50,14 +56,10 @@ public: return K == X.K && S == X.S; } - static RefState getAllocateUnchecked(const Stmt *s) { - return RefState(AllocateUnchecked, s); - } - static RefState getAllocateFailed() { - return RefState(AllocateFailed, 0); + static RefState getAllocated(const Stmt *s) { + return RefState(Allocated, s); } static RefState getReleased(const Stmt *s) { return RefState(Released, s); } - static RefState getEscaped(const Stmt *s) { return RefState(Escaped, s); } static RefState getRelinquished(const Stmt *s) { return RefState(Relinquished, s); } @@ -90,6 +92,7 @@ class MallocChecker : public Checker<check::DeadSymbols, check::PreStmt<CallExpr>, check::PostStmt<CallExpr>, check::PostStmt<BlockExpr>, + check::PreObjCMessage, check::Location, check::Bind, eval::Assume, @@ -117,6 +120,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 checkPostStmt(const BlockExpr *BE, CheckerContext &C) const; void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const; void checkEndPath(CheckerContext &C) const; @@ -132,17 +136,22 @@ public: const StoreManager::InvalidatedSymbols *invalidated, ArrayRef<const MemRegion *> ExplicitRegions, ArrayRef<const MemRegion *> Regions, - const CallOrObjCMessage *Call) const; + const CallEvent *Call) const; bool wantsRegionChangeUpdate(ProgramStateRef state) const { return true; } + void printState(raw_ostream &Out, ProgramStateRef State, + const char *NL, const char *Sep) const; + private: void initIdentifierInfo(ASTContext &C) const; /// Check if this is one of the functions which can allocate/reallocate memory /// pointed to by one of its arguments. bool isMemFunction(const FunctionDecl *FD, ASTContext &C) const; + bool isFreeFunction(const FunctionDecl *FD, ASTContext &C) const; + bool isAllocationFunction(const FunctionDecl *FD, ASTContext &C) const; static ProgramStateRef MallocMemReturnsAttr(CheckerContext &C, const CallExpr *CE, @@ -167,20 +176,26 @@ private: ProgramStateRef FreeMemAttr(CheckerContext &C, const CallExpr *CE, const OwnershipAttr* Att) const; ProgramStateRef FreeMemAux(CheckerContext &C, const CallExpr *CE, - ProgramStateRef state, unsigned Num, - bool Hold) const; + ProgramStateRef state, unsigned Num, + bool Hold) const; + ProgramStateRef FreeMemAux(CheckerContext &C, const Expr *Arg, + const Expr *ParentExpr, + ProgramStateRef state, + bool Hold) const; ProgramStateRef ReallocMem(CheckerContext &C, const CallExpr *CE, bool FreesMemOnFailure) const; static ProgramStateRef CallocMem(CheckerContext &C, const CallExpr *CE); - bool checkEscape(SymbolRef Sym, const Stmt *S, CheckerContext &C) const; + ///\brief Check if the memory associated with this symbol was released. + bool isReleased(SymbolRef Sym, CheckerContext &C) const; + bool checkUseAfterFree(SymbolRef Sym, CheckerContext &C, const Stmt *S = 0) const; /// Check if the function is not known to us. So, for example, we could /// conservatively assume it can free/reallocate it's pointer arguments. - bool doesNotFreeMemory(const CallOrObjCMessage *Call, + bool doesNotFreeMemory(const CallEvent *Call, ProgramStateRef State) const; static bool SummarizeValue(raw_ostream &os, SVal V); @@ -207,15 +222,17 @@ private: // The allocated region symbol tracked by the main analysis. SymbolRef Sym; - // The mode we are in, i.e. what kind of diagnostics will be emitted. - NotificationMode Mode; + // The mode we are in, i.e. what kind of diagnostics will be emitted. + NotificationMode Mode; - // A symbol from when the primary region should have been reallocated. - SymbolRef FailedReallocSymbol; + // A symbol from when the primary region should have been reallocated. + SymbolRef FailedReallocSymbol; - public: - MallocBugVisitor(SymbolRef S) - : Sym(S), Mode(Normal), FailedReallocSymbol(0) {} + bool IsLeak; + + public: + MallocBugVisitor(SymbolRef S, bool isLeak = false) + : Sym(S), Mode(Normal), FailedReallocSymbol(0), IsLeak(isLeak) {} virtual ~MallocBugVisitor() {} @@ -239,6 +256,15 @@ private: (S && S->isReleased()) && (!SPrev || !SPrev->isReleased())); } + inline bool isRelinquished(const RefState *S, const RefState *SPrev, + const Stmt *Stmt) { + // Did not track -> relinquished. Other state (allocated) -> relinquished. + return (Stmt && (isa<CallExpr>(Stmt) || isa<ObjCMessageExpr>(Stmt) || + isa<ObjCPropertyRefExpr>(Stmt)) && + (S && S->isRelinquished()) && + (!SPrev || !SPrev->isRelinquished())); + } + inline bool isReallocFailedCheck(const RefState *S, const RefState *SPrev, const Stmt *Stmt) { // If the expression is not a call, and the state change is @@ -253,6 +279,20 @@ private: const ExplodedNode *PrevN, BugReporterContext &BRC, BugReport &BR); + + PathDiagnosticPiece* getEndPath(BugReporterContext &BRC, + const ExplodedNode *EndPathNode, + BugReport &BR) { + if (!IsLeak) + return 0; + + PathDiagnosticLocation L = + PathDiagnosticLocation::createEndOfPath(EndPathNode, + BRC.getSourceManager()); + // Do not add the statement itself as a range in case of leak. + return new PathDiagnosticEventPiece(L, BR.getDescription(), false); + } + private: class StackHintGeneratorForReallocationFailed : public StackHintGeneratorForSymbol { @@ -315,44 +355,73 @@ public: } // end anonymous namespace void MallocChecker::initIdentifierInfo(ASTContext &Ctx) const { - if (!II_malloc) - II_malloc = &Ctx.Idents.get("malloc"); - if (!II_free) - II_free = &Ctx.Idents.get("free"); - if (!II_realloc) - II_realloc = &Ctx.Idents.get("realloc"); - if (!II_reallocf) - II_reallocf = &Ctx.Idents.get("reallocf"); - if (!II_calloc) - II_calloc = &Ctx.Idents.get("calloc"); - if (!II_valloc) - II_valloc = &Ctx.Idents.get("valloc"); - if (!II_strdup) - II_strdup = &Ctx.Idents.get("strdup"); - if (!II_strndup) - II_strndup = &Ctx.Idents.get("strndup"); + if (II_malloc) + return; + II_malloc = &Ctx.Idents.get("malloc"); + II_free = &Ctx.Idents.get("free"); + II_realloc = &Ctx.Idents.get("realloc"); + II_reallocf = &Ctx.Idents.get("reallocf"); + II_calloc = &Ctx.Idents.get("calloc"); + II_valloc = &Ctx.Idents.get("valloc"); + II_strdup = &Ctx.Idents.get("strdup"); + II_strndup = &Ctx.Idents.get("strndup"); } bool MallocChecker::isMemFunction(const FunctionDecl *FD, ASTContext &C) const { + if (isFreeFunction(FD, C)) + return true; + + if (isAllocationFunction(FD, C)) + return true; + + return false; +} + +bool MallocChecker::isAllocationFunction(const FunctionDecl *FD, + ASTContext &C) const { if (!FD) return false; - IdentifierInfo *FunI = FD->getIdentifier(); - if (!FunI) - return false; - initIdentifierInfo(C); + if (FD->getKind() == Decl::Function) { + IdentifierInfo *FunI = FD->getIdentifier(); + initIdentifierInfo(C); - if (FunI == II_malloc || FunI == II_free || FunI == II_realloc || - FunI == II_reallocf || FunI == II_calloc || FunI == II_valloc || - FunI == II_strdup || FunI == II_strndup) - return true; + if (FunI == II_malloc || FunI == II_realloc || + FunI == II_reallocf || FunI == II_calloc || FunI == II_valloc || + FunI == II_strdup || FunI == II_strndup) + return true; + } - if (Filter.CMallocOptimistic && FD->hasAttrs() && - FD->specific_attr_begin<OwnershipAttr>() != - FD->specific_attr_end<OwnershipAttr>()) - return true; + if (Filter.CMallocOptimistic && FD->hasAttrs()) + for (specific_attr_iterator<OwnershipAttr> + i = FD->specific_attr_begin<OwnershipAttr>(), + e = FD->specific_attr_end<OwnershipAttr>(); + i != e; ++i) + if ((*i)->getOwnKind() == OwnershipAttr::Returns) + return true; + return false; +} + +bool MallocChecker::isFreeFunction(const FunctionDecl *FD, ASTContext &C) const { + if (!FD) + return false; + + if (FD->getKind() == Decl::Function) { + IdentifierInfo *FunI = FD->getIdentifier(); + initIdentifierInfo(C); + if (FunI == II_free || FunI == II_realloc || FunI == II_reallocf) + return true; + } + if (Filter.CMallocOptimistic && FD->hasAttrs()) + for (specific_attr_iterator<OwnershipAttr> + i = FD->specific_attr_begin<OwnershipAttr>(), + e = FD->specific_attr_end<OwnershipAttr>(); + i != e; ++i) + if ((*i)->getOwnKind() == OwnershipAttr::Takes || + (*i)->getOwnKind() == OwnershipAttr::Holds) + return true; return false; } @@ -361,29 +430,32 @@ void MallocChecker::checkPostStmt(const CallExpr *CE, CheckerContext &C) const { if (!FD) return; - initIdentifierInfo(C.getASTContext()); - IdentifierInfo *FunI = FD->getIdentifier(); - if (!FunI) - return; - ProgramStateRef State = C.getState(); - if (FunI == II_malloc || FunI == II_valloc) { - if (CE->getNumArgs() < 1) - return; - State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State); - } else if (FunI == II_realloc) { - State = ReallocMem(C, CE, false); - } else if (FunI == II_reallocf) { - State = ReallocMem(C, CE, true); - } else if (FunI == II_calloc) { - State = CallocMem(C, CE); - } else if (FunI == II_free) { - State = FreeMemAux(C, CE, C.getState(), 0, false); - } else if (FunI == II_strdup) { - State = MallocUpdateRefState(C, CE, State); - } else if (FunI == II_strndup) { - State = MallocUpdateRefState(C, CE, State); - } else if (Filter.CMallocOptimistic) { + + if (FD->getKind() == Decl::Function) { + initIdentifierInfo(C.getASTContext()); + IdentifierInfo *FunI = FD->getIdentifier(); + + if (FunI == II_malloc || FunI == II_valloc) { + if (CE->getNumArgs() < 1) + return; + State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State); + } else if (FunI == II_realloc) { + State = ReallocMem(C, CE, false); + } else if (FunI == II_reallocf) { + State = ReallocMem(C, CE, true); + } else if (FunI == II_calloc) { + State = CallocMem(C, CE); + } else if (FunI == II_free) { + State = FreeMemAux(C, CE, State, 0, false); + } else if (FunI == II_strdup) { + State = MallocUpdateRefState(C, CE, State); + } else if (FunI == II_strndup) { + State = MallocUpdateRefState(C, CE, State); + } + } + + if (Filter.CMallocOptimistic) { // Check all the attributes, if there are any. // There can be multiple of these attributes. if (FD->hasAttrs()) @@ -405,6 +477,34 @@ void MallocChecker::checkPostStmt(const CallExpr *CE, CheckerContext &C) const { C.addTransition(State); } +static bool isFreeWhenDoneSetToZero(const ObjCMethodCall &Call) { + Selector S = Call.getSelector(); + for (unsigned i = 1; i < S.getNumArgs(); ++i) + if (S.getNameForSlot(i).equals("freeWhenDone")) + if (Call.getArgSVal(i).isConstant(0)) + return true; + + return false; +} + +void MallocChecker::checkPreObjCMessage(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. + 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 MallocChecker::MallocMemReturnsAttr(CheckerContext &C, const CallExpr *CE, const OwnershipAttr* Att) { @@ -422,19 +522,27 @@ ProgramStateRef MallocChecker::MallocMemAux(CheckerContext &C, const CallExpr *CE, SVal Size, SVal Init, ProgramStateRef state) { - // Get the return value. - SVal retVal = state->getSVal(CE, C.getLocationContext()); + + // 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(); + SValBuilder &svalBuilder = C.getSValBuilder(); + const LocationContext *LCtx = C.getPredecessor()->getLocationContext(); + DefinedSVal RetVal = + cast<DefinedSVal>(svalBuilder.getConjuredHeapSymbolVal(CE, LCtx, Count)); + state = state->BindExpr(CE, C.getLocationContext(), RetVal); // We expect the malloc functions to return a pointer. - if (!isa<Loc>(retVal)) + if (!isa<Loc>(RetVal)) return 0; // Fill the region with the initialization value. - state = state->bindDefault(retVal, Init); + state = state->bindDefault(RetVal, Init); // Set the region's extent equal to the Size parameter. const SymbolicRegion *R = - dyn_cast_or_null<SymbolicRegion>(retVal.getAsRegion()); + dyn_cast_or_null<SymbolicRegion>(RetVal.getAsRegion()); if (!R) return 0; if (isa<DefinedOrUnknownSVal>(Size)) { @@ -465,7 +573,7 @@ ProgramStateRef MallocChecker::MallocUpdateRefState(CheckerContext &C, assert(Sym); // Set the symbol's state to Allocated. - return state->set<RegionState>(Sym, RefState::getAllocateUnchecked(CE)); + return state->set<RegionState>(Sym, RefState::getAllocated(CE)); } @@ -495,7 +603,15 @@ ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C, if (CE->getNumArgs() < (Num + 1)) return 0; - const Expr *ArgExpr = CE->getArg(Num); + return FreeMemAux(C, CE->getArg(Num), CE, state, Hold); +} + +ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C, + const Expr *ArgExpr, + const Expr *ParentExpr, + ProgramStateRef state, + bool Hold) const { + SVal ArgVal = state->getSVal(ArgExpr, C.getLocationContext()); if (!isa<DefinedOrUnknownSVal>(ArgVal)) return 0; @@ -558,20 +674,15 @@ ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C, SymbolRef Sym = SR->getSymbol(); const RefState *RS = state->get<RegionState>(Sym); - // If the symbol has not been tracked, return. This is possible when free() is - // called on a pointer that does not get its pointee directly from malloc(). - // Full support of this requires inter-procedural analysis. - if (!RS) - return 0; - // Check double free. - if (RS->isReleased()) { + if (RS && (RS->isReleased() || RS->isRelinquished())) { if (ExplodedNode *N = C.generateSink()) { if (!BT_DoubleFree) BT_DoubleFree.reset( new BugType("Double free", "Memory Error")); BugReport *R = new BugReport(*BT_DoubleFree, - "Attempt to free released memory", N); + (RS->isReleased() ? "Attempt to free released memory" : + "Attempt to free non-owned memory"), N); R->addRange(ArgExpr->getSourceRange()); R->markInteresting(Sym); R->addVisitor(new MallocBugVisitor(Sym)); @@ -582,8 +693,8 @@ ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C, // Normal free. if (Hold) - return state->set<RegionState>(Sym, RefState::getRelinquished(CE)); - return state->set<RegionState>(Sym, RefState::getReleased(CE)); + return state->set<RegionState>(Sym, RefState::getRelinquished(ParentExpr)); + return state->set<RegionState>(Sym, RefState::getReleased(ParentExpr)); } bool MallocChecker::SummarizeValue(raw_ostream &os, SVal V) { @@ -780,10 +891,8 @@ ProgramStateRef MallocChecker::ReallocMem(CheckerContext &C, if (ProgramStateRef stateFree = FreeMemAux(C, CE, StateSizeIsZero,0,false)){ // 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. - stateFree = stateFree->set<ReallocPairs>(ToPtr, - ReallocPair(FromPtr, FreesOnFail)); - C.getSymbolManager().addSymbolDependency(ToPtr, FromPtr); + // to free() is returned. We just free the input pointer and do not add + // any constrains on the output pointer. return stateFree; } @@ -851,8 +960,10 @@ MallocChecker::getAllocationSite(const ExplodedNode *N, SymbolRef Sym, ProgramPoint P = AllocNode->getLocation(); const Stmt *AllocationStmt = 0; - if (isa<StmtPoint>(P)) - AllocationStmt = cast<StmtPoint>(P).getStmt(); + if (CallExitEnd *Exit = dyn_cast<CallExitEnd>(&P)) + AllocationStmt = Exit->getCalleeContext()->getCallSite(); + else if (StmtPoint *SP = dyn_cast<StmtPoint>(&P)) + AllocationStmt = SP->getStmt(); return LeakInfo(AllocationStmt, ReferenceRegion); } @@ -884,15 +995,15 @@ void MallocChecker::reportLeak(SymbolRef Sym, ExplodedNode *N, SmallString<200> buf; llvm::raw_svector_ostream os(buf); os << "Memory is never released; potential leak"; - if (Region) { + if (Region && Region->canPrintPretty()) { os << " of memory pointed to by '"; - Region->dumpPretty(os); - os <<'\''; + Region->printPretty(os); + os << '\''; } BugReport *R = new BugReport(*BT_Leak, os.str(), N, LocUsedForUniqueing); R->markInteresting(Sym); - R->addVisitor(new MallocBugVisitor(Sym)); + R->addVisitor(new MallocBugVisitor(Sym, true)); C.EmitReport(R); } @@ -960,23 +1071,9 @@ void MallocChecker::checkEndPath(CheckerContext &C) const { } } -bool MallocChecker::checkEscape(SymbolRef Sym, const Stmt *S, - CheckerContext &C) const { - ProgramStateRef state = C.getState(); - const RefState *RS = state->get<RegionState>(Sym); - if (!RS) - return false; - - if (RS->isAllocated()) { - state = state->set<RegionState>(Sym, RefState::getEscaped(S)); - C.addTransition(state); - return true; - } - return false; -} - void MallocChecker::checkPreStmt(const CallExpr *CE, CheckerContext &C) const { - if (isMemFunction(C.getCalleeDecl(CE), C.getASTContext())) + // We will check for double free in the post visit. + if (isFreeFunction(C.getCalleeDecl(CE), C.getASTContext())) return; // Check use after free, when a freed pointer is passed to a call. @@ -1000,7 +1097,8 @@ void MallocChecker::checkPreStmt(const ReturnStmt *S, CheckerContext &C) const { return; // Check if we are returning a symbol. - SVal RetVal = C.getState()->getSVal(E, C.getLocationContext()); + ProgramStateRef State = C.getState(); + SVal RetVal = State->getSVal(E, C.getLocationContext()); SymbolRef Sym = RetVal.getAsSymbol(); if (!Sym) // If we are returning a field of the allocated struct or an array element, @@ -1011,16 +1109,18 @@ void MallocChecker::checkPreStmt(const ReturnStmt *S, CheckerContext &C) const { if (const SymbolicRegion *BMR = dyn_cast<SymbolicRegion>(MR->getBaseRegion())) Sym = BMR->getSymbol(); - if (!Sym) - return; // Check if we are returning freed memory. - if (checkUseAfterFree(Sym, C, E)) - return; + if (Sym) + if (checkUseAfterFree(Sym, C, E)) + return; - // If this function body is not inlined, check if the symbol is escaping. - if (C.getLocationContext()->getParent() == 0) - checkEscape(Sym, E, C); + // If this function body is not inlined, stop tracking any returned symbols. + if (C.getLocationContext()->getParent() == 0) { + State = + State->scanReachableSymbols<StopTrackingCallback>(RetVal).getState(); + C.addTransition(State); + } } // TODO: Blocks should be either inlined or should call invalidate regions @@ -1063,11 +1163,15 @@ void MallocChecker::checkPostStmt(const BlockExpr *BE, C.addTransition(state); } -bool MallocChecker::checkUseAfterFree(SymbolRef Sym, CheckerContext &C, - const Stmt *S) const { +bool MallocChecker::isReleased(SymbolRef Sym, CheckerContext &C) const { assert(Sym); const RefState *RS = C.getState()->get<RegionState>(Sym); - if (RS && RS->isReleased()) { + return (RS && RS->isReleased()); +} + +bool MallocChecker::checkUseAfterFree(SymbolRef Sym, CheckerContext &C, + const Stmt *S) const { + if (isReleased(Sym, C)) { if (ExplodedNode *N = C.generateSink()) { if (!BT_UseFree) BT_UseFree.reset(new BugType("Use-after-free", "Memory Error")); @@ -1090,7 +1194,7 @@ void MallocChecker::checkLocation(SVal l, bool isLoad, const Stmt *S, CheckerContext &C) const { SymbolRef Sym = l.getLocSymbolInBase(); if (Sym) - checkUseAfterFree(Sym, C); + checkUseAfterFree(Sym, C, S); } //===----------------------------------------------------------------------===// @@ -1118,13 +1222,11 @@ void MallocChecker::checkBind(SVal loc, SVal val, const Stmt *S, // 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). - escapes = (state == (state->bindLoc(*regionLoc, val))); - } - if (!escapes) { - // Case 4: We do not currently model what happens when a symbol is - // assigned to a struct field, so be conservative here and let the symbol - // go. TODO: This could definitely be improved upon. - escapes = !isa<VarRegion>(regionLoc->getRegion()); + // 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))); } } @@ -1165,7 +1267,7 @@ ProgramStateRef MallocChecker::evalAssume(ProgramStateRef state, if (RS) { if (RS->isReleased() && ! I.getData().IsFreeOnFailure) state = state->set<RegionState>(ReallocSym, - RefState::getAllocateUnchecked(RS->getStmt())); + RefState::getAllocated(RS->getStmt())); } state = state->remove<ReallocPairs>(I.getKey()); } @@ -1175,118 +1277,30 @@ ProgramStateRef MallocChecker::evalAssume(ProgramStateRef state, } // Check if the function is known to us. So, for example, we could -// conservatively assume it can free/reallocate it's pointer arguments. +// conservatively assume it can free/reallocate its pointer arguments. // (We assume that the pointers cannot escape through calls to system // functions not handled by this checker.) -bool MallocChecker::doesNotFreeMemory(const CallOrObjCMessage *Call, +bool MallocChecker::doesNotFreeMemory(const CallEvent *Call, ProgramStateRef State) const { - if (!Call) - return false; + assert(Call); // For now, assume that any C++ call can free memory. // TODO: If we want to be more optimistic here, we'll need to make sure that // regions escape to C++ containers. They seem to do that even now, but for // mysterious reasons. - if (Call->isCXXCall()) - return false; - - const Decl *D = Call->getDecl(); - if (!D) + if (!(isa<FunctionCall>(Call) || isa<ObjCMethodCall>(Call))) return false; - ASTContext &ASTC = State->getStateManager().getContext(); - - // If it's one of the allocation functions we can reason about, we model - // its behavior explicitly. - if (isa<FunctionDecl>(D) && isMemFunction(cast<FunctionDecl>(D), ASTC)) { - return true; - } - - // If it's not a system call, assume it frees memory. - SourceManager &SM = ASTC.getSourceManager(); - if (!SM.isInSystemHeader(D->getLocation())) - return false; - - // Process C/ObjC functions. - if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) { - // White list the system functions whose arguments escape. - const IdentifierInfo *II = FD->getIdentifier(); - if (!II) - return true; - StringRef FName = II->getName(); - - // White list thread local storage. - if (FName.equals("pthread_setspecific")) - return false; - - // White list the 'XXXNoCopy' ObjC functions. - if (FName.endswith("NoCopy")) { - // Look for the deallocator argument. We know that the memory ownership - // is not transfered only if the deallocator argument is - // 'kCFAllocatorNull'. - for (unsigned i = 1; i < Call->getNumArgs(); ++i) { - const Expr *ArgE = Call->getArg(i)->IgnoreParenCasts(); - if (const DeclRefExpr *DE = dyn_cast<DeclRefExpr>(ArgE)) { - StringRef DeallocatorName = DE->getFoundDecl()->getName(); - if (DeallocatorName == "kCFAllocatorNull") - return true; - } - } - return false; - } - - // PR12101 - // Many CoreFoundation and CoreGraphics might allow a tracked object - // to escape. - if (Call->isCFCGAllowingEscape(FName)) + // Check Objective-C messages by selector name. + if (const ObjCMethodCall *Msg = dyn_cast<ObjCMethodCall>(Call)) { + // If it's not a framework call, or if it takes a callback, assume it + // can free memory. + if (!Call->isInSystemHeader() || Call->hasNonZeroCallbackArg()) return false; - // Associating streams with malloced buffers. The pointer can escape if - // 'closefn' is specified (and if that function does free memory). - // Currently, we do not inspect the 'closefn' function (PR12101). - if (FName == "funopen") - if (Call->getNumArgs() >= 4 && !Call->getArgSVal(4).isConstant(0)) - return false; - - // Do not warn on pointers passed to 'setbuf' when used with std streams, - // these leaks might be intentional when setting the buffer for stdio. - // http://stackoverflow.com/questions/2671151/who-frees-setvbuf-buffer - if (FName == "setbuf" || FName =="setbuffer" || - FName == "setlinebuf" || FName == "setvbuf") { - if (Call->getNumArgs() >= 1) - if (const DeclRefExpr *Arg = - dyn_cast<DeclRefExpr>(Call->getArg(0)->IgnoreParenCasts())) - if (const VarDecl *D = dyn_cast<VarDecl>(Arg->getDecl())) - if (D->getCanonicalDecl()->getName().find("std") - != StringRef::npos) - return false; - } - - // A bunch of other functions, which take ownership of a pointer (See retain - // release checker). Not all the parameters here are invalidated, but the - // Malloc checker cannot differentiate between them. The right way of doing - // this would be to implement a pointer escapes callback. - if (FName == "CVPixelBufferCreateWithBytes" || - FName == "CGBitmapContextCreateWithData" || - FName == "CVPixelBufferCreateWithPlanarBytes" || - FName == "OSAtomicEnqueue") { - return false; - } - - // Whitelist NSXXInsertXX, for example NSMapInsertIfAbsent, since they can - // be deallocated by NSMapRemove. - if (FName.startswith("NS") && (FName.find("Insert") != StringRef::npos)) - return false; - - // Otherwise, assume that the function does not free memory. - // Most system calls, do not free the memory. - return true; - - // Process ObjC functions. - } else if (const ObjCMethodDecl * ObjCD = dyn_cast<ObjCMethodDecl>(D)) { - Selector S = ObjCD->getSelector(); + Selector S = Msg->getSelector(); - // White list the ObjC functions which do free memory. + // Whitelist the ObjC methods which do free memory. // - Anything containing 'freeWhenDone' param set to 1. // Ex: dataWithBytesNoCopy:length:freeWhenDone. for (unsigned i = 1; i < S.getNumArgs(); ++i) { @@ -1299,20 +1313,111 @@ bool MallocChecker::doesNotFreeMemory(const CallOrObjCMessage *Call, } // If the first selector ends with NoCopy, assume that the ownership is - // transfered as well. + // transferred as well. // Ex: [NSData dataWithBytesNoCopy:bytes length:10]; - if (S.getNameForSlot(0).endswith("NoCopy")) { + StringRef FirstSlot = S.getNameForSlot(0); + if (FirstSlot.endswith("NoCopy")) + return false; + + // If the first selector starts with addPointer, insertPointer, + // or replacePointer, assume we are dealing with NSPointerArray or similar. + // This is similar to C++ containers (vector); we still might want to check + // that the pointers get freed by following the container itself. + if (FirstSlot.startswith("addPointer") || + FirstSlot.startswith("insertPointer") || + FirstSlot.startswith("replacePointer")) { return false; } - // Otherwise, assume that the function does not free memory. - // Most system calls, do not free the memory. + // Otherwise, assume that the method does not free memory. + // Most framework methods do not free memory. return true; } - // Otherwise, assume that the function can free memory. - return false; + // At this point the only thing left to handle is straight function calls. + const FunctionDecl *FD = cast<FunctionCall>(Call)->getDecl(); + if (!FD) + return false; + + ASTContext &ASTC = State->getStateManager().getContext(); + + // If it's one of the allocation functions we can reason about, we model + // its behavior explicitly. + if (isMemFunction(FD, ASTC)) + return true; + // If it's not a system call, assume it frees memory. + if (!Call->isInSystemHeader()) + return false; + + // White list the system functions whose arguments escape. + const IdentifierInfo *II = FD->getIdentifier(); + if (!II) + return false; + StringRef FName = II->getName(); + + // White list the 'XXXNoCopy' CoreFoundation functions. + // We specifically check these before + if (FName.endswith("NoCopy")) { + // Look for the deallocator argument. We know that the memory ownership + // is not transferred only if the deallocator argument is + // 'kCFAllocatorNull'. + for (unsigned i = 1; i < Call->getNumArgs(); ++i) { + const Expr *ArgE = Call->getArgExpr(i)->IgnoreParenCasts(); + if (const DeclRefExpr *DE = dyn_cast<DeclRefExpr>(ArgE)) { + StringRef DeallocatorName = DE->getFoundDecl()->getName(); + if (DeallocatorName == "kCFAllocatorNull") + return true; + } + } + return false; + } + + // Associating streams with malloced buffers. The pointer can escape if + // 'closefn' is specified (and if that function does free memory), + // but it will not if closefn is not specified. + // Currently, we do not inspect the 'closefn' function (PR12101). + if (FName == "funopen") + if (Call->getNumArgs() >= 4 && Call->getArgSVal(4).isConstant(0)) + return true; + + // Do not warn on pointers passed to 'setbuf' when used with std streams, + // these leaks might be intentional when setting the buffer for stdio. + // http://stackoverflow.com/questions/2671151/who-frees-setvbuf-buffer + if (FName == "setbuf" || FName =="setbuffer" || + FName == "setlinebuf" || FName == "setvbuf") { + if (Call->getNumArgs() >= 1) { + const Expr *ArgE = Call->getArgExpr(0)->IgnoreParenCasts(); + if (const DeclRefExpr *ArgDRE = dyn_cast<DeclRefExpr>(ArgE)) + if (const VarDecl *D = dyn_cast<VarDecl>(ArgDRE->getDecl())) + if (D->getCanonicalDecl()->getName().find("std") != StringRef::npos) + return false; + } + } + + // A bunch of other functions which either take ownership of a pointer or + // wrap the result up in a struct or object, meaning it can be freed later. + // (See RetainCountChecker.) Not all the parameters here are invalidated, + // but the Malloc checker cannot differentiate between them. The right way + // of doing this would be to implement a pointer escapes callback. + if (FName == "CGBitmapContextCreate" || + FName == "CGBitmapContextCreateWithData" || + FName == "CVPixelBufferCreateWithBytes" || + FName == "CVPixelBufferCreateWithPlanarBytes" || + FName == "OSAtomicEnqueue") { + return false; + } + + // Handle cases where we know a buffer's /address/ can escape. + // Note that the above checks handle some special cases where we know that + // even though the address escapes, it's still our responsibility to free the + // buffer. + if (Call->argumentsMayEscape()) + return false; + + // Otherwise, assume that the function does not free memory. + // Most system calls do not free the memory. + return true; } // If the symbol we are tracking is invalidated, but not explicitly (ex: the &p @@ -1323,7 +1428,7 @@ MallocChecker::checkRegionChanges(ProgramStateRef State, const StoreManager::InvalidatedSymbols *invalidated, ArrayRef<const MemRegion *> ExplicitRegions, ArrayRef<const MemRegion *> Regions, - const CallOrObjCMessage *Call) const { + const CallEvent *Call) const { if (!invalidated || invalidated->empty()) return State; llvm::SmallPtrSet<SymbolRef, 8> WhitelistedSymbols; @@ -1345,9 +1450,13 @@ MallocChecker::checkRegionChanges(ProgramStateRef State, SymbolRef sym = *I; if (WhitelistedSymbols.count(sym)) continue; - // The symbol escaped. - if (const RefState *RS = State->get<RegionState>(sym)) - State = State->set<RegionState>(sym, RefState::getEscaped(RS->getStmt())); + // The symbol escaped. Note, we assume that if the symbol is released, + // passing it out will result in a use after free. We also keep tracking + // relinquished symbols. + if (const RefState *RS = State->get<RegionState>(sym)) { + if (RS->isAllocated()) + State = State->remove<RegionState>(sym); + } } return State; } @@ -1377,7 +1486,7 @@ MallocChecker::MallocBugVisitor::VisitNode(const ExplodedNode *N, const RefState *RS = state->get<RegionState>(Sym); const RefState *RSPrev = statePrev->get<RegionState>(Sym); - if (!RS && !RSPrev) + if (!RS) return 0; const Stmt *S = 0; @@ -1386,17 +1495,22 @@ MallocChecker::MallocBugVisitor::VisitNode(const ExplodedNode *N, // Retrieve the associated statement. ProgramPoint ProgLoc = N->getLocation(); - if (isa<StmtPoint>(ProgLoc)) - S = cast<StmtPoint>(ProgLoc).getStmt(); + if (StmtPoint *SP = dyn_cast<StmtPoint>(&ProgLoc)) + S = SP->getStmt(); + else if (CallExitEnd *Exit = dyn_cast<CallExitEnd>(&ProgLoc)) + S = Exit->getCalleeContext()->getCallSite(); // If an assumption was made on a branch, it should be caught // here by looking at the state transition. - if (isa<BlockEdge>(ProgLoc)) { - const CFGBlock *srcBlk = cast<BlockEdge>(ProgLoc).getSrc(); + else if (BlockEdge *Edge = dyn_cast<BlockEdge>(&ProgLoc)) { + const CFGBlock *srcBlk = Edge->getSrc(); S = srcBlk->getTerminator(); } if (!S) return 0; + // FIXME: We will eventually need to handle non-statement-based events + // (__attribute__((cleanup))). + // Find out if this is an interesting point and what is the kind. if (Mode == Normal) { if (isAllocated(RS, RSPrev, S)) { @@ -1407,6 +1521,9 @@ MallocChecker::MallocBugVisitor::VisitNode(const ExplodedNode *N, Msg = "Memory is released"; StackHint = new StackHintGeneratorForSymbol(Sym, "Returned released memory"); + } else if (isRelinquished(RS, RSPrev, S)) { + Msg = "Memory ownership is transfered"; + StackHint = new StackHintGeneratorForSymbol(Sym, ""); } else if (isReallocFailedCheck(RS, RSPrev, S)) { Mode = ReallocationFailed; Msg = "Reallocation failed"; @@ -1428,11 +1545,6 @@ MallocChecker::MallocBugVisitor::VisitNode(const ExplodedNode *N, // Is this is the first appearance of the reallocated symbol? if (!statePrev->get<RegionState>(FailedReallocSymbol)) { - // If we ever hit this assert, that means BugReporter has decided to skip - // node pairs or visit them out of order. - assert(state->get<RegionState>(FailedReallocSymbol) && - "Missed the reallocation point"); - // We're at the reallocation point. Msg = "Attempt to reallocate memory"; StackHint = new StackHintGeneratorForSymbol(Sym, @@ -1452,6 +1564,14 @@ MallocChecker::MallocBugVisitor::VisitNode(const ExplodedNode *N, return new PathDiagnosticEventPiece(Pos, Msg, true, StackHint); } +void MallocChecker::printState(raw_ostream &Out, ProgramStateRef State, + const char *NL, const char *Sep) const { + + RegionStateTy RS = State->get<RegionState>(); + + if (!RS.isEmpty()) + Out << "Has Malloc data" << NL; +} #define REGISTER_CHECKER(name) \ void ento::register##name(CheckerManager &mgr) {\ diff --git a/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp b/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp index 08a9da1..6292a47 100644 --- a/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp @@ -118,11 +118,6 @@ public: Visit(E->getRHS()); } - void VisitBinAdd(const BinaryOperator *E) { - Visit(E->getLHS()); - Visit(E->getRHS()); - } - void VisitImplicitCastExpr(const ImplicitCastExpr *E) { return Visit(E->getSubExpr()); } @@ -139,6 +134,29 @@ public: } }; +// Determine if the pointee and sizeof types are compatible. Here +// we ignore constness of pointer types. +static bool typesCompatible(ASTContext &C, QualType A, QualType B) { + while (true) { + A = A.getCanonicalType(); + B = B.getCanonicalType(); + + if (A.getTypePtr() == B.getTypePtr()) + return true; + + if (const PointerType *ptrA = A->getAs<PointerType>()) + if (const PointerType *ptrB = B->getAs<PointerType>()) { + A = ptrA->getPointeeType(); + B = ptrB->getPointeeType(); + continue; + } + + break; + } + + return false; +} + class MallocSizeofChecker : public Checker<check::ASTCodeBody> { public: void checkASTCodeBody(const Decl *D, AnalysisManager& mgr, @@ -166,7 +184,7 @@ public: continue; QualType SizeofType = SFinder.Sizeofs[0]->getTypeOfArgument(); - if (!BR.getContext().hasSameUnqualifiedType(PointeeType, SizeofType)) { + if (!typesCompatible(BR.getContext(), PointeeType, SizeofType)) { const TypeSourceInfo *TSI = 0; if (i->CastedExprParent.is<const VarDecl *>()) { TSI = @@ -180,9 +198,8 @@ public: OS << "Result of '" << i->AllocCall->getDirectCallee()->getIdentifier()->getName() - << "' is converted to type '" - << CastedType.getAsString() << "', whose pointee type '" - << PointeeType.getAsString() << "' is incompatible with " + << "' 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()); @@ -194,7 +211,7 @@ public: PathDiagnosticLocation::createBegin(i->AllocCall->getCallee(), BR.getSourceManager(), ADC); - BR.EmitBasicReport(D, "allocator sizeof operand mismatch", + 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 4989ba8..aad3b0f 100644 --- a/lib/StaticAnalyzer/Checkers/NSAutoreleasePoolChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/NSAutoreleasePoolChecker.cpp @@ -20,9 +20,9 @@ #include "clang/StaticAnalyzer/Core/CheckerManager.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h" -#include "clang/StaticAnalyzer/Core/PathSensitive/ObjCMessage.h" #include "clang/AST/DeclObjC.h" #include "clang/AST/Decl.h" @@ -36,29 +36,20 @@ class NSAutoreleasePoolChecker mutable Selector releaseS; public: - void checkPreObjCMessage(ObjCMessage msg, CheckerContext &C) const; + void checkPreObjCMessage(const ObjCMethodCall &msg, CheckerContext &C) const; }; } // end anonymous namespace -void NSAutoreleasePoolChecker::checkPreObjCMessage(ObjCMessage msg, +void NSAutoreleasePoolChecker::checkPreObjCMessage(const ObjCMethodCall &msg, CheckerContext &C) const { - - const Expr *receiver = msg.getInstanceReceiver(); - if (!receiver) + if (!msg.isInstanceMessage()) return; - - // FIXME: Enhance with value-tracking information instead of consulting - // the type of the expression. - const ObjCObjectPointerType* PT = - receiver->getType()->getAs<ObjCObjectPointerType>(); - - if (!PT) - return; - const ObjCInterfaceDecl *OD = PT->getInterfaceDecl(); + + const ObjCInterfaceDecl *OD = msg.getReceiverInterface(); if (!OD) return; - if (!OD->getIdentifier()->getName().equals("NSAutoreleasePool")) + if (!OD->getIdentifier()->isStr("NSAutoreleasePool")) return; if (releaseS.isNull()) diff --git a/lib/StaticAnalyzer/Checkers/NoReturnFunctionChecker.cpp b/lib/StaticAnalyzer/Checkers/NoReturnFunctionChecker.cpp index c2d7c09..efb7072 100644 --- a/lib/StaticAnalyzer/Checkers/NoReturnFunctionChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/NoReturnFunctionChecker.cpp @@ -15,8 +15,8 @@ #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/PathSensitive/ObjCMessage.h" #include "llvm/ADT/StringSwitch.h" #include <cstdarg> @@ -29,7 +29,7 @@ class NoReturnFunctionChecker : public Checker< check::PostStmt<CallExpr>, check::PostObjCMessage > { public: void checkPostStmt(const CallExpr *CE, CheckerContext &C) const; - void checkPostObjCMessage(const ObjCMessage &msg, CheckerContext &C) const; + void checkPostObjCMessage(const ObjCMethodCall &msg, CheckerContext &C) const; }; } @@ -98,7 +98,7 @@ static bool END_WITH_NULL isMultiArgSelector(const Selector *Sel, ...) { return (Arg == NULL); } -void NoReturnFunctionChecker::checkPostObjCMessage(const ObjCMessage &Msg, +void NoReturnFunctionChecker::checkPostObjCMessage(const ObjCMethodCall &Msg, CheckerContext &C) const { // HACK: This entire check is to handle two messages in the Cocoa frameworks: // -[NSAssertionHandler diff --git a/lib/StaticAnalyzer/Checkers/ObjCAtSyncChecker.cpp b/lib/StaticAnalyzer/Checkers/ObjCAtSyncChecker.cpp index 777e9ea..4cc92ce 100644 --- a/lib/StaticAnalyzer/Checkers/ObjCAtSyncChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/ObjCAtSyncChecker.cpp @@ -50,8 +50,7 @@ void ObjCAtSyncChecker::checkPreStmt(const ObjCAtSynchronizedStmt *S, "for @synchronized")); BugReport *report = new BugReport(*BT_undef, BT_undef->getDescription(), N); - report->addVisitor(bugreporter::getTrackNullOrUndefValueVisitor(N, Ex, - report)); + bugreporter::addTrackNullOrUndefValueVisitor(N, Ex, report); C.EmitReport(report); } return; @@ -74,8 +73,7 @@ void ObjCAtSyncChecker::checkPreStmt(const ObjCAtSynchronizedStmt *S, "(no synchronization will occur)")); BugReport *report = new BugReport(*BT_null, BT_null->getDescription(), N); - report->addVisitor(bugreporter::getTrackNullOrUndefValueVisitor(N, Ex, - report)); + bugreporter::addTrackNullOrUndefValueVisitor(N, Ex, report); C.EmitReport(report); return; diff --git a/lib/StaticAnalyzer/Checkers/ObjCContainersChecker.cpp b/lib/StaticAnalyzer/Checkers/ObjCContainersChecker.cpp index f4655b6..2ab49ed 100644 --- a/lib/StaticAnalyzer/Checkers/ObjCContainersChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/ObjCContainersChecker.cpp @@ -21,7 +21,6 @@ #include "clang/StaticAnalyzer/Core/CheckerManager.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h" -#include "clang/StaticAnalyzer/Core/PathSensitive/ObjCMessage.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" #include "clang/AST/ParentMap.h" diff --git a/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp b/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp index 97b58cf..be45da1 100644 --- a/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp @@ -39,9 +39,9 @@ #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/PathSensitive/ProgramStateTrait.h" -#include "clang/StaticAnalyzer/Core/PathSensitive/ObjCMessage.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" #include "clang/AST/ParentMap.h" @@ -50,29 +50,27 @@ using namespace ento; static bool shouldRunOnFunctionOrMethod(const NamedDecl *ND); static bool isInitializationMethod(const ObjCMethodDecl *MD); -static bool isInitMessage(const ObjCMessage &msg); +static bool isInitMessage(const ObjCMethodCall &Msg); static bool isSelfVar(SVal location, CheckerContext &C); namespace { -class ObjCSelfInitChecker : public Checker< check::PreObjCMessage, - check::PostObjCMessage, +class ObjCSelfInitChecker : public Checker< check::PostObjCMessage, check::PostStmt<ObjCIvarRefExpr>, check::PreStmt<ReturnStmt>, - check::PreStmt<CallExpr>, - check::PostStmt<CallExpr>, - check::Location > { + check::PreCall, + check::PostCall, + check::Location, + check::Bind > { public: - void checkPreObjCMessage(ObjCMessage msg, CheckerContext &C) const; - void checkPostObjCMessage(ObjCMessage msg, CheckerContext &C) const; + void checkPostObjCMessage(const ObjCMethodCall &Msg, CheckerContext &C) const; void checkPostStmt(const ObjCIvarRefExpr *E, CheckerContext &C) const; void checkPreStmt(const ReturnStmt *S, CheckerContext &C) const; - void checkPreStmt(const CallExpr *CE, CheckerContext &C) const; - void checkPostStmt(const CallExpr *CE, CheckerContext &C) const; void checkLocation(SVal location, bool isLoad, const Stmt *S, CheckerContext &C) const; + void checkBind(SVal loc, SVal val, const Stmt *S, CheckerContext &C) const; - void checkPreStmt(const CallOrObjCMessage &CE, CheckerContext &C) const; - void checkPostStmt(const CallOrObjCMessage &CE, CheckerContext &C) const; + void checkPreCall(const CallEvent &CE, CheckerContext &C) const; + void checkPostCall(const CallEvent &CE, CheckerContext &C) const; }; } // end anonymous namespace @@ -181,7 +179,7 @@ static void checkForInvalidSelf(const Expr *E, CheckerContext &C, C.EmitReport(report); } -void ObjCSelfInitChecker::checkPostObjCMessage(ObjCMessage msg, +void ObjCSelfInitChecker::checkPostObjCMessage(const ObjCMethodCall &Msg, CheckerContext &C) const { // When encountering a message that does initialization (init rule), // tag the return value so that we know later on that if self has this value @@ -192,7 +190,7 @@ void ObjCSelfInitChecker::checkPostObjCMessage(ObjCMessage msg, C.getCurrentAnalysisDeclContext()->getDecl()))) return; - if (isInitMessage(msg)) { + if (isInitMessage(Msg)) { // Tag the return value as the result of an initializer. ProgramStateRef state = C.getState(); @@ -201,14 +199,11 @@ void ObjCSelfInitChecker::checkPostObjCMessage(ObjCMessage msg, // value out when we return from this method. state = state->set<CalledInit>(true); - SVal V = state->getSVal(msg.getMessageExpr(), C.getLocationContext()); + SVal V = state->getSVal(Msg.getOriginExpr(), C.getLocationContext()); addSelfFlag(state, V, SelfFlag_InitRes, C); return; } - CallOrObjCMessage MsgWrapper(msg, C.getState(), C.getLocationContext()); - checkPostStmt(MsgWrapper, C); - // We don't check for an invalid 'self' in an obj-c message expression to cut // down false positives where logging functions get information from self // (like its class) or doing "invalidation" on self when the initialization @@ -239,8 +234,8 @@ void ObjCSelfInitChecker::checkPreStmt(const ReturnStmt *S, "'[(super or self) init...]'"); } -// When a call receives a reference to 'self', [Pre/Post]VisitGenericCall pass -// the SelfFlags from the object 'self' point to before the call, to the new +// When a call receives a reference to 'self', [Pre/Post]Call pass +// the SelfFlags from the object 'self' points to before the call to the new // object after the call. This is to avoid invalidation of 'self' by logging // functions. // Another common pattern in classes with multiple initializers is to put the @@ -255,26 +250,13 @@ void ObjCSelfInitChecker::checkPreStmt(const ReturnStmt *S, // Until we can use inter-procedural analysis, in such a call, transfer the // SelfFlags to the result of the call. -void ObjCSelfInitChecker::checkPreStmt(const CallExpr *CE, +void ObjCSelfInitChecker::checkPreCall(const CallEvent &CE, CheckerContext &C) const { - CallOrObjCMessage CEWrapper(CE, C.getState(), C.getLocationContext()); - checkPreStmt(CEWrapper, C); -} - -void ObjCSelfInitChecker::checkPostStmt(const CallExpr *CE, - CheckerContext &C) const { - CallOrObjCMessage CEWrapper(CE, C.getState(), C.getLocationContext()); - checkPostStmt(CEWrapper, C); -} - -void ObjCSelfInitChecker::checkPreObjCMessage(ObjCMessage Msg, - CheckerContext &C) const { - CallOrObjCMessage MsgWrapper(Msg, C.getState(), C.getLocationContext()); - checkPreStmt(MsgWrapper, C); -} + // FIXME: A callback should disable checkers at the start of functions. + if (!shouldRunOnFunctionOrMethod(dyn_cast<NamedDecl>( + C.getCurrentAnalysisDeclContext()->getDecl()))) + return; -void ObjCSelfInitChecker::checkPreStmt(const CallOrObjCMessage &CE, - CheckerContext &C) const { ProgramStateRef state = C.getState(); unsigned NumArgs = CE.getNumArgs(); // If we passed 'self' as and argument to the call, record it in the state @@ -296,9 +278,19 @@ void ObjCSelfInitChecker::checkPreStmt(const CallOrObjCMessage &CE, } } -void ObjCSelfInitChecker::checkPostStmt(const CallOrObjCMessage &CE, +void ObjCSelfInitChecker::checkPostCall(const CallEvent &CE, CheckerContext &C) const { + // FIXME: A callback should disable checkers at the start of functions. + if (!shouldRunOnFunctionOrMethod(dyn_cast<NamedDecl>( + C.getCurrentAnalysisDeclContext()->getDecl()))) + return; + ProgramStateRef state = C.getState(); + SelfFlagEnum prevFlags = (SelfFlagEnum)state->get<PreCallSelfFlags>(); + if (!prevFlags) + return; + state = state->remove<PreCallSelfFlags>(); + unsigned NumArgs = CE.getNumArgs(); for (unsigned i = 0; i < NumArgs; ++i) { SVal argV = CE.getArgSVal(i); @@ -306,8 +298,6 @@ void ObjCSelfInitChecker::checkPostStmt(const CallOrObjCMessage &CE, // If the address of 'self' is being passed to the call, assume that the // 'self' after the call will have the same flags. // EX: log(&self) - SelfFlagEnum prevFlags = (SelfFlagEnum)state->get<PreCallSelfFlags>(); - state = state->remove<PreCallSelfFlags>(); addSelfFlag(state, state->getSVal(cast<Loc>(argV)), prevFlags, C); return; } else if (hasSelfFlag(argV, SelfFlag_Self, C)) { @@ -315,8 +305,6 @@ void ObjCSelfInitChecker::checkPostStmt(const CallOrObjCMessage &CE, // returns 'self'. So assign the flags, which were set on 'self' to the // return value. // EX: self = performMoreInitialization(self) - SelfFlagEnum prevFlags = (SelfFlagEnum)state->get<PreCallSelfFlags>(); - state = state->remove<PreCallSelfFlags>(); const Expr *CallExpr = CE.getOriginExpr(); if (CallExpr) addSelfFlag(state, state->getSVal(CallExpr, C.getLocationContext()), @@ -336,6 +324,28 @@ void ObjCSelfInitChecker::checkLocation(SVal location, bool isLoad, addSelfFlag(state, state->getSVal(cast<Loc>(location)), SelfFlag_Self, C); } + +void ObjCSelfInitChecker::checkBind(SVal loc, SVal val, const Stmt *S, + CheckerContext &C) const { + // Allow assignment of anything to self. Self is a local variable in the + // initializer, so it is legal to assign anything to it, like results of + // static functions/method calls. After self is assigned something we cannot + // reason about, stop enforcing the rules. + // (Only continue checking if the assigned value should be treated as self.) + if ((isSelfVar(loc, C)) && + !hasSelfFlag(val, SelfFlag_InitRes, C) && + !hasSelfFlag(val, SelfFlag_Self, C) && + !isSelfVar(val, C)) { + + // Stop tracking the checker-specific state in the state. + ProgramStateRef State = C.getState(); + State = State->remove<CalledInit>(); + if (SymbolRef sym = loc.getAsSymbol()) + State = State->remove<SelfFlag>(sym); + C.addTransition(State); + } +} + // FIXME: A callback should disable checkers at the start of functions. static bool shouldRunOnFunctionOrMethod(const NamedDecl *ND) { if (!ND) @@ -383,8 +393,8 @@ static bool isInitializationMethod(const ObjCMethodDecl *MD) { return MD->getMethodFamily() == OMF_init; } -static bool isInitMessage(const ObjCMessage &msg) { - return msg.getMethodFamily() == OMF_init; +static bool isInitMessage(const ObjCMethodCall &Call) { + return Call.getMethodFamily() == OMF_init; } //===----------------------------------------------------------------------===// diff --git a/lib/StaticAnalyzer/Checkers/ObjCUnusedIVarsChecker.cpp b/lib/StaticAnalyzer/Checkers/ObjCUnusedIVarsChecker.cpp index 4718dc7..582269c 100644 --- a/lib/StaticAnalyzer/Checkers/ObjCUnusedIVarsChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/ObjCUnusedIVarsChecker.cpp @@ -47,6 +47,15 @@ static void Scan(IvarUsageMap& M, const Stmt *S) { return; } + if (const PseudoObjectExpr *POE = dyn_cast<PseudoObjectExpr>(S)) + for (PseudoObjectExpr::const_semantics_iterator + i = POE->semantics_begin(), e = POE->semantics_end(); i != e; ++i) { + const Expr *sub = *i; + if (const OpaqueValueExpr *OVE = dyn_cast<OpaqueValueExpr>(sub)) + sub = OVE->getSourceExpr(); + Scan(M, sub); + } + for (Stmt::const_child_iterator I=S->child_begin(),E=S->child_end(); I!=E;++I) Scan(M, *I); } diff --git a/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp b/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp index b569e41..5503b23 100644 --- a/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp @@ -23,10 +23,10 @@ #include "clang/StaticAnalyzer/Core/CheckerManager.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" #include "clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h" #include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h" -#include "clang/StaticAnalyzer/Core/PathSensitive/ObjCMessage.h" #include "llvm/ADT/DenseMap.h" #include "llvm/ADT/FoldingSet.h" #include "llvm/ADT/ImmutableList.h" @@ -66,8 +66,9 @@ public: /// particular argument. enum ArgEffect { DoNothing, Autorelease, Dealloc, DecRef, DecRefMsg, DecRefBridgedTransfered, + DecRefAndStopTracking, DecRefMsgAndStopTracking, IncRefMsg, IncRef, MakeCollectable, MayEscape, - NewAutoreleasePool, SelfOwn, StopTracking }; + NewAutoreleasePool, StopTracking }; namespace llvm { template <> struct FoldingSetTrait<ArgEffect> { @@ -351,6 +352,20 @@ struct ProgramStateTrait<RefBindings> } } +static inline const RefVal *getRefBinding(ProgramStateRef State, + SymbolRef Sym) { + return State->get<RefBindings>(Sym); +} + +static inline ProgramStateRef setRefBinding(ProgramStateRef State, + SymbolRef Sym, RefVal Val) { + return State->set<RefBindings>(Sym, Val); +} + +static ProgramStateRef removeRefBinding(ProgramStateRef State, SymbolRef Sym) { + return State->remove<RefBindings>(Sym); +} + //===----------------------------------------------------------------------===// // Function/Method behavior summaries. //===----------------------------------------------------------------------===// @@ -431,6 +446,12 @@ public: bool isSimple() const { return Args.isEmpty(); } + +private: + ArgEffects getArgEffects() const { return Args; } + ArgEffect getDefaultArgEffect() const { return DefaultArgEffect; } + + friend class RetainSummaryManager; }; } // end anonymous namespace @@ -449,9 +470,6 @@ public: ObjCSummaryKey(const ObjCInterfaceDecl *d, Selector s) : II(d ? d->getIdentifier() : 0), S(s) {} - ObjCSummaryKey(const ObjCInterfaceDecl *d, IdentifierInfo *ii, Selector s) - : II(d ? d->getIdentifier() : ii), S(s) {} - ObjCSummaryKey(Selector s) : II(0), S(s) {} @@ -473,17 +491,14 @@ template <> struct DenseMapInfo<ObjCSummaryKey> { } static unsigned getHashValue(const ObjCSummaryKey &V) { - return (DenseMapInfo<IdentifierInfo*>::getHashValue(V.getIdentifier()) - & 0x88888888) - | (DenseMapInfo<Selector>::getHashValue(V.getSelector()) - & 0x55555555); + typedef std::pair<IdentifierInfo*, Selector> PairTy; + return DenseMapInfo<PairTy>::getHashValue(PairTy(V.getIdentifier(), + V.getSelector())); } static bool isEqual(const ObjCSummaryKey& LHS, const ObjCSummaryKey& RHS) { - return DenseMapInfo<IdentifierInfo*>::isEqual(LHS.getIdentifier(), - RHS.getIdentifier()) && - DenseMapInfo<Selector>::isEqual(LHS.getSelector(), - RHS.getSelector()); + return LHS.getIdentifier() == RHS.getIdentifier() && + LHS.getSelector() == RHS.getSelector(); } }; @@ -498,21 +513,16 @@ class ObjCSummaryCache { public: ObjCSummaryCache() {} - const RetainSummary * find(const ObjCInterfaceDecl *D, IdentifierInfo *ClsName, - Selector S) { - // Lookup the method using the decl for the class @interface. If we - // have no decl, lookup using the class name. - return D ? find(D, S) : find(ClsName, S); - } - const RetainSummary * find(const ObjCInterfaceDecl *D, Selector S) { // Do a lookup with the (D,S) pair. If we find a match return // the iterator. ObjCSummaryKey K(D, S); MapTy::iterator I = M.find(K); - if (I != M.end() || !D) + if (I != M.end()) return I->second; + if (!D) + return NULL; // Walk the super chain. If we find a hit with a parent, we'll end // up returning that summary. We actually allow that key (null,S), as @@ -628,9 +638,6 @@ class RetainSummaryManager { ArgEffects getArgEffects(); enum UnaryFuncKind { cfretain, cfrelease, cfmakecollectable }; - -public: - RetEffect getObjAllocRetEffect() const { return ObjCAllocRetE; } const RetainSummary *getUnarySummary(const FunctionType* FT, UnaryFuncKind func); @@ -648,6 +655,10 @@ public: return getPersistentSummary(Summ); } + const RetainSummary *getDoNothingSummary() { + return getPersistentSummary(RetEffect::MakeNoRet(), DoNothing, DoNothing); + } + const RetainSummary *getDefaultSummary() { return getPersistentSummary(RetEffect::MakeNoRet(), DoNothing, MayEscape); @@ -739,41 +750,32 @@ public: InitializeMethodSummaries(); } - const RetainSummary *getSummary(const FunctionDecl *FD); + const RetainSummary *getSummary(const CallEvent &Call, + ProgramStateRef State = 0); - const RetainSummary *getMethodSummary(Selector S, IdentifierInfo *ClsName, - const ObjCInterfaceDecl *ID, + const RetainSummary *getFunctionSummary(const FunctionDecl *FD); + + const RetainSummary *getMethodSummary(Selector S, const ObjCInterfaceDecl *ID, const ObjCMethodDecl *MD, QualType RetTy, ObjCMethodSummariesTy &CachedSummaries); - const RetainSummary *getInstanceMethodSummary(const ObjCMessage &msg, - ProgramStateRef state, - const LocationContext *LC); - - const RetainSummary *getInstanceMethodSummary(const ObjCMessage &msg, - const ObjCInterfaceDecl *ID) { - return getMethodSummary(msg.getSelector(), 0, ID, msg.getMethodDecl(), - msg.getType(Ctx), ObjCMethodSummaries); - } + const RetainSummary *getInstanceMethodSummary(const ObjCMethodCall &M, + ProgramStateRef State); - const RetainSummary *getClassMethodSummary(const ObjCMessage &msg) { - const ObjCInterfaceDecl *Class = 0; - if (!msg.isInstanceMessage()) - Class = msg.getReceiverInterface(); + const RetainSummary *getClassMethodSummary(const ObjCMethodCall &M) { + assert(!M.isInstanceMessage()); + const ObjCInterfaceDecl *Class = M.getReceiverInterface(); - return getMethodSummary(msg.getSelector(), Class->getIdentifier(), - Class, msg.getMethodDecl(), msg.getType(Ctx), - ObjCClassMethodSummaries); + return getMethodSummary(M.getSelector(), Class, M.getDecl(), + M.getResultType(), ObjCClassMethodSummaries); } /// getMethodSummary - This version of getMethodSummary is used to query /// the summary for the current method being analyzed. const RetainSummary *getMethodSummary(const ObjCMethodDecl *MD) { - // FIXME: Eventually this should be unneeded. const ObjCInterfaceDecl *ID = MD->getClassInterface(); Selector S = MD->getSelector(); - IdentifierInfo *ClsName = ID->getIdentifier(); QualType ResultTy = MD->getResultType(); ObjCMethodSummariesTy *CachedSummaries; @@ -782,11 +784,11 @@ public: else CachedSummaries = &ObjCClassMethodSummaries; - return getMethodSummary(S, ClsName, ID, MD, ResultTy, *CachedSummaries); + return getMethodSummary(S, ID, MD, ResultTy, *CachedSummaries); } const RetainSummary *getStandardMethodSummary(const ObjCMethodDecl *MD, - Selector S, QualType RetTy); + Selector S, QualType RetTy); void updateSummaryFromAnnotations(const RetainSummary *&Summ, const ObjCMethodDecl *MD); @@ -794,11 +796,18 @@ public: void updateSummaryFromAnnotations(const RetainSummary *&Summ, const FunctionDecl *FD); + void updateSummaryForCall(const RetainSummary *&Summ, + const CallEvent &Call); + bool isGCEnabled() const { return GCEnabled; } bool isARCEnabled() const { return ARCEnabled; } bool isARCorGCEnabled() const { return GCEnabled || ARCEnabled; } + + RetEffect getObjAllocRetEffect() const { return ObjCAllocRetE; } + + friend class RetainSummaryTemplate; }; // Used to avoid allocating long-term (BPAlloc'd) memory for default retain @@ -811,10 +820,8 @@ class RetainSummaryTemplate { RetainSummary ScratchSummary; bool Accessed; public: - RetainSummaryTemplate(const RetainSummary *&real, const RetainSummary &base, - RetainSummaryManager &mgr) - : Manager(mgr), RealSummary(real), ScratchSummary(real ? *real : base), - Accessed(false) {} + RetainSummaryTemplate(const RetainSummary *&real, RetainSummaryManager &mgr) + : Manager(mgr), RealSummary(real), ScratchSummary(*real), Accessed(false) {} ~RetainSummaryTemplate() { if (Accessed) @@ -886,7 +893,101 @@ static bool isMakeCollectable(const FunctionDecl *FD, StringRef FName) { return FName.find("MakeCollectable") != StringRef::npos; } -const RetainSummary * RetainSummaryManager::getSummary(const FunctionDecl *FD) { +static ArgEffect getStopTrackingEquivalent(ArgEffect E) { + switch (E) { + case DoNothing: + case Autorelease: + case DecRefBridgedTransfered: + case IncRef: + case IncRefMsg: + case MakeCollectable: + case MayEscape: + case NewAutoreleasePool: + case StopTracking: + return StopTracking; + case DecRef: + case DecRefAndStopTracking: + return DecRefAndStopTracking; + case DecRefMsg: + case DecRefMsgAndStopTracking: + return DecRefMsgAndStopTracking; + case Dealloc: + return Dealloc; + } + + llvm_unreachable("Unknown ArgEffect kind"); +} + +void RetainSummaryManager::updateSummaryForCall(const RetainSummary *&S, + const CallEvent &Call) { + if (Call.hasNonZeroCallbackArg()) { + ArgEffect RecEffect = getStopTrackingEquivalent(S->getReceiverEffect()); + ArgEffect DefEffect = getStopTrackingEquivalent(S->getDefaultArgEffect()); + + ArgEffects CustomArgEffects = S->getArgEffects(); + for (ArgEffects::iterator I = CustomArgEffects.begin(), + E = CustomArgEffects.end(); + I != E; ++I) { + ArgEffect Translated = getStopTrackingEquivalent(I->second); + if (Translated != DefEffect) + ScratchArgs = AF.add(ScratchArgs, I->first, Translated); + } + + RetEffect RE = RetEffect::MakeNoRet(); + + // 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(); + } + + S = getPersistentSummary(RE, RecEffect, DefEffect); + } +} + +const RetainSummary * +RetainSummaryManager::getSummary(const CallEvent &Call, + ProgramStateRef State) { + const RetainSummary *Summ; + switch (Call.getKind()) { + case CE_Function: + Summ = getFunctionSummary(cast<FunctionCall>(Call).getDecl()); + break; + case CE_CXXMember: + case CE_CXXMemberOperator: + case CE_Block: + case CE_CXXConstructor: + case CE_CXXDestructor: + case CE_CXXAllocator: + // FIXME: These calls are currently unsupported. + return getPersistentStopSummary(); + case CE_ObjCMessage: { + const ObjCMethodCall &Msg = cast<ObjCMethodCall>(Call); + if (Msg.isInstanceMessage()) + Summ = getInstanceMethodSummary(Msg, State); + else + Summ = getClassMethodSummary(Msg); + break; + } + } + + updateSummaryForCall(Summ, Call); + + assert(Summ && "Unknown call type?"); + return Summ; +} + +const RetainSummary * +RetainSummaryManager::getFunctionSummary(const FunctionDecl *FD) { + // If we don't know what function we're calling, use our default summary. + if (!FD) + return getDefaultSummary(); + // Look up a summary in our cache of FunctionDecls -> Summaries. FuncSummariesTy::iterator I = FuncSummaries.find(FD); if (I != FuncSummaries.end()) @@ -894,6 +995,7 @@ const RetainSummary * RetainSummaryManager::getSummary(const FunctionDecl *FD) { // No summary? Generate one. const RetainSummary *S = 0; + bool AllowAnnotations = true; do { // We generate "stop" summaries for implicitly defined functions. @@ -901,13 +1003,6 @@ const RetainSummary * RetainSummaryManager::getSummary(const FunctionDecl *FD) { S = getPersistentStopSummary(); break; } - // For C++ methods, generate an implicit "stop" summary as well. We - // can relax this once we have a clear policy for C++ methods and - // ownership attributes. - if (isa<CXXMethodDecl>(FD)) { - S = getPersistentStopSummary(); - break; - } // [PR 3337] Use 'getAs<FunctionType>' to strip away any typedefs on the // function's type. @@ -929,18 +1024,22 @@ const RetainSummary * RetainSummaryManager::getSummary(const FunctionDecl *FD) { // filters. assert(ScratchArgs.isEmpty()); - if (FName == "pthread_create") { - // Part of: <rdar://problem/7299394>. This will be addressed - // better with IPA. + if (FName == "pthread_create" || FName == "pthread_setspecific") { + // Part of: <rdar://problem/7299394> and <rdar://problem/11282706>. + // This will be addressed better with IPA. S = getPersistentStopSummary(); } else if (FName == "NSMakeCollectable") { // Handle: id NSMakeCollectable(CFTypeRef) S = (RetTy->isObjCIdType()) ? getUnarySummary(FT, cfmakecollectable) : getPersistentStopSummary(); + // 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 == "IOBSDNameMatching" || FName == "IOServiceMatching" || FName == "IOServiceNameMatching" || + FName == "IORegistryEntrySearchCFProperty" || FName == "IORegistryEntryIDMatching" || FName == "IOOpenFirmwarePathMatching") { // Part of <rdar://problem/6961230>. (IOKit) @@ -993,6 +1092,8 @@ const RetainSummary * RetainSummaryManager::getSummary(const FunctionDecl *FD) { // libdispatch finalizers. ScratchArgs = AF.add(ScratchArgs, 1, StopTracking); S = getPersistentSummary(RetEffect::MakeNoRet(), DoNothing, DoNothing); + } else if (FName.startswith("NSLog")) { + S = getDoNothingSummary(); } else if (FName.startswith("NS") && (FName.find("Insert") != StringRef::npos)) { // Whitelist NSXXInsertXX, for example NSMapInsertIfAbsent, since they can @@ -1090,8 +1191,13 @@ const RetainSummary * RetainSummaryManager::getSummary(const FunctionDecl *FD) { } while (0); + // If we got all the way here without any luck, use a default summary. + if (!S) + S = getDefaultSummary(); + // Annotations override defaults. - updateSummaryFromAnnotations(S, FD); + if (AllowAnnotations) + updateSummaryFromAnnotations(S, FD); FuncSummaries[FD] = S; return S; @@ -1152,7 +1258,8 @@ RetainSummaryManager::updateSummaryFromAnnotations(const RetainSummary *&Summ, if (!FD) return; - RetainSummaryTemplate Template(Summ, *getDefaultSummary(), *this); + assert(Summ && "Must have a summary to add annotations to."); + RetainSummaryTemplate Template(Summ, *this); // Effects on the parameters. unsigned parm_idx = 0; @@ -1200,7 +1307,8 @@ RetainSummaryManager::updateSummaryFromAnnotations(const RetainSummary *&Summ, if (!MD) return; - RetainSummaryTemplate Template(Summ, *getDefaultSummary(), *this); + assert(Summ && "Must have a valid summary to add annotations to"); + RetainSummaryTemplate Template(Summ, *this); bool isTrackedLoc = false; // Effects on the receiver. @@ -1341,10 +1449,15 @@ RetainSummaryManager::getStandardMethodSummary(const ObjCMethodDecl *MD, // because the reference count is quite possibly handled by a delegate // method. if (S.isKeywordSelector()) { - const std::string &str = S.getAsString(); - assert(!str.empty()); - if (StrInStrNoCase(str, "delegate:") != StringRef::npos) - ReceiverEff = StopTracking; + for (unsigned i = 0, e = S.getNumArgs(); i != e; ++i) { + StringRef Slot = S.getNameForSlot(i); + if (Slot.substr(Slot.size() - 8).equals_lower("delegate")) { + if (ResultEff == ObjCInitRetE) + ResultEff = RetEffect::MakeNoRet(); + else + ReceiverEff = StopTracking; + } + } } if (ScratchArgs.isEmpty() && ReceiverEff == DoNothing && @@ -1355,56 +1468,46 @@ RetainSummaryManager::getStandardMethodSummary(const ObjCMethodDecl *MD, } const RetainSummary * -RetainSummaryManager::getInstanceMethodSummary(const ObjCMessage &msg, - ProgramStateRef state, - const LocationContext *LC) { - - // We need the type-information of the tracked receiver object - // Retrieve it from the state. - const Expr *Receiver = msg.getInstanceReceiver(); - const ObjCInterfaceDecl *ID = 0; - - // FIXME: Is this really working as expected? There are cases where - // we just use the 'ID' from the message expression. - SVal receiverV; - - if (Receiver) { - receiverV = state->getSValAsScalarOrLoc(Receiver, LC); - - // FIXME: Eventually replace the use of state->get<RefBindings> with - // a generic API for reasoning about the Objective-C types of symbolic - // objects. - if (SymbolRef Sym = receiverV.getAsLocSymbol()) - if (const RefVal *T = state->get<RefBindings>(Sym)) - if (const ObjCObjectPointerType* PT = +RetainSummaryManager::getInstanceMethodSummary(const ObjCMethodCall &Msg, + ProgramStateRef State) { + const ObjCInterfaceDecl *ReceiverClass = 0; + + // We do better tracking of the type of the object than the core ExprEngine. + // See if we have its type in our private state. + // FIXME: Eventually replace the use of state->get<RefBindings> with + // a generic API for reasoning about the Objective-C types of symbolic + // objects. + SVal ReceiverV = Msg.getReceiverSVal(); + if (SymbolRef Sym = ReceiverV.getAsLocSymbol()) + if (const RefVal *T = getRefBinding(State, Sym)) + if (const ObjCObjectPointerType *PT = T->getType()->getAs<ObjCObjectPointerType>()) - ID = PT->getInterfaceDecl(); + ReceiverClass = PT->getInterfaceDecl(); - // FIXME: this is a hack. This may or may not be the actual method - // that is called. - if (!ID) { - if (const ObjCObjectPointerType *PT = - Receiver->getType()->getAs<ObjCObjectPointerType>()) - ID = PT->getInterfaceDecl(); - } - } else { - // FIXME: Hack for 'super'. - ID = msg.getReceiverInterface(); - } + // If we don't know what kind of object this is, fall back to its static type. + if (!ReceiverClass) + ReceiverClass = Msg.getReceiverInterface(); // FIXME: The receiver could be a reference to a class, meaning that // we should use the class method. - return getInstanceMethodSummary(msg, ID); + // id x = [NSObject class]; + // [x performSelector:... withObject:... afterDelay:...]; + Selector S = Msg.getSelector(); + const ObjCMethodDecl *Method = Msg.getDecl(); + if (!Method && ReceiverClass) + Method = ReceiverClass->getInstanceMethod(S); + + return getMethodSummary(S, ReceiverClass, Method, Msg.getResultType(), + ObjCMethodSummaries); } const RetainSummary * -RetainSummaryManager::getMethodSummary(Selector S, IdentifierInfo *ClsName, - const ObjCInterfaceDecl *ID, +RetainSummaryManager::getMethodSummary(Selector S, const ObjCInterfaceDecl *ID, const ObjCMethodDecl *MD, QualType RetTy, ObjCMethodSummariesTy &CachedSummaries) { // Look up a summary in our summary cache. - const RetainSummary *Summ = CachedSummaries.find(ID, ClsName, S); + const RetainSummary *Summ = CachedSummaries.find(ID, S); if (!Summ) { Summ = getStandardMethodSummary(MD, S, RetTy); @@ -1413,7 +1516,7 @@ RetainSummaryManager::getMethodSummary(Selector S, IdentifierInfo *ClsName, updateSummaryFromAnnotations(Summ, MD); // Memoize the summary. - CachedSummaries[ObjCSummaryKey(ID, ClsName, S)] = Summ; + CachedSummaries[ObjCSummaryKey(ID, S)] = Summ; } return Summ; @@ -1430,29 +1533,6 @@ void RetainSummaryManager::InitializeClassMethodSummaries() { addClassMethSummary("NSAutoreleasePool", "addObject", getPersistentSummary(RetEffect::MakeNoRet(), DoNothing, Autorelease)); - - // Create the summaries for [NSObject performSelector...]. We treat - // these as 'stop tracking' for the arguments because they are often - // used for delegates that can release the object. When we have better - // inter-procedural analysis we can potentially do something better. This - // workaround is to remove false positives. - const RetainSummary *Summ = - getPersistentSummary(RetEffect::MakeNoRet(), DoNothing, StopTracking); - IdentifierInfo *NSObjectII = &Ctx.Idents.get("NSObject"); - addClsMethSummary(NSObjectII, Summ, "performSelector", "withObject", - "afterDelay", NULL); - addClsMethSummary(NSObjectII, Summ, "performSelector", "withObject", - "afterDelay", "inModes", NULL); - addClsMethSummary(NSObjectII, Summ, "performSelectorOnMainThread", - "withObject", "waitUntilDone", NULL); - addClsMethSummary(NSObjectII, Summ, "performSelectorOnMainThread", - "withObject", "waitUntilDone", "modes", NULL); - addClsMethSummary(NSObjectII, Summ, "performSelector", "onThread", - "withObject", "waitUntilDone", NULL); - addClsMethSummary(NSObjectII, Summ, "performSelector", "onThread", - "withObject", "waitUntilDone", "modes", NULL); - addClsMethSummary(NSObjectII, Summ, "performSelectorInBackground", - "withObject", NULL); } void RetainSummaryManager::InitializeMethodSummaries() { @@ -1558,6 +1638,10 @@ void RetainSummaryManager::InitializeMethodSummaries() { //===----------------------------------------------------------------------===// // AutoreleaseBindings - State used to track objects in autorelease pools. //===----------------------------------------------------------------------===// +#define AUTORELEASE_POOL_MODELING (0) +// We do not currently have complete modeling of autorelease pools. + +#if AUTORELEASE_POOL_MODELING typedef llvm::ImmutableMap<SymbolRef, unsigned> ARCounts; typedef llvm::ImmutableMap<SymbolRef, ARCounts> ARPoolContents; @@ -1605,6 +1689,7 @@ SendAutorelease(ProgramStateRef state, return state->set<AutoreleasePoolContents>(pool, newCnts); } +#endif //===----------------------------------------------------------------------===// // Error reporting. @@ -1690,32 +1775,18 @@ namespace { }; class Leak : public CFRefBug { - const bool isReturn; - protected: - Leak(StringRef name, bool isRet) - : CFRefBug(name), isReturn(isRet) { + public: + Leak(StringRef name) + : CFRefBug(name) { // Leaks should not be reported if they are post-dominated by a sink. setSuppressOnSink(true); } - public: const char *getDescription() const { return ""; } bool isLeak() const { return true; } }; - class LeakAtReturn : public Leak { - public: - LeakAtReturn(StringRef name) - : Leak(name, true) {} - }; - - class LeakWithinFunction : public Leak { - public: - LeakWithinFunction(StringRef name) - : Leak(name, false) {} - }; - //===---------===// // Bug Reports. // //===---------===// @@ -1854,25 +1925,21 @@ static inline bool contains(const SmallVectorImpl<ArgEffect>& V, return false; } -static bool isPropertyAccess(const Stmt *S, ParentMap &PM) { - unsigned maxDepth = 4; - while (S && maxDepth) { - if (const PseudoObjectExpr *PO = dyn_cast<PseudoObjectExpr>(S)) { - if (!isa<ObjCMessageExpr>(PO->getSyntacticForm())) - return true; - return false; - } - S = PM.getParent(S); - --maxDepth; - } - return false; +static bool isNumericLiteralExpression(const Expr *E) { + // FIXME: This set of cases was copied from SemaExprObjC. + return isa<IntegerLiteral>(E) || + isa<CharacterLiteral>(E) || + isa<FloatingLiteral>(E) || + isa<ObjCBoolLiteralExpr>(E) || + isa<CXXBoolLiteralExpr>(E); } PathDiagnosticPiece *CFRefReportVisitor::VisitNode(const ExplodedNode *N, const ExplodedNode *PrevN, BugReporterContext &BRC, BugReport &BR) { - + // FIXME: We will eventually need to handle non-statement-based events + // (__attribute__((cleanup))). if (!isa<StmtPoint>(N->getLocation())) return NULL; @@ -1881,11 +1948,11 @@ PathDiagnosticPiece *CFRefReportVisitor::VisitNode(const ExplodedNode *N, ProgramStateRef CurrSt = N->getState(); const LocationContext *LCtx = N->getLocationContext(); - const RefVal* CurrT = CurrSt->get<RefBindings>(Sym); + const RefVal* CurrT = getRefBinding(CurrSt, Sym); if (!CurrT) return NULL; const RefVal &CurrV = *CurrT; - const RefVal *PrevT = PrevSt->get<RefBindings>(Sym); + const RefVal *PrevT = getRefBinding(PrevSt, Sym); // Create a string buffer to constain all the useful things we want // to tell the user. @@ -1903,6 +1970,24 @@ PathDiagnosticPiece *CFRefReportVisitor::VisitNode(const ExplodedNode *N, else if (isa<ObjCDictionaryLiteral>(S)) { os << "NSDictionary literal is an object with a +0 retain count"; } + else if (const ObjCBoxedExpr *BL = dyn_cast<ObjCBoxedExpr>(S)) { + if (isNumericLiteralExpression(BL->getSubExpr())) + os << "NSNumber literal is an object with a +0 retain count"; + else { + const ObjCInterfaceDecl *BoxClass = 0; + if (const ObjCMethodDecl *Method = BL->getBoxingMethod()) + BoxClass = Method->getClassInterface(); + + // We should always be able to find the boxing class interface, + // but consider this future-proofing. + if (BoxClass) + os << *BoxClass << " b"; + else + os << "B"; + + os << "oxed expression produces an object with a +0 retain count"; + } + } else { if (const CallExpr *CE = dyn_cast<CallExpr>(S)) { // Get the name of the callee (if it is available). @@ -1913,10 +1998,22 @@ PathDiagnosticPiece *CFRefReportVisitor::VisitNode(const ExplodedNode *N, os << "function call"; } else { - assert(isa<ObjCMessageExpr>(S)); - // The message expression may have between written directly or as - // a property access. Lazily determine which case we are looking at. - os << (isPropertyAccess(S, N->getParentMap()) ? "Property" : "Method"); + assert(isa<ObjCMessageExpr>(S)); + CallEventManager &Mgr = CurrSt->getStateManager().getCallEventManager(); + CallEventRef<ObjCMethodCall> Call + = Mgr.getObjCMethodCall(cast<ObjCMessageExpr>(S), CurrSt, LCtx); + + switch (Call->getMessageKind()) { + case OCM_Message: + os << "Method"; + break; + case OCM_PropertyAccess: + os << "Property"; + break; + case OCM_Subscript: + os << "Subscript"; + break; + } } if (CurrV.getObjKind() == RetEffect::CF) { @@ -2143,9 +2240,8 @@ GetAllocationSite(ProgramStateManager& StateMgr, const ExplodedNode *N, while (N) { ProgramStateRef St = N->getState(); - RefBindings B = St->get<RefBindings>(); - if (!B.lookup(Sym)) + if (!getRefBinding(St, Sym)) break; StoreManager::FindUniqueBinding FB(Sym); @@ -2216,7 +2312,7 @@ CFRefLeakReportVisitor::getEndPath(BugReporterContext &BRC, os << "allocated object"; // Get the retain count. - const RefVal* RV = EndN->getState()->get<RefBindings>(Sym); + const RefVal* RV = getRefBinding(EndN->getState(), Sym); if (RV->getKind() == RefVal::ErrorLeakReturned) { // FIXME: Per comments in rdar://6320065, "create" only applies to CF @@ -2276,8 +2372,15 @@ CFRefLeakReport::CFRefLeakReport(CFRefBug &D, const LangOptions &LOpts, GetAllocationSite(Ctx.getStateManager(), getErrorNode(), sym); // Get the SourceLocation for the allocation site. + // FIXME: This will crash the analyzer if an allocation comes from an + // implicit call. (Currently there are no such allocations in Cocoa, though.) + const Stmt *AllocStmt; ProgramPoint P = AllocNode->getLocation(); - const Stmt *AllocStmt = cast<PostStmt>(P).getStmt(); + if (CallExitEnd *Exit = dyn_cast<CallExitEnd>(&P)) + AllocStmt = Exit->getCalleeContext()->getCallSite(); + else + AllocStmt = cast<PostStmt>(P).getStmt(); + assert(AllocStmt && "All allocations must come from explicit calls"); Location = PathDiagnosticLocation::createBegin(AllocStmt, SMgr, n->getLocationContext()); // Fill in the description of the bug. @@ -2307,11 +2410,10 @@ class RetainCountChecker check::EndPath, check::PostStmt<BlockExpr>, check::PostStmt<CastExpr>, - check::PostStmt<CallExpr>, - check::PostStmt<CXXConstructExpr>, check::PostStmt<ObjCArrayLiteral>, check::PostStmt<ObjCDictionaryLiteral>, - check::PostObjCMessage, + check::PostStmt<ObjCBoxedExpr>, + check::PostCall, check::PreStmt<ReturnStmt>, check::RegionChanges, eval::Assume, @@ -2330,7 +2432,9 @@ class RetainCountChecker mutable OwningPtr<RetainSummaryManager> Summaries; mutable OwningPtr<RetainSummaryManager> SummariesGC; +#if AUTORELEASE_POOL_MODELING mutable ARCounts::Factory ARCountFactory; +#endif mutable SummaryLogTy SummaryLog; mutable bool ShouldResetSummaryLog; @@ -2382,20 +2486,17 @@ public: bool GCEnabled) const { if (GCEnabled) { if (!leakWithinFunctionGC) - leakWithinFunctionGC.reset(new LeakWithinFunction("Leak of object when " - "using garbage " - "collection")); + leakWithinFunctionGC.reset(new Leak("Leak of object when using " + "garbage collection")); return leakWithinFunctionGC.get(); } else { if (!leakWithinFunction) { if (LOpts.getGC() == LangOptions::HybridGC) { - leakWithinFunction.reset(new LeakWithinFunction("Leak of object when " - "not using garbage " - "collection (GC) in " - "dual GC/non-GC " - "code")); + leakWithinFunction.reset(new Leak("Leak of object when not using " + "garbage collection (GC) in " + "dual GC/non-GC code")); } else { - leakWithinFunction.reset(new LeakWithinFunction("Leak")); + leakWithinFunction.reset(new Leak("Leak")); } } return leakWithinFunction.get(); @@ -2405,17 +2506,17 @@ public: CFRefBug *getLeakAtReturnBug(const LangOptions &LOpts, bool GCEnabled) const { if (GCEnabled) { if (!leakAtReturnGC) - leakAtReturnGC.reset(new LeakAtReturn("Leak of returned object when " - "using garbage collection")); + leakAtReturnGC.reset(new Leak("Leak of returned object when using " + "garbage collection")); return leakAtReturnGC.get(); } else { if (!leakAtReturn) { if (LOpts.getGC() == LangOptions::HybridGC) { - leakAtReturn.reset(new LeakAtReturn("Leak of returned object when " - "not using garbage collection " - "(GC) in dual GC/non-GC code")); + leakAtReturn.reset(new Leak("Leak of returned object when not using " + "garbage collection (GC) in dual " + "GC/non-GC code")); } else { - leakAtReturn.reset(new LeakAtReturn("Leak of returned object")); + leakAtReturn.reset(new Leak("Leak of returned object")); } } return leakAtReturn.get(); @@ -2453,13 +2554,13 @@ public: void checkPostStmt(const BlockExpr *BE, CheckerContext &C) const; void checkPostStmt(const CastExpr *CE, CheckerContext &C) const; - void checkPostStmt(const CallExpr *CE, CheckerContext &C) const; - void checkPostStmt(const CXXConstructExpr *CE, CheckerContext &C) const; void checkPostStmt(const ObjCArrayLiteral *AL, CheckerContext &C) const; void checkPostStmt(const ObjCDictionaryLiteral *DL, CheckerContext &C) const; - void checkPostObjCMessage(const ObjCMessage &Msg, CheckerContext &C) const; + void checkPostStmt(const ObjCBoxedExpr *BE, CheckerContext &C) const; + + void checkPostCall(const CallEvent &Call, CheckerContext &C) const; - void checkSummary(const RetainSummary &Summ, const CallOrObjCMessage &Call, + void checkSummary(const RetainSummary &Summ, const CallEvent &Call, CheckerContext &C) const; bool evalCall(const CallExpr *CE, CheckerContext &C) const; @@ -2472,7 +2573,7 @@ public: const StoreManager::InvalidatedSymbols *invalidated, ArrayRef<const MemRegion *> ExplicitRegions, ArrayRef<const MemRegion *> Regions, - const CallOrObjCMessage *Call) const; + const CallEvent *Call) const; bool wantsRegionChangeUpdate(ProgramStateRef state) const { return true; @@ -2499,8 +2600,8 @@ public: const ProgramPointTag *getDeadSymbolTag(SymbolRef sym) const; ProgramStateRef handleSymbolDeath(ProgramStateRef state, - SymbolRef sid, RefVal V, - SmallVectorImpl<SymbolRef> &Leaked) const; + SymbolRef sid, RefVal V, + SmallVectorImpl<SymbolRef> &Leaked) const; std::pair<ExplodedNode *, ProgramStateRef > handleAutoreleaseCounts(ProgramStateRef state, @@ -2597,7 +2698,7 @@ void RetainCountChecker::checkPostStmt(const CastExpr *CE, SymbolRef Sym = state->getSVal(CE, C.getLocationContext()).getAsLocSymbol(); if (!Sym) return; - const RefVal* T = state->get<RefBindings>(Sym); + const RefVal* T = getRefBinding(state, Sym); if (!T) return; @@ -2613,54 +2714,6 @@ void RetainCountChecker::checkPostStmt(const CastExpr *CE, C.addTransition(state); } -void RetainCountChecker::checkPostStmt(const CallExpr *CE, - CheckerContext &C) const { - if (C.wasInlined) - return; - - // Get the callee. - ProgramStateRef state = C.getState(); - const Expr *Callee = CE->getCallee(); - SVal L = state->getSVal(Callee, C.getLocationContext()); - - RetainSummaryManager &Summaries = getSummaryManager(C); - const RetainSummary *Summ = 0; - - // FIXME: Better support for blocks. For now we stop tracking anything - // that is passed to blocks. - // FIXME: Need to handle variables that are "captured" by the block. - if (dyn_cast_or_null<BlockDataRegion>(L.getAsRegion())) { - Summ = Summaries.getPersistentStopSummary(); - } else if (const FunctionDecl *FD = L.getAsFunctionDecl()) { - Summ = Summaries.getSummary(FD); - } else if (const CXXMemberCallExpr *me = dyn_cast<CXXMemberCallExpr>(CE)) { - if (const CXXMethodDecl *MD = me->getMethodDecl()) - Summ = Summaries.getSummary(MD); - } - - if (!Summ) - Summ = Summaries.getDefaultSummary(); - - checkSummary(*Summ, CallOrObjCMessage(CE, state, C.getLocationContext()), C); -} - -void RetainCountChecker::checkPostStmt(const CXXConstructExpr *CE, - CheckerContext &C) const { - const CXXConstructorDecl *Ctor = CE->getConstructor(); - if (!Ctor) - return; - - RetainSummaryManager &Summaries = getSummaryManager(C); - const RetainSummary *Summ = Summaries.getSummary(Ctor); - - // If we didn't get a summary, this constructor doesn't affect retain counts. - if (!Summ) - return; - - ProgramStateRef state = C.getState(); - checkSummary(*Summ, CallOrObjCMessage(CE, state, C.getLocationContext()), C); -} - void RetainCountChecker::processObjCLiterals(CheckerContext &C, const Expr *Ex) const { ProgramStateRef state = C.getState(); @@ -2670,7 +2723,7 @@ void RetainCountChecker::processObjCLiterals(CheckerContext &C, const Stmt *child = *it; SVal V = state->getSVal(child, pred->getLocationContext()); if (SymbolRef sym = V.getAsSymbol()) - if (const RefVal* T = state->get<RefBindings>(sym)) { + if (const RefVal* T = getRefBinding(state, sym)) { RefVal::Kind hasErr = (RefVal::Kind) 0; state = updateSymbol(state, sym, *T, MayEscape, hasErr, C); if (hasErr) { @@ -2685,8 +2738,8 @@ void RetainCountChecker::processObjCLiterals(CheckerContext &C, if (SymbolRef sym = state->getSVal(Ex, pred->getLocationContext()).getAsSymbol()) { QualType ResultTy = Ex->getType(); - state = state->set<RefBindings>(sym, RefVal::makeNotOwned(RetEffect::ObjC, - ResultTy)); + state = setRefBinding(state, sym, + RefVal::makeNotOwned(RetEffect::ObjC, ResultTy)); } C.addTransition(state); @@ -2704,30 +2757,34 @@ void RetainCountChecker::checkPostStmt(const ObjCDictionaryLiteral *DL, processObjCLiterals(C, DL); } -void RetainCountChecker::checkPostObjCMessage(const ObjCMessage &Msg, - CheckerContext &C) const { - ProgramStateRef state = C.getState(); - - RetainSummaryManager &Summaries = getSummaryManager(C); +void RetainCountChecker::checkPostStmt(const ObjCBoxedExpr *Ex, + CheckerContext &C) const { + const ExplodedNode *Pred = C.getPredecessor(); + const LocationContext *LCtx = Pred->getLocationContext(); + ProgramStateRef State = Pred->getState(); - const RetainSummary *Summ; - if (Msg.isInstanceMessage()) { - const LocationContext *LC = C.getLocationContext(); - Summ = Summaries.getInstanceMethodSummary(Msg, state, LC); - } else { - Summ = Summaries.getClassMethodSummary(Msg); + if (SymbolRef Sym = State->getSVal(Ex, LCtx).getAsSymbol()) { + QualType ResultTy = Ex->getType(); + State = setRefBinding(State, Sym, + RefVal::makeNotOwned(RetEffect::ObjC, ResultTy)); } - // If we didn't get a summary, this message doesn't affect retain counts. - if (!Summ) + C.addTransition(State); +} + +void RetainCountChecker::checkPostCall(const CallEvent &Call, + CheckerContext &C) const { + if (C.wasInlined) return; - checkSummary(*Summ, CallOrObjCMessage(Msg, state, C.getLocationContext()), C); + RetainSummaryManager &Summaries = getSummaryManager(C); + const RetainSummary *Summ = Summaries.getSummary(Call, C.getState()); + checkSummary(*Summ, Call, C); } /// GetReturnType - Used to get the return type of a message expression or /// function call with the intention of affixing that type to a tracked symbol. -/// While the the return type can be queried directly from RetEx, when +/// While the return type can be queried directly from RetEx, when /// invoking class methods we augment to the return type to be that of /// a pointer to the class (as opposed it just being id). // FIXME: We may be able to do this with related result types instead. @@ -2754,7 +2811,7 @@ static QualType GetReturnType(const Expr *RetE, ASTContext &Ctx) { } void RetainCountChecker::checkSummary(const RetainSummary &Summ, - const CallOrObjCMessage &CallOrMsg, + const CallEvent &CallOrMsg, CheckerContext &C) const { ProgramStateRef state = C.getState(); @@ -2767,7 +2824,7 @@ void RetainCountChecker::checkSummary(const RetainSummary &Summ, SVal V = CallOrMsg.getArgSVal(idx); if (SymbolRef Sym = V.getAsLocSymbol()) { - if (RefBindings::data_type *T = state->get<RefBindings>(Sym)) { + if (const RefVal *T = getRefBinding(state, Sym)) { state = updateSymbol(state, Sym, *T, Summ.getArg(idx), hasErr, C); if (hasErr) { ErrorRange = CallOrMsg.getArgSourceRange(idx); @@ -2780,17 +2837,18 @@ void RetainCountChecker::checkSummary(const RetainSummary &Summ, // Evaluate the effect on the message receiver. bool ReceiverIsTracked = false; - if (!hasErr && CallOrMsg.isObjCMessage()) { - const LocationContext *LC = C.getLocationContext(); - SVal Receiver = CallOrMsg.getInstanceMessageReceiver(LC); - if (SymbolRef Sym = Receiver.getAsLocSymbol()) { - if (const RefVal *T = state->get<RefBindings>(Sym)) { - ReceiverIsTracked = true; - state = updateSymbol(state, Sym, *T, Summ.getReceiverEffect(), - hasErr, C); - if (hasErr) { - ErrorRange = CallOrMsg.getReceiverSourceRange(); - ErrorSym = Sym; + if (!hasErr) { + const ObjCMethodCall *MsgInvocation = dyn_cast<ObjCMethodCall>(&CallOrMsg); + if (MsgInvocation) { + if (SymbolRef Sym = MsgInvocation->getReceiverSVal().getAsLocSymbol()) { + if (const RefVal *T = getRefBinding(state, Sym)) { + ReceiverIsTracked = true; + state = updateSymbol(state, Sym, *T, Summ.getReceiverEffect(), + hasErr, C); + if (hasErr) { + ErrorRange = MsgInvocation->getOriginExpr()->getReceiverRange(); + ErrorSym = Sym; + } } } } @@ -2827,11 +2885,11 @@ void RetainCountChecker::checkSummary(const RetainSummary &Summ, if (!Sym) break; - // Use the result type from callOrMsg as it automatically adjusts + // Use the result type from the CallEvent as it automatically adjusts // for methods/functions that return references. - QualType ResultTy = CallOrMsg.getResultType(C.getASTContext()); - state = state->set<RefBindings>(Sym, RefVal::makeOwned(RE.getObjKind(), - ResultTy)); + QualType ResultTy = CallOrMsg.getResultType(); + state = setRefBinding(state, Sym, RefVal::makeOwned(RE.getObjKind(), + ResultTy)); // FIXME: Add a flag to the checker where allocations are assumed to // *not* fail. (The code below is out-of-date, though.) @@ -2856,8 +2914,8 @@ void RetainCountChecker::checkSummary(const RetainSummary &Summ, // Use GetReturnType in order to give [NSFoo alloc] the type NSFoo *. QualType ResultTy = GetReturnType(Ex, C.getASTContext()); - state = state->set<RefBindings>(Sym, RefVal::makeNotOwned(RE.getObjKind(), - ResultTy)); + state = setRefBinding(state, Sym, RefVal::makeNotOwned(RE.getObjKind(), + ResultTy)); break; } } @@ -2895,25 +2953,37 @@ RetainCountChecker::updateSymbol(ProgramStateRef state, SymbolRef sym, IgnoreRetainMsg = (bool)C.getASTContext().getLangOpts().ObjCAutoRefCount; switch (E) { - default: break; - case IncRefMsg: E = IgnoreRetainMsg ? DoNothing : IncRef; break; - case DecRefMsg: E = IgnoreRetainMsg ? DoNothing : DecRef; break; - case MakeCollectable: E = C.isObjCGCEnabled() ? DecRef : DoNothing; break; - case NewAutoreleasePool: E = C.isObjCGCEnabled() ? DoNothing : - NewAutoreleasePool; break; + default: + break; + case IncRefMsg: + E = IgnoreRetainMsg ? DoNothing : IncRef; + break; + case DecRefMsg: + E = IgnoreRetainMsg ? DoNothing : DecRef; + break; + case DecRefMsgAndStopTracking: + E = IgnoreRetainMsg ? StopTracking : DecRefAndStopTracking; + break; + case MakeCollectable: + E = C.isObjCGCEnabled() ? DecRef : DoNothing; + break; + case NewAutoreleasePool: + E = C.isObjCGCEnabled() ? DoNothing : NewAutoreleasePool; + break; } // Handle all use-after-releases. if (!C.isObjCGCEnabled() && V.getKind() == RefVal::Released) { V = V ^ RefVal::ErrorUseAfterRelease; hasErr = V.getKind(); - return state->set<RefBindings>(sym, V); + return setRefBinding(state, sym, V); } switch (E) { case DecRefMsg: case IncRefMsg: case MakeCollectable: + case DecRefMsgAndStopTracking: llvm_unreachable("DecRefMsg/IncRefMsg/MakeCollectable already converted"); case Dealloc: @@ -2931,7 +3001,7 @@ RetainCountChecker::updateSymbol(ProgramStateRef state, SymbolRef sym, // The object immediately transitions to the released state. V = V ^ RefVal::Released; V.clearCounts(); - return state->set<RefBindings>(sym, V); + return setRefBinding(state, sym, V); case RefVal::NotOwned: V = V ^ RefVal::ErrorDeallocNotOwned; hasErr = V.getKind(); @@ -2941,7 +3011,10 @@ RetainCountChecker::updateSymbol(ProgramStateRef state, SymbolRef sym, case NewAutoreleasePool: assert(!C.isObjCGCEnabled()); - return state->add<AutoreleaseStack>(sym); +#if AUTORELEASE_POOL_MODELING + state = state->add<AutoreleaseStack>(sym); +#endif + return state; case MayEscape: if (V.getKind() == RefVal::Owned) { @@ -2959,12 +3032,16 @@ RetainCountChecker::updateSymbol(ProgramStateRef state, SymbolRef sym, return state; // Update the autorelease counts. + // TODO: AutoreleasePoolContents are not currently used. We will need to + // call SendAutorelease after it's wired up. +#if AUTORELEASE_POOL_MODELING state = SendAutorelease(state, ARCountFactory, sym); +#endif V = V.autorelease(); break; case StopTracking: - return state->remove<RefBindings>(sym); + return removeRefBinding(state, sym); case IncRef: switch (V.getKind()) { @@ -2982,11 +3059,9 @@ RetainCountChecker::updateSymbol(ProgramStateRef state, SymbolRef sym, } break; - case SelfOwn: - V = V ^ RefVal::NotOwned; - // Fall-through. case DecRef: case DecRefBridgedTransfered: + case DecRefAndStopTracking: switch (V.getKind()) { default: // case 'RefVal::Released' handled above. @@ -2997,13 +3072,18 @@ RetainCountChecker::updateSymbol(ProgramStateRef state, SymbolRef sym, if (V.getCount() == 1) V = V ^ (E == DecRefBridgedTransfered ? RefVal::NotOwned : RefVal::Released); + else if (E == DecRefAndStopTracking) + return removeRefBinding(state, sym); + V = V - 1; break; case RefVal::NotOwned: - if (V.getCount() > 0) + if (V.getCount() > 0) { + if (E == DecRefAndStopTracking) + return removeRefBinding(state, sym); V = V - 1; - else { + } else { V = V ^ RefVal::ErrorReleaseNotOwned; hasErr = V.getKind(); } @@ -3018,7 +3098,7 @@ RetainCountChecker::updateSymbol(ProgramStateRef state, SymbolRef sym, } break; } - return state->set<RefBindings>(sym, V); + return setRefBinding(state, sym, V); } void RetainCountChecker::processNonLeakError(ProgramStateRef St, @@ -3117,7 +3197,7 @@ bool RetainCountChecker::evalCall(const CallExpr *CE, CheckerContext &C) const { // If the receiver is unknown, conjure a return value. SValBuilder &SVB = C.getSValBuilder(); unsigned Count = C.getCurrentBlockCount(); - SVal RetVal = SVB.getConjuredSymbolVal(0, CE, LCtx, ResultTy, Count); + RetVal = SVB.getConjuredSymbolVal(0, CE, LCtx, ResultTy, Count); } state = state->BindExpr(CE, LCtx, RetVal, false); @@ -3126,9 +3206,9 @@ bool RetainCountChecker::evalCall(const CallExpr *CE, CheckerContext &C) const { if (const MemRegion *ArgRegion = RetVal.getAsRegion()) { // Save the refcount status of the argument. SymbolRef Sym = RetVal.getAsLocSymbol(); - RefBindings::data_type *Binding = 0; + const RefVal *Binding = 0; if (Sym) - Binding = state->get<RefBindings>(Sym); + Binding = getRefBinding(state, Sym); // Invalidate the argument region. unsigned Count = C.getCurrentBlockCount(); @@ -3136,7 +3216,7 @@ bool RetainCountChecker::evalCall(const CallExpr *CE, CheckerContext &C) const { // Restore the refcount status of the argument. if (Binding) - state = state->set<RefBindings>(Sym, *Binding); + state = setRefBinding(state, Sym, *Binding); } C.addTransition(state); @@ -3175,7 +3255,7 @@ void RetainCountChecker::checkPreStmt(const ReturnStmt *S, return; // Get the reference count binding (if any). - const RefVal *T = state->get<RefBindings>(Sym); + const RefVal *T = getRefBinding(state, Sym); if (!T) return; @@ -3208,7 +3288,7 @@ void RetainCountChecker::checkPreStmt(const ReturnStmt *S, } // Update the binding. - state = state->set<RefBindings>(Sym, X); + state = setRefBinding(state, Sym, X); ExplodedNode *Pred = C.addTransition(state); // At this point we have updated the state properly. @@ -3230,29 +3310,27 @@ void RetainCountChecker::checkPreStmt(const ReturnStmt *S, return; // Get the updated binding. - T = state->get<RefBindings>(Sym); + T = getRefBinding(state, Sym); assert(T); X = *T; // Consult the summary of the enclosing method. RetainSummaryManager &Summaries = getSummaryManager(C); const Decl *CD = &Pred->getCodeDecl(); + RetEffect RE = RetEffect::MakeNoRet(); + // FIXME: What is the convention for blocks? Is there one? if (const ObjCMethodDecl *MD = dyn_cast<ObjCMethodDecl>(CD)) { - // Unlike regular functions, /all/ ObjC methods are assumed to always - // follow Cocoa retain-count conventions, not just those with special - // names or attributes. const RetainSummary *Summ = Summaries.getMethodSummary(MD); - RetEffect RE = Summ ? Summ->getRetEffect() : RetEffect::MakeNoRet(); - checkReturnWithRetEffect(S, C, Pred, RE, X, Sym, state); + RE = Summ->getRetEffect(); + } else if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(CD)) { + if (!isa<CXXMethodDecl>(FD)) { + const RetainSummary *Summ = Summaries.getFunctionSummary(FD); + RE = Summ->getRetEffect(); + } } - if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(CD)) { - if (!isa<CXXMethodDecl>(FD)) - if (const RetainSummary *Summ = Summaries.getSummary(FD)) - checkReturnWithRetEffect(S, C, Pred, Summ->getRetEffect(), X, - Sym, state); - } + checkReturnWithRetEffect(S, C, Pred, RE, X, Sym, state); } void RetainCountChecker::checkReturnWithRetEffect(const ReturnStmt *S, @@ -3283,7 +3361,7 @@ void RetainCountChecker::checkReturnWithRetEffect(const ReturnStmt *S, if (hasError) { // Generate an error node. - state = state->set<RefBindings>(Sym, X); + state = setRefBinding(state, Sym, X); static SimpleProgramPointTag ReturnOwnLeakTag("RetainCountChecker : ReturnsOwnLeak"); @@ -3303,7 +3381,7 @@ void RetainCountChecker::checkReturnWithRetEffect(const ReturnStmt *S, if (RE.isOwned()) { // Trying to return a not owned object to a caller expecting an // owned object. - state = state->set<RefBindings>(Sym, X ^ RefVal::ErrorReturnedNotOwned); + state = setRefBinding(state, Sym, X ^ RefVal::ErrorReturnedNotOwned); static SimpleProgramPointTag ReturnNotOwnedTag("RetainCountChecker : ReturnNotOwnedForOwned"); @@ -3346,7 +3424,11 @@ void RetainCountChecker::checkBind(SVal loc, SVal val, const Stmt *S, // 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). - escapes = (state == (state->bindLoc(*regionLoc, val))); + // 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 (!escapes) { // Case 4: We do not currently model what happens when a symbol is @@ -3406,7 +3488,7 @@ RetainCountChecker::checkRegionChanges(ProgramStateRef state, const StoreManager::InvalidatedSymbols *invalidated, ArrayRef<const MemRegion *> ExplicitRegions, ArrayRef<const MemRegion *> Regions, - const CallOrObjCMessage *Call) const { + const CallEvent *Call) const { if (!invalidated) return state; @@ -3423,7 +3505,7 @@ RetainCountChecker::checkRegionChanges(ProgramStateRef state, if (WhitelistedSymbols.count(sym)) continue; // Remove any existing reference-count binding. - state = state->remove<RefBindings>(sym); + state = removeRefBinding(state, sym); } return state; } @@ -3463,7 +3545,7 @@ RetainCountChecker::handleAutoreleaseCounts(ProgramStateRef state, V.setCount(Cnt - ACnt); V.setAutoreleaseCount(0); } - state = state->set<RefBindings>(Sym, V); + state = setRefBinding(state, Sym, V); ExplodedNode *N = Bd.MakeNode(state, Pred); if (N == 0) state = 0; @@ -3473,7 +3555,7 @@ RetainCountChecker::handleAutoreleaseCounts(ProgramStateRef state, // Woah! More autorelease counts then retain counts left. // Emit hard error. V = V ^ RefVal::ErrorOverAutorelease; - state = state->set<RefBindings>(Sym, V); + state = setRefBinding(state, Sym, V); if (ExplodedNode *N = Bd.MakeNode(state, Pred, true)) { SmallString<128> sbuf; @@ -3507,10 +3589,10 @@ RetainCountChecker::handleSymbolDeath(ProgramStateRef state, hasLeak = (V.getCount() > 0); if (!hasLeak) - return state->remove<RefBindings>(sid); + return removeRefBinding(state, sid); Leaked.push_back(sid); - return state->set<RefBindings>(sid, V ^ RefVal::ErrorLeak); + return setRefBinding(state, sid, V ^ RefVal::ErrorLeak); } ExplodedNode * @@ -3559,7 +3641,7 @@ void RetainCountChecker::checkEndPath(CheckerContext &Ctx) const { // If the current LocationContext has a parent, don't check for leaks. // We will do that later. - // FIXME: we should instead check for imblances of the retain/releases, + // FIXME: we should instead check for imbalances of the retain/releases, // and suggest annotations. if (Ctx.getLocationContext()->getParent()) return; @@ -3641,6 +3723,7 @@ void RetainCountChecker::checkDeadSymbols(SymbolReaper &SymReaper, // Debug printing of refcount bindings and autorelease pools. //===----------------------------------------------------------------------===// +#if AUTORELEASE_POOL_MODELING static void PrintPool(raw_ostream &Out, SymbolRef Sym, ProgramStateRef State) { Out << ' '; @@ -3654,7 +3737,6 @@ static void PrintPool(raw_ostream &Out, SymbolRef Sym, if (const ARCounts *Cnts = State->get<AutoreleasePoolContents>(Sym)) for (ARCounts::iterator I = Cnts->begin(), E = Cnts->end(); I != E; ++I) Out << '(' << I.getKey() << ',' << I.getData() << ')'; - Out << '}'; } @@ -3664,6 +3746,7 @@ static bool UsesAutorelease(ProgramStateRef state) { return !state->get<AutoreleaseStack>().isEmpty() || state->get<AutoreleasePoolContents>(SymbolRef()); } +#endif void RetainCountChecker::printState(raw_ostream &Out, ProgramStateRef State, const char *NL, const char *Sep) const { @@ -3679,6 +3762,7 @@ void RetainCountChecker::printState(raw_ostream &Out, ProgramStateRef State, Out << NL; } +#if AUTORELEASE_POOL_MODELING // Print the autorelease stack. if (UsesAutorelease(State)) { Out << Sep << NL << "AR pool stack:"; @@ -3690,6 +3774,7 @@ void RetainCountChecker::printState(raw_ostream &Out, ProgramStateRef State, Out << NL; } +#endif } //===----------------------------------------------------------------------===// diff --git a/lib/StaticAnalyzer/Checkers/ReturnUndefChecker.cpp b/lib/StaticAnalyzer/Checkers/ReturnUndefChecker.cpp index 7b1f0b1..ca2a55d 100644 --- a/lib/StaticAnalyzer/Checkers/ReturnUndefChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/ReturnUndefChecker.cpp @@ -53,9 +53,9 @@ void ReturnUndefChecker::checkPreStmt(const ReturnStmt *RS, BugReport *report = new BugReport(*BT, BT->getDescription(), N); + report->disablePathPruning(); report->addRange(RetE->getSourceRange()); - report->addVisitor(bugreporter::getTrackNullOrUndefValueVisitor(N, RetE, - report)); + bugreporter::addTrackNullOrUndefValueVisitor(N, RetE, report); C.EmitReport(report); } diff --git a/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/lib/StaticAnalyzer/Checkers/StreamChecker.cpp index 3745d4a..731dd66 100644 --- a/lib/StaticAnalyzer/Checkers/StreamChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/StreamChecker.cpp @@ -116,7 +116,7 @@ namespace ento { bool StreamChecker::evalCall(const CallExpr *CE, CheckerContext &C) const { const FunctionDecl *FD = C.getCalleeDecl(CE); - if (!FD) + if (!FD || FD->getKind() != Decl::Function) return false; ASTContext &Ctx = C.getASTContext(); diff --git a/lib/StaticAnalyzer/Checkers/TraversalChecker.cpp b/lib/StaticAnalyzer/Checkers/TraversalChecker.cpp new file mode 100644 index 0000000..b97cd6c --- /dev/null +++ b/lib/StaticAnalyzer/Checkers/TraversalChecker.cpp @@ -0,0 +1,84 @@ +//== TraversalChecker.cpp -------------------------------------- -*- C++ -*--=// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +// +// These checkers print various aspects of the ExprEngine's traversal of the CFG +// as it builds the ExplodedGraph. +// +//===----------------------------------------------------------------------===// +#include "ClangSACheckers.h" +#include "clang/AST/ParentMap.h" +#include "clang/AST/StmtObjC.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" + +using namespace clang; +using namespace ento; + +namespace { +class TraversalDumper : public Checker< check::BranchCondition, + check::EndPath > { +public: + void checkBranchCondition(const Stmt *Condition, CheckerContext &C) const; + void checkEndPath(CheckerContext &C) const; +}; +} + +void TraversalDumper::checkBranchCondition(const Stmt *Condition, + CheckerContext &C) const { + // Special-case Objective-C's for-in loop, which uses the entire loop as its + // condition. We just print the collection expression. + const Stmt *Parent = dyn_cast<ObjCForCollectionStmt>(Condition); + if (!Parent) { + const ParentMap &Parents = C.getLocationContext()->getParentMap(); + Parent = Parents.getParent(Condition); + } + + // It is mildly evil to print directly to llvm::outs() rather than emitting + // warnings, but this ensures things do not get filtered out by the rest of + // the static analyzer machinery. + SourceLocation Loc = Parent->getLocStart(); + llvm::outs() << C.getSourceManager().getSpellingLineNumber(Loc) << " " + << Parent->getStmtClassName() << "\n"; +} + +void TraversalDumper::checkEndPath(CheckerContext &C) const { + llvm::outs() << "--END PATH--\n"; +} + +void ento::registerTraversalDumper(CheckerManager &mgr) { + mgr.registerChecker<TraversalDumper>(); +} + +//------------------------------------------------------------------------------ + +namespace { +class CallDumper : public Checker< check::PreCall > { +public: + void checkPreCall(const CallEvent &Call, CheckerContext &C) const; +}; +} + +void CallDumper::checkPreCall(const CallEvent &Call, CheckerContext &C) const { + unsigned Indentation = 0; + for (const LocationContext *LC = C.getLocationContext()->getParent(); + LC != 0; LC = LC->getParent()) + ++Indentation; + + // It is mildly evil to print directly to llvm::outs() rather than emitting + // warnings, but this ensures things do not get filtered out by the rest of + // the static analyzer machinery. + llvm::outs().indent(Indentation); + Call.dump(llvm::outs()); +} + +void ento::registerCallDumper(CheckerManager &mgr) { + mgr.registerChecker<CallDumper>(); +} diff --git a/lib/StaticAnalyzer/Checkers/UndefBranchChecker.cpp b/lib/StaticAnalyzer/Checkers/UndefBranchChecker.cpp index a30f6d5..70a33c7 100644 --- a/lib/StaticAnalyzer/Checkers/UndefBranchChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/UndefBranchChecker.cpp @@ -99,8 +99,9 @@ void UndefBranchChecker::checkBranchCondition(const Stmt *Condition, // Emit the bug report. BugReport *R = new BugReport(*BT, BT->getDescription(), N); - R->addVisitor(bugreporter::getTrackNullOrUndefValueVisitor(N, Ex, R)); + bugreporter::addTrackNullOrUndefValueVisitor(N, Ex, R); R->addRange(Ex->getSourceRange()); + R->disablePathPruning(); Ctx.EmitReport(R); } diff --git a/lib/StaticAnalyzer/Checkers/UndefCapturedBlockVarChecker.cpp b/lib/StaticAnalyzer/Checkers/UndefCapturedBlockVarChecker.cpp index d57767e..675b38a 100644 --- a/lib/StaticAnalyzer/Checkers/UndefCapturedBlockVarChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/UndefCapturedBlockVarChecker.cpp @@ -94,6 +94,7 @@ UndefCapturedBlockVarChecker::checkPostStmt(const BlockExpr *BE, if (const Expr *Ex = FindBlockDeclRefExpr(BE->getBody(), VD)) R->addRange(Ex->getSourceRange()); R->addVisitor(new FindLastStoreBRVisitor(VRVal, VR)); + R->disablePathPruning(); // need location of block C.EmitReport(R); } diff --git a/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp b/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp index c3c9ed7..e220499 100644 --- a/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp @@ -76,12 +76,12 @@ void UndefResultChecker::checkPostStmt(const BinaryOperator *B, BugReport *report = new BugReport(*BT, OS.str(), N); if (Ex) { report->addRange(Ex->getSourceRange()); - report->addVisitor(bugreporter::getTrackNullOrUndefValueVisitor(N, Ex, - report)); + bugreporter::addTrackNullOrUndefValueVisitor(N, Ex, report); } else - report->addVisitor(bugreporter::getTrackNullOrUndefValueVisitor(N, B, - report)); + bugreporter::addTrackNullOrUndefValueVisitor(N, B, report); + + report->disablePathPruning(); C.EmitReport(report); } } diff --git a/lib/StaticAnalyzer/Checkers/UndefinedArraySubscriptChecker.cpp b/lib/StaticAnalyzer/Checkers/UndefinedArraySubscriptChecker.cpp index 0297c4e..6ae3c18 100644 --- a/lib/StaticAnalyzer/Checkers/UndefinedArraySubscriptChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/UndefinedArraySubscriptChecker.cpp @@ -42,9 +42,7 @@ UndefinedArraySubscriptChecker::checkPreStmt(const ArraySubscriptExpr *A, // Generate a report for this bug. BugReport *R = new BugReport(*BT, BT->getName(), N); R->addRange(A->getIdx()->getSourceRange()); - R->addVisitor(bugreporter::getTrackNullOrUndefValueVisitor(N, - A->getIdx(), - R)); + bugreporter::addTrackNullOrUndefValueVisitor(N, A->getIdx(), R); C.EmitReport(R); } } diff --git a/lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp b/lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp index 78f7fa6..14a884e 100644 --- a/lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp @@ -78,8 +78,9 @@ void UndefinedAssignmentChecker::checkBind(SVal location, SVal val, BugReport *R = new BugReport(*BT, str, N); if (ex) { R->addRange(ex->getSourceRange()); - R->addVisitor(bugreporter::getTrackNullOrUndefValueVisitor(N, ex, R)); + bugreporter::addTrackNullOrUndefValueVisitor(N, ex, R); } + R->disablePathPruning(); C.EmitReport(R); } diff --git a/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp b/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp index 60e665fe..d35455c 100644 --- a/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp @@ -224,8 +224,7 @@ bool UnixAPIChecker::ReportZeroByteAllocation(CheckerContext &C, BugReport *report = new BugReport(*BT_mallocZero, os.str(), N); report->addRange(arg->getSourceRange()); - report->addVisitor(bugreporter::getTrackNullOrUndefValueVisitor(N, arg, - report)); + bugreporter::addTrackNullOrUndefValueVisitor(N, arg, report); C.EmitReport(report); return true; @@ -256,7 +255,7 @@ void UnixAPIChecker::BasicAllocationCheck(CheckerContext &C, (void) ReportZeroByteAllocation(C, falseState, arg, fn); return; } - // Assume the the value is non-zero going forward. + // Assume the value is non-zero going forward. assert(trueState); if (trueState != state) C.addTransition(trueState); @@ -292,7 +291,7 @@ void UnixAPIChecker::CheckCallocZero(CheckerContext &C, } } - // Assume the the value is non-zero going forward. + // Assume the value is non-zero going forward. assert(trueState); if (trueState != state) C.addTransition(trueState); @@ -325,7 +324,11 @@ void UnixAPIChecker::CheckVallocZero(CheckerContext &C, void UnixAPIChecker::checkPreStmt(const CallExpr *CE, CheckerContext &C) const { - StringRef FName = C.getCalleeName(CE); + const FunctionDecl *FD = C.getCalleeDecl(CE); + if (!FD || FD->getKind() != Decl::Function) + return; + + StringRef FName = C.getCalleeName(FD); if (FName.empty()) return; diff --git a/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp b/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp index 38c9cc1..fab4adf 100644 --- a/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp @@ -69,8 +69,7 @@ void VLASizeChecker::reportBug(VLASize_Kind Kind, BugReport *report = new BugReport(*BT, os.str(), N); report->addRange(SizeE->getSourceRange()); - report->addVisitor(bugreporter::getTrackNullOrUndefValueVisitor(N, SizeE, - report)); + bugreporter::addTrackNullOrUndefValueVisitor(N, SizeE, report); C.EmitReport(report); return; } diff --git a/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp b/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp index f7c7c0c..bdc9627 100644 --- a/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp @@ -46,7 +46,7 @@ class WalkAST : public StmtVisitor<WalkAST> { visited. */ PostVisited /**< A CallExpr to this FunctionDecl is in the worklist, and the body has been visited. */ - } K; + }; /// A DenseMap that records visited states of FunctionDecls. llvm::DenseMap<const FunctionDecl *, Kind> VisitedFunctions; |
