From 7ba138abd329e591a8f6d5001f60dd7082f71b3b Mon Sep 17 00:00:00 2001 From: Steve Naroff Date: Tue, 3 Mar 2009 19:52:17 +0000 Subject: [PATCH] Fix Exception handling executes wrong clause (Daniel, please verify). Also necessary to fix: [sema] non object types should not be allowed in @catch statements [sema] qualified id should be disallowed in @catch statements git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@65964 91177308-0d34-0410-b5e6-96231b3b80d8 --- Driver/PrintParserCallbacks.cpp | 2 +- Driver/RewriteObjC.cpp | 8 ++++---- include/clang/AST/Stmt.h | 17 +++++++++------- include/clang/Parse/Action.h | 2 +- lib/AST/Stmt.cpp | 4 ++-- lib/AST/StmtPrinter.cpp | 6 +++--- lib/CodeGen/CGObjCMac.cpp | 36 ++++++++++++++++----------------- lib/Parse/ParseObjc.cpp | 23 +++++++++------------ lib/Sema/Sema.h | 2 +- lib/Sema/SemaStmt.cpp | 4 ++-- 10 files changed, 51 insertions(+), 53 deletions(-) diff --git a/Driver/PrintParserCallbacks.cpp b/Driver/PrintParserCallbacks.cpp index bc28db525b..b14f2660b8 100644 --- a/Driver/PrintParserCallbacks.cpp +++ b/Driver/PrintParserCallbacks.cpp @@ -382,7 +382,7 @@ namespace { // Objective-c statements virtual OwningStmtResult ActOnObjCAtCatchStmt(SourceLocation AtLoc, SourceLocation RParen, - StmtArg Parm, StmtArg Body, + DeclTy *Parm, StmtArg Body, StmtArg CatchList) { llvm::cout << __FUNCTION__ << "\n"; return StmtEmpty(); diff --git a/Driver/RewriteObjC.cpp b/Driver/RewriteObjC.cpp index 7293201166..c4f67627ce 100644 --- a/Driver/RewriteObjC.cpp +++ b/Driver/RewriteObjC.cpp @@ -1567,7 +1567,7 @@ Stmt *RewriteObjC::RewriteObjCTryStmt(ObjCAtTryStmt *S) { bool sawIdTypedCatch = false; Stmt *lastCatchBody = 0; while (catchList) { - Stmt *catchStmt = catchList->getCatchParamStmt(); + ParmVarDecl *catchDecl = catchList->getCatchParamDecl(); if (catchList == S->getCatchStmts()) buf = "if ("; // we are generating code for the first catch clause @@ -1592,8 +1592,8 @@ Stmt *RewriteObjC::RewriteObjCTryStmt(ObjCAtTryStmt *S) { buf += "1) { id _tmp = _caught;"; Rewrite.ReplaceText(startLoc, bodyBuf-startBuf+1, buf.c_str(), buf.size()); - } else if (DeclStmt *declStmt = dyn_cast(catchStmt)) { - QualType t = dyn_cast(declStmt->getSolitaryDecl())->getType(); + } else if (catchDecl) { + QualType t = catchDecl->getType(); if (t == Context->getObjCIdType()) { buf += "1) { "; ReplaceText(startLoc, lParenLoc-startBuf+1, buf.c_str(), buf.size()); @@ -1622,7 +1622,7 @@ Stmt *RewriteObjC::RewriteObjCTryStmt(ObjCAtTryStmt *S) { // Here we replace ") {" with "= _caught;" (which initializes and // declares the @catch parameter). ReplaceText(rParenLoc, bodyBuf-rParenBuf+1, buf.c_str(), buf.size()); - } else if (!isa(catchStmt)) { + } else { assert(false && "@catch rewrite bug"); } // make sure all the catch bodies get rewritten! diff --git a/include/clang/AST/Stmt.h b/include/clang/AST/Stmt.h index c4c2291f2c..0e2daed91c 100644 --- a/include/clang/AST/Stmt.h +++ b/include/clang/AST/Stmt.h @@ -31,6 +31,7 @@ namespace clang { class ASTContext; class Expr; class Decl; + class ParmVarDecl; class QualType; class IdentifierInfo; class SourceManager; @@ -1056,7 +1057,8 @@ public: /// ObjCAtCatchStmt - This represents objective-c's @catch statement. class ObjCAtCatchStmt : public Stmt { private: - enum { SELECTOR, BODY, NEXT_CATCH, END_EXPR }; + enum { BODY, NEXT_CATCH, END_EXPR }; + ParmVarDecl *ExceptionDecl; Stmt *SubExprs[END_EXPR]; SourceLocation AtCatchLoc, RParenLoc; @@ -1066,7 +1068,7 @@ private: public: ObjCAtCatchStmt(SourceLocation atCatchLoc, SourceLocation rparenloc, - DeclStmt *catchVarStmtDecl, + ParmVarDecl *catchVarDecl, Stmt *atCatchStmt, Stmt *atCatchList); const Stmt *getCatchBody() const { return SubExprs[BODY]; } @@ -1079,10 +1081,11 @@ public: return static_cast(SubExprs[NEXT_CATCH]); } - const DeclStmt *getCatchParamStmt() const { - return static_cast(SubExprs[SELECTOR]); } - DeclStmt *getCatchParamStmt() { - return static_cast(SubExprs[SELECTOR]); + const ParmVarDecl *getCatchParamDecl() const { + return ExceptionDecl; + } + ParmVarDecl *getCatchParamDecl() { + return ExceptionDecl; } SourceLocation getRParenLoc() const { return RParenLoc; } @@ -1091,7 +1094,7 @@ public: return SourceRange(AtCatchLoc, SubExprs[BODY]->getLocEnd()); } - bool hasEllipsis() const { return getCatchParamStmt() == 0; } + bool hasEllipsis() const { return getCatchParamDecl() == 0; } static bool classof(const Stmt *T) { return T->getStmtClass() == ObjCAtCatchStmtClass; diff --git a/include/clang/Parse/Action.h b/include/clang/Parse/Action.h index 9b4e9802dc..e1ed5cd109 100644 --- a/include/clang/Parse/Action.h +++ b/include/clang/Parse/Action.h @@ -500,7 +500,7 @@ public: // Objective-c statements virtual OwningStmtResult ActOnObjCAtCatchStmt(SourceLocation AtLoc, SourceLocation RParen, - StmtArg Parm, StmtArg Body, + DeclTy *Parm, StmtArg Body, StmtArg CatchList) { return StmtEmpty(); } diff --git a/lib/AST/Stmt.cpp b/lib/AST/Stmt.cpp index e987d84c23..63d3e7f7b4 100644 --- a/lib/AST/Stmt.cpp +++ b/lib/AST/Stmt.cpp @@ -180,10 +180,10 @@ ObjCForCollectionStmt::ObjCForCollectionStmt(Stmt *Elem, Expr *Collect, ObjCAtCatchStmt::ObjCAtCatchStmt(SourceLocation atCatchLoc, SourceLocation rparenloc, - DeclStmt *catchVarStmtDecl, Stmt *atCatchStmt, + ParmVarDecl *catchVarDecl, Stmt *atCatchStmt, Stmt *atCatchList) : Stmt(ObjCAtCatchStmtClass) { - SubExprs[SELECTOR] = catchVarStmtDecl; + ExceptionDecl = catchVarDecl; SubExprs[BODY] = atCatchStmt; SubExprs[NEXT_CATCH] = NULL; // FIXME: O(N^2) in number of catch blocks. diff --git a/lib/AST/StmtPrinter.cpp b/lib/AST/StmtPrinter.cpp index 7e25b465b1..e6ac310b83 100644 --- a/lib/AST/StmtPrinter.cpp +++ b/lib/AST/StmtPrinter.cpp @@ -448,9 +448,9 @@ void StmtPrinter::VisitObjCAtTryStmt(ObjCAtTryStmt *Node) { catchStmt = static_cast(catchStmt->getNextCatchStmt())) { Indent() << "@catch("; - if (catchStmt->getCatchParamStmt()) { - if (DeclStmt *DS = dyn_cast(catchStmt->getCatchParamStmt())) - PrintRawDeclStmt(DS); + if (catchStmt->getCatchParamDecl()) { + if (Decl *DS = catchStmt->getCatchParamDecl()) + PrintRawDecl(DS); } OS << ")"; if (CompoundStmt *CS = dyn_cast(catchStmt->getCatchBody())) diff --git a/lib/CodeGen/CGObjCMac.cpp b/lib/CodeGen/CGObjCMac.cpp index 0f38eb94bc..bbbed3fe2b 100644 --- a/lib/CodeGen/CGObjCMac.cpp +++ b/lib/CodeGen/CGObjCMac.cpp @@ -2014,30 +2014,28 @@ void CGObjCMac::EmitTryOrSynchronizedStmt(CodeGen::CodeGenFunction &CGF, for (; CatchStmt; CatchStmt = CatchStmt->getNextCatchStmt()) { llvm::BasicBlock *NextCatchBlock = CGF.createBasicBlock("catch"); - const DeclStmt *CatchParam = CatchStmt->getCatchParamStmt(); - const VarDecl *VD = 0; + const ParmVarDecl *CatchParam = CatchStmt->getCatchParamDecl(); const PointerType *PT = 0; // catch(...) always matches. if (!CatchParam) { AllMatched = true; } else { - VD = cast(CatchParam->getSolitaryDecl()); - PT = VD->getType()->getAsPointerType(); + PT = CatchParam->getType()->getAsPointerType(); // catch(id e) always matches. // FIXME: For the time being we also match id; this should // be rejected by Sema instead. if ((PT && CGF.getContext().isObjCIdStructType(PT->getPointeeType())) || - VD->getType()->isObjCQualifiedIdType()) + CatchParam->getType()->isObjCQualifiedIdType()) AllMatched = true; } if (AllMatched) { if (CatchParam) { - CGF.EmitStmt(CatchParam); + CGF.EmitLocalBlockVarDecl(*CatchParam); assert(CGF.HaveInsertPoint() && "DeclStmt destroyed insert point?"); - CGF.Builder.CreateStore(Caught, CGF.GetAddrOfLocalVar(VD)); + CGF.Builder.CreateStore(Caught, CGF.GetAddrOfLocalVar(CatchParam)); } CGF.EmitStmt(CatchStmt->getCatchBody()); @@ -2063,13 +2061,13 @@ void CGObjCMac::EmitTryOrSynchronizedStmt(CodeGen::CodeGenFunction &CGF, // Emit the @catch block. CGF.EmitBlock(MatchedBlock); - CGF.EmitStmt(CatchParam); + CGF.EmitLocalBlockVarDecl(*CatchParam); assert(CGF.HaveInsertPoint() && "DeclStmt destroyed insert point?"); llvm::Value *Tmp = - CGF.Builder.CreateBitCast(Caught, CGF.ConvertType(VD->getType()), + CGF.Builder.CreateBitCast(Caught, CGF.ConvertType(CatchParam->getType()), "tmp"); - CGF.Builder.CreateStore(Tmp, CGF.GetAddrOfLocalVar(VD)); + CGF.Builder.CreateStore(Tmp, CGF.GetAddrOfLocalVar(CatchParam)); CGF.EmitStmt(CatchStmt->getCatchBody()); CGF.EmitBranchThroughCleanup(FinallyEnd); @@ -4778,17 +4776,17 @@ CGObjCNonFragileABIMac::EmitTryOrSynchronizedStmt(CodeGen::CodeGenFunction &CGF, SelectorArgs.push_back(ObjCTypes.EHPersonalityPtr); // Construct the lists of (type, catch body) to handle. - llvm::SmallVector, 8> Handlers; + llvm::SmallVector, 8> Handlers; bool HasCatchAll = false; if (isTry) { if (const ObjCAtCatchStmt* CatchStmt = cast(S).getCatchStmts()) { for (; CatchStmt; CatchStmt = CatchStmt->getNextCatchStmt()) { - const DeclStmt *DS = CatchStmt->getCatchParamStmt(); - Handlers.push_back(std::make_pair(DS, CatchStmt->getCatchBody())); + const Decl *CatchDecl = CatchStmt->getCatchParamDecl(); + Handlers.push_back(std::make_pair(CatchDecl, CatchStmt->getCatchBody())); // catch(...) always matches. - if (!DS) { + if (!CatchDecl) { // Use i8* null here to signal this is a catch all, not a cleanup. llvm::Value *Null = llvm::Constant::getNullValue(ObjCTypes.Int8PtrTy); SelectorArgs.push_back(Null); @@ -4796,7 +4794,7 @@ CGObjCNonFragileABIMac::EmitTryOrSynchronizedStmt(CodeGen::CodeGenFunction &CGF, break; } - const VarDecl *VD = cast(DS->getSolitaryDecl()); + const VarDecl *VD = cast(CatchDecl); if (CGF.getContext().isObjCIdType(VD->getType()) || VD->getType()->isObjCQualifiedIdType()) { llvm::Value *IDEHType = @@ -4826,7 +4824,7 @@ CGObjCNonFragileABIMac::EmitTryOrSynchronizedStmt(CodeGen::CodeGenFunction &CGF, // We use a cleanup unless there was already a catch all. if (!HasCatchAll) { SelectorArgs.push_back(llvm::ConstantInt::get(llvm::Type::Int32Ty, 0)); - Handlers.push_back(std::make_pair((const DeclStmt*) 0, (const Stmt*) 0)); + Handlers.push_back(std::make_pair((const Decl*) 0, (const Stmt*) 0)); } llvm::Value *Selector = @@ -4834,7 +4832,7 @@ CGObjCNonFragileABIMac::EmitTryOrSynchronizedStmt(CodeGen::CodeGenFunction &CGF, SelectorArgs.begin(), SelectorArgs.end(), "selector"); for (unsigned i = 0, e = Handlers.size(); i != e; ++i) { - const DeclStmt *CatchParam = Handlers[i].first; + const Decl *CatchParam = Handlers[i].first; const Stmt *CatchBody = Handlers[i].second; llvm::BasicBlock *Next = 0; @@ -4871,10 +4869,10 @@ CGObjCNonFragileABIMac::EmitTryOrSynchronizedStmt(CodeGen::CodeGenFunction &CGF, // Bind the catch parameter if it exists. if (CatchParam) { - const VarDecl *VD = cast(CatchParam->getSolitaryDecl()); + const VarDecl *VD = dyn_cast(CatchParam); ExcObject = CGF.Builder.CreateBitCast(ExcObject, CGF.ConvertType(VD->getType())); - CGF.EmitStmt(CatchParam); + CGF.EmitLocalBlockVarDecl(*VD); CGF.Builder.CreateStore(ExcObject, CGF.GetAddrOfLocalVar(VD)); } diff --git a/lib/Parse/ParseObjc.cpp b/lib/Parse/ParseObjc.cpp index 193a73f664..3637d144d6 100644 --- a/lib/Parse/ParseObjc.cpp +++ b/lib/Parse/ParseObjc.cpp @@ -1285,7 +1285,7 @@ Parser::OwningStmtResult Parser::ParseObjCTryStmt(SourceLocation atLoc) { SourceLocation AtCatchFinallyLoc = ConsumeToken(); if (Tok.isObjCAtKeyword(tok::objc_catch)) { - OwningStmtResult FirstPart(Actions); + DeclTy *FirstPart = 0; ConsumeToken(); // consume catch if (Tok.is(tok::l_paren)) { ConsumeParen(); @@ -1294,17 +1294,14 @@ Parser::OwningStmtResult Parser::ParseObjCTryStmt(SourceLocation atLoc) { DeclSpec DS; ParseDeclarationSpecifiers(DS); // For some odd reason, the name of the exception variable is - // optional. As a result, we need to use PrototypeContext. - Declarator DeclaratorInfo(DS, Declarator::PrototypeContext); - ParseDeclarator(DeclaratorInfo); - if (DeclaratorInfo.getIdentifier()) { - DeclTy *aBlockVarDecl = Actions.ActOnDeclarator(CurScope, - DeclaratorInfo, 0); - FirstPart = - Actions.ActOnDeclStmt(aBlockVarDecl, - DS.getSourceRange().getBegin(), - DeclaratorInfo.getSourceRange().getEnd()); - } + // optional. As a result, we need to use "PrototypeContext", because + // we must accept either 'declarator' or 'abstract-declarator' here. + Declarator ParmDecl(DS, Declarator::PrototypeContext); + ParseDeclarator(ParmDecl); + + // Inform the actions module about the parameter declarator, so it + // gets added to the current scope. + FirstPart = Actions.ActOnParamDeclarator(CurScope, ParmDecl); } else ConsumeToken(); // consume '...' SourceLocation RParenLoc = ConsumeParen(); @@ -1317,7 +1314,7 @@ Parser::OwningStmtResult Parser::ParseObjCTryStmt(SourceLocation atLoc) { if (CatchBody.isInvalid()) CatchBody = Actions.ActOnNullStmt(Tok.getLocation()); CatchStmts = Actions.ActOnObjCAtCatchStmt(AtCatchFinallyLoc, - RParenLoc, move(FirstPart), move(CatchBody), + RParenLoc, FirstPart, move(CatchBody), move(CatchStmts)); } else { Diag(AtCatchFinallyLoc, diag::err_expected_lparen_after) diff --git a/lib/Sema/Sema.h b/lib/Sema/Sema.h index 77f2a3d8b6..deff504864 100644 --- a/lib/Sema/Sema.h +++ b/lib/Sema/Sema.h @@ -1054,7 +1054,7 @@ public: virtual OwningStmtResult ActOnObjCAtCatchStmt(SourceLocation AtLoc, SourceLocation RParen, - StmtArg Parm, StmtArg Body, + DeclTy *Parm, StmtArg Body, StmtArg CatchList); virtual OwningStmtResult ActOnObjCAtFinallyStmt(SourceLocation AtLoc, diff --git a/lib/Sema/SemaStmt.cpp b/lib/Sema/SemaStmt.cpp index e7dff6d5b3..49bd85a9e7 100644 --- a/lib/Sema/SemaStmt.cpp +++ b/lib/Sema/SemaStmt.cpp @@ -962,11 +962,11 @@ Sema::OwningStmtResult Sema::ActOnAsmStmt(SourceLocation AsmLoc, Action::OwningStmtResult Sema::ActOnObjCAtCatchStmt(SourceLocation AtLoc, - SourceLocation RParen, StmtArg Parm, + SourceLocation RParen, DeclTy *Parm, StmtArg Body, StmtArg catchList) { Stmt *CatchList = static_cast(catchList.release()); ObjCAtCatchStmt *CS = new (Context) ObjCAtCatchStmt(AtLoc, RParen, - static_cast(Parm.release()), static_cast(Body.release()), + static_cast(Parm), static_cast(Body.release()), CatchList); return Owned(CatchList ? CatchList : CS); } -- 2.40.0