Bug 1114999 - Part 1: Create an analysis for disallowing calling AddRef and Release on the return value of functions; r=jrmuizel

build/clang-plugin/clang-plugin.cpp

@@ -74,12 +74,18 @@ private:
     virtual void run(const MatchFinder::MatchResult &Result);
   };
 
+  class NoAddRefReleaseOnReturnChecker : public MatchFinder::MatchCallback {
+  public:
+    virtual void run(const MatchFinder::MatchResult &Result);
+  };
+
   ScopeChecker stackClassChecker;
   ScopeChecker globalClassChecker;
   NonHeapClassChecker nonheapClassChecker;
   ArithmeticArgChecker arithmeticArgChecker;
   TrivialCtorDtorChecker trivialCtorDtorChecker;
   NaNExprChecker nanExprChecker;
+  NoAddRefReleaseOnReturnChecker noAddRefReleaseOnReturnChecker;
   MatchFinder astMatcher;
 };
 
@@ -389,6 +395,12 @@ AST_MATCHER(CXXRecordDecl, hasTrivialCtorDtor) {
   return MozChecker::hasCustomAnnotation(&Node, "moz_trivial_ctor_dtor");
 }
 
+/// This matcher will match any function declaration that is marked to prohibit
+/// calling AddRef or Release on its return value.
+AST_MATCHER(FunctionDecl, hasNoAddRefReleaseOnReturnAttr) {
+  return MozChecker::hasCustomAnnotation(&Node, "moz_no_addref_release_on_return");
+}
+
 /// This matcher will match all arithmetic binary operators.
 AST_MATCHER(BinaryOperator, binaryArithmeticOperator) {
   BinaryOperatorKind opcode = Node.getOpcode();
@@ -458,6 +470,17 @@ AST_MATCHER(BinaryOperator, isInSkScalarDotH) {
   return llvm::sys::path::rbegin(FileName)->equals("SkScalar.h");
 }
 
+/// This matcher will match all accesses to AddRef or Release methods.
+AST_MATCHER(MemberExpr, isAddRefOrRelease) {
+  ValueDecl *Member = Node.getMemberDecl();
+  CXXMethodDecl *Method = dyn_cast<CXXMethodDecl>(Member);
+  if (Method) {
+    std::string Name = Method->getNameAsString();
+    return Name == "AddRef" || Name == "Release";
+  }
+  return false;
+}
+
 }
 }
 
@@ -548,6 +571,12 @@ DiagnosticsMatcher::DiagnosticsMatcher()
           unless(anyOf(isInSystemHeader(), isInSkScalarDotH()))
       )).bind("node"),
     &nanExprChecker);
+
+  astMatcher.addMatcher(callExpr(callee(functionDecl(hasNoAddRefReleaseOnReturnAttr()).bind("func")),
+                                 hasParent(memberExpr(isAddRefOrRelease(),
+                                                      hasParent(callExpr())).bind("member")
+      )).bind("node"),
+    &noAddRefReleaseOnReturnChecker);
 }
 
 void DiagnosticsMatcher::ScopeChecker::run(
@@ -733,6 +762,19 @@ void DiagnosticsMatcher::NaNExprChecker::run(
   }
 }
 
+void DiagnosticsMatcher::NoAddRefReleaseOnReturnChecker::run(
+    const MatchFinder::MatchResult &Result) {
+  DiagnosticsEngine &Diag = Result.Context->getDiagnostics();
+  unsigned errorID = Diag.getDiagnosticIDs()->getCustomDiagID(
+      DiagnosticIDs::Error, "%1 cannot be called on the return value of %0");
+  const Stmt *node = Result.Nodes.getNodeAs<Stmt>("node");
+  const FunctionDecl *func = Result.Nodes.getNodeAs<FunctionDecl>("func");
+  const MemberExpr *member = Result.Nodes.getNodeAs<MemberExpr>("member");
+  const CXXMethodDecl *method = dyn_cast<CXXMethodDecl>(member->getMemberDecl());
+
+  Diag.Report(node->getLocStart(), errorID) << func << method;
+}
+
 class MozCheckAction : public PluginASTAction {
 public:
   ASTConsumerPtr CreateASTConsumer(CompilerInstance &CI, StringRef fileName) override {

build/clang-plugin/tests/Makefile.in

@@ -3,9 +3,9 @@
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 # Build without any warning flags, and with clang verify flag for a
-# syntax-only build (no codegen).
-OS_CFLAGS := $(filter-out -W%,$(OS_CFLAGS)) -fsyntax-only -Xclang -verify
-OS_CXXFLAGS := $(filter-out -W%,$(OS_CXXFLAGS)) -fsyntax-only -Xclang -verify
+# syntax-only build (no codegen), without a limit on the number of errors.
+OS_CFLAGS := $(filter-out -W%,$(OS_CFLAGS)) -fsyntax-only -Xclang -verify -ferror-limit=0
+OS_CXXFLAGS := $(filter-out -W%,$(OS_CXXFLAGS)) -fsyntax-only -Xclang -verify -ferror-limit=0
 
 include $(topsrcdir)/config/rules.mk
 

build/clang-plugin/tests/TestNoAddRefReleaseOnReturn.cpp

@@ -0,0 +1,65 @@
+#define MOZ_NO_ADDREF_RELEASE_ON_RETURN __attribute__((annotate("moz_no_addref_release_on_return")))
+
+struct Test {
+  void AddRef();
+  void Release();
+  void foo();
+};
+
+struct S {
+  Test* f() MOZ_NO_ADDREF_RELEASE_ON_RETURN;
+  Test& g() MOZ_NO_ADDREF_RELEASE_ON_RETURN;
+  Test  h() MOZ_NO_ADDREF_RELEASE_ON_RETURN;
+};
+
+template<class T>
+struct X {
+  T* f() MOZ_NO_ADDREF_RELEASE_ON_RETURN;
+  T& g() MOZ_NO_ADDREF_RELEASE_ON_RETURN;
+  T  h() MOZ_NO_ADDREF_RELEASE_ON_RETURN;
+};
+
+template<class T>
+struct SP {
+  T* operator->() MOZ_NO_ADDREF_RELEASE_ON_RETURN;
+};
+
+Test* f() MOZ_NO_ADDREF_RELEASE_ON_RETURN;
+Test& g() MOZ_NO_ADDREF_RELEASE_ON_RETURN;
+Test  h() MOZ_NO_ADDREF_RELEASE_ON_RETURN;
+
+void test() {
+  S s;
+  s.f()->AddRef(); // expected-error{{'AddRef' cannot be called on the return value of 'f'}}
+  s.f()->Release(); // expected-error{{'Release' cannot be called on the return value of 'f'}}
+  s.f()->foo();
+  s.g().AddRef(); // expected-error{{'AddRef' cannot be called on the return value of 'g'}}
+  s.g().Release(); // expected-error{{'Release' cannot be called on the return value of 'g'}}
+  s.g().foo();
+  s.h().AddRef(); // expected-error{{'AddRef' cannot be called on the return value of 'h'}}
+  s.h().Release(); // expected-error{{'Release' cannot be called on the return value of 'h'}}
+  s.h().foo();
+  X<Test> x;
+  x.f()->AddRef(); // expected-error{{'AddRef' cannot be called on the return value of 'f'}}
+  x.f()->Release(); // expected-error{{'Release' cannot be called on the return value of 'f'}}
+  x.f()->foo();
+  x.g().AddRef(); // expected-error{{'AddRef' cannot be called on the return value of 'g'}}
+  x.g().Release(); // expected-error{{'Release' cannot be called on the return value of 'g'}}
+  x.g().foo();
+  x.h().AddRef(); // expected-error{{'AddRef' cannot be called on the return value of 'h'}}
+  x.h().Release(); // expected-error{{'Release' cannot be called on the return value of 'h'}}
+  x.h().foo();
+  SP<Test> sp;
+  sp->AddRef(); // expected-error{{'AddRef' cannot be called on the return value of 'operator->'}}
+  sp->Release(); // expected-error{{'Release' cannot be called on the return value of 'operator->'}}
+  sp->foo();
+  f()->AddRef(); // expected-error{{'AddRef' cannot be called on the return value of 'f'}}
+  f()->Release(); // expected-error{{'Release' cannot be called on the return value of 'f'}}
+  f()->foo();
+  g().AddRef(); // expected-error{{'AddRef' cannot be called on the return value of 'g'}}
+  g().Release(); // expected-error{{'Release' cannot be called on the return value of 'g'}}
+  g().foo();
+  h().AddRef(); // expected-error{{'AddRef' cannot be called on the return value of 'h'}}
+  h().Release(); // expected-error{{'Release' cannot be called on the return value of 'h'}}
+  h().foo();
+}

build/clang-plugin/tests/moz.build

@@ -11,6 +11,7 @@ SOURCES += [
     'TestMustOverride.cpp',
     'TestNANTestingExpr.cpp',
     'TestNANTestingExprC.c',
+    'TestNoAddRefReleaseOnReturn.cpp',
     'TestNoArithmeticExprInArgument.cpp',
     'TestNonHeapClass.cpp',
     'TestStackClass.cpp',