Improve validation of C++ exception handling: diagnose throwing incomplete types and jumps into protected try-catch scopes.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@70242 91177308-0d34-0410-b5e6-96231b3b80d8

include/clang/Basic/DiagnosticSemaKinds.td

@@ -866,6 +866,10 @@ def note_protected_by_objc_finally : Note<
   "jump bypasses initialization of @finally block">;
 def note_protected_by_objc_synchronized : Note<
   "jump bypasses initialization of @synchronized block">;
+def note_protected_by_cxx_try : Note<
+  "jump bypasses initialization of try block">;
+def note_protected_by_cxx_catch : Note<
+  "jump bypasses initialization of catch block">;
 
 def err_func_returning_array_function : Error<
   "function cannot return array or function type %0">;
@@ -1196,6 +1200,11 @@ def err_conditional_ambiguous_ovl : Error<
   "conditional expression is ambiguous; %0 and %1 can be converted to several "
   "common types">;
 
+def err_throw_incomplete : Error<
+  "cannot throw object of incomplete type %0">;
+def err_throw_incomplete_ptr : Error<
+  "cannot throw pointer to object of incomplete type %0">;
+
 def err_invalid_use_of_function_type : Error<
   "a function type is not allowed here">;
 def err_invalid_use_of_array_type : Error<"an array type is not allowed here">;

lib/Sema/JumpDiagnostics.cpp

@@ -15,6 +15,7 @@
 #include "Sema.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/StmtObjC.h"
+#include "clang/AST/StmtCXX.h"
 using namespace clang;
 
 namespace {
@@ -115,7 +116,6 @@ void JumpScopeChecker::BuildScopeInformation(Stmt *S, unsigned ParentScope) {
     
     // FIXME: diagnose jumps past initialization: required in C++, warning in C.
     //   goto L; int X = 4;   L: ;
-    // FIXME: what about jumps into C++ catch blocks, what are the rules?
 
     // If this is a declstmt with a VLA definition, it defines a scope from here
     // to the end of the containing context.
@@ -184,7 +184,27 @@ void JumpScopeChecker::BuildScopeInformation(Stmt *S, unsigned ParentScope) {
       BuildScopeInformation(AS->getSynchBody(), Scopes.size()-1);
       continue;
     }
-    
+
+    // Disallow jumps into any part of a C++ try statement. This is pretty
+    // much the same as for Obj-C.
+    if (CXXTryStmt *TS = dyn_cast<CXXTryStmt>(SubStmt)) {
+      Scopes.push_back(GotoScope(ParentScope, diag::note_protected_by_cxx_try,
+                                 TS->getSourceRange().getBegin()));
+      if (Stmt *TryBlock = TS->getTryBlock())
+        BuildScopeInformation(TryBlock, Scopes.size()-1);
+
+      // Jump from the catch into the try is not allowed either.
+      for(unsigned I = 0, E = TS->getNumHandlers(); I != E; ++I) {
+        CXXCatchStmt *CS = TS->getHandler(I);
+        Scopes.push_back(GotoScope(ParentScope,
+                                   diag::note_protected_by_cxx_catch,
+                                   CS->getSourceRange().getBegin()));
+        BuildScopeInformation(CS->getHandlerBlock(), Scopes.size()-1);
+      }
+
+      continue;
+    }
+
     // Recursively walk the AST.
     BuildScopeInformation(SubStmt, ParentScope);
   }

lib/Sema/Sema.h

@@ -1494,6 +1494,7 @@ public:
   //// ActOnCXXThrow -  Parse throw expressions.
   virtual OwningExprResult ActOnCXXThrow(SourceLocation OpLoc,
                                          ExprArg expr);
+  bool CheckCXXThrowOperand(SourceLocation ThrowLoc, Expr *&E);
 
   /// ActOnCXXTypeConstructExpr - Parse construction of a specified type.
   /// Can be interpreted either as function-style casting ("int(x)")

lib/Sema/SemaExprCXX.cpp

@@ -88,8 +88,37 @@ Sema::ActOnCXXBoolLiteral(SourceLocation OpLoc, tok::TokenKind Kind) {
 /// ActOnCXXThrow - Parse throw expressions.
 Action::OwningExprResult
 Sema::ActOnCXXThrow(SourceLocation OpLoc, ExprArg E) {
-  return Owned(new (Context) CXXThrowExpr((Expr*)E.release(), Context.VoidTy,
-                                          OpLoc));
+  Expr *Ex = E.takeAs<Expr>();
+  if (Ex && !Ex->isTypeDependent() && CheckCXXThrowOperand(OpLoc, Ex))
+    return ExprError();
+  return Owned(new (Context) CXXThrowExpr(Ex, Context.VoidTy, OpLoc));
+}
+
+/// CheckCXXThrowOperand - Validate the operand of a throw.
+bool Sema::CheckCXXThrowOperand(SourceLocation ThrowLoc, Expr *&E) {
+  // C++ [except.throw]p3:
+  //   [...] adjusting the type from "array of T" or "function returning T"
+  //   to "pointer to T" or "pointer to function returning T", [...]
+  DefaultFunctionArrayConversion(E);
+
+  //   If the type of the exception would be an incomplete type or a pointer
+  //   to an incomplete type other than (cv) void the program is ill-formed.
+  QualType Ty = E->getType();
+  int isPointer = 0;
+  if (const PointerType* Ptr = Ty->getAsPointerType()) {
+    Ty = Ptr->getPointeeType();
+    isPointer = 1;
+  }
+  if (!isPointer || !Ty->isVoidType()) {
+    if (RequireCompleteType(ThrowLoc, Ty,
+                            isPointer ? diag::err_throw_incomplete_ptr
+                                      : diag::err_throw_incomplete,
+                            E->getSourceRange(), SourceRange(), QualType()))
+      return true;
+  }
+
+  // FIXME: Construct a temporary here.
+  return false;
 }
 
 Action::OwningExprResult Sema::ActOnCXXThis(SourceLocation ThisLoc) {

lib/Sema/SemaStmt.cpp

@@ -1156,6 +1156,7 @@ Sema::ActOnCXXTryBlock(SourceLocation TryLoc, StmtArg TryBlock,
   // Neither of these are explicitly forbidden, but every compiler detects them
   // and warns.
 
+  CurFunctionNeedsScopeChecking = true;
   RawHandlers.release();
   return Owned(new (Context) CXXTryStmt(TryLoc,
                                         static_cast<Stmt*>(TryBlock.release()),

test/SemaCXX/exceptions.cpp

@@ -0,0 +1,67 @@
+// RUN: clang-cc -fsyntax-only -verify %s
+
+struct A; // expected-note 4 {{forward declaration of 'struct A'}}
+
+void trys() {
+  try {
+  } catch(int i) { // expected-note {{previous definition}}
+    int j = i;
+    int i; // expected-error {{redefinition of 'i'}}
+  } catch(float i) {
+  } catch(void v) { // expected-error {{cannot catch incomplete type 'void'}}
+  } catch(A a) { // expected-error {{cannot catch incomplete type 'struct A'}}
+  } catch(A *a) { // expected-error {{cannot catch pointer to incomplete type 'struct A'}}
+  } catch(A &a) { // expected-error {{cannot catch reference to incomplete type 'struct A'}}
+  } catch(...) {
+    int j = i; // expected-error {{use of undeclared identifier 'i'}}
+  }
+
+  try {
+  } catch(...) { // expected-error {{catch-all handler must come last}}
+  } catch(int) {
+  }
+}
+
+void throws() {
+  throw;
+  throw 0;
+  throw throw; // expected-error {{cannot throw object of incomplete type 'void'}}
+  throw (A*)0; // expected-error {{cannot throw pointer to object of incomplete type 'struct A'}}
+}
+
+void jumps() {
+l1:
+  goto l5;
+  goto l4; // expected-error {{illegal goto into protected scope}}
+  goto l3; // expected-error {{illegal goto into protected scope}}
+  goto l2; // expected-error {{illegal goto into protected scope}}
+  goto l1;
+  try { // expected-note 4 {{jump bypasses initialization of try block}}
+  l2:
+    goto l5;
+    goto l4; // expected-error {{illegal goto into protected scope}}
+    goto l3; // expected-error {{illegal goto into protected scope}}
+    goto l2;
+    goto l1;
+  } catch(int) { // expected-note 4 {{jump bypasses initialization of catch block}}
+  l3:
+    goto l5;
+    goto l4; // expected-error {{illegal goto into protected scope}}
+    goto l3;
+    goto l2; // expected-error {{illegal goto into protected scope}}
+    goto l1;
+  } catch(...) { // expected-note 4 {{jump bypasses initialization of catch block}}
+  l4:
+    goto l5;
+    goto l4;
+    goto l3; // expected-error {{illegal goto into protected scope}}
+    goto l2; // expected-error {{illegal goto into protected scope}}
+    goto l1;
+  }
+l5:
+  goto l5;
+  goto l4; // expected-error {{illegal goto into protected scope}}
+  goto l3; // expected-error {{illegal goto into protected scope}}
+  goto l2; // expected-error {{illegal goto into protected scope}}
+  goto l1;
+}

test/SemaCXX/try-catch.cpp

@@ -1,24 +0,0 @@
-// RUN: clang-cc -fsyntax-only -verify %s
-
-struct A; // expected-note 3 {{forward declaration of 'struct A'}}
-
-void f()
-{
-  try {
-  } catch(int i) { // expected-note {{previous definition}}
-    int j = i;
-    int i; // expected-error {{redefinition of 'i'}}
-  } catch(float i) {
-  } catch(void v) { // expected-error {{cannot catch incomplete type 'void'}}
-  } catch(A a) { // expected-error {{cannot catch incomplete type 'struct A'}}
-  } catch(A *a) { // expected-error {{cannot catch pointer to incomplete type 'struct A'}}
-  } catch(A &a) { // expected-error {{cannot catch reference to incomplete type 'struct A'}}
-  } catch(...) {
-    int j = i; // expected-error {{use of undeclared identifier 'i'}}
-  }
-
-  try {
-  } catch(...) { // expected-error {{catch-all handler must come last}}
-  } catch(int) {
-  }
-}

www/cxx_status.html

@@ -1638,7 +1638,7 @@ welcome!</p>
   <td>15 [except]</td>
   <td class="complete" align="center">&#x2713;</td>
   <td class="complete" align="center">&#x2713;</td>
-  <td class="advanced" align="center">Illegal gotos are not diagnosed</td>
+  <td class="complete" align="center">&#x2713;</td>
   <td></td>
   <td></td>
 </tr>
@@ -1646,9 +1646,9 @@ welcome!</p>
   <td>&nbsp;&nbsp;15.1 [except.throw]</td>
   <td class="na" align="center">N/A</td>
   <td class="na" align="center">N/A</td>
-  <td class="advanced" align="center">Does not check for existence of copy constructor and destructor, and some other details</td>
-  <td></td>
+  <td class="advanced" align="center"></td>
   <td></td>
+  <td>Does not check for existence of copy constructor and destructor, and some other details</td>
 </tr>
 <tr>
   <td>&nbsp;&nbsp;15.2 [except.ctor]</td>
@@ -1662,9 +1662,9 @@ welcome!</p>
   <td>&nbsp;&nbsp;15.3 [except.handle]</td>
   <td class="na" align="center">N/A</td>
   <td class="na" align="center">N/A</td>
-  <td class="advanced" align="center">Not all constraints are checked, such as existence of return statements in function-try-block handlers of constructors</td>
-  <td></td>
+  <td class="advanced" align="center"></td>
   <td></td>
+  <td>Not all constraints are checked, such as existence of return statements in function-try-block handlers of constructors</td>
 </tr>
 <tr>
   <td>&nbsp;&nbsp;15.4 [except.spec]</td>
@@ -1710,9 +1710,9 @@ welcome!</p>
   <td>&nbsp;&nbsp;15.6 [except.access]</td>
   <td class="na" align="center">N/A</td>
   <td class="na" align="center">N/A</td>
-  <td class="na" align="center">Redundant - struck from C++0x</td>
   <td class="na" align="center">N/A</td>
-  <td></td>
+  <td class="na" align="center">N/A</td>
+  <td>Redundant - struck from C++0x</td>
 </tr>
 <tr><td>16 [cpp]</td><td></td><td></td><td></td><td></td><td></td></tr>
 <tr><td>&nbsp;&nbsp;16.1 [cpp.cond]</td><td></td><td></td><td></td><td></td><td></td></tr>