[libc++abi] Simplify __gxx_personality_v0

[MaskRay]

Summary

In three cases we call scan_eh_tab to parse LSDA:

• actions & _UA_SEARCH_PHASE
• actions & _UA_CLEANUP_PHASE && actions & _UA_HANDLER_FRAME && !native_exception
• actions & _UA_CLEANUP_PHASE && !(actions & _UA_HANDLER_FRAME)

Check `actions & _UA_CLEANUP_PHASE && actions & _UA_HANDLER_FRAME &&
native_exception` first, then we can move three scan_eh_tab onto one place.

Another simplification is that we can check whether the result of scan_eh_tab
is _UA_CONTINUE_UNWIND or _UA_FATAL_PHASE1_ERROR first. Then many of the
original checks will be dead and can thus be deleted.

[MaskRay]

😳

[MaskRay]

😳

[ldionne]

So, just to clarify because I'm really not familiar with this code -- this is supposed to be a NFC change, right? My understanding so far is that this doesn't change functionality, but I'd like to confirm. Also, do you know someone else that knows this code and can give a good review?

libcxxabi/src/cxa_personality.cpp

969

I don't think it hurts readability to leave it as __cxa_exception* exception_header, despite the cast.

[MaskRay]

auto* -> __cxa_exception*

[MaskRay]

In D93186#2485076, @ldionne wrote:

So, just to clarify because I'm really not familiar with this code -- this is supposed to be a NFC change, right? My understanding so far is that this doesn't change functionality, but I'd like to confirm. Also, do you know someone else that knows this code and can give a good review?

Yes. Both this and D93190 are NFC. check-cxxabi (libcxxabi/test/*catch* libcxxabi/test/*except*) is actually quite comprehensive.
The two main functions are mostly intact Howard after wrote them in 2012 (@mstorsjo has at least added the SEH part so may be familiar).
If it helps, I've written down the simplified version of scan_eh_tab and __gxx_personality_v0 (what they should be like) in https://maskray.me/blog/2020-12-12-c++-exception-handling-abi

Both libsupc++ and libcxxrt are simpler than our version. With the two refactoring our version should be less verbose.

[mstorsjo]

FWIW, while I've touched this code on the outside, I'm not really familiar with it. But I can at least try out the patch in my setups (dwarf + SEH) and let you know whether it seems to work as intended there.

[mstorsjo]

In D93186#2485140, @mstorsjo wrote:

FWIW, while I've touched this code on the outside, I'm not really familiar with it. But I can at least try out the patch in my setups (dwarf + SEH) and let you know whether it seems to work as intended there.

Looks like it works fine in with my exception handling test set for mingw.

[ldionne]

In that case, this LGTM. I think we do have a problem of code ownership in this area, but it's a separate topic!

libcxxabi/src/cxa_personality.cpp

#endif
                      _Unwind_Exception* unwind_exception, _Unwind_Context* context)
 {
     if (version != 1 || unwind_exception == 0 || context == 0)
         return _URC_FATAL_PHASE1_ERROR;
 
     bool native_exception = (exceptionClass     & get_vendor_and_language) ==
                             (kOurExceptionClass & get_vendor_and_language);
     scan_results results;
-    if (actions & _UA_SEARCH_PHASE)
-    {
-        // Phase 1 search:  All we're looking for in phase 1 is a handler that
-        //   halts unwinding
-        scan_eh_tab(results, actions, native_exception, unwind_exception, context);
-        if (results.reason == _URC_HANDLER_FOUND)
-        {
-            // Found one.  Can we cache the results somewhere to optimize phase 2?
-            if (native_exception)
-            {
-                __cxa_exception* exception_header = (__cxa_exception*)(unwind_exception+1) - 1;
-                exception_header->handlerSwitchValue = static_cast<int>(results.ttypeIndex);
-                exception_header->actionRecord = results.actionRecord;
-                exception_header->languageSpecificData = results.languageSpecificData;
-                exception_header->catchTemp = reinterpret_cast<void*>(results.landingPad);
-                exception_header->adjustedPtr = results.adjustedPtr;
-            }
-            return _URC_HANDLER_FOUND;
-        }
-        // Did not find a catching-handler.  Return the results of the scan
-        //    (normally _URC_CONTINUE_UNWIND, but could have been _URC_FATAL_PHASE1_ERROR
-        //     if we were called improperly).
-        return results.reason;
-    }
-    if (actions & _UA_CLEANUP_PHASE)
-    {
-        // Phase 2 search:
-        //  Did we find a catching handler in phase 1?
-        if (actions & _UA_HANDLER_FRAME)
-        {
-            // Yes, phase 1 said we have a catching handler here.
-            // Did we cache the results of the scan?
-            if (native_exception)
-            {
-                // Yes, reload the results from the cache.
-                __cxa_exception* exception_header = (__cxa_exception*)(unwind_exception+1) - 1;
-                results.ttypeIndex = exception_header->handlerSwitchValue;
-                results.actionRecord = exception_header->actionRecord;
-                results.languageSpecificData = exception_header->languageSpecificData;
-                results.landingPad = reinterpret_cast<uintptr_t>(exception_header->catchTemp);
-                results.adjustedPtr = exception_header->adjustedPtr;
-            }
-            else
-            {
-                // No, do the scan again to reload the results.
-                scan_eh_tab(results, actions, native_exception, unwind_exception, context);
-                // Phase 1 told us we would find a handler.  Now in Phase 2 we
-                //   didn't find a handler.  The eh table should not be changing!
-                if (results.reason != _URC_HANDLER_FOUND)
-                    call_terminate(native_exception, unwind_exception);
-            }
-            // Jump to the handler
-            set_registers(unwind_exception, context, results);
-            return _URC_INSTALL_CONTEXT;
-        }
-        // Either we didn't do a phase 1 search (due to forced unwinding), or
-        //   phase 1 reported no catching-handlers.
-        // Search for a (non-catching) cleanup
-        scan_eh_tab(results, actions, native_exception, unwind_exception, context);
-        if (results.reason == _URC_HANDLER_FOUND)
-        {
-            // Found a non-catching handler.  Jump to it:
-            set_registers(unwind_exception, context, results);
-            return _URC_INSTALL_CONTEXT;
+    // Process a catch handler for a native exception first.
+    if (actions == (_UA_CLEANUP_PHASE | _UA_HANDLER_FRAME) &&
+        native_exception) {
+        // Reload the results from the phase 1 cache.
+        __cxa_exception* exception_header =
+            (__cxa_exception*)(unwind_exception + 1) - 1;
+        results.landingPad =
+            reinterpret_cast<uintptr_t>(exception_header->catchTemp);
+
+        // Jump to the handler.
+
+    // In other cases we need to scan LSDA.
+    if (results.reason == _URC_CONTINUE_UNWIND ||
+        results.reason == _URC_FATAL_PHASE1_ERROR)
+        return results.reason;
+
+    if (actions & _UA_SEARCH_PHASE)
+        // Phase 1 search:  All we're looking for in phase 1 is a handler that
+        //   halts unwinding
+        assert(results.reason == _URC_HANDLER_FOUND);
+        if (native_exception) {
+            // For a native exception, cache the LSDA result.
+            __cxa_exception* exc = (__cxa_exception*)(unwind_exception + 1) - 1;
+            exc->handlerSwitchValue = static_cast<int>(results.ttypeIndex);
+            exc->actionRecord = results.actionRecord;
+            exc->languageSpecificData = results.languageSpecificData;
+            exc->catchTemp = reinterpret_cast<void*>(results.landingPad);
+            exc->adjustedPtr = results.adjustedPtr;
         }
-        // Did not find a cleanup.  Return the results of the scan
-        //    (normally _URC_CONTINUE_UNWIND, but could have been _URC_FATAL_PHASE2_ERROR
-        //     if we were called improperly).
-        return results.reason;
+        return _URC_HANDLER_FOUND;
     }
-    // We were called improperly: neither a phase 1 or phase 2 search
-    return _URC_FATAL_PHASE1_ERROR;
+
+    assert(actions & _UA_CLEANUP_PHASE);
+    assert(results.reason == _URC_HANDLER_FOUND);
+    set_registers(unwind_exception, context, results);
+    return _URC_INSTALL_CONTEXT;
 }
 
 #if defined(__SEH__) && !defined(__USING_SJLJ_EXCEPTIONS__)
 extern "C" _LIBCXXABI_FUNC_VIS EXCEPTION_DISPOSITION
 __gxx_personality_seh0(PEXCEPTION_RECORD ms_exc, void *this_frame,
                        PCONTEXT ms_orig_context, PDISPATCHER_CONTEXT ms_disp)
 {
   return _GCC_specific_handler(ms_exc, this_frame, ms_orig_context, ms_disp,