Bug 1332078 - Ignore trivial nodes when checking for MOZ_NO_ADDREF_RELEASE_ON_RETURN calls, r=ehsan
authorMichael Layzell <michael@thelayzells.com>
Wed, 18 Jan 2017 15:45:55 -0500
changeset 375230 d6c6e2dae5f431fba8e51518a4b61c5808d74ed1
parent 375229 31b797ff27420565e351df1321ac03bcc26d3fa9
child 375231 0fdc7261d01752d1a6d45c16f67078eb5903658c
push id6996
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 20:48:21 +0000
treeherdermozilla-beta@d89512dab048 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersehsan
bugs1332078
milestone53.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1332078 - Ignore trivial nodes when checking for MOZ_NO_ADDREF_RELEASE_ON_RETURN calls, r=ehsan MozReview-Commit-ID: JWBQXSTrlad
build/clang-plugin/NoAddRefReleaseOnReturnChecker.cpp
build/clang-plugin/OverrideBaseCallChecker.cpp
build/clang-plugin/Utils.h
--- a/build/clang-plugin/NoAddRefReleaseOnReturnChecker.cpp
+++ b/build/clang-plugin/NoAddRefReleaseOnReturnChecker.cpp
@@ -1,40 +1,32 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "NoAddRefReleaseOnReturnChecker.h"
 #include "CustomMatchers.h"
 
 void NoAddRefReleaseOnReturnChecker::registerMatchers(MatchFinder* AstMatcher) {
-  // First, look for direct parents of the MemberExpr.
-  AstMatcher->addMatcher(
-      callExpr(
-          callee(functionDecl(hasNoAddRefReleaseOnReturnAttr()).bind("func")),
-          hasParent(memberExpr(isAddRefOrRelease(), hasParent(callExpr()))
-                        .bind("member")))
-          .bind("node"),
-      this);
-  // Then, look for MemberExpr that need to be casted to the right type using
-  // an intermediary CastExpr before we get to the CallExpr.
-  AstMatcher->addMatcher(
-      callExpr(
-          callee(functionDecl(hasNoAddRefReleaseOnReturnAttr()).bind("func")),
-          hasParent(castExpr(
-              hasParent(memberExpr(isAddRefOrRelease(), hasParent(callExpr()))
-                            .bind("member")))))
-          .bind("node"),
-      this);
+  // Look for all of the calls to AddRef() or Release()
+  AstMatcher->addMatcher(memberExpr(isAddRefOrRelease(), hasParent(callExpr())).bind("member"),
+                         this);
 }
 
 void NoAddRefReleaseOnReturnChecker::check(
     const MatchFinder::MatchResult &Result) {
-  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());
+  const Expr *Base = IgnoreTrivials(Member->getBase());
 
-  diag(Node->getLocStart(),
-       "%1 cannot be called on the return value of %0",
-       DiagnosticIDs::Error) << Func << Method;
+  // Check if the call to AddRef() or Release() was made on the result of a call
+  // to a MOZ_NO_ADDREF_RELEASE_ON_RETURN function or method.
+  if (auto *Call = dyn_cast<CallExpr>(Base)) {
+    if (auto *Callee = Call->getDirectCallee()) {
+      if (hasCustomAnnotation(Callee, "moz_no_addref_release_on_return")) {
+        diag(Call->getLocStart(),
+             "%1 cannot be called on the return value of %0",
+             DiagnosticIDs::Error)
+          << Callee
+          << dyn_cast<CXXMethodDecl>(Member->getMemberDecl());
+      }
+    }
+  }
 }
--- a/build/clang-plugin/OverrideBaseCallChecker.cpp
+++ b/build/clang-plugin/OverrideBaseCallChecker.cpp
@@ -96,17 +96,17 @@ void OverrideBaseCallChecker::check(
 
     // Loop through the body of our method and search for calls to
     // base methods
     evaluateExpression(Method->getBody(), MethodsList);
 
     // If list is not empty pop up errors
     for (auto BaseMethod : MethodsList) {
       std::string QualName;
-      llvm::raw_string_ostream OS(QualName);
+      raw_string_ostream OS(QualName);
       BaseMethod->printQualifiedName(OS);
 
       diag(Method->getLocation(), Error, DiagnosticIDs::Error)
           << OS.str()
           << Decl->getName();
     }
   }
 }
--- a/build/clang-plugin/Utils.h
+++ b/build/clang-plugin/Utils.h
@@ -301,17 +301,19 @@ inline bool typeIsRefPtr(QualType Q) {
   return false;
 }
 
 // The method defined in clang for ignoring implicit nodes doesn't work with
 // some AST trees. To get around this, we define our own implementation of
 // IgnoreTrivials.
 inline const Stmt *IgnoreTrivials(const Stmt *s) {
   while (true) {
-    if (auto *ewc = dyn_cast<ExprWithCleanups>(s)) {
+    if (!s) {
+      return nullptr;
+    } else if (auto *ewc = dyn_cast<ExprWithCleanups>(s)) {
       s = ewc->getSubExpr();
     } else if (auto *mte = dyn_cast<MaterializeTemporaryExpr>(s)) {
       s = mte->GetTemporaryExpr();
     } else if (auto *bte = dyn_cast<CXXBindTemporaryExpr>(s)) {
       s = bte->getSubExpr();
     } else if (auto *ice = dyn_cast<ImplicitCastExpr>(s)) {
       s = ice->getSubExpr();
     } else if (auto *pe = dyn_cast<ParenExpr>(s)) {