From 12849d0ed2e88f8c6420944538b9c9d1985df14a Mon Sep 17 00:00:00 2001 From: Douglas Gregor Date: Mon, 8 Apr 2013 20:52:24 +0000 Subject: [PATCH] Fix a crasher when an Objective-C for-in loop gets a non-variable iteration declaration. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@179053 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/Basic/DiagnosticSemaKinds.td | 4 ++- lib/Parse/ParseDecl.cpp | 23 ++++++++++----- lib/Sema/SemaStmt.cpp | 33 ++++++++++++++++------ test/SemaObjCXX/foreach.mm | 6 ++++ 4 files changed, 50 insertions(+), 16 deletions(-) diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index e47c9a4277..95c9c804f3 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -6029,8 +6029,10 @@ def ext_mixed_decls_code : Extension< "ISO C90 forbids mixing declarations and code">, InGroup>; -def err_non_variable_decl_in_for : Error< +def err_non_local_variable_decl_in_for : Error< "declaration of non-local variable in 'for' loop">; +def err_non_variable_decl_in_for : Error< + "non-variable declaration in 'for' loop">; def err_toomany_element_decls : Error< "only one element declaration is allowed">; def err_selector_element_not_lvalue : Error< diff --git a/lib/Parse/ParseDecl.cpp b/lib/Parse/ParseDecl.cpp index 990a9097ac..1979bb12cd 100644 --- a/lib/Parse/ParseDecl.cpp +++ b/lib/Parse/ParseDecl.cpp @@ -1527,14 +1527,23 @@ Parser::DeclGroupPtrTy Parser::ParseDeclGroup(ParsingDeclSpec &DS, // C++0x [stmt.iter]p1: Check if we have a for-range-declarator. If so, we // must parse and analyze the for-range-initializer before the declaration is // analyzed. - if (FRI && Tok.is(tok::colon)) { - FRI->ColonLoc = ConsumeToken(); - if (Tok.is(tok::l_brace)) - FRI->RangeExpr = ParseBraceInitializer(); - else - FRI->RangeExpr = ParseExpression(); + // + // Handle the Objective-C for-in loop variable similarly, although we + // don't need to parse the container in advance. + if (FRI && (Tok.is(tok::colon) || isTokIdentifier_in())) { + bool IsForRangeLoop = false; + if (Tok.is(tok::colon)) { + IsForRangeLoop = true; + FRI->ColonLoc = ConsumeToken(); + if (Tok.is(tok::l_brace)) + FRI->RangeExpr = ParseBraceInitializer(); + else + FRI->RangeExpr = ParseExpression(); + } + Decl *ThisDecl = Actions.ActOnDeclarator(getCurScope(), D); - Actions.ActOnCXXForRangeDecl(ThisDecl); + if (IsForRangeLoop) + Actions.ActOnCXXForRangeDecl(ThisDecl); Actions.FinalizeDeclaration(ThisDecl); D.complete(ThisDecl); return Actions.FinalizeDeclaratorGroup(getCurScope(), DS, &ThisDecl, 1); diff --git a/lib/Sema/SemaStmt.cpp b/lib/Sema/SemaStmt.cpp index dea01d5be2..b3558a548b 100644 --- a/lib/Sema/SemaStmt.cpp +++ b/lib/Sema/SemaStmt.cpp @@ -76,10 +76,23 @@ StmtResult Sema::ActOnDeclStmt(DeclGroupPtrTy dg, SourceLocation StartLoc, void Sema::ActOnForEachDeclStmt(DeclGroupPtrTy dg) { DeclGroupRef DG = dg.getAsVal(); + + // If we don't have a declaration, or we have an invalid declaration, + // just return. + if (DG.isNull() || !DG.isSingleDecl()) + return; + + Decl *decl = DG.getSingleDecl(); + if (!decl || decl->isInvalidDecl()) + return; - // If we have an invalid decl, just return. - if (DG.isNull() || !DG.isSingleDecl()) return; - VarDecl *var = cast(DG.getSingleDecl()); + // Only variable declarations are permitted. + VarDecl *var = dyn_cast(decl); + if (!var) { + Diag(decl->getLocation(), diag::err_non_variable_decl_in_for); + decl->setInvalidDecl(); + return; + } // suppress any potential 'unused variable' warning. var->setUsed(); @@ -1426,9 +1439,10 @@ Sema::ActOnForStmt(SourceLocation ForLoc, SourceLocation LParenLoc, VarDecl *VD = dyn_cast(*DI); if (VD && VD->isLocalVarDecl() && !VD->hasLocalStorage()) VD = 0; - if (VD == 0) - Diag((*DI)->getLocation(), diag::err_non_variable_decl_in_for); - // FIXME: mark decl erroneous! + if (VD == 0) { + Diag((*DI)->getLocation(), diag::err_non_local_variable_decl_in_for); + (*DI)->setInvalidDecl(); + } } } } @@ -1562,14 +1576,17 @@ Sema::ActOnObjCForCollectionStmt(SourceLocation ForLoc, return StmtError(Diag((*DS->decl_begin())->getLocation(), diag::err_toomany_element_decls)); - VarDecl *D = cast(DS->getSingleDecl()); + VarDecl *D = dyn_cast(DS->getSingleDecl()); + if (!D || D->isInvalidDecl()) + return StmtError(); + FirstType = D->getType(); // C99 6.8.5p3: The declaration part of a 'for' statement shall only // declare identifiers for objects having storage class 'auto' or // 'register'. if (!D->hasLocalStorage()) return StmtError(Diag(D->getLocation(), - diag::err_non_variable_decl_in_for)); + diag::err_non_local_variable_decl_in_for)); // If the type contained 'auto', deduce the 'auto' to 'id'. if (FirstType->getContainedAutoType()) { diff --git a/test/SemaObjCXX/foreach.mm b/test/SemaObjCXX/foreach.mm index ac787bd09e..d1302c19a5 100644 --- a/test/SemaObjCXX/foreach.mm +++ b/test/SemaObjCXX/foreach.mm @@ -69,3 +69,9 @@ void test2(NSObject *collection) { // expected-warning {{property access result unused - getters should not be used for side effects}} } } + +void testErrors(NSArray *array) { + typedef int fn(int); + + for (fn x in array) { } // expected-error{{non-variable declaration in 'for' loop}} +} -- 2.40.0