diff mbox

[ARM] FreeBSD ARM support, EABI, v3

Message ID 54AEEDCE.8020302@fgznet.ch
State New
Headers show

Commit Message

Andreas Tobler Jan. 8, 2015, 8:51 p.m. UTC
On 08.01.15 17:27, Richard Earnshaw wrote:
> On 29/12/14 18:44, Andreas Tobler wrote:
>> All,
>>
>> here is the third attempt to support ARM with FreeBSD.
>>
>> In the meantime we found another issue in the unwinder where I had to
>> adapt some stuff.
>>
>> The unwind_phase2_forced function in libgcc calls a stop_fn function.
>> This stop_fn is in FreeBSD's libthr implementation and is called
>> thread_unwind_stop. This thread_unwind_stop is a generic function used
>> on all FreeBSD archs.
>>
>> The issue is now that this thread_unwind_stop expects a double int for
>> the exception_class, like on every other arch. For ARM EABI this
>> exception_class is an array of char which is passed in one register as
>> pointer vs. two registers for a double int.
>>
>> To solve this issue we defined the exception_class as double integer for
>> FreeBSD.
>>
>> This adaptation reduced the failure count in libstdc++ by about 40 fails.
>>
>> I build and test this port on a regular basis and I post the results to
>> the usual place.

...

> Umm, sorry, just seen this update to the previous patch.
>
> The changes to the exception unwinding look a bit more involved.  Could
> you separate that out into a separate patch, so that it's easier to see
> what you're changing?

Ok, here the mentioned part as separate diff. The comments are above. 
The CL below :)

Thank you very much!

Andreas

gcc:

	* ginclude/unwind-arm-common.h (_Uwind_Control_Block): Define
	exception_class as double integer for FreeBSD ARM.
	(_Unwind_Exception): Define _Unwind_Exception_Class as double integer
	for FreeBSD ARM.

libstc++-v3:

	* libsupc++/unwind-cxx.h (__is_gxx_exception_class,
	__is_dependent_exception): Exclude FreeBSD ARM from the
	__ARM_EABI_UNWINDER__ ifdef.

Comments

Ramana Radhakrishnan Jan. 13, 2015, 10:25 a.m. UTC | #1
On Thu, Jan 8, 2015 at 8:51 PM, Andreas Tobler <andreast-list@fgznet.ch> wrote:
> On 08.01.15 17:27, Richard Earnshaw wrote:
>>
>> On 29/12/14 18:44, Andreas Tobler wrote:
>>>
>>> All,
>>>
>>> here is the third attempt to support ARM with FreeBSD.
>>>
>>> In the meantime we found another issue in the unwinder where I had to
>>> adapt some stuff.
>>>
>>> The unwind_phase2_forced function in libgcc calls a stop_fn function.
>>> This stop_fn is in FreeBSD's libthr implementation and is called
>>> thread_unwind_stop. This thread_unwind_stop is a generic function used
>>> on all FreeBSD archs.
>>>
>>> The issue is now that this thread_unwind_stop expects a double int for
>>> the exception_class, like on every other arch. For ARM EABI this
>>> exception_class is an array of char which is passed in one register as
>>> pointer vs. two registers for a double int.
>>>
>>> To solve this issue we defined the exception_class as double integer for
>>> FreeBSD.

My apologies for the slow response, some other work and then holidays
intervened.

From my understanding of the ABI document the implementation is
currently as mandated by the ABI. Also this isn't a part of the ABI
that's available for the platform (here FreeBSD to manipulate and
change as per it's wishes). ARM EHABI is special for software, making
FreeBSD more "special" for ARM appears to be counter intuitive from my
point of view. A number of exception unwinding libraries. for e.g.
libobjc , libstdc++ all use this implementation of exception_class.
Therefore this creates a divergence for the FreeBSD port which is
different from everything else. I expect that a number of language run
time support libraries that supported the ARM EHABI would be using
such an implementation, therefore you need to fix every single
implementation of this in every unwinder that supports the ARM EHABI
which I expect to have been ported to in a number of libraries
already. (I already see this in libobjc and libstdc++ in the GCC tree)

I would rather fix the thread_unwind_stop implementation in libthr for
ARM EHABI rather than make this change.


>>>
>>> This adaptation reduced the failure count in libstdc++ by about 40 fails.
>>>
>>> I build and test this port on a regular basis and I post the results to
>>> the usual place.

Thanks for doing this. I'm really glad that FreeBSD is finally moving to EABI.


regards
Ramana

>
>
> ...
>
>> Umm, sorry, just seen this update to the previous patch.
>>
>> The changes to the exception unwinding look a bit more involved.  Could
>> you separate that out into a separate patch, so that it's easier to see
>> what you're changing?
>
>
> Ok, here the mentioned part as separate diff. The comments are above. The CL
> below :)
>
> Thank you very much!
>
> Andreas
>
> gcc:
>
>         * ginclude/unwind-arm-common.h (_Uwind_Control_Block): Define
>         exception_class as double integer for FreeBSD ARM.
>         (_Unwind_Exception): Define _Unwind_Exception_Class as double
> integer
>         for FreeBSD ARM.
>
> libstc++-v3:
>
>
>         * libsupc++/unwind-cxx.h (__is_gxx_exception_class,
>         __is_dependent_exception): Exclude FreeBSD ARM from the
>         __ARM_EABI_UNWINDER__ ifdef.
>
>
Andreas Tobler Jan. 13, 2015, 9:08 p.m. UTC | #2
On 13.01.15 11:25, Ramana Radhakrishnan wrote:
> On Thu, Jan 8, 2015 at 8:51 PM, Andreas Tobler <andreast-list@fgznet.ch> wrote:
>> On 08.01.15 17:27, Richard Earnshaw wrote:
>>>
>>> On 29/12/14 18:44, Andreas Tobler wrote:
>>>>
>>>> All,
>>>>
>>>> here is the third attempt to support ARM with FreeBSD.
>>>>
>>>> In the meantime we found another issue in the unwinder where I had to
>>>> adapt some stuff.
>>>>
>>>> The unwind_phase2_forced function in libgcc calls a stop_fn function.
>>>> This stop_fn is in FreeBSD's libthr implementation and is called
>>>> thread_unwind_stop. This thread_unwind_stop is a generic function used
>>>> on all FreeBSD archs.
>>>>
>>>> The issue is now that this thread_unwind_stop expects a double int for
>>>> the exception_class, like on every other arch. For ARM EABI this
>>>> exception_class is an array of char which is passed in one register as
>>>> pointer vs. two registers for a double int.
>>>>
>>>> To solve this issue we defined the exception_class as double integer for
>>>> FreeBSD.
>
> My apologies for the slow response, some other work and then holidays
> intervened.

Np, the only issue which made me hurry was the stage 4 entering this week.

>>From my understanding of the ABI document the implementation is
> currently as mandated by the ABI. Also this isn't a part of the ABI
> that's available for the platform (here FreeBSD to manipulate and
> change as per it's wishes). ARM EHABI is special for software, making
> FreeBSD more "special" for ARM appears to be counter intuitive from my
> point of view. A number of exception unwinding libraries. for e.g.
> libobjc , libstdc++ all use this implementation of exception_class.
> Therefore this creates a divergence for the FreeBSD port which is
> different from everything else. I expect that a number of language run
> time support libraries that supported the ARM EHABI would be using
> such an implementation, therefore you need to fix every single
> implementation of this in every unwinder that supports the ARM EHABI
> which I expect to have been ported to in a number of libraries
> already. (I already see this in libobjc and libstdc++ in the GCC tree)

Grr ;) I didn't want to hear this answer, but I expected it somehow.
My proposal was the least effort for me.
The other way round is going to be very hard. Maybe impossible.

It is not only FreeBSD which is affected but also llvm and friends. They 
use for exception_class uint64_t.

I have to take a picture how the effort is and if it would be possible 
to do such a change in FreeBSD and more important in llvm etc.

> I would rather fix the thread_unwind_stop implementation in libthr for
> ARM EHABI rather than make this change.

It wouldn't be a 'fix' but more a wrapper I think.

>>>> This adaptation reduced the failure count in libstdc++ by about 40 fails.
>>>>
>>>> I build and test this port on a regular basis and I post the results to
>>>> the usual place.
>
> Thanks for doing this. I'm really glad that FreeBSD is finally moving to EABI.

Thanks for the review and the feedback.

Gruss,
Andreas
Richard Earnshaw Jan. 14, 2015, 9:43 a.m. UTC | #3
On 13/01/15 21:08, Andreas Tobler wrote:
> On 13.01.15 11:25, Ramana Radhakrishnan wrote:
>> On Thu, Jan 8, 2015 at 8:51 PM, Andreas Tobler <andreast-list@fgznet.ch> wrote:
>>> On 08.01.15 17:27, Richard Earnshaw wrote:
>>>>
>>>> On 29/12/14 18:44, Andreas Tobler wrote:
>>>>>
>>>>> All,
>>>>>
>>>>> here is the third attempt to support ARM with FreeBSD.
>>>>>
>>>>> In the meantime we found another issue in the unwinder where I had to
>>>>> adapt some stuff.
>>>>>
>>>>> The unwind_phase2_forced function in libgcc calls a stop_fn function.
>>>>> This stop_fn is in FreeBSD's libthr implementation and is called
>>>>> thread_unwind_stop. This thread_unwind_stop is a generic function used
>>>>> on all FreeBSD archs.
>>>>>
>>>>> The issue is now that this thread_unwind_stop expects a double int for
>>>>> the exception_class, like on every other arch. For ARM EABI this
>>>>> exception_class is an array of char which is passed in one register as
>>>>> pointer vs. two registers for a double int.
>>>>>
>>>>> To solve this issue we defined the exception_class as double integer for
>>>>> FreeBSD.
>>
>> My apologies for the slow response, some other work and then holidays
>> intervened.
> 
> Np, the only issue which made me hurry was the stage 4 entering this week.
> 
>> >From my understanding of the ABI document the implementation is
>> currently as mandated by the ABI. Also this isn't a part of the ABI
>> that's available for the platform (here FreeBSD to manipulate and
>> change as per it's wishes). ARM EHABI is special for software, making
>> FreeBSD more "special" for ARM appears to be counter intuitive from my
>> point of view. A number of exception unwinding libraries. for e.g.
>> libobjc , libstdc++ all use this implementation of exception_class.
>> Therefore this creates a divergence for the FreeBSD port which is
>> different from everything else. I expect that a number of language run
>> time support libraries that supported the ARM EHABI would be using
>> such an implementation, therefore you need to fix every single
>> implementation of this in every unwinder that supports the ARM EHABI
>> which I expect to have been ported to in a number of libraries
>> already. (I already see this in libobjc and libstdc++ in the GCC tree)
> 
> Grr ;) I didn't want to hear this answer, but I expected it somehow.
> My proposal was the least effort for me.
> The other way round is going to be very hard. Maybe impossible.
> 
> It is not only FreeBSD which is affected but also llvm and friends. They 
> use for exception_class uint64_t.
> 
> I have to take a picture how the effort is and if it would be possible 
> to do such a change in FreeBSD and more important in llvm etc.
> 
>> I would rather fix the thread_unwind_stop implementation in libthr for
>> ARM EHABI rather than make this change.
> 
> It wouldn't be a 'fix' but more a wrapper I think.
> 
>>>>> This adaptation reduced the failure count in libstdc++ by about 40 fails.
>>>>>
>>>>> I build and test this port on a regular basis and I post the results to
>>>>> the usual place.
>>
>> Thanks for doing this. I'm really glad that FreeBSD is finally moving to EABI.
> 

I agree with Ramana on this one, I feel that FreeBSD trying to plough a
slightly different furrow is just going to cause major problems for
everyone.

I don't believe a claim that LLVM can't be made to work with the
standard EHABI data structures.  It can do this on Linux, so I can't
conceive of any reason why it cannot also do so on FreeBSD.

R.

> Thanks for the review and the feedback.
> 
> Gruss,
> Andreas
> 
>
Andreas Tobler Jan. 15, 2015, 10:42 p.m. UTC | #4
On 14.01.15 10:43, Richard Earnshaw wrote:
> On 13/01/15 21:08, Andreas Tobler wrote:
>> On 13.01.15 11:25, Ramana Radhakrishnan wrote:
>>> On Thu, Jan 8, 2015 at 8:51 PM, Andreas Tobler <andreast-list@fgznet.ch> wrote:
>>>> On 08.01.15 17:27, Richard Earnshaw wrote:
>>>>>
>>>>> On 29/12/14 18:44, Andreas Tobler wrote:
>>>>>>
>>>>>> All,
>>>>>>
>>>>>> here is the third attempt to support ARM with FreeBSD.
>>>>>>
>>>>>> In the meantime we found another issue in the unwinder where I had to
>>>>>> adapt some stuff.
>>>>>>
>>>>>> The unwind_phase2_forced function in libgcc calls a stop_fn function.
>>>>>> This stop_fn is in FreeBSD's libthr implementation and is called
>>>>>> thread_unwind_stop. This thread_unwind_stop is a generic function used
>>>>>> on all FreeBSD archs.
>>>>>>
>>>>>> The issue is now that this thread_unwind_stop expects a double int for
>>>>>> the exception_class, like on every other arch. For ARM EABI this
>>>>>> exception_class is an array of char which is passed in one register as
>>>>>> pointer vs. two registers for a double int.
>>>>>>
>>>>>> To solve this issue we defined the exception_class as double integer for
>>>>>> FreeBSD.
>>>
>>> My apologies for the slow response, some other work and then holidays
>>> intervened.
>>
>> Np, the only issue which made me hurry was the stage 4 entering this week.
>>
>>> >From my understanding of the ABI document the implementation is
>>> currently as mandated by the ABI. Also this isn't a part of the ABI
>>> that's available for the platform (here FreeBSD to manipulate and
>>> change as per it's wishes). ARM EHABI is special for software, making
>>> FreeBSD more "special" for ARM appears to be counter intuitive from my
>>> point of view. A number of exception unwinding libraries. for e.g.
>>> libobjc , libstdc++ all use this implementation of exception_class.
>>> Therefore this creates a divergence for the FreeBSD port which is
>>> different from everything else. I expect that a number of language run
>>> time support libraries that supported the ARM EHABI would be using
>>> such an implementation, therefore you need to fix every single
>>> implementation of this in every unwinder that supports the ARM EHABI
>>> which I expect to have been ported to in a number of libraries
>>> already. (I already see this in libobjc and libstdc++ in the GCC tree)
>>
>> Grr ;) I didn't want to hear this answer, but I expected it somehow.
>> My proposal was the least effort for me.
>> The other way round is going to be very hard. Maybe impossible.
>>
>> It is not only FreeBSD which is affected but also llvm and friends. They
>> use for exception_class uint64_t.
>>
>> I have to take a picture how the effort is and if it would be possible
>> to do such a change in FreeBSD and more important in llvm etc.
>>
>>> I would rather fix the thread_unwind_stop implementation in libthr for
>>> ARM EHABI rather than make this change.
>>
>> It wouldn't be a 'fix' but more a wrapper I think.
>>
>>>>>> This adaptation reduced the failure count in libstdc++ by about 40 fails.
>>>>>>
>>>>>> I build and test this port on a regular basis and I post the results to
>>>>>> the usual place.
>>>
>>> Thanks for doing this. I'm really glad that FreeBSD is finally moving to EABI.
>>
>
> I agree with Ramana on this one, I feel that FreeBSD trying to plough a
> slightly different furrow is just going to cause major problems for
> everyone.
>
> I don't believe a claim that LLVM can't be made to work with the
> standard EHABI data structures.  It can do this on Linux, so I can't
> conceive of any reason why it cannot also do so on FreeBSD.

Thanks for the feedback, both! I'm busy to get a picture and I come back 
as soon I have a plan.

It is not a technical problem, it is, from my point of view, a 
distribution issue. Iow, we 'shipped' ABI/API X and we want/have to 
change to ABI/API Y.
So it takes a while till I get the plan :)

Andreas
diff mbox

Patch

Index: gcc/ginclude/unwind-arm-common.h
===================================================================
--- gcc/ginclude/unwind-arm-common.h	(revision 219357)
+++ gcc/ginclude/unwind-arm-common.h	(working copy)
@@ -82,7 +82,11 @@ 
 
   struct _Unwind_Control_Block
     {
+#ifdef __FreeBSD__
+      unsigned exception_class __attribute__((__mode__(__DI__)));
+#else
       char exception_class[8];
+#endif
       void (*exception_cleanup)(_Unwind_Reason_Code, _Unwind_Control_Block *);
       /* Unwinder cache, private fields for the unwinder's use */
       struct
@@ -181,7 +185,11 @@ 
 
   /* Support functions for the PR.  */
 #define _Unwind_Exception _Unwind_Control_Block
+#ifdef __FreeBSD__
+  typedef unsigned _Unwind_Exception_Class __attribute__((__mode__(__DI__)));
+#else
   typedef char _Unwind_Exception_Class[8];
+#endif
 
   void * _Unwind_GetLanguageSpecificData (_Unwind_Context *);
   _Unwind_Ptr _Unwind_GetRegionStart (_Unwind_Context *);
Index: libstdc++-v3/libsupc++/unwind-cxx.h
===================================================================
--- libstdc++-v3/libsupc++/unwind-cxx.h	(revision 219357)
+++ libstdc++-v3/libsupc++/unwind-cxx.h	(working copy)
@@ -235,7 +235,7 @@ 
   return reinterpret_cast<__cxa_dependent_exception *>(exc + 1) - 1;
 }
 
-#ifdef __ARM_EABI_UNWINDER__
+#if defined (__ARM_EABI_UNWINDER__) && !defined (__FreeBSD__)
 static inline bool
 __is_gxx_exception_class(_Unwind_Exception_Class c)
 {
@@ -309,13 +309,7 @@ 
   c[6] = 'R';
   c[7] = '\0';
 }
-
-static inline void*
-__gxx_caught_object(_Unwind_Exception* eo)
-{
-  return (void*)eo->barrier_cache.bitpattern[0];
-}
-#else // !__ARM_EABI_UNWINDER__
+#else // !__ARM_EABI_UNWINDER__ || __FreeBSD__
 // This is the primary exception class we report -- "GNUCC++\0".
 const _Unwind_Exception_Class __gxx_primary_exception_class
 = ((((((((_Unwind_Exception_Class) 'G' 
@@ -339,6 +333,16 @@ 
     << 8 | (_Unwind_Exception_Class) '+')
    << 8 | (_Unwind_Exception_Class) '\x01');
 
+const _Unwind_Exception_Class __gxx_forced_unwind_class
+= ((((((((_Unwind_Exception_Class) 'G'
+        << 8 | (_Unwind_Exception_Class) 'N')
+       << 8 | (_Unwind_Exception_Class) 'U')
+      << 8 | (_Unwind_Exception_Class) 'C')
+     << 8 | (_Unwind_Exception_Class) 'F')
+    << 8 | (_Unwind_Exception_Class) 'O')
+   << 8 | (_Unwind_Exception_Class) 'R')
+  << 8 | (_Unwind_Exception_Class) '\0');
+
 static inline bool
 __is_gxx_exception_class(_Unwind_Exception_Class c)
 {
@@ -346,6 +350,12 @@ 
       || c == __gxx_dependent_exception_class;
 }
 
+static inline bool
+__is_gxx_forced_unwind_class(_Unwind_Exception_Class c)
+{
+  return c ==  __gxx_forced_unwind_class;
+}
+
 // Only checks for primary or dependent, but not that it is a C++ exception at
 // all.
 static inline bool
@@ -357,7 +367,18 @@ 
 #define __GXX_INIT_PRIMARY_EXCEPTION_CLASS(c) c = __gxx_primary_exception_class
 #define __GXX_INIT_DEPENDENT_EXCEPTION_CLASS(c) \
   c = __gxx_dependent_exception_class
+#define __GXX_INIT_FORCED_UNWIND_CLASS(c) c = __gxx_forced_unwind_class
+#endif // __ARM_EABI_UNWINDER__ && !__FreeBSD__
 
+#ifdef __ARM_EABI_UNWINDER__
+static inline void*
+__gxx_caught_object(_Unwind_Exception* eo)
+{
+    return (void*)eo->barrier_cache.bitpattern[0];
+}
+
+#else // !__ARM_EABI_UNWINDER__
+
 // GNU C++ personality routine, Version 0.
 extern "C" _Unwind_Reason_Code __gxx_personality_v0
      (int, _Unwind_Action, _Unwind_Exception_Class,