[analyzer] Move the GCDAsyncSemaphoreChecker to optin.performance

rdar://38383753

Differential Revision: https://reviews.llvm.org/D44228

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@327309 91177308-0d34-0410-b5e6-96231b3b80d8

4 files changed, 82 insertions, 43 deletions

diff --git a/include/clang/StaticAnalyzer/Checkers/Checkers.td b/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 3b94a9b161..0fb1b08e1b 100644
--- a/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -610,12 +610,12 @@ def ObjCSuperDeallocChecker : Checker<"SuperDealloc">,
 
 } // end "osx.cocoa"
 
-let ParentPackage = OSXAlpha in {
+let ParentPackage = Performance in {
 
-def GCDAsyncSemaphoreChecker : Checker<"GCDAsyncSemaphore">,
-  HelpText<"Checker for performance anti-pattern when using semaphors from async API">,
-  DescFile<"GCDAsyncSemaphoreChecker.cpp">;
-} // end "alpha.osx"
+def GCDAntipattern : Checker<"GCDAntipattern">,
+  HelpText<"Check for performance anti-patterns when using Grand Central Dispatch">,
+  DescFile<"GCDAntipatternChecker.cpp">;
+} // end "optin.performance"
 
 let ParentPackage = CocoaAlpha in {
 
diff --git a/lib/StaticAnalyzer/Checkers/CMakeLists.txt b/lib/StaticAnalyzer/Checkers/CMakeLists.txt
index 9b42a5581e..baf36e03d6 100644
--- a/lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ b/lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -37,7 +37,7 @@ add_clang_library(clangStaticAnalyzerCheckers
   DynamicTypeChecker.cpp
   ExprInspectionChecker.cpp
   FixedAddressChecker.cpp
-  GCDAsyncSemaphoreChecker.cpp
+  GCDAntipatternChecker.cpp
   GenericTaintChecker.cpp
   GTestChecker.cpp
   IdenticalExprChecker.cpp
diff --git a/lib/StaticAnalyzer/Checkers/GCDAsyncSemaphoreChecker.cpp b/lib/StaticAnalyzer/Checkers/GCDAntipatternChecker.cpp
index eda7a5fcd1..51a3fc1938 100644
--- a/lib/StaticAnalyzer/Checkers/GCDAsyncSemaphoreChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/GCDAntipatternChecker.cpp
@@ -1,4 +1,4 @@
-//===- GCDAsyncSemaphoreChecker.cpp -----------------------------*- C++ -*-==//
+//===- GCDAntipatternChecker.cpp ---------------------------------*- C++ -*-==//
 //
 //                     The LLVM Compiler Infrastructure
 //
@@ -7,20 +7,20 @@
 //
 //===----------------------------------------------------------------------===//
 //
-// This file defines GCDAsyncSemaphoreChecker which checks against a common
+// This file defines GCDAntipatternChecker which checks against a common
 // antipattern when synchronous API is emulated from asynchronous callbacks
-// using a semaphor:
+// using a semaphore:
+//
+//   dispatch_semaphore_t sema = dispatch_semaphore_create(0);
 //
-//   dispatch_semapshore_t sema = dispatch_semaphore_create(0);
-
 //   AnyCFunctionCall(^{
 //     // code…
-//     dispatch_semapshore_signal(sema);
+//     dispatch_semaphore_signal(sema);
 //   })
-//   dispatch_semapshore_wait(sema, *)
+//   dispatch_semaphore_wait(sema, *)
 //
 // Such code is a common performance problem, due to inability of GCD to
-// properly handle QoS when a combination of queues and semaphors is used.
+// properly handle QoS when a combination of queues and semaphores is used.
 // Good code would either use asynchronous API (when available), or perform
 // the necessary action in asynchronous callback.
 //
@@ -37,8 +37,6 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
 #include "llvm/Support/Debug.h"
 
-#define DEBUG_TYPE "gcdasyncsemaphorechecker"
-
 using namespace clang;
 using namespace ento;
 using namespace ast_matchers;
@@ -47,7 +45,7 @@ namespace {
 
 const char *WarningBinding = "semaphore_wait";
 
-class GCDAsyncSemaphoreChecker : public Checker<check::ASTCodeBody> {
+class GCDAntipatternChecker : public Checker<check::ASTCodeBody> {
 public:
   void checkASTCodeBody(const Decl *D,
                         AnalysisManager &AM,
@@ -56,13 +54,13 @@ public:
 
 class Callback : public MatchFinder::MatchCallback {
   BugReporter &BR;
-  const GCDAsyncSemaphoreChecker *C;
+  const GCDAntipatternChecker *C;
   AnalysisDeclContext *ADC;
 
 public:
   Callback(BugReporter &BR,
            AnalysisDeclContext *ADC,
-           const GCDAsyncSemaphoreChecker *C) : BR(BR), C(C), ADC(ADC) {}
+           const GCDAntipatternChecker *C) : BR(BR), C(C), ADC(ADC) {}
 
   virtual void run(const MatchFinder::MatchResult &Result) override;
 };
@@ -83,7 +81,7 @@ auto bindAssignmentToDecl(const char *DeclName) -> decltype(hasLHS(expr())) {
                          declRefExpr(to(varDecl().bind(DeclName)))));
 }
 
-void GCDAsyncSemaphoreChecker::checkASTCodeBody(const Decl *D,
+void GCDAntipatternChecker::checkASTCodeBody(const Decl *D,
                                                AnalysisManager &AM,
                                                BugReporter &BR) const {
 
@@ -93,6 +91,14 @@ void GCDAsyncSemaphoreChecker::checkASTCodeBody(const Decl *D,
     if (StringRef(DeclName).startswith("test"))
       return;
   }
+  if (const auto *OD = dyn_cast<ObjCMethodDecl>(D)) {
+    if (const auto *CD = dyn_cast<ObjCContainerDecl>(OD->getParent())) {
+      std::string ContainerName = CD->getNameAsString();
+      StringRef CN(ContainerName);
+      if (CN.contains_lower("test") || CN.contains_lower("mock"))
+        return;
+    }
+  }
 
   const char *SemaphoreBinding = "semaphore_name";
   auto SemaphoreCreateM = callExpr(callsName("dispatch_semaphore_create"));
@@ -146,14 +152,15 @@ void Callback::run(const MatchFinder::MatchResult &Result) {
       ADC->getDecl(), C,
       /*Name=*/"Semaphore performance anti-pattern",
       /*Category=*/"Performance",
-      "Possible semaphore performance anti-pattern: wait on a semaphore "
-      "signalled to in a callback",
+      "Waiting on a semaphore with Grand Central Dispatch creates useless "
+      "threads and is subject to priority inversion; consider "
+      "using a synchronous API or changing the caller to be asynchronous",
       PathDiagnosticLocation::createBegin(SW, BR.getSourceManager(), ADC),
       SW->getSourceRange());
 }
 
 }
 
-void ento::registerGCDAsyncSemaphoreChecker(CheckerManager &Mgr) {
-  Mgr.registerChecker<GCDAsyncSemaphoreChecker>();
+void ento::registerGCDAntipattern(CheckerManager &Mgr) {
+  Mgr.registerChecker<GCDAntipatternChecker>();
 }
diff --git a/test/Analysis/gcdasyncsemaphorechecker_test.m b/test/Analysis/gcdantipatternchecker_test.m
index ff434d3652..19f446787c 100644
--- a/test/Analysis/gcdasyncsemaphorechecker_test.m
+++ b/test/Analysis/gcdantipatternchecker_test.m
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.osx.GCDAsyncSemaphore %s -fblocks -verify
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,optin.performance.GCDAntipattern %s -fblocks -verify
 typedef signed char BOOL;
 @protocol NSObject  - (BOOL)isEqual:(id)object; @end
 @interface NSObject <NSObject> {}
@@ -27,7 +27,7 @@ void use_semaphor_antipattern() {
   func(^{
       dispatch_semaphore_signal(sema);
   });
-  dispatch_semaphore_wait(sema, 100); // expected-warning{{Possible semaphore performance anti-pattern}}
+  dispatch_semaphore_wait(sema, 100); // expected-warning{{Waiting on a semaphore with Grand Central Dispatch creates useless threads and is subject to priority inversion}}
 }
 
 // It's OK to use pattern in tests.
@@ -47,14 +47,14 @@ void use_semaphor_antipattern_multiple_times() {
   func(^{
       dispatch_semaphore_signal(sema1);
   });
-  dispatch_semaphore_wait(sema1, 100); // expected-warning{{Possible semaphore performance anti-pattern}}
+  dispatch_semaphore_wait(sema1, 100); // expected-warning{{Waiting on a semaphore with Grand Central Dispatch creates useless threads and is subject to priority inversion}}
 
   dispatch_semaphore_t sema2 = dispatch_semaphore_create(0);
 
   func(^{
       dispatch_semaphore_signal(sema2);
   });
-  dispatch_semaphore_wait(sema2, 100); // expected-warning{{Possible semaphore performance anti-pattern}}
+  dispatch_semaphore_wait(sema2, 100); // expected-warning{{Waiting on a semaphore with Grand Central Dispatch creates useless threads and is subject to priority inversion}}
 }
 
 void use_semaphor_antipattern_multiple_wait() {
@@ -64,8 +64,8 @@ void use_semaphor_antipattern_multiple_wait() {
       dispatch_semaphore_signal(sema1);
   });
   // FIXME: multiple waits on same semaphor should not raise a warning.
-  dispatch_semaphore_wait(sema1, 100); // expected-warning{{Possible semaphore performance anti-pattern}}
-  dispatch_semaphore_wait(sema1, 100); // expected-warning{{Possible semaphore performance anti-pattern}}
+  dispatch_semaphore_wait(sema1, 100); // expected-warning{{Waiting on a semaphore with Grand Central Dispatch creates useless threads and is subject to priority inversion}}
+  dispatch_semaphore_wait(sema1, 100); // expected-warning{{Waiting on a semaphore with Grand Central Dispatch creates useless threads and is subject to priority inversion}}
 }
 
 void warn_incorrect_order() {
@@ -73,7 +73,7 @@ void warn_incorrect_order() {
   // if out of order.
   dispatch_semaphore_t sema = dispatch_semaphore_create(0);
 
-  dispatch_semaphore_wait(sema, 100); // expected-warning{{Possible semaphore performance anti-pattern}}
+  dispatch_semaphore_wait(sema, 100); // expected-warning{{Waiting on a semaphore with Grand Central Dispatch creates useless threads and is subject to priority inversion}}
   func(^{
       dispatch_semaphore_signal(sema);
   });
@@ -85,7 +85,7 @@ void warn_w_typedef() {
   func_w_typedef(^{
       dispatch_semaphore_signal(sema);
   });
-  dispatch_semaphore_wait(sema, 100); // expected-warning{{Possible semaphore performance anti-pattern}}
+  dispatch_semaphore_wait(sema, 100); // expected-warning{{Waiting on a semaphore with Grand Central Dispatch creates useless threads and is subject to priority inversion}}
 }
 
 void warn_nested_ast() {
@@ -100,7 +100,7 @@ void warn_nested_ast() {
          dispatch_semaphore_signal(sema);
          });
   }
-  dispatch_semaphore_wait(sema, 100); // expected-warning{{Possible semaphore performance anti-pattern}}
+  dispatch_semaphore_wait(sema, 100); // expected-warning{{Waiting on a semaphore with Grand Central Dispatch creates useless threads and is subject to priority inversion}}
 }
 
 void use_semaphore_assignment() {
@@ -110,7 +110,7 @@ void use_semaphore_assignment() {
   func(^{
       dispatch_semaphore_signal(sema);
   });
-  dispatch_semaphore_wait(sema, 100); // expected-warning{{Possible semaphore performance anti-pattern}}
+  dispatch_semaphore_wait(sema, 100); // expected-warning{{Waiting on a semaphore with Grand Central Dispatch creates useless threads and is subject to priority inversion}}
 }
 
 void use_semaphore_assignment_init() {
@@ -120,7 +120,7 @@ void use_semaphore_assignment_init() {
   func(^{
       dispatch_semaphore_signal(sema);
   });
-  dispatch_semaphore_wait(sema, 100); // expected-warning{{Possible semaphore performance anti-pattern}}
+  dispatch_semaphore_wait(sema, 100); // expected-warning{{Waiting on a semaphore with Grand Central Dispatch creates useless threads and is subject to priority inversion}}
 }
 
 void differentsemaphoreok() {
@@ -172,17 +172,17 @@ void warn_with_cast() {
   func(^{
       dispatch_semaphore_signal((int)sema);
   });
-  dispatch_semaphore_wait((int)sema, 100); // expected-warning{{Possible semaphore performance anti-pattern}}
+  dispatch_semaphore_wait((int)sema, 100); // expected-warning{{Waiting on a semaphore with Grand Central Dispatch creates useless threads and is subject to priority inversion}}
 }
 
-@interface Test1 : NSObject
+@interface MyInterface1 : NSObject
 -(void)use_method_warn;
 -(void)use_objc_callback_warn;
 -(void)testNoWarn;
 -(void)acceptBlock:(block_t)callback;
 @end
 
-@implementation Test1
+@implementation MyInterface1
 
 -(void)use_method_warn {
   dispatch_semaphore_t sema = dispatch_semaphore_create(0);
@@ -190,7 +190,7 @@ void warn_with_cast() {
   func(^{
       dispatch_semaphore_signal(sema);
   });
-  dispatch_semaphore_wait(sema, 100); // expected-warning{{Possible semaphore performance anti-pattern}}
+  dispatch_semaphore_wait(sema, 100); // expected-warning{{Waiting on a semaphore with Grand Central Dispatch creates useless threads and is subject to priority inversion}}
 }
 
 -(void)testNoWarn {
@@ -212,23 +212,55 @@ void warn_with_cast() {
   [self acceptBlock:^{
       dispatch_semaphore_signal(sema);
   }];
-  dispatch_semaphore_wait(sema, 100); // expected-warning{{Possible semaphore performance anti-pattern}}
+  dispatch_semaphore_wait(sema, 100); // expected-warning{{Waiting on a semaphore with Grand Central Dispatch creates useless threads and is subject to priority inversion}}
 }
 
-void use_objc_and_c_callback(Test1 *t) {
+void use_objc_and_c_callback(MyInterface1 *t) {
   dispatch_semaphore_t sema = dispatch_semaphore_create(0);
 
   func(^{
       dispatch_semaphore_signal(sema);
   });
-  dispatch_semaphore_wait(sema, 100); // expected-warning{{Possible semaphore performance anti-pattern}}
+  dispatch_semaphore_wait(sema, 100); // expected-warning{{Waiting on a semaphore with Grand Central Dispatch creates useless threads and is subject to priority inversion}}
 
   dispatch_semaphore_t sema1 = dispatch_semaphore_create(0);
 
   [t acceptBlock:^{
       dispatch_semaphore_signal(sema1);
   }];
-  dispatch_semaphore_wait(sema1, 100); // expected-warning{{Possible semaphore performance anti-pattern}}
+  dispatch_semaphore_wait(sema1, 100); // expected-warning{{Waiting on a semaphore with Grand Central Dispatch creates useless threads and is subject to priority inversion}}
+}
+@end
+
+// No warnings: class name contains "test"
+@interface Test1 : NSObject
+-(void)use_method_warn;
+@end
+
+@implementation Test1
+-(void)use_method_warn {
+  dispatch_semaphore_t sema = dispatch_semaphore_create(0);
+
+  func(^{
+      dispatch_semaphore_signal(sema);
+  });
+  dispatch_semaphore_wait(sema, 100);
 }
+@end
+
 
+// No warnings: class name contains "mock"
+@interface Mock1 : NSObject
+-(void)use_method_warn;
+@end
+
+@implementation Mock1
+-(void)use_method_warn {
+  dispatch_semaphore_t sema = dispatch_semaphore_create(0);
+
+  func(^{
+      dispatch_semaphore_signal(sema);
+  });
+  dispatch_semaphore_wait(sema, 100);
+}
 @end