Bug 1307702 - clang-plugin - static analysis to reject ussage of MOZ_REQUIRED_BASE_METHOD on non-virtual base methods. r?mystor draft
authorAndi-Bogdan Postelnicu <bpostelnicu@mozilla.com>
Wed, 05 Oct 2016 18:30:01 +0300
changeset 421183 90b732068ee4ba0160d1e8fc1c52730112f2535c
parent 420017 955840bfd3c20eb24dd5a01be27bdc55c489a285
child 533003 d024b38707d767235c5c10ee8c7ca73c1bb34532
push id31420
push userbmo:bpostelnicu@mozilla.com
push dateWed, 05 Oct 2016 15:30:29 +0000
reviewersmystor
bugs1307702
milestone52.0a1
Bug 1307702 - clang-plugin - static analysis to reject ussage of MOZ_REQUIRED_BASE_METHOD on non-virtual base methods. r?mystor MozReview-Commit-ID: 9G79Wm5fbHc
build/clang-plugin/clang-plugin.cpp
build/clang-plugin/tests/TestOverrideBaseCallAnnotation.cpp
build/clang-plugin/tests/moz.build
--- a/build/clang-plugin/clang-plugin.cpp
+++ b/build/clang-plugin/clang-plugin.cpp
@@ -158,16 +158,25 @@ private:
     virtual void run(const MatchFinder::MatchResult &Result);
   };
 
   class KungFuDeathGripChecker : public MatchFinder::MatchCallback {
   public:
     virtual void run(const MatchFinder::MatchResult &Result);
   };
 
+/*
+ *  This is a companion checker for OverrideBaseCallChecker that rejects
+ *  the usage of MOZ_REQUIRED_BASE_METHOD on non-virtual base methods.
+ */
+  class OverrideBaseCallUsageChecker : public MatchFinder::MatchCallback {
+  public:
+    virtual void run(const MatchFinder::MatchResult &Result);
+  };
+
   ScopeChecker Scope;
   ArithmeticArgChecker ArithmeticArg;
   TrivialCtorDtorChecker TrivialCtorDtor;
   NaNExprChecker NaNExpr;
   NoAddRefReleaseOnReturnChecker NoAddRefReleaseOnReturn;
   RefCountedInsideLambdaChecker RefCountedInsideLambda;
   ExplicitOperatorBoolChecker ExplicitOperatorBool;
   NoDuplicateRefCntMemberChecker NoDuplicateRefCntMember;
@@ -175,16 +184,17 @@ private:
   NonMemMovableTemplateArgChecker NonMemMovableTemplateArg;
   NonMemMovableMemberChecker NonMemMovableMember;
   ExplicitImplicitChecker ExplicitImplicit;
   NoAutoTypeChecker NoAutoType;
   NoExplicitMoveConstructorChecker NoExplicitMoveConstructor;
   RefCountedCopyConstructorChecker RefCountedCopyConstructor;
   AssertAssignmentChecker AssertAttribution;
   KungFuDeathGripChecker KungFuDeathGrip;
+  OverrideBaseCallUsageChecker OverrideBaseCallUsage;
   MatchFinder AstMatcher;
 };
 
 namespace {
 
 std::string getDeclarationNamespace(const Decl *Declaration) {
   const DeclContext *DC =
       Declaration->getDeclContext()->getEnclosingNamespaceContext();
@@ -863,16 +873,27 @@ AST_MATCHER(CallExpr, isAssertAssignment
 
 AST_MATCHER(CXXRecordDecl, isLambdaDecl) {
   return Node.isLambda();
 }
 
 AST_MATCHER(QualType, isRefPtr) {
   return typeIsRefPtr(Node);
 }
+
+AST_MATCHER(CXXMethodDecl, isRequiredBaseMethod) {
+  const CXXMethodDecl *Decl = Node.getCanonicalDecl();
+  return Decl
+      && MozChecker::hasCustomAnnotation(Decl, "moz_required_base_method");
+}
+
+AST_MATCHER(CXXMethodDecl, isNonVirtual) {
+  const CXXMethodDecl *Decl = Node.getCanonicalDecl();
+  return Decl && !Decl->isVirtual();
+}
 }
 }
 
 namespace {
 
 void CustomTypeAnnotation::dumpAnnotationReason(DiagnosticsEngine &Diag,
                                                 QualType T,
                                                 SourceLocation Loc) {
@@ -1190,16 +1211,20 @@ DiagnosticsMatcher::DiagnosticsMatcher()
       &RefCountedCopyConstructor);
 
   AstMatcher.addMatcher(
       callExpr(isAssertAssignmentTestFunc()).bind("funcCall"),
       &AssertAttribution);
 
   AstMatcher.addMatcher(varDecl(hasType(isRefPtr())).bind("decl"),
                         &KungFuDeathGrip);
+
+  AstMatcher.addMatcher(
+      cxxMethodDecl(isNonVirtual(), isRequiredBaseMethod()).bind("method"),
+      &OverrideBaseCallUsage);
 }
 
 // These enum variants determine whether an allocation has occured in the code.
 enum AllocationVariety {
   AV_None,
   AV_Global,
   AV_Automatic,
   AV_Temporary,
@@ -1829,16 +1854,27 @@ void DiagnosticsMatcher::KungFuDeathGrip
     NoteThing = "value";
   }
 
   // We cannot provide the note if we don't have an initializer
   Diag.Report(D->getLocStart(), ErrorID) << D->getType() << ErrThing;
   Diag.Report(E->getLocStart(), NoteID) << NoteThing << getNameChecked(D);
 }
 
+void DiagnosticsMatcher::OverrideBaseCallUsageChecker::run(
+    const MatchFinder::MatchResult &Result) {
+  DiagnosticsEngine &Diag = Result.Context->getDiagnostics();
+  unsigned ErrorID = Diag.getDiagnosticIDs()->getCustomDiagID(
+      DiagnosticIDs::Error,
+      "MOZ_REQUIRED_BASE_METHOD can be used only on virtual methods");
+  const CXXMethodDecl *Method = Result.Nodes.getNodeAs<CXXMethodDecl>("method");
+
+  Diag.Report(Method->getLocation(), ErrorID);
+}
+
 class MozCheckAction : public PluginASTAction {
 public:
   ASTConsumerPtr CreateASTConsumer(CompilerInstance &CI,
                                    StringRef FileName) override {
 #if CLANG_VERSION_FULL >= 306
     std::unique_ptr<MozChecker> Checker(llvm::make_unique<MozChecker>(CI));
     ASTConsumerPtr Other(Checker->getOtherConsumer());
 
new file mode 100644
--- /dev/null
+++ b/build/clang-plugin/tests/TestOverrideBaseCallAnnotation.cpp
@@ -0,0 +1,46 @@
+#define MOZ_REQUIRED_BASE_METHOD __attribute__((annotate("moz_required_base_method")))
+
+class Base {
+public:
+  virtual void fo() MOZ_REQUIRED_BASE_METHOD {
+  }
+};
+
+class BaseNonVirtual {
+public:
+  void fo() MOZ_REQUIRED_BASE_METHOD { // expected-error {{MOZ_REQUIRED_BASE_METHOD can be used only on virtual methods}}
+  }
+};
+
+class Deriv : public BaseNonVirtual {
+public:
+  virtual void fo() MOZ_REQUIRED_BASE_METHOD {
+  }
+};
+
+class DerivVirtual : public Base {
+public:
+  void fo() MOZ_REQUIRED_BASE_METHOD {
+  }
+};
+
+class BaseOperator {
+public:
+  BaseOperator& operator++() MOZ_REQUIRED_BASE_METHOD { // expected-error {{MOZ_REQUIRED_BASE_METHOD can be used only on virtual methods}}
+    return *this;
+  }
+};
+
+class DerivOperator : public BaseOperator {
+public:
+  virtual DerivOperator& operator++() {
+    return *this;
+  }
+};
+
+class DerivPrimeOperator : public DerivOperator {
+public:
+  DerivPrimeOperator& operator++() {
+    return *this;
+  }
+};
--- a/build/clang-plugin/tests/moz.build
+++ b/build/clang-plugin/tests/moz.build
@@ -27,15 +27,16 @@ SOURCES += [
     'TestNoAutoType.cpp',
     'TestNoDuplicateRefCntMember.cpp',
     'TestNoExplicitMoveConstructor.cpp',
     'TestNonHeapClass.cpp',
     'TestNonMemMovable.cpp',
     'TestNonMemMovableStd.cpp',
     'TestNonTemporaryClass.cpp',
     'TestNoRefcountedInsideLambdas.cpp',
+    'TestOverrideBaseCallAnnotation.cpp',
     'TestRefCountedCopyConstructor.cpp',
     'TestStackClass.cpp',
     'TestTrivialCtorDtor.cpp',
 ]
 
 DISABLE_STL_WRAPPING = True
 NO_VISIBILITY_FLAGS = True