[libcxxabi] r228903 - unwind: move exported APIs out of header

Logan Chien tzuhsiang.chien at gmail.com
Tue Mar 31 09:29:42 PDT 2015 

Ping?

On Tue, Mar 24, 2015 at 5:46 AM, Logan Chien <tzuhsiang.chien at gmail.com>
wrote:

> Hi Saleem,
>
> Sorry for the late reply.
>
> Although I am not strongly oppose to the idea to provide both inline and
> extern version, I am concerned that exporting these symbols will further
> fragmentize the ecosystem.  With these symbol exported, some application
> will start to simply declaring their own prototype and referencing the
> these functions directly instead of including <unwind.h>.  IMO, it should
> be conservative to extend an ABI especially when the extension is neither
> documented nor de facto in the ARM ecosystem.
>
> > On the other hand, when applications are using the interfaces, expecting
> the unwind APIs, I think that they should continue to function.  Providing
> both the external as well as the inlined version should achieve that.
>
> In fact, this is what I wish to avoid.  IMO, for the application
> developers, they should simply include <unwind.h> if they need these
> functions, instead of declaring their own function prototype.
>
> Sincerely,
> Logan
>
> On Wed, Mar 18, 2015 at 10:14 AM, Saleem Abdulrasool <
> compnerd at compnerd.org> wrote:
>
>> On Tue, Mar 17, 2015 at 10:42 AM, Logan Chien <tzuhsiang.chien at gmail.com>
>> wrote:
>>
>>> Hi Saleem,
>>>
>>> > Why would libc++abi have undefined references, it it doesn't use the
>>> functions?
>>>
>>> Short answer:
>>>
>>> The libc++abi library does use these functions (see
>>> src/cxa_personality.cpp.)  However, it expect that these functions have to
>>> be replaced with (or inlined as) _Unwind_VRS_{Get,Set}().  After this
>>> commit, it will simply leave an external function call which will lead to
>>> an undefined reference.
>>>
>>
>> Ah; okay, so as I mentioned in the commit, we really do need to figure
>> out a way to do the equivalent of __declspec(dllexport) inline, that is, we
>> need both the inlined verison as well as an external version.
>>
>>
>>> Long answer:
>>>
>>> AFAIK, ARM EHABI [1] and Itanium C++ ABI [2] are independently
>>> developed.  Thus, ARM EHABI is slightly different from Itanium C++ ABI and
>>> having different language independent unwinder API.  That's the reason why
>>> _Unwind_{Get,Set}{IP,GR} are not available in the
>>>
>>
>> Yes, this is correct.  They were independently designed.  However,
>> libunwind needs to provider the *libunwind* interfaces.  It can extend that
>> with the EHABI interfaces as well.  In the case of EHABI, it could use the
>> EHABI interfaces to reimplement the functionality.
>>
>>
>>> On the other hand, many developers wish to port their existing code base
>>> to ARM without wrapping their function calls to _Unwind_{Get,Set}{IP,GR}()
>>> with sandwich-like #ifdef directives, thus the inline functions are added
>>> to the <unwind.h> header [3].  Although new functions are added to the
>>> header, they will be inlined and replaced properly so that the ported
>>> applications can still be linked with the old libgcc prebuilt binaries.  On
>>> the contrary, this commit (r228903) will only declare these functions as
>>> external functions, thus the compiler will simply emit an undefined
>>> reference to _Unwind_{Get,Set}{IP,GR}().  Consequently, the ported
>>> application won't link with libgcc anymore.
>>>
>>
>> On the other hand, when applications are using the interfaces, expecting
>> the unwind APIs, I think that they should continue to function.  Providing
>> both the external as well as the inlined version should achieve that.
>>
>>
>>> What make the situation even worse is that the <unwind.h> from libc++abi
>>> will be installed (and overwrite the one from <clang>/lib/Headers/unwind.h)
>>> if we are running an in-tree build.  The <unwind.h> from libc++abi will be
>>> chosen when we are compiling source code with clang.  As a result, all of
>>> _Unwind_{Get,Set}{IP,GR}() will become an external reference.  Even if we
>>> would like to compile the source code with libstdc++v3 (the GCC STL
>>> implementation), the external reference will be generated as well.
>>>
>>> To wrap up, I think we should bring the inline function back at least.
>>> Otherwise, the libgcc support will be completely unusable in the
>>> arm-linux-gnueabi architecture.  Whether should we provide an
>>> implementation _Unwind_{Get,Set}{IP,GR}() in the libunwind or not is
>>> another question, although I personally disagree with this idea due to
>>> other reasons.  Feel free to let me know have any other questions.  Thanks.
>>>
>>
>> I think we agree on the fact that we need the inline version as well
>> given your explanation.  However, I think that we need both the inlined
>> version as well as the exported version.
>>
>>
>>> Sincerely,
>>> Logan
>>>
>>> [1]
>>> http://infocenter.arm.com/help/topic/com.arm.doc.ihi0038a/IHI0038A_ehabi.pdf
>>> [2] https://mentorembedded.github.io/cxx-abi/abi-eh.html
>>> [3] Note that _Unwind_{Get,Set}GR is not a complete replacement for
>>> _Unwind_VRS_{Get,Set} anyway.  For example, VFP registers can't be accessed
>>> with _Unwind_{Get,Set}GR().
>>>
>>> On Tue, Mar 17, 2015 at 10:34 AM, Saleem Abdulrasool <
>>> compnerd at compnerd.org> wrote:
>>>
>>>> On Sat, Mar 14, 2015 at 8:43 AM, Logan Chien <tzuhsiang.chien at gmail.com
>>>> > wrote:
>>>>
>>>>> Hi Saleem,
>>>>>
>>>>> Similar change has been committed and reverted (see r219629 and
>>>>> r226818.)
>>>>>
>>>>> This change WILL BREAK the libc++abi + libgcc build.  With this
>>>>> change, the libc++abi shared library will come with undefined references to
>>>>> _Unwind_{Get,Set}{IP,GR}, which are not available in libgcc.
>>>>>
>>>>> AFAICT, these functions are not a part of public interface of ARM
>>>>> EHABI.  The only documented public interface is
>>>>> _Unwind_VRS_Get/_Unwind_VRS_Set.  Even though, most existing
>>>>> implementations, including the unwind.h shipped with gcc and clang, define
>>>>> these inline functions.  But none of them will lead to an external
>>>>> reference to _Unwind_{Get,Set}{IP,GR}.
>>>>>
>>>>
>>>> They aren't part of the EHABI, but are part of the Unwind (public)
>>>> interfaces.  I do think that they need to be made available, even if in an
>>>> alternate form.  Why would libc++abi have undefined references, it it
>>>> doesn't use the functions?
>>>>
>>>>
>>>>> Would you mind to revert this change?  Or, is there any other
>>>>> situation, which requires this change, that I haven't considered?  Thanks.
>>>>>
>>>>> Sincerely,
>>>>> Logan
>>>>>
>>>>>
>>>>> On Thu, Feb 12, 2015 at 12:25 PM, Saleem Abdulrasool <
>>>>> compnerd at compnerd.org> wrote:
>>>>>
>>>>>> Author: compnerd
>>>>>> Date: Wed Feb 11 22:25:03 2015
>>>>>> New Revision: 228903
>>>>>>
>>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=228903&view=rev
>>>>>> Log:
>>>>>> unwind: move exported APIs out of header
>>>>>>
>>>>>> Ideally, we would do something like inline __declspec(dllexport) to
>>>>>> ensure that
>>>>>> the symbol was inlined within libunwind as well as emitted into the
>>>>>> final DSO.
>>>>>> This simply moves the definition out of the header to ensure that the
>>>>>> *public*
>>>>>> interfaces are defined and exported into the final DSO.
>>>>>>
>>>>>> This change also has "gratuitous" code movement so that the EHABI and
>>>>>> generic
>>>>>> implementations are co-located making it easier to find them.
>>>>>>
>>>>>> The movement from the header has one minor change introduced into the
>>>>>> code:
>>>>>> additional tracing to mirror the behaviour of the non-EHABI
>>>>>> interfaces.
>>>>>>
>>>>>> Modified:
>>>>>>     libcxxabi/trunk/include/unwind.h
>>>>>>     libcxxabi/trunk/src/Unwind/UnwindLevel1.c
>>>>>>
>>>>>> Modified: libcxxabi/trunk/include/unwind.h
>>>>>> URL:
>>>>>> http://llvm.org/viewvc/llvm-project/libcxxabi/trunk/include/unwind.h?rev=228903&r1=228902&r2=228903&view=diff
>>>>>>
>>>>>> ==============================================================================
>>>>>> --- libcxxabi/trunk/include/unwind.h (original)
>>>>>> +++ libcxxabi/trunk/include/unwind.h Wed Feb 11 22:25:03 2015
>>>>>> @@ -202,37 +202,13 @@ extern _Unwind_VRS_Result
>>>>>>  _Unwind_VRS_Pop(_Unwind_Context *context, _Unwind_VRS_RegClass
>>>>>> regclass,
>>>>>>                  uint32_t discriminator,
>>>>>>                  _Unwind_VRS_DataRepresentation representation);
>>>>>> +#endif
>>>>>>
>>>>>> -static inline uintptr_t _Unwind_GetGR(struct _Unwind_Context*
>>>>>> context,
>>>>>> -                                      int index) {
>>>>>> -  uintptr_t value = 0;
>>>>>> -  _Unwind_VRS_Get(context, _UVRSC_CORE, (uint32_t)index,
>>>>>> _UVRSD_UINT32, &value);
>>>>>> -  return value;
>>>>>> -}
>>>>>> -
>>>>>> -static inline void _Unwind_SetGR(struct _Unwind_Context* context,
>>>>>> int index,
>>>>>> -                                 uintptr_t new_value) {
>>>>>> -  _Unwind_VRS_Set(context, _UVRSC_CORE, (uint32_t)index,
>>>>>> -                  _UVRSD_UINT32, &new_value);
>>>>>> -}
>>>>>> -
>>>>>> -static inline uintptr_t _Unwind_GetIP(struct _Unwind_Context*
>>>>>> context) {
>>>>>> -  // remove the thumb-bit before returning
>>>>>> -  return (_Unwind_GetGR(context, 15) & (~(uintptr_t)0x1));
>>>>>> -}
>>>>>> -
>>>>>> -static inline void _Unwind_SetIP(struct _Unwind_Context* context,
>>>>>> -                                 uintptr_t new_value) {
>>>>>> -  uintptr_t thumb_bit = _Unwind_GetGR(context, 15) &
>>>>>> ((uintptr_t)0x1);
>>>>>> -  _Unwind_SetGR(context, 15, new_value | thumb_bit);
>>>>>> -}
>>>>>> -#else
>>>>>>  extern uintptr_t _Unwind_GetGR(struct _Unwind_Context *context, int
>>>>>> index);
>>>>>>  extern void _Unwind_SetGR(struct _Unwind_Context *context, int index,
>>>>>>                            uintptr_t new_value);
>>>>>>  extern uintptr_t _Unwind_GetIP(struct _Unwind_Context *context);
>>>>>>  extern void _Unwind_SetIP(struct _Unwind_Context *, uintptr_t
>>>>>> new_value);
>>>>>> -#endif
>>>>>>
>>>>>>  extern uintptr_t _Unwind_GetRegionStart(struct _Unwind_Context
>>>>>> *context);
>>>>>>  extern uintptr_t
>>>>>>
>>>>>> Modified: libcxxabi/trunk/src/Unwind/UnwindLevel1.c
>>>>>> URL:
>>>>>> http://llvm.org/viewvc/llvm-project/libcxxabi/trunk/src/Unwind/UnwindLevel1.c?rev=228903&r1=228902&r2=228903&view=diff
>>>>>>
>>>>>> ==============================================================================
>>>>>> --- libcxxabi/trunk/src/Unwind/UnwindLevel1.c (original)
>>>>>> +++ libcxxabi/trunk/src/Unwind/UnwindLevel1.c Wed Feb 11 22:25:03 2015
>>>>>> @@ -422,10 +422,73 @@ _Unwind_GetLanguageSpecificData(struct _
>>>>>>  }
>>>>>>
>>>>>>
>>>>>> +/// Called by personality handler during phase 2 to find the start
>>>>>> of the
>>>>>> +/// function.
>>>>>> +_LIBUNWIND_EXPORT uintptr_t
>>>>>> +_Unwind_GetRegionStart(struct _Unwind_Context *context) {
>>>>>> +  unw_cursor_t *cursor = (unw_cursor_t *)context;
>>>>>> +  unw_proc_info_t frameInfo;
>>>>>> +  uintptr_t result = 0;
>>>>>> +  if (unw_get_proc_info(cursor, &frameInfo) == UNW_ESUCCESS)
>>>>>> +    result = (uintptr_t)frameInfo.start_ip;
>>>>>> +  _LIBUNWIND_TRACE_API("_Unwind_GetRegionStart(context=%p) => 0x%"
>>>>>> PRIxPTR "\n",
>>>>>> +                       (void *)context, result);
>>>>>> +  return result;
>>>>>> +}
>>>>>> +
>>>>>> +
>>>>>> +/// Called by personality handler during phase 2 if a foreign
>>>>>> exception
>>>>>> +// is caught.
>>>>>> +_LIBUNWIND_EXPORT void
>>>>>> +_Unwind_DeleteException(_Unwind_Exception *exception_object) {
>>>>>> +  _LIBUNWIND_TRACE_API("_Unwind_DeleteException(ex_obj=%p)\n",
>>>>>> +                       (void *)exception_object);
>>>>>> +  if (exception_object->exception_cleanup != NULL)
>>>>>> +
>>>>>> (*exception_object->exception_cleanup)(_URC_FOREIGN_EXCEPTION_CAUGHT,
>>>>>> +                                           exception_object);
>>>>>> +}
>>>>>> +
>>>>>> +#endif // _LIBUNWIND_BUILD_ZERO_COST_APIS && !LIBCXXABI_ARM_EHABI
>>>>>> +
>>>>>> +#if LIBCXXABI_ARM_EHABI
>>>>>> +
>>>>>> +_LIBUNWIND_EXPORT uintptr_t
>>>>>> +_Unwind_GetGR(struct _Unwind_Context *context, int index) {
>>>>>> +  uintptr_t value = 0;
>>>>>> +  _Unwind_VRS_Get(context, _UVRSC_CORE, (uint32_t)index,
>>>>>> _UVRSD_UINT32, &value);
>>>>>> +  _LIBUNWIND_TRACE_API("_Unwind_GetGR(context=%p, reg=%d) => 0x%"
>>>>>> PRIx64 "\n",
>>>>>> +                       (void *)context, index, (uint64_t)value);
>>>>>> +  return value;
>>>>>> +}
>>>>>> +
>>>>>> +_LIBUNWIND_EXPORT void _Unwind_SetGR(struct _Unwind_Context
>>>>>> *context, int index,
>>>>>> +                                     uintptr_t value) {
>>>>>> +  _LIBUNWIND_TRACE_API("_Unwind_SetGR(context=%p, reg=%d,
>>>>>> value=0x%0"PRIx64")\n",
>>>>>> +                       (void *)context, index, (uint64_t)value);
>>>>>> +  _Unwind_VRS_Set(context, _UVRSC_CORE, (uint32_t)index,
>>>>>> _UVRSD_UINT32, &value);
>>>>>> +}
>>>>>> +
>>>>>> +_LIBUNWIND_EXPORT uintptr_t _Unwind_GetIP(struct _Unwind_Context
>>>>>> *context) {
>>>>>> +  // remove the thumb-bit before returning
>>>>>> +  uintptr_t value = _Unwind_GetGR(context, 15) & (~(uintptr_t)0x1);
>>>>>> +  _LIBUNWIND_TRACE_API("_Unwind_GetIP(context=%p) => 0x%" PRIx64
>>>>>> "\n",
>>>>>> +                       (void *)context, (uint64_t)value);
>>>>>> +  return value;
>>>>>> +}
>>>>>> +
>>>>>> +_LIBUNWIND_EXPORT void _Unwind_SetIP(struct _Unwind_Context *context,
>>>>>> +                                     uintptr_t value) {
>>>>>> +  _LIBUNWIND_TRACE_API("_Unwind_SetIP(context=%p, value=0x%0" PRIx64
>>>>>> ")\n",
>>>>>> +                       (void *)context, (uint64_t)value);
>>>>>> +  uintptr_t thumb_bit = _Unwind_GetGR(context, 15) &
>>>>>> ((uintptr_t)0x1);
>>>>>> +  _Unwind_SetGR(context, 15, value | thumb_bit);
>>>>>> +}
>>>>>> +
>>>>>> +#else
>>>>>>
>>>>>>  /// Called by personality handler during phase 2 to get register
>>>>>> values.
>>>>>> -_LIBUNWIND_EXPORT uintptr_t _Unwind_GetGR(struct _Unwind_Context
>>>>>> *context,
>>>>>> -                                          int index) {
>>>>>> +_LIBUNWIND_EXPORT uintptr_t
>>>>>> +_Unwind_GetGR(struct _Unwind_Context *context, int index) {
>>>>>>    unw_cursor_t *cursor = (unw_cursor_t *)context;
>>>>>>    unw_word_t result;
>>>>>>    unw_get_reg(cursor, index, &result);
>>>>>> @@ -434,20 +497,16 @@ _LIBUNWIND_EXPORT uintptr_t _Unwind_GetG
>>>>>>    return (uintptr_t)result;
>>>>>>  }
>>>>>>
>>>>>> -
>>>>>> -
>>>>>>  /// Called by personality handler during phase 2 to alter register
>>>>>> values.
>>>>>>  _LIBUNWIND_EXPORT void _Unwind_SetGR(struct _Unwind_Context
>>>>>> *context, int index,
>>>>>> -                                     uintptr_t new_value) {
>>>>>> +                                     uintptr_t value) {
>>>>>>    _LIBUNWIND_TRACE_API("_Unwind_SetGR(context=%p, reg=%d,
>>>>>> value=0x%0" PRIx64
>>>>>>                         ")\n",
>>>>>> -                       (void *)context, index, (uint64_t)new_value);
>>>>>> +                       (void *)context, index, (uint64_t)value);
>>>>>>    unw_cursor_t *cursor = (unw_cursor_t *)context;
>>>>>> -  unw_set_reg(cursor, index, new_value);
>>>>>> +  unw_set_reg(cursor, index, value);
>>>>>>  }
>>>>>>
>>>>>> -
>>>>>> -
>>>>>>  /// Called by personality handler during phase 2 to get instruction
>>>>>> pointer.
>>>>>>  _LIBUNWIND_EXPORT uintptr_t _Unwind_GetIP(struct _Unwind_Context
>>>>>> *context) {
>>>>>>    unw_cursor_t *cursor = (unw_cursor_t *)context;
>>>>>> @@ -458,44 +517,16 @@ _LIBUNWIND_EXPORT uintptr_t _Unwind_GetI
>>>>>>    return (uintptr_t)result;
>>>>>>  }
>>>>>>
>>>>>> -
>>>>>> -
>>>>>>  /// Called by personality handler during phase 2 to alter
>>>>>> instruction pointer,
>>>>>>  /// such as setting where the landing pad is, so _Unwind_Resume()
>>>>>> will
>>>>>>  /// start executing in the landing pad.
>>>>>>  _LIBUNWIND_EXPORT void _Unwind_SetIP(struct _Unwind_Context *context,
>>>>>> -                                     uintptr_t new_value) {
>>>>>> +                                     uintptr_t value) {
>>>>>>    _LIBUNWIND_TRACE_API("_Unwind_SetIP(context=%p, value=0x%0" PRIx64
>>>>>> ")\n",
>>>>>> -                       (void *)context, (uint64_t)new_value);
>>>>>> +                       (void *)context, (uint64_t)value);
>>>>>>    unw_cursor_t *cursor = (unw_cursor_t *)context;
>>>>>> -  unw_set_reg(cursor, UNW_REG_IP, new_value);
>>>>>> +  unw_set_reg(cursor, UNW_REG_IP, value);
>>>>>>  }
>>>>>>
>>>>>> +#endif
>>>>>>
>>>>>> -/// Called by personality handler during phase 2 to find the start
>>>>>> of the
>>>>>> -/// function.
>>>>>> -_LIBUNWIND_EXPORT uintptr_t
>>>>>> -_Unwind_GetRegionStart(struct _Unwind_Context *context) {
>>>>>> -  unw_cursor_t *cursor = (unw_cursor_t *)context;
>>>>>> -  unw_proc_info_t frameInfo;
>>>>>> -  uintptr_t result = 0;
>>>>>> -  if (unw_get_proc_info(cursor, &frameInfo) == UNW_ESUCCESS)
>>>>>> -    result = (uintptr_t)frameInfo.start_ip;
>>>>>> -  _LIBUNWIND_TRACE_API("_Unwind_GetRegionStart(context=%p) => 0x%"
>>>>>> PRIxPTR "\n",
>>>>>> -                       (void *)context, result);
>>>>>> -  return result;
>>>>>> -}
>>>>>> -
>>>>>> -
>>>>>> -/// Called by personality handler during phase 2 if a foreign
>>>>>> exception
>>>>>> -// is caught.
>>>>>> -_LIBUNWIND_EXPORT void
>>>>>> -_Unwind_DeleteException(_Unwind_Exception *exception_object) {
>>>>>> -  _LIBUNWIND_TRACE_API("_Unwind_DeleteException(ex_obj=%p)\n",
>>>>>> -                       (void *)exception_object);
>>>>>> -  if (exception_object->exception_cleanup != NULL)
>>>>>> -
>>>>>> (*exception_object->exception_cleanup)(_URC_FOREIGN_EXCEPTION_CAUGHT,
>>>>>> -                                           exception_object);
>>>>>> -}
>>>>>> -
>>>>>> -#endif // _LIBUNWIND_BUILD_ZERO_COST_APIS && !LIBCXXABI_ARM_EHABI
>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> cfe-commits mailing list
>>>>>> cfe-commits at cs.uiuc.edu
>>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>> --
>>>> Saleem Abdulrasool
>>>> compnerd (at) compnerd (dot) org
>>>>
>>>
>>>
>>
>>
>> --
>> Saleem Abdulrasool
>> compnerd (at) compnerd (dot) org
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150401/1079aee6/attachment.html>

More information about the cfe-commits mailing list