Ping? On Tue, Mar 24, 2015 at 5:46 AM, Logan Chien <[email protected]> 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 < > [email protected]> wrote: > >> On Tue, Mar 17, 2015 at 10:42 AM, Logan Chien <[email protected]> >> 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 < >>> [email protected]> wrote: >>> >>>> On Sat, Mar 14, 2015 at 8:43 AM, Logan Chien <[email protected] >>>> > 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 < >>>>> [email protected]> 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 >>>>>> [email protected] >>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >>>>>> >>>>> >>>>> >>>> >>>> >>>> -- >>>> Saleem Abdulrasool >>>> compnerd (at) compnerd (dot) org >>>> >>> >>> >> >> >> -- >> Saleem Abdulrasool >> compnerd (at) compnerd (dot) org >> > >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
