[libcxxabi] r228903 - unwind: move exported APIs out of header
Logan Chien
tzuhsiang.chien at gmail.com
Mon Mar 23 14:46:36 PDT 2015
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/20150324/e3761d33/attachment.html>
More information about the cfe-commits
mailing list