diff options
Diffstat (limited to 'contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers')
13 files changed, 738 insertions, 273 deletions
diff --git a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp index e91a7e1..0f5741b 100644 --- a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp +++ b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp @@ -1922,10 +1922,6 @@ bool CStringChecker::evalCall(const CallExpr *CE, CheckerContext &C) const { if (!evalFunction) return false; - // Make sure each function sets its own description. - // (But don't bother in a release build.) - assert(!(CurrentFunctionDescription = nullptr)); - // Check and evaluate the call. (this->*evalFunction)(C, CE); diff --git a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp index 45768b2..0beb917 100644 --- a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp +++ b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp @@ -28,6 +28,7 @@ using namespace ento; static bool isArc4RandomAvailable(const ASTContext &Ctx) { const llvm::Triple &T = Ctx.getTargetInfo().getTriple(); return T.getVendor() == llvm::Triple::Apple || + T.getOS() == llvm::Triple::CloudABI || T.getOS() == llvm::Triple::FreeBSD || T.getOS() == llvm::Triple::NetBSD || T.getOS() == llvm::Triple::OpenBSD || diff --git a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/Checkers.td b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/Checkers.td index ba5b4fa..d1d6ac2 100644 --- a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/Checkers.td +++ b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/Checkers.td @@ -295,7 +295,7 @@ def UnixAPIChecker : Checker<"API">, HelpText<"Check calls to various UNIX/Posix functions">, DescFile<"UnixAPIChecker.cpp">; -def MallocPessimistic : Checker<"Malloc">, +def MallocChecker: Checker<"Malloc">, HelpText<"Check for memory leaks, double free, and use-after-free problems. Traces memory managed by malloc()/free().">, DescFile<"MallocChecker.cpp">; @@ -315,10 +315,6 @@ def ChrootChecker : Checker<"Chroot">, HelpText<"Check improper use of chroot">, DescFile<"ChrootChecker.cpp">; -def MallocOptimistic : Checker<"MallocWithAnnotations">, - HelpText<"Check for memory leaks, double free, and use-after-free problems. Traces memory managed by malloc()/free(). Assumes that all user-defined functions which might free a pointer are annotated.">, - DescFile<"MallocChecker.cpp">; - def PthreadLockChecker : Checker<"PthreadLock">, HelpText<"Simple lock -> unlock checker">, DescFile<"PthreadLockChecker.cpp">; diff --git a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp index c1ea767..f4be5b3 100644 --- a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp +++ b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp @@ -142,7 +142,7 @@ public: : cfg(cfg), Ctx(ctx), BR(br), Checker(checker), AC(ac), Parents(parents), Escaped(escaped), currentBlock(nullptr) {} - virtual ~DeadStoreObs() {} + ~DeadStoreObs() override {} bool isLive(const LiveVariables::LivenessValues &Live, const VarDecl *D) { if (Live.isLive(D)) diff --git a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp index 4ee0223..2e442c7 100644 --- a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp +++ b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp @@ -162,7 +162,7 @@ void DereferenceChecker::reportBug(ProgramStateRef State, const Stmt *S, os.flush(); BugReport *report = new BugReport(*BT_null, - buf.empty() ? BT_null->getDescription() : buf.str(), + buf.empty() ? BT_null->getDescription() : StringRef(buf), N); bugreporter::trackNullOrUndefValue(N, bugreporter::getDerefExpr(S), *report); diff --git a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp index 08ba26a..275481f 100644 --- a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp +++ b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp @@ -199,7 +199,7 @@ GenericTaintChecker::TaintPropagationRule::getTaintPropagationRule( const FunctionDecl *FDecl, StringRef Name, CheckerContext &C) { - // TODO: Currently, we might loose precision here: we always mark a return + // TODO: Currently, we might lose precision here: we always mark a return // value as tainted even if it's just a pointer, pointing to tainted data. // Check for exact name match for functions without builtin substitutes. diff --git a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/InterCheckerAPI.h b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/InterCheckerAPI.h index b7549fd..d38d63c 100644 --- a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/InterCheckerAPI.h +++ b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/InterCheckerAPI.h @@ -13,6 +13,8 @@ #ifndef LLVM_CLANG_LIB_STATICANALYZER_CHECKERS_INTERCHECKERAPI_H #define LLVM_CLANG_LIB_STATICANALYZER_CHECKERS_INTERCHECKERAPI_H namespace clang { +class CheckerManager; + namespace ento { /// Register the checker which evaluates CString API calls. diff --git a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp index 1926600..02c1209 100644 --- a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp +++ b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp @@ -336,7 +336,7 @@ const ObjCIvarDecl *IvarInvalidationCheckerImpl::findPropertyBackingIvar( llvm::raw_svector_ostream os(PropNameWithUnderscore); os << '_' << PropName; } - if (IvarName == PropNameWithUnderscore.str()) + if (IvarName == PropNameWithUnderscore) return Iv; } diff --git a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp index 13ea4d3..52e2936 100644 --- a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp +++ b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp @@ -137,7 +137,7 @@ private: public: SecKeychainBugVisitor(SymbolRef S) : Sym(S) {} - virtual ~SecKeychainBugVisitor() {} + ~SecKeychainBugVisitor() override {} void Profile(llvm::FoldingSetNodeID &ID) const override { static int X = 0; @@ -292,7 +292,11 @@ void MacOSKeychainAPIChecker::checkPreStmt(const CallExpr *CE, // If it is a call to an allocator function, it could be a double allocation. idx = getTrackedFunctionIndex(funName, true); if (idx != InvalidIdx) { - const Expr *ArgExpr = CE->getArg(FunctionsToTrack[idx].Param); + unsigned paramIdx = FunctionsToTrack[idx].Param; + if (CE->getNumArgs() <= paramIdx) + return; + + const Expr *ArgExpr = CE->getArg(paramIdx); if (SymbolRef V = getAsPointeeSymbol(ArgExpr, C)) if (const AllocationState *AS = State->get<AllocatedData>(V)) { if (!definitelyReturnedError(AS->Region, State, C.getSValBuilder())) { @@ -325,8 +329,12 @@ void MacOSKeychainAPIChecker::checkPreStmt(const CallExpr *CE, if (idx == InvalidIdx) return; + unsigned paramIdx = FunctionsToTrack[idx].Param; + if (CE->getNumArgs() <= paramIdx) + return; + // Check the argument to the deallocator. - const Expr *ArgExpr = CE->getArg(FunctionsToTrack[idx].Param); + const Expr *ArgExpr = CE->getArg(paramIdx); SVal ArgSVal = State->getSVal(ArgExpr, C.getLocationContext()); // Undef is reported by another checker. @@ -499,9 +507,11 @@ MacOSKeychainAPIChecker::getAllocationNode(const ExplodedNode *N, while (N) { if (!N->getState()->get<AllocatedData>(Sym)) break; - // Allocation node, is the last node in the current context in which the - // symbol was tracked. - if (N->getLocationContext() == LeakContext) + // Allocation node, is the last node in the current or parent context in + // which the symbol was tracked. + const LocationContext *NContext = N->getLocationContext(); + if (NContext == LeakContext || + NContext->isParentOf(LeakContext)) AllocNode = N; N = N->pred_empty() ? nullptr : *(N->pred_begin()); } diff --git a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp index aee5a43..0cf0094 100644 --- a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -43,12 +43,15 @@ enum AllocationFamily { AF_Malloc, AF_CXXNew, AF_CXXNewArray, - AF_IfNameIndex + AF_IfNameIndex, + AF_Alloca }; class RefState { enum Kind { // Reference to allocated memory. Allocated, + // Reference to zero-allocated memory. + AllocatedOfSizeZero, // Reference to released/freed memory. Released, // The responsibility for freeing resources has transferred from @@ -61,8 +64,8 @@ class RefState { }; const Stmt *S; - unsigned K : 2; // Kind enum, but stored as a bitfield. - unsigned Family : 30; // Rest of 32-bit word, currently just an allocation + unsigned K : 3; // Kind enum, but stored as a bitfield. + unsigned Family : 29; // Rest of 32-bit word, currently just an allocation // family. RefState(Kind k, const Stmt *s, unsigned family) @@ -71,6 +74,7 @@ class RefState { } public: bool isAllocated() const { return K == Allocated; } + bool isAllocatedOfSizeZero() const { return K == AllocatedOfSizeZero; } bool isReleased() const { return K == Released; } bool isRelinquished() const { return K == Relinquished; } bool isEscaped() const { return K == Escaped; } @@ -86,6 +90,10 @@ public: static RefState getAllocated(unsigned family, const Stmt *s) { return RefState(Allocated, s, family); } + static RefState getAllocatedOfSizeZero(const RefState *RS) { + return RefState(AllocatedOfSizeZero, RS->getStmt(), + RS->getAllocationFamily()); + } static RefState getReleased(unsigned family, const Stmt *s) { return RefState(Released, s, family); } @@ -106,6 +114,7 @@ public: switch (static_cast<Kind>(K)) { #define CASE(ID) case ID: OS << #ID; break; CASE(Allocated) + CASE(AllocatedOfSizeZero) CASE(Released) CASE(Relinquished) CASE(Escaped) @@ -160,16 +169,16 @@ class MallocChecker : public Checker<check::DeadSymbols, { public: MallocChecker() - : II_malloc(nullptr), II_free(nullptr), II_realloc(nullptr), - II_calloc(nullptr), II_valloc(nullptr), II_reallocf(nullptr), - II_strndup(nullptr), II_strdup(nullptr), II_kmalloc(nullptr), - II_if_nameindex(nullptr), II_if_freenameindex(nullptr) {} + : II_alloca(nullptr), II_malloc(nullptr), II_free(nullptr), + II_realloc(nullptr), II_calloc(nullptr), II_valloc(nullptr), + II_reallocf(nullptr), II_strndup(nullptr), II_strdup(nullptr), + II_kmalloc(nullptr), II_if_nameindex(nullptr), + II_if_freenameindex(nullptr) {} /// In pessimistic mode, the checker assumes that it does not know which /// functions might free the memory. enum CheckKind { - CK_MallocPessimistic, - CK_MallocOptimistic, + CK_MallocChecker, CK_NewDeleteChecker, CK_NewDeleteLeaksChecker, CK_MismatchedDeallocatorChecker, @@ -182,6 +191,8 @@ public: MOK_Any }; + DefaultBool IsOptimistic; + DefaultBool ChecksEnabled[CK_NumCheckKinds]; CheckName CheckNames[CK_NumCheckKinds]; @@ -216,11 +227,14 @@ private: mutable std::unique_ptr<BugType> BT_Leak[CK_NumCheckKinds]; mutable std::unique_ptr<BugType> BT_UseFree[CK_NumCheckKinds]; mutable std::unique_ptr<BugType> BT_BadFree[CK_NumCheckKinds]; + mutable std::unique_ptr<BugType> BT_FreeAlloca[CK_NumCheckKinds]; mutable std::unique_ptr<BugType> BT_MismatchedDealloc; mutable std::unique_ptr<BugType> BT_OffsetFree[CK_NumCheckKinds]; - mutable IdentifierInfo *II_malloc, *II_free, *II_realloc, *II_calloc, - *II_valloc, *II_reallocf, *II_strndup, *II_strdup, - *II_kmalloc, *II_if_nameindex, *II_if_freenameindex; + mutable std::unique_ptr<BugType> BT_UseZerroAllocated[CK_NumCheckKinds]; + mutable IdentifierInfo *II_alloca, *II_malloc, *II_free, *II_realloc, + *II_calloc, *II_valloc, *II_reallocf, *II_strndup, + *II_strdup, *II_kmalloc, *II_if_nameindex, + *II_if_freenameindex; mutable Optional<uint64_t> KernelZeroFlagVal; void initIdentifierInfo(ASTContext &C) const; @@ -252,22 +266,24 @@ private: MemoryOperationKind MemKind) const; bool isStandardNewDelete(const FunctionDecl *FD, ASTContext &C) const; ///@} + + /// \brief Perform a zero-allocation check. + ProgramStateRef ProcessZeroAllocation(CheckerContext &C, const Expr *E, + const unsigned AllocationSizeArg, + ProgramStateRef State) const; + ProgramStateRef MallocMemReturnsAttr(CheckerContext &C, const CallExpr *CE, - const OwnershipAttr* Att) const; + const OwnershipAttr* Att, + ProgramStateRef State) const; static ProgramStateRef MallocMemAux(CheckerContext &C, const CallExpr *CE, - const Expr *SizeEx, SVal Init, - ProgramStateRef State, - AllocationFamily Family = AF_Malloc) { - return MallocMemAux(C, CE, - State->getSVal(SizeEx, C.getLocationContext()), - Init, State, Family); - } - + const Expr *SizeEx, SVal Init, + ProgramStateRef State, + AllocationFamily Family = AF_Malloc); static ProgramStateRef MallocMemAux(CheckerContext &C, const CallExpr *CE, - SVal SizeEx, SVal Init, - ProgramStateRef State, - AllocationFamily Family = AF_Malloc); + SVal SizeEx, SVal Init, + ProgramStateRef State, + AllocationFamily Family = AF_Malloc); // Check if this malloc() for special flags. At present that means M_ZERO or // __GFP_ZERO (in which case, treat it like calloc). @@ -281,7 +297,8 @@ private: AllocationFamily Family = AF_Malloc); ProgramStateRef FreeMemAttr(CheckerContext &C, const CallExpr *CE, - const OwnershipAttr* Att) const; + const OwnershipAttr* Att, + ProgramStateRef State) const; ProgramStateRef FreeMemAux(CheckerContext &C, const CallExpr *CE, ProgramStateRef state, unsigned Num, bool Hold, @@ -295,14 +312,19 @@ private: bool ReturnsNullOnFailure = false) const; ProgramStateRef ReallocMem(CheckerContext &C, const CallExpr *CE, - bool FreesMemOnFailure) const; - static ProgramStateRef CallocMem(CheckerContext &C, const CallExpr *CE); + bool FreesMemOnFailure, + ProgramStateRef State) const; + static ProgramStateRef CallocMem(CheckerContext &C, const CallExpr *CE, + ProgramStateRef State); ///\brief Check if the memory associated with this symbol was released. bool isReleased(SymbolRef Sym, CheckerContext &C) const; bool checkUseAfterFree(SymbolRef Sym, CheckerContext &C, const Stmt *S) const; + void checkUseZeroAllocated(SymbolRef Sym, CheckerContext &C, + const Stmt *S) const; + bool checkDoubleDelete(SymbolRef Sym, CheckerContext &C) const; /// Check if the function is known free memory, or if it is @@ -330,15 +352,20 @@ private: /// Tells if a given family/call/symbol is tracked by the current checker. /// Sets CheckKind to the kind of the checker responsible for this /// family/call/symbol. - Optional<CheckKind> getCheckIfTracked(AllocationFamily Family) const; + Optional<CheckKind> getCheckIfTracked(AllocationFamily Family, + bool IsALeakCheck = false) const; Optional<CheckKind> getCheckIfTracked(CheckerContext &C, - const Stmt *AllocDeallocStmt) const; - Optional<CheckKind> getCheckIfTracked(CheckerContext &C, SymbolRef Sym) const; + const Stmt *AllocDeallocStmt, + bool IsALeakCheck = false) const; + Optional<CheckKind> getCheckIfTracked(CheckerContext &C, SymbolRef Sym, + bool IsALeakCheck = false) 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 ReportFreeAlloca(CheckerContext &C, SVal ArgVal, + SourceRange Range) const; void ReportMismatchedDealloc(CheckerContext &C, SourceRange Range, const Expr *DeallocExpr, const RefState *RS, SymbolRef Sym, bool OwnershipTransferred) const; @@ -352,6 +379,9 @@ private: void ReportDoubleDelete(CheckerContext &C, SymbolRef Sym) const; + void ReportUseZeroAllocated(CheckerContext &C, SourceRange Range, + SymbolRef Sym) const; + /// Find the location of the allocation for Sym on the path leading to the /// exploded node N. LeakInfo getAllocationSite(const ExplodedNode *N, SymbolRef Sym, @@ -384,7 +414,7 @@ private: MallocBugVisitor(SymbolRef S, bool isLeak = false) : Sym(S), Mode(Normal), FailedReallocSymbol(nullptr), IsLeak(isLeak) {} - virtual ~MallocBugVisitor() {} + ~MallocBugVisitor() override {} void Profile(llvm::FoldingSetNodeID &ID) const override { static int X = 0; @@ -396,7 +426,9 @@ private: const Stmt *Stmt) { // Did not track -> allocated. Other state (released) -> allocated. return (Stmt && (isa<CallExpr>(Stmt) || isa<CXXNewExpr>(Stmt)) && - (S && S->isAllocated()) && (!SPrev || !SPrev->isAllocated())); + (S && (S->isAllocated() || S->isAllocatedOfSizeZero())) && + (!SPrev || !(SPrev->isAllocated() || + SPrev->isAllocatedOfSizeZero()))); } inline bool isReleased(const RefState *S, const RefState *SPrev, @@ -422,7 +454,9 @@ private: // check. If we have to handle more cases here, it might be cleaner just // to track this extra bit in the state itself. return ((!Stmt || !isa<CallExpr>(Stmt)) && - (S && S->isAllocated()) && (SPrev && !SPrev->isAllocated())); + (S && (S->isAllocated() || S->isAllocatedOfSizeZero())) && + (SPrev && !(SPrev->isAllocated() || + SPrev->isAllocatedOfSizeZero()))); } PathDiagnosticPiece *VisitNode(const ExplodedNode *N, @@ -497,6 +531,7 @@ public: void MallocChecker::initIdentifierInfo(ASTContext &Ctx) const { if (II_malloc) return; + II_alloca = &Ctx.Idents.get("alloca"); II_malloc = &Ctx.Idents.get("malloc"); II_free = &Ctx.Idents.get("free"); II_realloc = &Ctx.Idents.get("realloc"); @@ -517,6 +552,9 @@ bool MallocChecker::isMemFunction(const FunctionDecl *FD, ASTContext &C) const { if (isCMemFunction(FD, C, AF_IfNameIndex, MemoryOperationKind::MOK_Any)) return true; + if (isCMemFunction(FD, C, AF_Alloca, MemoryOperationKind::MOK_Any)) + return true; + if (isStandardNewDelete(FD, C)) return true; @@ -560,12 +598,17 @@ bool MallocChecker::isCMemFunction(const FunctionDecl *FD, if (FunI == II_if_nameindex) return true; } + + if (Family == AF_Alloca && CheckAlloc) { + if (FunI == II_alloca) + return true; + } } if (Family != AF_Malloc) return false; - if (ChecksEnabled[CK_MallocOptimistic] && FD->hasAttrs()) { + if (IsOptimistic && FD->hasAttrs()) { for (const auto *I : FD->specific_attrs<OwnershipAttr>()) { OwnershipAttr::OwnershipKind OwnKind = I->getOwnKind(); if(OwnKind == OwnershipAttr::Takes || OwnKind == OwnershipAttr::Holds) { @@ -712,6 +755,8 @@ void MallocChecker::checkPostStmt(const CallExpr *CE, CheckerContext &C) const { return; if (CE->getNumArgs() < 3) { State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State); + if (CE->getNumArgs() == 1) + State = ProcessZeroAllocation(C, CE, 0, State); } else if (CE->getNumArgs() == 3) { llvm::Optional<ProgramStateRef> MaybeState = performKernelMalloc(CE, C, State); @@ -731,31 +776,43 @@ void MallocChecker::checkPostStmt(const CallExpr *CE, CheckerContext &C) const { if (CE->getNumArgs() < 1) return; State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State); + State = ProcessZeroAllocation(C, CE, 0, State); } else if (FunI == II_realloc) { - State = ReallocMem(C, CE, false); + State = ReallocMem(C, CE, false, State); + State = ProcessZeroAllocation(C, CE, 1, State); } else if (FunI == II_reallocf) { - State = ReallocMem(C, CE, true); + State = ReallocMem(C, CE, true, State); + State = ProcessZeroAllocation(C, CE, 1, State); } else if (FunI == II_calloc) { - State = CallocMem(C, CE); + State = CallocMem(C, CE, State); + State = ProcessZeroAllocation(C, CE, 0, State); + State = ProcessZeroAllocation(C, CE, 1, State); } else if (FunI == II_free) { State = FreeMemAux(C, CE, State, 0, false, ReleasedAllocatedMemory); } else if (FunI == II_strdup) { State = MallocUpdateRefState(C, CE, State); } else if (FunI == II_strndup) { State = MallocUpdateRefState(C, CE, State); - } - else if (isStandardNewDelete(FD, C.getASTContext())) { + } else if (FunI == II_alloca) { + State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State, + AF_Alloca); + State = ProcessZeroAllocation(C, CE, 0, State); + } else if (isStandardNewDelete(FD, C.getASTContext())) { // Process direct calls to operator new/new[]/delete/delete[] functions // as distinct from new/new[]/delete/delete[] expressions that are // processed by the checkPostStmt callbacks for CXXNewExpr and // CXXDeleteExpr. OverloadedOperatorKind K = FD->getOverloadedOperator(); - if (K == OO_New) + if (K == OO_New) { State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State, AF_CXXNew); - else if (K == OO_Array_New) + State = ProcessZeroAllocation(C, CE, 0, State); + } + else if (K == OO_Array_New) { State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State, AF_CXXNewArray); + State = ProcessZeroAllocation(C, CE, 0, State); + } else if (K == OO_Delete || K == OO_Array_Delete) State = FreeMemAux(C, CE, State, 0, false, ReleasedAllocatedMemory); else @@ -770,19 +827,18 @@ void MallocChecker::checkPostStmt(const CallExpr *CE, CheckerContext &C) const { } } - if (ChecksEnabled[CK_MallocOptimistic] || - ChecksEnabled[CK_MismatchedDeallocatorChecker]) { + if (IsOptimistic || ChecksEnabled[CK_MismatchedDeallocatorChecker]) { // Check all the attributes, if there are any. // There can be multiple of these attributes. if (FD->hasAttrs()) for (const auto *I : FD->specific_attrs<OwnershipAttr>()) { switch (I->getOwnKind()) { case OwnershipAttr::Returns: - State = MallocMemReturnsAttr(C, CE, I); + State = MallocMemReturnsAttr(C, CE, I, State); break; case OwnershipAttr::Takes: case OwnershipAttr::Holds: - State = FreeMemAttr(C, CE, I); + State = FreeMemAttr(C, CE, I, State); break; } } @@ -790,6 +846,68 @@ void MallocChecker::checkPostStmt(const CallExpr *CE, CheckerContext &C) const { C.addTransition(State); } +// Performs a 0-sized allocations check. +ProgramStateRef MallocChecker::ProcessZeroAllocation(CheckerContext &C, + const Expr *E, + const unsigned AllocationSizeArg, + ProgramStateRef State) const { + if (!State) + return nullptr; + + const Expr *Arg = nullptr; + + if (const CallExpr *CE = dyn_cast<CallExpr>(E)) { + Arg = CE->getArg(AllocationSizeArg); + } + else if (const CXXNewExpr *NE = dyn_cast<CXXNewExpr>(E)) { + if (NE->isArray()) + Arg = NE->getArraySize(); + else + return State; + } + else + llvm_unreachable("not a CallExpr or CXXNewExpr"); + + assert(Arg); + + Optional<DefinedSVal> DefArgVal = + State->getSVal(Arg, C.getLocationContext()).getAs<DefinedSVal>(); + + if (!DefArgVal) + return State; + + // Check if the allocation size is 0. + ProgramStateRef TrueState, FalseState; + SValBuilder &SvalBuilder = C.getSValBuilder(); + DefinedSVal Zero = + SvalBuilder.makeZeroVal(Arg->getType()).castAs<DefinedSVal>(); + + std::tie(TrueState, FalseState) = + State->assume(SvalBuilder.evalEQ(State, *DefArgVal, Zero)); + + if (TrueState && !FalseState) { + SVal retVal = State->getSVal(E, C.getLocationContext()); + SymbolRef Sym = retVal.getAsLocSymbol(); + if (!Sym) + return State; + + const RefState *RS = State->get<RegionState>(Sym); + if (!RS) + return State; // TODO: change to assert(RS); after realloc() will + // guarantee have a RegionState attached. + + if (!RS->isAllocated()) + return State; + + return TrueState->set<RegionState>(Sym, + RefState::getAllocatedOfSizeZero(RS)); + } + + // Assume the value is non-zero going forward. + assert(FalseState); + return FalseState; +} + static QualType getDeepPointeeType(QualType T) { QualType Result = T, PointeeType = T->getPointeeType(); while (!PointeeType.isNull()) { @@ -849,6 +967,7 @@ void MallocChecker::checkPostStmt(const CXXNewExpr *NE, // existing binding. State = MallocUpdateRefState(C, NE, State, NE->isArray() ? AF_CXXNewArray : AF_CXXNew); + State = ProcessZeroAllocation(C, NE, 0, State); C.addTransition(State); } @@ -919,15 +1038,31 @@ void MallocChecker::checkPostObjCMessage(const ObjCMethodCall &Call, ProgramStateRef MallocChecker::MallocMemReturnsAttr(CheckerContext &C, const CallExpr *CE, - const OwnershipAttr *Att) const { + const OwnershipAttr *Att, + ProgramStateRef State) const { + if (!State) + return nullptr; + if (Att->getModule() != II_malloc) return nullptr; OwnershipAttr::args_iterator I = Att->args_begin(), E = Att->args_end(); if (I != E) { - return MallocMemAux(C, CE, CE->getArg(*I), UndefinedVal(), C.getState()); + return MallocMemAux(C, CE, CE->getArg(*I), UndefinedVal(), State); } - return MallocMemAux(C, CE, UnknownVal(), UndefinedVal(), C.getState()); + return MallocMemAux(C, CE, UnknownVal(), UndefinedVal(), State); +} + +ProgramStateRef MallocChecker::MallocMemAux(CheckerContext &C, + const CallExpr *CE, + const Expr *SizeEx, SVal Init, + ProgramStateRef State, + AllocationFamily Family) { + if (!State) + return nullptr; + + return MallocMemAux(C, CE, State->getSVal(SizeEx, C.getLocationContext()), + Init, State, Family); } ProgramStateRef MallocChecker::MallocMemAux(CheckerContext &C, @@ -935,6 +1070,8 @@ ProgramStateRef MallocChecker::MallocMemAux(CheckerContext &C, SVal Size, SVal Init, ProgramStateRef State, AllocationFamily Family) { + if (!State) + return nullptr; // We expect the malloc functions to return a pointer. if (!Loc::isLocType(CE->getType())) @@ -976,6 +1113,9 @@ ProgramStateRef MallocChecker::MallocUpdateRefState(CheckerContext &C, const Expr *E, ProgramStateRef State, AllocationFamily Family) { + if (!State) + return nullptr; + // Get the return value. SVal retVal = State->getSVal(E, C.getLocationContext()); @@ -992,11 +1132,14 @@ ProgramStateRef MallocChecker::MallocUpdateRefState(CheckerContext &C, ProgramStateRef MallocChecker::FreeMemAttr(CheckerContext &C, const CallExpr *CE, - const OwnershipAttr *Att) const { + const OwnershipAttr *Att, + ProgramStateRef State) const { + if (!State) + return nullptr; + if (Att->getModule() != II_malloc) return nullptr; - ProgramStateRef State = C.getState(); bool ReleasedAllocated = false; for (const auto &Arg : Att->args()) { @@ -1011,15 +1154,18 @@ ProgramStateRef MallocChecker::FreeMemAttr(CheckerContext &C, ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C, const CallExpr *CE, - ProgramStateRef state, + ProgramStateRef State, unsigned Num, bool Hold, bool &ReleasedAllocated, bool ReturnsNullOnFailure) const { + if (!State) + return nullptr; + if (CE->getNumArgs() < (Num + 1)) return nullptr; - return FreeMemAux(C, CE->getArg(Num), CE, state, Hold, + return FreeMemAux(C, CE->getArg(Num), CE, State, Hold, ReleasedAllocated, ReturnsNullOnFailure); } @@ -1065,6 +1211,9 @@ AllocationFamily MallocChecker::getAllocationFamily(CheckerContext &C, if (isCMemFunction(FD, Ctx, AF_IfNameIndex, MemoryOperationKind::MOK_Any)) return AF_IfNameIndex; + if (isCMemFunction(FD, Ctx, AF_Alloca, MemoryOperationKind::MOK_Any)) + return AF_Alloca; + return AF_None; } @@ -1129,6 +1278,7 @@ void MallocChecker::printExpectedAllocName(raw_ostream &os, CheckerContext &C, case AF_CXXNew: os << "'new'"; return; case AF_CXXNewArray: os << "'new[]'"; return; case AF_IfNameIndex: os << "'if_nameindex()'"; return; + case AF_Alloca: case AF_None: llvm_unreachable("not a deallocation expression"); } } @@ -1140,7 +1290,8 @@ void MallocChecker::printExpectedDeallocName(raw_ostream &os, case AF_CXXNew: os << "'delete'"; return; case AF_CXXNewArray: os << "'delete[]'"; return; case AF_IfNameIndex: os << "'if_freenameindex()'"; return; - case AF_None: llvm_unreachable("suspicious AF_None argument"); + case AF_Alloca: + case AF_None: llvm_unreachable("suspicious argument"); } } @@ -1152,6 +1303,9 @@ ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C, bool &ReleasedAllocated, bool ReturnsNullOnFailure) const { + if (!State) + return nullptr; + SVal ArgVal = State->getSVal(ArgExpr, C.getLocationContext()); if (!ArgVal.getAs<DefinedOrUnknownSVal>()) return nullptr; @@ -1191,8 +1345,8 @@ ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C, const MemSpaceRegion *MS = R->getMemorySpace(); - // Parameters, locals, statics, globals, and memory returned by alloca() - // shouldn't be freed. + // Parameters, locals, statics, globals, and memory returned by + // __builtin_alloca() shouldn't be freed. if (!(isa<UnknownSpaceRegion>(MS) || isa<HeapSpaceRegion>(MS))) { // FIXME: at the time this code was written, malloc() regions were // represented by conjured symbols, which are all in UnknownSpaceRegion. @@ -1201,8 +1355,12 @@ ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C, // Of course, free() can work on memory allocated outside the current // function, so UnknownSpaceRegion is always a possibility. // False negatives are better than false positives. - - ReportBadFree(C, ArgVal, ArgExpr->getSourceRange(), ParentExpr); + + if (isa<AllocaRegion>(R)) + ReportFreeAlloca(C, ArgVal, ArgExpr->getSourceRange()); + else + ReportBadFree(C, ArgVal, ArgExpr->getSourceRange(), ParentExpr); + return nullptr; } @@ -1218,6 +1376,12 @@ ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C, if (RsBase) { + // Memory returned by alloca() shouldn't be freed. + if (RsBase->getAllocationFamily() == AF_Alloca) { + ReportFreeAlloca(C, ArgVal, ArgExpr->getSourceRange()); + return nullptr; + } + // Check for double free first. if ((RsBase->isReleased() || RsBase->isRelinquished()) && !didPreviousFreeFail(State, SymBase, PreviousRetStatusSymbol)) { @@ -1227,7 +1391,8 @@ ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C, // 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()) { + } else if (RsBase->isAllocated() || RsBase->isAllocatedOfSizeZero() || + RsBase->isEscaped()) { // Check if an expected deallocation function matches the real one. bool DeallocMatchesAlloc = @@ -1252,7 +1417,8 @@ ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C, } } - ReleasedAllocated = (RsBase != nullptr) && RsBase->isAllocated(); + ReleasedAllocated = (RsBase != nullptr) && (RsBase->isAllocated() || + RsBase->isAllocatedOfSizeZero()); // Clean out the info on previous call to free return info. State = State->remove<FreeReturnValue>(SymBase); @@ -1281,21 +1447,26 @@ ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C, } Optional<MallocChecker::CheckKind> -MallocChecker::getCheckIfTracked(AllocationFamily Family) const { +MallocChecker::getCheckIfTracked(AllocationFamily Family, + bool IsALeakCheck) const { switch (Family) { case AF_Malloc: + case AF_Alloca: case AF_IfNameIndex: { - if (ChecksEnabled[CK_MallocOptimistic]) { - return CK_MallocOptimistic; - } else if (ChecksEnabled[CK_MallocPessimistic]) { - return CK_MallocPessimistic; - } + if (ChecksEnabled[CK_MallocChecker]) + return CK_MallocChecker; + return Optional<MallocChecker::CheckKind>(); } case AF_CXXNew: case AF_CXXNewArray: { - if (ChecksEnabled[CK_NewDeleteChecker]) { - return CK_NewDeleteChecker; + if (IsALeakCheck) { + if (ChecksEnabled[CK_NewDeleteLeaksChecker]) + return CK_NewDeleteLeaksChecker; + } + else { + if (ChecksEnabled[CK_NewDeleteChecker]) + return CK_NewDeleteChecker; } return Optional<MallocChecker::CheckKind>(); } @@ -1308,16 +1479,18 @@ MallocChecker::getCheckIfTracked(AllocationFamily Family) const { Optional<MallocChecker::CheckKind> MallocChecker::getCheckIfTracked(CheckerContext &C, - const Stmt *AllocDeallocStmt) const { - return getCheckIfTracked(getAllocationFamily(C, AllocDeallocStmt)); + const Stmt *AllocDeallocStmt, + bool IsALeakCheck) const { + return getCheckIfTracked(getAllocationFamily(C, AllocDeallocStmt), + IsALeakCheck); } Optional<MallocChecker::CheckKind> -MallocChecker::getCheckIfTracked(CheckerContext &C, SymbolRef Sym) const { - +MallocChecker::getCheckIfTracked(CheckerContext &C, SymbolRef Sym, + bool IsALeakCheck) const { const RefState *RS = C.getState()->get<RegionState>(Sym); assert(RS); - return getCheckIfTracked(RS->getAllocationFamily()); + return getCheckIfTracked(RS->getAllocationFamily(), IsALeakCheck); } bool MallocChecker::SummarizeValue(raw_ostream &os, SVal V) { @@ -1411,8 +1584,7 @@ void MallocChecker::ReportBadFree(CheckerContext &C, SVal ArgVal, SourceRange Range, const Expr *DeallocExpr) const { - if (!ChecksEnabled[CK_MallocOptimistic] && - !ChecksEnabled[CK_MallocPessimistic] && + if (!ChecksEnabled[CK_MallocChecker] && !ChecksEnabled[CK_NewDeleteChecker]) return; @@ -1433,23 +1605,19 @@ void MallocChecker::ReportBadFree(CheckerContext &C, SVal ArgVal, while (const ElementRegion *ER = dyn_cast_or_null<ElementRegion>(MR)) MR = ER->getSuperRegion(); - if (MR && isa<AllocaRegion>(MR)) - os << "Memory allocated by alloca() should not be deallocated"; - else { - os << "Argument to "; - if (!printAllocDeallocName(os, C, DeallocExpr)) - os << "deallocator"; - - os << " is "; - bool Summarized = MR ? SummarizeRegion(os, MR) - : SummarizeValue(os, ArgVal); - if (Summarized) - os << ", which is not memory allocated by "; - else - os << "not memory allocated by "; + os << "Argument to "; + if (!printAllocDeallocName(os, C, DeallocExpr)) + os << "deallocator"; - printExpectedAllocName(os, C, DeallocExpr); - } + os << " is "; + bool Summarized = MR ? SummarizeRegion(os, MR) + : SummarizeValue(os, ArgVal); + if (Summarized) + os << ", which is not memory allocated by "; + else + os << "not memory allocated by "; + + printExpectedAllocName(os, C, DeallocExpr); BugReport *R = new BugReport(*BT_BadFree[*CheckKind], os.str(), N); R->markInteresting(MR); @@ -1458,6 +1626,31 @@ void MallocChecker::ReportBadFree(CheckerContext &C, SVal ArgVal, } } +void MallocChecker::ReportFreeAlloca(CheckerContext &C, SVal ArgVal, + SourceRange Range) const { + + Optional<MallocChecker::CheckKind> CheckKind; + + if (ChecksEnabled[CK_MallocChecker]) + CheckKind = CK_MallocChecker; + else if (ChecksEnabled[CK_MismatchedDeallocatorChecker]) + CheckKind = CK_MismatchedDeallocatorChecker; + else + return; + + if (ExplodedNode *N = C.generateSink()) { + if (!BT_FreeAlloca[*CheckKind]) + BT_FreeAlloca[*CheckKind].reset( + new BugType(CheckNames[*CheckKind], "Free alloca()", "Memory Error")); + + BugReport *R = new BugReport(*BT_FreeAlloca[*CheckKind], + "Memory allocated by alloca() should not be deallocated", N); + R->markInteresting(ArgVal.getAsRegion()); + R->addRange(Range); + C.emitReport(R); + } +} + void MallocChecker::ReportMismatchedDealloc(CheckerContext &C, SourceRange Range, const Expr *DeallocExpr, @@ -1517,8 +1710,8 @@ void MallocChecker::ReportOffsetFree(CheckerContext &C, SVal ArgVal, SourceRange Range, const Expr *DeallocExpr, const Expr *AllocExpr) const { - if (!ChecksEnabled[CK_MallocOptimistic] && - !ChecksEnabled[CK_MallocPessimistic] && + + if (!ChecksEnabled[CK_MallocChecker] && !ChecksEnabled[CK_NewDeleteChecker]) return; @@ -1573,8 +1766,7 @@ void MallocChecker::ReportOffsetFree(CheckerContext &C, SVal ArgVal, void MallocChecker::ReportUseAfterFree(CheckerContext &C, SourceRange Range, SymbolRef Sym) const { - if (!ChecksEnabled[CK_MallocOptimistic] && - !ChecksEnabled[CK_MallocPessimistic] && + if (!ChecksEnabled[CK_MallocChecker] && !ChecksEnabled[CK_NewDeleteChecker]) return; @@ -1601,8 +1793,7 @@ void MallocChecker::ReportDoubleFree(CheckerContext &C, SourceRange Range, bool Released, SymbolRef Sym, SymbolRef PrevSym) const { - if (!ChecksEnabled[CK_MallocOptimistic] && - !ChecksEnabled[CK_MallocPessimistic] && + if (!ChecksEnabled[CK_MallocChecker] && !ChecksEnabled[CK_NewDeleteChecker]) return; @@ -1637,7 +1828,6 @@ void MallocChecker::ReportDoubleDelete(CheckerContext &C, SymbolRef Sym) const { Optional<MallocChecker::CheckKind> CheckKind = getCheckIfTracked(C, Sym); if (!CheckKind.hasValue()) return; - assert(*CheckKind == CK_NewDeleteChecker && "invalid check kind"); if (ExplodedNode *N = C.generateSink()) { if (!BT_DoubleDelete) @@ -1653,16 +1843,49 @@ void MallocChecker::ReportDoubleDelete(CheckerContext &C, SymbolRef Sym) const { } } +void MallocChecker::ReportUseZeroAllocated(CheckerContext &C, + SourceRange Range, + SymbolRef Sym) const { + + if (!ChecksEnabled[CK_MallocChecker] && + !ChecksEnabled[CK_NewDeleteChecker]) + return; + + Optional<MallocChecker::CheckKind> CheckKind = getCheckIfTracked(C, Sym); + + if (!CheckKind.hasValue()) + return; + + if (ExplodedNode *N = C.generateSink()) { + if (!BT_UseZerroAllocated[*CheckKind]) + BT_UseZerroAllocated[*CheckKind].reset(new BugType( + CheckNames[*CheckKind], "Use of zero allocated", "Memory Error")); + + BugReport *R = new BugReport(*BT_UseZerroAllocated[*CheckKind], + "Use of zero-allocated memory", N); + + R->addRange(Range); + if (Sym) { + R->markInteresting(Sym); + R->addVisitor(llvm::make_unique<MallocBugVisitor>(Sym)); + } + C.emitReport(R); + } +} + ProgramStateRef MallocChecker::ReallocMem(CheckerContext &C, const CallExpr *CE, - bool FreesOnFail) const { + bool FreesOnFail, + ProgramStateRef State) const { + if (!State) + return nullptr; + if (CE->getNumArgs() < 2) return nullptr; - ProgramStateRef state = C.getState(); const Expr *arg0Expr = CE->getArg(0); const LocationContext *LCtx = C.getLocationContext(); - SVal Arg0Val = state->getSVal(arg0Expr, LCtx); + SVal Arg0Val = State->getSVal(arg0Expr, LCtx); if (!Arg0Val.getAs<DefinedOrUnknownSVal>()) return nullptr; DefinedOrUnknownSVal arg0Val = Arg0Val.castAs<DefinedOrUnknownSVal>(); @@ -1670,7 +1893,7 @@ ProgramStateRef MallocChecker::ReallocMem(CheckerContext &C, SValBuilder &svalBuilder = C.getSValBuilder(); DefinedOrUnknownSVal PtrEQ = - svalBuilder.evalEQ(state, arg0Val, svalBuilder.makeNull()); + svalBuilder.evalEQ(State, arg0Val, svalBuilder.makeNull()); // Get the size argument. If there is no size arg then give up. const Expr *Arg1 = CE->getArg(1); @@ -1678,20 +1901,20 @@ ProgramStateRef MallocChecker::ReallocMem(CheckerContext &C, return nullptr; // Get the value of the size argument. - SVal Arg1ValG = state->getSVal(Arg1, LCtx); + SVal Arg1ValG = State->getSVal(Arg1, LCtx); if (!Arg1ValG.getAs<DefinedOrUnknownSVal>()) return nullptr; DefinedOrUnknownSVal Arg1Val = Arg1ValG.castAs<DefinedOrUnknownSVal>(); // Compare the size argument to 0. DefinedOrUnknownSVal SizeZero = - svalBuilder.evalEQ(state, Arg1Val, + svalBuilder.evalEQ(State, Arg1Val, svalBuilder.makeIntValWithPtrWidth(0, false)); ProgramStateRef StatePtrIsNull, StatePtrNotNull; - std::tie(StatePtrIsNull, StatePtrNotNull) = state->assume(PtrEQ); + std::tie(StatePtrIsNull, StatePtrNotNull) = State->assume(PtrEQ); ProgramStateRef StateSizeIsZero, StateSizeNotZero; - std::tie(StateSizeIsZero, StateSizeNotZero) = state->assume(SizeZero); + std::tie(StateSizeIsZero, StateSizeNotZero) = State->assume(SizeZero); // We only assume exceptional states if they are definitely true; if the // state is under-constrained, assume regular realloc behavior. bool PrtIsNull = StatePtrIsNull && !StatePtrNotNull; @@ -1711,7 +1934,7 @@ ProgramStateRef MallocChecker::ReallocMem(CheckerContext &C, // Get the from and to pointer symbols as in toPtr = realloc(fromPtr, size). assert(!PrtIsNull); SymbolRef FromPtr = arg0Val.getAsSymbol(); - SVal RetVal = state->getSVal(CE, LCtx); + SVal RetVal = State->getSVal(CE, LCtx); SymbolRef ToPtr = RetVal.getAsSymbol(); if (!FromPtr || !ToPtr) return nullptr; @@ -1731,7 +1954,7 @@ ProgramStateRef MallocChecker::ReallocMem(CheckerContext &C, // Default behavior. if (ProgramStateRef stateFree = - FreeMemAux(C, CE, state, 0, false, ReleasedAllocated)) { + FreeMemAux(C, CE, State, 0, false, ReleasedAllocated)) { ProgramStateRef stateRealloc = MallocMemAux(C, CE, CE->getArg(1), UnknownVal(), stateFree); @@ -1755,20 +1978,23 @@ ProgramStateRef MallocChecker::ReallocMem(CheckerContext &C, return nullptr; } -ProgramStateRef MallocChecker::CallocMem(CheckerContext &C, const CallExpr *CE){ +ProgramStateRef MallocChecker::CallocMem(CheckerContext &C, const CallExpr *CE, + ProgramStateRef State) { + if (!State) + return nullptr; + if (CE->getNumArgs() < 2) return nullptr; - ProgramStateRef state = C.getState(); SValBuilder &svalBuilder = C.getSValBuilder(); const LocationContext *LCtx = C.getLocationContext(); - SVal count = state->getSVal(CE->getArg(0), LCtx); - SVal elementSize = state->getSVal(CE->getArg(1), LCtx); - SVal TotalSize = svalBuilder.evalBinOp(state, BO_Mul, count, elementSize, + SVal count = State->getSVal(CE->getArg(0), LCtx); + SVal elementSize = State->getSVal(CE->getArg(1), LCtx); + SVal TotalSize = svalBuilder.evalBinOp(State, BO_Mul, count, elementSize, svalBuilder.getContext().getSizeType()); SVal zeroVal = svalBuilder.makeZeroVal(svalBuilder.getContext().CharTy); - return MallocMemAux(C, CE, TotalSize, zeroVal, state); + return MallocMemAux(C, CE, TotalSize, zeroVal, State); } LeakInfo @@ -1801,9 +2027,11 @@ MallocChecker::getAllocationSite(const ExplodedNode *N, SymbolRef Sym, } } - // Allocation node, is the last node in the current context in which the - // symbol was tracked. - if (N->getLocationContext() == LeakContext) + // Allocation node, is the last node in the current or parent context in + // which the symbol was tracked. + const LocationContext *NContext = N->getLocationContext(); + if (NContext == LeakContext || + NContext->isParentOf(LeakContext)) AllocNode = N; N = N->pred_empty() ? nullptr : *(N->pred_begin()); } @@ -1814,23 +2042,22 @@ MallocChecker::getAllocationSite(const ExplodedNode *N, SymbolRef Sym, void MallocChecker::reportLeak(SymbolRef Sym, ExplodedNode *N, CheckerContext &C) const { - if (!ChecksEnabled[CK_MallocOptimistic] && - !ChecksEnabled[CK_MallocPessimistic] && + if (!ChecksEnabled[CK_MallocChecker] && !ChecksEnabled[CK_NewDeleteLeaksChecker]) return; const RefState *RS = C.getState()->get<RegionState>(Sym); assert(RS && "cannot leak an untracked symbol"); AllocationFamily Family = RS->getAllocationFamily(); - Optional<MallocChecker::CheckKind> CheckKind = getCheckIfTracked(Family); - if (!CheckKind.hasValue()) + + if (Family == AF_Alloca) 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 (!ChecksEnabled[CK_NewDeleteLeaksChecker]) - return; + Optional<MallocChecker::CheckKind> + CheckKind = getCheckIfTracked(Family, true); + + if (!CheckKind.hasValue()) + return; assert(N); if (!BT_Leak[*CheckKind]) { @@ -1893,7 +2120,7 @@ void MallocChecker::checkDeadSymbols(SymbolReaper &SymReaper, SmallVector<SymbolRef, 2> Errors; for (RegionStateTy::iterator I = RS.begin(), E = RS.end(); I != E; ++I) { if (SymReaper.isDead(I->first)) { - if (I->second.isAllocated()) + if (I->second.isAllocated() || I->second.isAllocatedOfSizeZero()) Errors.push_back(I->first); // Remove the dead symbol from the map. RS = F.remove(RS, I->first); @@ -1949,8 +2176,7 @@ void MallocChecker::checkPreCall(const CallEvent &Call, return; ASTContext &Ctx = C.getASTContext(); - if ((ChecksEnabled[CK_MallocOptimistic] || - ChecksEnabled[CK_MallocPessimistic]) && + if (ChecksEnabled[CK_MallocChecker] && (isCMemFunction(FD, Ctx, AF_Malloc, MemoryOperationKind::MOK_Free) || isCMemFunction(FD, Ctx, AF_IfNameIndex, MemoryOperationKind::MOK_Free))) @@ -2062,6 +2288,15 @@ bool MallocChecker::checkUseAfterFree(SymbolRef Sym, CheckerContext &C, return false; } +void MallocChecker::checkUseZeroAllocated(SymbolRef Sym, CheckerContext &C, + const Stmt *S) const { + assert(Sym); + const RefState *RS = C.getState()->get<RegionState>(Sym); + + if (RS && RS->isAllocatedOfSizeZero()) + ReportUseZeroAllocated(C, RS->getStmt()->getSourceRange(), Sym); +} + bool MallocChecker::checkDoubleDelete(SymbolRef Sym, CheckerContext &C) const { if (isReleased(Sym, C)) { @@ -2075,8 +2310,10 @@ bool MallocChecker::checkDoubleDelete(SymbolRef Sym, CheckerContext &C) const { void MallocChecker::checkLocation(SVal l, bool isLoad, const Stmt *S, CheckerContext &C) const { SymbolRef Sym = l.getLocSymbolInBase(); - if (Sym) + if (Sym) { checkUseAfterFree(Sym, C, S); + checkUseZeroAllocated(Sym, C, S); + } } // If a symbolic region is assumed to NULL (or another constant), stop tracking @@ -2320,7 +2557,8 @@ ProgramStateRef MallocChecker::checkPointerEscapeAux(ProgramStateRef State, continue; if (const RefState *RS = State->get<RegionState>(sym)) { - if (RS->isAllocated() && CheckRefState(RS)) { + if ((RS->isAllocated() || RS->isAllocatedOfSizeZero()) && + CheckRefState(RS)) { State = State->remove<RegionState>(sym); State = State->set<RegionState>(sym, RefState::getEscaped(RS)); } @@ -2443,6 +2681,8 @@ void MallocChecker::printState(raw_ostream &Out, ProgramStateRef State, const RefState *RefS = State->get<RegionState>(I.getKey()); AllocationFamily Family = RefS->getAllocationFamily(); Optional<MallocChecker::CheckKind> CheckKind = getCheckIfTracked(Family); + if (!CheckKind.hasValue()) + CheckKind = getCheckIfTracked(Family, true); I.getKey()->dumpToStream(Out); Out << " : "; @@ -2457,6 +2697,8 @@ void MallocChecker::printState(raw_ostream &Out, ProgramStateRef State, void ento::registerNewDeleteLeaksChecker(CheckerManager &mgr) { registerCStringCheckerBasic(mgr); MallocChecker *checker = mgr.registerChecker<MallocChecker>(); + checker->IsOptimistic = mgr.getAnalyzerOptions().getBooleanOption( + "Optimistic", false, checker); checker->ChecksEnabled[MallocChecker::CK_NewDeleteLeaksChecker] = true; checker->CheckNames[MallocChecker::CK_NewDeleteLeaksChecker] = mgr.getCurrentCheckName(); @@ -2470,11 +2712,12 @@ void ento::registerNewDeleteLeaksChecker(CheckerManager &mgr) { void ento::register##name(CheckerManager &mgr) { \ registerCStringCheckerBasic(mgr); \ MallocChecker *checker = mgr.registerChecker<MallocChecker>(); \ + checker->IsOptimistic = mgr.getAnalyzerOptions().getBooleanOption( \ + "Optimistic", false, checker); \ checker->ChecksEnabled[MallocChecker::CK_##name] = true; \ checker->CheckNames[MallocChecker::CK_##name] = mgr.getCurrentCheckName(); \ } -REGISTER_CHECKER(MallocPessimistic) -REGISTER_CHECKER(MallocOptimistic) +REGISTER_CHECKER(MallocChecker) REGISTER_CHECKER(NewDeleteChecker) REGISTER_CHECKER(MismatchedDeallocatorChecker) diff --git a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp index f38ce77..e913479 100644 --- a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp +++ b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp @@ -142,13 +142,13 @@ private: } } } - else if (isa<MemberExpr>(E)) { + else if (const auto *ME = dyn_cast<MemberExpr>(E)) { // No points-to analysis, just look at the member - const Decl * EmeMD = dyn_cast<MemberExpr>(E)->getMemberDecl(); + const Decl *EmeMD = ME->getMemberDecl(); while (i != e) { --i; - if (isa<MemberExpr>(i->variable)) { - if (dyn_cast<MemberExpr>(i->variable)->getMemberDecl() == EmeMD) + if (const auto *ME_i = dyn_cast<MemberExpr>(i->variable)) { + if (ME_i->getMemberDecl() == EmeMD) i = toScanFor.erase (i); } } diff --git a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/ObjCContainersChecker.cpp b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/ObjCContainersChecker.cpp index 8e51154..4f0b7e5 100644 --- a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/ObjCContainersChecker.cpp +++ b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/ObjCContainersChecker.cpp @@ -29,7 +29,8 @@ using namespace ento; namespace { class ObjCContainersChecker : public Checker< check::PreStmt<CallExpr>, - check::PostStmt<CallExpr> > { + check::PostStmt<CallExpr>, + check::PointerEscape> { mutable std::unique_ptr<BugType> BT; inline void initBugType() const { if (!BT) @@ -52,6 +53,10 @@ public: void checkPostStmt(const CallExpr *CE, CheckerContext &C) const; void checkPreStmt(const CallExpr *CE, CheckerContext &C) const; + ProgramStateRef checkPointerEscape(ProgramStateRef State, + const InvalidatedSymbols &Escaped, + const CallEvent *Call, + PointerEscapeKind Kind) const; }; } // end anonymous namespace @@ -110,7 +115,8 @@ void ObjCContainersChecker::checkPreStmt(const CallExpr *CE, if (Name.equals("CFArrayGetValueAtIndex")) { ProgramStateRef State = C.getState(); // Retrieve the size. - // Find out if we saw this array symbol before and have information about it. + // Find out if we saw this array symbol before and have information about + // it. const Expr *ArrayExpr = CE->getArg(0); SymbolRef ArraySym = getArraySym(ArrayExpr, C); if (!ArraySym) @@ -145,6 +151,24 @@ void ObjCContainersChecker::checkPreStmt(const CallExpr *CE, } } +ProgramStateRef +ObjCContainersChecker::checkPointerEscape(ProgramStateRef State, + const InvalidatedSymbols &Escaped, + const CallEvent *Call, + PointerEscapeKind Kind) const { + for (InvalidatedSymbols::const_iterator I = Escaped.begin(), + E = Escaped.end(); + I != E; ++I) { + SymbolRef Sym = *I; + // When a symbol for a mutable array escapes, we can't reason precisely + // about its size any more -- so remove it from the map. + // Note that we aren't notified here when a CFMutableArrayRef escapes as a + // CFArrayRef. This is because CFArrayRef is typedef'd as a pointer to a + // const-qualified type. + State = State->remove<ArraySizeMap>(Sym); + } + return State; +} /// Register checker. void ento::registerObjCContainersChecker(CheckerManager &mgr) { mgr.registerChecker<ObjCContainersChecker>(); diff --git a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp index 9a460ba..49eef23 100644 --- a/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp +++ b/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp @@ -94,6 +94,16 @@ public: ErrorReturnedNotOwned }; + /// Tracks how an object referenced by an ivar has been used. + /// + /// This accounts for us not knowing if an arbitrary ivar is supposed to be + /// stored at +0 or +1. + enum class IvarAccessHistory { + None, + AccessedDirectly, + ReleasedAfterDirectAccess + }; + private: /// The number of outstanding retains. unsigned Cnt; @@ -121,14 +131,16 @@ private: /// This setting should not be propagated to state derived from this state. /// Once we start deriving new states, it would be inconsistent to override /// them. - unsigned IsOverridable : 1; + unsigned RawIvarAccessHistory : 2; RefVal(Kind k, RetEffect::ObjKind o, unsigned cnt, unsigned acnt, QualType t, - bool Overridable = false) + IvarAccessHistory IvarAccess) : Cnt(cnt), ACnt(acnt), T(t), RawKind(static_cast<unsigned>(k)), - RawObjectKind(static_cast<unsigned>(o)), IsOverridable(Overridable) { + RawObjectKind(static_cast<unsigned>(o)), + RawIvarAccessHistory(static_cast<unsigned>(IvarAccess)) { assert(getKind() == k && "not enough bits for the kind"); assert(getObjKind() == o && "not enough bits for the object kind"); + assert(getIvarAccessHistory() == IvarAccess && "not enough bits"); } public: @@ -144,20 +156,24 @@ public: void clearCounts() { Cnt = 0; ACnt = 0; - IsOverridable = false; } void setCount(unsigned i) { Cnt = i; - IsOverridable = false; } void setAutoreleaseCount(unsigned i) { ACnt = i; - IsOverridable = false; } QualType getType() const { return T; } - bool isOverridable() const { return IsOverridable; } + /// Returns what the analyzer knows about direct accesses to a particular + /// instance variable. + /// + /// If the object with this refcount wasn't originally from an Objective-C + /// ivar region, this should always return IvarAccessHistory::None. + IvarAccessHistory getIvarAccessHistory() const { + return static_cast<IvarAccessHistory>(RawIvarAccessHistory); + } bool isOwned() const { return getKind() == Owned; @@ -181,7 +197,7 @@ public: /// Most commonly, this is an owned object with a retain count of +1. static RefVal makeOwned(RetEffect::ObjKind o, QualType t, unsigned Count = 1) { - return RefVal(Owned, o, Count, 0, t); + return RefVal(Owned, o, Count, 0, t, IvarAccessHistory::None); } /// Create a state for an object whose lifetime is not the responsibility of @@ -190,47 +206,49 @@ public: /// Most commonly, this is an unowned object with a retain count of +0. static RefVal makeNotOwned(RetEffect::ObjKind o, QualType t, unsigned Count = 0) { - return RefVal(NotOwned, o, Count, 0, t); - } - - /// Create an "overridable" state for an unowned object at +0. - /// - /// An overridable state is one that provides a good approximation of the - /// reference counting state now, but which may be discarded later if the - /// checker sees the object being used in new ways. - static RefVal makeOverridableNotOwned(RetEffect::ObjKind o, QualType t) { - return RefVal(NotOwned, o, 0, 0, t, /*Overridable=*/true); + return RefVal(NotOwned, o, Count, 0, t, IvarAccessHistory::None); } RefVal operator-(size_t i) const { return RefVal(getKind(), getObjKind(), getCount() - i, - getAutoreleaseCount(), getType()); + getAutoreleaseCount(), getType(), getIvarAccessHistory()); } RefVal operator+(size_t i) const { return RefVal(getKind(), getObjKind(), getCount() + i, - getAutoreleaseCount(), getType()); + getAutoreleaseCount(), getType(), getIvarAccessHistory()); } RefVal operator^(Kind k) const { return RefVal(k, getObjKind(), getCount(), getAutoreleaseCount(), - getType()); + getType(), getIvarAccessHistory()); } RefVal autorelease() const { return RefVal(getKind(), getObjKind(), getCount(), getAutoreleaseCount()+1, - getType()); + getType(), getIvarAccessHistory()); + } + + RefVal withIvarAccess() const { + assert(getIvarAccessHistory() == IvarAccessHistory::None); + return RefVal(getKind(), getObjKind(), getCount(), getAutoreleaseCount(), + getType(), IvarAccessHistory::AccessedDirectly); + } + RefVal releaseViaIvar() const { + assert(getIvarAccessHistory() == IvarAccessHistory::AccessedDirectly); + return RefVal(getKind(), getObjKind(), getCount(), getAutoreleaseCount(), + getType(), IvarAccessHistory::ReleasedAfterDirectAccess); } // Comparison, profiling, and pretty-printing. bool hasSameState(const RefVal &X) const { - return getKind() == X.getKind() && Cnt == X.Cnt && ACnt == X.ACnt; + return getKind() == X.getKind() && Cnt == X.Cnt && ACnt == X.ACnt && + getIvarAccessHistory() == X.getIvarAccessHistory(); } bool operator==(const RefVal& X) const { - return T == X.T && hasSameState(X) && getObjKind() == X.getObjKind() && - IsOverridable == X.IsOverridable; + return T == X.T && hasSameState(X) && getObjKind() == X.getObjKind(); } void Profile(llvm::FoldingSetNodeID& ID) const { @@ -239,7 +257,7 @@ public: ID.AddInteger(Cnt); ID.AddInteger(ACnt); ID.AddInteger(RawObjectKind); - ID.AddBoolean(IsOverridable); + ID.AddInteger(RawIvarAccessHistory); } void print(raw_ostream &Out) const; @@ -249,9 +267,6 @@ void RefVal::print(raw_ostream &Out) const { if (!T.isNull()) Out << "Tracked " << T.getAsString() << '/'; - if (isOverridable()) - Out << "(overridable) "; - switch (getKind()) { default: llvm_unreachable("Invalid RefVal kind"); case Owned: { @@ -323,8 +338,18 @@ void RefVal::print(raw_ostream &Out) const { break; } + switch (getIvarAccessHistory()) { + case IvarAccessHistory::None: + break; + case IvarAccessHistory::AccessedDirectly: + Out << " [direct ivar access]"; + break; + case IvarAccessHistory::ReleasedAfterDirectAccess: + Out << " [released after direct ivar access]"; + } + if (ACnt) { - Out << " [ARC +" << ACnt << ']'; + Out << " [autorelease -" << ACnt << ']'; } } } //end anonymous namespace @@ -881,6 +906,8 @@ static ArgEffect getStopTrackingHardEquivalent(ArgEffect E) { case IncRef: case IncRefMsg: case MakeCollectable: + case UnretainedOutParameter: + case RetainedOutParameter: case MayEscape: case StopTracking: case StopTrackingHard: @@ -1310,7 +1337,18 @@ RetainSummaryManager::updateSummaryFromAnnotations(const RetainSummary *&Summ, if (pd->hasAttr<NSConsumedAttr>()) Template->addArg(AF, parm_idx, DecRefMsg); else if (pd->hasAttr<CFConsumedAttr>()) - Template->addArg(AF, parm_idx, DecRef); + Template->addArg(AF, parm_idx, DecRef); + else if (pd->hasAttr<CFReturnsRetainedAttr>()) { + QualType PointeeTy = pd->getType()->getPointeeType(); + if (!PointeeTy.isNull()) + if (coreFoundation::isCFObjectRef(PointeeTy)) + Template->addArg(AF, parm_idx, RetainedOutParameter); + } else if (pd->hasAttr<CFReturnsNotRetainedAttr>()) { + QualType PointeeTy = pd->getType()->getPointeeType(); + if (!PointeeTy.isNull()) + if (coreFoundation::isCFObjectRef(PointeeTy)) + Template->addArg(AF, parm_idx, UnretainedOutParameter); + } } QualType RetTy = FD->getReturnType(); @@ -1341,7 +1379,17 @@ RetainSummaryManager::updateSummaryFromAnnotations(const RetainSummary *&Summ, Template->addArg(AF, parm_idx, DecRefMsg); else if (pd->hasAttr<CFConsumedAttr>()) { Template->addArg(AF, parm_idx, DecRef); - } + } else if (pd->hasAttr<CFReturnsRetainedAttr>()) { + QualType PointeeTy = pd->getType()->getPointeeType(); + if (!PointeeTy.isNull()) + if (coreFoundation::isCFObjectRef(PointeeTy)) + Template->addArg(AF, parm_idx, RetainedOutParameter); + } else if (pd->hasAttr<CFReturnsNotRetainedAttr>()) { + QualType PointeeTy = pd->getType()->getPointeeType(); + if (!PointeeTy.isNull()) + if (coreFoundation::isCFObjectRef(PointeeTy)) + Template->addArg(AF, parm_idx, UnretainedOutParameter); + } } QualType RetTy = MD->getReturnType(); @@ -1763,12 +1811,11 @@ namespace { addGCModeDescription(LOpts, GCEnabled); } - std::pair<ranges_iterator, ranges_iterator> getRanges() override { + llvm::iterator_range<ranges_iterator> getRanges() override { const CFRefBug& BugTy = static_cast<CFRefBug&>(getBugType()); if (!BugTy.isLeak()) return BugReport::getRanges(); - else - return std::make_pair(ranges_iterator(), ranges_iterator()); + return llvm::make_range(ranges_iterator(), ranges_iterator()); } }; @@ -1829,6 +1876,16 @@ static bool isNumericLiteralExpression(const Expr *E) { isa<CXXBoolLiteralExpr>(E); } +/// Returns true if this stack frame is for an Objective-C method that is a +/// property getter or setter whose body has been synthesized by the analyzer. +static bool isSynthesizedAccessor(const StackFrameContext *SFC) { + auto Method = dyn_cast_or_null<ObjCMethodDecl>(SFC->getDecl()); + if (!Method || !Method->isPropertyAccessor()) + return false; + + return SFC->getAnalysisDeclContext()->isBodyAutosynthesized(); +} + PathDiagnosticPiece *CFRefReportVisitor::VisitNode(const ExplodedNode *N, const ExplodedNode *PrevN, BugReporterContext &BRC, @@ -1859,6 +1916,11 @@ PathDiagnosticPiece *CFRefReportVisitor::VisitNode(const ExplodedNode *N, if (!PrevT) { const Stmt *S = N->getLocation().castAs<StmtPoint>().getStmt(); + if (isa<ObjCIvarRefExpr>(S) && + isSynthesizedAccessor(LCtx->getCurrentStackFrame())) { + S = LCtx->getCurrentStackFrame()->getCallSite(); + } + if (isa<ObjCArrayLiteral>(S)) { os << "NSArray literal is an object with a +0 retain count"; } @@ -1883,6 +1945,9 @@ PathDiagnosticPiece *CFRefReportVisitor::VisitNode(const ExplodedNode *N, os << "oxed expression produces an object with a +0 retain count"; } } + else if (isa<ObjCIvarRefExpr>(S)) { + os << "Object loaded from instance variable"; + } else { if (const CallExpr *CE = dyn_cast<CallExpr>(S)) { // Get the name of the callee (if it is available). @@ -2034,7 +2099,6 @@ PathDiagnosticPiece *CFRefReportVisitor::VisitNode(const ExplodedNode *N, switch (CurrV.getKind()) { case RefVal::Owned: case RefVal::NotOwned: - if (PrevV.getCount() == CurrV.getCount()) { // Did an autorelease message get sent? if (PrevV.getAutoreleaseCount() == CurrV.getAutoreleaseCount()) @@ -2062,6 +2126,11 @@ PathDiagnosticPiece *CFRefReportVisitor::VisitNode(const ExplodedNode *N, break; case RefVal::Released: + if (CurrV.getIvarAccessHistory() == + RefVal::IvarAccessHistory::ReleasedAfterDirectAccess && + CurrV.getIvarAccessHistory() != PrevV.getIvarAccessHistory()) { + os << "Strong instance variable relinquished. "; + } os << "Object released."; break; @@ -2143,7 +2212,7 @@ static AllocationInfo GetAllocationSite(ProgramStateManager& StateMgr, const ExplodedNode *N, SymbolRef Sym) { const ExplodedNode *AllocationNode = N; - const ExplodedNode *AllocationNodeInCurrentContext = N; + const ExplodedNode *AllocationNodeInCurrentOrParentContext = N; const MemRegion *FirstBinding = nullptr; const LocationContext *LeakContext = N->getLocationContext(); @@ -2173,10 +2242,15 @@ GetAllocationSite(ProgramStateManager& StateMgr, const ExplodedNode *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; + // AllocationNodeInCurrentContext, is the last node in the current or + // parent context in which the symbol was tracked. + // + // Note that the allocation site might be in the parent conext. For example, + // the case where an allocation happens in a block that captures a reference + // to it and that reference is overwritten/dropped by another call to + // the block. + if (NContext == LeakContext || NContext->isParentOf(LeakContext)) + AllocationNodeInCurrentOrParentContext = N; // Find the last init that was called on the given symbol and store the // init method's location context. @@ -2214,7 +2288,7 @@ GetAllocationSite(ProgramStateManager& StateMgr, const ExplodedNode *N, FirstBinding = nullptr; } - return AllocationInfo(AllocationNodeInCurrentContext, + return AllocationInfo(AllocationNodeInCurrentOrParentContext, FirstBinding, InterestingMethodContext); } @@ -2345,20 +2419,8 @@ CFRefLeakReport::CFRefLeakReport(CFRefBug &D, const LangOptions &LOpts, ProgramPoint P = AllocNode->getLocation(); if (Optional<CallExitEnd> Exit = P.getAs<CallExitEnd>()) AllocStmt = Exit->getCalleeContext()->getCallSite(); - else { - // We are going to get a BlockEdge when the leak and allocation happen in - // different, non-nested frames (contexts). For example, the case where an - // allocation happens in a block that captures a reference to it and - // that reference is overwritten/dropped by another call to the block. - if (Optional<BlockEdge> Edge = P.getAs<BlockEdge>()) { - if (Optional<CFGStmt> St = Edge->getDst()->front().getAs<CFGStmt>()) { - AllocStmt = St->getStmt(); - } - } - else { - AllocStmt = P.castAs<PostStmt>().getStmt(); - } - } + else + AllocStmt = P.castAs<PostStmt>().getStmt(); assert(AllocStmt && "Cannot find allocation statement"); PathDiagnosticLocation AllocLocation = @@ -2436,9 +2498,7 @@ public: : ShouldResetSummaryLog(false), IncludeAllocationLine(shouldIncludeAllocationSiteInLeakDiagnostics(AO)) {} - virtual ~RetainCountChecker() { - DeleteContainerSeconds(DeadSymbolTags); - } + ~RetainCountChecker() override { DeleteContainerSeconds(DeadSymbolTags); } void checkEndAnalysis(ExplodedGraph &G, BugReporter &BR, ExprEngine &Eng) const { @@ -2709,7 +2769,6 @@ void RetainCountChecker::checkPostStmt(const CastExpr *CE, if (hasErr) { // FIXME: If we get an error during a bridge cast, should we report it? - // Should we assert that there is no error? return; } @@ -2774,17 +2833,64 @@ void RetainCountChecker::checkPostStmt(const ObjCBoxedExpr *Ex, C.addTransition(State); } +static bool wasLoadedFromIvar(SymbolRef Sym) { + if (auto DerivedVal = dyn_cast<SymbolDerived>(Sym)) + return isa<ObjCIvarRegion>(DerivedVal->getRegion()); + if (auto RegionVal = dyn_cast<SymbolRegionValue>(Sym)) + return isa<ObjCIvarRegion>(RegionVal->getRegion()); + return false; +} + void RetainCountChecker::checkPostStmt(const ObjCIvarRefExpr *IRE, CheckerContext &C) const { + Optional<Loc> IVarLoc = C.getSVal(IRE).getAs<Loc>(); + if (!IVarLoc) + return; + ProgramStateRef State = C.getState(); - // If an instance variable was previously accessed through a property, - // it may have a synthesized refcount of +0. Override right now that we're - // doing direct access. - if (Optional<Loc> IVarLoc = C.getSVal(IRE).getAs<Loc>()) - if (SymbolRef Sym = State->getSVal(*IVarLoc).getAsSymbol()) - if (const RefVal *RV = getRefBinding(State, Sym)) - if (RV->isOverridable()) - State = removeRefBinding(State, Sym); + SymbolRef Sym = State->getSVal(*IVarLoc).getAsSymbol(); + if (!Sym || !wasLoadedFromIvar(Sym)) + return; + + // Accessing an ivar directly is unusual. If we've done that, be more + // forgiving about what the surrounding code is allowed to do. + + QualType Ty = Sym->getType(); + RetEffect::ObjKind Kind; + if (Ty->isObjCRetainableType()) + Kind = RetEffect::ObjC; + else if (coreFoundation::isCFObjectRef(Ty)) + Kind = RetEffect::CF; + else + return; + + // If the value is already known to be nil, don't bother tracking it. + ConstraintManager &CMgr = State->getConstraintManager(); + if (CMgr.isNull(State, Sym).isConstrainedTrue()) + return; + + if (const RefVal *RV = getRefBinding(State, Sym)) { + // If we've seen this symbol before, or we're only seeing it now because + // of something the analyzer has synthesized, don't do anything. + if (RV->getIvarAccessHistory() != RefVal::IvarAccessHistory::None || + isSynthesizedAccessor(C.getStackFrame())) { + return; + } + + // Note that this value has been loaded from an ivar. + C.addTransition(setRefBinding(State, Sym, RV->withIvarAccess())); + return; + } + + RefVal PlusZero = RefVal::makeNotOwned(Kind, Ty); + + // In a synthesized accessor, the effective retain count is +0. + if (isSynthesizedAccessor(C.getStackFrame())) { + C.addTransition(setRefBinding(State, Sym, PlusZero)); + return; + } + + State = setRefBinding(State, Sym, PlusZero.withIvarAccess()); C.addTransition(State); } @@ -2828,16 +2934,6 @@ static QualType GetReturnType(const Expr *RetE, ASTContext &Ctx) { return RetTy; } -static bool wasSynthesizedProperty(const ObjCMethodCall *Call, - ExplodedNode *N) { - if (!Call || !Call->getDecl()->isPropertyAccessor()) - return false; - - CallExitEnd PP = N->getLocation().castAs<CallExitEnd>(); - const StackFrameContext *Frame = PP.getCalleeContext(); - return Frame->getAnalysisDeclContext()->isBodyAutosynthesized(); -} - // We don't always get the exact modeling of the function with regards to the // retain count checker even when the function is inlined. For example, we need // to stop tracking the symbols which were marked with StopTrackingHard. @@ -2872,24 +2968,45 @@ void RetainCountChecker::processSummaryOfInlined(const RetainSummary &Summ, SymbolRef Sym = CallOrMsg.getReturnValue().getAsSymbol(); if (Sym) state = removeRefBinding(state, Sym); - } else if (RE.getKind() == RetEffect::NotOwnedSymbol) { - if (wasSynthesizedProperty(MsgInvocation, C.getPredecessor())) { - // Believe the summary if we synthesized the body of a property getter - // and the return value is currently untracked. If the corresponding - // instance variable is later accessed directly, however, we're going to - // want to override this state, so that the owning object can perform - // reference counting operations on its own ivars. - SymbolRef Sym = CallOrMsg.getReturnValue().getAsSymbol(); - if (Sym && !getRefBinding(state, Sym)) - state = setRefBinding(state, Sym, - RefVal::makeOverridableNotOwned(RE.getObjKind(), - Sym->getType())); - } } C.addTransition(state); } +static ProgramStateRef updateOutParameter(ProgramStateRef State, + SVal ArgVal, + ArgEffect Effect) { + auto *ArgRegion = dyn_cast_or_null<TypedValueRegion>(ArgVal.getAsRegion()); + if (!ArgRegion) + return State; + + QualType PointeeTy = ArgRegion->getValueType(); + if (!coreFoundation::isCFObjectRef(PointeeTy)) + return State; + + SVal PointeeVal = State->getSVal(ArgRegion); + SymbolRef Pointee = PointeeVal.getAsLocSymbol(); + if (!Pointee) + return State; + + switch (Effect) { + case UnretainedOutParameter: + State = setRefBinding(State, Pointee, + RefVal::makeNotOwned(RetEffect::CF, PointeeTy)); + break; + case RetainedOutParameter: + // Do nothing. Retained out parameters will either point to a +1 reference + // or NULL, but the way you check for failure differs depending on the API. + // Consequently, we don't have a good way to track them yet. + break; + + default: + llvm_unreachable("only for out parameters"); + } + + return State; +} + void RetainCountChecker::checkSummary(const RetainSummary &Summ, const CallEvent &CallOrMsg, CheckerContext &C) const { @@ -2903,9 +3020,12 @@ void RetainCountChecker::checkSummary(const RetainSummary &Summ, for (unsigned idx = 0, e = CallOrMsg.getNumArgs(); idx != e; ++idx) { SVal V = CallOrMsg.getArgSVal(idx); - if (SymbolRef Sym = V.getAsLocSymbol()) { + ArgEffect Effect = Summ.getArg(idx); + if (Effect == RetainedOutParameter || Effect == UnretainedOutParameter) { + state = updateOutParameter(state, V, Effect); + } else if (SymbolRef Sym = V.getAsLocSymbol()) { if (const RefVal *T = getRefBinding(state, Sym)) { - state = updateSymbol(state, Sym, *T, Summ.getArg(idx), hasErr, C); + state = updateSymbol(state, Sym, *T, Effect, hasErr, C); if (hasErr) { ErrorRange = CallOrMsg.getArgSourceRange(idx); ErrorSym = Sym; @@ -3054,6 +3174,11 @@ RetainCountChecker::updateSymbol(ProgramStateRef state, SymbolRef sym, case DecRefMsgAndStopTrackingHard: llvm_unreachable("DecRefMsg/IncRefMsg/MakeCollectable already converted"); + case UnretainedOutParameter: + case RetainedOutParameter: + llvm_unreachable("Applies to pointer-to-pointer parameters, which should " + "not have ref state."); + case Dealloc: // Any use of -dealloc in GC is *bad*. if (C.isObjCGCEnabled()) { @@ -3125,11 +3250,16 @@ RetainCountChecker::updateSymbol(ProgramStateRef state, SymbolRef sym, case RefVal::Owned: assert(V.getCount() > 0); - if (V.getCount() == 1) - V = V ^ (E == DecRefBridgedTransferred ? RefVal::NotOwned - : RefVal::Released); - else if (E == DecRefAndStopTrackingHard) + if (V.getCount() == 1) { + if (E == DecRefBridgedTransferred || + V.getIvarAccessHistory() == + RefVal::IvarAccessHistory::AccessedDirectly) + V = V ^ RefVal::NotOwned; + else + V = V ^ RefVal::Released; + } else if (E == DecRefAndStopTrackingHard) { return removeRefBinding(state, sym); + } V = V - 1; break; @@ -3139,6 +3269,13 @@ RetainCountChecker::updateSymbol(ProgramStateRef state, SymbolRef sym, if (E == DecRefAndStopTrackingHard) return removeRefBinding(state, sym); V = V - 1; + } else if (V.getIvarAccessHistory() == + RefVal::IvarAccessHistory::AccessedDirectly) { + // Assume that the instance variable was holding on the object at + // +1, and we just didn't know. + if (E == DecRefAndStopTrackingHard) + return removeRefBinding(state, sym); + V = V.releaseViaIvar() ^ RefVal::Released; } else { V = V ^ RefVal::ErrorReleaseNotOwned; hasErr = V.getKind(); @@ -3162,6 +3299,16 @@ void RetainCountChecker::processNonLeakError(ProgramStateRef St, RefVal::Kind ErrorKind, SymbolRef Sym, CheckerContext &C) const { + // HACK: Ignore retain-count issues on values accessed through ivars, + // because of cases like this: + // [_contentView retain]; + // [_contentView removeFromSuperview]; + // [self addSubview:_contentView]; // invalidates 'self' + // [_contentView release]; + if (const RefVal *RV = getRefBinding(St, Sym)) + if (RV->getIvarAccessHistory() != RefVal::IvarAccessHistory::None) + return; + ExplodedNode *N = C.generateSink(St); if (!N) return; @@ -3229,7 +3376,7 @@ bool RetainCountChecker::evalCall(const CallExpr *CE, CheckerContext &C) const { // See if it's one of the specific functions we know how to eval. bool canEval = false; - QualType ResultTy = CE->getCallReturnType(); + QualType ResultTy = CE->getCallReturnType(C.getASTContext()); if (ResultTy->isObjCIdType()) { // Handle: id NSMakeCollectable(CFTypeRef) canEval = II->isStr("NSMakeCollectable"); @@ -3388,6 +3535,15 @@ void RetainCountChecker::checkReturnWithRetEffect(const ReturnStmt *S, RetEffect RE, RefVal X, SymbolRef Sym, ProgramStateRef state) const { + // HACK: Ignore retain-count issues on values accessed through ivars, + // because of cases like this: + // [_contentView retain]; + // [_contentView removeFromSuperview]; + // [self addSubview:_contentView]; // invalidates 'self' + // [_contentView release]; + if (X.getIvarAccessHistory() != RefVal::IvarAccessHistory::None) + return; + // Any leaks or other errors? if (X.isReturnedOwned() && X.getCount() == 0) { if (RE.getKind() != RetEffect::NoRet) { @@ -3428,22 +3584,31 @@ void RetainCountChecker::checkReturnWithRetEffect(const ReturnStmt *S, } } else if (X.isReturnedNotOwned()) { if (RE.isOwned()) { - // Trying to return a not owned object to a caller expecting an - // owned object. - state = setRefBinding(state, Sym, X ^ RefVal::ErrorReturnedNotOwned); - - static CheckerProgramPointTag ReturnNotOwnedTag(this, - "ReturnNotOwnedForOwned"); - ExplodedNode *N = C.addTransition(state, Pred, &ReturnNotOwnedTag); - if (N) { - if (!returnNotOwnedForOwned) - returnNotOwnedForOwned.reset(new ReturnedNotOwnedForOwned(this)); - - CFRefReport *report = - new CFRefReport(*returnNotOwnedForOwned, - C.getASTContext().getLangOpts(), - C.isObjCGCEnabled(), SummaryLog, N, Sym); - C.emitReport(report); + if (X.getIvarAccessHistory() == + RefVal::IvarAccessHistory::AccessedDirectly) { + // Assume the method was trying to transfer a +1 reference from a + // strong ivar to the caller. + state = setRefBinding(state, Sym, + X.releaseViaIvar() ^ RefVal::ReturnedOwned); + } else { + // Trying to return a not owned object to a caller expecting an + // owned object. + state = setRefBinding(state, Sym, X ^ RefVal::ErrorReturnedNotOwned); + + static CheckerProgramPointTag + ReturnNotOwnedTag(this, "ReturnNotOwnedForOwned"); + + ExplodedNode *N = C.addTransition(state, Pred, &ReturnNotOwnedTag); + if (N) { + if (!returnNotOwnedForOwned) + returnNotOwnedForOwned.reset(new ReturnedNotOwnedForOwned(this)); + + CFRefReport *report = + new CFRefReport(*returnNotOwnedForOwned, + C.getASTContext().getLangOpts(), + C.isObjCGCEnabled(), SummaryLog, N, Sym); + C.emitReport(report); + } } } } @@ -3594,6 +3759,14 @@ RetainCountChecker::handleAutoreleaseCounts(ProgramStateRef state, if (V.getKind() == RefVal::ReturnedOwned) ++Cnt; + // If we would over-release here, but we know the value came from an ivar, + // assume it was a strong ivar that's just been relinquished. + if (ACnt > Cnt && + V.getIvarAccessHistory() == RefVal::IvarAccessHistory::AccessedDirectly) { + V = V.releaseViaIvar(); + --ACnt; + } + if (ACnt <= Cnt) { if (ACnt == Cnt) { V.clearCounts(); @@ -3608,6 +3781,15 @@ RetainCountChecker::handleAutoreleaseCounts(ProgramStateRef state, return setRefBinding(state, Sym, V); } + // HACK: Ignore retain-count issues on values accessed through ivars, + // because of cases like this: + // [_contentView retain]; + // [_contentView removeFromSuperview]; + // [self addSubview:_contentView]; // invalidates 'self' + // [_contentView release]; + if (V.getIvarAccessHistory() != RefVal::IvarAccessHistory::None) + return state; + // Woah! More autorelease counts then retain counts left. // Emit hard error. V = V ^ RefVal::ErrorOverAutorelease; @@ -3641,11 +3823,22 @@ ProgramStateRef RetainCountChecker::handleSymbolDeath(ProgramStateRef state, SymbolRef sid, RefVal V, SmallVectorImpl<SymbolRef> &Leaked) const { - bool hasLeak = false; - if (V.isOwned()) + bool hasLeak; + + // HACK: Ignore retain-count issues on values accessed through ivars, + // because of cases like this: + // [_contentView retain]; + // [_contentView removeFromSuperview]; + // [self addSubview:_contentView]; // invalidates 'self' + // [_contentView release]; + if (V.getIvarAccessHistory() != RefVal::IvarAccessHistory::None) + hasLeak = false; + else if (V.isOwned()) hasLeak = true; else if (V.isNotOwned() || V.isReturnedOwned()) hasLeak = (V.getCount() > 0); + else + hasLeak = false; if (!hasLeak) return removeRefBinding(state, sid); |
