replica
/
hanchenye-llvm-project
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

[analyzer] Move the GCDAsyncSemaphoreChecker to optin.performance 

rdar://38383753

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

llvm-svn: 327309
This commit is contained in:
George Karpenkov 2018-03-12 18:27:36 +00:00
parent 2d08f2ebf8
commit 44a3b7c130
4 changed files with 83 additions and 44 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
View File

@ -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 {

2
clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
View File

@ -37,7 +37,7 @@ add_clang_library(clangStaticAnalyzerCheckers
DynamicTypeChecker.cpp
ExprInspectionChecker.cpp
FixedAddressChecker.cpp
GCDAsyncSemaphoreChecker.cpp
GCDAntipatternChecker.cpp
GenericTaintChecker.cpp
GTestChecker.cpp
IdenticalExprChecker.cpp

43
clang/lib/StaticAnalyzer/Checkers/GCDAsyncSemaphoreChecker.cpp → clang/lib/StaticAnalyzer/Checkers/GCDAntipatternChecker.cpp
View File

@ -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>();
}

72
clang/test/Analysis/gcdasyncsemaphorechecker_test.m → clang/test/Analysis/gcdantipatternchecker_test.m
View File

@ -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