Index: lib/StaticAnalyzer/Checkers/MallocChecker.cpp =================================================================== --- lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -15,6 +15,8 @@ #include "ClangSACheckers.h" #include "InterCheckerAPI.h" #include "clang/AST/Attr.h" +#include "clang/AST/ParentMap.h" +#include "clang/AST/DeclFriend.h" #include "clang/Basic/SourceManager.h" #include "clang/Basic/TargetInfo.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" @@ -143,6 +145,8 @@ typedef std::pair LeakInfo; +typedef llvm::SmallDenseMap RecordDeclToBoolMap; + class MallocChecker : public Checker KernelZeroFlagVal; + typedef llvm::DenseMap> RecToReferencedTypesMapTy; + // Cache used to speed up unusedNewCanEscape. + mutable RecToReferencedTypesMapTy RecToReferencedTypesMap; + + void initIdentifierInfo(ASTContext &C) const; /// \brief Determine family of a deallocation expression. @@ -241,6 +251,10 @@ bool isAllocationFunction(const FunctionDecl *FD, ASTContext &C) const; bool isStandardNewDelete(const FunctionDecl *FD, ASTContext &C) const; ///@} + + /// \brief Check if a given unassigned new can potentially escape. + bool unusedNewCanEscape(const CXXNewExpr *NE) const; + ProgramStateRef MallocMemReturnsAttr(CheckerContext &C, const CallExpr *CE, const OwnershipAttr* Att) const; @@ -753,6 +767,169 @@ C.addTransition(State); } +static QualType getDeepPointeeType(QualType T) { + QualType Result = T, PointeeType = T->getPointeeType(); + while (!PointeeType.isNull()) { + Result = PointeeType; + PointeeType = PointeeType->getPointeeType(); + } + return Result; +} + +// Tells if Ty is a friend of RD. +static bool isFriendOfRD(const CXXRecordDecl *RD, QualType Ty) { + for (const auto *FriendOfRD : RD->friends()) { + if (TypeSourceInfo *FriendType = FriendOfRD->getFriendType()) { + if (FriendType->getType().getCanonicalType() == Ty) { + return true; + } + } + } + + return false; +}; + +// Collect RD and all superclasses with exception of those whose methods are +// guaranteed inaccessible from the ConstructedTy record. +static void +collectRecordsWithAccessibleMethods(RecordDeclToBoolMap &Records, + const CXXRecordDecl *RD, + QualType ConstructedTy) { + if (!Records.count(RD)) + Records[RD] = isFriendOfRD(RD, ConstructedTy); + + for (const auto &Base : RD->bases()) { + if (Base.getAccessSpecifier() != AS_public && !Records[RD]) + continue; + + if (const CXXRecordDecl *BaseRD = Base.getType()->getAsCXXRecordDecl()) + collectRecordsWithAccessibleMethods(Records, BaseRD, ConstructedTy); + } +} + +bool MallocChecker::unusedNewCanEscape(const CXXNewExpr *NE) const { + // There are cases when the result of a call to nonplacement operator new is + // not used, but nevertheless, the memory may not be leaked. The memory can + // escape in the constructor of the constructed object. For example: + // + // void f(B* b) { + // new A(b); + // } + // + // 'A's constructor: + // A(B *b) { + // b->escapeA(this); // memory escapes! + // } + // + // (PR19102 is devoted to the problem and contain a full example). + // To reduce the number of false-positives we assume the unused operator new + // as not a leak if the following requirement is met: + // * The invoked constructor has a parameter - pointer or reference to a + // record that has a public method (any access level , if a constructed + // record is a friend of parameter record), that accepts a pointer to the + // type of the constructed record or one of its bases. + + const CXXConstructExpr* ConstructE = NE->getConstructExpr(); + if (!ConstructE) + return false; + + QualType ConstructedTy = NE->getAllocatedType().getCanonicalType(); + const CXXRecordDecl *ConstructedRD = ConstructedTy->getAsCXXRecordDecl(); + if (!ConstructedRD) + return false; + + CXXConstructorDecl* CtorD = ConstructE->getConstructor(); + + // Iterate over the constructor parameters. + for (const auto *CtorParam : CtorD->params()) { + + QualType CtorParamPointeeT = CtorParam->getType()->getPointeeType(); + if (CtorParamPointeeT.isNull()) + continue; + + CtorParamPointeeT = getDeepPointeeType(CtorParamPointeeT). + getCanonicalType().getUnqualifiedType(); + + const CXXRecordDecl *RD = CtorParamPointeeT->getAsCXXRecordDecl(); + if (!RD) + continue; + // Found a parameter of the pointer-to-a-record type. + + // Collect the record and all its superclasses that have methods accessible + // from a constructed record. + RecordDeclToBoolMap recordsWithAccessibleMethods; + collectRecordsWithAccessibleMethods(recordsWithAccessibleMethods, RD, + ConstructedTy); + + // Lookup for a method that accepts a pointer to a constructed record type + // among the collected records. If the method is found we do not consider an + // unassigned memory as a leak as the memory ('this' pointer) could + // potentially be passed to the method and escape. + for (const auto &Record : recordsWithAccessibleMethods) { + + // First lookup in the cache. + if (RecToReferencedTypesMap.count(Record.first)) { + auto const &ReferencedTypes = RecToReferencedTypesMap[Record.first]; + + if (ReferencedTypes.count(ConstructedTy)) + return true; + + // Iterate over bases of a constructed class. + for (const auto &ConstructedBase : ConstructedRD->bases()) { + QualType CBTy = ConstructedBase.getType(); + if (!CBTy.isNull() && ReferencedTypes.count(CBTy.getUnqualifiedType())) + return true; + } + + continue; + } + + // We collect record types referenced by methods of a record being + // processed to speed up lookup next time. + RecToReferencedTypesMapTy::mapped_type ReferencedTypesCollector; + + // Types referenced by a records methods were not cached, lookup among + // all parameters of all accessible methods. + for (const auto *Method : Record.first->methods()) { + + if (!Record.second && Method->getAccess() != AS_public) + // Method access level is not public and constructed record is + // not a friend of a processed record - the method is inaccessible + // from a constructed record. + continue; + + // Iterate over method parameters, try to find one of a type of a + // constructed record or one of its bases. + for (const auto *MethodParam : Method->params()) { + QualType MPPointeeT = MethodParam->getType()->getPointeeType(); + if (MPPointeeT.isNull()) + continue; + + MPPointeeT = getDeepPointeeType(MPPointeeT).getCanonicalType(); + if (!isa(MPPointeeT)) + continue; + + ReferencedTypesCollector.insert(MPPointeeT); + + if (MPPointeeT == ConstructedTy) + return true; + + // Iterate over bases of a constructed class. + for (const auto &ConstructedBase : ConstructedRD->bases()) { + QualType CBTy = ConstructedBase.getType(); + if (!CBTy.isNull() && MPPointeeT == CBTy.getUnqualifiedType()) + return true; + } + } + } + // Cache referenced types. + RecToReferencedTypesMap[Record.first] = ReferencedTypesCollector; + } + } + + return false; +} + void MallocChecker::checkPostStmt(const CXXNewExpr *NE, CheckerContext &C) const { @@ -763,6 +940,10 @@ checkUseAfterFree(Sym, C, *I); if (!isStandardNewDelete(NE->getOperatorNew(), C.getASTContext())) + return; + + ParentMap &PM = C.getLocationContext()->getParentMap(); + if (!PM.isConsumedExpr(NE) && unusedNewCanEscape(NE)) return; ProgramStateRef State = C.getState(); Index: test/Analysis/NewDeleteLeaks-PR19102.cpp =================================================================== --- test/Analysis/NewDeleteLeaks-PR19102.cpp +++ test/Analysis/NewDeleteLeaks-PR19102.cpp @@ -0,0 +1,191 @@ +// RUN: %clang_cc1 -analyze -analyzer-checker=core,alpha.cplusplus.NewDeleteLeaks -std=c++11 -verify %s + +class B1; + +class A0 { +public: + // Parameter is passed by value, b1 is not treated escaped. + void SetB1(B1 b1); +}; + +class A1 { +public: + void SetB1(B1* b1); +}; + +class A2 { + // SetB1 is private and A2 is not a friend of B1, b1 is not treated escaped. + void SetB1(B1* b1); +}; + +class A3 { +protected: + // SetB1 is protected and A2 is not a friend of B1, b1 is not treated escaped. + void SetB1(B1* b1); +}; + +class A4 { + friend class B1; + void SetB1(B1* b1); +}; + +class A5 { +protected: + friend class B1; + void SetB1(int, B1& b1); +}; + +class A6 { +public: + void SetB1(B1* &b1); +}; + +class AA_base0 { + void SetB1(B1* b1); +}; + +class AA_base1 { +public: + void SetB1(B1* b1); +}; + +class AA_base2 { +friend class B1; + void SetB1(B1* b1); +}; + +class AA_base3 : public AA_base2 {}; + +class AA0 : public AA_base0 {}; // SetB1 is private, inaccessible from outside. + +class AA1 : public AA_base1 {}; + +class AA2 : public AA_base2 {}; + +class AA3 : AA_base1 {}; // private inheritance, SetB1 is inaccessible from outside. + +class AA4 : protected AA_base1 {}; // protected inheritance, SetB1 is inaccessible from outside. + +class AA5 : private AA_base2 {}; // private inheritance, SetB1 is inaccessible from outside. + +class AA6 : private AA_base1 { +friend class B1; +}; + +class AA7 : private AA_base3 { +friend class B1; +}; + +class AA8 : public AA_base0, public AA_base1 {}; + +class AA9 : public AA_base0, protected AA_base1 {}; // SetB1 is inaccessible from outside. + +class AA10 : virtual AA_base0, virtual protected AA_base2, virtual public AA_base3 {}; + +class B1 { +public: + B1(A0 &a0); + B1(int); + B1(A1 a1); + B1(A1* a1); + B1(int, A1& a1); + + B1(A2 &a2); + + B1(A3 &a3); + + B1(A4 &a4); + + B1(A5 &a5); + + B1(A6 &a6); + B1(A6 **a6); + + B1(AA0 &aa0); + + B1(AA1 &aa1); + + B1(AA2 &aa2); + + B1(AA3 &aa3); + + B1(AA4 &aa4); + + B1(AA5 &aa5); + + B1(AA6 &aa6); + + B1(AA7 &aa7); + + B1(AA8 &aa8); + + B1(AA9 &aa9); + + B1(AA10 &aa10); +}; + +class B2: public B1 { +public: + B2(A1 &a1); +}; + +void f() { + A0 a0; + new B1(a0); // expected-warning@+2 {{Potential memory leak}} + + A1 a1; + new B1(1); // expected-warning@+1 {{Potential memory leak}} + new B1(a1); // expected-warning@+1 {{Potential memory leak}} + new B1(&a1); // no warning + new B1(1, a1); // no warning + new B2(a1); // no warning + + A2 a2; + new B1(a2); // expected-warning@+2 {{Potential memory leak}} + + A3 a3; + new B1(a3); // expected-warning@+2 {{Potential memory leak}} + + A4 a4; + new B1(a4); // no warning + + A5 a5; + new B1(a5); // no warning + + A6 a6, *a6p = &a6; + new B1(a6); // no warning + new B1(&a6p); // no warning + + AA0 aa0; + new B1(aa0); // expected-warning@+2 {{Potential memory leak}} + + AA1 aa1; + new B1(aa1); // no warning + + AA2 aa2; + new B1(aa2); // no warning + + AA3 aa3; + new B1(aa3); // expected-warning@+2 {{Potential memory leak}} + + AA4 aa4; + new B1(aa4); // expected-warning@+2 {{Potential memory leak}} + + AA5 aa5; + new B1(aa5); // expected-warning@+2 {{Potential memory leak}} + + AA6 aa6; + new B1(aa6); // no warning + + AA7 aa7; + new B1(aa7); // no warning + + AA8 aa8; + new B1(aa8); // no warning + + AA9 aa9; + new B1(aa9); // expected-warning@+2 {{Potential memory leak}} + + AA10 aa10; + new B1(aa10); +}