[PATCH] Implement ARM EHABI exception handling.

[Jon Roelofs]

Changes LGTM. Might be a good idea for kledzik or nbjoerg to have a look too, but post-commit review is probably fine for that.

================
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*);
----------------
Logan Chien wrote:
> Jon Roelofs wrote:
> > #7.2 says this should be:
> > char exception_class[8];
> > 
> > Maybe add a FIXME here for that?
> I am not sure whether we are going to fix this.  There has been several pieces of code using integer arithmetic to determinate whether this is a native exception or foreign exception.  I can add the comment if you insist.
Ok. It's not super important to me.

================
Comment at: src/cxa_exception.cpp:311
@@ +310,3 @@
+*/
+__attribute__((visibility("hidden"))) _Unwind_Exception *
+__cxa_end_cleanup_impl()
----------------
Logan Chien wrote:
> Jon Roelofs wrote:
> > Is this a public interface? or should it be static?
> No this is not a public interface.  When I was writing this code, I have tried to use `static`.  However, this function will be ignored by compiler because there is no function call to this static function.
> 
> But now, I have found `__attribute__((used))`, which can solve the issue. I will change this in the next revision.
Ok. Sounds good.

================
Comment at: src/cxa_exception.cpp:340
@@ +339,3 @@
+asm (
+    "	.pushsection	.text.__cxa_end_cleanup\n"
+    "	.globl	__cxa_end_cleanup\n"
----------------
Logan Chien wrote:
> Jon Roelofs wrote:
> > 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.
> `__cxa_end_cleanup()` is a bridge for the cleanup handler to return to unwinder.  Thus, we don't have to unwind the stack containing from the cleanup handler.  In fact, `_Unwind_Resume` will load the information from `_Unwind_Control_Block`, which contains the actual stack to be unwinded.
> 
> Thus, it is fine for not having `.fnstart` and `.fnend` directives here.
Oh, right.

================
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"
----------------
Logan Chien wrote:
> Jon Roelofs wrote:
> > 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)?
> I am not sure about this, but this is what clang generates for `extern` functions.  I guess .globl is for the linkage and .type is for the ELF symbol type, thus we need both of them.
Ok. Doing what clang does for `extern`s sounds good to me.

================
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"
----------------
Logan Chien wrote:
> Jon Roelofs wrote:
> > 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?
> r0-r3 are caller save registers.  However, r0 will (and should) be covered by the return value, thus we don't have to push them to stack.  On the other hand, r4 is only pushed for stack alignment.
Ohhhhh, I see now.

================
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))
                             {
----------------
Logan Chien wrote:
> Jon Roelofs wrote:
> > Maybe the part added here should go under #if LIBCXXABI_ARM_EHABI ?
> These won't be needed and will be removed in next revision.  (I have fixed the TODO in the phase 2 search.  Thus, we don't have to perform the search again.)  Thanks.
Ok.

http://reviews.llvm.org/D3613