[libcxx-commits] [PATCH] D100504: [libc++abi][AIX] add personality and helper functions for the state table EH

[Hubert Tong via Phabricator via libcxx-commits]

hubert.reinterpretcast added inline comments.


================
Comment at: libcxxabi/src/cxa_personality.cpp:1490
+                            reinterpret_cast<void*>(fsmEntry));
+  try {
+    _LIBCXXABI_TRACE_STATETAB("..calling delete()\n");
----------------
MaskRay wrote:
> MaskRay wrote:
> > I don't think the personality impl is supposed to use `try catch`. You may use other mechanisms to avoid try catch.
> You can use something like `st_terminate` in cxa_vector.cpp to catch exceptions in destructors.
@MaskRay, is there a specific reason you can give for why `try`/`catch` should not be used here?

It may be worth noting that the expected build compiler for this code uses a personality routine that is not the one being implemented here. It may also be worth noting that this is less part of the personality routine and more an action that could be thought of as taking place in the corresponding application frame's context (except that, instead of transferring control to code in that frame, the encoded action is called from the personality routine).

That a search phase triggered by throwing an exception during a stack unwinding action finds this `try`/`catch` seems compatible with the behaviour seen with other implementations: Unwinding for the new exception happens until the stack unwinding action actually exits via an exception.

It is not the case that entry into this personality routine necessarily means that a new search phase will find a frame with a handler. It is a desirable property that this `try`/`catch` is present for the new search phase to consider that there is a handler (and thus perform unwinding).

As for the `st_terminate` suggestion, the current usage of `st_terminate` seems to be the source of https://github.com/llvm/llvm-project/issues/55032. That is, entering a handler prior to the call to `terminate`, as the code here does, is desirable.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100504