Bug 1281935 - Part 1: Relax raw pointer inside lambda analysis, r=ehsan

build/clang-plugin/clang-plugin.cpp

@@ -803,6 +803,11 @@ AST_MATCHER(CallExpr, isAssertAssignmentTestFunc) {
       && method->getDeclName().isIdentifier()
       && method->getName() == assertName;
 }
+
+AST_MATCHER(CXXRecordDecl, isLambdaDecl) {
+  return Node.isLambda();
+}
+
 }
 }
 
@@ -1051,12 +1056,21 @@ DiagnosticsMatcher::DiagnosticsMatcher() {
           .bind("node"),
       &noAddRefReleaseOnReturnChecker);
 
-  // Match declrefs with type "pointer to object of ref-counted type" inside a
-  // lambda, where the declaration they reference is not inside the lambda.
-  // This excludes arguments and local variables, leaving only captured
-  // variables.
-  astMatcher.addMatcher(lambdaExpr().bind("lambda"),
-                        &refCountedInsideLambdaChecker);
+  // We want to reject any code which captures a pointer to an object of a
+  // refcounted type, and then lets that value escape. As a primitive analysis,
+  // we reject any occurances of the lambda as a template parameter to a class
+  // (which could allow it to escape), as well as any presence of such a lambda
+  // in a return value (either from lambdas, or in c++14, auto functions).
+  //
+  // We check these lambdas' capture lists for raw pointers to refcounted types.
+  astMatcher.addMatcher(
+      functionDecl(returns(recordType(hasDeclaration(cxxRecordDecl(isLambdaDecl()).bind("decl"))))),
+      &refCountedInsideLambdaChecker);
+  astMatcher.addMatcher(lambdaExpr().bind("lambdaExpr"), &refCountedInsideLambdaChecker);
+  astMatcher.addMatcher(
+      classTemplateSpecializationDecl(hasAnyTemplateArgument(refersToType(
+        recordType(hasDeclaration(cxxRecordDecl(isLambdaDecl()).bind("decl")))))),
+      &refCountedInsideLambdaChecker);
 
   // Older clang versions such as the ones used on the infra recognize these
   // conversions as 'operator _Bool', but newer clang versions recognize these
@@ -1358,13 +1372,34 @@ void DiagnosticsMatcher::NoAddRefReleaseOnReturnChecker::run(
 
 void DiagnosticsMatcher::RefCountedInsideLambdaChecker::run(
     const MatchFinder::MatchResult &Result) {
+  static DenseSet<const CXXRecordDecl*> CheckedDecls;
+
   DiagnosticsEngine &Diag = Result.Context->getDiagnostics();
   unsigned errorID = Diag.getDiagnosticIDs()->getCustomDiagID(
       DiagnosticIDs::Error,
       "Refcounted variable %0 of type %1 cannot be captured by a lambda");
   unsigned noteID = Diag.getDiagnosticIDs()->getCustomDiagID(
       DiagnosticIDs::Note, "Please consider using a smart pointer");
-  const LambdaExpr *Lambda = Result.Nodes.getNodeAs<LambdaExpr>("lambda");
+
+  const CXXRecordDecl *Lambda = Result.Nodes.getNodeAs<CXXRecordDecl>("decl");
+
+  if (const LambdaExpr *OuterLambda = Result.Nodes.getNodeAs<LambdaExpr>("lambdaExpr")) {
+    const CXXMethodDecl *OpCall = OuterLambda->getCallOperator();
+    QualType ReturnTy = OpCall->getReturnType();
+    if (const CXXRecordDecl *Record = ReturnTy->getAsCXXRecordDecl()) {
+      Lambda = Record;
+    }
+  }
+
+  if (!Lambda || !Lambda->isLambda()) {
+    return;
+  }
+
+  // Don't report errors on the same declarations more than once.
+  if (CheckedDecls.count(Lambda)) {
+    return;
+  }
+  CheckedDecls.insert(Lambda);
 
   for (const LambdaCapture Capture : Lambda->captures()) {
     if (Capture.capturesVariable() && Capture.getCaptureKind() != LCK_ByRef) {
@@ -1374,6 +1409,7 @@ void DiagnosticsMatcher::RefCountedInsideLambdaChecker::run(
         Diag.Report(Capture.getLocation(), errorID) << Capture.getCapturedVar()
                                                     << Pointee;
         Diag.Report(Capture.getLocation(), noteID);
+        return;
       }
     }
   }

build/clang-plugin/tests/TestNoRefcountedInsideLambdas.cpp

@@ -1,3 +1,5 @@
+#include <functional>
+#include "mozilla/Function.h"
 #define MOZ_STRONG_REF __attribute__((annotate("moz_strong_ref")))
 
 struct RefCountedBase {
@@ -45,7 +47,7 @@ void foo() {
   });
   take([=](R* argptr) {
     R* localptr;
-    ptr->method(); // expected-error{{Refcounted variable 'ptr' of type 'R' cannot be captured by a lambda}} expected-note{{Please consider using a smart pointer}}
+    ptr->method();
     argptr->method();
     localptr->method();
   });
@@ -57,7 +59,7 @@ void foo() {
   });
   take([=](R* argptr) {
     R* localptr;
-    take(ptr); // expected-error{{Refcounted variable 'ptr' of type 'R' cannot be captured by a lambda}} expected-note{{Please consider using a smart pointer}}
+    take(ptr);
     take(argptr);
     take(localptr);
   });
@@ -67,7 +69,7 @@ void foo() {
     take(argsp);
     take(localsp);
   });
-  take([ptr](R* argptr) { // expected-error{{Refcounted variable 'ptr' of type 'R' cannot be captured by a lambda}} expected-note{{Please consider using a smart pointer}}
+  take([ptr](R* argptr) {
     R* localptr;
     ptr->method();
     argptr->method();
@@ -79,7 +81,7 @@ void foo() {
     argsp->method();
     localsp->method();
   });
-  take([ptr](R* argptr) { // expected-error{{Refcounted variable 'ptr' of type 'R' cannot be captured by a lambda}} expected-note{{Please consider using a smart pointer}}
+  take([ptr](R* argptr) {
     R* localptr;
     take(ptr);
     take(argptr);
@@ -92,27 +94,558 @@ void foo() {
     take(localsp);
   });
   take([&ptr](R* argptr) {
+    R* localptr;
+    ptr->method();
+    argptr->method();
+    localptr->method();
+  });
+  take([&sp](SmartPtr<R> argsp) {
+    SmartPtr<R> localsp;
+    sp->method();
+    argsp->method();
+    localsp->method();
+  });
+  take([&ptr](R* argptr) {
+    R* localptr;
+    take(ptr);
+    take(argptr);
+    take(localptr);
+  });
+  take([&sp](SmartPtr<R> argsp) {
+    SmartPtr<R> localsp;
+    take(sp);
+    take(argsp);
+    take(localsp);
+  });
+}
+
+void b() {
+  R* ptr;
+  SmartPtr<R> sp;
+  std::function<void(R*)>([&](R* argptr) {
+    R* localptr;
+    ptr->method();
+    argptr->method();
+    localptr->method();
+  });
+  std::function<void(SmartPtr<R>)>([&](SmartPtr<R> argsp) {
+    SmartPtr<R> localsp;
+    sp->method();
+    argsp->method();
+    localsp->method();
+  });
+  std::function<void(R*)>([&](R* argptr) {
+    R* localptr;
+    take(ptr);
+    take(argptr);
+    take(localptr);
+  });
+  std::function<void(SmartPtr<R>)>([&](SmartPtr<R> argsp) {
+    SmartPtr<R> localsp;
+    take(sp);
+    take(argsp);
+    take(localsp);
+  });
+  std::function<void(R*)>([=](R* argptr) {
+    R* localptr;
+    ptr->method(); // expected-error{{Refcounted variable 'ptr' of type 'R' cannot be captured by a lambda}} expected-note{{Please consider using a smart pointer}}
+    argptr->method();
+    localptr->method();
+  });
+  std::function<void(SmartPtr<R>)>([=](SmartPtr<R> argsp) {
+    SmartPtr<R> localsp;
+    sp->method();
+    argsp->method();
+    localsp->method();
+  });
+  std::function<void(R*)>([=](R* argptr) {
+    R* localptr;
+    take(ptr); // expected-error{{Refcounted variable 'ptr' of type 'R' cannot be captured by a lambda}} expected-note{{Please consider using a smart pointer}}
+    take(argptr);
+    take(localptr);
+  });
+  std::function<void(SmartPtr<R>)>([=](SmartPtr<R> argsp) {
+    SmartPtr<R> localsp;
+    take(sp);
+    take(argsp);
+    take(localsp);
+  });
+  std::function<void(R*)>([ptr](R* argptr) { // expected-error{{Refcounted variable 'ptr' of type 'R' cannot be captured by a lambda}} expected-note{{Please consider using a smart pointer}}
+    R* localptr;
+    ptr->method();
+    argptr->method();
+    localptr->method();
+  });
+  std::function<void(SmartPtr<R>)>([sp](SmartPtr<R> argsp) {
+    SmartPtr<R> localsp;
+    sp->method();
+    argsp->method();
+    localsp->method();
+  });
+  std::function<void(R*)>([ptr](R* argptr) { // expected-error{{Refcounted variable 'ptr' of type 'R' cannot be captured by a lambda}} expected-note{{Please consider using a smart pointer}}
+    R* localptr;
+    take(ptr);
+    take(argptr);
+    take(localptr);
+  });
+  std::function<void(SmartPtr<R>)>([sp](SmartPtr<R> argsp) {
+    SmartPtr<R> localsp;
+    take(sp);
+    take(argsp);
+    take(localsp);
+  });
+  std::function<void(R*)>([&ptr](R* argptr) {
+    R* localptr;
+    ptr->method();
+    argptr->method();
+    localptr->method();
+  });
+  std::function<void(SmartPtr<R>)>([&sp](SmartPtr<R> argsp) {
+    SmartPtr<R> localsp;
+    sp->method();
+    argsp->method();
+    localsp->method();
+  });
+  std::function<void(R*)>([&ptr](R* argptr) {
+    R* localptr;
+    take(ptr);
+    take(argptr);
+    take(localptr);
+  });
+  std::function<void(SmartPtr<R>)>([&sp](SmartPtr<R> argsp) {
+    SmartPtr<R> localsp;
+    take(sp);
+    take(argsp);
+    take(localsp);
+  });
+}
+
+void c() {
+  R* ptr;
+  SmartPtr<R> sp;
+  mozilla::function<void(R*)>([&](R* argptr) {
+    R* localptr;
+    ptr->method();
+    argptr->method();
+    localptr->method();
+  });
+  mozilla::function<void(SmartPtr<R>)>([&](SmartPtr<R> argsp) {
+    SmartPtr<R> localsp;
+    sp->method();
+    argsp->method();
+    localsp->method();
+  });
+  mozilla::function<void(R*)>([&](R* argptr) {
+    R* localptr;
+    take(ptr);
+    take(argptr);
+    take(localptr);
+  });
+  mozilla::function<void(SmartPtr<R>)>([&](SmartPtr<R> argsp) {
+    SmartPtr<R> localsp;
+    take(sp);
+    take(argsp);
+    take(localsp);
+  });
+  mozilla::function<void(R*)>([=](R* argptr) {
+    R* localptr;
+    ptr->method(); // expected-error{{Refcounted variable 'ptr' of type 'R' cannot be captured by a lambda}} expected-note{{Please consider using a smart pointer}}
+    argptr->method();
+    localptr->method();
+  });
+  mozilla::function<void(SmartPtr<R>)>([=](SmartPtr<R> argsp) {
+    SmartPtr<R> localsp;
+    sp->method();
+    argsp->method();
+    localsp->method();
+  });
+  mozilla::function<void(R*)>([=](R* argptr) {
+    R* localptr;
+    take(ptr); // expected-error{{Refcounted variable 'ptr' of type 'R' cannot be captured by a lambda}} expected-note{{Please consider using a smart pointer}}
+    take(argptr);
+    take(localptr);
+  });
+  mozilla::function<void(SmartPtr<R>)>([=](SmartPtr<R> argsp) {
+    SmartPtr<R> localsp;
+    take(sp);
+    take(argsp);
+    take(localsp);
+  });
+  mozilla::function<void(R*)>([ptr](R* argptr) { // expected-error{{Refcounted variable 'ptr' of type 'R' cannot be captured by a lambda}} expected-note{{Please consider using a smart pointer}}
+    R* localptr;
+    ptr->method();
+    argptr->method();
+    localptr->method();
+  });
+  mozilla::function<void(SmartPtr<R>)>([sp](SmartPtr<R> argsp) {
+    SmartPtr<R> localsp;
+    sp->method();
+    argsp->method();
+    localsp->method();
+  });
+  mozilla::function<void(R*)>([ptr](R* argptr) { // expected-error{{Refcounted variable 'ptr' of type 'R' cannot be captured by a lambda}} expected-note{{Please consider using a smart pointer}}
+    R* localptr;
+    take(ptr);
+    take(argptr);
+    take(localptr);
+  });
+  mozilla::function<void(SmartPtr<R>)>([sp](SmartPtr<R> argsp) {
+    SmartPtr<R> localsp;
+    take(sp);
+    take(argsp);
+    take(localsp);
+  });
+  mozilla::function<void(R*)>([&ptr](R* argptr) {
+    R* localptr;
+    ptr->method();
+    argptr->method();
+    localptr->method();
+  });
+  mozilla::function<void(SmartPtr<R>)>([&sp](SmartPtr<R> argsp) {
+    SmartPtr<R> localsp;
+    sp->method();
+    argsp->method();
+    localsp->method();
+  });
+  mozilla::function<void(R*)>([&ptr](R* argptr) {
+    R* localptr;
+    take(ptr);
+    take(argptr);
+    take(localptr);
+  });
+  mozilla::function<void(SmartPtr<R>)>([&sp](SmartPtr<R> argsp) {
+    SmartPtr<R> localsp;
+    take(sp);
+    take(argsp);
+    take(localsp);
+  });
+}
+
+// These tests would check c++14 deduced return types, if they were supported in
+// our codebase. They are being kept here for convenience in the future if we do
+// add support for c++14 deduced return types
+#if 0
+auto d1() {
+  R* ptr;
+  SmartPtr<R> sp;
+  return ([&](R* argptr) {
+    R* localptr;
+    ptr->method();
+    argptr->method();
+    localptr->method();
+  });
+}
+auto d2() {
+  R* ptr;
+  SmartPtr<R> sp;
+  return ([&](SmartPtr<R> argsp) {
+    SmartPtr<R> localsp;
+    sp->method();
+    argsp->method();
+    localsp->method();
+  });
+}
+auto d3() {
+  R* ptr;
+  SmartPtr<R> sp;
+  return ([&](R* argptr) {
+    R* localptr;
+    take(ptr);
+    take(argptr);
+    take(localptr);
+  });
+}
+auto d4() {
+  R* ptr;
+  SmartPtr<R> sp;
+  return ([&](SmartPtr<R> argsp) {
+    SmartPtr<R> localsp;
+    take(sp);
+    take(argsp);
+    take(localsp);
+  });
+}
+auto d5() {
+  R* ptr;
+  SmartPtr<R> sp;
+  return ([=](R* argptr) {
+    R* localptr;
+    ptr->method(); // expected-error{{Refcounted variable 'ptr' of type 'R' cannot be captured by a lambda}} expected-note{{Please consider using a smart pointer}}
+    argptr->method();
+    localptr->method();
+  });
+}
+auto d6() {
+  R* ptr;
+  SmartPtr<R> sp;
+  return ([=](SmartPtr<R> argsp) {
+    SmartPtr<R> localsp;
+    sp->method();
+    argsp->method();
+    localsp->method();
+  });
+}
+auto d8() {
+  R* ptr;
+  SmartPtr<R> sp;
+  return ([=](R* argptr) {
+    R* localptr;
+    take(ptr); // expected-error{{Refcounted variable 'ptr' of type 'R' cannot be captured by a lambda}} expected-note{{Please consider using a smart pointer}}
+    take(argptr);
+    take(localptr);
+  });
+}
+auto d9() {
+  R* ptr;
+  SmartPtr<R> sp;
+  return ([=](SmartPtr<R> argsp) {
+    SmartPtr<R> localsp;
+    take(sp);
+    take(argsp);
+    take(localsp);
+  });
+}
+auto d10() {
+  R* ptr;
+  SmartPtr<R> sp;
+  return ([ptr](R* argptr) { // expected-error{{Refcounted variable 'ptr' of type 'R' cannot be captured by a lambda}} expected-note{{Please consider using a smart pointer}}
+    R* localptr;
+    ptr->method();
+    argptr->method();
+    localptr->method();
+  });
+}
+auto d11() {
+  R* ptr;
+  SmartPtr<R> sp;
+  return ([sp](SmartPtr<R> argsp) {
+    SmartPtr<R> localsp;
+    sp->method();
+    argsp->method();
+    localsp->method();
+  });
+}
+auto d12() {
+  R* ptr;
+  SmartPtr<R> sp;
+  return ([ptr](R* argptr) { // expected-error{{Refcounted variable 'ptr' of type 'R' cannot be captured by a lambda}} expected-note{{Please consider using a smart pointer}}
+    R* localptr;
+    take(ptr);
+    take(argptr);
+    take(localptr);
+  });
+}
+auto d13() {
+  R* ptr;
+  SmartPtr<R> sp;
+  return ([sp](SmartPtr<R> argsp) {
+    SmartPtr<R> localsp;
+    take(sp);
+    take(argsp);
+    take(localsp);
+  });
+}
+auto d14() {
+  R* ptr;
+  SmartPtr<R> sp;
+  return ([&ptr](R* argptr) {
+    R* localptr;
+    ptr->method();
+    argptr->method();
+    localptr->method();
+  });
+}
+auto d15() {
+  R* ptr;
+  SmartPtr<R> sp;
+  return ([&sp](SmartPtr<R> argsp) {
+    SmartPtr<R> localsp;
+    sp->method();
+    argsp->method();
+    localsp->method();
+  });
+}
+auto d16() {
+  R* ptr;
+  SmartPtr<R> sp;
+  return ([&ptr](R* argptr) {
+    R* localptr;
+    take(ptr);
+    take(argptr);
+    take(localptr);
+  });
+}
+auto d17() {
+  R* ptr;
+  SmartPtr<R> sp;
+  return ([&sp](SmartPtr<R> argsp) {
+    SmartPtr<R> localsp;
+    take(sp);
+    take(argsp);
+    take(localsp);
+  });
+}
+#endif
+
+void e() {
+  auto e1 = []() {
+    R* ptr;
+    SmartPtr<R> sp;
+    return ([&](R* argptr) {
       R* localptr;
       ptr->method();
       argptr->method();
       localptr->method();
     });
-  take([&sp](SmartPtr<R> argsp) {
+  };
+  auto e2 = []() {
+    R* ptr;
+    SmartPtr<R> sp;
+    return ([&](SmartPtr<R> argsp) {
       SmartPtr<R> localsp;
       sp->method();
       argsp->method();
       localsp->method();
     });
-  take([&ptr](R* argptr) {
+  };
+  auto e3 = []() {
+    R* ptr;
+    SmartPtr<R> sp;
+    return ([&](R* argptr) {
       R* localptr;
       take(ptr);
       take(argptr);
       take(localptr);
     });
-  take([&sp](SmartPtr<R> argsp) {
+  };
+  auto e4 = []() {
+    R* ptr;
+    SmartPtr<R> sp;
+    return ([&](SmartPtr<R> argsp) {
       SmartPtr<R> localsp;
       take(sp);
       take(argsp);
       take(localsp);
     });
+  };
+  auto e5 = []() {
+    R* ptr;
+    SmartPtr<R> sp;
+    return ([=](R* argptr) {
+      R* localptr;
+      ptr->method(); // expected-error{{Refcounted variable 'ptr' of type 'R' cannot be captured by a lambda}} expected-note{{Please consider using a smart pointer}}
+      argptr->method();
+      localptr->method();
+    });
+  };
+  auto e6 = []() {
+    R* ptr;
+    SmartPtr<R> sp;
+    return ([=](SmartPtr<R> argsp) {
+      SmartPtr<R> localsp;
+      sp->method();
+      argsp->method();
+      localsp->method();
+    });
+  };
+  auto e8 = []() {
+    R* ptr;
+    SmartPtr<R> sp;
+    return ([=](R* argptr) {
+      R* localptr;
+      take(ptr); // expected-error{{Refcounted variable 'ptr' of type 'R' cannot be captured by a lambda}} expected-note{{Please consider using a smart pointer}}
+      take(argptr);
+      take(localptr);
+    });
+  };
+  auto e9 = []() {
+    R* ptr;
+    SmartPtr<R> sp;
+    return ([=](SmartPtr<R> argsp) {
+      SmartPtr<R> localsp;
+      take(sp);
+      take(argsp);
+      take(localsp);
+    });
+  };
+  auto e10 = []() {
+    R* ptr;
+    SmartPtr<R> sp;
+    return ([ptr](R* argptr) { // expected-error{{Refcounted variable 'ptr' of type 'R' cannot be captured by a lambda}} expected-note{{Please consider using a smart pointer}}
+      R* localptr;
+      ptr->method();
+      argptr->method();
+      localptr->method();
+    });
+  };
+  auto e11 = []() {
+    R* ptr;
+    SmartPtr<R> sp;
+    return ([sp](SmartPtr<R> argsp) {
+      SmartPtr<R> localsp;
+      sp->method();
+      argsp->method();
+      localsp->method();
+    });
+  };
+  auto e12 = []() {
+    R* ptr;
+    SmartPtr<R> sp;
+    return ([ptr](R* argptr) { // expected-error{{Refcounted variable 'ptr' of type 'R' cannot be captured by a lambda}} expected-note{{Please consider using a smart pointer}}
+      R* localptr;
+      take(ptr);
+      take(argptr);
+      take(localptr);
+    });
+  };
+  auto e13 = []() {
+    R* ptr;
+    SmartPtr<R> sp;
+    return ([sp](SmartPtr<R> argsp) {
+      SmartPtr<R> localsp;
+      take(sp);
+      take(argsp);
+      take(localsp);
+    });
+  };
+  auto e14 = []() {
+    R* ptr;
+    SmartPtr<R> sp;
+    return ([&ptr](R* argptr) {
+      R* localptr;
+      ptr->method();
+      argptr->method();
+      localptr->method();
+    });
+  };
+  auto e15 = []() {
+    R* ptr;
+    SmartPtr<R> sp;
+    return ([&sp](SmartPtr<R> argsp) {
+      SmartPtr<R> localsp;
+      sp->method();
+      argsp->method();
+      localsp->method();
+    });
+  };
+  auto e16 = []() {
+    R* ptr;
+    SmartPtr<R> sp;
+    return ([&ptr](R* argptr) {
+      R* localptr;
+      take(ptr);
+      take(argptr);
+      take(localptr);
+    });
+  };
+  auto e17 = []() {
+    R* ptr;
+    SmartPtr<R> sp;
+    return ([&sp](SmartPtr<R> argsp) {
+      SmartPtr<R> localsp;
+      take(sp);
+      take(argsp);
+      take(localsp);
+    });
+  };
 }