PNaCl: Change exception info format to distinguish catch-all/cleanup clauses

The initial version of ExceptionInfoWriter.cpp encodes a landingpad's
"cleanup" clause the same as "catch i8* null" (a catch-all).

But it turns out we do want to distinguish these two: If an uncaught
exception occurs, we don't want the runtime to run C++ destructors
(cleanups), because this involves unwinding the stack to jump to
landingpads, which loses the context of where the uncaught exception
was thrown from, which is unhelpful for debugging.  (The C++ standard
says it's optional whether destructors are run when an uncaught
exception occurs.)

So, change the encoding as follows:

 * Use 0 as the ID for "cleanup" clauses.
 * Add 1 to the IDs of types and "catch" clauses.  (Adding 1 to both
   seems neater than just one of them.)
 * This means we can use 0 for the terminator of filter lists instead
   of -1, which seems a little neater.

This requires a corresponding change to eh_pnacl.cc in libsupc++.

BUG=https://code.google.com/p/nativeclient/issues/detail?id=3696
TEST=NaCl's EH tests with libsupc++ change applied

Review URL: https://codereview.chromium.org/69923008

2 files changed, 39 insertions, 31 deletions

diff --git a/lib/Transforms/NaCl/ExceptionInfoWriter.cpp b/lib/Transforms/NaCl/ExceptionInfoWriter.cpp
index 7fbf5245c4..ca88384003 100644
--- a/lib/Transforms/NaCl/ExceptionInfoWriter.cpp
+++ b/lib/Transforms/NaCl/ExceptionInfoWriter.cpp
@@ -34,8 +34,8 @@
 //       type.
 //     * Clang generates this for a "catch" block in the C++ source.
 //     * @ExcType is NULL for "catch (...)" (catch-all) blocks.
-//     * This is encoded as the integer "type ID" @ExcType, X,
-//       such that: __pnacl_eh_type_table[X] == @ExcType, and X >= 0.
+//     * This is encoded as the "type ID" for @ExcType, defined below,
+//       which is a positive integer.
 //
 //  2) "filter [i8* @ExcType1, ..., i8* @ExcTypeN]"
 //     * This clause means that the landingpad should be entered if
@@ -45,14 +45,24 @@
 //     * Clang uses this to implement C++ exception specifications, e.g.
 //          void foo() throw(ExcType1, ..., ExcTypeN) { ... }
 //     * This is encoded as the filter ID, X, where X < 0, and
-//       &__pnacl_eh_filter_table[-X-1] points to a -1-terminated
+//       &__pnacl_eh_filter_table[-X-1] points to a 0-terminated
 //       array of integer "type IDs".
 //
 //  3) "cleanup"
 //     * This means that the landingpad should always be entered.
 //     * Clang uses this for calling objects' destructors.
-//     * ExceptionInfoWriter encodes this the same as "catch i8* null"
-//       (which is a catch-all).
+//     * This is encoded as 0.
+//     * The runtime may treat "cleanup" differently from "catch i8*
+//       null" (a catch-all).  In C++, if an unhandled exception
+//       occurs, the language runtime may abort execution without
+//       running any destructors.  The runtime may implement this by
+//       searching for a matching non-"cleanup" clause, and aborting
+//       if it does not find one, before entering any landingpad
+//       blocks.
+//
+// The "type ID" for a type @ExcType is a 1-based index into the array
+// __pnacl_eh_type_table[].  That is, the type ID is a value X such
+// that __pnacl_eh_type_table[X-1] == @ExcType, and X >= 1.
 //
 // ExceptionInfoWriter generates the following data structures:
 //
@@ -68,7 +78,7 @@
 //   extern std::type_info *const __pnacl_eh_type_table[];
 //
 //   // Used to represent type arrays for "filter" clauses.
-//   extern const int32_t __pnacl_eh_filter_table[];
+//   extern const uint32_t __pnacl_eh_filter_table[];
 //
 // A "clause list ID" is either:
 //  * 0, representing the empty list; or
@@ -78,20 +88,20 @@
 // Example:
 //
 //   std::type_info *const __pnacl_eh_type_table[] = {
-//     // defines type ID 0 == ExcA and clause ID 0 == "catch ExcA"
+//     // defines type ID 1 == ExcA and clause ID 1 == "catch ExcA"
 //     &typeinfo(ExcA),
-//     // defines type ID 1 == ExcB and clause ID 1 == "catch ExcB"
+//     // defines type ID 2 == ExcB and clause ID 2 == "catch ExcB"
 //     &typeinfo(ExcB),
-//     // defines type ID 2 == ExcC and clause ID 2 == "catch ExcC"
+//     // defines type ID 3 == ExcC and clause ID 3 == "catch ExcC"
 //     &typeinfo(ExcC),
 //   };
 //
-//   const int32_t __pnacl_eh_filter_table[] = {
-//     0,   // refers to ExcA;  defines clause ID -1 as "filter [ExcA, ExcB]"
-//     1,   // refers to ExcB;  defines clause ID -2 as "filter [ExcB]"
-//     -1,  // list terminator; defines clause ID -3 as "filter []"
-//     2,   // refers to ExcC;  defines clause ID -4 as "filter [ExcC]"
-//     -1,  // list terminator; defines clause ID -5 as "filter []"
+//   const uint32_t __pnacl_eh_filter_table[] = {
+//     1,  // refers to ExcA;  defines clause ID -1 as "filter [ExcA, ExcB]"
+//     2,  // refers to ExcB;  defines clause ID -2 as "filter [ExcB]"
+//     0,  // list terminator; defines clause ID -3 as "filter []"
+//     3,  // refers to ExcC;  defines clause ID -4 as "filter [ExcC]"
+//     0,  // list terminator; defines clause ID -5 as "filter []"
 //   };
 //
 //   const struct action_table_entry __pnacl_eh_action_table[] = {
@@ -107,12 +117,12 @@
 //     },
 //     // defines clause list ID 3:
 //     {
-//       1,  // "catch ExcB"
+//       2,  // "catch ExcB"
 //       2,  // else go to clause list ID 2
 //     },
 //     // defines clause list ID 4:
 //     {
-//       0,  // "catch ExcA"
+//       1,  // "catch ExcA"
 //       3,  // else go to clause list ID 3
 //     },
 //   };
@@ -165,7 +175,7 @@ unsigned ExceptionInfoWriter::getIDForExceptionType(Value *ExcTy) {
   if (Iter != TypeTableIDMap.end())
     return Iter->second;
 
-  unsigned Index = TypeTableData.size();
+  unsigned Index = TypeTableData.size() + 1;
   TypeTableIDMap[ExcTyConst] = Index;
   TypeTableData.push_back(ExcTyConst);
   return Index;
@@ -206,11 +216,12 @@ unsigned ExceptionInfoWriter::getIDForFilterClause(Value *Filter) {
       report_fatal_error("Landingpad filter clause is not a ConstantArray");
     for (unsigned I = 0; I < FilterLength; ++I) {
       unsigned TypeID = getIDForExceptionType(Array->getOperand(I));
+      assert(TypeID > 0);
       FilterTableData.push_back(ConstantInt::get(I32, TypeID));
     }
   }
   // Add array terminator.
-  FilterTableData.push_back(ConstantInt::get(I32, -1));
+  FilterTableData.push_back(ConstantInt::get(I32, 0));
   return FilterClauseID;
 }
 
@@ -218,11 +229,8 @@ unsigned ExceptionInfoWriter::getIDForLandingPadClauseList(LandingPadInst *LP) {
   unsigned NextClauseListID = 0;  // ID for empty list.
 
   if (LP->isCleanup()) {
-    // Add catch-all entry.  There doesn't appear to be any need to
-    // treat "cleanup" differently from a catch-all.
-    unsigned TypeID = getIDForExceptionType(
-        ConstantPointerNull::get(Type::getInt8PtrTy(*Context)));
-    NextClauseListID = getIDForClauseListNode(TypeID, NextClauseListID);
+    // Add cleanup clause at the end of the list.
+    NextClauseListID = getIDForClauseListNode(0, NextClauseListID);
   }
 
   for (int I = (int) LP->getNumClauses() - 1; I >= 0; --I) {
@@ -234,6 +242,7 @@ unsigned ExceptionInfoWriter::getIDForLandingPadClauseList(LandingPadInst *LP) {
     } else {
       report_fatal_error("Unknown kind of landingpad clause");
     }
+    assert(ClauseID > 0);
     NextClauseListID = getIDForClauseListNode(ClauseID, NextClauseListID);
   }
 
diff --git a/test/Transforms/NaCl/pnacl-eh-exception-info.ll b/test/Transforms/NaCl/pnacl-eh-exception-info.ll
index a330f6f5ac..478bc97d3f 100644
--- a/test/Transforms/NaCl/pnacl-eh-exception-info.ll
+++ b/test/Transforms/NaCl/pnacl-eh-exception-info.ll
@@ -21,9 +21,9 @@ declare void @external_func()
 
 ; CHECK: @__pnacl_eh_type_table = internal constant [4 x i8*] [i8* @exc_typeid1, i8* @exc_typeid2, i8* @exc_typeid3, i8* null]
 
-; CHECK: @__pnacl_eh_action_table = internal constant [6 x %action_table_entry] [%action_table_entry { i32 2, i32 0 }, %action_table_entry { i32 1, i32 1 }, %action_table_entry { i32 0, i32 2 }, %action_table_entry { i32 -1, i32 0 }, %action_table_entry { i32 -2, i32 0 }, %action_table_entry { i32 3, i32 0 }]
+; CHECK: @__pnacl_eh_action_table = internal constant [7 x %action_table_entry] [%action_table_entry { i32 3, i32 0 }, %action_table_entry { i32 2, i32 1 }, %action_table_entry { i32 1, i32 2 }, %action_table_entry { i32 -1, i32 0 }, %action_table_entry { i32 -2, i32 0 }, %action_table_entry { i32 4, i32 0 }, %action_table_entry zeroinitializer]
 
-; CHECK: @__pnacl_eh_filter_table = internal constant [5 x i32] [i32 -1, i32 1, i32 2, i32 0, i32 -1]
+; CHECK: @__pnacl_eh_filter_table = internal constant [5 x i32] [i32 0, i32 2, i32 3, i32 1, i32 0]
 
 
 ; Exception type pointers are allocated IDs which specify the index
@@ -38,9 +38,9 @@ define void @test_eh_typeid(i32 %arg) {
   ret void
 }
 ; CHECK: define void @test_eh_typeid
-; CHECK-NEXT: %cmp1 = icmp eq i32 %arg, 0
-; CHECK-NEXT: %cmp2 = icmp eq i32 %arg, 1
-; CHECK-NEXT: %cmp3 = icmp eq i32 %arg, 2
+; CHECK-NEXT: %cmp1 = icmp eq i32 %arg, 1
+; CHECK-NEXT: %cmp2 = icmp eq i32 %arg, 2
+; CHECK-NEXT: %cmp3 = icmp eq i32 %arg, 3
 ; CHECK-NEXT: ret void
 
 
@@ -114,7 +114,6 @@ lpad:
 ; CHECK: store i32 6, i32* %exc_info_ptr
 
 
-; "cleanup" is treated the same as "catch i8* null".
 define void @test_cleanup_clause() {
   invoke void @external_func() to label %cont unwind label %lpad
 cont:
@@ -125,4 +124,4 @@ lpad:
   ret void
 }
 ; CHECK: define void @test_cleanup_clause
-; CHECK: store i32 6, i32* %exc_info_ptr
+; CHECK: store i32 7, i32* %exc_info_ptr