diff --git a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp --- a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp @@ -61,7 +61,6 @@ ASTContext &Ctx); static SmallVector 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(R); -} - bool StackAddrEscapeChecker::isNotInCurrentFrame(const MemRegion *R, CheckerContext &C) { const StackSpaceRegion *S = cast(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(R)) checkReturnedBlockCaptures(*B, C); - if (!isa(R->getMemorySpace()) || - isNotInCurrentFrame(R, C) || isArcManagedBlock(R, C)) + if (!isa(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(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 --- a/clang/lib/StaticAnalyzer/Core/MemRegion.cpp +++ b/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(); diff --git a/clang/test/Analysis/stack-capture-leak-arc.mm b/clang/test/Analysis/stack-capture-leak-arc.mm --- 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 @@ } 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 --- 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 @@ 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(); +}