Now searching on the class and all superclasses that has methods accessible
from the constructed record. Referenced types are now cached.
The search is expensive, but the case is very rare I hope. Only unassigned new
are analyzed this way.
http://reviews.llvm.org/D4025
Files:
lib/StaticAnalyzer/Checkers/MallocChecker.cpp
test/Analysis/NewDeleteLeaks-PR19102.cpp
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<const ExplodedNode*, const MemRegion*> LeakInfo;
+typedef llvm::SmallDenseMap<const CXXRecordDecl *, bool, 8> RecordDeclToBoolMap;
+
class MallocChecker : public Checker<check::DeadSymbols,
check::PointerEscape,
check::ConstPointerEscape,
@@ -214,6 +218,12 @@
*II_kmalloc;
mutable Optional<uint64_t> KernelZeroFlagVal;
+ typedef llvm::DenseMap<const CXXRecordDecl *,
+ llvm::SmallPtrSet<QualType, 32>> 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<RecordType>(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);
+}
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
