Fix <rdar://problem/6640991> Exception handling executes wrong clause (Daniel, please verify).

Also necessary to fix:

<rdar://problem/6632061> [sema] non object types should not be allowed in @catch statements
<rdar://problem/6252237> [sema] qualified id should be disallowed in @catch statements

llvm-svn: 65964

6 files changed, 35 insertions, 40 deletions

diff --git a/clang/lib/AST/Stmt.cpp b/clang/lib/AST/Stmt.cpp
index e987d84c231..63d3e7f7b4b 100644
--- a/clang/lib/AST/Stmt.cpp
+++ b/clang/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/clang/lib/AST/StmtPrinter.cpp b/clang/lib/AST/StmtPrinter.cpp
index 7e25b465b12..e6ac310b835 100644
--- a/clang/lib/AST/StmtPrinter.cpp
+++ b/clang/lib/AST/StmtPrinter.cpp
@@ -448,9 +448,9 @@ void StmtPrinter::VisitObjCAtTryStmt(ObjCAtTryStmt *Node) {
        catchStmt = 
          static_cast<ObjCAtCatchStmt *>(catchStmt->getNextCatchStmt())) {
     Indent() << "@catch(";
-    if (catchStmt->getCatchParamStmt()) {
-      if (DeclStmt *DS = dyn_cast<DeclStmt>(catchStmt->getCatchParamStmt()))
-        PrintRawDeclStmt(DS);
+    if (catchStmt->getCatchParamDecl()) {
+      if (Decl *DS = catchStmt->getCatchParamDecl())
+        PrintRawDecl(DS);
     }
     OS << ")";
     if (CompoundStmt *CS = dyn_cast<CompoundStmt>(catchStmt->getCatchBody())) 
diff --git a/clang/lib/CodeGen/CGObjCMac.cpp b/clang/lib/CodeGen/CGObjCMac.cpp
index 0f38eb94bc8..bbbed3fe2b1 100644
--- a/clang/lib/CodeGen/CGObjCMac.cpp
+++ b/clang/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<VarDecl>(CatchParam->getSolitaryDecl());
-        PT = VD->getType()->getAsPointerType();
+        PT = CatchParam->getType()->getAsPointerType();
         
         // catch(id e) always matches. 
         // FIXME: For the time being we also match id<X>; 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<std::pair<const DeclStmt*, const Stmt*>, 8> Handlers;
+  llvm::SmallVector<std::pair<const Decl*, const Stmt*>, 8> Handlers;
   bool HasCatchAll = false;
   if (isTry) {
     if (const ObjCAtCatchStmt* CatchStmt =
         cast<ObjCAtTryStmt>(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<VarDecl>(DS->getSolitaryDecl());
+        const VarDecl *VD = cast<VarDecl>(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<VarDecl>(CatchParam->getSolitaryDecl());
+        const VarDecl *VD = dyn_cast<VarDecl>(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/clang/lib/Parse/ParseObjc.cpp b/clang/lib/Parse/ParseObjc.cpp
index 193a73f664d..3637d144d63 100644
--- a/clang/lib/Parse/ParseObjc.cpp
+++ b/clang/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/clang/lib/Sema/Sema.h b/clang/lib/Sema/Sema.h
index 77f2a3d8b69..deff5048648 100644
--- a/clang/lib/Sema/Sema.h
+++ b/clang/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/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index e7dff6d5b32..49bd85a9e7e 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/clang/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<Stmt*>(catchList.release());
   ObjCAtCatchStmt *CS = new (Context) ObjCAtCatchStmt(AtLoc, RParen,
-    static_cast<DeclStmt*>(Parm.release()), static_cast<Stmt*>(Body.release()),
+    static_cast<ParmVarDecl*>(Parm), static_cast<Stmt*>(Body.release()),
     CatchList);
   return Owned(CatchList ? CatchList : CS);
 }