NoQ created this revision.
Herald added subscribers: cfe-commits, a.sidorin, szepet, xazax.hun.
I'm seeing false positives on the new check added in
https://reviews.llvm.org/D39438 of the following kind:
void foo() {
T buf[16];
for ( int n = 0; n < 16; ++n) {
T *ptr = &buf[n];
dispatch_async(queue, ^{ use(ptr); });
}
dispatch_barrier_sync(queue, ^{});
}
In this case, pointer to the local variable `buf` is captured by a block that
goes into `dispatch_async`, yet it is not a bug because synchronization ensures
that this block quits before the variable goes out of scope.
I guess it's kinda similar to the semaphore synchronization case. We could
probably add another suppression for functions that contain any
`dispatch_barrier_async` calls. The ideal solution is to delay the warning
until `checkEndFunction` (like the escape-to-global check does) to see if
things get synced up until then (of course, once we have scopes we can make it
even more fine-grained).
Alexander, do you think we should keep the checker under a flag (not enabled
for regular users) until we get a better understanding of what kinds of
synchronizations can get on the way? This diff splits your new check into
`alpha.core.StackAddressAsyncEscape`. Or do you already have good quick fixes
coming?
Repository:
rC Clang
https://reviews.llvm.org/D41042
Files:
include/clang/StaticAnalyzer/Checkers/Checkers.td
lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
test/Analysis/stack-capture-leak-arc.mm
Index: test/Analysis/stack-capture-leak-arc.mm
===================================================================
--- test/Analysis/stack-capture-leak-arc.mm
+++ test/Analysis/stack-capture-leak-arc.mm
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10
-analyzer-checker=core -fblocks -fobjc-arc -verify %s
+// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10
-analyzer-checker=core,alpha.core.StackAddressAsyncEscape -fblocks -fobjc-arc
-verify %s
typedef struct dispatch_queue_s *dispatch_queue_t;
typedef void (^dispatch_block_t)(void);
Index: lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
+++ lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
@@ -37,6 +37,14 @@
mutable std::unique_ptr<BuiltinBug> BT_capturedstackret;
public:
+ enum CheckKind {
+ CK_StackAddrEscapeChecker,
+ CK_StackAddrAsyncEscapeChecker,
+ CK_NumCheckKinds
+ };
+
+ DefaultBool ChecksEnabled[CK_NumCheckKinds];
+
void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
void checkPreStmt(const ReturnStmt *RS, CheckerContext &C) const;
void checkEndFunction(CheckerContext &Ctx) const;
@@ -225,6 +233,8 @@
void StackAddrEscapeChecker::checkPreCall(const CallEvent &Call,
CheckerContext &C) const {
+ if (!ChecksEnabled[CK_StackAddrAsyncEscapeChecker])
+ return;
if (!Call.isGlobalCFunction("dispatch_after") &&
!Call.isGlobalCFunction("dispatch_async"))
return;
@@ -237,6 +247,8 @@
void StackAddrEscapeChecker::checkPreStmt(const ReturnStmt *RS,
CheckerContext &C) const {
+ if (!ChecksEnabled[CK_StackAddrEscapeChecker])
+ return;
const Expr *RetE = RS->getRetValue();
if (!RetE)
@@ -277,6 +289,9 @@
}
void StackAddrEscapeChecker::checkEndFunction(CheckerContext &Ctx) const {
+ if (!ChecksEnabled[CK_StackAddrEscapeChecker])
+ return;
+
ProgramStateRef State = Ctx.getState();
// Iterate over all bindings to global variables and see if it contains
@@ -346,6 +361,12 @@
}
}
-void ento::registerStackAddrEscapeChecker(CheckerManager &Mgr) {
- Mgr.registerChecker<StackAddrEscapeChecker>();
-}
+#define REGISTER_CHECKER(name) \
+ void ento::register##name(CheckerManager &Mgr) { \
+ StackAddrEscapeChecker *Chk = \
+ Mgr.registerChecker<StackAddrEscapeChecker>(); \
+ Chk->ChecksEnabled[StackAddrEscapeChecker::CK_##name] = true; \
+ }
+
+REGISTER_CHECKER(StackAddrEscapeChecker)
+REGISTER_CHECKER(StackAddrAsyncEscapeChecker)
Index: include/clang/StaticAnalyzer/Checkers/Checkers.td
===================================================================
--- include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -188,6 +188,10 @@
HelpText<"Check for cases where the dynamic and the static type of an object
are unrelated.">,
DescFile<"DynamicTypeChecker.cpp">;
+def StackAddrAsyncEscapeChecker : Checker<"StackAddressAsyncEscape">,
+ HelpText<"Check that addresses to stack memory do not escape the function">,
+ DescFile<"StackAddrEscapeChecker.cpp">;
+
} // end "alpha.core"
let ParentPackage = Nullability in {
Index: test/Analysis/stack-capture-leak-arc.mm
===================================================================
--- test/Analysis/stack-capture-leak-arc.mm
+++ test/Analysis/stack-capture-leak-arc.mm
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 -analyzer-checker=core -fblocks -fobjc-arc -verify %s
+// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 -analyzer-checker=core,alpha.core.StackAddressAsyncEscape -fblocks -fobjc-arc -verify %s
typedef struct dispatch_queue_s *dispatch_queue_t;
typedef void (^dispatch_block_t)(void);
Index: lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
+++ lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
@@ -37,6 +37,14 @@
mutable std::unique_ptr<BuiltinBug> BT_capturedstackret;
public:
+ enum CheckKind {
+ CK_StackAddrEscapeChecker,
+ CK_StackAddrAsyncEscapeChecker,
+ CK_NumCheckKinds
+ };
+
+ DefaultBool ChecksEnabled[CK_NumCheckKinds];
+
void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
void checkPreStmt(const ReturnStmt *RS, CheckerContext &C) const;
void checkEndFunction(CheckerContext &Ctx) const;
@@ -225,6 +233,8 @@
void StackAddrEscapeChecker::checkPreCall(const CallEvent &Call,
CheckerContext &C) const {
+ if (!ChecksEnabled[CK_StackAddrAsyncEscapeChecker])
+ return;
if (!Call.isGlobalCFunction("dispatch_after") &&
!Call.isGlobalCFunction("dispatch_async"))
return;
@@ -237,6 +247,8 @@
void StackAddrEscapeChecker::checkPreStmt(const ReturnStmt *RS,
CheckerContext &C) const {
+ if (!ChecksEnabled[CK_StackAddrEscapeChecker])
+ return;
const Expr *RetE = RS->getRetValue();
if (!RetE)
@@ -277,6 +289,9 @@
}
void StackAddrEscapeChecker::checkEndFunction(CheckerContext &Ctx) const {
+ if (!ChecksEnabled[CK_StackAddrEscapeChecker])
+ return;
+
ProgramStateRef State = Ctx.getState();
// Iterate over all bindings to global variables and see if it contains
@@ -346,6 +361,12 @@
}
}
-void ento::registerStackAddrEscapeChecker(CheckerManager &Mgr) {
- Mgr.registerChecker<StackAddrEscapeChecker>();
-}
+#define REGISTER_CHECKER(name) \
+ void ento::register##name(CheckerManager &Mgr) { \
+ StackAddrEscapeChecker *Chk = \
+ Mgr.registerChecker<StackAddrEscapeChecker>(); \
+ Chk->ChecksEnabled[StackAddrEscapeChecker::CK_##name] = true; \
+ }
+
+REGISTER_CHECKER(StackAddrEscapeChecker)
+REGISTER_CHECKER(StackAddrAsyncEscapeChecker)
Index: include/clang/StaticAnalyzer/Checkers/Checkers.td
===================================================================
--- include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -188,6 +188,10 @@
HelpText<"Check for cases where the dynamic and the static type of an object are unrelated.">,
DescFile<"DynamicTypeChecker.cpp">;
+def StackAddrAsyncEscapeChecker : Checker<"StackAddressAsyncEscape">,
+ HelpText<"Check that addresses to stack memory do not escape the function">,
+ DescFile<"StackAddrEscapeChecker.cpp">;
+
} // end "alpha.core"
let ParentPackage = Nullability in {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
