Index: include/clang/StaticAnalyzer/Checkers/Checkers.td =================================================================== --- include/clang/StaticAnalyzer/Checkers/Checkers.td +++ include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -610,12 +610,12 @@ } // 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 { Index: lib/StaticAnalyzer/Checkers/CMakeLists.txt =================================================================== --- lib/StaticAnalyzer/Checkers/CMakeLists.txt +++ lib/StaticAnalyzer/Checkers/CMakeLists.txt @@ -37,7 +37,7 @@ DynamicTypeChecker.cpp ExprInspectionChecker.cpp FixedAddressChecker.cpp - GCDAsyncSemaphoreChecker.cpp + GCDAntipatternChecker.cpp GenericTaintChecker.cpp GTestChecker.cpp IdenticalExprChecker.cpp Index: lib/StaticAnalyzer/Checkers/GCDAntipatternChecker.cpp =================================================================== --- lib/StaticAnalyzer/Checkers/GCDAntipatternChecker.cpp +++ 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 @@ const char *WarningBinding = "semaphore_wait"; -class GCDAsyncSemaphoreChecker : public Checker { +class GCDAntipatternChecker : public Checker { public: void checkASTCodeBody(const Decl *D, AnalysisManager &AM, @@ -56,13 +54,13 @@ 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 @@ 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 @@ if (StringRef(DeclName).startswith("test")) return; } + if (const auto *OD = dyn_cast(D)) { + if (const auto *CD = dyn_cast(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,14 @@ 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", PathDiagnosticLocation::createBegin(SW, BR.getSourceManager(), ADC), SW->getSourceRange()); } } -void ento::registerGCDAsyncSemaphoreChecker(CheckerManager &Mgr) { - Mgr.registerChecker(); +void ento::registerGCDAntipattern(CheckerManager &Mgr) { + Mgr.registerChecker(); } Index: test/Analysis/gcdantipatternchecker_test.m =================================================================== --- test/Analysis/gcdantipatternchecker_test.m +++ 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 {} @@ -27,7 +27,7 @@ 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 @@ 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 @@ 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 @@ // 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ [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