<div dir="ltr">Ping?<br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Mar 24, 2015 at 5:46 AM, Logan Chien <span dir="ltr"><<a href="mailto:tzuhsiang.chien@gmail.com" target="_blank">tzuhsiang.chien@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><div><div><div><div>Hi Saleem,<br><br></div>Sorry for the late reply.<br><br></div>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.<span class=""><br><br>> 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.<br><br></span></div>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.<br><br></div>Sincerely,<br></div>Logan<br></div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Mar 18, 2015 at 10:14 AM, Saleem Abdulrasool <span dir="ltr"><<a href="mailto:compnerd@compnerd.org" target="_blank">compnerd@compnerd.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><span>On Tue, Mar 17, 2015 at 10:42 AM, Logan Chien <span dir="ltr"><<a href="mailto:tzuhsiang.chien@gmail.com" target="_blank">tzuhsiang.chien@gmail.com</a>></span> wrote:<br></span><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><div>Hi Saleem,<br><br></div><span><span>> Why would libc++abi have undefined references, it it doesn't use the functions?<br><br></span></span></div><span><div>Short answer:<br><br>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.<br></div></span></div></blockquote><div><br></div><div>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.</div><span><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div></div><div>Long answer:<br><br></div><div>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 <br></div></div></blockquote><div><br></div></span><div>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.</div><span><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div>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.<br></div></div></blockquote><div><br></div></span><div>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.</div><span><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div></div><div>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.<br><br></div><div>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.<br></div></div></blockquote><div><br></div></span><div>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.</div><div><div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div></div><div>Sincerely,<br></div><div>Logan<br></div><div><br>[1] <a href="http://infocenter.arm.com/help/topic/com.arm.doc.ihi0038a/IHI0038A_ehabi.pdf" target="_blank">http://infocenter.arm.com/help/topic/com.arm.doc.ihi0038a/IHI0038A_ehabi.pdf</a><br>[2] <a href="https://mentorembedded.github.io/cxx-abi/abi-eh.html" target="_blank">https://mentorembedded.github.io/cxx-abi/abi-eh.html</a><br></div><div>[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().<br></div></div><div><div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Mar 17, 2015 at 10:34 AM, Saleem Abdulrasool <span dir="ltr"><<a href="mailto:compnerd@compnerd.org" target="_blank">compnerd@compnerd.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><span>On Sat, Mar 14, 2015 at 8:43 AM, Logan Chien <span dir="ltr"><<a href="mailto:tzuhsiang.chien@gmail.com" target="_blank">tzuhsiang.chien@gmail.com</a>></span> wrote:<br></span><div class="gmail_extra"><div class="gmail_quote"><span><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><div><div>Hi Saleem,<br><br>Similar change has been committed and reverted (see r219629 and r226818.)<br><br></div>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.<br><br></div>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}.<br></div></div></blockquote><div><br></div></span><div>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?</div><div><div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div></div><div>Would you mind to revert this change? Or, is there any other situation, which requires this change, that I haven't considered? Thanks.<br><br></div><div>Sincerely,<br></div><div>Logan<br></div><div><div><br></div></div></div><div><div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Feb 12, 2015 at 12:25 PM, Saleem Abdulrasool <span dir="ltr"><<a href="mailto:compnerd@compnerd.org" target="_blank">compnerd@compnerd.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: compnerd<br>
Date: Wed Feb 11 22:25:03 2015<br>
New Revision: 228903<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=228903&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=228903&view=rev</a><br>
Log:<br>
unwind: move exported APIs out of header<br>
<br>
Ideally, we would do something like inline __declspec(dllexport) to ensure that<br>
the symbol was inlined within libunwind as well as emitted into the final DSO.<br>
This simply moves the definition out of the header to ensure that the *public*<br>
interfaces are defined and exported into the final DSO.<br>
<br>
This change also has "gratuitous" code movement so that the EHABI and generic<br>
implementations are co-located making it easier to find them.<br>
<br>
The movement from the header has one minor change introduced into the code:<br>
additional tracing to mirror the behaviour of the non-EHABI interfaces.<br>
<br>
Modified:<br>
libcxxabi/trunk/include/unwind.h<br>
libcxxabi/trunk/src/Unwind/UnwindLevel1.c<br>
<br>
Modified: libcxxabi/trunk/include/unwind.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/libcxxabi/trunk/include/unwind.h?rev=228903&r1=228902&r2=228903&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/libcxxabi/trunk/include/unwind.h?rev=228903&r1=228902&r2=228903&view=diff</a><br>
==============================================================================<br>
--- libcxxabi/trunk/include/unwind.h (original)<br>
+++ libcxxabi/trunk/include/unwind.h Wed Feb 11 22:25:03 2015<br>
@@ -202,37 +202,13 @@ extern _Unwind_VRS_Result<br>
_Unwind_VRS_Pop(_Unwind_Context *context, _Unwind_VRS_RegClass regclass,<br>
uint32_t discriminator,<br>
_Unwind_VRS_DataRepresentation representation);<br>
+#endif<br>
<br>
-static inline uintptr_t _Unwind_GetGR(struct _Unwind_Context* context,<br>
- int index) {<br>
- uintptr_t value = 0;<br>
- _Unwind_VRS_Get(context, _UVRSC_CORE, (uint32_t)index, _UVRSD_UINT32, &value);<br>
- return value;<br>
-}<br>
-<br>
-static inline void _Unwind_SetGR(struct _Unwind_Context* context, int index,<br>
- uintptr_t new_value) {<br>
- _Unwind_VRS_Set(context, _UVRSC_CORE, (uint32_t)index,<br>
- _UVRSD_UINT32, &new_value);<br>
-}<br>
-<br>
-static inline uintptr_t _Unwind_GetIP(struct _Unwind_Context* context) {<br>
- // remove the thumb-bit before returning<br>
- return (_Unwind_GetGR(context, 15) & (~(uintptr_t)0x1));<br>
-}<br>
-<br>
-static inline void _Unwind_SetIP(struct _Unwind_Context* context,<br>
- uintptr_t new_value) {<br>
- uintptr_t thumb_bit = _Unwind_GetGR(context, 15) & ((uintptr_t)0x1);<br>
- _Unwind_SetGR(context, 15, new_value | thumb_bit);<br>
-}<br>
-#else<br>
extern uintptr_t _Unwind_GetGR(struct _Unwind_Context *context, int index);<br>
extern void _Unwind_SetGR(struct _Unwind_Context *context, int index,<br>
uintptr_t new_value);<br>
extern uintptr_t _Unwind_GetIP(struct _Unwind_Context *context);<br>
extern void _Unwind_SetIP(struct _Unwind_Context *, uintptr_t new_value);<br>
-#endif<br>
<br>
extern uintptr_t _Unwind_GetRegionStart(struct _Unwind_Context *context);<br>
extern uintptr_t<br>
<br>
Modified: libcxxabi/trunk/src/Unwind/UnwindLevel1.c<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/libcxxabi/trunk/src/Unwind/UnwindLevel1.c?rev=228903&r1=228902&r2=228903&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/libcxxabi/trunk/src/Unwind/UnwindLevel1.c?rev=228903&r1=228902&r2=228903&view=diff</a><br>
==============================================================================<br>
--- libcxxabi/trunk/src/Unwind/UnwindLevel1.c (original)<br>
+++ libcxxabi/trunk/src/Unwind/UnwindLevel1.c Wed Feb 11 22:25:03 2015<br>
@@ -422,10 +422,73 @@ _Unwind_GetLanguageSpecificData(struct _<br>
}<br>
<br>
<br>
+/// Called by personality handler during phase 2 to find the start of the<br>
+/// function.<br>
+_LIBUNWIND_EXPORT uintptr_t<br>
+_Unwind_GetRegionStart(struct _Unwind_Context *context) {<br>
+ unw_cursor_t *cursor = (unw_cursor_t *)context;<br>
+ unw_proc_info_t frameInfo;<br>
+ uintptr_t result = 0;<br>
+ if (unw_get_proc_info(cursor, &frameInfo) == UNW_ESUCCESS)<br>
+ result = (uintptr_t)frameInfo.start_ip;<br>
+ _LIBUNWIND_TRACE_API("_Unwind_GetRegionStart(context=%p) => 0x%" PRIxPTR "\n",<br>
+ (void *)context, result);<br>
+ return result;<br>
+}<br>
+<br>
+<br>
+/// Called by personality handler during phase 2 if a foreign exception<br>
+// is caught.<br>
+_LIBUNWIND_EXPORT void<br>
+_Unwind_DeleteException(_Unwind_Exception *exception_object) {<br>
+ _LIBUNWIND_TRACE_API("_Unwind_DeleteException(ex_obj=%p)\n",<br>
+ (void *)exception_object);<br>
+ if (exception_object->exception_cleanup != NULL)<br>
+ (*exception_object->exception_cleanup)(_URC_FOREIGN_EXCEPTION_CAUGHT,<br>
+ exception_object);<br>
+}<br>
+<br>
+#endif // _LIBUNWIND_BUILD_ZERO_COST_APIS && !LIBCXXABI_ARM_EHABI<br>
+<br>
+#if LIBCXXABI_ARM_EHABI<br>
+<br>
+_LIBUNWIND_EXPORT uintptr_t<br>
+_Unwind_GetGR(struct _Unwind_Context *context, int index) {<br>
+ uintptr_t value = 0;<br>
+ _Unwind_VRS_Get(context, _UVRSC_CORE, (uint32_t)index, _UVRSD_UINT32, &value);<br>
+ _LIBUNWIND_TRACE_API("_Unwind_GetGR(context=%p, reg=%d) => 0x%" PRIx64 "\n",<br>
+ (void *)context, index, (uint64_t)value);<br>
+ return value;<br>
+}<br>
+<br>
+_LIBUNWIND_EXPORT void _Unwind_SetGR(struct _Unwind_Context *context, int index,<br>
+ uintptr_t value) {<br>
+ _LIBUNWIND_TRACE_API("_Unwind_SetGR(context=%p, reg=%d, value=0x%0"PRIx64")\n",<br>
+ (void *)context, index, (uint64_t)value);<br>
+ _Unwind_VRS_Set(context, _UVRSC_CORE, (uint32_t)index, _UVRSD_UINT32, &value);<br>
+}<br>
+<br>
+_LIBUNWIND_EXPORT uintptr_t _Unwind_GetIP(struct _Unwind_Context *context) {<br>
+ // remove the thumb-bit before returning<br>
+ uintptr_t value = _Unwind_GetGR(context, 15) & (~(uintptr_t)0x1);<br>
+ _LIBUNWIND_TRACE_API("_Unwind_GetIP(context=%p) => 0x%" PRIx64 "\n",<br>
+ (void *)context, (uint64_t)value);<br>
+ return value;<br>
+}<br>
+<br>
+_LIBUNWIND_EXPORT void _Unwind_SetIP(struct _Unwind_Context *context,<br>
+ uintptr_t value) {<br>
+ _LIBUNWIND_TRACE_API("_Unwind_SetIP(context=%p, value=0x%0" PRIx64 ")\n",<br>
+ (void *)context, (uint64_t)value);<br>
+ uintptr_t thumb_bit = _Unwind_GetGR(context, 15) & ((uintptr_t)0x1);<br>
+ _Unwind_SetGR(context, 15, value | thumb_bit);<br>
+}<br>
+<br>
+#else<br>
<br>
/// Called by personality handler during phase 2 to get register values.<br>
-_LIBUNWIND_EXPORT uintptr_t _Unwind_GetGR(struct _Unwind_Context *context,<br>
- int index) {<br>
+_LIBUNWIND_EXPORT uintptr_t<br>
+_Unwind_GetGR(struct _Unwind_Context *context, int index) {<br>
unw_cursor_t *cursor = (unw_cursor_t *)context;<br>
unw_word_t result;<br>
unw_get_reg(cursor, index, &result);<br>
@@ -434,20 +497,16 @@ _LIBUNWIND_EXPORT uintptr_t _Unwind_GetG<br>
return (uintptr_t)result;<br>
}<br>
<br>
-<br>
-<br>
/// Called by personality handler during phase 2 to alter register values.<br>
_LIBUNWIND_EXPORT void _Unwind_SetGR(struct _Unwind_Context *context, int index,<br>
- uintptr_t new_value) {<br>
+ uintptr_t value) {<br>
_LIBUNWIND_TRACE_API("_Unwind_SetGR(context=%p, reg=%d, value=0x%0" PRIx64<br>
")\n",<br>
- (void *)context, index, (uint64_t)new_value);<br>
+ (void *)context, index, (uint64_t)value);<br>
unw_cursor_t *cursor = (unw_cursor_t *)context;<br>
- unw_set_reg(cursor, index, new_value);<br>
+ unw_set_reg(cursor, index, value);<br>
}<br>
<br>
-<br>
-<br>
/// Called by personality handler during phase 2 to get instruction pointer.<br>
_LIBUNWIND_EXPORT uintptr_t _Unwind_GetIP(struct _Unwind_Context *context) {<br>
unw_cursor_t *cursor = (unw_cursor_t *)context;<br>
@@ -458,44 +517,16 @@ _LIBUNWIND_EXPORT uintptr_t _Unwind_GetI<br>
return (uintptr_t)result;<br>
}<br>
<br>
-<br>
-<br>
/// Called by personality handler during phase 2 to alter instruction pointer,<br>
/// such as setting where the landing pad is, so _Unwind_Resume() will<br>
/// start executing in the landing pad.<br>
_LIBUNWIND_EXPORT void _Unwind_SetIP(struct _Unwind_Context *context,<br>
- uintptr_t new_value) {<br>
+ uintptr_t value) {<br>
_LIBUNWIND_TRACE_API("_Unwind_SetIP(context=%p, value=0x%0" PRIx64 ")\n",<br>
- (void *)context, (uint64_t)new_value);<br>
+ (void *)context, (uint64_t)value);<br>
unw_cursor_t *cursor = (unw_cursor_t *)context;<br>
- unw_set_reg(cursor, UNW_REG_IP, new_value);<br>
+ unw_set_reg(cursor, UNW_REG_IP, value);<br>
}<br>
<br>
+#endif<br>
<br>
-/// Called by personality handler during phase 2 to find the start of the<br>
-/// function.<br>
-_LIBUNWIND_EXPORT uintptr_t<br>
-_Unwind_GetRegionStart(struct _Unwind_Context *context) {<br>
- unw_cursor_t *cursor = (unw_cursor_t *)context;<br>
- unw_proc_info_t frameInfo;<br>
- uintptr_t result = 0;<br>
- if (unw_get_proc_info(cursor, &frameInfo) == UNW_ESUCCESS)<br>
- result = (uintptr_t)frameInfo.start_ip;<br>
- _LIBUNWIND_TRACE_API("_Unwind_GetRegionStart(context=%p) => 0x%" PRIxPTR "\n",<br>
- (void *)context, result);<br>
- return result;<br>
-}<br>
-<br>
-<br>
-/// Called by personality handler during phase 2 if a foreign exception<br>
-// is caught.<br>
-_LIBUNWIND_EXPORT void<br>
-_Unwind_DeleteException(_Unwind_Exception *exception_object) {<br>
- _LIBUNWIND_TRACE_API("_Unwind_DeleteException(ex_obj=%p)\n",<br>
- (void *)exception_object);<br>
- if (exception_object->exception_cleanup != NULL)<br>
- (*exception_object->exception_cleanup)(_URC_FOREIGN_EXCEPTION_CAUGHT,<br>
- exception_object);<br>
-}<br>
-<br>
-#endif // _LIBUNWIND_BUILD_ZERO_COST_APIS && !LIBCXXABI_ARM_EHABI<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br></div>
</div></div></blockquote></div></div></div><span><font color="#888888"><br><br clear="all"><div><br></div>-- <br><div>Saleem Abdulrasool<br>compnerd (at) compnerd (dot) org</div>
</font></span></div></div>
</blockquote></div><br></div>
</div></div></blockquote></div></div></div><div><div><br><br clear="all"><div><br></div>-- <br><div>Saleem Abdulrasool<br>compnerd (at) compnerd (dot) org</div>
</div></div></div></div>
</blockquote></div><br></div>
</div></div></blockquote></div><br></div></div>