[libc++abi] NFC: adding a new parameter base to functions for calculating addresses with relative encodings

[xingxue]

Summary

This NFC patch adds a new parameter base to functions invoked by scan_eh_tab() for calculating the address of the encoding with a relative value. base defaults to 0. This is in preparation for the AIX implementation which uses the DW_EH_PE_datarel encoding (https://reviews.llvm.org/D101298).

[MaskRay]

I keep feeling that the AIX implementation probably needs its own file. Making Itanium/ARM/SEH share some functions actually make generic code improvement harder.

[sfertile]

In D101545#2726036, @MaskRay wrote:

I keep feeling that the AIX implementation probably needs its own file. Making Itanium/ARM/SEH share some functions actually make generic code improvement harder.

I agree on the second part, but I don't think AIX should be split out to separate files. The AIX range table implementation which this change is meant to enable, from what I understand, is a completely straight-forward implementation of Itanium using DW_EH_PE_datarel. We would essentially be duplicating all the Itanium code for AIX with only a handful of differing lines. Splitting ARM and SEH into distinct files from Itanium might be useful, but we still need to make this exact change to the Itanium implementation.

[xingxue]

In D101545#2726036, @MaskRay wrote:

I keep feeling that the AIX implementation probably needs its own file. Making Itanium/ARM/SEH share some functions actually make generic code improvement harder.

I understand your concern. However, the AIX personality routine for the range table is essentially the same as the implementation of gxx_personality_v0 except it uses the DW_EH_PE_datarel encoding which the current Itanium implementation does not support. Therefore, this patch and the follow-on patch https://reviews.llvm.org/D101298 extend the existing Itanium implementation to allow relative addresses with a base. These two patches are straightforward and is all the AIX personality for the range table needs.

The AIX EH implementation also has a personality routine for the state table, which accounts for most changes to cxa_personality.cpp. The personality for the state table is orthogonal and can be placed in a separate file if needed.

[sfertile]

libcxxabi/src/cxa_personality.cpp

922

My idea behind splitting out an NFC patch was to have just the interface changes needed to support a datarel base implementation, without any of the functional changes for the datarel implementation. This part of the diff looks like it falls under the later. I'm guessing the intent it to show that this part of the patch is NFC for all existing targets?

IMO I think we would be best with 1 interface only patch, and 1 implementation. only patch but others might disagree with me.

Can you remove this, the write to exception_header->catchTemp in the bloc on lines 955-960, and then the read of exception_header->catchTemp on line 1150 to make the patch an interface change only?

[xingxue]

Addressed comment:

• removed code for caching the base and getting it from the cache.

[xingxue]

This comment was removed by xingxue.

[xingxue]

libcxxabi/src/cxa_personality.cpp

922

Done. Also removed the code that gets base from cache.

[sfertile]

Thanks Xing, LGTM.

[xingxue]

Hi @MaskRay, Do you have any further comments/concerns? Thanks!

[xingxue]

@libc++abi, @mclow.lists Hi, This NFC patch has been reviewed and approved by @sfertile but it needs the approval from blocking reviewer the libc++abi group before it can be landed. We are working on the Clang compiler for the AIX platform and libunwind/libc++abi are used for EH. For AIX, the DW_EH_PE_datarel encoding is used for calculating relative addresses in the range table. Therefore, the code for scanning the range table is extended to have one more parameter base. This NFC patch and the follow-on patch https://reviews.llvm.org/D101298 are the changes needed to process the range table for Clang on AIX. It is important to us to get the changes reviewed and landed to unblock the buildbot. It would be greatly appreciated if the group can take a look and let us know if there are any concerns.

We are also trying to replace the runtime EH implementation of the earlier compiler xlclang++ on AIX with the libc++abi/libunwind approach. Since xlclang++ uses the state table for EH instead of the range table, a separate personality routine __xlcxx_personality_v0() for the state table is implemented in cxa_personality.cpp (patch https://reviews.llvm.org/D100504). The implementation for the state table is AIX specific and orthogonal to the changes for the range table mentioned above.

[xingxue]

Ping.

[compnerd]

I’d say wait a day or so to make sure that @ldionne doesn’t have any concerns, but this seems like it should be safe. Rebasing a pointer seems like a useful enough thing and this don’t impact the public interfaces.

[ldionne]

In D101545#2796567, @compnerd wrote:

I’d say wait a day or so to make sure that @ldionne doesn’t have any concerns, but this seems like it should be safe. Rebasing a pointer seems like a useful enough thing and this don’t impact the public interfaces.

TBH, cxa_personality is somewhat below what I usually touch, so I don't really have a concern here.

I'd like to note that this patch does trigger -Wunused-parameter, though - would it be possible to fix those, or are those seemingly unused parameters actually necessary to reduce the diff between upstream and a downstream fork?

[xingxue]

TBH, cxa_personality is somewhat below what I usually touch, so I don't really have a concern here.

I'd like to note that this patch does trigger -Wunused-parameter, though - would it be possible to fix those, or are those seemingly unused parameters actually necessary to reduce the diff between upstream and a downstream fork?

Thanks for catching the unused parameter warning, @ldionne ! I will open a separate NFC patch to address it. This patch was committed yesterday morning but somehow not closed.

[xingxue]

Commit: rG7f0244afa828: [libc++abi] NFC: adding a new parameter base to functions for calculating… (authored by xingxue). Closing.

libcxxabi/src/cxa_personality.cpp

readSLEB128(const uint8_t** data)
     return static_cast<intptr_t>(result);
 }
 
 /// Read a pointer encoded value and advance pointer
 /// See Variable Length Data in:
 /// @link http://dwarfstd.org/Dwarf3.pdf @unlink
 /// @param data reference variable holding memory pointer to decode from
 /// @param encoding dwarf encoding type
+/// @param base for adding relative offset, default to 0
 /// @returns decoded value
 static
 uintptr_t
-readEncodedPointer(const uint8_t** data, uint8_t encoding)
+readEncodedPointer(const uint8_t** data, uint8_t encoding, uintptr_t base = 0)
 {
     uintptr_t result = 0;
     if (encoding == DW_EH_PE_omit)
         return result;
     const uint8_t* p = *data;
     // first get value
     switch (encoding & 0x0F)
     {
#else
     return *reinterpret_cast<const void **>(reinterpret_cast<uintptr_t>(ptr) +
                                             offset);
 #endif
 }
 
 static const __shim_type_info*
 get_shim_type_info(uint64_t ttypeIndex, const uint8_t* classInfo,
                    uint8_t ttypeEncoding, bool native_exception,
-                   _Unwind_Exception* unwind_exception)
+                   _Unwind_Exception* unwind_exception, uintptr_t base = 0)
 {
     if (classInfo == 0)
     {
         // this should not happen.  Indicates corrupted eh_table.
         call_terminate(native_exception, unwind_exception);
     }
 
     assert(((ttypeEncoding == DW_EH_PE_absptr) ||  // LLVM or GCC 4.6
             (ttypeEncoding == DW_EH_PE_pcrel) ||  // GCC 4.7 baremetal
             (ttypeEncoding == (DW_EH_PE_pcrel | DW_EH_PE_indirect))) &&  // GCC 4.7 linux
            "Unexpected TTypeEncoding");
     (void)ttypeEncoding;
 
     const uint8_t* ttypePtr = classInfo - ttypeIndex * sizeof(uintptr_t);
     return reinterpret_cast<const __shim_type_info *>(
         read_target2_value(ttypePtr));
 }
 #else // !defined(_LIBCXXABI_ARM_EHABI)
 static
 const __shim_type_info*
 get_shim_type_info(uint64_t ttypeIndex, const uint8_t* classInfo,
                    uint8_t ttypeEncoding, bool native_exception,
-                   _Unwind_Exception* unwind_exception)
+                   _Unwind_Exception* unwind_exception, uintptr_t base = 0)
 {
     if (classInfo == 0)
     {
         // this should not happen.  Indicates corrupted eh_table.
         call_terminate(native_exception, unwind_exception);
     }
     switch (ttypeEncoding & 0x0F)
     {
get_shim_type_info(uint64_t ttypeIndex, const uint8_t* classInfo,
     case DW_EH_PE_sdata8:
         ttypeIndex *= 8;
         break;
     default:
         // this should not happen.   Indicates corrupted eh_table.
         call_terminate(native_exception, unwind_exception);
     }
     classInfo -= ttypeIndex;
-    return (const __shim_type_info*)readEncodedPointer(&classInfo, ttypeEncoding);
+    return (const __shim_type_info*)readEncodedPointer(&classInfo,
+                                                       ttypeEncoding, base);
 }
 #endif // !defined(_LIBCXXABI_ARM_EHABI)
 
 /*
     This is checking a thrown exception type, excpType, against a possibly empty
     list of catchType's which make up an exception spec.
 
     An exception spec acts like a catch handler, but in reverse.  This "catch
     handler" will catch an excpType if and only if none of the catchType's in
     the list will catch a excpType.  If any catchType in the list can catch an
     excpType, then this exception spec does not catch the excpType.
 */
 #if defined(_LIBCXXABI_ARM_EHABI)
 static
 bool
 exception_spec_can_catch(int64_t specIndex, const uint8_t* classInfo,
                          uint8_t ttypeEncoding, const __shim_type_info* excpType,
-                         void* adjustedPtr, _Unwind_Exception* unwind_exception)
+                         void* adjustedPtr, _Unwind_Exception* unwind_exception,
+                         uintptr_t base = 0)
 {
     if (classInfo == 0)
     {
         // this should not happen.   Indicates corrupted eh_table.
         call_terminate(false, unwind_exception);
     }
 
     assert(((ttypeEncoding == DW_EH_PE_absptr) ||  // LLVM or GCC 4.6
exception_spec_can_catch(int64_t specIndex, const uint8_t* classInfo,
     }
     return true;
 }
 #else
 static
 bool
 exception_spec_can_catch(int64_t specIndex, const uint8_t* classInfo,
                          uint8_t ttypeEncoding, const __shim_type_info* excpType,
-                         void* adjustedPtr, _Unwind_Exception* unwind_exception)
+                         void* adjustedPtr, _Unwind_Exception* unwind_exception,
+                         uintptr_t base = 0)
 {
     if (classInfo == 0)
     {
         // this should not happen.   Indicates corrupted eh_table.
         call_terminate(false, unwind_exception);
     }
     // specIndex is negative of 1-based byte offset into classInfo;
     specIndex = -specIndex;
     --specIndex;
     const uint8_t* temp = classInfo + specIndex;
     // If any type in the spec list can catch excpType, return false, else return true
     //    adjustments to adjustedPtr are ignored.
     while (true)
     {
         uint64_t ttypeIndex = readULEB128(&temp);
         if (ttypeIndex == 0)
             break;
         const __shim_type_info* catchType = get_shim_type_info(ttypeIndex,
                                                                classInfo,
                                                                ttypeEncoding,
                                                                true,
-                                                               unwind_exception);
+                                                               unwind_exception,
+                                                               base);
         void* tempPtr = adjustedPtr;
         if (catchType->can_catch(excpType, tempPtr))
             return false;
     }
     return true;
 }
 #endif
 
static void scan_eh_tab(scan_results &results, _Unwind_Action actions,
     const uint8_t *lsda = (const uint8_t *)_Unwind_GetLanguageSpecificData(context);
     if (lsda == 0)
     {
         // There is no exception table
         results.reason = _URC_CONTINUE_UNWIND;
         return;
     }
     results.languageSpecificData = lsda;
+    uintptr_t base = 0;
     // Get the current instruction pointer and offset it before next
     // instruction in the current frame which threw the exception.
     uintptr_t ip = _Unwind_GetIP(context) - 1;
     // Get beginning current frame's code (as defined by the
     // emitted dwarf code)
     uintptr_t funcStart = _Unwind_GetRegionStart(context);
 #ifdef __USING_SJLJ_EXCEPTIONS__
     if (ip == uintptr_t(-1))
#endif // __USING_SJLJ_EXCEPTIONS__
                 int64_t ttypeIndex = readSLEB128(&action);
                 if (ttypeIndex > 0)
                 {
                     // Found a catch, does it actually catch?
                     // First check for catch (...)
                     const __shim_type_info* catchType =
                         get_shim_type_info(static_cast<uint64_t>(ttypeIndex),
                                            classInfo, ttypeEncoding,
-                                           native_exception, unwind_exception);
+                                           native_exception, unwind_exception,
+                                           base);
                     if (catchType == 0)
                     {
                         // Found catch (...) catches everything, including
                         // foreign exceptions. This is search phase, cleanup
                         // phase with foreign exception, or forced unwinding.
                         assert(actions & (_UA_SEARCH_PHASE | _UA_HANDLER_FRAME |
                                           _UA_FORCE_UNWIND));
                         results.ttypeIndex = ttypeIndex;
#endif // __USING_SJLJ_EXCEPTIONS__
                             static_cast<const __shim_type_info*>(exception_header->exceptionType);
                         if (adjustedPtr == 0 || excpType == 0)
                         {
                             // Something very bad happened
                             call_terminate(native_exception, unwind_exception);
                         }
                         if (exception_spec_can_catch(ttypeIndex, classInfo,
                                                      ttypeEncoding, excpType,
-                                                     adjustedPtr, unwind_exception))
+                                                     adjustedPtr,
+                                                     unwind_exception, base))
                         {
                             // Native exception caught by exception
                             // specification.
                             assert(actions & _UA_SEARCH_PHASE);
                             results.ttypeIndex = ttypeIndex;
                             results.actionRecord = actionRecord;
                             results.adjustedPtr = adjustedPtr;
                             results.reason = _URC_HANDLER_FOUND;
if (actions == (_UA_CLEANUP_PHASE | _UA_HANDLER_FRAME) &&
         results.actionRecord = exception_header->actionRecord;
         results.languageSpecificData = exception_header->languageSpecificData;
         results.landingPad =
             reinterpret_cast<uintptr_t>(exception_header->catchTemp);
         results.adjustedPtr = exception_header->adjustedPtr;
 
         // Jump to the handler.
         set_registers(unwind_exception, context, results);
         return _URC_INSTALL_CONTEXT;
     }
 
     // In other cases we need to scan LSDA.
     scan_eh_tab(results, actions, native_exception, unwind_exception, context);
     if (results.reason == _URC_CONTINUE_UNWIND ||
         results.reason == _URC_FATAL_PHASE1_ERROR)
         return results.reason;
 
if (unwind_exception == 0)
         call_terminate(false, unwind_exception);
     __cxa_begin_catch(unwind_exception);
     bool native_old_exception = __isOurExceptionClass(unwind_exception);
     std::unexpected_handler u_handler;
     std::terminate_handler t_handler;
     __cxa_exception* old_exception_header = 0;
     int64_t ttypeIndex;
     const uint8_t* lsda;
+    uintptr_t base = 0;
+
     if (native_old_exception)
     {
         old_exception_header = (__cxa_exception*)(unwind_exception+1) - 1;
         t_handler = old_exception_header->terminateHandler;
         u_handler = old_exception_header->unexpectedHandler;
         // If std::__unexpected(u_handler) rethrows the same exception,
         //   these values get overwritten by the rethrow.  So save them now:
 #if defined(_LIBCXXABI_ARM_EHABI)
catch (...)
             {
                 const __shim_type_info* excpType =
                     static_cast<const __shim_type_info*>(new_exception_header->exceptionType);
                 adjustedPtr =
                     __getExceptionClass(&new_exception_header->unwindHeader) == kOurDependentExceptionClass ?
                         ((__cxa_dependent_exception*)new_exception_header)->primaryException :
                         new_exception_header + 1;
                 if (!exception_spec_can_catch(ttypeIndex, classInfo, ttypeEncoding,
-                                              excpType, adjustedPtr, unwind_exception))
+                                              excpType, adjustedPtr,
+                                              unwind_exception, base))
                 {
                     // We need to __cxa_end_catch, but for the old exception,
                     //   not the new one.  This is a little tricky ...
                     // Disguise new_exception_header as a rethrown exception, but
                     //   don't actually rethrow it.  This means you can temporarily
                     //   end the catch clause enclosing new_exception_header without
                     //   __cxa_end_catch destroying new_exception_header.
                     new_exception_header->handlerCount = -new_exception_header->handlerCount;
catch (...)
             //   ttypeIndex?
             // If no
             //    throw std::bad_exception();
             const __shim_type_info* excpType =
                 static_cast<const __shim_type_info*>(&typeid(std::bad_exception));
             std::bad_exception be;
             adjustedPtr = &be;
             if (!exception_spec_can_catch(ttypeIndex, classInfo, ttypeEncoding,
-                                          excpType, adjustedPtr, unwind_exception))
+                                          excpType, adjustedPtr,
+                                          unwind_exception, base))
             {
                 // We need to __cxa_end_catch for both the old exception and the
                 //   new exception.  Technically we should do it in that order.
                 //   But it is expedient to do it in the opposite order:
                 // Call __cxa_end_catch for new_exception_header
                 __cxa_end_catch();
                 // Throw std::bad_exception will __cxa_end_catch for
                 //   old_exception_header