Index: include/clang/StaticAnalyzer/Checkers/Checkers.td =================================================================== --- include/clang/StaticAnalyzer/Checkers/Checkers.td +++ include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -610,6 +610,13 @@ } // end "osx.cocoa" +let ParentPackage = OSXAlpha in { + +def GCDAsyncSemaphoreChecker : Checker<"GCDAsyncSemaphore">, + HelpText<"Checker for performance anti-pattern when using semaphors from async API">, + DescFile<"GCDAsyncSemaphoreChecker.cpp">; +} // end "alpha.osx" + let ParentPackage = CocoaAlpha in { def InstanceVariableInvalidation : Checker<"InstanceVariableInvalidation">, Index: lib/StaticAnalyzer/Checkers/CMakeLists.txt =================================================================== --- lib/StaticAnalyzer/Checkers/CMakeLists.txt +++ lib/StaticAnalyzer/Checkers/CMakeLists.txt @@ -37,6 +37,7 @@ DynamicTypeChecker.cpp ExprInspectionChecker.cpp FixedAddressChecker.cpp + GCDAsyncSemaphoreChecker.cpp GenericTaintChecker.cpp GTestChecker.cpp IdenticalExprChecker.cpp Index: lib/StaticAnalyzer/Checkers/GCDAsyncSemaphoreChecker.cpp =================================================================== --- lib/StaticAnalyzer/Checkers/GCDAsyncSemaphoreChecker.cpp +++ lib/StaticAnalyzer/Checkers/GCDAsyncSemaphoreChecker.cpp @@ -0,0 +1,154 @@ +//===- GCDAsyncSemaphoreChecker.cpp -----------------------------*- C++ -*-==// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +// +// This file defines GCDAsyncSemaphoreChecker which checks against a common +// antipattern when synchronous API is emulated from asynchronous callbacks +// using a semaphor: +// +// dispatch_semapshore_t sema = dispatch_semaphore_create(0); + +// AnyCFunctionCall(^{ +// // code… +// dispatch_semapshore_signal(sema); +// }) +// dispatch_semapshore_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. +// Good code would either use asynchronous API (when available), or perform +// the necessary action in asynchronous callback. +// +// Currently, the check is performed using a simple heuristical AST pattern +// matching. +// +//===----------------------------------------------------------------------===// + +#include "ClangSACheckers.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h" +#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" +#include "clang/StaticAnalyzer/Core/Checker.h" +#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; + +namespace { + +const char *WarningBinding = "semaphore_wait"; + +class GCDAsyncSemaphoreChecker : public Checker { +public: + void checkASTCodeBody(const Decl *D, + AnalysisManager &AM, + BugReporter &BR) const; +}; + +class Callback : public MatchFinder::MatchCallback { + BugReporter &BR; + const GCDAsyncSemaphoreChecker *C; + AnalysisDeclContext *ADC; + +public: + Callback(BugReporter &BR, + AnalysisDeclContext *ADC, + const GCDAsyncSemaphoreChecker *C) : BR(BR), C(C), ADC(ADC) {} + + virtual void run(const MatchFinder::MatchResult &Result) override; +}; + +auto callsName(const char *FunctionName) + -> decltype(callee(functionDecl())) { + return callee(functionDecl(hasName(FunctionName))); +} + +auto equalsBoundArgDecl(int ArgIdx, const char *DeclName) + -> decltype(hasArgument(0, expr())) { + return hasArgument(ArgIdx, ignoringParenCasts(declRefExpr( + to(varDecl(equalsBoundNode(DeclName)))))); +} + +auto bindAssignmentToDecl(const char *DeclName) -> decltype(hasLHS(expr())) { + return hasLHS(ignoringParenImpCasts( + declRefExpr(to(varDecl().bind(DeclName))))); +} + +void GCDAsyncSemaphoreChecker::checkASTCodeBody(const Decl *D, + AnalysisManager &AM, + BugReporter &BR) const { + + // The pattern is very common in tests, and it is OK to use it there. + if (const auto* ND = dyn_cast(D)) + if (ND->getName().startswith("test")) + return; + + const char *SemaphoreBinding = "semaphore_name"; + auto SemaphoreCreateM = callExpr(callsName("dispatch_semaphore_create")); + + auto SemaphoreBindingM = anyOf( + forEachDescendant( + varDecl(hasDescendant(SemaphoreCreateM)).bind(SemaphoreBinding)), + forEachDescendant(binaryOperator(bindAssignmentToDecl(SemaphoreBinding), + hasRHS(SemaphoreCreateM)))); + + auto SemaphoreWaitM = forEachDescendant( + callExpr( + allOf( + callsName("dispatch_semaphore_wait"), + equalsBoundArgDecl(0, SemaphoreBinding) + ) + ).bind(WarningBinding)); + + auto AcceptsBlockM = + forEachDescendant(callExpr(hasAnyArgument(hasType( + hasCanonicalType(blockPointerType()) + )))); + + auto BlockSignallingM = + forEachDescendant(callExpr(hasAnyArgument(hasDescendant(callExpr( + allOf( + callsName("dispatch_semaphore_signal"), + equalsBoundArgDecl(0, SemaphoreBinding) + )))))); + + auto FinalM = compoundStmt( + SemaphoreBindingM, + SemaphoreWaitM, + AcceptsBlockM, + BlockSignallingM); + + MatchFinder F; + Callback CB(BR, AM.getAnalysisDeclContext(D), this); + + F.addMatcher(FinalM, &CB); + F.match(*D->getBody(), AM.getASTContext()); +} + +void Callback::run(const MatchFinder::MatchResult &Result) { + const auto *SW = Result.Nodes.getNodeAs(WarningBinding); + assert(SW); + BR.EmitBasicReport( + ADC->getDecl(), C, + /*Name=*/"Semaphore performance anti-pattern", + /*Category=*/"Performance", + "Possible semaphore performance anti-pattern: wait on a semaphore " + "signalled to in a callback", + PathDiagnosticLocation::createBegin(SW, BR.getSourceManager(), ADC), + SW->getSourceRange()); +} + +} + +void ento::registerGCDAsyncSemaphoreChecker(CheckerManager &Mgr) { + Mgr.registerChecker(); +} Index: test/gcdasyncsemaphorechecker_test.m =================================================================== --- test/gcdasyncsemaphorechecker_test.m +++ test/gcdasyncsemaphorechecker_test.m @@ -0,0 +1,169 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.osx.GCDAsyncSemaphore %s -fblocks -verify +// +typedef int dispatch_semaphore_t; +typedef void (^block_t)(); + +dispatch_semaphore_t dispatch_semaphore_create(int); +void dispatch_semaphore_wait(dispatch_semaphore_t, int); +void dispatch_semaphore_signal(dispatch_semaphore_t); + +void func(void (^)(void)); +void func_w_typedef(block_t); + +int coin(); + +void use_semaphor_antipattern() { + 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}} +} + +// It's OK to use pattern in tests. +// We simply match the containing function name against ^test. +void test_no_warning() { + dispatch_semaphore_t sema = dispatch_semaphore_create(0); + + func(^{ + dispatch_semaphore_signal(sema); + }); + dispatch_semaphore_wait(sema, 100); +} + +void use_semaphor_antipattern_multiple_times() { + dispatch_semaphore_t sema1 = dispatch_semaphore_create(0); + + func(^{ + dispatch_semaphore_signal(sema1); + }); + dispatch_semaphore_wait(sema1, 100); // expected-warning{{Possible semaphore performance anti-pattern}} + + 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}} +} + +void use_semaphor_antipattern_multiple_wait() { + dispatch_semaphore_t sema1 = dispatch_semaphore_create(0); + + func(^{ + 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}} +} + +void warn_incorrect_order() { + // FIXME: ASTMatchers do not allow ordered matching, so would match even + // if out of order. + dispatch_semaphore_t sema = dispatch_semaphore_create(0); + + dispatch_semaphore_wait(sema, 100); // expected-warning{{Possible semaphore performance anti-pattern}} + func(^{ + dispatch_semaphore_signal(sema); + }); +} + +void warn_w_typedef() { + dispatch_semaphore_t sema = dispatch_semaphore_create(0); + + func_w_typedef(^{ + dispatch_semaphore_signal(sema); + }); + dispatch_semaphore_wait(sema, 100); // expected-warning{{Possible semaphore performance anti-pattern}} +} + +void warn_nested_ast() { + dispatch_semaphore_t sema = dispatch_semaphore_create(0); + + if (coin()) { + func(^{ + dispatch_semaphore_signal(sema); + }); + } else { + func(^{ + dispatch_semaphore_signal(sema); + }); + } + dispatch_semaphore_wait(sema, 100); // expected-warning{{Possible semaphore performance anti-pattern}} +} + +void use_semaphore_assignment() { + dispatch_semaphore_t sema; + sema = dispatch_semaphore_create(0); + + func(^{ + dispatch_semaphore_signal(sema); + }); + dispatch_semaphore_wait(sema, 100); // expected-warning{{Possible semaphore performance anti-pattern}} +} + +void use_semaphore_assignment_init() { + dispatch_semaphore_t sema = dispatch_semaphore_create(0); + sema = dispatch_semaphore_create(1); + + func(^{ + dispatch_semaphore_signal(sema); + }); + dispatch_semaphore_wait(sema, 100); // expected-warning{{Possible semaphore performance anti-pattern}} +} + +void differentsemaphoreok() { + dispatch_semaphore_t sema1 = dispatch_semaphore_create(0); + dispatch_semaphore_t sema2 = dispatch_semaphore_create(0); + + func(^{ + dispatch_semaphore_signal(sema1); + }); + dispatch_semaphore_wait(sema2, 100); // no-warning +} + +void nosignalok() { + dispatch_semaphore_t sema1 = dispatch_semaphore_create(0); + dispatch_semaphore_wait(sema1, 100); +} + +void nowaitok() { + dispatch_semaphore_t sema = dispatch_semaphore_create(0); + func(^{ + dispatch_semaphore_signal(sema); + }); +} + +void noblockok() { + dispatch_semaphore_t sema = dispatch_semaphore_create(0); + dispatch_semaphore_signal(sema); + dispatch_semaphore_wait(sema, 100); +} + +void storedblockok() { + dispatch_semaphore_t sema = dispatch_semaphore_create(0); + block_t b = ^{ + dispatch_semaphore_signal(sema); + }; + dispatch_semaphore_wait(sema, 100); +} + +void passed_semaphore_ok(dispatch_semaphore_t sema) { + func(^{ + dispatch_semaphore_signal(sema); + }); + dispatch_semaphore_wait(sema, 100); +} + +void warn_with_cast() { + dispatch_semaphore_t sema = dispatch_semaphore_create(0); + + func(^{ + dispatch_semaphore_signal((int)sema); + }); + dispatch_semaphore_wait((int)sema, 100); // expected-warning{{Possible semaphore performance anti-pattern}} +} + +