r328281 - [analyzer] Extend GCDAntipatternChecker to match group_enter/group_leave pattern
George Karpenkov via cfe-commits
cfe-commits at lists.llvm.org
Thu Mar 22 17:16:02 PDT 2018
Author: george.karpenkov
Date: Thu Mar 22 17:16:02 2018
New Revision: 328281
URL: http://llvm.org/viewvc/llvm-project?rev=328281&view=rev
Log:
[analyzer] Extend GCDAntipatternChecker to match group_enter/group_leave pattern
rdar://38480416
Differential Revision: https://reviews.llvm.org/D44653
Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/GCDAntipatternChecker.cpp
cfe/trunk/test/Analysis/gcdantipatternchecker_test.m
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/GCDAntipatternChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/GCDAntipatternChecker.cpp?rev=328281&r1=328280&r2=328281&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/GCDAntipatternChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/GCDAntipatternChecker.cpp Thu Mar 22 17:16:02 2018
@@ -43,7 +43,8 @@ using namespace ast_matchers;
namespace {
-const char *WarningBinding = "semaphore_wait";
+// ID of a node at which the diagnostic would be emitted.
+const char *WarnAtNode = "waitcall";
class GCDAntipatternChecker : public Checker<check::ASTCodeBody> {
public:
@@ -52,19 +53,6 @@ public:
BugReporter &BR) const;
};
-class Callback : public MatchFinder::MatchCallback {
- BugReporter &BR;
- const GCDAntipatternChecker *C;
- AnalysisDeclContext *ADC;
-
-public:
- Callback(BugReporter &BR,
- AnalysisDeclContext *ADC,
- const GCDAntipatternChecker *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)));
@@ -81,24 +69,28 @@ auto bindAssignmentToDecl(const char *De
declRefExpr(to(varDecl().bind(DeclName)))));
}
-void GCDAntipatternChecker::checkASTCodeBody(const Decl *D,
- AnalysisManager &AM,
- BugReporter &BR) const {
-
- // The pattern is very common in tests, and it is OK to use it there.
+/// The pattern is very common in tests, and it is OK to use it there.
+/// We have to heuristics for detecting tests: method name starts with "test"
+/// (used in XCTest), and a class name contains "mock" or "test" (used in
+/// helpers which are not tests themselves, but used exclusively in tests).
+static bool isTest(const Decl *D) {
if (const auto* ND = dyn_cast<NamedDecl>(D)) {
std::string DeclName = ND->getNameAsString();
if (StringRef(DeclName).startswith("test"))
- return;
+ return true;
}
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;
+ return true;
}
}
+ return false;
+}
+
+static auto findGCDAntiPatternWithSemaphore() -> decltype(compoundStmt()) {
const char *SemaphoreBinding = "semaphore_name";
auto SemaphoreCreateM = callExpr(callsName("dispatch_semaphore_create"));
@@ -109,13 +101,51 @@ void GCDAntipatternChecker::checkASTCode
forEachDescendant(binaryOperator(bindAssignmentToDecl(SemaphoreBinding),
hasRHS(SemaphoreCreateM))));
+ auto HasBlockArgumentM = hasAnyArgument(hasType(
+ hasCanonicalType(blockPointerType())
+ ));
+
+ auto ArgCallsSignalM = hasAnyArgument(stmt(hasDescendant(callExpr(
+ allOf(
+ callsName("dispatch_semaphore_signal"),
+ equalsBoundArgDecl(0, SemaphoreBinding)
+ )))));
+
+ auto HasBlockAndCallsSignalM = allOf(HasBlockArgumentM, ArgCallsSignalM);
+
+ auto HasBlockCallingSignalM =
+ forEachDescendant(
+ stmt(anyOf(
+ callExpr(HasBlockAndCallsSignalM),
+ objcMessageExpr(HasBlockAndCallsSignalM)
+ )));
+
auto SemaphoreWaitM = forEachDescendant(
callExpr(
allOf(
callsName("dispatch_semaphore_wait"),
equalsBoundArgDecl(0, SemaphoreBinding)
)
- ).bind(WarningBinding));
+ ).bind(WarnAtNode));
+
+ return compoundStmt(
+ SemaphoreBindingM, HasBlockCallingSignalM, SemaphoreWaitM);
+}
+
+static auto findGCDAntiPatternWithGroup() -> decltype(compoundStmt()) {
+
+ const char *GroupBinding = "group_name";
+ auto DispatchGroupCreateM = callExpr(callsName("dispatch_group_create"));
+
+ auto GroupBindingM = anyOf(
+ forEachDescendant(
+ varDecl(hasDescendant(DispatchGroupCreateM)).bind(GroupBinding)),
+ forEachDescendant(binaryOperator(bindAssignmentToDecl(GroupBinding),
+ hasRHS(DispatchGroupCreateM))));
+
+ auto GroupEnterM = forEachDescendant(
+ stmt(callExpr(allOf(callsName("dispatch_group_enter"),
+ equalsBoundArgDecl(0, GroupBinding)))));
auto HasBlockArgumentM = hasAnyArgument(hasType(
hasCanonicalType(blockPointerType())
@@ -123,40 +153,71 @@ void GCDAntipatternChecker::checkASTCode
auto ArgCallsSignalM = hasAnyArgument(stmt(hasDescendant(callExpr(
allOf(
- callsName("dispatch_semaphore_signal"),
- equalsBoundArgDecl(0, SemaphoreBinding)
+ callsName("dispatch_group_leave"),
+ equalsBoundArgDecl(0, GroupBinding)
)))));
- auto HasBlockAndCallsSignalM = allOf(HasBlockArgumentM, ArgCallsSignalM);
+ auto HasBlockAndCallsLeaveM = allOf(HasBlockArgumentM, ArgCallsSignalM);
auto AcceptsBlockM =
forEachDescendant(
stmt(anyOf(
- callExpr(HasBlockAndCallsSignalM),
- objcMessageExpr(HasBlockAndCallsSignalM)
+ callExpr(HasBlockAndCallsLeaveM),
+ objcMessageExpr(HasBlockAndCallsLeaveM)
)));
- auto FinalM = compoundStmt(SemaphoreBindingM, SemaphoreWaitM, AcceptsBlockM);
-
- MatchFinder F;
- Callback CB(BR, AM.getAnalysisDeclContext(D), this);
+ auto GroupWaitM = forEachDescendant(
+ callExpr(
+ allOf(
+ callsName("dispatch_group_wait"),
+ equalsBoundArgDecl(0, GroupBinding)
+ )
+ ).bind(WarnAtNode));
- F.addMatcher(FinalM, &CB);
- F.match(*D->getBody(), AM.getASTContext());
+ return compoundStmt(GroupBindingM, GroupEnterM, AcceptsBlockM, GroupWaitM);
}
-void Callback::run(const MatchFinder::MatchResult &Result) {
- const auto *SW = Result.Nodes.getNodeAs<CallExpr>(WarningBinding);
+static void emitDiagnostics(const BoundNodes &Nodes,
+ const char* Type,
+ BugReporter &BR,
+ AnalysisDeclContext *ADC,
+ const GCDAntipatternChecker *Checker) {
+ const auto *SW = Nodes.getNodeAs<CallExpr>(WarnAtNode);
assert(SW);
+
+ std::string Diagnostics;
+ llvm::raw_string_ostream OS(Diagnostics);
+ OS << "Waiting on a " << Type << " 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";
+
BR.EmitBasicReport(
- ADC->getDecl(), C,
- /*Name=*/"Semaphore performance anti-pattern",
- /*Category=*/"Performance",
- "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());
+ ADC->getDecl(),
+ Checker,
+ /*Name=*/"GCD performance anti-pattern",
+ /*Category=*/"Performance",
+ OS.str(),
+ PathDiagnosticLocation::createBegin(SW, BR.getSourceManager(), ADC),
+ SW->getSourceRange());
+}
+
+void GCDAntipatternChecker::checkASTCodeBody(const Decl *D,
+ AnalysisManager &AM,
+ BugReporter &BR) const {
+ if (isTest(D))
+ return;
+
+ AnalysisDeclContext *ADC = AM.getAnalysisDeclContext(D);
+
+ auto SemaphoreMatcherM = findGCDAntiPatternWithSemaphore();
+ auto Matches = match(SemaphoreMatcherM, *D->getBody(), AM.getASTContext());
+ for (BoundNodes Match : Matches)
+ emitDiagnostics(Match, "semaphore", BR, ADC, this);
+
+ auto GroupMatcherM = findGCDAntiPatternWithGroup();
+ Matches = match(GroupMatcherM, *D->getBody(), AM.getASTContext());
+ for (BoundNodes Match : Matches)
+ emitDiagnostics(Match, "group", BR, ADC, this);
}
}
Modified: cfe/trunk/test/Analysis/gcdantipatternchecker_test.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/gcdantipatternchecker_test.m?rev=328281&r1=328280&r2=328281&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/gcdantipatternchecker_test.m (original)
+++ cfe/trunk/test/Analysis/gcdantipatternchecker_test.m Thu Mar 22 17:16:02 2018
@@ -10,9 +10,16 @@ typedef signed char BOOL;
@end
typedef int dispatch_semaphore_t;
+typedef int dispatch_group_t;
typedef void (^block_t)();
dispatch_semaphore_t dispatch_semaphore_create(int);
+dispatch_group_t dispatch_group_create();
+void dispatch_group_enter(dispatch_group_t);
+void dispatch_group_leave(dispatch_group_t);
+void dispatch_group_wait(dispatch_group_t, int);
+
+
void dispatch_semaphore_wait(dispatch_semaphore_t, int);
void dispatch_semaphore_signal(dispatch_semaphore_t);
@@ -179,6 +186,7 @@ void warn_with_cast() {
-(void)use_method_warn;
-(void) pass_block_as_second_param_warn;
-(void)use_objc_callback_warn;
+-(void) use_dispatch_group;
-(void)testNoWarn;
-(void)acceptBlock:(block_t)callback;
-(void)flag:(int)flag acceptBlock:(block_t)callback;
@@ -230,6 +238,16 @@ void warn_with_cast() {
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_dispatch_group {
+ dispatch_group_t group = dispatch_group_create();
+ dispatch_group_enter(group);
+ [self acceptBlock:^{
+ dispatch_group_leave(group);
+ }];
+ dispatch_group_wait(group, 100); // expected-warning{{Waiting on a group with Grand Central Dispatch}}
+
+}
+
void use_objc_and_c_callback(MyInterface1 *t) {
dispatch_semaphore_t sema = dispatch_semaphore_create(0);
@@ -279,3 +297,39 @@ void use_objc_and_c_callback(MyInterface
dispatch_semaphore_wait(sema, 100);
}
@end
+
+void dispatch_group_wait_func(MyInterface1 *M) {
+ dispatch_group_t group = dispatch_group_create();
+ dispatch_group_enter(group);
+
+ func(^{
+ dispatch_group_leave(group);
+ });
+ dispatch_group_wait(group, 100); // expected-warning{{Waiting on a group with Grand Central Dispatch}}
+}
+
+
+void dispatch_group_wait_cfunc(MyInterface1 *M) {
+ dispatch_group_t group = dispatch_group_create();
+ dispatch_group_enter(group);
+ [M acceptBlock:^{
+ dispatch_group_leave(group);
+ }];
+ dispatch_group_wait(group, 100); // expected-warning{{Waiting on a group with Grand Central Dispatch}}
+}
+
+void dispatch_group_and_semaphore_use(MyInterface1 *M) {
+ dispatch_group_t group = dispatch_group_create();
+ dispatch_group_enter(group);
+ [M acceptBlock:^{
+ dispatch_group_leave(group);
+ }];
+ dispatch_group_wait(group, 100); // expected-warning{{Waiting on a group with Grand Central Dispatch}}
+
+ dispatch_semaphore_t sema1 = dispatch_semaphore_create(0);
+
+ [M acceptBlock:^{
+ dispatch_semaphore_signal(sema1);
+ }];
+ dispatch_semaphore_wait(sema1, 100); // expected-warning{{Waiting on a semaphore with Grand Central Dispatch creates useless threads and is subject to priority inversion}}
+}
More information about the cfe-commits
mailing list