NoQ created this revision.
NoQ added reviewers: zaks.anna, dcoughlin.
NoQ added a subscriber: cfe-commits.
The checker already warns for `__block`-storage variables being used as a
`dispatch_once()` predicate, however it refers to them as local which is not
quite accurate, so we fix that.
Also add tests to make sure it works.
FIXME: The analyzer sets stack memory space for `__block` variables when they
are referenced outside the block (eg. `test_block_var_from_outside_block()`
line 108). Will try to fix in a separate patch; i'm not relying on the memory
space in this patch.
https://reviews.llvm.org/D26159
Files:
lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp
test/Analysis/dispatch-once.m
Index: test/Analysis/dispatch-once.m
===================================================================
--- test/Analysis/dispatch-once.m
+++ test/Analysis/dispatch-once.m
@@ -90,3 +90,20 @@
void test_ivar_array_from_external_obj(Object *o) {
dispatch_once(&o->once_array[1], ^{}); // expected-warning{{Call to
'dispatch_once' uses memory within the instance variable 'once_array' for the
predicate value.}}
}
+
+void test_block_var_from_block() {
+ __block dispatch_once_t once;
+ ^{
+ dispatch_once(&once, ^{}); // expected-warning{{Call to 'dispatch_once'
uses the block variable 'once' for the predicate value.}}
+ };
+}
+
+void use_block_var(dispatch_once_t *once);
+
+void test_block_var_from_outside_block() {
+ __block dispatch_once_t once;
+ ^{
+ use_block_var(&once);
+ };
+ dispatch_once(&once, ^{}); // expected-warning{{Call to 'dispatch_once' uses
the block variable 'once' for the predicate value.}}
+}
Index: lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp
@@ -94,10 +94,15 @@
bool SuggestStatic = false;
os << "Call to '" << FName << "' uses";
if (const VarRegion *VR = dyn_cast<VarRegion>(RB)) {
- // We filtered out globals earlier, so it must be a local variable.
+ // We filtered out globals earlier, so it must be a local variable
+ // or a block variable which is under UnknownSpaceRegion.
if (VR != R)
os << " memory within";
- os << " the local variable '" << VR->getDecl()->getName() << '\'';
+ if (VR->getDecl()->hasAttr<BlocksAttr>())
+ os << " the block variable '";
+ else
+ os << " the local variable '";
+ os << VR->getDecl()->getName() << '\'';
SuggestStatic = true;
} else if (const ObjCIvarRegion *IVR = getParentIvarRegion(R)) {
if (IVR != R)
@@ -108,6 +113,9 @@
} else if (isa<UnknownSpaceRegion>(RS)) {
// Presence of an IVar superregion has priority over this branch, because
// ObjC objects are on the heap even if the core doesn't realize this.
+ // Presence of a block variable base region has priority over this branch,
+ // because block variables are known to be either on stack or on heap
+ // (might actually move between the two, hence UnknownSpace).
return;
} else {
os << " stack allocated memory";
Index: test/Analysis/dispatch-once.m
===================================================================
--- test/Analysis/dispatch-once.m
+++ test/Analysis/dispatch-once.m
@@ -90,3 +90,20 @@
void test_ivar_array_from_external_obj(Object *o) {
dispatch_once(&o->once_array[1], ^{}); // expected-warning{{Call to 'dispatch_once' uses memory within the instance variable 'once_array' for the predicate value.}}
}
+
+void test_block_var_from_block() {
+ __block dispatch_once_t once;
+ ^{
+ dispatch_once(&once, ^{}); // expected-warning{{Call to 'dispatch_once' uses the block variable 'once' for the predicate value.}}
+ };
+}
+
+void use_block_var(dispatch_once_t *once);
+
+void test_block_var_from_outside_block() {
+ __block dispatch_once_t once;
+ ^{
+ use_block_var(&once);
+ };
+ dispatch_once(&once, ^{}); // expected-warning{{Call to 'dispatch_once' uses the block variable 'once' for the predicate value.}}
+}
Index: lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp
@@ -94,10 +94,15 @@
bool SuggestStatic = false;
os << "Call to '" << FName << "' uses";
if (const VarRegion *VR = dyn_cast<VarRegion>(RB)) {
- // We filtered out globals earlier, so it must be a local variable.
+ // We filtered out globals earlier, so it must be a local variable
+ // or a block variable which is under UnknownSpaceRegion.
if (VR != R)
os << " memory within";
- os << " the local variable '" << VR->getDecl()->getName() << '\'';
+ if (VR->getDecl()->hasAttr<BlocksAttr>())
+ os << " the block variable '";
+ else
+ os << " the local variable '";
+ os << VR->getDecl()->getName() << '\'';
SuggestStatic = true;
} else if (const ObjCIvarRegion *IVR = getParentIvarRegion(R)) {
if (IVR != R)
@@ -108,6 +113,9 @@
} else if (isa<UnknownSpaceRegion>(RS)) {
// Presence of an IVar superregion has priority over this branch, because
// ObjC objects are on the heap even if the core doesn't realize this.
+ // Presence of a block variable base region has priority over this branch,
+ // because block variables are known to be either on stack or on heap
+ // (might actually move between the two, hence UnknownSpace).
return;
} else {
os << " stack allocated memory";
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
