[PATCH] Implement ARM EHABI exception handling.

Jon Roelofs jonathan at codesourcery.com
Mon May 5 13:34:53 PDT 2014


Thanks for working on this Logan!

================
Comment at: include/unwind.h:28
@@ +27,3 @@
+#define LIBCXXABI_ARM_EHABI 1
+#endif
+
----------------
#else
#define LIBCXXABI_ARM_EHABI 0
#endif

================
Comment at: include/unwind.h:67
@@ +66,3 @@
+struct _Unwind_Control_Block {
+  uint64_t exception_class;
+  void (*exception_cleanup)(_Unwind_Reason_Code, _Unwind_Control_Block*);
----------------
#7.2 says this should be:
char exception_class[8];

Maybe add a FIXME here for that?

================
Comment at: src/cxa_exception.cpp:311
@@ +310,3 @@
+*/
+__attribute__((visibility("hidden"))) _Unwind_Exception *
+__cxa_end_cleanup_impl()
----------------
Is this a public interface? or should it be static?

================
Comment at: src/cxa_exception.cpp:340
@@ +339,3 @@
+asm (
+    "	.pushsection	.text.__cxa_end_cleanup\n"
+    "	.globl	__cxa_end_cleanup\n"
----------------
Is _Unwind_Resume going to need a manual entry for the .ARM.exidx section for this function so that _Unwind_Resume can find it?

I think it just needs to be a Su16 entry with 3 Finish opcodes.

================
Comment at: src/cxa_exception.cpp:343
@@ +342,3 @@
+    "	.type	__cxa_end_cleanup,%function\n"
+    "__cxa_end_cleanup:\n"
+    "	push	{r1, r2, r3, r4}\n"
----------------
Does this need a
".globl __cxa_end_cleanup\n"
line to make it visible to ld (or does the .type line take care of that)?

================
Comment at: src/cxa_exception.cpp:344
@@ +343,3 @@
+    "__cxa_end_cleanup:\n"
+    "	push	{r1, r2, r3, r4}\n"
+    "	bl	__cxa_end_cleanup_impl\n"
----------------
I'm not sure from reading #8.4.1 and #7.4.6 why this is r1-r4 and not r0-r3... r0 is callee save and likely to get clobbered by the call to isOurExceptionClass in __cxa_end_cleanup_impl, no?

================
Comment at: src/cxa_personality.cpp:324
@@ +323,3 @@
+{
+    if (classInfo == 0)
+    {
----------------
Should we assert that the ttypeEncoding is DW_EH_PE_absptr?

================
Comment at: src/cxa_personality.cpp:396
@@ +395,3 @@
+    //    adjustments to adjustedPtr are ignored.
+    for (const void** ttypePtr = reinterpret_cast<const void**>(table); *ttypePtr; ++ttypePtr)
+    {
----------------
Maybe add a comment here to highlight the difference between ARM EHABI and the Itanium ABI, and explain why the implementation needs to be different. On ARM, these are 0-delimited runs of TARGET2 relocation pointers to the typeinfos, whereas in the rest of the Itanium EH ABI, these are 0-delimited runs of uleb128 indexes into the type table.

================
Comment at: src/cxa_personality.cpp:712
@@ -658,3 +711,3 @@
                             // If this is a type 3 search and _UA_FORCE_UNWIND, ignore handler and continue scan
-                            if (actions & _UA_SEARCH_PHASE)
+                            if ((actions & _UA_SEARCH_PHASE) || (actions & _UA_HANDLER_FRAME))
                             {
----------------
Maybe the part added here should go under #if LIBCXXABI_ARM_EHABI ?

================
Comment at: src/cxa_personality.cpp:756
@@ -702,3 +755,3 @@
                             // If this is a type 3 search _UA_FORCE_UNWIND, ignore handler and continue scan
-                            if (actions & _UA_SEARCH_PHASE)
+                            if ((actions & _UA_SEARCH_PHASE) || (actions & _UA_HANDLER_FRAME))
                             {
----------------
Maybe the part added here should go under #if LIBCXXABI_ARM_EHABI ?

http://reviews.llvm.org/D3613






More information about the cfe-commits mailing list