diff options
Diffstat (limited to 'lib/StaticAnalyzer')
35 files changed, 1858 insertions, 715 deletions
diff --git a/lib/StaticAnalyzer/Checkers/AllocationDiagnostics.cpp b/lib/StaticAnalyzer/Checkers/AllocationDiagnostics.cpp new file mode 100644 index 0000000..3dec8a5 --- /dev/null +++ b/lib/StaticAnalyzer/Checkers/AllocationDiagnostics.cpp @@ -0,0 +1,24 @@ +//=- AllocationDiagnostics.cpp - Config options for allocation diags *- C++ -*-// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +// +// Declares the configuration functions for leaks/allocation diagnostics. +// +//===-------------------------- + +#include "AllocationDiagnostics.h" + +namespace clang { +namespace ento { + +bool shouldIncludeAllocationSiteInLeakDiagnostics(AnalyzerOptions &AOpts) { + return AOpts.getBooleanOption("leak-diagnostics-reference-allocation", + false); +} + +}} diff --git a/lib/StaticAnalyzer/Checkers/AllocationDiagnostics.h b/lib/StaticAnalyzer/Checkers/AllocationDiagnostics.h new file mode 100644 index 0000000..2b314a3 --- /dev/null +++ b/lib/StaticAnalyzer/Checkers/AllocationDiagnostics.h @@ -0,0 +1,31 @@ +//=--- AllocationDiagnostics.h - Config options for allocation diags *- C++ -*-// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +// +// Declares the configuration functions for leaks/allocation diagnostics. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_SA_LIB_CHECKERS_ALLOC_DIAGS_H +#define LLVM_CLANG_SA_LIB_CHECKERS_ALLOC_DIAGS_H + +#include "clang/StaticAnalyzer/Core/AnalyzerOptions.h" + +namespace clang { namespace ento { + +/// \brief Returns true if leak diagnostics should directly reference +/// the allocatin site (where possible). +/// +/// The default is false. +/// +bool shouldIncludeAllocationSiteInLeakDiagnostics(AnalyzerOptions &AOpts); + +}} + +#endif + diff --git a/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp b/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp index 533a324..fba14a0 100644 --- a/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp +++ b/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp @@ -123,18 +123,29 @@ void NilArgChecker::WarnIfNilArg(CheckerContext &C, if (Class == FC_NSArray) { os << "Array element cannot be nil"; } else if (Class == FC_NSDictionary) { - if (Arg == 0) - os << "Dictionary object cannot be nil"; - else { + if (Arg == 0) { + os << "Value stored into '"; + os << GetReceiverInterfaceName(msg) << "' cannot be nil"; + } else { assert(Arg == 1); - os << "Dictionary key cannot be nil"; + os << "'"<< GetReceiverInterfaceName(msg) << "' key cannot be nil"; } } else llvm_unreachable("Missing foundation class for the subscript expr"); } else { - os << "Argument to '" << GetReceiverInterfaceName(msg) << "' method '" - << msg.getSelector().getAsString() << "' cannot be nil"; + if (Class == FC_NSDictionary) { + if (Arg == 0) + os << "Value argument "; + else { + assert(Arg == 1); + os << "Key argument "; + } + os << "to '" << msg.getSelector().getAsString() << "' cannot be nil"; + } else { + os << "Argument to '" << GetReceiverInterfaceName(msg) << "' method '" + << msg.getSelector().getAsString() << "' cannot be nil"; + } } BugReport *R = new BugReport(*BT, os.str(), N); @@ -377,7 +388,7 @@ void CFNumberCreateChecker::checkPreStmt(const CallExpr *CE, // FIXME: If the pointee isn't an integer type, should we flag a warning? // People can do weird stuff with pointers. - if (!T->isIntegerType()) + if (!T->isIntegralOrEnumerationType()) return; uint64_t SourceSize = Ctx.getTypeSize(T); @@ -748,38 +759,81 @@ static bool isKnownNonNilCollectionType(QualType T) { } } -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; - +/// Assumes that the collection is non-nil. +/// +/// If the collection is known to be nil, returns NULL to indicate an infeasible +/// path. +static ProgramStateRef checkCollectionNonNil(CheckerContext &C, + ProgramStateRef State, + const ObjCForCollectionStmt *FCS) { + if (!State) + return NULL; + + SVal CollectionVal = C.getSVal(FCS->getCollection()); + Optional<DefinedSVal> KnownCollection = CollectionVal.getAs<DefinedSVal>(); + if (!KnownCollection) + return State; + + ProgramStateRef StNonNil, StNil; + llvm::tie(StNonNil, StNil) = State->assume(*KnownCollection); + if (StNil && !StNonNil) { + // The collection is nil. This path is infeasible. + return NULL; + } + + return StNonNil; +} + +/// Assumes that the collection elements are non-nil. +/// +/// This only applies if the collection is one of those known not to contain +/// nil values. +static ProgramStateRef checkElementNonNil(CheckerContext &C, + ProgramStateRef State, + const ObjCForCollectionStmt *FCS) { + if (!State) + return NULL; + // 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. + if (!isKnownNonNilCollectionType(FCS->getCollection()->getType())) + return State; + + const LocationContext *LCtx = C.getLocationContext(); const Stmt *Element = FCS->getElement(); - SVal ElementVar; + + // FIXME: Copied from ExprEngineObjC. + Optional<Loc> ElementLoc; 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()); + ElementLoc = State->getLValue(ElemDecl, LCtx); } else { - ElementVar = State->getSVal(Element, C.getLocationContext()); + ElementLoc = State->getSVal(Element, LCtx).getAs<Loc>(); } - if (!ElementVar.getAs<Loc>()) - return; + if (!ElementLoc) + return State; // Go ahead and assume the value is non-nil. - SVal Val = State->getSVal(ElementVar.castAs<Loc>()); - State = State->assume(Val.castAs<DefinedOrUnknownSVal>(), true); - C.addTransition(State); + SVal Val = State->getSVal(*ElementLoc); + return State->assume(Val.castAs<DefinedOrUnknownSVal>(), true); +} + +void ObjCLoopChecker::checkPostStmt(const ObjCForCollectionStmt *FCS, + CheckerContext &C) const { + // Check if this is the branch for the end of the loop. + SVal CollectionSentinel = C.getSVal(FCS); + if (CollectionSentinel.isZeroConstant()) + return; + + ProgramStateRef State = C.getState(); + State = checkCollectionNonNil(C, State, FCS); + State = checkElementNonNil(C, State, FCS); + + if (!State) + C.generateSink(); + else if (State != C.getState()) + C.addTransition(State); } namespace { diff --git a/lib/StaticAnalyzer/Checkers/CMakeLists.txt b/lib/StaticAnalyzer/Checkers/CMakeLists.txt index b7df10e..7da6825 100644 --- a/lib/StaticAnalyzer/Checkers/CMakeLists.txt +++ b/lib/StaticAnalyzer/Checkers/CMakeLists.txt @@ -4,6 +4,7 @@ clang_tablegen(Checkers.inc -gen-clang-sa-checkers TARGET ClangSACheckers) add_clang_library(clangStaticAnalyzerCheckers + AllocationDiagnostics.cpp AnalyzerStatsChecker.cpp ArrayBoundChecker.cpp ArrayBoundCheckerV2.cpp @@ -42,8 +43,8 @@ add_clang_library(clangStaticAnalyzerCheckers MallocSizeofChecker.cpp NSAutoreleasePoolChecker.cpp NSErrorChecker.cpp - NonNullParamChecker.cpp NoReturnFunctionChecker.cpp + NonNullParamChecker.cpp ObjCAtSyncChecker.cpp ObjCContainersASTChecker.cpp ObjCContainersChecker.cpp diff --git a/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/lib/StaticAnalyzer/Checkers/CStringChecker.cpp index cc55e9f..aa1ca6f 100644 --- a/lib/StaticAnalyzer/Checkers/CStringChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/CStringChecker.cpp @@ -114,6 +114,8 @@ public: bool isBounded = false, bool ignoreCase = false) const; + void evalStrsep(CheckerContext &C, const CallExpr *CE) const; + // Utility methods std::pair<ProgramStateRef , ProgramStateRef > static assumeZero(CheckerContext &C, @@ -1752,6 +1754,63 @@ void CStringChecker::evalStrcmpCommon(CheckerContext &C, const CallExpr *CE, C.addTransition(state); } +void CStringChecker::evalStrsep(CheckerContext &C, const CallExpr *CE) const { + //char *strsep(char **stringp, const char *delim); + if (CE->getNumArgs() < 2) + return; + + // Sanity: does the search string parameter match the return type? + const Expr *SearchStrPtr = CE->getArg(0); + QualType CharPtrTy = SearchStrPtr->getType()->getPointeeType(); + if (CharPtrTy.isNull() || + CE->getType().getUnqualifiedType() != CharPtrTy.getUnqualifiedType()) + return; + + CurrentFunctionDescription = "strsep()"; + ProgramStateRef State = C.getState(); + const LocationContext *LCtx = C.getLocationContext(); + + // Check that the search string pointer is non-null (though it may point to + // a null string). + SVal SearchStrVal = State->getSVal(SearchStrPtr, LCtx); + State = checkNonNull(C, State, SearchStrPtr, SearchStrVal); + if (!State) + return; + + // Check that the delimiter string is non-null. + const Expr *DelimStr = CE->getArg(1); + SVal DelimStrVal = State->getSVal(DelimStr, LCtx); + State = checkNonNull(C, State, DelimStr, DelimStrVal); + if (!State) + return; + + SValBuilder &SVB = C.getSValBuilder(); + SVal Result; + if (Optional<Loc> SearchStrLoc = SearchStrVal.getAs<Loc>()) { + // Get the current value of the search string pointer, as a char*. + Result = State->getSVal(*SearchStrLoc, CharPtrTy); + + // Invalidate the search string, representing the change of one delimiter + // character to NUL. + State = InvalidateBuffer(C, State, SearchStrPtr, Result); + + // Overwrite the search string pointer. The new value is either an address + // further along in the same string, or NULL if there are no more tokens. + State = State->bindLoc(*SearchStrLoc, + SVB.conjureSymbolVal(getTag(), CE, LCtx, CharPtrTy, + C.blockCount())); + } else { + assert(SearchStrVal.isUnknown()); + // Conjure a symbolic value. It's the best we can do. + Result = SVB.conjureSymbolVal(0, CE, LCtx, C.blockCount()); + } + + // Set the return value, and finish. + State = State->BindExpr(CE, LCtx, Result); + C.addTransition(State); +} + + //===----------------------------------------------------------------------===// // The driver method, and other Checker callbacks. //===----------------------------------------------------------------------===// @@ -1762,6 +1821,7 @@ bool CStringChecker::evalCall(const CallExpr *CE, CheckerContext &C) const { if (!FDecl) return false; + // FIXME: Poorly-factored string switches are slow. FnCheck evalFunction = 0; if (C.isCLibraryFunction(FDecl, "memcpy")) evalFunction = &CStringChecker::evalMemcpy; @@ -1793,6 +1853,8 @@ bool CStringChecker::evalCall(const CallExpr *CE, CheckerContext &C) const { evalFunction = &CStringChecker::evalStrcasecmp; else if (C.isCLibraryFunction(FDecl, "strncasecmp")) evalFunction = &CStringChecker::evalStrncasecmp; + else if (C.isCLibraryFunction(FDecl, "strsep")) + evalFunction = &CStringChecker::evalStrsep; else if (C.isCLibraryFunction(FDecl, "bcopy")) evalFunction = &CStringChecker::evalBcopy; else if (C.isCLibraryFunction(FDecl, "bcmp")) diff --git a/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp b/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp index 3a57a56..92c0eef 100644 --- a/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp @@ -101,6 +101,8 @@ public: // - strncat(dst, src, sizeof(dst) - 1); // - strncat(dst, src, sizeof(dst)); bool WalkAST::containsBadStrncatPattern(const CallExpr *CE) { + if (CE->getNumArgs() != 3) + return false; const Expr *DstArg = CE->getArg(0); const Expr *SrcArg = CE->getArg(1); const Expr *LenArg = CE->getArg(2); diff --git a/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp b/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp index 7ef13ab..63080ea 100644 --- a/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp +++ b/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp @@ -345,7 +345,7 @@ void WalkAST::checkCall_getpw(const CallExpr *CE, const FunctionDecl *FD) { return; // Verify the first argument type is integer. - if (!FPT->getArgType(0)->isIntegerType()) + if (!FPT->getArgType(0)->isIntegralOrUnscopedEnumerationType()) return; // Verify the second argument type is char*. @@ -602,7 +602,7 @@ void WalkAST::checkCall_rand(const CallExpr *CE, const FunctionDecl *FD) { if (!PT) return; - if (! PT->getPointeeType()->isIntegerType()) + if (! PT->getPointeeType()->isIntegralOrUnscopedEnumerationType()) return; } else if (FTP->getNumArgs() != 0) @@ -725,7 +725,7 @@ void WalkAST::checkUncheckedReturnValue(CallExpr *CE) { // The arguments must be integers. for (unsigned i = 0; i < FTP->getNumArgs(); i++) - if (! FTP->getArgType(i)->isIntegerType()) + if (! FTP->getArgType(i)->isIntegralOrUnscopedEnumerationType()) return; // Issue a warning. diff --git a/lib/StaticAnalyzer/Checkers/Checkers.td b/lib/StaticAnalyzer/Checkers/Checkers.td index 3db3fb9..fc35b22 100644 --- a/lib/StaticAnalyzer/Checkers/Checkers.td +++ b/lib/StaticAnalyzer/Checkers/Checkers.td @@ -167,6 +167,11 @@ def ReturnUndefChecker : Checker<"UndefReturn">, //===----------------------------------------------------------------------===// let ParentPackage = Cplusplus in { + +def NewDeleteChecker : Checker<"NewDelete">, + HelpText<"Check for double-free and use-after-free problems. Traces memory managed by new/delete.">, + DescFile<"MallocChecker.cpp">; + } // end: "cplusplus" let ParentPackage = CplusplusAlpha in { @@ -175,8 +180,8 @@ def VirtualCallChecker : Checker<"VirtualCall">, HelpText<"Check virtual function calls during construction or destruction">, DescFile<"VirtualCallChecker.cpp">; -def NewDeleteChecker : Checker<"NewDelete">, - HelpText<"Check for memory leaks, double free, and use-after-free problems. Traces memory managed by new/delete.">, +def NewDeleteLeaksChecker : Checker<"NewDeleteLeaks">, + HelpText<"Check for memory leaks. Traces memory managed by new/delete.">, DescFile<"MallocChecker.cpp">; } // end: "alpha.cplusplus" diff --git a/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp b/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp index f2e3e6d..f336a6e 100644 --- a/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp @@ -101,7 +101,8 @@ void ReachableCode::computeReachableBlocks() { } } -static const Expr *LookThroughTransitiveAssignments(const Expr *Ex) { +static const Expr * +LookThroughTransitiveAssignmentsAndCommaOperators(const Expr *Ex) { while (Ex) { const BinaryOperator *BO = dyn_cast<BinaryOperator>(Ex->IgnoreParenCasts()); @@ -111,6 +112,10 @@ static const Expr *LookThroughTransitiveAssignments(const Expr *Ex) { Ex = BO->getRHS(); continue; } + if (BO->getOpcode() == BO_Comma) { + Ex = BO->getRHS(); + continue; + } break; } return Ex; @@ -266,7 +271,9 @@ public: if (VarDecl *VD = dyn_cast<VarDecl>(DR->getDecl())) { // Special case: check for assigning null to a pointer. // This is a common form of defensive programming. - const Expr *RHS = LookThroughTransitiveAssignments(B->getRHS()); + const Expr *RHS = + LookThroughTransitiveAssignmentsAndCommaOperators(B->getRHS()); + RHS = RHS->IgnoreParenCasts(); QualType T = VD->getType(); if (T->isPointerType() || T->isObjCObjectPointerType()) { @@ -274,7 +281,6 @@ public: return; } - RHS = RHS->IgnoreParenCasts(); // Special case: self-assignments. These are often used to shut up // "unused variable" compiler warnings. if (const DeclRefExpr *RhsDR = dyn_cast<DeclRefExpr>(RHS)) @@ -326,7 +332,7 @@ public: // Look through transitive assignments, e.g.: // int x = y = 0; - E = LookThroughTransitiveAssignments(E); + E = LookThroughTransitiveAssignmentsAndCommaOperators(E); // Don't warn on C++ objects (yet) until we can show that their // constructors/destructors don't have side effects. diff --git a/lib/StaticAnalyzer/Checkers/DebugCheckers.cpp b/lib/StaticAnalyzer/Checkers/DebugCheckers.cpp index 29b4a63..fe12866 100644 --- a/lib/StaticAnalyzer/Checkers/DebugCheckers.cpp +++ b/lib/StaticAnalyzer/Checkers/DebugCheckers.cpp @@ -7,7 +7,7 @@ // //===----------------------------------------------------------------------===// // -// This file defines a checkers that display debugging information. +// This file defines checkers that display debugging information. // //===----------------------------------------------------------------------===// diff --git a/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp b/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp index 9f176a4..759aa66 100644 --- a/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp +++ b/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp @@ -27,7 +27,8 @@ namespace { class DynamicTypePropagation: public Checker< check::PreCall, check::PostCall, - check::PostStmt<ImplicitCastExpr> > { + check::PostStmt<ImplicitCastExpr>, + check::PostStmt<CXXNewExpr> > { const ObjCObjectType *getObjectTypeForAllocAndNew(const ObjCMessageExpr *MsgE, CheckerContext &C) const; @@ -38,6 +39,7 @@ public: void checkPreCall(const CallEvent &Call, CheckerContext &C) const; void checkPostCall(const CallEvent &Call, CheckerContext &C) const; void checkPostStmt(const ImplicitCastExpr *CastE, CheckerContext &C) const; + void checkPostStmt(const CXXNewExpr *NewE, CheckerContext &C) const; }; } @@ -190,6 +192,20 @@ void DynamicTypePropagation::checkPostStmt(const ImplicitCastExpr *CastE, return; } +void DynamicTypePropagation::checkPostStmt(const CXXNewExpr *NewE, + CheckerContext &C) const { + if (NewE->isArray()) + return; + + // We only track dynamic type info for regions. + const MemRegion *MR = C.getSVal(NewE).getAsRegion(); + if (!MR) + return; + + C.addTransition(C.getState()->setDynamicTypeInfo(MR, NewE->getType(), + /*CanBeSubclass=*/false)); +} + const ObjCObjectType * DynamicTypePropagation::getObjectTypeForAllocAndNew(const ObjCMessageExpr *MsgE, CheckerContext &C) const { diff --git a/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp b/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp index 5ed28e9..cc940be 100644 --- a/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp @@ -437,6 +437,7 @@ visit(const ObjCImplementationDecl *ImplD) const { // Remove ivars invalidated by the partial invalidation methods. They do not // need to be invalidated in the regular invalidation methods. + bool AtImplementationContainsAtLeastOnePartialInvalidationMethod = false; for (MethodSet::iterator I = PartialInfo.InvalidationMethods.begin(), E = PartialInfo.InvalidationMethods.end(); I != E; ++I) { @@ -446,6 +447,8 @@ visit(const ObjCImplementationDecl *ImplD) const { const ObjCMethodDecl *D = ImplD->getMethod(InterfD->getSelector(), InterfD->isInstanceMethod()); if (D && D->hasBody()) { + AtImplementationContainsAtLeastOnePartialInvalidationMethod = true; + bool CalledAnotherInvalidationMethod = false; // The MethodCrowler is going to remove the invalidated ivars. MethodCrawler(Ivars, @@ -471,7 +474,7 @@ visit(const ObjCImplementationDecl *ImplD) const { containsInvalidationMethod(InterfaceD, Info, /*LookForPartial*/ false); // Report an error in case none of the invalidation methods are declared. - if (!Info.needsInvalidation()) { + if (!Info.needsInvalidation() && !PartialInfo.needsInvalidation()) { if (Filter.check_MissingInvalidationMethod) reportNoInvalidationMethod(FirstIvarDecl, IvarToPopertyMap, InterfaceD, /*MissingDeclaration*/ true); @@ -520,9 +523,19 @@ visit(const ObjCImplementationDecl *ImplD) const { } // Report an error in case none of the invalidation methods are implemented. - if (!AtImplementationContainsAtLeastOneInvalidationMethod) - reportNoInvalidationMethod(FirstIvarDecl, IvarToPopertyMap, InterfaceD, - /*MissingDeclaration*/ false); + if (!AtImplementationContainsAtLeastOneInvalidationMethod) { + if (AtImplementationContainsAtLeastOnePartialInvalidationMethod) { + // Warn on the ivars that were not invalidated by the prrtial + // invalidation methods. + for (IvarSet::const_iterator + I = Ivars.begin(), E = Ivars.end(); I != E; ++I) + reportIvarNeedsInvalidation(I->first, IvarToPopertyMap, 0); + } else { + // Otherwise, no invalidation methods were implemented. + reportNoInvalidationMethod(FirstIvarDecl, IvarToPopertyMap, InterfaceD, + /*MissingDeclaration*/ false); + } + } } void IvarInvalidationCheckerImpl:: @@ -551,19 +564,27 @@ reportNoInvalidationMethod(const ObjCIvarDecl *FirstIvarDecl, void IvarInvalidationCheckerImpl:: reportIvarNeedsInvalidation(const ObjCIvarDecl *IvarD, - const IvarToPropMapTy &IvarToPopertyMap, - const ObjCMethodDecl *MethodD) const { + const IvarToPropMapTy &IvarToPopertyMap, + const ObjCMethodDecl *MethodD) const { SmallString<128> sbuf; llvm::raw_svector_ostream os(sbuf); printIvar(os, IvarD, IvarToPopertyMap); os << "needs to be invalidated or set to nil"; - PathDiagnosticLocation MethodDecLocation = - PathDiagnosticLocation::createEnd(MethodD->getBody(), - BR.getSourceManager(), - Mgr.getAnalysisDeclContext(MethodD)); - BR.EmitBasicReport(MethodD, "Incomplete invalidation", - categories::CoreFoundationObjectiveC, os.str(), - MethodDecLocation); + if (MethodD) { + PathDiagnosticLocation MethodDecLocation = + PathDiagnosticLocation::createEnd(MethodD->getBody(), + BR.getSourceManager(), + Mgr.getAnalysisDeclContext(MethodD)); + BR.EmitBasicReport(MethodD, "Incomplete invalidation", + categories::CoreFoundationObjectiveC, os.str(), + MethodDecLocation); + } else { + BR.EmitBasicReport(IvarD, "Incomplete invalidation", + categories::CoreFoundationObjectiveC, os.str(), + PathDiagnosticLocation::createBegin(IvarD, + BR.getSourceManager())); + + } } void IvarInvalidationCheckerImpl::MethodCrawler::markInvalidated( diff --git a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp index 4b0e766..5d3eb65 100644 --- a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -50,7 +50,12 @@ class RefState { Released, // The responsibility for freeing resources has transfered from // this reference. A relinquished symbol should not be freed. - Relinquished }; + Relinquished, + // We are no longer guaranteed to have observed all manipulations + // of this pointer/memory. For example, it could have been + // passed as a parameter to an opaque function. + Escaped + }; const Stmt *S; unsigned K : 2; // Kind enum, but stored as a bitfield. @@ -58,12 +63,15 @@ class RefState { // family. RefState(Kind k, const Stmt *s, unsigned family) - : S(s), K(k), Family(family) {} + : S(s), K(k), Family(family) { + assert(family != AF_None); + } public: bool isAllocated() const { return K == Allocated; } bool isReleased() const { return K == Released; } bool isRelinquished() const { return K == Relinquished; } - AllocationFamily getAllocationFamily() const { + bool isEscaped() const { return K == Escaped; } + AllocationFamily getAllocationFamily() const { return (AllocationFamily)Family; } const Stmt *getStmt() const { return S; } @@ -81,6 +89,9 @@ public: static RefState getRelinquished(unsigned family, const Stmt *s) { return RefState(Relinquished, s, family); } + static RefState getEscaped(const RefState *RS) { + return RefState(Escaped, RS->getStmt(), RS->getAllocationFamily()); + } void Profile(llvm::FoldingSetNodeID &ID) const { ID.AddInteger(K); @@ -136,7 +147,7 @@ class MallocChecker : public Checker<check::DeadSymbols, check::PointerEscape, check::ConstPointerEscape, check::PreStmt<ReturnStmt>, - check::PreStmt<CallExpr>, + check::PreCall, check::PostStmt<CallExpr>, check::PostStmt<CXXNewExpr>, check::PreStmt<CXXDeleteExpr>, @@ -164,12 +175,13 @@ public: DefaultBool CMallocPessimistic; DefaultBool CMallocOptimistic; DefaultBool CNewDeleteChecker; + DefaultBool CNewDeleteLeaksChecker; DefaultBool CMismatchedDeallocatorChecker; }; ChecksFilter Filter; - void checkPreStmt(const CallExpr *S, CheckerContext &C) const; + void checkPreCall(const CallEvent &Call, CheckerContext &C) const; void checkPostStmt(const CallExpr *CE, CheckerContext &C) const; void checkPostStmt(const CXXNewExpr *NE, CheckerContext &C) const; void checkPreStmt(const CXXDeleteExpr *DE, CheckerContext &C) const; @@ -282,19 +294,20 @@ private: PointerEscapeKind Kind, bool(*CheckRefState)(const RefState*)) const; - // Used to suppress warnings if they are not related to the tracked family - // (derived from AllocDeallocStmt). - bool isTrackedFamily(AllocationFamily Family) const; - bool isTrackedFamily(CheckerContext &C, const Stmt *AllocDeallocStmt) const; - bool isTrackedFamily(CheckerContext &C, SymbolRef Sym) const; - + ///@{ + /// Tells if a given family/call/symbol is tracked by the current checker. + bool isTrackedByCurrentChecker(AllocationFamily Family) const; + bool isTrackedByCurrentChecker(CheckerContext &C, + const Stmt *AllocDeallocStmt) const; + bool isTrackedByCurrentChecker(CheckerContext &C, SymbolRef Sym) const; + ///@} static bool SummarizeValue(raw_ostream &os, SVal V); static bool SummarizeRegion(raw_ostream &os, const MemRegion *MR); void ReportBadFree(CheckerContext &C, SVal ArgVal, SourceRange Range, const Expr *DeallocExpr) const; void ReportMismatchedDealloc(CheckerContext &C, SourceRange Range, - const Expr *DeallocExpr, - const RefState *RS) const; + const Expr *DeallocExpr, const RefState *RS, + SymbolRef Sym) const; void ReportOffsetFree(CheckerContext &C, SVal ArgVal, SourceRange Range, const Expr *DeallocExpr, const Expr *AllocExpr = 0) const; @@ -1007,36 +1020,37 @@ ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C, if (RsBase) { - bool DeallocMatchesAlloc = - RsBase->getAllocationFamily() == AF_None || - RsBase->getAllocationFamily() == getAllocationFamily(C, ParentExpr); - - // Check if an expected deallocation function matches the real one. - if (!DeallocMatchesAlloc && RsBase->isAllocated()) { - ReportMismatchedDealloc(C, ArgExpr->getSourceRange(), ParentExpr, RsBase); - return 0; - } - - // Check double free. - if (DeallocMatchesAlloc && - (RsBase->isReleased() || RsBase->isRelinquished()) && + // Check for double free first. + if ((RsBase->isReleased() || RsBase->isRelinquished()) && !didPreviousFreeFail(State, SymBase, PreviousRetStatusSymbol)) { ReportDoubleFree(C, ParentExpr->getSourceRange(), RsBase->isReleased(), SymBase, PreviousRetStatusSymbol); return 0; - } - // Check if the memory location being freed is the actual location - // allocated, or an offset. - RegionOffset Offset = R->getAsOffset(); - if (RsBase->isAllocated() && - Offset.isValid() && - !Offset.hasSymbolicOffset() && - Offset.getOffset() != 0) { - const Expr *AllocExpr = cast<Expr>(RsBase->getStmt()); - ReportOffsetFree(C, ArgVal, ArgExpr->getSourceRange(), ParentExpr, - AllocExpr); - return 0; + // If the pointer is allocated or escaped, but we are now trying to free it, + // check that the call to free is proper. + } else if (RsBase->isAllocated() || RsBase->isEscaped()) { + + // Check if an expected deallocation function matches the real one. + bool DeallocMatchesAlloc = + RsBase->getAllocationFamily() == getAllocationFamily(C, ParentExpr); + if (!DeallocMatchesAlloc) { + ReportMismatchedDealloc(C, ArgExpr->getSourceRange(), + ParentExpr, RsBase, SymBase); + return 0; + } + + // Check if the memory location being freed is the actual location + // allocated, or an offset. + RegionOffset Offset = R->getAsOffset(); + if (Offset.isValid() && + !Offset.hasSymbolicOffset() && + Offset.getOffset() != 0) { + const Expr *AllocExpr = cast<Expr>(RsBase->getStmt()); + ReportOffsetFree(C, ArgVal, ArgExpr->getSourceRange(), ParentExpr, + AllocExpr); + return 0; + } } } @@ -1056,7 +1070,8 @@ ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C, } } - AllocationFamily Family = RsBase ? RsBase->getAllocationFamily() : AF_None; + AllocationFamily Family = RsBase ? RsBase->getAllocationFamily() + : getAllocationFamily(C, ParentExpr); // Normal free. if (Hold) return State->set<RegionState>(SymBase, @@ -1067,7 +1082,7 @@ ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C, RefState::getReleased(Family, ParentExpr)); } -bool MallocChecker::isTrackedFamily(AllocationFamily Family) const { +bool MallocChecker::isTrackedByCurrentChecker(AllocationFamily Family) const { switch (Family) { case AF_Malloc: { if (!Filter.CMallocOptimistic && !Filter.CMallocPessimistic) @@ -1081,22 +1096,24 @@ bool MallocChecker::isTrackedFamily(AllocationFamily Family) const { return true; } case AF_None: { - return true; + llvm_unreachable("no family"); } } llvm_unreachable("unhandled family"); } -bool MallocChecker::isTrackedFamily(CheckerContext &C, - const Stmt *AllocDeallocStmt) const { - return isTrackedFamily(getAllocationFamily(C, AllocDeallocStmt)); +bool +MallocChecker::isTrackedByCurrentChecker(CheckerContext &C, + const Stmt *AllocDeallocStmt) const { + return isTrackedByCurrentChecker(getAllocationFamily(C, AllocDeallocStmt)); } -bool MallocChecker::isTrackedFamily(CheckerContext &C, SymbolRef Sym) const { - const RefState *RS = C.getState()->get<RegionState>(Sym); +bool MallocChecker::isTrackedByCurrentChecker(CheckerContext &C, + SymbolRef Sym) const { - return RS ? isTrackedFamily(RS->getAllocationFamily()) - : isTrackedFamily(AF_None); + const RefState *RS = C.getState()->get<RegionState>(Sym); + assert(RS); + return isTrackedByCurrentChecker(RS->getAllocationFamily()); } bool MallocChecker::SummarizeValue(raw_ostream &os, SVal V) { @@ -1194,7 +1211,7 @@ void MallocChecker::ReportBadFree(CheckerContext &C, SVal ArgVal, !Filter.CNewDeleteChecker) return; - if (!isTrackedFamily(C, DeallocExpr)) + if (!isTrackedByCurrentChecker(C, DeallocExpr)) return; if (ExplodedNode *N = C.generateSink()) { @@ -1236,7 +1253,8 @@ void MallocChecker::ReportBadFree(CheckerContext &C, SVal ArgVal, void MallocChecker::ReportMismatchedDealloc(CheckerContext &C, SourceRange Range, const Expr *DeallocExpr, - const RefState *RS) const { + const RefState *RS, + SymbolRef Sym) const { if (!Filter.CMismatchedDeallocatorChecker) return; @@ -1266,7 +1284,9 @@ void MallocChecker::ReportMismatchedDealloc(CheckerContext &C, os << ", not " << DeallocOs.str(); BugReport *R = new BugReport(*BT_MismatchedDealloc, os.str(), N); + R->markInteresting(Sym); R->addRange(Range); + R->addVisitor(new MallocBugVisitor(Sym)); C.emitReport(R); } } @@ -1279,7 +1299,7 @@ void MallocChecker::ReportOffsetFree(CheckerContext &C, SVal ArgVal, !Filter.CNewDeleteChecker) return; - if (!isTrackedFamily(C, AllocExpr)) + if (!isTrackedByCurrentChecker(C, AllocExpr)) return; ExplodedNode *N = C.generateSink(); @@ -1331,7 +1351,7 @@ void MallocChecker::ReportUseAfterFree(CheckerContext &C, SourceRange Range, !Filter.CNewDeleteChecker) return; - if (!isTrackedFamily(C, Sym)) + if (!isTrackedByCurrentChecker(C, Sym)) return; if (ExplodedNode *N = C.generateSink()) { @@ -1356,7 +1376,7 @@ void MallocChecker::ReportDoubleFree(CheckerContext &C, SourceRange Range, !Filter.CNewDeleteChecker) return; - if (!isTrackedFamily(C, Sym)) + if (!isTrackedByCurrentChecker(C, Sym)) return; if (ExplodedNode *N = C.generateSink()) { @@ -1510,13 +1530,19 @@ MallocChecker::getAllocationSite(const ExplodedNode *N, SymbolRef Sym, // Find the most recent expression bound to the symbol in the current // context. - if (!ReferenceRegion) { - if (const MemRegion *MR = C.getLocationRegionIfPostStore(N)) { - SVal Val = State->getSVal(MR); - if (Val.getAsLocSymbol() == Sym) - ReferenceRegion = MR; + if (!ReferenceRegion) { + if (const MemRegion *MR = C.getLocationRegionIfPostStore(N)) { + SVal Val = State->getSVal(MR); + if (Val.getAsLocSymbol() == Sym) { + const VarRegion* VR = MR->getBaseRegion()->getAs<VarRegion>(); + // Do not show local variables belonging to a function other than + // where the error is reported. + if (!VR || + (VR->getStackFrame() == LeakContext->getCurrentStackFrame())) + ReferenceRegion = MR; + } + } } - } // Allocation node, is the last node in the current context in which the // symbol was tracked. @@ -1532,12 +1558,21 @@ void MallocChecker::reportLeak(SymbolRef Sym, ExplodedNode *N, CheckerContext &C) const { if (!Filter.CMallocOptimistic && !Filter.CMallocPessimistic && - !Filter.CNewDeleteChecker) + !Filter.CNewDeleteLeaksChecker) return; - if (!isTrackedFamily(C, Sym)) + const RefState *RS = C.getState()->get<RegionState>(Sym); + assert(RS && "cannot leak an untracked symbol"); + AllocationFamily Family = RS->getAllocationFamily(); + if (!isTrackedByCurrentChecker(Family)) return; + // Special case for new and new[]; these are controlled by a separate checker + // flag so that they can be selectively disabled. + if (Family == AF_CXXNew || Family == AF_CXXNewArray) + if (!Filter.CNewDeleteLeaksChecker) + return; + assert(N); if (!BT_Leak) { BT_Leak.reset(new BugType("Memory leak", "Memory Error")); @@ -1570,11 +1605,11 @@ 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 && Region->canPrintPretty()) { - os << " of memory pointed to by '"; + os << "Potential leak of memory pointed to by "; Region->printPretty(os); - os << '\''; + } else { + os << "Potential memory leak"; } BugReport *R = new BugReport(*BT_Leak, os.str(), N, @@ -1638,26 +1673,39 @@ void MallocChecker::checkDeadSymbols(SymbolReaper &SymReaper, C.addTransition(state->set<RegionState>(RS), N); } -void MallocChecker::checkPreStmt(const CallExpr *CE, CheckerContext &C) const { +void MallocChecker::checkPreCall(const CallEvent &Call, + CheckerContext &C) const { + // We will check for double free in the post visit. - if ((Filter.CMallocOptimistic || Filter.CMallocPessimistic) && - isFreeFunction(C.getCalleeDecl(CE), C.getASTContext())) - return; + if (const AnyFunctionCall *FC = dyn_cast<AnyFunctionCall>(&Call)) { + const FunctionDecl *FD = FC->getDecl(); + if (!FD) + return; - if (Filter.CNewDeleteChecker && - isStandardNewDelete(C.getCalleeDecl(CE), C.getASTContext())) - return; + if ((Filter.CMallocOptimistic || Filter.CMallocPessimistic) && + isFreeFunction(FD, C.getASTContext())) + return; - // Check use after free, when a freed pointer is passed to a call. - ProgramStateRef State = C.getState(); - for (CallExpr::const_arg_iterator I = CE->arg_begin(), - E = CE->arg_end(); I != E; ++I) { - const Expr *A = *I; - if (A->getType().getTypePtr()->isAnyPointerType()) { - SymbolRef Sym = C.getSVal(A).getAsSymbol(); + if (Filter.CNewDeleteChecker && + isStandardNewDelete(FD, C.getASTContext())) + return; + } + + // Check if the callee of a method is deleted. + if (const CXXInstanceCall *CC = dyn_cast<CXXInstanceCall>(&Call)) { + SymbolRef Sym = CC->getCXXThisVal().getAsSymbol(); + if (!Sym || checkUseAfterFree(Sym, C, CC->getCXXThisExpr())) + return; + } + + // Check arguments for being used after free. + for (unsigned I = 0, E = Call.getNumArgs(); I != E; ++I) { + SVal ArgSVal = Call.getArgSVal(I); + if (ArgSVal.getAs<Loc>()) { + SymbolRef Sym = ArgSVal.getAsSymbol(); if (!Sym) continue; - if (checkUseAfterFree(Sym, C, A)) + if (checkUseAfterFree(Sym, C, Call.getArgExpr(I))) return; } } @@ -1976,8 +2024,10 @@ ProgramStateRef MallocChecker::checkPointerEscapeAux(ProgramStateRef State, SymbolRef sym = *I; if (const RefState *RS = State->get<RegionState>(sym)) { - if (RS->isAllocated() && CheckRefState(RS)) + if (RS->isAllocated() && CheckRefState(RS)) { State = State->remove<RegionState>(sym); + State = State->set<RegionState>(sym, RefState::getEscaped(RS)); + } } } return State; @@ -2042,7 +2092,7 @@ MallocChecker::MallocBugVisitor::VisitNode(const ExplodedNode *N, } else if (isReleased(RS, RSPrev, S)) { Msg = "Memory is released"; StackHint = new StackHintGeneratorForSymbol(Sym, - "Returned released memory"); + "Returning; memory was released"); } else if (isRelinquished(RS, RSPrev, S)) { Msg = "Memory ownership is transfered"; StackHint = new StackHintGeneratorForSymbol(Sym, ""); @@ -2102,6 +2152,14 @@ void MallocChecker::printState(raw_ostream &Out, ProgramStateRef State, } } +void ento::registerNewDeleteLeaksChecker(CheckerManager &mgr) { + registerCStringCheckerBasic(mgr); + mgr.registerChecker<MallocChecker>()->Filter.CNewDeleteLeaksChecker = true; + // We currently treat NewDeleteLeaks checker as a subchecker of NewDelete + // checker. + mgr.registerChecker<MallocChecker>()->Filter.CNewDeleteChecker = true; +} + #define REGISTER_CHECKER(name) \ void ento::register##name(CheckerManager &mgr) {\ registerCStringCheckerBasic(mgr); \ diff --git a/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp b/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp index ce7d4cc..d29f34f 100644 --- a/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp @@ -188,7 +188,7 @@ public: for (CallExpr::const_arg_iterator ai = i->AllocCall->arg_begin(), ae = i->AllocCall->arg_end(); ai != ae; ++ai) { - if (!(*ai)->getType()->isIntegerType()) + if (!(*ai)->getType()->isIntegralOrUnscopedEnumerationType()) continue; SizeofFinder SFinder; diff --git a/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp b/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp index 79409e8..0f456ea 100644 --- a/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp @@ -37,6 +37,8 @@ #include "llvm/ADT/StringExtras.h" #include <cstdarg> +#include "AllocationDiagnostics.h" + using namespace clang; using namespace ento; using llvm::StrInStrNoCase; @@ -324,7 +326,7 @@ void RefVal::print(raw_ostream &Out) const { break; case RefVal::ErrorOverAutorelease: - Out << "Over autoreleased"; + Out << "Over-autoreleased"; break; case RefVal::ErrorReturnedNotOwned: @@ -1114,12 +1116,14 @@ RetainSummaryManager::getFunctionSummary(const FunctionDecl *FD) { // correctly. ScratchArgs = AF.add(ScratchArgs, 12, StopTracking); S = getPersistentSummary(RetEffect::MakeNoRet(), DoNothing, DoNothing); - } else if (FName == "dispatch_set_context") { + } else if (FName == "dispatch_set_context" || + FName == "xpc_connection_set_context") { // <rdar://problem/11059275> - The analyzer currently doesn't have // a good way to reason about the finalizer function for libdispatch. // If we pass a context object that is memory managed, stop tracking it. + // <rdar://problem/13783514> - Same problem, but for XPC. // FIXME: this hack should possibly go away once we can handle - // libdispatch finalizers. + // libdispatch and XPC finalizers. ScratchArgs = AF.add(ScratchArgs, 1, StopTracking); S = getPersistentSummary(RetEffect::MakeNoRet(), DoNothing, DoNothing); } else if (FName.startswith("NSLog")) { @@ -1660,10 +1664,10 @@ namespace { class OverAutorelease : public CFRefBug { public: OverAutorelease() - : CFRefBug("Object sent -autorelease too many times") {} + : CFRefBug("Object autoreleased too many times") {} const char *getDescription() const { - return "Object sent -autorelease too many times"; + return "Object autoreleased too many times"; } }; @@ -1773,11 +1777,11 @@ namespace { class CFRefLeakReport : public CFRefReport { const MemRegion* AllocBinding; - public: CFRefLeakReport(CFRefBug &D, const LangOptions &LOpts, bool GCEnabled, const SummaryLogTy &Log, ExplodedNode *n, SymbolRef sym, - CheckerContext &Ctx); + CheckerContext &Ctx, + bool IncludeAllocationLine); PathDiagnosticLocation getLocation(const SourceManager &SM) const { assert(Location.isValid()); @@ -2048,7 +2052,7 @@ PathDiagnosticPiece *CFRefReportVisitor::VisitNode(const ExplodedNode *N, return 0; assert(PrevV.getAutoreleaseCount() < CurrV.getAutoreleaseCount()); - os << "Object sent -autorelease message"; + os << "Object autoreleased"; break; } @@ -2134,32 +2138,86 @@ PathDiagnosticPiece *CFRefReportVisitor::VisitNode(const ExplodedNode *N, // Find the first node in the current function context that referred to the // tracked symbol and the memory location that value was stored to. Note, the // value is only reported if the allocation occurred in the same function as -// the leak. -static std::pair<const ExplodedNode*,const MemRegion*> +// the leak. The function can also return a location context, which should be +// treated as interesting. +struct AllocationInfo { + const ExplodedNode* N; + const MemRegion *R; + const LocationContext *InterestingMethodContext; + AllocationInfo(const ExplodedNode *InN, + const MemRegion *InR, + const LocationContext *InInterestingMethodContext) : + N(InN), R(InR), InterestingMethodContext(InInterestingMethodContext) {} +}; + +static AllocationInfo GetAllocationSite(ProgramStateManager& StateMgr, const ExplodedNode *N, SymbolRef Sym) { - const ExplodedNode *Last = N; + const ExplodedNode *AllocationNode = N; + const ExplodedNode *AllocationNodeInCurrentContext = N; const MemRegion* FirstBinding = 0; const LocationContext *LeakContext = N->getLocationContext(); + // The location context of the init method called on the leaked object, if + // available. + const LocationContext *InitMethodContext = 0; + while (N) { ProgramStateRef St = N->getState(); + const LocationContext *NContext = N->getLocationContext(); if (!getRefBinding(St, Sym)) break; StoreManager::FindUniqueBinding FB(Sym); StateMgr.iterBindings(St, FB); - if (FB) FirstBinding = FB.getRegion(); + + if (FB) { + const MemRegion *R = FB.getRegion(); + const VarRegion *VR = R->getBaseRegion()->getAs<VarRegion>(); + // Do not show local variables belonging to a function other than + // where the error is reported. + if (!VR || VR->getStackFrame() == LeakContext->getCurrentStackFrame()) + FirstBinding = R; + } - // Allocation node, is the last node in the current context in which the - // symbol was tracked. - if (N->getLocationContext() == LeakContext) - Last = N; + // AllocationNode is the last node in which the symbol was tracked. + AllocationNode = N; + + // AllocationNodeInCurrentContext, is the last node in the current context + // in which the symbol was tracked. + if (NContext == LeakContext) + AllocationNodeInCurrentContext = N; + + // Find the last init that was called on the given symbol and store the + // init method's location context. + if (!InitMethodContext) + if (Optional<CallEnter> CEP = N->getLocation().getAs<CallEnter>()) { + const Stmt *CE = CEP->getCallExpr(); + if (const ObjCMessageExpr *ME = dyn_cast_or_null<ObjCMessageExpr>(CE)) { + const Stmt *RecExpr = ME->getInstanceReceiver(); + if (RecExpr) { + SVal RecV = St->getSVal(RecExpr, NContext); + if (ME->getMethodFamily() == OMF_init && RecV.getAsSymbol() == Sym) + InitMethodContext = CEP->getCalleeContext(); + } + } + } N = N->pred_empty() ? NULL : *(N->pred_begin()); } + // If we are reporting a leak of the object that was allocated with alloc, + // mark its init method as interesting. + const LocationContext *InterestingMethodContext = 0; + if (InitMethodContext) { + const ProgramPoint AllocPP = AllocationNode->getLocation(); + if (Optional<StmtPoint> SP = AllocPP.getAs<StmtPoint>()) + if (const ObjCMessageExpr *ME = SP->getStmtAs<ObjCMessageExpr>()) + if (ME->getMethodFamily() == OMF_alloc) + InterestingMethodContext = InitMethodContext; + } + // If allocation happened in a function different from the leak node context, // do not report the binding. assert(N && "Could not find allocation node"); @@ -2167,7 +2225,9 @@ GetAllocationSite(ProgramStateManager& StateMgr, const ExplodedNode *N, FirstBinding = 0; } - return std::make_pair(Last, FirstBinding); + return AllocationInfo(AllocationNodeInCurrentContext, + FirstBinding, + InterestingMethodContext); } PathDiagnosticPiece* @@ -2190,12 +2250,12 @@ CFRefLeakReportVisitor::getEndPath(BugReporterContext &BRC, // We are reporting a leak. Walk up the graph to get to the first node where // the symbol appeared, and also get the first VarDecl that tracked object // is stored to. - const ExplodedNode *AllocNode = 0; - const MemRegion* FirstBinding = 0; - - llvm::tie(AllocNode, FirstBinding) = + AllocationInfo AllocI = GetAllocationSite(BRC.getStateManager(), EndN, Sym); + const MemRegion* FirstBinding = AllocI.R; + BR.markInteresting(AllocI.InterestingMethodContext); + SourceManager& SM = BRC.getSourceManager(); // Compute an actual location for the leak. Sometimes a leak doesn't @@ -2267,8 +2327,9 @@ CFRefLeakReportVisitor::getEndPath(BugReporterContext &BRC, CFRefLeakReport::CFRefLeakReport(CFRefBug &D, const LangOptions &LOpts, bool GCEnabled, const SummaryLogTy &Log, ExplodedNode *n, SymbolRef sym, - CheckerContext &Ctx) -: CFRefReport(D, LOpts, GCEnabled, Log, n, sym, false) { + CheckerContext &Ctx, + bool IncludeAllocationLine) + : CFRefReport(D, LOpts, GCEnabled, Log, n, sym, false) { // Most bug reports are cached at the location where they occurred. // With leaks, we want to unique them by the location where they were @@ -2282,9 +2343,13 @@ CFRefLeakReport::CFRefLeakReport(CFRefBug &D, const LangOptions &LOpts, const SourceManager& SMgr = Ctx.getSourceManager(); - llvm::tie(AllocNode, AllocBinding) = // Set AllocBinding. + AllocationInfo AllocI = GetAllocationSite(Ctx.getStateManager(), getErrorNode(), sym); + AllocNode = AllocI.N; + AllocBinding = AllocI.R; + markInteresting(AllocI.InterestingMethodContext); + // 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.) @@ -2295,8 +2360,17 @@ CFRefLeakReport::CFRefLeakReport(CFRefBug &D, const LangOptions &LOpts, else AllocStmt = P.castAs<PostStmt>().getStmt(); assert(AllocStmt && "All allocations must come from explicit calls"); - Location = PathDiagnosticLocation::createBegin(AllocStmt, SMgr, - n->getLocationContext()); + + PathDiagnosticLocation AllocLocation = + PathDiagnosticLocation::createBegin(AllocStmt, SMgr, + AllocNode->getLocationContext()); + Location = AllocLocation; + + // Set uniqieing info, which will be used for unique the bug reports. The + // leaks should be uniqued on the allocation site. + UniqueingLocation = AllocLocation; + UniqueingDecl = AllocNode->getLocationContext()->getDecl(); + // Fill in the description of the bug. Description.clear(); llvm::raw_string_ostream os(Description); @@ -2305,9 +2379,13 @@ CFRefLeakReport::CFRefLeakReport(CFRefBug &D, const LangOptions &LOpts, os << "(when using garbage collection) "; os << "of an object"; - // FIXME: AllocBinding doesn't get populated for RegionStore yet. - if (AllocBinding) + if (AllocBinding) { os << " stored into '" << AllocBinding->getString() << '\''; + if (IncludeAllocationLine) { + FullSourceLoc SL(AllocStmt->getLocStart(), Ctx.getSourceManager()); + os << " (allocated on line " << SL.getSpellingLineNumber() << ")"; + } + } addVisitor(new CFRefLeakReportVisitor(sym, GCEnabled, Log)); } @@ -2348,8 +2426,14 @@ class RetainCountChecker mutable SummaryLogTy SummaryLog; mutable bool ShouldResetSummaryLog; + /// Optional setting to indicate if leak reports should include + /// the allocation line. + mutable bool IncludeAllocationLine; + public: - RetainCountChecker() : ShouldResetSummaryLog(false) {} + RetainCountChecker(AnalyzerOptions &AO) + : ShouldResetSummaryLog(false), + IncludeAllocationLine(shouldIncludeAllocationSiteInLeakDiagnostics(AO)) {} virtual ~RetainCountChecker() { DeleteContainerSeconds(DeadSymbolTags); @@ -3294,7 +3378,8 @@ void RetainCountChecker::checkReturnWithRetEffect(const ReturnStmt *S, CFRefReport *report = new CFRefLeakReport(*getLeakAtReturnBug(LOpts, GCEnabled), LOpts, GCEnabled, SummaryLog, - N, Sym, C); + N, Sym, C, IncludeAllocationLine); + C.emitReport(report); } } @@ -3480,10 +3565,12 @@ RetainCountChecker::handleAutoreleaseCounts(ProgramStateRef state, if (N) { SmallString<128> sbuf; llvm::raw_svector_ostream os(sbuf); - os << "Object over-autoreleased: object was sent -autorelease "; + os << "Object was autoreleased "; if (V.getAutoreleaseCount() > 1) - os << V.getAutoreleaseCount() << " times "; - os << "but the object has a +" << V.getCount() << " retain count"; + os << V.getAutoreleaseCount() << " times but the object "; + else + os << "but "; + os << "has a +" << V.getCount() << " retain count"; if (!overAutorelease) overAutorelease.reset(new OverAutorelease()); @@ -3534,7 +3621,8 @@ RetainCountChecker::processLeaks(ProgramStateRef state, assert(BT && "BugType not initialized."); CFRefLeakReport *report = new CFRefLeakReport(*BT, LOpts, GCEnabled, - SummaryLog, N, *I, Ctx); + SummaryLog, N, *I, Ctx, + IncludeAllocationLine); Ctx.emitReport(report); } } @@ -3656,6 +3744,6 @@ void RetainCountChecker::printState(raw_ostream &Out, ProgramStateRef State, //===----------------------------------------------------------------------===// void ento::registerRetainCountChecker(CheckerManager &Mgr) { - Mgr.registerChecker<RetainCountChecker>(); + Mgr.registerChecker<RetainCountChecker>(Mgr.getAnalyzerOptions()); } diff --git a/lib/StaticAnalyzer/Checkers/ReturnUndefChecker.cpp b/lib/StaticAnalyzer/Checkers/ReturnUndefChecker.cpp index 7a5d993..ed96c40 100644 --- a/lib/StaticAnalyzer/Checkers/ReturnUndefChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/ReturnUndefChecker.cpp @@ -55,8 +55,17 @@ void ReturnUndefChecker::checkPreStmt(const ReturnStmt *RS, // void test() { // return foo(); // } - if (RT.isNull() || !RT->isVoidType()) - emitUndef(C, RetE); + if (!RT.isNull() && RT->isVoidType()) + return; + + // Not all blocks have explicitly-specified return types; if the return type + // is not available, but the return value expression has 'void' type, assume + // Sema already checked it. + if (RT.isNull() && isa<BlockDecl>(SFC->getDecl()) && + RetE->getType()->isVoidType()) + return; + + emitUndef(C, RetE); return; } diff --git a/lib/StaticAnalyzer/Core/BugReporter.cpp b/lib/StaticAnalyzer/Core/BugReporter.cpp index 8f8eb3b..a85235c 100644 --- a/lib/StaticAnalyzer/Core/BugReporter.cpp +++ b/lib/StaticAnalyzer/Core/BugReporter.cpp @@ -52,77 +52,22 @@ void BugReporterContext::anchor() {} // Helper routines for walking the ExplodedGraph and fetching statements. //===----------------------------------------------------------------------===// -static inline const Stmt *GetStmt(const ProgramPoint &P) { - if (Optional<StmtPoint> SP = P.getAs<StmtPoint>()) - return SP->getStmt(); - if (Optional<BlockEdge> BE = P.getAs<BlockEdge>()) - return BE->getSrc()->getTerminator(); - if (Optional<CallEnter> CE = P.getAs<CallEnter>()) - return CE->getCallExpr(); - if (Optional<CallExitEnd> CEE = P.getAs<CallExitEnd>()) - return CEE->getCalleeContext()->getCallSite(); - - return 0; -} - -static inline const ExplodedNode* -GetPredecessorNode(const ExplodedNode *N) { - return N->pred_empty() ? NULL : *(N->pred_begin()); -} - -static inline const ExplodedNode* -GetSuccessorNode(const ExplodedNode *N) { - return N->succ_empty() ? NULL : *(N->succ_begin()); -} - static const Stmt *GetPreviousStmt(const ExplodedNode *N) { - for (N = GetPredecessorNode(N); N; N = GetPredecessorNode(N)) - if (const Stmt *S = GetStmt(N->getLocation())) + for (N = N->getFirstPred(); N; N = N->getFirstPred()) + if (const Stmt *S = PathDiagnosticLocation::getStmt(N)) return S; return 0; } -static const Stmt *GetNextStmt(const ExplodedNode *N) { - for (N = GetSuccessorNode(N); N; N = GetSuccessorNode(N)) - if (const Stmt *S = GetStmt(N->getLocation())) { - // Check if the statement is '?' or '&&'/'||'. These are "merges", - // not actual statement points. - switch (S->getStmtClass()) { - case Stmt::ChooseExprClass: - case Stmt::BinaryConditionalOperatorClass: continue; - case Stmt::ConditionalOperatorClass: continue; - case Stmt::BinaryOperatorClass: { - BinaryOperatorKind Op = cast<BinaryOperator>(S)->getOpcode(); - if (Op == BO_LAnd || Op == BO_LOr) - continue; - break; - } - default: - break; - } - return S; - } - - return 0; -} - static inline const Stmt* GetCurrentOrPreviousStmt(const ExplodedNode *N) { - if (const Stmt *S = GetStmt(N->getLocation())) + if (const Stmt *S = PathDiagnosticLocation::getStmt(N)) return S; return GetPreviousStmt(N); } -static inline const Stmt* -GetCurrentOrNextStmt(const ExplodedNode *N) { - if (const Stmt *S = GetStmt(N->getLocation())) - return S; - - return GetNextStmt(N); -} - //===----------------------------------------------------------------------===// // Diagnostic cleanup. //===----------------------------------------------------------------------===// @@ -198,10 +143,16 @@ static void removeRedundantMsgs(PathPieces &path) { } } +/// A map from PathDiagnosticPiece to the LocationContext of the inlined +/// function call it represents. +typedef llvm::DenseMap<const PathPieces *, const LocationContext *> + LocationContextMap; + /// Recursively scan through a path and prune out calls and macros pieces /// that aren't needed. Return true if afterwards the path contains /// "interesting stuff" which means it shouldn't be pruned from the parent path. -bool BugReporter::RemoveUnneededCalls(PathPieces &pieces, BugReport *R) { +static bool removeUnneededCalls(PathPieces &pieces, BugReport *R, + LocationContextMap &LCM) { bool containsSomethingInteresting = false; const unsigned N = pieces.size(); @@ -222,13 +173,13 @@ bool BugReporter::RemoveUnneededCalls(PathPieces &pieces, BugReport *R) { case PathDiagnosticPiece::Call: { PathDiagnosticCallPiece *call = cast<PathDiagnosticCallPiece>(piece); // Check if the location context is interesting. - assert(LocationContextMap.count(call)); - if (R->isInteresting(LocationContextMap[call])) { + assert(LCM.count(&call->path)); + if (R->isInteresting(LCM[&call->path])) { containsSomethingInteresting = true; break; } - if (!RemoveUnneededCalls(call->path, R)) + if (!removeUnneededCalls(call->path, R, LCM)) continue; containsSomethingInteresting = true; @@ -236,7 +187,7 @@ bool BugReporter::RemoveUnneededCalls(PathPieces &pieces, BugReport *R) { } case PathDiagnosticPiece::Macro: { PathDiagnosticMacroPiece *macro = cast<PathDiagnosticMacroPiece>(piece); - if (!RemoveUnneededCalls(macro->subPieces, R)) + if (!removeUnneededCalls(macro->subPieces, R, LCM)) continue; containsSomethingInteresting = true; break; @@ -355,7 +306,7 @@ public: PathDiagnosticLocation PathDiagnosticBuilder::ExecutionContinues(const ExplodedNode *N) { - if (const Stmt *S = GetNextStmt(N)) + if (const Stmt *S = PathDiagnosticLocation::getNextStmt(N)) return PathDiagnosticLocation(S, getSourceManager(), LC); return PathDiagnosticLocation::createDeclEnd(N->getLocationContext(), @@ -566,6 +517,7 @@ static void CompactPathDiagnostic(PathPieces &path, const SourceManager& SM); static bool GenerateMinimalPathDiagnostic(PathDiagnostic& PD, PathDiagnosticBuilder &PDB, const ExplodedNode *N, + LocationContextMap &LCM, ArrayRef<BugReporterVisitor *> visitors) { SourceManager& SMgr = PDB.getSourceManager(); @@ -578,7 +530,7 @@ static bool GenerateMinimalPathDiagnostic(PathDiagnostic& PD, while (NextNode) { N = NextNode; PDB.LC = N->getLocationContext(); - NextNode = GetPredecessorNode(N); + NextNode = N->getFirstPred(); ProgramPoint P = N->getLocation(); @@ -586,8 +538,8 @@ static bool GenerateMinimalPathDiagnostic(PathDiagnostic& PD, if (Optional<CallExitEnd> CE = P.getAs<CallExitEnd>()) { PathDiagnosticCallPiece *C = PathDiagnosticCallPiece::construct(N, *CE, SMgr); - GRBugReporter& BR = PDB.getBugReporter(); - BR.addCallPieceLocationContextPair(C, CE->getCalleeContext()); + // Record the mapping from call piece to LocationContext. + LCM[&C->path] = CE->getCalleeContext(); PD.getActivePath().push_front(C); PD.pushActivePath(&C->path); CallStack.push_back(StackDiagPair(C, N)); @@ -610,8 +562,8 @@ static bool GenerateMinimalPathDiagnostic(PathDiagnostic& PD, } else { const Decl *Caller = CE->getLocationContext()->getDecl(); C = PathDiagnosticCallPiece::construct(PD.getActivePath(), Caller); - GRBugReporter& BR = PDB.getBugReporter(); - BR.addCallPieceLocationContextPair(C, CE->getCalleeContext()); + // Record the mapping from call piece to LocationContext. + LCM[&C->path] = CE->getCalleeContext(); } C->setCallee(*CE, SMgr); @@ -640,7 +592,7 @@ static bool GenerateMinimalPathDiagnostic(PathDiagnostic& PD, case Stmt::GotoStmtClass: case Stmt::IndirectGotoStmtClass: { - const Stmt *S = GetNextStmt(N); + const Stmt *S = PathDiagnosticLocation::getNextStmt(N); if (!S) break; @@ -929,6 +881,50 @@ public: bool isDead() const { return IsDead; } }; +static PathDiagnosticLocation cleanUpLocation(PathDiagnosticLocation L, + const LocationContext *LC, + bool firstCharOnly = false) { + if (const Stmt *S = L.asStmt()) { + const Stmt *Original = S; + while (1) { + // Adjust the location for some expressions that are best referenced + // by one of their subexpressions. + switch (S->getStmtClass()) { + default: + break; + case Stmt::ParenExprClass: + case Stmt::GenericSelectionExprClass: + S = cast<Expr>(S)->IgnoreParens(); + firstCharOnly = true; + continue; + case Stmt::BinaryConditionalOperatorClass: + case Stmt::ConditionalOperatorClass: + S = cast<AbstractConditionalOperator>(S)->getCond(); + firstCharOnly = true; + continue; + case Stmt::ChooseExprClass: + S = cast<ChooseExpr>(S)->getCond(); + firstCharOnly = true; + continue; + case Stmt::BinaryOperatorClass: + S = cast<BinaryOperator>(S)->getLHS(); + firstCharOnly = true; + continue; + } + + break; + } + + if (S != Original) + L = PathDiagnosticLocation(S, L.getManager(), LC); + } + + if (firstCharOnly) + L = PathDiagnosticLocation::createSingleLocation(L); + + return L; +} + class EdgeBuilder { std::vector<ContextLocation> CLocs; typedef std::vector<ContextLocation>::iterator iterator; @@ -943,53 +939,12 @@ class EdgeBuilder { PathDiagnosticLocation getContextLocation(const PathDiagnosticLocation &L); - PathDiagnosticLocation cleanUpLocation(PathDiagnosticLocation L, - bool firstCharOnly = false) { - if (const Stmt *S = L.asStmt()) { - const Stmt *Original = S; - while (1) { - // Adjust the location for some expressions that are best referenced - // by one of their subexpressions. - switch (S->getStmtClass()) { - default: - break; - case Stmt::ParenExprClass: - case Stmt::GenericSelectionExprClass: - S = cast<Expr>(S)->IgnoreParens(); - firstCharOnly = true; - continue; - case Stmt::BinaryConditionalOperatorClass: - case Stmt::ConditionalOperatorClass: - S = cast<AbstractConditionalOperator>(S)->getCond(); - firstCharOnly = true; - continue; - case Stmt::ChooseExprClass: - S = cast<ChooseExpr>(S)->getCond(); - firstCharOnly = true; - continue; - case Stmt::BinaryOperatorClass: - S = cast<BinaryOperator>(S)->getLHS(); - firstCharOnly = true; - continue; - } - - break; - } - if (S != Original) - L = PathDiagnosticLocation(S, L.getManager(), PDB.LC); - } - - if (firstCharOnly) - L = PathDiagnosticLocation::createSingleLocation(L); - - return L; - } void popLocation() { if (!CLocs.back().isDead() && CLocs.back().asLocation().isFileID()) { // For contexts, we only one the first character as the range. - rawAddEdge(cleanUpLocation(CLocs.back(), true)); + rawAddEdge(cleanUpLocation(CLocs.back(), PDB.LC, true)); } CLocs.pop_back(); } @@ -1026,7 +981,8 @@ public: PrevLoc = PathDiagnosticLocation(); } - void addEdge(PathDiagnosticLocation NewLoc, bool alwaysAdd = false); + void addEdge(PathDiagnosticLocation NewLoc, bool alwaysAdd = false, + bool IsPostJump = false); void rawAddEdge(PathDiagnosticLocation NewLoc); @@ -1102,8 +1058,8 @@ void EdgeBuilder::rawAddEdge(PathDiagnosticLocation NewLoc) { return; } - const PathDiagnosticLocation &NewLocClean = cleanUpLocation(NewLoc); - const PathDiagnosticLocation &PrevLocClean = cleanUpLocation(PrevLoc); + const PathDiagnosticLocation &NewLocClean = cleanUpLocation(NewLoc, PDB.LC); + const PathDiagnosticLocation &PrevLocClean = cleanUpLocation(PrevLoc, PDB.LC); if (PrevLocClean.asLocation().isInvalid()) { PrevLoc = NewLoc; @@ -1122,7 +1078,8 @@ void EdgeBuilder::rawAddEdge(PathDiagnosticLocation NewLoc) { PrevLoc = NewLoc; } -void EdgeBuilder::addEdge(PathDiagnosticLocation NewLoc, bool alwaysAdd) { +void EdgeBuilder::addEdge(PathDiagnosticLocation NewLoc, bool alwaysAdd, + bool IsPostJump) { if (!alwaysAdd && NewLoc.asLocation().isMacroID()) return; @@ -1135,13 +1092,14 @@ void EdgeBuilder::addEdge(PathDiagnosticLocation NewLoc, bool alwaysAdd) { // Is the top location context the same as the one for the new location? if (TopContextLoc == CLoc) { if (alwaysAdd) { - if (IsConsumedExpr(TopContextLoc) && - !IsControlFlowExpr(TopContextLoc.asStmt())) - TopContextLoc.markDead(); + if (IsConsumedExpr(TopContextLoc)) + TopContextLoc.markDead(); rawAddEdge(NewLoc); } + if (IsPostJump) + TopContextLoc.markDead(); return; } @@ -1149,13 +1107,13 @@ void EdgeBuilder::addEdge(PathDiagnosticLocation NewLoc, bool alwaysAdd) { if (alwaysAdd) { rawAddEdge(NewLoc); - if (IsConsumedExpr(CLoc) && !IsControlFlowExpr(CLoc.asStmt())) { - CLocs.push_back(ContextLocation(CLoc, true)); + if (IsConsumedExpr(CLoc)) { + CLocs.push_back(ContextLocation(CLoc, /*IsDead=*/true)); return; } } - CLocs.push_back(CLoc); + CLocs.push_back(ContextLocation(CLoc, /*IsDead=*/IsPostJump)); return; } @@ -1340,7 +1298,7 @@ static const Stmt *getStmtBeforeCond(ParentMap &PM, const Stmt *Term, if (!isContainedByStmt(PM, Term, S)) return S; } - N = GetPredecessorNode(N); + N = N->getFirstPred(); } return 0; } @@ -1376,6 +1334,7 @@ static bool isInLoopBody(ParentMap &PM, const Stmt *S, const Stmt *Term) { static bool GenerateExtensivePathDiagnostic(PathDiagnostic& PD, PathDiagnosticBuilder &PDB, const ExplodedNode *N, + LocationContextMap &LCM, ArrayRef<BugReporterVisitor *> visitors) { EdgeBuilder EB(PD, PDB); const SourceManager& SM = PDB.getSourceManager(); @@ -1385,7 +1344,7 @@ static bool GenerateExtensivePathDiagnostic(PathDiagnostic& PD, const ExplodedNode *NextNode = N->pred_empty() ? NULL : *(N->pred_begin()); while (NextNode) { N = NextNode; - NextNode = GetPredecessorNode(N); + NextNode = N->getFirstPred(); ProgramPoint P = N->getLocation(); do { @@ -1406,10 +1365,9 @@ static bool GenerateExtensivePathDiagnostic(PathDiagnostic& PD, PathDiagnosticCallPiece *C = PathDiagnosticCallPiece::construct(N, *CE, SM); - GRBugReporter& BR = PDB.getBugReporter(); - BR.addCallPieceLocationContextPair(C, CE->getCalleeContext()); + LCM[&C->path] = CE->getCalleeContext(); - EB.addEdge(C->callReturn, true); + EB.addEdge(C->callReturn, /*AlwaysAdd=*/true, /*IsPostJump=*/true); EB.flushLocations(); PD.getActivePath().push_front(C); @@ -1444,8 +1402,7 @@ static bool GenerateExtensivePathDiagnostic(PathDiagnostic& PD, } else { const Decl *Caller = CE->getLocationContext()->getDecl(); C = PathDiagnosticCallPiece::construct(PD.getActivePath(), Caller); - GRBugReporter& BR = PDB.getBugReporter(); - BR.addCallPieceLocationContextPair(C, CE->getCalleeContext()); + LCM[&C->path] = CE->getCalleeContext(); } C->setCallee(*CE, SM); @@ -1573,6 +1530,458 @@ static bool GenerateExtensivePathDiagnostic(PathDiagnostic& PD, return PDB.getBugReport()->isValid(); } +/// \brief Adds a sanitized control-flow diagnostic edge to a path. +static void addEdgeToPath(PathPieces &path, + PathDiagnosticLocation &PrevLoc, + PathDiagnosticLocation NewLoc, + const LocationContext *LC) { + if (!NewLoc.isValid()) + return; + + SourceLocation NewLocL = NewLoc.asLocation(); + if (NewLocL.isInvalid() || NewLocL.isMacroID()) + return; + + if (!PrevLoc.isValid()) { + PrevLoc = NewLoc; + return; + } + + // FIXME: ignore intra-macro edges for now. + if (NewLoc.asLocation().getExpansionLoc() == + PrevLoc.asLocation().getExpansionLoc()) + return; + + path.push_front(new PathDiagnosticControlFlowPiece(NewLoc, + PrevLoc)); + PrevLoc = NewLoc; +} + +static bool +GenerateAlternateExtensivePathDiagnostic(PathDiagnostic& PD, + PathDiagnosticBuilder &PDB, + const ExplodedNode *N, + LocationContextMap &LCM, + ArrayRef<BugReporterVisitor *> visitors) { + + BugReport *report = PDB.getBugReport(); + const SourceManager& SM = PDB.getSourceManager(); + StackDiagVector CallStack; + InterestingExprs IE; + + // Record the last location for a given visited stack frame. + llvm::DenseMap<const StackFrameContext *, PathDiagnosticLocation> + PrevLocMap; + + const ExplodedNode *NextNode = N->getFirstPred(); + while (NextNode) { + N = NextNode; + NextNode = N->getFirstPred(); + ProgramPoint P = N->getLocation(); + const LocationContext *LC = N->getLocationContext(); + assert(!LCM[&PD.getActivePath()] || LCM[&PD.getActivePath()] == LC); + LCM[&PD.getActivePath()] = LC; + PathDiagnosticLocation &PrevLoc = PrevLocMap[LC->getCurrentStackFrame()]; + + do { + if (Optional<PostStmt> PS = P.getAs<PostStmt>()) { + // For expressions, make sure we propagate the + // interesting symbols correctly. + if (const Expr *Ex = PS->getStmtAs<Expr>()) + reversePropagateIntererstingSymbols(*PDB.getBugReport(), IE, + N->getState().getPtr(), Ex, + N->getLocationContext()); + + PathDiagnosticLocation L = + PathDiagnosticLocation(PS->getStmt(), SM, LC); + addEdgeToPath(PD.getActivePath(), PrevLoc, L, LC); + break; + } + + // Have we encountered an exit from a function call? + if (Optional<CallExitEnd> CE = P.getAs<CallExitEnd>()) { + const Stmt *S = CE->getCalleeContext()->getCallSite(); + // Propagate the interesting symbols accordingly. + if (const Expr *Ex = dyn_cast_or_null<Expr>(S)) { + reversePropagateIntererstingSymbols(*PDB.getBugReport(), IE, + N->getState().getPtr(), Ex, + N->getLocationContext()); + } + + // We are descending into a call (backwards). Construct + // a new call piece to contain the path pieces for that call. + PathDiagnosticCallPiece *C = + PathDiagnosticCallPiece::construct(N, *CE, SM); + + // Record the location context for this call piece. + LCM[&C->path] = CE->getCalleeContext(); + + // Add the edge to the return site. + addEdgeToPath(PD.getActivePath(), PrevLoc, C->callReturn, LC); + + // Make the contents of the call the active path for now. + PD.pushActivePath(&C->path); + CallStack.push_back(StackDiagPair(C, N)); + break; + } + + // Have we encountered an entrance to a call? It may be + // the case that we have not encountered a matching + // call exit before this point. This means that the path + // terminated within the call itself. + if (Optional<CallEnter> CE = P.getAs<CallEnter>()) { + // Add an edge to the start of the function. + const Decl *D = CE->getCalleeContext()->getDecl(); + addEdgeToPath(PD.getActivePath(), PrevLoc, + PathDiagnosticLocation::createBegin(D, SM), LC); + + // Did we visit an entire call? + bool VisitedEntireCall = PD.isWithinCall(); + PD.popActivePath(); + + PathDiagnosticCallPiece *C; + if (VisitedEntireCall) { + C = cast<PathDiagnosticCallPiece>(PD.getActivePath().front()); + } else { + const Decl *Caller = CE->getLocationContext()->getDecl(); + C = PathDiagnosticCallPiece::construct(PD.getActivePath(), Caller); + LCM[&C->path] = CE->getCalleeContext(); + } + C->setCallee(*CE, SM); + + if (!CallStack.empty()) { + assert(CallStack.back().first == C); + CallStack.pop_back(); + } + break; + } + + // Block edges. + if (Optional<BlockEdge> BE = P.getAs<BlockEdge>()) { + // Does this represent entering a call? If so, look at propagating + // interesting symbols across call boundaries. + if (NextNode) { + const LocationContext *CallerCtx = NextNode->getLocationContext(); + const LocationContext *CalleeCtx = PDB.LC; + if (CallerCtx != CalleeCtx) { + reversePropagateInterestingSymbols(*PDB.getBugReport(), IE, + N->getState().getPtr(), + CalleeCtx, CallerCtx); + } + } + + // Are we jumping to the head of a loop? Add a special diagnostic. + if (const Stmt *Loop = BE->getSrc()->getLoopTarget()) { + PathDiagnosticLocation L(Loop, SM, PDB.LC); + const CompoundStmt *CS = NULL; + + if (const ForStmt *FS = dyn_cast<ForStmt>(Loop)) + CS = dyn_cast<CompoundStmt>(FS->getBody()); + else if (const WhileStmt *WS = dyn_cast<WhileStmt>(Loop)) + CS = dyn_cast<CompoundStmt>(WS->getBody()); + + PathDiagnosticEventPiece *p = + new PathDiagnosticEventPiece(L, "Looping back to the head " + "of the loop"); + p->setPrunable(true); + + addEdgeToPath(PD.getActivePath(), PrevLoc, p->getLocation(), LC); + PD.getActivePath().push_front(p); + + if (CS) { + addEdgeToPath(PD.getActivePath(), PrevLoc, + PathDiagnosticLocation::createEndBrace(CS, SM), LC); + } + } + + const CFGBlock *BSrc = BE->getSrc(); + ParentMap &PM = PDB.getParentMap(); + + if (const Stmt *Term = BSrc->getTerminator()) { + // Are we jumping past the loop body without ever executing the + // loop (because the condition was false)? + if (isLoopJumpPastBody(Term, &*BE) && + !isInLoopBody(PM, + getStmtBeforeCond(PM, + BSrc->getTerminatorCondition(), + N), + Term)) + { + PathDiagnosticLocation L(Term, SM, PDB.LC); + PathDiagnosticEventPiece *PE = + new PathDiagnosticEventPiece(L, "Loop body executed 0 times"); + PE->setPrunable(true); + addEdgeToPath(PD.getActivePath(), PrevLoc, + PE->getLocation(), LC); + PD.getActivePath().push_front(PE); + } + } + break; + } + } while (0); + + if (!NextNode) + continue; + + // Add pieces from custom visitors. + for (ArrayRef<BugReporterVisitor *>::iterator I = visitors.begin(), + E = visitors.end(); + I != E; ++I) { + if (PathDiagnosticPiece *p = (*I)->VisitNode(N, NextNode, PDB, *report)) { + addEdgeToPath(PD.getActivePath(), PrevLoc, p->getLocation(), LC); + PD.getActivePath().push_front(p); + updateStackPiecesWithMessage(p, CallStack); + } + } + } + + return report->isValid(); +} + +const Stmt *getLocStmt(PathDiagnosticLocation L) { + if (!L.isValid()) + return 0; + return L.asStmt(); +} + +const Stmt *getStmtParent(const Stmt *S, ParentMap &PM) { + if (!S) + return 0; + return PM.getParentIgnoreParens(S); +} + +#if 0 +static bool isConditionForTerminator(const Stmt *S, const Stmt *Cond) { + // Note that we intentionally to do not handle || and && here. + switch (S->getStmtClass()) { + case Stmt::ForStmtClass: + return cast<ForStmt>(S)->getCond() == Cond; + case Stmt::WhileStmtClass: + return cast<WhileStmt>(S)->getCond() == Cond; + case Stmt::DoStmtClass: + return cast<DoStmt>(S)->getCond() == Cond; + case Stmt::ChooseExprClass: + return cast<ChooseExpr>(S)->getCond() == Cond; + case Stmt::IndirectGotoStmtClass: + return cast<IndirectGotoStmt>(S)->getTarget() == Cond; + case Stmt::SwitchStmtClass: + return cast<SwitchStmt>(S)->getCond() == Cond; + case Stmt::BinaryConditionalOperatorClass: + return cast<BinaryConditionalOperator>(S)->getCond() == Cond; + case Stmt::ConditionalOperatorClass: + return cast<ConditionalOperator>(S)->getCond() == Cond; + case Stmt::ObjCForCollectionStmtClass: + return cast<ObjCForCollectionStmt>(S)->getElement() == Cond; + default: + return false; + } +} +#endif + +typedef llvm::DenseSet<const PathDiagnosticControlFlowPiece *> + ControlFlowBarrierSet; + +typedef llvm::DenseSet<const PathDiagnosticCallPiece *> + OptimizedCallsSet; + +static bool isBarrier(ControlFlowBarrierSet &CFBS, + const PathDiagnosticControlFlowPiece *P) { + return CFBS.count(P); +} + +static bool optimizeEdges(PathPieces &path, SourceManager &SM, + ControlFlowBarrierSet &CFBS, + OptimizedCallsSet &OCS, + LocationContextMap &LCM) { + bool hasChanges = false; + const LocationContext *LC = LCM[&path]; + assert(LC); + bool isFirst = true; + + for (PathPieces::iterator I = path.begin(), E = path.end(); I != E; ) { + bool wasFirst = isFirst; + isFirst = false; + + // Optimize subpaths. + if (PathDiagnosticCallPiece *CallI = dyn_cast<PathDiagnosticCallPiece>(*I)){ + // Record the fact that a call has been optimized so we only do the + // effort once. + if (!OCS.count(CallI)) { + while (optimizeEdges(CallI->path, SM, CFBS, OCS, LCM)) {} + OCS.insert(CallI); + } + ++I; + continue; + } + + // Pattern match the current piece and its successor. + PathDiagnosticControlFlowPiece *PieceI = + dyn_cast<PathDiagnosticControlFlowPiece>(*I); + + if (!PieceI) { + ++I; + continue; + } + + ParentMap &PM = LC->getParentMap(); + const Stmt *s1Start = getLocStmt(PieceI->getStartLocation()); + const Stmt *s1End = getLocStmt(PieceI->getEndLocation()); + const Stmt *level1 = getStmtParent(s1Start, PM); + const Stmt *level2 = getStmtParent(s1End, PM); + + if (wasFirst) { +#if 0 + // Apply the "first edge" case for Rule V. here. + if (s1Start && level1 && isConditionForTerminator(level1, s1Start)) { + PathDiagnosticLocation NewLoc(level2, SM, LC); + PieceI->setStartLocation(NewLoc); + CFBS.insert(PieceI); + return true; + } +#endif + // Apply the "first edge" case for Rule III. here. + if (!isBarrier(CFBS, PieceI) && + level1 && level2 && level2 == PM.getParent(level1)) { + path.erase(I); + // Since we are erasing the current edge at the start of the + // path, just return now so we start analyzing the start of the path + // again. + return true; + } + } + + PathPieces::iterator NextI = I; ++NextI; + if (NextI == E) + break; + + PathDiagnosticControlFlowPiece *PieceNextI = + dyn_cast<PathDiagnosticControlFlowPiece>(*NextI); + + if (!PieceNextI) { + ++I; + continue; + } + + const Stmt *s2Start = getLocStmt(PieceNextI->getStartLocation()); + const Stmt *s2End = getLocStmt(PieceNextI->getEndLocation()); + const Stmt *level3 = getStmtParent(s2Start, PM); + const Stmt *level4 = getStmtParent(s2End, PM); + + // Rule I. + // + // If we have two consecutive control edges whose end/begin locations + // are at the same level (e.g. statements or top-level expressions within + // a compound statement, or siblings share a single ancestor expression), + // then merge them if they have no interesting intermediate event. + // + // For example: + // + // (1.1 -> 1.2) -> (1.2 -> 1.3) becomes (1.1 -> 1.3) because the common + // parent is '1'. Here 'x.y.z' represents the hierarchy of statements. + // + // NOTE: this will be limited later in cases where we add barriers + // to prevent this optimization. + // + if (level1 && level1 == level2 && level1 == level3 && level1 == level4) { + PieceI->setEndLocation(PieceNextI->getEndLocation()); + path.erase(NextI); + hasChanges = true; + continue; + } + + // Rule II. + // + // If we have two consecutive control edges where we decend to a + // subexpression and then pop out merge them. + // + // NOTE: this will be limited later in cases where we add barriers + // to prevent this optimization. + // + // For example: + // + // (1.1 -> 1.1.1) -> (1.1.1 -> 1.2) becomes (1.1 -> 1.2). + if (level1 && level2 && + level1 == level4 && + level2 == level3 && PM.getParentIgnoreParens(level2) == level1) { + PieceI->setEndLocation(PieceNextI->getEndLocation()); + path.erase(NextI); + hasChanges = true; + continue; + } + + // Rule III. + // + // Eliminate unnecessary edges where we descend to a subexpression from + // a statement at the same level as our parent. + // + // NOTE: this will be limited later in cases where we add barriers + // to prevent this optimization. + // + // For example: + // + // (1.1 -> 1.1.1) -> (1.1.1 -> X) becomes (1.1 -> X). + // + if (level1 && level2 && level1 == PM.getParentIgnoreParens(level2)) { + PieceI->setEndLocation(PieceNextI->getEndLocation()); + path.erase(NextI); + hasChanges = true; + continue; + } + + // Rule IV. + // + // Eliminate unnecessary edges where we ascend from a subexpression to + // a statement at the same level as our parent. + // + // NOTE: this will be limited later in cases where we add barriers + // to prevent this optimization. + // + // For example: + // + // (X -> 1.1.1) -> (1.1.1 -> 1.1) becomes (X -> 1.1). + // [first edge] (1.1.1 -> 1.1) -> eliminate + // + if (level2 && level4 && level2 == level3 && level4 == PM.getParent(level2)){ + PieceI->setEndLocation(PieceNextI->getEndLocation()); + path.erase(NextI); + hasChanges = true; + continue; + } +#if 0 + // Rule V. + // + // Replace terminator conditions with terminators when the condition + // itself has no control-flow. + // + // For example: + // + // (X -> condition) -> (condition -> Y) becomes (X -> term) -> (term -> Y) + // [first edge] (condition -> Y) becomes (term -> Y) + // + // This applies to 'if', 'for', 'while', 'do .. while', 'switch'... + // + if (!isBarrier(CFBS, PieceNextI) && + s1End && s1End == s2Start && level2) { + if (isConditionForTerminator(level2, s1End)) { + PathDiagnosticLocation NewLoc(level2, SM, LC); + PieceI->setEndLocation(NewLoc); + PieceNextI->setStartLocation(NewLoc); + CFBS.insert(PieceI); + hasChanges = true; + continue; + } + + } +#endif + + // No changes at this index? Move to the next one. + ++I; + } + + // No changes. + return hasChanges; +} + //===----------------------------------------------------------------------===// // Methods for BugType and subclasses. //===----------------------------------------------------------------------===// @@ -1758,7 +2167,7 @@ const Stmt *BugReport::getStmt() const { S = GetPreviousStmt(ErrorNode); } if (!S) - S = GetStmt(ProgP); + S = PathDiagnosticLocation::getStmt(ErrorNode); return S; } @@ -1785,22 +2194,7 @@ PathDiagnosticLocation BugReport::getLocation(const SourceManager &SM) const { if (ErrorNode) { assert(!Location.isValid() && "Either Location or ErrorNode should be specified but not both."); - - if (const Stmt *S = GetCurrentOrPreviousStmt(ErrorNode)) { - const LocationContext *LC = ErrorNode->getLocationContext(); - - // For member expressions, return the location of the '.' or '->'. - if (const MemberExpr *ME = dyn_cast<MemberExpr>(S)) - return PathDiagnosticLocation::createMemberLoc(ME, SM); - // For binary operators, return the location of the operator. - if (const BinaryOperator *B = dyn_cast<BinaryOperator>(S)) - return PathDiagnosticLocation::createOperatorLoc(B, SM); - - if (ErrorNode->getLocation().getAs<PostStmtPurgeDeadSymbols>()) - return PathDiagnosticLocation::createEnd(S, SM, LC); - - return PathDiagnosticLocation::createBegin(S, SM, LC); - } + return PathDiagnosticLocation::createEndOfPath(ErrorNode, SM); } else { assert(Location.isValid()); return Location; @@ -2010,7 +2404,8 @@ bool TrimmedGraph::popNextReportGraph(ReportGraph &GraphWrapper) { while (true) { // Create the equivalent node in the new graph with the same state // and location. - ExplodedNode *NewN = GNew->getNode(OrigN->getLocation(), OrigN->getState()); + ExplodedNode *NewN = GNew->getNode(OrigN->getLocation(), OrigN->getState(), + OrigN->isSink()); // Store the mapping to the original node. InterExplodedGraphMap::const_iterator IMitr = InverseMap.find(OrigN); @@ -2165,6 +2560,13 @@ bool GRBugReporter::generatePathDiagnostic(PathDiagnostic& PD, typedef PathDiagnosticConsumer::PathGenerationScheme PathGenerationScheme; PathGenerationScheme ActiveScheme = PC.getGenerationScheme(); + if (ActiveScheme == PathDiagnosticConsumer::Extensive) { + AnalyzerOptions &options = getEngine().getAnalysisManager().options; + if (options.getBooleanOption("path-diagnostics-alternate", false)) { + ActiveScheme = PathDiagnosticConsumer::AlternateExtensive; + } + } + TrimmedGraph TrimG(&getGraph(), errorNodes); ReportGraph ErrorGraph; @@ -2186,6 +2588,7 @@ bool GRBugReporter::generatePathDiagnostic(PathDiagnostic& PD, BugReport::VisitorList visitors; unsigned origReportConfigToken, finalReportConfigToken; + LocationContextMap LCM; // While generating diagnostics, it's possible the visitors will decide // new symbols and regions are interesting, or add other visitors based on @@ -2220,12 +2623,19 @@ bool GRBugReporter::generatePathDiagnostic(PathDiagnostic& PD, PD.setEndOfPath(LastPiece); } + // Make sure we get a clean location context map so we don't + // hold onto old mappings. + LCM.clear(); + switch (ActiveScheme) { + case PathDiagnosticConsumer::AlternateExtensive: + GenerateAlternateExtensivePathDiagnostic(PD, PDB, N, LCM, visitors); + break; case PathDiagnosticConsumer::Extensive: - GenerateExtensivePathDiagnostic(PD, PDB, N, visitors); + GenerateExtensivePathDiagnostic(PD, PDB, N, LCM, visitors); break; case PathDiagnosticConsumer::Minimal: - GenerateMinimalPathDiagnostic(PD, PDB, N, visitors); + GenerateMinimalPathDiagnostic(PD, PDB, N, LCM, visitors); break; case PathDiagnosticConsumer::None: GenerateVisitorsOnlyPathDiagnostic(PD, PDB, N, visitors); @@ -2249,12 +2659,19 @@ bool GRBugReporter::generatePathDiagnostic(PathDiagnostic& PD, if (R->shouldPrunePath() && getEngine().getAnalysisManager().options.shouldPrunePaths()) { - bool stillHasNotes = RemoveUnneededCalls(PD.getMutablePieces(), R); + bool stillHasNotes = removeUnneededCalls(PD.getMutablePieces(), R, LCM); assert(stillHasNotes); (void)stillHasNotes; } adjustCallLocations(PD.getMutablePieces()); + + if (ActiveScheme == PathDiagnosticConsumer::AlternateExtensive) { + ControlFlowBarrierSet CFBS; + OptimizedCallsSet OCS; + while (optimizeEdges(PD.getMutablePieces(), getSourceManager(), CFBS, + OCS, LCM)) {} + } } // We found a report and didn't suppress it. diff --git a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp index f600362..e078745 100644 --- a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp +++ b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp @@ -41,7 +41,7 @@ bool bugreporter::isDeclRefExprToReference(const Expr *E) { } const Expr *bugreporter::getDerefExpr(const Stmt *S) { - // Pattern match for a few useful cases (do something smarter later): + // Pattern match for a few useful cases: // a[0], p->f, *p const Expr *E = dyn_cast<Expr>(S); if (!E) @@ -61,6 +61,10 @@ const Expr *bugreporter::getDerefExpr(const Stmt *S) { else if (const MemberExpr *ME = dyn_cast<MemberExpr>(E)) { if (ME->isArrow() || isDeclRefExprToReference(ME->getBase())) { return ME->getBase()->IgnoreParenCasts(); + } else { + // If we have a member expr with a dot, the base must have been + // dereferenced. + return getDerefExpr(ME->getBase()); } } else if (const ObjCIvarRefExpr *IvarRef = dyn_cast<ObjCIvarRefExpr>(E)) { @@ -69,6 +73,9 @@ const Expr *bugreporter::getDerefExpr(const Stmt *S) { else if (const ArraySubscriptExpr *AE = dyn_cast<ArraySubscriptExpr>(E)) { return AE->getBase(); } + else if (isDeclRefExprToReference(E)) { + return E; + } break; } @@ -307,9 +314,9 @@ public: if (LValue) { if (const MemRegion *MR = LValue->getAsRegion()) { if (MR->canPrintPretty()) { - Out << " (reference to '"; + Out << " (reference to "; MR->printPretty(Out); - Out << "')"; + Out << ")"; } } } else { @@ -411,6 +418,35 @@ void FindLastStoreBRVisitor ::Profile(llvm::FoldingSetNodeID &ID) const { ID.AddBoolean(EnableNullFPSuppression); } +/// Returns true if \p N represents the DeclStmt declaring and initializing +/// \p VR. +static bool isInitializationOfVar(const ExplodedNode *N, const VarRegion *VR) { + Optional<PostStmt> P = N->getLocationAs<PostStmt>(); + if (!P) + return false; + + const DeclStmt *DS = P->getStmtAs<DeclStmt>(); + if (!DS) + return false; + + if (DS->getSingleDecl() != VR->getDecl()) + return false; + + const MemSpaceRegion *VarSpace = VR->getMemorySpace(); + const StackSpaceRegion *FrameSpace = dyn_cast<StackSpaceRegion>(VarSpace); + if (!FrameSpace) { + // If we ever directly evaluate global DeclStmts, this assertion will be + // invalid, but this still seems preferable to silently accepting an + // initialization that may be for a path-sensitive variable. + assert(VR->getDecl()->isStaticLocal() && "non-static stackless VarRegion"); + return true; + } + + assert(VR->getDecl()->hasLocalStorage()); + const LocationContext *LCtx = N->getLocationContext(); + return FrameSpace->getStackFrame() == LCtx->getCurrentStackFrame(); +} + PathDiagnosticPiece *FindLastStoreBRVisitor::VisitNode(const ExplodedNode *Succ, const ExplodedNode *Pred, BugReporterContext &BRC, @@ -425,13 +461,9 @@ PathDiagnosticPiece *FindLastStoreBRVisitor::VisitNode(const ExplodedNode *Succ, // First see if we reached the declaration of the region. if (const VarRegion *VR = dyn_cast<VarRegion>(R)) { - if (Optional<PostStmt> P = Pred->getLocationAs<PostStmt>()) { - if (const DeclStmt *DS = P->getStmtAs<DeclStmt>()) { - if (DS->getSingleDecl() == VR->getDecl()) { - StoreSite = Pred; - InitE = VR->getDecl()->getInit(); - } - } + if (isInitializationOfVar(Pred, VR)) { + StoreSite = Pred; + InitE = VR->getDecl()->getInit(); } } @@ -511,9 +543,6 @@ PathDiagnosticPiece *FindLastStoreBRVisitor::VisitNode(const ExplodedNode *Succ, } } - if (!R->canPrintPretty()) - return 0; - // Okay, we've found the binding. Emit an appropriate message. SmallString<256> sbuf; llvm::raw_svector_ostream os(sbuf); @@ -525,9 +554,11 @@ PathDiagnosticPiece *FindLastStoreBRVisitor::VisitNode(const ExplodedNode *Succ, const VarRegion *VR = dyn_cast<VarRegion>(R); if (DS) { - action = "initialized to "; + action = R->canPrintPretty() ? "initialized to " : + "Initializing to "; } else if (isa<BlockExpr>(S)) { - action = "captured by block as "; + action = R->canPrintPretty() ? "captured by block as " : + "Captured by block as "; if (VR) { // See if we can get the BlockVarRegion. ProgramStateRef State = StoreSite->getState(); @@ -545,12 +576,10 @@ PathDiagnosticPiece *FindLastStoreBRVisitor::VisitNode(const ExplodedNode *Succ, } if (action) { - if (!R) - return 0; - - os << '\''; - R->printPretty(os); - os << "' "; + if (R->canPrintPretty()) { + R->printPretty(os); + os << " "; + } if (V.getAs<loc::ConcreteInt>()) { bool b = false; @@ -573,14 +602,18 @@ PathDiagnosticPiece *FindLastStoreBRVisitor::VisitNode(const ExplodedNode *Succ, if (V.isUndef()) { if (isa<VarRegion>(R)) { const VarDecl *VD = cast<VarDecl>(DS->getSingleDecl()); - if (VD->getInit()) - os << "initialized to a garbage value"; - else - os << "declared without an initial value"; + if (VD->getInit()) { + os << (R->canPrintPretty() ? "initialized" : "Initializing") + << " to a garbage value"; + } else { + os << (R->canPrintPretty() ? "declared" : "Declaring") + << " without an initial value"; + } } } else { - os << "initialized here"; + os << (R->canPrintPretty() ? "initialized" : "Initialized") + << " here"; } } } @@ -606,10 +639,11 @@ PathDiagnosticPiece *FindLastStoreBRVisitor::VisitNode(const ExplodedNode *Succ, // Printed parameter indexes are 1-based, not 0-based. unsigned Idx = Param->getFunctionScopeIndex() + 1; - os << " via " << Idx << llvm::getOrdinalSuffix(Idx) << " parameter '"; - - R->printPretty(os); - os << '\''; + os << " via " << Idx << llvm::getOrdinalSuffix(Idx) << " parameter"; + if (R->canPrintPretty()) { + os << " "; + R->printPretty(os); + } } } @@ -619,27 +653,42 @@ PathDiagnosticPiece *FindLastStoreBRVisitor::VisitNode(const ExplodedNode *Succ, if (R->isBoundable()) { if (const TypedValueRegion *TR = dyn_cast<TypedValueRegion>(R)) { if (TR->getValueType()->isObjCObjectPointerType()) { - os << "nil object reference stored to "; + os << "nil object reference stored"; b = true; } } } + if (!b) { + if (R->canPrintPretty()) + os << "Null pointer value stored"; + else + os << "Storing null pointer value"; + } + + } else if (V.isUndef()) { + if (R->canPrintPretty()) + os << "Uninitialized value stored"; + else + os << "Storing uninitialized value"; - if (!b) - os << "Null pointer value stored to "; - } - else if (V.isUndef()) { - os << "Uninitialized value stored to "; } else if (Optional<nonloc::ConcreteInt> CV = V.getAs<nonloc::ConcreteInt>()) { - os << "The value " << CV->getValue() << " is assigned to "; - } - else - os << "Value assigned to "; + if (R->canPrintPretty()) + os << "The value " << CV->getValue() << " is assigned"; + else + os << "Assigning " << CV->getValue(); - os << '\''; - R->printPretty(os); - os << '\''; + } else { + if (R->canPrintPretty()) + os << "Value assigned"; + else + os << "Assigning value"; + } + + if (R->canPrintPretty()) { + os << " to "; + R->printPretty(os); + } } // Construct a new PathDiagnosticPiece. @@ -682,6 +731,14 @@ TrackConstraintBRVisitor::VisitNode(const ExplodedNode *N, if (IsSatisfied) return NULL; + // Start tracking after we see the first state in which the value is + // constrained. + if (!IsTrackingTurnedOn) + if (!isUnderconstrained(N)) + IsTrackingTurnedOn = true; + if (!IsTrackingTurnedOn) + return 0; + // Check if in the previous state it was feasible for this constraint // to *not* be true. if (isUnderconstrained(PrevN)) { @@ -691,8 +748,7 @@ TrackConstraintBRVisitor::VisitNode(const ExplodedNode *N, // As a sanity check, make sure that the negation of the constraint // was infeasible in the current state. If it is feasible, we somehow // missed the transition point. - if (isUnderconstrained(N)) - return NULL; + assert(!isUnderconstrained(N)); // We found the transition point for the constraint. We now need to // pretty-print the constraint. (work-in-progress) @@ -853,7 +909,7 @@ bool bugreporter::trackNullOrUndefValue(const ExplodedNode *N, Inner = Ex; } - if (IsArg) { + if (IsArg && !Inner) { assert(N->getLocation().getAs<CallEnter>() && "Tracking arg but not at call"); } else { // Walk through nodes until we get one that matches the statement exactly. @@ -882,7 +938,7 @@ bool bugreporter::trackNullOrUndefValue(const ExplodedNode *N, // At this point in the path, the receiver should be live since we are at the // message send expr. If it is nil, start tracking it. if (const Expr *Receiver = NilReceiverBRVisitor::getNilReceiver(S, N)) - trackNullOrUndefValue(N, Receiver, report, IsArg, EnableNullFPSuppression); + trackNullOrUndefValue(N, Receiver, report, false, EnableNullFPSuppression); // See if the expression we're interested refers to a variable. @@ -926,45 +982,24 @@ bool bugreporter::trackNullOrUndefValue(const ExplodedNode *N, if (R) { // Mark both the variable region and its contents as interesting. - SVal V = state->getRawSVal(loc::MemRegionVal(R)); - - // If the value matches the default for the variable region, that - // might mean that it's been cleared out of the state. Fall back to - // the full argument expression (with casts and such intact). - if (IsArg) { - bool UseArgValue = V.isUnknownOrUndef() || V.isZeroConstant(); - if (!UseArgValue) { - const SymbolRegionValue *SRV = - dyn_cast_or_null<SymbolRegionValue>(V.getAsLocSymbol()); - if (SRV) - UseArgValue = (SRV->getRegion() == R); - } - if (UseArgValue) - V = state->getSValAsScalarOrLoc(S, N->getLocationContext()); - } + SVal V = LVState->getRawSVal(loc::MemRegionVal(R)); report.markInteresting(R); report.markInteresting(V); report.addVisitor(new UndefOrNullArgVisitor(R)); - if (isa<SymbolicRegion>(R)) { - TrackConstraintBRVisitor *VI = - new TrackConstraintBRVisitor(loc::MemRegionVal(R), false); - report.addVisitor(VI); - } - // If the contents are symbolic, find out when they became null. - if (V.getAsLocSymbol()) { + if (V.getAsLocSymbol(/*IncludeBaseRegions*/ true)) { BugReporterVisitor *ConstraintTracker = new TrackConstraintBRVisitor(V.castAs<DefinedSVal>(), false); report.addVisitor(ConstraintTracker); // Add visitor, which will suppress inline defensive checks. - if (N->getState()->isNull(V).isConstrainedTrue() && + if (LVState->isNull(V).isConstrainedTrue() && EnableNullFPSuppression) { BugReporterVisitor *IDCSuppressor = new SuppressInlineDefensiveChecksVisitor(V.castAs<DefinedSVal>(), - N); + LVNode); report.addVisitor(IDCSuppressor); } } @@ -994,7 +1029,13 @@ bool bugreporter::trackNullOrUndefValue(const ExplodedNode *N, if (Optional<loc::MemRegionVal> L = V.getAs<loc::MemRegionVal>()) { // At this point we are dealing with the region's LValue. // However, if the rvalue is a symbolic region, we should track it as well. - SVal RVal = state->getSVal(L->getRegion()); + // Try to use the correct type when looking up the value. + SVal RVal; + if (const Expr *E = dyn_cast<Expr>(S)) + RVal = state->getRawSVal(L.getValue(), E->getType()); + else + RVal = state->getSVal(L->getRegion()); + const MemRegion *RegionRVal = RVal.getAsRegion(); report.addVisitor(new UndefOrNullArgVisitor(L->getRegion())); @@ -1030,10 +1071,22 @@ PathDiagnosticPiece *NilReceiverBRVisitor::VisitNode(const ExplodedNode *N, if (!P) return 0; - const Expr *Receiver = getNilReceiver(P->getStmt(), N); + const Stmt *S = P->getStmt(); + const Expr *Receiver = getNilReceiver(S, N); if (!Receiver) return 0; + llvm::SmallString<256> Buf; + llvm::raw_svector_ostream OS(Buf); + + if (const ObjCMessageExpr *ME = dyn_cast<ObjCMessageExpr>(S)) { + OS << "'" << ME->getSelector().getAsString() << "' not called"; + } + else { + OS << "No method is called"; + } + OS << " because the receiver is nil"; + // The receiver was nil, and hence the method was skipped. // Register a BugReporterVisitor to issue a message telling us how // the receiver was null. @@ -1042,8 +1095,7 @@ PathDiagnosticPiece *NilReceiverBRVisitor::VisitNode(const ExplodedNode *N, // Issue a message saying that the method was skipped. PathDiagnosticLocation L(Receiver, BRC.getSourceManager(), N->getLocationContext()); - return new PathDiagnosticEventPiece(L, "No method is called " - "because the receiver is nil"); + return new PathDiagnosticEventPiece(L, OS.str()); } // Registers every VarDecl inside a Stmt with a last store visitor. @@ -1372,7 +1424,7 @@ ConditionBRVisitor::VisitConditionVariable(StringRef LhsString, Out << (tookTrue ? "not nil" : "nil"); else if (Ty->isBooleanType()) Out << (tookTrue ? "true" : "false"); - else if (Ty->isIntegerType()) + else if (Ty->isIntegralOrEnumerationType()) Out << (tookTrue ? "non-zero" : "zero"); else return 0; diff --git a/lib/StaticAnalyzer/Core/CallEvent.cpp b/lib/StaticAnalyzer/Core/CallEvent.cpp index 45b2e21..dfd20b8 100644 --- a/lib/StaticAnalyzer/Core/CallEvent.cpp +++ b/lib/StaticAnalyzer/Core/CallEvent.cpp @@ -239,8 +239,20 @@ QualType CallEvent::getDeclaredResultType(const Decl *D) { assert(D); if (const FunctionDecl* FD = dyn_cast<FunctionDecl>(D)) return FD->getResultType(); - else if (const ObjCMethodDecl* MD = dyn_cast<ObjCMethodDecl>(D)) + if (const ObjCMethodDecl* MD = dyn_cast<ObjCMethodDecl>(D)) return MD->getResultType(); + if (const BlockDecl *BD = dyn_cast<BlockDecl>(D)) { + // Blocks are difficult because the return type may not be stored in the + // BlockDecl itself. The AST should probably be enhanced, but for now we + // just do what we can. + QualType Ty = BD->getSignatureAsWritten()->getType(); + if (const FunctionType *FT = Ty->getAs<FunctionType>()) + if (!FT->getResultType()->isDependentType()) + return FT->getResultType(); + + return QualType(); + } + return QualType(); } diff --git a/lib/StaticAnalyzer/Core/Environment.cpp b/lib/StaticAnalyzer/Core/Environment.cpp index fe352aa..7b133f6 100644 --- a/lib/StaticAnalyzer/Core/Environment.cpp +++ b/lib/StaticAnalyzer/Core/Environment.cpp @@ -80,43 +80,17 @@ SVal Environment::getSVal(const EnvironmentEntry &Entry, llvm_unreachable("Should have been handled by ignoreTransparentExprs"); case Stmt::AddrLabelExprClass: - return svalBuilder.makeLoc(cast<AddrLabelExpr>(S)); - - case Stmt::CharacterLiteralClass: { - const CharacterLiteral *C = cast<CharacterLiteral>(S); - return svalBuilder.makeIntVal(C->getValue(), C->getType()); - } - + case Stmt::CharacterLiteralClass: case Stmt::CXXBoolLiteralExprClass: - return svalBuilder.makeBoolVal(cast<CXXBoolLiteralExpr>(S)); - case Stmt::CXXScalarValueInitExprClass: - case Stmt::ImplicitValueInitExprClass: { - QualType Ty = cast<Expr>(S)->getType(); - return svalBuilder.makeZeroVal(Ty); - } - + case Stmt::ImplicitValueInitExprClass: case Stmt::IntegerLiteralClass: - return svalBuilder.makeIntVal(cast<IntegerLiteral>(S)); - case Stmt::ObjCBoolLiteralExprClass: - return svalBuilder.makeBoolVal(cast<ObjCBoolLiteralExpr>(S)); - - // For special C0xx nullptr case, make a null pointer SVal. case Stmt::CXXNullPtrLiteralExprClass: - return svalBuilder.makeNull(); - - case Stmt::ObjCStringLiteralClass: { - MemRegionManager &MRMgr = svalBuilder.getRegionManager(); - const ObjCStringLiteral *SL = cast<ObjCStringLiteral>(S); - return svalBuilder.makeLoc(MRMgr.getObjCStringRegion(SL)); - } - - case Stmt::StringLiteralClass: { - MemRegionManager &MRMgr = svalBuilder.getRegionManager(); - const StringLiteral *SL = cast<StringLiteral>(S); - return svalBuilder.makeLoc(MRMgr.getStringRegion(SL)); - } + case Stmt::ObjCStringLiteralClass: + case Stmt::StringLiteralClass: + // Known constants; defer to SValBuilder. + return svalBuilder.getConstantVal(cast<Expr>(S)).getValue(); case Stmt::ReturnStmtClass: { const ReturnStmt *RS = cast<ReturnStmt>(S); @@ -127,10 +101,8 @@ SVal Environment::getSVal(const EnvironmentEntry &Entry, // Handle all other Stmt* using a lookup. default: - break; + return lookupExpr(EnvironmentEntry(S, LCtx)); } - - return lookupExpr(EnvironmentEntry(S, LCtx)); } Environment EnvironmentManager::bindExpr(Environment Env, diff --git a/lib/StaticAnalyzer/Core/ExprEngine.cpp b/lib/StaticAnalyzer/Core/ExprEngine.cpp index ab4dbd7..bfe4e15 100644 --- a/lib/StaticAnalyzer/Core/ExprEngine.cpp +++ b/lib/StaticAnalyzer/Core/ExprEngine.cpp @@ -106,7 +106,8 @@ ProgramStateRef ExprEngine::getInitialState(const LocationContext *InitLoc) { const ParmVarDecl *PD = FD->getParamDecl(0); QualType T = PD->getType(); - if (!T->isIntegerType()) + const BuiltinType *BT = dyn_cast<BuiltinType>(T); + if (!BT || !BT->isInteger()) break; const MemRegion *R = state->getRegion(PD, InitLoc); @@ -180,7 +181,8 @@ ExprEngine::createTemporaryRegionIfNeeded(ProgramStateRef State, } else { // We need to create a region no matter what. For sanity, make sure we don't // try to stuff a Loc into a non-pointer temporary region. - assert(!V.getAs<Loc>() || Loc::isLocType(Result->getType())); + assert(!V.getAs<Loc>() || Loc::isLocType(Result->getType()) || + Result->getType()->isMemberPointerType()); } ProgramStateManager &StateMgr = State->getStateManager(); @@ -602,11 +604,13 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred, switch (S->getStmtClass()) { // C++ and ARC stuff we don't support yet. case Expr::ObjCIndirectCopyRestoreExprClass: + case Stmt::CXXDefaultInitExprClass: case Stmt::CXXDependentScopeMemberExprClass: case Stmt::CXXPseudoDestructorExprClass: case Stmt::CXXTryStmtClass: case Stmt::CXXTypeidExprClass: case Stmt::CXXUuidofExprClass: + case Stmt::MSPropertyRefExprClass: case Stmt::CXXUnresolvedConstructExprClass: case Stmt::DependentScopeDeclRefExprClass: case Stmt::UnaryTypeTraitExprClass: @@ -653,6 +657,7 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred, case Stmt::SwitchStmtClass: case Stmt::WhileStmtClass: case Expr::MSDependentExistsStmtClass: + case Stmt::CapturedStmtClass: llvm_unreachable("Stmt should not be in analyzer evaluation loop"); case Stmt::ObjCSubscriptRefExprClass: @@ -736,21 +741,22 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred, const CXXDefaultArgExpr *DefaultE = cast<CXXDefaultArgExpr>(S); const Expr *ArgE = DefaultE->getExpr(); - // Avoid creating and destroying a lot of APSInts. - SVal V; - llvm::APSInt Result; + bool IsTemporary = false; + if (const MaterializeTemporaryExpr *MTE = + dyn_cast<MaterializeTemporaryExpr>(ArgE)) { + ArgE = MTE->GetTemporaryExpr(); + IsTemporary = true; + } + + Optional<SVal> ConstantVal = svalBuilder.getConstantVal(ArgE); + if (!ConstantVal) + ConstantVal = UnknownVal(); for (ExplodedNodeSet::iterator I = PreVisit.begin(), E = PreVisit.end(); I != E; ++I) { ProgramStateRef State = (*I)->getState(); - - if (ArgE->EvaluateAsInt(Result, getContext())) - V = svalBuilder.makeIntVal(Result); - else - V = State->getSVal(ArgE, LCtx); - - State = State->BindExpr(DefaultE, LCtx, V); - if (DefaultE->isGLValue()) + State = State->BindExpr(DefaultE, LCtx, *ConstantVal); + if (IsTemporary) State = createTemporaryRegionIfNeeded(State, LCtx, DefaultE, DefaultE); Bldr2.generateNode(S, *I, State); @@ -860,9 +866,13 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred, const LocationContext *LCtx = Pred->getLocationContext(); ProgramStateRef NewState = createTemporaryRegionIfNeeded(State, LCtx, OCE->getArg(0)); - if (NewState != State) + if (NewState != State) { Pred = Bldr.generateNode(OCE, Pred, NewState, /*Tag=*/0, ProgramPoint::PreStmtKind); + // Did we cache out? + if (!Pred) + break; + } } } // FALLTHROUGH @@ -1235,7 +1245,7 @@ static SVal RecoverCastedSymbol(ProgramStateManager& StateMgr, while (const CastExpr *CE = dyn_cast<CastExpr>(Ex)) { QualType T = CE->getType(); - if (!T->isIntegerType()) + if (!T->isIntegralOrEnumerationType()) return UnknownVal(); uint64_t newBits = Ctx.getTypeSize(T); @@ -1250,7 +1260,8 @@ static SVal RecoverCastedSymbol(ProgramStateManager& StateMgr, // We reached a non-cast. Is it a symbolic value? QualType T = Ex->getType(); - if (!bitsInit || !T->isIntegerType() || Ctx.getTypeSize(T) > bits) + if (!bitsInit || !T->isIntegralOrEnumerationType() || + Ctx.getTypeSize(T) > bits) return UnknownVal(); return state->getSVal(Ex, LCtx); @@ -1342,7 +1353,7 @@ void ExprEngine::processBranch(const Stmt *Condition, const Stmt *Term, if (X.isUnknownOrUndef()) { // Give it a chance to recover from unknown. if (const Expr *Ex = dyn_cast<Expr>(Condition)) { - if (Ex->getType()->isIntegerType()) { + if (Ex->getType()->isIntegralOrEnumerationType()) { // Try to recover some path-sensitivity. Right now casts of symbolic // integers that promote their values are currently not tracked well. // If 'Condition' is such an expression, try and recover the @@ -1802,7 +1813,8 @@ ExprEngine::notifyCheckersOfPointerEscape(ProgramStateRef State, return getCheckerManager().runCheckersForPointerEscape(State, *Invalidated, 0, - PSK_EscapeOther); + PSK_EscapeOther, + IsConst); // Note: Due to current limitations of RegionStore, we only process the top // level const pointers correctly. The lower level const pointers are diff --git a/lib/StaticAnalyzer/Core/ExprEngineC.cpp b/lib/StaticAnalyzer/Core/ExprEngineC.cpp index 3a3c971..67aeab6 100644 --- a/lib/StaticAnalyzer/Core/ExprEngineC.cpp +++ b/lib/StaticAnalyzer/Core/ExprEngineC.cpp @@ -68,12 +68,14 @@ void ExprEngine::VisitBinaryOperator(const BinaryOperator* B, // SymSymExpr. unsigned Count = currBldrCtx->blockCount(); if (LeftV.getAs<Loc>() && - RHS->getType()->isIntegerType() && RightV.isUnknown()) { + RHS->getType()->isIntegralOrEnumerationType() && + RightV.isUnknown()) { RightV = svalBuilder.conjureSymbolVal(RHS, LCtx, RHS->getType(), Count); } if (RightV.getAs<Loc>() && - LHS->getType()->isIntegerType() && LeftV.isUnknown()) { + LHS->getType()->isIntegralOrEnumerationType() && + LeftV.isUnknown()) { LeftV = svalBuilder.conjureSymbolVal(LHS, LCtx, LHS->getType(), Count); } @@ -401,26 +403,32 @@ void ExprEngine::VisitCompoundLiteralExpr(const CompoundLiteralExpr *CL, ExplodedNodeSet &Dst) { StmtNodeBuilder B(Pred, Dst, *currBldrCtx); - const InitListExpr *ILE - = cast<InitListExpr>(CL->getInitializer()->IgnoreParens()); + ProgramStateRef State = Pred->getState(); + const LocationContext *LCtx = Pred->getLocationContext(); + + const Expr *Init = CL->getInitializer(); + SVal V = State->getSVal(CL->getInitializer(), LCtx); - ProgramStateRef state = Pred->getState(); - SVal ILV = state->getSVal(ILE, Pred->getLocationContext()); - const LocationContext *LC = Pred->getLocationContext(); - state = state->bindCompoundLiteral(CL, LC, ILV); - - // Compound literal expressions are a GNU extension in C++. - // Unlike in C, where CLs are lvalues, in C++ CLs are prvalues, - // and like temporary objects created by the functional notation T() - // CLs are destroyed at the end of the containing full-expression. - // HOWEVER, an rvalue of array type is not something the analyzer can - // reason about, since we expect all regions to be wrapped in Locs. - // So we treat array CLs as lvalues as well, knowing that they will decay - // to pointers as soon as they are used. - if (CL->isGLValue() || CL->getType()->isArrayType()) - B.generateNode(CL, Pred, state->BindExpr(CL, LC, state->getLValue(CL, LC))); - else - B.generateNode(CL, Pred, state->BindExpr(CL, LC, ILV)); + if (isa<CXXConstructExpr>(Init)) { + // No work needed. Just pass the value up to this expression. + } else { + assert(isa<InitListExpr>(Init)); + Loc CLLoc = State->getLValue(CL, LCtx); + State = State->bindLoc(CLLoc, V); + + // Compound literal expressions are a GNU extension in C++. + // Unlike in C, where CLs are lvalues, in C++ CLs are prvalues, + // and like temporary objects created by the functional notation T() + // CLs are destroyed at the end of the containing full-expression. + // HOWEVER, an rvalue of array type is not something the analyzer can + // reason about, since we expect all regions to be wrapped in Locs. + // So we treat array CLs as lvalues as well, knowing that they will decay + // to pointers as soon as they are used. + if (CL->isGLValue() || CL->getType()->isArrayType()) + V = CLLoc; + } + + B.generateNode(CL, Pred, State->BindExpr(CL, LCtx, V)); } void ExprEngine::VisitDeclStmt(const DeclStmt *DS, ExplodedNode *Pred, @@ -615,11 +623,15 @@ void ExprEngine::VisitGuardedExpr(const Expr *Ex, const Expr *R, ExplodedNode *Pred, ExplodedNodeSet &Dst) { + assert(L && R); + StmtNodeBuilder B(Pred, Dst, *currBldrCtx); ProgramStateRef state = Pred->getState(); const LocationContext *LCtx = Pred->getLocationContext(); const CFGBlock *SrcBlock = 0; + // Find the predecessor block. + ProgramStateRef SrcState = state; for (const ExplodedNode *N = Pred ; N ; N = *N->pred_begin()) { ProgramPoint PP = N->getLocation(); if (PP.getAs<PreStmtPurgeDeadSymbols>() || PP.getAs<BlockEntrance>()) { @@ -627,6 +639,7 @@ void ExprEngine::VisitGuardedExpr(const Expr *Ex, continue; } SrcBlock = PP.castAs<BlockEdge>().getSrc(); + SrcState = N->getState(); break; } @@ -642,14 +655,25 @@ void ExprEngine::VisitGuardedExpr(const Expr *Ex, CFGElement CE = *I; if (Optional<CFGStmt> CS = CE.getAs<CFGStmt>()) { const Expr *ValEx = cast<Expr>(CS->getStmt()); - hasValue = true; - V = state->getSVal(ValEx, LCtx); + ValEx = ValEx->IgnoreParens(); + + // For GNU extension '?:' operator, the left hand side will be an + // OpaqueValueExpr, so get the underlying expression. + if (const OpaqueValueExpr *OpaqueEx = dyn_cast<OpaqueValueExpr>(L)) + L = OpaqueEx->getSourceExpr(); + + // If the last expression in the predecessor block matches true or false + // subexpression, get its the value. + if (ValEx == L->IgnoreParens() || ValEx == R->IgnoreParens()) { + hasValue = true; + V = SrcState->getSVal(ValEx, LCtx); + } break; } } - assert(hasValue); - (void) hasValue; + if (!hasValue) + V = svalBuilder.conjureSymbolVal(0, Ex, LCtx, currBldrCtx->blockCount()); // Generate a new node with the binding from the appropriate path. B.generateNode(Ex, Pred, state->BindExpr(Ex, LCtx, V, true)); @@ -662,8 +686,9 @@ VisitOffsetOfExpr(const OffsetOfExpr *OOE, APSInt IV; if (OOE->EvaluateAsInt(IV, getContext())) { assert(IV.getBitWidth() == getContext().getTypeSize(OOE->getType())); - assert(OOE->getType()->isIntegerType()); - assert(IV.isSigned() == OOE->getType()->isSignedIntegerOrEnumerationType()); + assert(OOE->getType()->isBuiltinType()); + assert(OOE->getType()->getAs<BuiltinType>()->isInteger()); + assert(IV.isSigned() == OOE->getType()->isSignedIntegerType()); SVal X = svalBuilder.makeIntVal(IV); B.generateNode(OOE, Pred, Pred->getState()->BindExpr(OOE, Pred->getLocationContext(), diff --git a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp index f01e4e7..06570a4 100644 --- a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp +++ b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp @@ -676,46 +676,40 @@ static CallInlinePolicy mayInlineCallKind(const CallEvent &Call, return CIP_Allowed; } -/// Returns true if the given C++ class is a container. -/// -/// Our heuristic for this is whether it contains a method named 'begin()' or a -/// nested type named 'iterator'. -static bool isContainerClass(const ASTContext &Ctx, const CXXRecordDecl *RD) { - // Don't record any path information. - CXXBasePaths Paths(false, false, false); - - const IdentifierInfo &BeginII = Ctx.Idents.get("begin"); - DeclarationName BeginName = Ctx.DeclarationNames.getIdentifier(&BeginII); - DeclContext::lookup_const_result BeginDecls = RD->lookup(BeginName); - if (!BeginDecls.empty()) - return true; - if (RD->lookupInBases(&CXXRecordDecl::FindOrdinaryMember, - BeginName.getAsOpaquePtr(), - Paths)) - return true; - - const IdentifierInfo &IterII = Ctx.Idents.get("iterator"); - DeclarationName IteratorName = Ctx.DeclarationNames.getIdentifier(&IterII); - DeclContext::lookup_const_result IterDecls = RD->lookup(IteratorName); - if (!IterDecls.empty()) +/// Returns true if the given C++ class contains a member with the given name. +static bool hasMember(const ASTContext &Ctx, const CXXRecordDecl *RD, + StringRef Name) { + const IdentifierInfo &II = Ctx.Idents.get(Name); + DeclarationName DeclName = Ctx.DeclarationNames.getIdentifier(&II); + if (!RD->lookup(DeclName).empty()) return true; + + CXXBasePaths Paths(false, false, false); if (RD->lookupInBases(&CXXRecordDecl::FindOrdinaryMember, - IteratorName.getAsOpaquePtr(), + DeclName.getAsOpaquePtr(), Paths)) return true; return false; } +/// Returns true if the given C++ class is a container or iterator. +/// +/// Our heuristic for this is whether it contains a method named 'begin()' or a +/// nested type named 'iterator' or 'iterator_category'. +static bool isContainerClass(const ASTContext &Ctx, const CXXRecordDecl *RD) { + return hasMember(Ctx, RD, "begin") || + hasMember(Ctx, RD, "iterator") || + hasMember(Ctx, RD, "iterator_category"); +} + /// Returns true if the given function refers to a constructor or destructor of -/// a C++ container. +/// a C++ container or iterator. /// /// We generally do a poor job modeling most containers right now, and would -/// prefer not to inline their methods. +/// prefer not to inline their setup and teardown. static bool isContainerCtorOrDtor(const ASTContext &Ctx, const FunctionDecl *FD) { - // Heuristic: a type is a container if it contains a "begin()" method - // or a type named "iterator". if (!(isa<CXXConstructorDecl>(FD) || isa<CXXDestructorDecl>(FD))) return false; diff --git a/lib/StaticAnalyzer/Core/MemRegion.cpp b/lib/StaticAnalyzer/Core/MemRegion.cpp index b3a1e65..42073d4 100644 --- a/lib/StaticAnalyzer/Core/MemRegion.cpp +++ b/lib/StaticAnalyzer/Core/MemRegion.cpp @@ -555,38 +555,75 @@ void StackLocalsSpaceRegion::dumpToStream(raw_ostream &os) const { } bool MemRegion::canPrintPretty() const { + return canPrintPrettyAsExpr(); +} + +bool MemRegion::canPrintPrettyAsExpr() const { return false; } void MemRegion::printPretty(raw_ostream &os) const { + assert(canPrintPretty() && "This region cannot be printed pretty."); + os << "'"; + printPrettyAsExpr(os); + os << "'"; + return; +} + +void MemRegion::printPrettyAsExpr(raw_ostream &os) const { + llvm_unreachable("This region cannot be printed pretty."); return; } -bool VarRegion::canPrintPretty() const { +bool VarRegion::canPrintPrettyAsExpr() const { return true; } -void VarRegion::printPretty(raw_ostream &os) const { +void VarRegion::printPrettyAsExpr(raw_ostream &os) const { os << getDecl()->getName(); } -bool ObjCIvarRegion::canPrintPretty() const { +bool ObjCIvarRegion::canPrintPrettyAsExpr() const { return true; } -void ObjCIvarRegion::printPretty(raw_ostream &os) const { +void ObjCIvarRegion::printPrettyAsExpr(raw_ostream &os) const { os << getDecl()->getName(); } bool FieldRegion::canPrintPretty() const { - return superRegion->canPrintPretty(); + return true; } -void FieldRegion::printPretty(raw_ostream &os) const { - superRegion->printPretty(os); +bool FieldRegion::canPrintPrettyAsExpr() const { + return superRegion->canPrintPrettyAsExpr(); +} + +void FieldRegion::printPrettyAsExpr(raw_ostream &os) const { + assert(canPrintPrettyAsExpr()); + superRegion->printPrettyAsExpr(os); os << "." << getDecl()->getName(); } +void FieldRegion::printPretty(raw_ostream &os) const { + if (canPrintPrettyAsExpr()) { + os << "\'"; + printPrettyAsExpr(os); + os << "'"; + } else { + os << "field " << "\'" << getDecl()->getName() << "'"; + } + return; +} + +bool CXXBaseObjectRegion::canPrintPrettyAsExpr() const { + return superRegion->canPrintPrettyAsExpr(); +} + +void CXXBaseObjectRegion::printPrettyAsExpr(raw_ostream &os) const { + superRegion->printPrettyAsExpr(os); +} + //===----------------------------------------------------------------------===// // MemRegionManager methods. //===----------------------------------------------------------------------===// @@ -1043,6 +1080,17 @@ const MemRegion *MemRegion::StripCasts(bool StripBaseCasts) const { } } +const SymbolicRegion *MemRegion::getSymbolicBase() const { + const SubRegion *SubR = dyn_cast<SubRegion>(this); + + while (SubR) { + if (const SymbolicRegion *SymR = dyn_cast<SymbolicRegion>(SubR)) + return SymR; + SubR = dyn_cast<SubRegion>(SubR->getSuperRegion()); + } + return 0; +} + // FIXME: Merge with the implementation of the same method in Store.cpp static bool IsCompleteType(ASTContext &Ctx, QualType Ty) { if (const RecordType *RT = Ty->getAs<RecordType>()) { diff --git a/lib/StaticAnalyzer/Core/PathDiagnostic.cpp b/lib/StaticAnalyzer/Core/PathDiagnostic.cpp index 7c0fb14..0351310 100644 --- a/lib/StaticAnalyzer/Core/PathDiagnostic.cpp +++ b/lib/StaticAnalyzer/Core/PathDiagnostic.cpp @@ -297,11 +297,16 @@ static Optional<bool> comparePiece(const PathDiagnosticPiece &X, static Optional<bool> comparePath(const PathPieces &X, const PathPieces &Y) { if (X.size() != Y.size()) return X.size() < Y.size(); - for (unsigned i = 0, n = X.size(); i != n; ++i) { - Optional<bool> b = comparePiece(*X[i], *Y[i]); + + PathPieces::const_iterator X_I = X.begin(), X_end = X.end(); + PathPieces::const_iterator Y_I = Y.begin(), Y_end = Y.end(); + + for ( ; X_I != X_end && Y_I != Y_end; ++X_I, ++Y_I) { + Optional<bool> b = comparePiece(**X_I, **Y_I); if (b.hasValue()) return b.getValue(); } + return None; } @@ -608,31 +613,73 @@ PathDiagnosticLocation return PathDiagnosticLocation(S, SMng, P.getLocationContext()); } +const Stmt *PathDiagnosticLocation::getStmt(const ExplodedNode *N) { + ProgramPoint P = N->getLocation(); + if (Optional<StmtPoint> SP = P.getAs<StmtPoint>()) + return SP->getStmt(); + if (Optional<BlockEdge> BE = P.getAs<BlockEdge>()) + return BE->getSrc()->getTerminator(); + if (Optional<CallEnter> CE = P.getAs<CallEnter>()) + return CE->getCallExpr(); + if (Optional<CallExitEnd> CEE = P.getAs<CallExitEnd>()) + return CEE->getCalleeContext()->getCallSite(); + if (Optional<PostInitializer> PIPP = P.getAs<PostInitializer>()) + return PIPP->getInitializer()->getInit(); + + return 0; +} + +const Stmt *PathDiagnosticLocation::getNextStmt(const ExplodedNode *N) { + for (N = N->getFirstSucc(); N; N = N->getFirstSucc()) { + if (const Stmt *S = getStmt(N)) { + // Check if the statement is '?' or '&&'/'||'. These are "merges", + // not actual statement points. + switch (S->getStmtClass()) { + case Stmt::ChooseExprClass: + case Stmt::BinaryConditionalOperatorClass: + case Stmt::ConditionalOperatorClass: + continue; + case Stmt::BinaryOperatorClass: { + BinaryOperatorKind Op = cast<BinaryOperator>(S)->getOpcode(); + if (Op == BO_LAnd || Op == BO_LOr) + continue; + break; + } + default: + break; + } + // We found the statement, so return it. + return S; + } + } + + return 0; +} + PathDiagnosticLocation - PathDiagnosticLocation::createEndOfPath(const ExplodedNode* N, + PathDiagnosticLocation::createEndOfPath(const ExplodedNode *N, const SourceManager &SM) { assert(N && "Cannot create a location with a null node."); + const Stmt *S = getStmt(N); - const ExplodedNode *NI = N; - const Stmt *S = 0; - - while (NI) { - ProgramPoint P = NI->getLocation(); - if (Optional<StmtPoint> PS = P.getAs<StmtPoint>()) { - S = PS->getStmt(); - if (P.getAs<PostStmtPurgeDeadSymbols>()) - return PathDiagnosticLocation::createEnd(S, SM, - NI->getLocationContext()); - break; - } else if (Optional<BlockEdge> BE = P.getAs<BlockEdge>()) { - S = BE->getSrc()->getTerminator(); - break; - } - NI = NI->succ_empty() ? 0 : *(NI->succ_begin()); - } + if (!S) + S = getNextStmt(N); if (S) { - const LocationContext *LC = NI->getLocationContext(); + ProgramPoint P = N->getLocation(); + const LocationContext *LC = N->getLocationContext(); + + // For member expressions, return the location of the '.' or '->'. + if (const MemberExpr *ME = dyn_cast<MemberExpr>(S)) + return PathDiagnosticLocation::createMemberLoc(ME, SM); + + // For binary operators, return the location of the operator. + if (const BinaryOperator *B = dyn_cast<BinaryOperator>(S)) + return PathDiagnosticLocation::createOperatorLoc(B, SM); + + if (P.getAs<PostStmtPurgeDeadSymbols>()) + return PathDiagnosticLocation::createEnd(S, SM, LC); + if (S->getLocStart().isValid()) return PathDiagnosticLocation(S, SM, LC); return PathDiagnosticLocation(getValidSourceLocation(S, LC), SM); diff --git a/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp b/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp index 7dcc088..8509555 100644 --- a/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp +++ b/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp @@ -296,6 +296,8 @@ static void ReportCall(raw_ostream &o, for (PathPieces::const_iterator I = P.path.begin(), E = P.path.end();I!=E;++I) ReportPiece(o, **I, FM, SM, LangOpts, indent, depth, true); + + --depth; IntrusiveRefCntPtr<PathDiagnosticEventPiece> callExit = P.getCallExitEvent(); diff --git a/lib/StaticAnalyzer/Core/ProgramState.cpp b/lib/StaticAnalyzer/Core/ProgramState.cpp index bff2242..653b69b 100644 --- a/lib/StaticAnalyzer/Core/ProgramState.cpp +++ b/lib/StaticAnalyzer/Core/ProgramState.cpp @@ -111,14 +111,6 @@ ProgramStateManager::removeDeadBindings(ProgramStateRef state, return ConstraintMgr->removeDeadBindings(Result, SymReaper); } -ProgramStateRef ProgramState::bindCompoundLiteral(const CompoundLiteralExpr *CL, - const LocationContext *LC, - SVal V) const { - const StoreRef &newStore = - getStateManager().StoreMgr->bindCompoundLiteral(getStore(), CL, LC, V); - return makeWithStore(newStore); -} - ProgramStateRef ProgramState::bindLoc(Loc LV, SVal V, bool notifyChanges) const { ProgramStateManager &Mgr = getStateManager(); ProgramStateRef newState = makeWithStore(Mgr.StoreMgr->Bind(getStore(), @@ -270,7 +262,7 @@ SVal ProgramState::getSValAsScalarOrLoc(const MemRegion *R) const { if (const TypedValueRegion *TR = dyn_cast<TypedValueRegion>(R)) { QualType T = TR->getValueType(); - if (Loc::isLocType(T) || T->isIntegerType()) + if (Loc::isLocType(T) || T->isIntegralOrEnumerationType()) return getSVal(R); } @@ -383,7 +375,7 @@ ConditionTruthVal ProgramState::isNull(SVal V) const { if (V.isConstant()) return false; - SymbolRef Sym = V.getAsSymbol(); + SymbolRef Sym = V.getAsSymbol(/* IncludeBaseRegion */ true); if (!Sym) return ConditionTruthVal(); diff --git a/lib/StaticAnalyzer/Core/RegionStore.cpp b/lib/StaticAnalyzer/Core/RegionStore.cpp index 0f4a682..88c4eee 100644 --- a/lib/StaticAnalyzer/Core/RegionStore.cpp +++ b/lib/StaticAnalyzer/Core/RegionStore.cpp @@ -19,10 +19,12 @@ #include "clang/Analysis/Analyses/LiveVariables.h" #include "clang/Analysis/AnalysisContext.h" #include "clang/Basic/TargetInfo.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h" #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h" #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/SubEngine.h" #include "llvm/ADT/ImmutableList.h" #include "llvm/ADT/ImmutableMap.h" #include "llvm/ADT/Optional.h" @@ -323,6 +325,7 @@ class invalidateRegionsWorker; class RegionStoreManager : public StoreManager { public: const RegionStoreFeatures Features; + RegionBindings::Factory RBFactory; mutable ClusterBindings::Factory CBFactory; @@ -332,6 +335,16 @@ private: SValListTy> LazyBindingsMapTy; LazyBindingsMapTy LazyBindingsMap; + /// The largest number of fields a struct can have and still be + /// considered "small". + /// + /// This is currently used to decide whether or not it is worth "forcing" a + /// LazyCompoundVal on bind. + /// + /// This is controlled by 'region-store-small-struct-limit' option. + /// To disable all small-struct-dependent behavior, set the option to "0". + unsigned SmallStructLimit; + /// \brief A helper used to populate the work list with the given set of /// regions. void populateWorkList(invalidateRegionsWorker &W, @@ -342,7 +355,14 @@ private: public: RegionStoreManager(ProgramStateManager& mgr, const RegionStoreFeatures &f) : StoreManager(mgr), Features(f), - RBFactory(mgr.getAllocator()), CBFactory(mgr.getAllocator()) {} + RBFactory(mgr.getAllocator()), CBFactory(mgr.getAllocator()), + SmallStructLimit(0) { + if (SubEngine *Eng = StateMgr.getOwningEngine()) { + AnalyzerOptions &Options = Eng->getAnalysisManager().options; + SmallStructLimit = + Options.getOptionAsInteger("region-store-small-struct-limit", 2); + } + } /// setImplicitDefaultValue - Set the default binding for the provided @@ -409,19 +429,20 @@ public: // Part of public interface to class. .getRootWithoutRetain(), *this); } - /// \brief Create a new store that binds a value to a compound literal. + /// Attempt to extract the fields of \p LCV and bind them to the struct region + /// \p R. /// - /// \param ST The original store whose bindings are the basis for the new - /// store. + /// This path is used when it seems advantageous to "force" loading the values + /// within a LazyCompoundVal to bind memberwise to the struct region, rather + /// than using a Default binding at the base of the entire region. This is a + /// heuristic attempting to avoid building long chains of LazyCompoundVals. /// - /// \param CL The compound literal to bind (the binding key). - /// - /// \param LC The LocationContext for the binding. - /// - /// \param V The value to bind to the compound literal. - StoreRef bindCompoundLiteral(Store ST, - const CompoundLiteralExpr *CL, - const LocationContext *LC, SVal V); + /// \returns The updated store bindings, or \c None if binding non-lazily + /// would be too expensive. + Optional<RegionBindingsRef> tryBindSmallStruct(RegionBindingsConstRef B, + const TypedValueRegion *R, + const RecordDecl *RD, + nonloc::LazyCompoundVal LCV); /// BindStruct - Bind a compound value to a structure. RegionBindingsRef bindStruct(RegionBindingsConstRef B, @@ -490,8 +511,7 @@ public: // Part of public interface to class. SVal getBindingForFieldOrElementCommon(RegionBindingsConstRef B, const TypedValueRegion *R, - QualType Ty, - const MemRegion *superR); + QualType Ty); SVal getLazyBinding(const SubRegion *LazyBindingRegion, RegionBindingsRef LazyBinding); @@ -604,6 +624,17 @@ ento::CreateFieldsOnlyRegionStoreManager(ProgramStateManager &StMgr) { //===----------------------------------------------------------------------===// namespace { +/// Used to determine which global regions are automatically included in the +/// initial worklist of a ClusterAnalysis. +enum GlobalsFilterKind { + /// Don't include any global regions. + GFK_None, + /// Only include system globals. + GFK_SystemOnly, + /// Include all global regions. + GFK_All +}; + template <typename DERIVED> class ClusterAnalysis { protected: @@ -620,19 +651,36 @@ protected: SValBuilder &svalBuilder; RegionBindingsRef B; - - const bool includeGlobals; +private: + GlobalsFilterKind GlobalsFilter; + +protected: const ClusterBindings *getCluster(const MemRegion *R) { return B.lookup(R); } + /// Returns true if the memory space of the given region is one of the global + /// regions specially included at the start of analysis. + bool isInitiallyIncludedGlobalRegion(const MemRegion *R) { + switch (GlobalsFilter) { + case GFK_None: + return false; + case GFK_SystemOnly: + return isa<GlobalSystemSpaceRegion>(R->getMemorySpace()); + case GFK_All: + return isa<NonStaticGlobalSpaceRegion>(R->getMemorySpace()); + } + + llvm_unreachable("unknown globals filter"); + } + public: ClusterAnalysis(RegionStoreManager &rm, ProgramStateManager &StateMgr, - RegionBindingsRef b, const bool includeGlobals) + RegionBindingsRef b, GlobalsFilterKind GFK) : RM(rm), Ctx(StateMgr.getContext()), svalBuilder(StateMgr.getSValBuilder()), - B(b), includeGlobals(includeGlobals) {} + B(b), GlobalsFilter(GFK) {} RegionBindingsRef getRegionBindings() const { return B; } @@ -650,9 +698,9 @@ public: assert(!Cluster.isEmpty() && "Empty clusters should be removed"); static_cast<DERIVED*>(this)->VisitAddedToCluster(Base, Cluster); - if (includeGlobals) - if (isa<NonStaticGlobalSpaceRegion>(Base->getMemorySpace())) - AddToWorkList(Base, &Cluster); + // If this is an interesting global region, add it the work list up front. + if (isInitiallyIncludedGlobalRegion(Base)) + AddToWorkList(WorkListElement(Base), &Cluster); } } @@ -905,8 +953,8 @@ public: InvalidatedSymbols &is, InvalidatedSymbols &inConstIS, StoreManager::InvalidatedRegions *r, - bool includeGlobals) - : ClusterAnalysis<invalidateRegionsWorker>(rm, stateMgr, b, includeGlobals), + GlobalsFilterKind GFK) + : ClusterAnalysis<invalidateRegionsWorker>(rm, stateMgr, b, GFK), Ex(ex), Count(count), LCtx(lctx), IS(is), ConstIS(inConstIS), Regions(r){} /// \param IsConst Specifies if the region we are invalidating is constant. @@ -949,6 +997,7 @@ void invalidateRegionsWorker::VisitCluster(const MemRegion *baseR, for (ClusterBindings::iterator I = C->begin(), E = C->end(); I != E; ++I) VisitBinding(I.getData()); + // Invalidate the contents of a non-const base region. if (!IsConst) B = B.remove(baseR); } @@ -981,18 +1030,19 @@ void invalidateRegionsWorker::VisitCluster(const MemRegion *baseR, } // Symbolic region? - SymbolRef RegionSym = 0; - if (const SymbolicRegion *SR = dyn_cast<SymbolicRegion>(baseR)) - RegionSym = SR->getSymbol(); + if (const SymbolicRegion *SR = dyn_cast<SymbolicRegion>(baseR)) { + SymbolRef RegionSym = SR->getSymbol(); - if (IsConst) { // Mark that symbol touched by the invalidation. - ConstIS.insert(RegionSym); - return; + if (IsConst) + ConstIS.insert(RegionSym); + else + IS.insert(RegionSym); } - - // Mark that symbol touched by the invalidation. - IS.insert(RegionSym); + + // Nothing else should be done for a const region. + if (IsConst) + return; // Otherwise, we have a normal data region. Record that we touched the region. if (Regions) @@ -1013,7 +1063,13 @@ void invalidateRegionsWorker::VisitCluster(const MemRegion *baseR, const TypedValueRegion *TR = cast<TypedValueRegion>(baseR); QualType T = TR->getValueType(); - // Invalidate the binding. + if (isInitiallyIncludedGlobalRegion(baseR)) { + // If the region is a global and we are invalidating all globals, + // erasing the entry is good enough. This causes all globals to be lazily + // symbolicated from the same base symbol. + return; + } + if (T->isStructureOrClassType()) { // Invalidate the region by setting its default value to // conjured symbol. The type of the symbol is irrelavant. @@ -1031,16 +1087,6 @@ void invalidateRegionsWorker::VisitCluster(const MemRegion *baseR, B = B.addBinding(baseR, BindingKey::Default, V); return; } - - if (includeGlobals && - isa<NonStaticGlobalSpaceRegion>(baseR->getMemorySpace())) { - // If the region is a global and we are invalidating all globals, - // just erase the entry. This causes all globals to be lazily - // symbolicated from the same base symbol. - B = B.removeBinding(baseR); - return; - } - DefinedOrUnknownSVal V = svalBuilder.conjureSymbolVal(baseR, Ex, LCtx, T,Count); @@ -1116,9 +1162,19 @@ RegionStoreManager::invalidateRegions(Store store, InvalidatedRegions *TopLevelRegions, InvalidatedRegions *TopLevelConstRegions, InvalidatedRegions *Invalidated) { - RegionBindingsRef B = RegionStoreManager::getRegionBindings(store); + GlobalsFilterKind GlobalsFilter; + if (Call) { + if (Call->isInSystemHeader()) + GlobalsFilter = GFK_SystemOnly; + else + GlobalsFilter = GFK_All; + } else { + GlobalsFilter = GFK_None; + } + + RegionBindingsRef B = getRegionBindings(store); invalidateRegionsWorker W(*this, StateMgr, B, Ex, Count, LCtx, IS, ConstIS, - Invalidated, false); + Invalidated, GlobalsFilter); // Scan the bindings and generate the clusters. W.GenerateClusters(); @@ -1138,14 +1194,17 @@ RegionStoreManager::invalidateRegions(Store store, // invalidate them. (Note that function-static and immutable globals are never // invalidated by this.) // TODO: This could possibly be more precise with modules. - if (Call) { + switch (GlobalsFilter) { + case GFK_All: + B = invalidateGlobalRegion(MemRegion::GlobalInternalSpaceRegionKind, + Ex, Count, LCtx, B, Invalidated); + // FALLTHROUGH + case GFK_SystemOnly: B = invalidateGlobalRegion(MemRegion::GlobalSystemSpaceRegionKind, Ex, Count, LCtx, B, Invalidated); - - if (!Call->isInSystemHeader()) { - B = invalidateGlobalRegion(MemRegion::GlobalInternalSpaceRegionKind, - Ex, Count, LCtx, B, Invalidated); - } + // FALLTHROUGH + case GFK_None: + break; } return StoreRef(B.asStore(), *this); @@ -1506,7 +1565,7 @@ SVal RegionStoreManager::getBindingForElement(RegionBindingsConstRef B, } } } - return getBindingForFieldOrElementCommon(B, R, R->getElementType(),superR); + return getBindingForFieldOrElementCommon(B, R, R->getElementType()); } SVal RegionStoreManager::getBindingForField(RegionBindingsConstRef B, @@ -1517,7 +1576,7 @@ SVal RegionStoreManager::getBindingForField(RegionBindingsConstRef B, return *V; QualType Ty = R->getValueType(); - return getBindingForFieldOrElementCommon(B, R, Ty, R->getSuperRegion()); + return getBindingForFieldOrElementCommon(B, R, Ty); } Optional<SVal> @@ -1580,8 +1639,7 @@ SVal RegionStoreManager::getLazyBinding(const SubRegion *LazyBindingRegion, SVal RegionStoreManager::getBindingForFieldOrElementCommon(RegionBindingsConstRef B, const TypedValueRegion *R, - QualType Ty, - const MemRegion *superR) { + QualType Ty) { // At this point we have already checked in either getBindingForElement or // getBindingForField if 'R' has a direct binding. @@ -1614,8 +1672,9 @@ RegionStoreManager::getBindingForFieldOrElementCommon(RegionBindingsConstRef B, // quickly result in a warning. bool hasPartialLazyBinding = false; - const SubRegion *Base = dyn_cast<SubRegion>(superR); - while (Base) { + const SubRegion *SR = dyn_cast<SubRegion>(R); + while (SR) { + const MemRegion *Base = SR->getSuperRegion(); if (Optional<SVal> D = getBindingForDerivedDefaultValue(B, Base, R, Ty)) { if (D->getAs<nonloc::LazyCompoundVal>()) { hasPartialLazyBinding = true; @@ -1633,7 +1692,7 @@ RegionStoreManager::getBindingForFieldOrElementCommon(RegionBindingsConstRef B, // If our super region is a field or element itself, walk up the region // hierarchy to see if there is a default value installed in an ancestor. - Base = dyn_cast<SubRegion>(Base->getSuperRegion()); + SR = dyn_cast<SubRegion>(Base); } if (R->hasStackNonParametersStorage()) { @@ -1641,7 +1700,7 @@ RegionStoreManager::getBindingForFieldOrElementCommon(RegionBindingsConstRef B, // Currently we don't reason specially about Clang-style vectors. Check // if superR is a vector and if so return Unknown. if (const TypedValueRegion *typedSuperR = - dyn_cast<TypedValueRegion>(superR)) { + dyn_cast<TypedValueRegion>(R->getSuperRegion())) { if (typedSuperR->getValueType()->isVectorType()) return UnknownVal(); } @@ -1682,26 +1741,6 @@ SVal RegionStoreManager::getBindingForObjCIvar(RegionBindingsConstRef B, return getBindingForLazySymbol(R); } -static Optional<SVal> getConstValue(SValBuilder &SVB, const VarDecl *VD) { - ASTContext &Ctx = SVB.getContext(); - if (!VD->getType().isConstQualified()) - return None; - - const Expr *Init = VD->getInit(); - if (!Init) - return None; - - llvm::APSInt Result; - if (!Init->isGLValue() && Init->EvaluateAsInt(Result, Ctx)) - return SVB.makeIntVal(Result); - - if (Init->isNullPointerConstant(Ctx, Expr::NPC_ValueDependentIsNotNull)) - return SVB.makeNull(); - - // FIXME: Handle other possible constant expressions. - return None; -} - SVal RegionStoreManager::getBindingForVar(RegionBindingsConstRef B, const VarRegion *R) { @@ -1718,8 +1757,10 @@ SVal RegionStoreManager::getBindingForVar(RegionBindingsConstRef B, return svalBuilder.getRegionValueSymbolVal(R); // Is 'VD' declared constant? If so, retrieve the constant value. - if (Optional<SVal> V = getConstValue(svalBuilder, VD)) - return *V; + if (VD->getType().isConstQualified()) + if (const Expr *Init = VD->getInit()) + if (Optional<SVal> V = svalBuilder.getConstantVal(Init)) + return *V; // This must come after the check for constants because closure-captured // constant variables may appear in UnknownSpaceRegion. @@ -1891,14 +1932,6 @@ RegionStoreManager::bind(RegionBindingsConstRef B, Loc L, SVal V) { return NewB.addBinding(BindingKey::Make(R, BindingKey::Direct), V); } -// FIXME: this method should be merged into Bind(). -StoreRef RegionStoreManager::bindCompoundLiteral(Store ST, - const CompoundLiteralExpr *CL, - const LocationContext *LC, - SVal V) { - return Bind(ST, loc::MemRegionVal(MRMgr.getCompoundLiteralRegion(CL, LC)), V); -} - RegionBindingsRef RegionStoreManager::setImplicitDefaultValue(RegionBindingsConstRef B, const MemRegion *R, @@ -1907,7 +1940,7 @@ RegionStoreManager::setImplicitDefaultValue(RegionBindingsConstRef B, if (Loc::isLocType(T)) V = svalBuilder.makeNull(); - else if (T->isIntegerType()) + else if (T->isIntegralOrEnumerationType()) V = svalBuilder.makeZeroVal(T); else if (T->isStructureOrClassType() || T->isArrayType()) { // Set the default value to a zero constant when it is a structure @@ -1977,7 +2010,7 @@ RegionStoreManager::bindArray(RegionBindingsConstRef B, else if (ElementTy->isArrayType()) NewB = bindArray(NewB, ER, *VI); else - NewB = bind(NewB, svalBuilder.makeLoc(ER), *VI); + NewB = bind(NewB, loc::MemRegionVal(ER), *VI); } // If the init list is shorter than the array length, set the @@ -2018,17 +2051,59 @@ RegionBindingsRef RegionStoreManager::bindVector(RegionBindingsConstRef B, NonLoc Idx = svalBuilder.makeArrayIndex(index); const ElementRegion *ER = MRMgr.getElementRegion(ElemType, Idx, R, Ctx); - + if (ElemType->isArrayType()) NewB = bindArray(NewB, ER, *VI); else if (ElemType->isStructureOrClassType()) NewB = bindStruct(NewB, ER, *VI); else - NewB = bind(NewB, svalBuilder.makeLoc(ER), *VI); + NewB = bind(NewB, loc::MemRegionVal(ER), *VI); } return NewB; } +Optional<RegionBindingsRef> +RegionStoreManager::tryBindSmallStruct(RegionBindingsConstRef B, + const TypedValueRegion *R, + const RecordDecl *RD, + nonloc::LazyCompoundVal LCV) { + FieldVector Fields; + + if (const CXXRecordDecl *Class = dyn_cast<CXXRecordDecl>(RD)) + if (Class->getNumBases() != 0 || Class->getNumVBases() != 0) + return None; + + for (RecordDecl::field_iterator I = RD->field_begin(), E = RD->field_end(); + I != E; ++I) { + const FieldDecl *FD = *I; + if (FD->isUnnamedBitfield()) + continue; + + // If there are too many fields, or if any of the fields are aggregates, + // just use the LCV as a default binding. + if (Fields.size() == SmallStructLimit) + return None; + + QualType Ty = FD->getType(); + if (!(Ty->isScalarType() || Ty->isReferenceType())) + return None; + + Fields.push_back(*I); + } + + RegionBindingsRef NewB = B; + + for (FieldVector::iterator I = Fields.begin(), E = Fields.end(); I != E; ++I){ + const FieldRegion *SourceFR = MRMgr.getFieldRegion(*I, LCV.getRegion()); + SVal V = getBindingForField(getRegionBindings(LCV.getStore()), SourceFR); + + const FieldRegion *DestFR = MRMgr.getFieldRegion(*I, R); + NewB = bind(NewB, loc::MemRegionVal(DestFR), V); + } + + return NewB; +} + RegionBindingsRef RegionStoreManager::bindStruct(RegionBindingsConstRef B, const TypedValueRegion* R, SVal V) { @@ -2039,13 +2114,19 @@ RegionBindingsRef RegionStoreManager::bindStruct(RegionBindingsConstRef B, assert(T->isStructureOrClassType()); const RecordType* RT = T->getAs<RecordType>(); - RecordDecl *RD = RT->getDecl(); + const RecordDecl *RD = RT->getDecl(); if (!RD->isCompleteDefinition()) return B; // Handle lazy compound values and symbolic values. - if (V.getAs<nonloc::LazyCompoundVal>() || V.getAs<nonloc::SymbolVal>()) + if (Optional<nonloc::LazyCompoundVal> LCV = + V.getAs<nonloc::LazyCompoundVal>()) { + if (Optional<RegionBindingsRef> NewB = tryBindSmallStruct(B, R, RD, *LCV)) + return *NewB; + return bindAggregate(B, R, V); + } + if (V.getAs<nonloc::SymbolVal>()) return bindAggregate(B, R, V); // We may get non-CompoundVal accidentally due to imprecise cast logic or @@ -2077,7 +2158,7 @@ RegionBindingsRef RegionStoreManager::bindStruct(RegionBindingsConstRef B, else if (FTy->isStructureOrClassType()) NewB = bindStruct(NewB, FR, *VI); else - NewB = bind(NewB, svalBuilder.makeLoc(FR), *VI); + NewB = bind(NewB, loc::MemRegionVal(FR), *VI); ++VI; } @@ -2115,8 +2196,7 @@ public: ProgramStateManager &stateMgr, RegionBindingsRef b, SymbolReaper &symReaper, const StackFrameContext *LCtx) - : ClusterAnalysis<removeDeadBindingsWorker>(rm, stateMgr, b, - /* includeGlobals = */ false), + : ClusterAnalysis<removeDeadBindingsWorker>(rm, stateMgr, b, GFK_None), SymReaper(symReaper), CurrentLCtx(LCtx) {} // Called by ClusterAnalysis. diff --git a/lib/StaticAnalyzer/Core/SValBuilder.cpp b/lib/StaticAnalyzer/Core/SValBuilder.cpp index c72e780..9d77a3e 100644 --- a/lib/StaticAnalyzer/Core/SValBuilder.cpp +++ b/lib/StaticAnalyzer/Core/SValBuilder.cpp @@ -33,7 +33,7 @@ DefinedOrUnknownSVal SValBuilder::makeZeroVal(QualType type) { if (Loc::isLocType(type)) return makeNull(); - if (type->isIntegerType()) + if (type->isIntegralOrEnumerationType()) return makeIntVal(0, type); // FIXME: Handle floats. @@ -106,12 +106,19 @@ SValBuilder::getRegionValueSymbolVal(const TypedValueRegion* region) { return nonloc::SymbolVal(sym); } -DefinedOrUnknownSVal SValBuilder::conjureSymbolVal(const void *symbolTag, - const Expr *expr, +DefinedOrUnknownSVal SValBuilder::conjureSymbolVal(const void *SymbolTag, + const Expr *Ex, const LocationContext *LCtx, - unsigned count) { - QualType T = expr->getType(); - return conjureSymbolVal(symbolTag, expr, LCtx, T, count); + unsigned Count) { + QualType T = Ex->getType(); + + // Compute the type of the result. If the expression is not an R-value, the + // result should be a location. + QualType ExType = Ex->getType(); + if (Ex->isGLValue()) + T = LCtx->getAnalysisDeclContext()->getASTContext().getPointerType(ExType); + + return conjureSymbolVal(SymbolTag, Ex, LCtx, T, Count); } DefinedOrUnknownSVal SValBuilder::conjureSymbolVal(const void *symbolTag, @@ -217,6 +224,68 @@ loc::MemRegionVal SValBuilder::getCXXThis(const CXXRecordDecl *D, return loc::MemRegionVal(getRegionManager().getCXXThisRegion(PT, SFC)); } +Optional<SVal> SValBuilder::getConstantVal(const Expr *E) { + E = E->IgnoreParens(); + + switch (E->getStmtClass()) { + // Handle expressions that we treat differently from the AST's constant + // evaluator. + case Stmt::AddrLabelExprClass: + return makeLoc(cast<AddrLabelExpr>(E)); + + case Stmt::CXXScalarValueInitExprClass: + case Stmt::ImplicitValueInitExprClass: + return makeZeroVal(E->getType()); + + case Stmt::ObjCStringLiteralClass: { + const ObjCStringLiteral *SL = cast<ObjCStringLiteral>(E); + return makeLoc(getRegionManager().getObjCStringRegion(SL)); + } + + case Stmt::StringLiteralClass: { + const StringLiteral *SL = cast<StringLiteral>(E); + return makeLoc(getRegionManager().getStringRegion(SL)); + } + + // Fast-path some expressions to avoid the overhead of going through the AST's + // constant evaluator + case Stmt::CharacterLiteralClass: { + const CharacterLiteral *C = cast<CharacterLiteral>(E); + return makeIntVal(C->getValue(), C->getType()); + } + + case Stmt::CXXBoolLiteralExprClass: + return makeBoolVal(cast<CXXBoolLiteralExpr>(E)); + + case Stmt::IntegerLiteralClass: + return makeIntVal(cast<IntegerLiteral>(E)); + + case Stmt::ObjCBoolLiteralExprClass: + return makeBoolVal(cast<ObjCBoolLiteralExpr>(E)); + + case Stmt::CXXNullPtrLiteralExprClass: + return makeNull(); + + // If we don't have a special case, fall back to the AST's constant evaluator. + default: { + // Don't try to come up with a value for materialized temporaries. + if (E->isGLValue()) + return None; + + ASTContext &Ctx = getContext(); + llvm::APSInt Result; + if (E->EvaluateAsInt(Result, Ctx)) + return makeIntVal(Result); + + if (Loc::isLocType(E->getType())) + if (E->isNullPointerConstant(Ctx, Expr::NPC_ValueDependentIsNotNull)) + return makeNull(); + + return None; + } + } +} + //===----------------------------------------------------------------------===// SVal SValBuilder::makeSymExprValNN(ProgramStateRef State, @@ -320,6 +389,22 @@ SVal SValBuilder::evalCast(SVal val, QualType castTy, QualType originalTy) { if (val.isUnknownOrUndef() || castTy == originalTy) return val; + if (castTy->isBooleanType()) { + if (val.isUnknownOrUndef()) + return val; + if (val.isConstant()) + return makeTruthVal(!val.isZeroConstant(), castTy); + if (SymbolRef Sym = val.getAsSymbol()) { + BasicValueFactory &BVF = getBasicValueFactory(); + // FIXME: If we had a state here, we could see if the symbol is known to + // be zero, but we don't. + return makeNonLoc(Sym, BO_NE, BVF.getValue(0, Sym->getType()), castTy); + } + + assert(val.getAs<Loc>()); + return makeTruthVal(true, castTy); + } + // For const casts, casts to void, just propagate the value. if (!castTy->isVariableArrayType() && !originalTy->isVariableArrayType()) if (shouldBeModeledWithNoOp(Context, Context.getPointerType(castTy), @@ -327,11 +412,11 @@ SVal SValBuilder::evalCast(SVal val, QualType castTy, QualType originalTy) { return val; // Check for casts from pointers to integers. - if (castTy->isIntegerType() && Loc::isLocType(originalTy)) + if (castTy->isIntegralOrEnumerationType() && Loc::isLocType(originalTy)) return evalCastFromLoc(val.castAs<Loc>(), castTy); // Check for casts from integers to pointers. - if (Loc::isLocType(castTy) && originalTy->isIntegerType()) { + if (Loc::isLocType(castTy) && originalTy->isIntegralOrEnumerationType()) { if (Optional<nonloc::LocAsInteger> LV = val.getAs<nonloc::LocAsInteger>()) { if (const MemRegion *R = LV->getLoc().getAsRegion()) { StoreManager &storeMgr = StateMgr.getStoreManager(); @@ -361,7 +446,7 @@ SVal SValBuilder::evalCast(SVal val, QualType castTy, QualType originalTy) { // Are we casting from an array to an integer? If so, cast the decayed // pointer value to an integer. - assert(castTy->isIntegerType()); + assert(castTy->isIntegralOrEnumerationType()); // FIXME: Keep these here for now in case we decide soon that we // need the original decayed type. @@ -373,7 +458,7 @@ SVal SValBuilder::evalCast(SVal val, QualType castTy, QualType originalTy) { // Check for casts from a region to a specific type. if (const MemRegion *R = val.getAsRegion()) { // Handle other casts of locations to integers. - if (castTy->isIntegerType()) + if (castTy->isIntegralOrEnumerationType()) return evalCastFromLoc(loc::MemRegionVal(R), castTy); // FIXME: We should handle the case where we strip off view layers to get diff --git a/lib/StaticAnalyzer/Core/SVals.cpp b/lib/StaticAnalyzer/Core/SVals.cpp index 38e216f..6506915 100644 --- a/lib/StaticAnalyzer/Core/SVals.cpp +++ b/lib/StaticAnalyzer/Core/SVals.cpp @@ -64,14 +64,18 @@ const FunctionDecl *SVal::getAsFunctionDecl() const { /// /// Implicit casts (ex: void* -> char*) can turn Symbolic region into Element /// region. If that is the case, gets the underlining region. -SymbolRef SVal::getAsLocSymbol() const { +/// When IncludeBaseRegions is set to true and the SubRegion is non-symbolic, +/// the first symbolic parent region is returned. +SymbolRef SVal::getAsLocSymbol(bool IncludeBaseRegions) const { // FIXME: should we consider SymbolRef wrapped in CodeTextRegion? if (Optional<nonloc::LocAsInteger> X = getAs<nonloc::LocAsInteger>()) return X->getLoc().getAsLocSymbol(); if (Optional<loc::MemRegionVal> X = getAs<loc::MemRegionVal>()) { - const MemRegion *R = X->stripCasts(); - if (const SymbolicRegion *SymR = dyn_cast<SymbolicRegion>(R)) + const MemRegion *R = X->getRegion(); + if (const SymbolicRegion *SymR = IncludeBaseRegions ? + R->getSymbolicBase() : + dyn_cast<SymbolicRegion>(R->StripCasts())) return SymR->getSymbol(); } return 0; @@ -99,13 +103,17 @@ SymbolRef SVal::getLocSymbolInBase() const { // TODO: The next 3 functions have to be simplified. /// \brief If this SVal wraps a symbol return that SymbolRef. -/// Otherwise return 0. -SymbolRef SVal::getAsSymbol() const { +/// Otherwise, return 0. +/// +/// Casts are ignored during lookup. +/// \param IncludeBaseRegions The boolean that controls whether the search +/// should continue to the base regions if the region is not symbolic. +SymbolRef SVal::getAsSymbol(bool IncludeBaseRegion) const { // FIXME: should we consider SymbolRef wrapped in CodeTextRegion? if (Optional<nonloc::SymbolVal> X = getAs<nonloc::SymbolVal>()) return X->getSymbol(); - return getAsLocSymbol(); + return getAsLocSymbol(IncludeBaseRegion); } /// getAsSymbolicExpression - If this Sval wraps a symbolic expression then diff --git a/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp b/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp index 9b759df..a06268d 100644 --- a/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp +++ b/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp @@ -90,20 +90,15 @@ ProgramStateRef SimpleConstraintManager::assumeAux(ProgramStateRef state, case loc::MemRegionKind: { // FIXME: Should this go into the storemanager? - const MemRegion *R = Cond.castAs<loc::MemRegionVal>().getRegion(); - const SubRegion *SubR = dyn_cast<SubRegion>(R); - - while (SubR) { - // FIXME: now we only find the first symbolic region. - if (const SymbolicRegion *SymR = dyn_cast<SymbolicRegion>(SubR)) { - const llvm::APSInt &zero = getBasicVals().getZeroWithPtrWidth(); - if (Assumption) - return assumeSymNE(state, SymR->getSymbol(), zero, zero); - else - return assumeSymEQ(state, SymR->getSymbol(), zero, zero); - } - SubR = dyn_cast<SubRegion>(SubR->getSuperRegion()); + + // FIXME: now we only find the first symbolic region. + if (const SymbolicRegion *SymR = R->getSymbolicBase()) { + const llvm::APSInt &zero = getBasicVals().getZeroWithPtrWidth(); + if (Assumption) + return assumeSymNE(state, SymR->getSymbol(), zero, zero); + else + return assumeSymEQ(state, SymR->getSymbol(), zero, zero); } // FALL-THROUGH. @@ -137,7 +132,7 @@ SimpleConstraintManager::assumeAuxForSymbol(ProgramStateRef State, QualType T = Sym->getType(); // None of the constraint solvers currently support non-integer types. - if (!T->isIntegerType()) + if (!T->isIntegralOrEnumerationType()) return State; const llvm::APSInt &zero = BVF.getValue(0, T); diff --git a/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp b/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp index 5cc8926..ee627f2 100644 --- a/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp +++ b/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp @@ -109,7 +109,7 @@ SVal SimpleSValBuilder::evalCastFromNonLoc(NonLoc val, QualType castTy) { // Only handle casts from integers to integers - if val is an integer constant // being cast to a non integer type, produce unknown. - if (!isLocType && !castTy->isIntegerType()) + if (!isLocType && !castTy->isIntegralOrEnumerationType()) return UnknownVal(); llvm::APSInt i = val.castAs<nonloc::ConcreteInt>().getValue(); @@ -137,7 +137,7 @@ SVal SimpleSValBuilder::evalCastFromLoc(Loc val, QualType castTy) { if (castTy->isUnionType()) return UnknownVal(); - if (castTy->isIntegerType()) { + if (castTy->isIntegralOrEnumerationType()) { unsigned BitWidth = Context.getTypeSize(castTy); if (!val.getAs<loc::ConcreteInt>()) @@ -438,9 +438,13 @@ SVal SimpleSValBuilder::evalBinOpNN(ProgramStateRef state, case BO_GE: case BO_EQ: case BO_NE: + assert(resultTy->isBooleanType() || + resultTy == getConditionType()); + assert(symIntExpr->getType()->isBooleanType() || + getContext().hasSameUnqualifiedType(symIntExpr->getType(), + getConditionType())); // Negate the comparison and make a value. opc = BinaryOperator::negateComparisonOp(opc); - assert(symIntExpr->getType() == resultTy); return makeNonLoc(symIntExpr->getLHS(), opc, symIntExpr->getRHS(), resultTy); } diff --git a/lib/StaticAnalyzer/Core/Store.cpp b/lib/StaticAnalyzer/Core/Store.cpp index a0c24fe..690ed08 100644 --- a/lib/StaticAnalyzer/Core/Store.cpp +++ b/lib/StaticAnalyzer/Core/Store.cpp @@ -289,62 +289,82 @@ SVal StoreManager::evalDerivedToBase(SVal Derived, QualType BaseType, return loc::MemRegionVal(BaseReg); } -SVal StoreManager::evalDynamicCast(SVal Base, QualType DerivedType, +/// Returns the static type of the given region, if it represents a C++ class +/// object. +/// +/// This handles both fully-typed regions, where the dynamic type is known, and +/// symbolic regions, where the dynamic type is merely bounded (and even then, +/// only ostensibly!), but does not take advantage of any dynamic type info. +static const CXXRecordDecl *getCXXRecordType(const MemRegion *MR) { + if (const TypedValueRegion *TVR = dyn_cast<TypedValueRegion>(MR)) + return TVR->getValueType()->getAsCXXRecordDecl(); + if (const SymbolicRegion *SR = dyn_cast<SymbolicRegion>(MR)) + return SR->getSymbol()->getType()->getPointeeCXXRecordDecl(); + return 0; +} + +SVal StoreManager::evalDynamicCast(SVal Base, QualType TargetType, bool &Failed) { Failed = false; - Optional<loc::MemRegionVal> BaseRegVal = Base.getAs<loc::MemRegionVal>(); - if (!BaseRegVal) + const MemRegion *MR = Base.getAsRegion(); + if (!MR) return UnknownVal(); - const MemRegion *BaseRegion = BaseRegVal->stripCasts(/*StripBases=*/false); // Assume the derived class is a pointer or a reference to a CXX record. - DerivedType = DerivedType->getPointeeType(); - assert(!DerivedType.isNull()); - const CXXRecordDecl *DerivedDecl = DerivedType->getAsCXXRecordDecl(); - if (!DerivedDecl && !DerivedType->isVoidType()) + TargetType = TargetType->getPointeeType(); + assert(!TargetType.isNull()); + const CXXRecordDecl *TargetClass = TargetType->getAsCXXRecordDecl(); + if (!TargetClass && !TargetType->isVoidType()) return UnknownVal(); // Drill down the CXXBaseObject chains, which represent upcasts (casts from // derived to base). - const MemRegion *SR = BaseRegion; - while (const TypedRegion *TSR = dyn_cast_or_null<TypedRegion>(SR)) { - QualType BaseType = TSR->getLocationType()->getPointeeType(); - assert(!BaseType.isNull()); - const CXXRecordDecl *SRDecl = BaseType->getAsCXXRecordDecl(); - if (!SRDecl) - return UnknownVal(); - + while (const CXXRecordDecl *MRClass = getCXXRecordType(MR)) { // If found the derived class, the cast succeeds. - if (SRDecl == DerivedDecl) - return loc::MemRegionVal(TSR); + if (MRClass == TargetClass) + return loc::MemRegionVal(MR); - if (!DerivedType->isVoidType()) { + if (!TargetType->isVoidType()) { // Static upcasts are marked as DerivedToBase casts by Sema, so this will // only happen when multiple or virtual inheritance is involved. CXXBasePaths Paths(/*FindAmbiguities=*/false, /*RecordPaths=*/true, /*DetectVirtual=*/false); - if (SRDecl->isDerivedFrom(DerivedDecl, Paths)) - return evalDerivedToBase(loc::MemRegionVal(TSR), Paths.front()); + if (MRClass->isDerivedFrom(TargetClass, Paths)) + return evalDerivedToBase(loc::MemRegionVal(MR), Paths.front()); } - if (const CXXBaseObjectRegion *R = dyn_cast<CXXBaseObjectRegion>(TSR)) + if (const CXXBaseObjectRegion *BaseR = dyn_cast<CXXBaseObjectRegion>(MR)) { // Drill down the chain to get the derived classes. - SR = R->getSuperRegion(); - else { - // We reached the bottom of the hierarchy. - - // If this is a cast to void*, return the region. - if (DerivedType->isVoidType()) - return loc::MemRegionVal(TSR); + MR = BaseR->getSuperRegion(); + continue; + } - // We did not find the derived class. We we must be casting the base to - // derived, so the cast should fail. - Failed = true; - return UnknownVal(); + // If this is a cast to void*, return the region. + if (TargetType->isVoidType()) + return loc::MemRegionVal(MR); + + // Strange use of reinterpret_cast can give us paths we don't reason + // about well, by putting in ElementRegions where we'd expect + // CXXBaseObjectRegions. If it's a valid reinterpret_cast (i.e. if the + // derived class has a zero offset from the base class), then it's safe + // to strip the cast; if it's invalid, -Wreinterpret-base-class should + // catch it. In the interest of performance, the analyzer will silently + // do the wrong thing in the invalid case (because offsets for subregions + // will be wrong). + const MemRegion *Uncasted = MR->StripCasts(/*IncludeBaseCasts=*/false); + if (Uncasted == MR) { + // We reached the bottom of the hierarchy and did not find the derived + // class. We we must be casting the base to derived, so the cast should + // fail. + break; } + + MR = Uncasted; } - + + // We failed if the region we ended up with has perfect type info. + Failed = isa<TypedValueRegion>(MR); return UnknownVal(); } diff --git a/lib/StaticAnalyzer/Core/SymbolManager.cpp b/lib/StaticAnalyzer/Core/SymbolManager.cpp index de2f5bc..7c75b6c 100644 --- a/lib/StaticAnalyzer/Core/SymbolManager.cpp +++ b/lib/StaticAnalyzer/Core/SymbolManager.cpp @@ -340,8 +340,8 @@ bool SymbolManager::canSymbolicate(QualType T) { if (Loc::isLocType(T)) return true; - if (T->isIntegerType()) - return T->isScalarType(); + if (T->isIntegralOrEnumerationType()) + return true; if (T->isRecordType() && !T->isUnionType()) return true; diff --git a/lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp b/lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp index 4fad5a8..e7def08 100644 --- a/lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp +++ b/lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp @@ -100,11 +100,12 @@ void ClangCheckerRegistry::warnIncompatible(DiagnosticsEngine *diags, } -CheckerManager *ento::createCheckerManager(const AnalyzerOptions &opts, +CheckerManager *ento::createCheckerManager(AnalyzerOptions &opts, const LangOptions &langOpts, ArrayRef<std::string> plugins, DiagnosticsEngine &diags) { - OwningPtr<CheckerManager> checkerMgr(new CheckerManager(langOpts)); + OwningPtr<CheckerManager> checkerMgr(new CheckerManager(langOpts, + &opts)); SmallVector<CheckerOptInfo, 8> checkerOpts; for (unsigned i = 0, e = opts.CheckersControlList.size(); i != e; ++i) { |
