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

[Daniel Kiss via Phabricator via libcxx-commits]

danielkiss added inline comments.


================
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"
----------------
rprichard wrote:
> 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.
> 
clobbering r4 is not in line with the aapcs. so it could be like this (not tested yet):

```
  push	{r1, r2, r3, lr}
  push	{r4}
  bl	__cxa_end_cleanup_impl
  pop {r1, r2, r3, r4}
  mov lr, r4
  pop {r4}

```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111703