The personality routine for the legacy AIX xlclang++ compiler uses the stack slot reserved for compilers to pass the exception object to the landing pad. The landing pad retrieves the exception object with a call to the runtime function __xlc_exception_handle(). The current implementation incorrectly assumes that __xlc_exception_handle() should go up one stack frame to get to the stack frame of the caller of __xlc_exception_handle(), which is supposedly the stack frame of the function containing the landing pad. However, this does not always work, e.g., the xlclang++ compiler sometimes generates a wrapper of __xlc_exception_handle() and calls the wrapper from the landing pad for optimization purposes. This patch changes the implementation to unwind the stack from __xlc_exception_handle() and skip frames not associated with functions that are C++ EH-aware until a frame associated with a C++ EH-aware function is found and then retrieving the exception object with the expectation that the located frame is the one that the personality routine installed as it transferred control to the landing pad.
Details
- Reviewers
ldionne MaskRay cebowleratibm hubert.reinterpretcast daltenty - Group Reviewers
Restricted Project - Commits
- rGb413b84a704e: [libc++abi][AIX] Skip non-C++ EH aware frames when retrieving exception object
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
libcxxabi/src/aix_state_tab_eh.inc | ||
---|---|---|
653 | unnecessary cast? | |
665 | I'm not convinced this can be removed. The implementation still moves up at least as many frames as it did before. | |
672 | is terminate the preferred solution? The exception won't be handled but we can't really say that the user code failed to catch the exception, rather an assertion failure condition has occurred in the runtime itself. My inclination was to call std::abort. | |
687–688 | Why the change to __builtin_frame_address? Does this simply read r1 as before? | |
687–688 | Suggest: just update callerStack rather than introduce a new variable. callerStack is dead afterwards as-is but the close names are just asking to introduce a future bug. | |
libcxxabi/test/vendor/ibm/aix_xlclang_passing_excp_obj_32.pass.sh.S | ||
30 | Should you also provide the t2.cpp source to demonstrate the wrap__xlc_exception_handle is a forwarder to __xlc_exception_handle? |
libcxxabi/src/aix_state_tab_eh.inc | ||
---|---|---|
665 | Suggest: add a print statement each time we skip a frame. In general this should only happen when the XL compiler injects a wrapper and it's probably of interest to the trace that we're bypassing wrapper frames enroute to the frame that contains the landing pad. |
libcxxabi/src/aix_state_tab_eh.inc | ||
---|---|---|
657 | Is there any possibility of encountering a function without traceback? |
libcxxabi/src/aix_state_tab_eh.inc | ||
---|---|---|
653 | Right, removed the cast, thanks! | |
665 | With the new implementation, __xlc_excpetion_handle() calls skipNonCxxEHAwareFrames(), which in turn calls std::abort(). force_a_stackframe() is no longer needed because __xlc_excpetion_handle() won't be a leaf function even if skipNonCxxEHAwareFrames() is inlined. I will add a comment in the code. | |
665 | I was dumping this info in the prototype but it caused timeouts when running a large application because the xlclang++ compiler generates so many calls to __xlc_exception_handle(), noting we've already added more code to skip the non-C++ EH frames there. To print the function name, we need to parse the traceback table to reach the function name field, in addition to printing it out, etc. I will look into implementing a verbose mode to only dump certain info later. | |
672 | Changed to std::abort() as suggested, thanks! | |
687–688 | Yes, __builtin_frame_address(0) is the same as read r1 as before. However, we now need the return address. We can use inline asm code for that too but the inline asm code needs 3 instructions and the instructions are different for 32- and 64-bit. So, __builtin_return_address(0) is used instead. I replaced the inline asm code for the frame address with __builtin_frame_address(0) to be consistent. | |
687–688 | Changed as suggested, thanks! | |
libcxxabi/test/vendor/ibm/aix_xlclang_passing_excp_obj_32.pass.sh.S | ||
30 | Good catch, thanks! |
Addressed comments:
- removed an unnecessary cast
- call std::abort() instead of std::terminate() when unwinding reaches the end of stack frames
- update callerStack instead of using another variable callerStack2
- added missing reference to t2.cpp source in the 32-bit test case
I made changes to the patch description (which I expect we will be using for the commit message).
libcxxabi/src/aix_state_tab_eh.inc | ||
---|---|---|
671 |
libcxxabi/src/aix_state_tab_eh.inc | ||
---|---|---|
667 | Suggestion: |
libcxxabi/src/aix_state_tab_eh.inc | ||
---|---|---|
665 | If we are in a debugger or have a core file, we might be able to map back to the function using just the program counter value. |
Changed function name from skipNonCxxEHAwareFrames to skip_non_cxx_eh_aware_frames to be consistent with function naming in snake_case in this file.
This patch is specific to the support for AIX legacy xlclang++ compiler generated code. It has been reviewed and approved by AIX compiler/runtime experts. If there are no objections, I will commit it tomorrow.
unnecessary cast?