[clang] a5e354e - [analyzer] Fixing a bug raising false positives of stack block object
via cfe-commits
cfe-commits at lists.llvm.org
Fri Aug 26 12:20:23 PDT 2022
Author: ziqingluo-90
Date: 2022-08-26T12:19:32-07:00
New Revision: a5e354ec4da14cfc02b9847be314524e8deb93c6
URL: https://github.com/llvm/llvm-project/commit/a5e354ec4da14cfc02b9847be314524e8deb93c6
DIFF: https://github.com/llvm/llvm-project/commit/a5e354ec4da14cfc02b9847be314524e8deb93c6.diff
LOG: [analyzer] Fixing a bug raising false positives of stack block object
leaking in ARC mode
When ARC (automatic reference count) is enabled, (objective-c) block
objects are automatically retained and released thus they do not leak.
Without ARC, they still can leak from an expiring stack frame like
other stack variables.
With this commit, the static analyzer now puts a block object in an
"unknown" region if ARC is enabled because it is up to the
implementation to choose whether to put the object on stack initially
(then move to heap when needed) or in heap directly under ARC.
Therefore, the `StackAddrEscapeChecker` has no need to know
specifically about ARC at all and it will not report errors on objects
in "unknown" regions.
Reviewed By: NoQ (Artem Dergachev)
Differential Revision: https://reviews.llvm.org/D131009
Added:
Modified:
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
Removed:
################################################################################
diff --git a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
index e6cea0fbff8cd..c4b7411e94011 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
@@ -61,7 +61,6 @@ class StackAddrEscapeChecker
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 @@ SourceRange StackAddrEscapeChecker::genName(raw_ostream &os, const MemRegion *R,
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::checkAsyncExecutedBlockCaptures(
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 @@ void StackAddrEscapeChecker::checkPreStmt(const ReturnStmt *RS,
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 @@ void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS,
// 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;
}
diff --git a/clang/lib/StaticAnalyzer/Core/MemRegion.cpp b/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
index bb64cbc4b71c4..482862a23030a 100644
--- a/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
+++ b/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
@@ -1076,14 +1076,18 @@ MemRegionManager::getBlockDataRegion(const BlockCodeRegion *BC,
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();
diff --git a/clang/test/Analysis/stack-capture-leak-arc.mm b/clang/test/Analysis/stack-capture-leak-arc.mm
index 1ffee934c890b..921487a86ca4d 100644
--- a/clang/test/Analysis/stack-capture-leak-arc.mm
+++ b/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 @@ void test_dispatch_barrier_sync() {
}
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);
+ };
+}
diff --git a/clang/test/Analysis/stack-capture-leak-no-arc.mm b/clang/test/Analysis/stack-capture-leak-no-arc.mm
index 33829f52e727c..84bbc5fdc5253 100644
--- a/clang/test/Analysis/stack-capture-leak-no-arc.mm
+++ b/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 @@ dispatch_block_t test_block_inside_block_async_leak() {
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();
+}
More information about the cfe-commits
mailing list