[libcxx-commits] [PATCH] D111703: [ARM] __cxa_end_cleanup should be called instead of _UnwindResume.

[Ryan Prichard via Phabricator via libcxx-commits]

rprichard added inline comments.
Herald added a project: All.


================
Comment at: libcxxabi/src/cxa_exception.cpp:383
     "	bl	__cxa_end_cleanup_impl\n"
-    "	pop	{r1, r2, r3, r4}\n"
-    "	bl	_Unwind_Resume\n"
-    "	bl	abort\n"
-    "	.popsection"
-);
+    "	pop	{r1, r2, r3, lr}\n"
+    "	b	_Unwind_Resume\n"
----------------
chill wrote:
> Could/should be
> ```
> pop {r1, r2, r3, r4}
> mov lr, r4
> ```
> then it'll be compatible with Armv6-M and Armv8.0-M.base.
> 
@danielkiss @chill 

The comment above `__cxa_end_cleanup` says:

> According to ARM EHABI 8.4.1, __cxa_end_cleanup() should not clobber any
> register, thus we have to write this function in assembly so that we can save
> {r1, r2, r3}.

We're currently saving/restoring r1-r3 (volatiles), but with the Armv6-M fix, we're clobbering r4 (a local/callee-save register) in order to restore lr. That seems like wrong behavior?

i.e. `__cxa_end_cleanup` should ensure r4 has the original value when it calls `_Unwind_Resume`. Before this LLVM patch, it wasn't necessary to preserve r4, because `__cxa_end_cleanup_impl` would do so automatically.



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111703/new/

https://reviews.llvm.org/D111703