Bug 602122 - Add a static analysis to find XPCOM classes with duplicate mRefCnt members; r=ehsan (4fffaa9e) · Commits · Tommy Webb / Tor Browser

build/clang-plugin/clang-plugin.cpp

+82 −0

Original line number	Diff line number	Diff line
		@@ -96,6 +96,11 @@ private:
		virtual void run(const MatchFinder::MatchResult &Result);
		};

		class NoDuplicateRefCntMemberChecker : public MatchFinder::MatchCallback {
		public:
		virtual void run(const MatchFinder::MatchResult &Result);
		};

		class NeedsNoVTableTypeChecker : public MatchFinder::MatchCallback {
		public:
		virtual void run(const MatchFinder::MatchResult &Result);
		@@ -115,6 +120,7 @@ private:
		NoAddRefReleaseOnReturnChecker noAddRefReleaseOnReturnChecker;
		RefCountedInsideLambdaChecker refCountedInsideLambdaChecker;
		ExplicitOperatorBoolChecker explicitOperatorBoolChecker;
		NoDuplicateRefCntMemberChecker noDuplicateRefCntMemberChecker;
		NeedsNoVTableTypeChecker needsNoVTableTypeChecker;
		NonMemMovableChecker nonMemMovableChecker;
		MatchFinder astMatcher;
		@@ -581,6 +587,48 @@ bool IsInSystemHeader(const ASTContext &AC, const T &D) {
		return SourceManager.isInSystemHeader(ExpansionLoc);
		}

		const FieldDecl getClassRefCntMember(const CXXRecordDecl D) {
		for (RecordDecl::field_iterator field = D->field_begin(), e = D->field_end();
		field != e; ++field) {
		if (field->getName() == "mRefCnt") {
		return *field;
		}
		}
		return 0;
		}

		const FieldDecl *getClassRefCntMember(QualType T) {
		while (const ArrayType *arrTy = T->getAsArrayTypeUnsafe())
		T = arrTy->getElementType();
		CXXRecordDecl *clazz = T->getAsCXXRecordDecl();
		return clazz ? getClassRefCntMember(clazz) : 0;
		}

		const FieldDecl *getBaseRefCntMember(QualType T);

		const FieldDecl getBaseRefCntMember(const CXXRecordDecl D) {
		const FieldDecl *refCntMember = getClassRefCntMember(D);
		if (refCntMember && isClassRefCounted(D)) {
		return refCntMember;
		}

		for (CXXRecordDecl::base_class_const_iterator base = D->bases_begin(), e = D->bases_end();
		base != e; ++base) {
		refCntMember = getBaseRefCntMember(base->getType());
		if (refCntMember) {
		return refCntMember;
		}
		}
		return 0;
		}

		const FieldDecl *getBaseRefCntMember(QualType T) {
		while (const ArrayType *arrTy = T->getAsArrayTypeUnsafe())
		T = arrTy->getElementType();
		CXXRecordDecl *clazz = T->getAsCXXRecordDecl();
		return clazz ? getBaseRefCntMember(clazz) : 0;
		}

		bool typeHasVTable(QualType T) {
		while (const ArrayType *arrTy = T->getAsArrayTypeUnsafe())
		T = arrTy->getElementType();
		@@ -742,6 +790,10 @@ AST_POLYMORPHIC_MATCHER_P(equalsBoundNode,

		#endif

		AST_MATCHER(CXXRecordDecl, hasRefCntMember) {
		return isClassRefCounted(&Node) && getClassRefCntMember(&Node);
		}

		AST_MATCHER(QualType, hasVTable) {
		return typeHasVTable(Node);
		}
		@@ -984,6 +1036,10 @@ DiagnosticsMatcher::DiagnosticsMatcher()
		hasName("operator _Bool"))).bind("node"),
		&explicitOperatorBoolChecker);

		astMatcher.addMatcher(recordDecl(allOf(decl().bind("decl"),
		hasRefCntMember())),
		&noDuplicateRefCntMemberChecker);

		astMatcher.addMatcher(classTemplateSpecializationDecl(
		allOf(hasAnyTemplateArgument(refersToType(hasVTable())),
		hasNeedsNoVTableTypeAttr())).bind("node"),
		@@ -1177,6 +1233,32 @@ void DiagnosticsMatcher::ExplicitOperatorBoolChecker::run(
		}
		}

		void DiagnosticsMatcher::NoDuplicateRefCntMemberChecker::run(
		const MatchFinder::MatchResult &Result) {
		DiagnosticsEngine &Diag = Result.Context->getDiagnostics();
		unsigned warningID = Diag.getDiagnosticIDs()->getCustomDiagID(
		DiagnosticIDs::Error, "Refcounted record %0 has multiple mRefCnt members");
		unsigned note1ID = Diag.getDiagnosticIDs()->getCustomDiagID(
		DiagnosticIDs::Note, "Superclass %0 also has an mRefCnt member");
		unsigned note2ID = Diag.getDiagnosticIDs()->getCustomDiagID(
		DiagnosticIDs::Note, "Consider using the _INHERITED macros for AddRef and Release here");

		const CXXRecordDecl *decl = Result.Nodes.getNodeAs<CXXRecordDecl>("decl");
		const FieldDecl *refCntMember = getClassRefCntMember(decl);
		assert(refCntMember && "The matcher checked to make sure we have a refCntMember");

		// Check every superclass for whether it has a base with a refcnt member, and warn for those which do
		for (CXXRecordDecl::base_class_const_iterator base = decl->bases_begin(), e = decl->bases_end();
		base != e; ++base) {
		const FieldDecl *baseRefCntMember = getBaseRefCntMember(base->getType());
		if (baseRefCntMember) {
		Diag.Report(decl->getLocStart(), warningID) << decl;
		Diag.Report(baseRefCntMember->getLocStart(), note1ID) << baseRefCntMember->getParent();
		Diag.Report(refCntMember->getLocStart(), note2ID);
		}
		}
		}

		void DiagnosticsMatcher::NeedsNoVTableTypeChecker::run(
		const MatchFinder::MatchResult &Result) {
		DiagnosticsEngine &Diag = Result.Context->getDiagnostics();

build/clang-plugin/tests/TestNoDuplicateRefCntMember.cpp

0 → 100644

+38 −0

Original line number	Diff line number	Diff line
		class C1 {};

		class RC1 {
		public:
		virtual void AddRef();
		virtual void Release();

		private:
		int mRefCnt; // expected-note 2 {{Superclass 'RC1' also has an mRefCnt member}}
		};

		class RC2 : public RC1 { // expected-error {{Refcounted record 'RC2' has multiple mRefCnt members}}
		public:
		virtual void AddRef();
		virtual void Release();

		private:
		int mRefCnt; // expected-note {{Consider using the _INHERITED macros for AddRef and Release here}}
		};

		class C2 : public RC1 {};

		class RC3 : public RC1 {};

		class RC4 : public RC3, public C2 {};

		class RC5 : public RC1 {};

		class RC6 : public C1, public RC5 { // expected-error {{Refcounted record 'RC6' has multiple mRefCnt members}}
		public:
		virtual void AddRef();
		virtual void Release();

		private:
		int mRefCnt; // expected-note {{Consider using the _INHERITED macros for AddRef and Release here}}
		};

		class Predecl;

build/clang-plugin/tests/moz.build

+1 −0

Original line number	Diff line number	Diff line
		@@ -17,6 +17,7 @@ SOURCES += [
		'TestNeedsNoVTableType.cpp',
		'TestNoAddRefReleaseOnReturn.cpp',
		'TestNoArithmeticExprInArgument.cpp',
		'TestNoDuplicateRefCntMember.cpp',
		'TestNonHeapClass.cpp',
		'TestNonMemMovable.cpp',
		'TestNoRefcountedInsideLambdas.cpp',