[cfe-commits] r134906 - in /cfe/trunk: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/JumpDiagnostics.cpp lib/Sema/SemaExpr.cpp test/SemaObjC/arc-jump-block.m
Fariborz Jahanian
fjahanian at apple.com
Mon Jul 11 11:04:54 PDT 2011
Author: fjahanian
Date: Mon Jul 11 13:04:54 2011
New Revision: 134906
URL: http://llvm.org/viewvc/llvm-project?rev=134906&view=rev
Log:
objc-arc: Diagnose when captured variable in block literals
require destruction and there is possibility of that without
construction. Thanks Johnm for review and suggestions offline.
// rdar://9535237.
Added:
cfe/trunk/test/SemaObjC/arc-jump-block.m
Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/JumpDiagnostics.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=134906&r1=134905&r2=134906&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Mon Jul 11 13:04:54 2011
@@ -2511,6 +2511,12 @@
"jump bypasses setup of __block variable">;
def note_protected_by_objc_ownership : Note<
"jump bypasses initialization of retaining variable">;
+def note_enters_block_captures_cxx_obj : Note<
+ "jump enters lifetime of block which captures a destructible c++ object">;
+def note_enters_block_captures_strong : Note<
+ "jump enters lifetime of block which strongly captures a variable">;
+def note_enters_block_captures_weak : Note<
+ "jump enters lifetime of block which weakly captures a variable">;
def note_exits_cleanup : Note<
"jump exits scope of variable with __attribute__((cleanup))">;
@@ -2534,6 +2540,12 @@
"jump exits autoreleasepool block">;
def note_exits_objc_ownership : Note<
"jump exits scope of retaining variable">;
+def note_exits_block_captures_cxx_obj : Note<
+ "jump exits lifetime of block which captures a destructible c++ object">;
+def note_exits_block_captures_strong : Note<
+ "jump exits lifetime of block which strongly captures a variable">;
+def note_exits_block_captures_weak : Note<
+ "jump exits lifetime of block which weakly captures a variable">;
def err_func_returning_array_function : Error<
"function cannot return %select{array|function}0 type %1">;
Modified: cfe/trunk/lib/Sema/JumpDiagnostics.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/JumpDiagnostics.cpp?rev=134906&r1=134905&r2=134906&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/JumpDiagnostics.cpp (original)
+++ cfe/trunk/lib/Sema/JumpDiagnostics.cpp Mon Jul 11 13:04:54 2011
@@ -68,7 +68,10 @@
JumpScopeChecker(Stmt *Body, Sema &S);
private:
void BuildScopeInformation(Decl *D, unsigned &ParentScope);
- void BuildScopeInformation(Stmt *S, unsigned ParentScope);
+ void BuildScopeInformation(VarDecl *D, const BlockDecl *BDecl,
+ unsigned &ParentScope);
+ void BuildScopeInformation(Stmt *S, unsigned &origParentScope);
+
void VerifyJumps();
void VerifyIndirectJumps();
void DiagnoseIndirectJump(IndirectGotoStmt *IG, unsigned IGScope,
@@ -87,7 +90,8 @@
// Build information for the top level compound statement, so that we have a
// defined scope record for every "goto" and label.
- BuildScopeInformation(Body, 0);
+ unsigned BodyParentScope = 0;
+ BuildScopeInformation(Body, BodyParentScope);
// Check that all jumps we saw are kosher.
VerifyJumps();
@@ -228,11 +232,55 @@
BuildScopeInformation(Init, ParentScope);
}
+/// \brief Build scope information for a captured block literal variables.
+void JumpScopeChecker::BuildScopeInformation(VarDecl *D,
+ const BlockDecl *BDecl,
+ unsigned &ParentScope) {
+ // exclude captured __block variables; there's no destructor
+ // associated with the block literal for them.
+ if (D->hasAttr<BlocksAttr>())
+ return;
+ QualType T = D->getType();
+ QualType::DestructionKind destructKind = T.isDestructedType();
+ if (destructKind != QualType::DK_none) {
+ std::pair<unsigned,unsigned> Diags;
+ switch (destructKind) {
+ case QualType::DK_cxx_destructor:
+ Diags = ScopePair(diag::note_enters_block_captures_cxx_obj,
+ diag::note_exits_block_captures_cxx_obj);
+ break;
+ case QualType::DK_objc_strong_lifetime:
+ Diags = ScopePair(diag::note_enters_block_captures_strong,
+ diag::note_exits_block_captures_strong);
+ break;
+ case QualType::DK_objc_weak_lifetime:
+ Diags = ScopePair(diag::note_enters_block_captures_weak,
+ diag::note_exits_block_captures_weak);
+ break;
+ case QualType::DK_none:
+ llvm_unreachable("no-liftime captured variable");
+ }
+ SourceLocation Loc = D->getLocation();
+ if (Loc.isInvalid())
+ Loc = BDecl->getLocation();
+ Scopes.push_back(GotoScope(ParentScope,
+ Diags.first, Diags.second, Loc));
+ ParentScope = Scopes.size()-1;
+ }
+}
+
/// BuildScopeInformation - The statements from CI to CE are known to form a
/// coherent VLA scope with a specified parent node. Walk through the
/// statements, adding any labels or gotos to LabelAndGotoScopes and recursively
/// walking the AST as needed.
-void JumpScopeChecker::BuildScopeInformation(Stmt *S, unsigned ParentScope) {
+void JumpScopeChecker::BuildScopeInformation(Stmt *S, unsigned &origParentScope) {
+ // If this is a statement, rather than an expression, scopes within it don't
+ // propagate out into the enclosing scope. Otherwise we have to worry
+ // about block literals, which have the lifetime of their enclosing statement.
+ unsigned independentParentScope = origParentScope;
+ unsigned &ParentScope = ((isa<Expr>(S) && !isa<StmtExpr>(S))
+ ? origParentScope : independentParentScope);
+
bool SkipFirstSubStmt = false;
// If we found a label, remember that it is in ParentScope scope.
@@ -314,17 +362,17 @@
BuildScopeInformation(*I, ParentScope);
continue;
}
-
// Disallow jumps into any part of an @try statement by pushing a scope and
// walking all sub-stmts in that scope.
if (ObjCAtTryStmt *AT = dyn_cast<ObjCAtTryStmt>(SubStmt)) {
+ unsigned newParentScope;
// Recursively walk the AST for the @try part.
Scopes.push_back(GotoScope(ParentScope,
diag::note_protected_by_objc_try,
diag::note_exits_objc_try,
AT->getAtTryLoc()));
if (Stmt *TryPart = AT->getTryBody())
- BuildScopeInformation(TryPart, Scopes.size()-1);
+ BuildScopeInformation(TryPart, (newParentScope = Scopes.size()-1));
// Jump from the catch to the finally or try is not valid.
for (unsigned I = 0, N = AT->getNumCatchStmts(); I != N; ++I) {
@@ -334,7 +382,8 @@
diag::note_exits_objc_catch,
AC->getAtCatchLoc()));
// @catches are nested and it isn't
- BuildScopeInformation(AC->getCatchBody(), Scopes.size()-1);
+ BuildScopeInformation(AC->getCatchBody(),
+ (newParentScope = Scopes.size()-1));
}
// Jump from the finally to the try or catch is not valid.
@@ -343,12 +392,13 @@
diag::note_protected_by_objc_finally,
diag::note_exits_objc_finally,
AF->getAtFinallyLoc()));
- BuildScopeInformation(AF, Scopes.size()-1);
+ BuildScopeInformation(AF, (newParentScope = Scopes.size()-1));
}
continue;
}
-
+
+ unsigned newParentScope;
// Disallow jumps into the protected statement of an @synchronized, but
// allow jumps into the object expression it protects.
if (ObjCAtSynchronizedStmt *AS = dyn_cast<ObjCAtSynchronizedStmt>(SubStmt)){
@@ -362,7 +412,8 @@
diag::note_protected_by_objc_synchronized,
diag::note_exits_objc_synchronized,
AS->getAtSynchronizedLoc()));
- BuildScopeInformation(AS->getSynchBody(), Scopes.size()-1);
+ BuildScopeInformation(AS->getSynchBody(),
+ (newParentScope = Scopes.size()-1));
continue;
}
@@ -374,7 +425,7 @@
diag::note_exits_cxx_try,
TS->getSourceRange().getBegin()));
if (Stmt *TryBlock = TS->getTryBlock())
- BuildScopeInformation(TryBlock, Scopes.size()-1);
+ BuildScopeInformation(TryBlock, (newParentScope = Scopes.size()-1));
// Jump from the catch into the try is not allowed either.
for (unsigned I = 0, E = TS->getNumHandlers(); I != E; ++I) {
@@ -383,7 +434,8 @@
diag::note_protected_by_cxx_catch,
diag::note_exits_cxx_catch,
CS->getSourceRange().getBegin()));
- BuildScopeInformation(CS->getHandlerBlock(), Scopes.size()-1);
+ BuildScopeInformation(CS->getHandlerBlock(),
+ (newParentScope = Scopes.size()-1));
}
continue;
@@ -397,10 +449,19 @@
diag::note_protected_by_objc_autoreleasepool,
diag::note_exits_objc_autoreleasepool,
AS->getAtLoc()));
- BuildScopeInformation(AS->getSubStmt(), Scopes.size()-1);
+ BuildScopeInformation(AS->getSubStmt(), (newParentScope = Scopes.size()-1));
continue;
}
-
+
+ if (const BlockExpr *BE = dyn_cast<BlockExpr>(SubStmt)) {
+ const BlockDecl *BDecl = BE->getBlockDecl();
+ for (BlockDecl::capture_const_iterator ci = BDecl->capture_begin(),
+ ce = BDecl->capture_end(); ci != ce; ++ci) {
+ VarDecl *variable = ci->getVariable();
+ BuildScopeInformation(variable, BDecl, ParentScope);
+ }
+ }
+
// Recursively walk the AST.
BuildScopeInformation(SubStmt, ParentScope);
}
Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=134906&r1=134905&r2=134906&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Mon Jul 11 13:04:54 2011
@@ -8501,6 +8501,15 @@
const AnalysisBasedWarnings::Policy &WP = AnalysisWarnings.getDefaultPolicy();
PopFunctionOrBlockScope(&WP, Result->getBlockDecl(), Result);
+ for (BlockDecl::capture_const_iterator ci = BSI->TheDecl->capture_begin(),
+ ce = BSI->TheDecl->capture_end(); ci != ce; ++ci) {
+ const VarDecl *variable = ci->getVariable();
+ QualType T = variable->getType();
+ QualType::DestructionKind destructKind = T.isDestructedType();
+ if (destructKind != QualType::DK_none)
+ getCurFunction()->setHasBranchProtectedScope();
+ }
+
return Owned(Result);
}
Added: cfe/trunk/test/SemaObjC/arc-jump-block.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/arc-jump-block.m?rev=134906&view=auto
==============================================================================
--- cfe/trunk/test/SemaObjC/arc-jump-block.m (added)
+++ cfe/trunk/test/SemaObjC/arc-jump-block.m Mon Jul 11 13:04:54 2011
@@ -0,0 +1,84 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin11 -fobjc-nonfragile-abi -fsyntax-only -fobjc-arc -fblocks -verify %s
+// rdar://9535237
+
+typedef struct dispatch_queue_s *dispatch_queue_t;
+
+typedef void (^dispatch_block_t)(void);
+
+void dispatch_async(dispatch_queue_t queue, dispatch_block_t block);
+
+extern __attribute__((visibility("default"))) struct dispatch_queue_s _dispatch_main_q;
+
+ at interface SwitchBlockCrashAppDelegate
+- (void)pageLeft;
+- (void)pageRight;;
+ at end
+
+ at implementation SwitchBlockCrashAppDelegate
+
+- (void)choose:(int)button {
+ switch (button) {
+ case 0:
+ dispatch_async((&_dispatch_main_q), ^{ [self pageLeft]; }); // expected-note 3 {{jump enters lifetime of block which strongly captures a variable}}
+ break;
+ case 2: // expected-error {{switch case is in protected scope}}
+ dispatch_async((&_dispatch_main_q), ^{ [self pageRight]; }); // expected-note 2 {{jump enters lifetime of block which strongly captures a variable}}
+ break;
+ case 3: // expected-error {{switch case is in protected scope}}
+ {
+ dispatch_async((&_dispatch_main_q), ^{ [self pageRight]; });
+ break;
+ }
+ case 4: // expected-error {{switch case is in protected scope}}
+ break;
+ }
+
+ __block SwitchBlockCrashAppDelegate *captured_block_obj;
+ switch (button) {
+ case 10:
+ {
+ dispatch_async((&_dispatch_main_q), ^{ [self pageLeft]; });
+ break;
+ }
+ case 12:
+ if (button)
+ dispatch_async((&_dispatch_main_q), ^{ [captured_block_obj pageRight]; });
+ break;
+ case 13:
+ while (button)
+ dispatch_async((&_dispatch_main_q), ^{ [self pageRight]; });
+ break;
+ case 14:
+ break;
+ }
+
+ switch (button) {
+ case 10:
+ {
+ dispatch_async((&_dispatch_main_q), ^{ [self pageLeft]; });
+ break;
+ }
+ case 12:
+ if (button)
+ dispatch_async((&_dispatch_main_q), ^{ [self pageRight]; });
+ switch (button) {
+ case 0:
+ {
+ dispatch_async((&_dispatch_main_q), ^{ [self pageLeft]; });
+ break;
+ }
+ case 4:
+ break;
+ }
+ break;
+ case 13:
+ while (button)
+ dispatch_async((&_dispatch_main_q), ^{ [self pageRight]; });
+ break;
+ case 14:
+ break;
+ }
+}
+- (void)pageLeft {}
+- (void)pageRight {}
+ at end
More information about the cfe-commits
mailing list