ziqingluo-90 updated this revision to Diff 453144.
ziqingluo-90 added a comment.
Addressing @NoQ 's comment. Let the symbolic execution simulator put a block
object in an `UnknownSpaceRegion` from the start if ARC is enabled. Because in
such cases whether the block is on stack or in heap depends on the
implementation. And, those blocks will be moved to the heap when necessary.
So they will never trigger a stack address leak issue.
Removing those ARC checking in `StackAddrEscapeChecker` since they are no
longer needed.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D131009/new/
https://reviews.llvm.org/D131009
Files:
clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
clang/lib/StaticAnalyzer/Core/MemRegion.cpp
clang/test/Analysis/stack-capture-leak-arc.mm
clang/test/Analysis/stack-capture-leak-no-arc.mm
Index: clang/test/Analysis/stack-capture-leak-no-arc.mm
===================================================================
--- clang/test/Analysis/stack-capture-leak-no-arc.mm
+++ clang/test/Analysis/stack-capture-leak-no-arc.mm
@@ -4,6 +4,7 @@
typedef void (^dispatch_block_t)(void);
void dispatch_async(dispatch_queue_t queue, dispatch_block_t block);
extern dispatch_queue_t queue;
+void f(int);
void test_block_inside_block_async_no_leak() {
int x = 123;
@@ -35,3 +36,46 @@
return outer; // expected-warning-re{{Address of stack-allocated block declared on line {{.+}} is captured by a returned block}}
}
+// The block literal defined in this function could leak once being
+// called.
+void output_block(dispatch_block_t * blk) {
+ int x = 0;
+ *blk = ^{ f(x); }; // expected-warning {{Address of stack-allocated block declared on line 43 is still referred to by the stack variable 'blk' upon returning to the caller. This will be a dangling reference [core.StackAddressEscape]}}
+}
+
+// The block literal captures nothing thus is treated as a constant.
+void output_constant_block(dispatch_block_t * blk) {
+ *blk = ^{ };
+}
+
+// A block can leak if it captures at least one variable and is not
+// under ARC when its' stack frame expires.
+void test_block_leak() {
+ __block dispatch_block_t blk;
+ int x = 0;
+ dispatch_block_t p = ^{
+ blk = ^{ // expected-warning {{Address of stack-allocated block declared on line 57 is still referred to by the stack variable 'blk' upon returning to the caller. This will be a dangling reference [core.StackAddressEscape]}}
+ f(x);
+ };
+ };
+
+ p();
+ blk();
+ output_block(&blk);
+ blk();
+}
+
+// A block captures nothing is a constant thus never leaks.
+void test_constant_block_no_leak() {
+ __block dispatch_block_t blk;
+ dispatch_block_t p = ^{
+ blk = ^{
+ f(0);
+ };
+ };
+
+ p();
+ blk();
+ output_constant_block(&blk);
+ blk();
+}
Index: clang/test/Analysis/stack-capture-leak-arc.mm
===================================================================
--- clang/test/Analysis/stack-capture-leak-arc.mm
+++ clang/test/Analysis/stack-capture-leak-arc.mm
@@ -8,6 +8,7 @@
typedef long dispatch_time_t;
void dispatch_after(dispatch_time_t when, dispatch_queue_t queue, dispatch_block_t block);
void dispatch_barrier_sync(dispatch_queue_t queue, dispatch_block_t block);
+void f(int);
extern dispatch_queue_t queue;
extern dispatch_once_t *predicate;
@@ -187,3 +188,40 @@
}
dispatch_barrier_sync(queue, ^{});
}
+
+void output_block(dispatch_block_t * blk) {
+ int x = 0;
+ *blk = ^{ f(x); };
+}
+
+// Block objects themselves can never leak under ARC.
+void test_no_block_leak() {
+ __block dispatch_block_t blk;
+ int x = 0;
+ dispatch_block_t p = ^{
+ blk = ^{
+ f(x);
+ };
+ };
+ p();
+ blk();
+ output_block(&blk);
+ blk();
+}
+
+// Block objects do not leak under ARC but stack variables of
+// non-object kind indirectly referred by a block can leak.
+dispatch_block_t test_block_referencing_variable_leak() {
+ int x = 0;
+ __block int * p = &x;
+ __block int * q = &x;
+
+ dispatch_async(queue, ^{// expected-warning {{Address of stack memory associated with local variable 'x' is captured by an asynchronously-executed block \
+[alpha.core.StackAddressAsyncEscape]}}
+ ++(*p);
+ });
+ return (dispatch_block_t) ^{// expected-warning {{Address of stack memory associated with local variable 'x' is captured by a returned block \
+[core.StackAddressEscape]}}
+ ++(*q);
+ };
+}
Index: clang/lib/StaticAnalyzer/Core/MemRegion.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/MemRegion.cpp
+++ clang/lib/StaticAnalyzer/Core/MemRegion.cpp
@@ -1076,14 +1076,18 @@
sReg = getGlobalsRegion(MemRegion::GlobalImmutableSpaceRegionKind);
}
else {
- if (LC) {
+ bool IsArcManagedBlock = BD->getASTContext().getLangOpts().ObjCAutoRefCount;
+
+ // ARC managed blocks can be initialized on stack or directly in heap
+ // depending on the implementations. So we initialize them with
+ // UnknownRegion.
+ if (!IsArcManagedBlock && LC) {
// FIXME: Once we implement scope handling, we want the parent region
// to be the scope.
const StackFrameContext *STC = LC->getStackFrame();
assert(STC);
sReg = getStackLocalsRegion(STC);
- }
- else {
+ } else {
// We allow 'LC' to be NULL for cases where want BlockDataRegions
// without context-sensitivity.
sReg = getUnknownRegion();
Index: clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
@@ -61,7 +61,6 @@
ASTContext &Ctx);
static SmallVector<const MemRegion *, 4>
getCapturedStackRegions(const BlockDataRegion &B, CheckerContext &C);
- static bool isArcManagedBlock(const MemRegion *R, CheckerContext &C);
static bool isNotInCurrentFrame(const MemRegion *R, CheckerContext &C);
};
} // namespace
@@ -110,13 +109,6 @@
return range;
}
-bool StackAddrEscapeChecker::isArcManagedBlock(const MemRegion *R,
- CheckerContext &C) {
- assert(R && "MemRegion should not be null");
- return C.getASTContext().getLangOpts().ObjCAutoRefCount &&
- isa<BlockDataRegion>(R);
-}
-
bool StackAddrEscapeChecker::isNotInCurrentFrame(const MemRegion *R,
CheckerContext &C) {
const StackSpaceRegion *S = cast<StackSpaceRegion>(R->getMemorySpace());
@@ -214,7 +206,7 @@
void StackAddrEscapeChecker::checkReturnedBlockCaptures(
const BlockDataRegion &B, CheckerContext &C) const {
for (const MemRegion *Region : getCapturedStackRegions(B, C)) {
- if (isArcManagedBlock(Region, C) || isNotInCurrentFrame(Region, C))
+ if (isNotInCurrentFrame(Region, C))
continue;
ExplodedNode *N = C.generateNonFatalErrorNode();
if (!N)
@@ -267,8 +259,7 @@
if (const BlockDataRegion *B = dyn_cast<BlockDataRegion>(R))
checkReturnedBlockCaptures(*B, C);
- if (!isa<StackSpaceRegion>(R->getMemorySpace()) ||
- isNotInCurrentFrame(R, C) || isArcManagedBlock(R, C))
+ if (!isa<StackSpaceRegion>(R->getMemorySpace()) || isNotInCurrentFrame(R, C))
return;
// Returning a record by value is fine. (In this case, the returned
@@ -348,8 +339,7 @@
// Check the globals for the same.
if (!isa<GlobalsSpaceRegion>(Region->getMemorySpace()))
return true;
- if (VR && VR->hasStackStorage() && !isArcManagedBlock(VR, Ctx) &&
- !isNotInCurrentFrame(VR, Ctx))
+ if (VR && VR->hasStackStorage() && !isNotInCurrentFrame(VR, Ctx))
V.emplace_back(Region, VR);
return true;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
