Commit 52ebd89f authored by George Karpenkov's avatar George Karpenkov
Browse files

[analyzer] AST-matching checker to detect global central dispatch performance anti-pattern 

rdar://37312818

NB: The checker does not care about the ordering of callbacks, see the
relevant FIXME in tests.

Differential Revision: https://reviews.llvm.org/D44059
parent 4b009fe7

clang/include/clang/StaticAnalyzer/Checkers/Checkers.td

+7 −0
Original line number Diff line number Diff line
@@ -610,6 +610,13 @@ def ObjCSuperDeallocChecker : Checker<"SuperDealloc">, 


} // 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">,

clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt

+1 −0
Original line number Diff line number Diff line
@@ -37,6 +37,7 @@ add_clang_library(clangStaticAnalyzerCheckers 
  DynamicTypeChecker.cpp

  ExprInspectionChecker.cpp

  FixedAddressChecker.cpp

  GCDAsyncSemaphoreChecker.cpp

  GenericTaintChecker.cpp

  GTestChecker.cpp

  IdenticalExprChecker.cpp

clang/lib/StaticAnalyzer/Checkers/GCDAsyncSemaphoreChecker.cpp

0 → 100644
+154 −0
Original line number Diff line number Diff line 
//===- 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<check::ASTCodeBody> {

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<NamedDecl>(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<CallExpr>(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<GCDAsyncSemaphoreChecker>();

}

clang/test/gcdasyncsemaphorechecker_test.m

0 → 100644
+169 −0
Original line number Diff line number Diff line 
// 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}}

}