This revision was automatically updated to reflect the committed changes.
Closed by commit rGa5e354ec4da1: [analyzer] Fixing a bug raising false
positives of stack block object (authored by ziqingluo-90).
Changed prior to commit:
https://reviews.llvm.org/D131009?vs=453144&id=455992#toc
Repository:
rG LLVM Github Monorepo
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 = Ctx.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
