diff mbox

[ARM] fix C++ EH interoperability

Message ID 4DDA82AB.3060805@codesourcery.com
State New
Headers show

Commit Message

Nathan Sidwell May 23, 2011, 3:52 p.m. UTC 
This patch fixes an interoperability issue with code generated by ARM's EABI 
compiler.

Unlike the generic C++ ABI, which always catches pointers by value, ARM's ABI 
only catches pointers by value when there's the possibility of derived->base 
conversion happening.  The ARM __cxa_type_matcher can return one of 3 values to 
indicate the three possibilities.  GCC was returning the success value when a 
pointer matched exactly, but returning the pointer by value.  This is 
incompatible with EABI compliant code, which expects a reference to the thrown 
pointer in such cases.

However, we cannot simply change the GCC type matcher to return a reference in 
such cases, as then it will be incompatible with existing GCC-compiled code. 
GCC compiled code expects the pointer value at the landing pad, ARM-compiled 
code expects a reference to the pointer value.  Hence ARM-compiled code 
(usually) inserts an additional dereference.

Note that the compiler generating the catching code, specifies the personality 
routine (unwinder), but the code doing the type-matching could come from any 
compliant library.

In discussion with ARM, I developed the attached patch, which changes 
__cxa_type_match to return the succeeded_with_ptr_to_base enum value whenever a 
pointer is caught (rather than only when a pointer-to-derived is converted to a 
pointer-to-base).  This value indicates to the (ARM) unwinding machinery that 
the pointer value is being returned. And makes the type matcher work with the 
ARM unwinder.  We do not change the conditions under which _cxa_type_match 
performs the pointer indirection.

At the other side of the interface, we have to make GCC's implementation of the 
EABI unwinder work with an ARM-provided type matcher.  Here, the unwinder notes 
the return value from _cxa_match_type, and inserts an additional indirection by 
using a spare field in the barrier_cache structure.  Fortunately GCC-generated 
code doesn't use the common unwinder, but uses a gnu personality routine, which 
is unchanged.

This patch results has been tested for arm-linux, and independently tested by 
ARM with mixed RVCT-generated code confirming the defect has been fixed.

ok?

nathan

Comments

Andrew Haley May 23, 2011, 3:54 p.m. UTC | #1 
On 05/23/2011 04:52 PM, Nathan Sidwell wrote:
> This patch fixes an interoperability issue with code generated by ARM's EABI 
> compiler.

> This patch results has been tested for arm-linux, and independently tested by 
> ARM with mixed RVCT-generated code confirming the defect has been fixed.
> 
> ok?

What did the Java test results look like?

Andrew.
Nathan Sidwell May 25, 2011, 1:40 p.m. UTC | #2 
On 05/23/11 16:54, Andrew Haley wrote:
> On 05/23/2011 04:52 PM, Nathan Sidwell wrote:
>> This patch fixes an interoperability issue with code generated by ARM's EABI
>> compiler.
>
>> This patch results has been tested for arm-linux, and independently tested by
>> ARM with mixed RVCT-generated code confirming the defect has been fixed.
>>
>> ok?
>
> What did the Java test results look like?

They are unchanged.

nathan
Nathan Sidwell June 15, 2011, 2:02 p.m. UTC | #3 
On 05/23/11 16:52, Nathan Sidwell wrote:
> This patch fixes an interoperability issue with code generated by ARM's EABI
> compiler.

ping?
Richard Earnshaw June 29, 2011, 1:40 p.m. UTC | #4 
On 23/05/11 16:52, Nathan Sidwell wrote:
> This patch fixes an interoperability issue with code generated by ARM's
> EABI compiler.
> 
> Unlike the generic C++ ABI, which always catches pointers by value,
> ARM's ABI only catches pointers by value when there's the possibility of
> derived->base conversion happening.  The ARM __cxa_type_matcher can
> return one of 3 values to indicate the three possibilities.  GCC was
> returning the success value when a pointer matched exactly, but
> returning the pointer by value.  This is incompatible with EABI
> compliant code, which expects a reference to the thrown pointer in such
> cases.
> 
> However, we cannot simply change the GCC type matcher to return a
> reference in such cases, as then it will be incompatible with existing
> GCC-compiled code. GCC compiled code expects the pointer value at the
> landing pad, ARM-compiled code expects a reference to the pointer
> value.  Hence ARM-compiled code (usually) inserts an additional
> dereference.
> 
> Note that the compiler generating the catching code, specifies the
> personality routine (unwinder), but the code doing the type-matching
> could come from any compliant library.
> 
> In discussion with ARM, I developed the attached patch, which changes
> __cxa_type_match to return the succeeded_with_ptr_to_base enum value
> whenever a pointer is caught (rather than only when a pointer-to-derived
> is converted to a pointer-to-base).  This value indicates to the (ARM)
> unwinding machinery that the pointer value is being returned. And makes
> the type matcher work with the ARM unwinder.  We do not change the
> conditions under which _cxa_type_match performs the pointer indirection.
> 
> At the other side of the interface, we have to make GCC's implementation
> of the EABI unwinder work with an ARM-provided type matcher.  Here, the
> unwinder notes the return value from _cxa_match_type, and inserts an
> additional indirection by using a spare field in the barrier_cache
> structure.  Fortunately GCC-generated code doesn't use the common
> unwinder, but uses a gnu personality routine, which is unchanged.
> 
> This patch results has been tested for arm-linux, and independently
> tested by ARM with mixed RVCT-generated code confirming the defect has
> been fixed.
> 
> ok?
> 
> nathan
> 
> 
> arm-eh.patch
> 
> 
> 2011-05-23  Nathan Sidwell  <nathan@codesourcery.com>
> 
> 	gcc/
> 	* config/arm/unwind-arm.c (enum __cxa_type_match_result): New.
> 	(cxa_type_match): Correct declaration.
> 	(__gnu_unwind_pr_common): Reconstruct
> 	additional indirection when __cxa_type_match returns
> 	succeeded_with_ptr_to_base.
> 
> 	libstdc++/
> 	* libsupc++/eh_arm.c (__cxa_type_match): Construct address of
> 	thrown object here.  Return succeded_with_ptr_to_base for all
> 	pointer cases.
> 

OK.

R.
diff mbox

Patch

2011-05-23  Nathan Sidwell  <nathan@codesourcery.com>

	gcc/
	* config/arm/unwind-arm.c (enum __cxa_type_match_result): New.
	(cxa_type_match): Correct declaration.
	(__gnu_unwind_pr_common): Reconstruct
	additional indirection when __cxa_type_match returns
	succeeded_with_ptr_to_base.

	libstdc++/
	* libsupc++/eh_arm.c (__cxa_type_match): Construct address of
	thrown object here.  Return succeded_with_ptr_to_base for all
	pointer cases.

Index: libstdc++-v3/libsupc++/eh_arm.cc
===================================================================
--- libstdc++-v3/libsupc++/eh_arm.cc	(revision 173851)
+++ libstdc++-v3/libsupc++/eh_arm.cc	(working copy)
@@ -30,10 +30,11 @@ 
 using namespace __cxxabiv1;
 
 
-// Given the thrown type THROW_TYPE, pointer to a variable containing a
-// pointer to the exception object THROWN_PTR_P and a type CATCH_TYPE to
-// compare against, return whether or not there is a match and if so,
-// update *THROWN_PTR_P.
+// Given the thrown type THROW_TYPE, exception object UE_HEADER and a
+// type CATCH_TYPE to compare against, return whether or not there is
+// a match and if so, update *THROWN_PTR_P to point to either the
+// type-matched object, or in the case of a pointer type, the object
+// pointed to by the pointer.
 
 extern "C" __cxa_type_match_result
 __cxa_type_match(_Unwind_Exception* ue_header,
@@ -41,51 +42,51 @@  __cxa_type_match(_Unwind_Exception* ue_h
 		 bool is_reference __attribute__((__unused__)),
 		 void** thrown_ptr_p)
 {
-  bool forced_unwind = __is_gxx_forced_unwind_class(ue_header->exception_class);
-  bool foreign_exception = !forced_unwind && !__is_gxx_exception_class(ue_header->exception_class);
-  bool dependent_exception =
-    __is_dependent_exception(ue_header->exception_class);
+  bool forced_unwind
+    = __is_gxx_forced_unwind_class(ue_header->exception_class);
+  bool foreign_exception
+    = !forced_unwind && !__is_gxx_exception_class(ue_header->exception_class);
+  bool dependent_exception
+    = __is_dependent_exception(ue_header->exception_class);
   __cxa_exception* xh = __get_exception_header_from_ue(ue_header);
   __cxa_dependent_exception *dx = __get_dependent_exception_from_ue(ue_header);
   const std::type_info* throw_type;
+  void *thrown_ptr = 0;
 
   if (forced_unwind)
     throw_type = &typeid(abi::__forced_unwind);
   else if (foreign_exception)
     throw_type = &typeid(abi::__foreign_exception);
-  else if (dependent_exception)
-    throw_type = __get_exception_header_from_obj
-      (dx->primaryException)->exceptionType;
   else
-    throw_type = xh->exceptionType;
-
-  void* thrown_ptr = *thrown_ptr_p;
+    {
+      if (dependent_exception)
+	xh = __get_exception_header_from_obj (dx->primaryException);
+      throw_type = xh->exceptionType;
+      // We used to require the caller set the target of thrown_ptr_p,
+      // but that's incorrect -- the EHABI makes no such requirement
+      // -- and not all callers will set it.  Fortunately callers that
+      // do initialize will always pass us the value we calculate
+      // here, so there's no backwards compatibility problem.
+      thrown_ptr = __get_object_from_ue (ue_header);
+    }
+  
+  __cxa_type_match_result result = ctm_succeeded;
 
   // Pointer types need to adjust the actual pointer, not
   // the pointer to pointer that is the exception object.
   // This also has the effect of passing pointer types
   // "by value" through the __cxa_begin_catch return value.
   if (throw_type->__is_pointer_p())
-    thrown_ptr = *(void**) thrown_ptr;
+    {
+      thrown_ptr = *(void**) thrown_ptr;
+      // We need to indicate the indirection to our caller.
+      result = ctm_succeeded_with_ptr_to_base;
+    }
 
   if (catch_type->__do_catch(throw_type, &thrown_ptr, 1))
     {
       *thrown_ptr_p = thrown_ptr;
-
-      if (typeid(*catch_type) == typeid (typeid(void*)))
-	{
-	  const __pointer_type_info *catch_pointer_type =
-	    static_cast<const __pointer_type_info *> (catch_type);
-	  const __pointer_type_info *throw_pointer_type =
-	    static_cast<const __pointer_type_info *> (throw_type);
-
-	  if (typeid (*catch_pointer_type->__pointee) != typeid (void)
-	      && (*catch_pointer_type->__pointee != 
-		  *throw_pointer_type->__pointee))
-	    return ctm_succeeded_with_ptr_to_base;
-	}
-
-      return ctm_succeeded;
+      return result;
     }
 
   return ctm_failed;
Index: gcc/config/arm/unwind-arm.c
===================================================================
--- gcc/config/arm/unwind-arm.c	(revision 173851)
+++ gcc/config/arm/unwind-arm.c	(working copy)
@@ -32,13 +32,18 @@  extern void abort (void);
 typedef unsigned char bool;
 
 typedef struct _ZSt9type_info type_info; /* This names C++ type_info type */
+enum __cxa_type_match_result
+  {
+    ctm_failed = 0,
+    ctm_succeeded = 1,
+    ctm_succeeded_with_ptr_to_base = 2
+  };
 
 void __attribute__((weak)) __cxa_call_unexpected(_Unwind_Control_Block *ucbp);
 bool __attribute__((weak)) __cxa_begin_cleanup(_Unwind_Control_Block *ucbp);
-bool __attribute__((weak)) __cxa_type_match(_Unwind_Control_Block *ucbp,
-					    const type_info *rttip,
-					    bool is_reference,
-					    void **matched_object);
+enum __cxa_type_match_result __attribute__((weak)) __cxa_type_match
+  (_Unwind_Control_Block *ucbp, const type_info *rttip,
+   bool is_reference, void **matched_object);
 
 _Unwind_Ptr __attribute__((weak))
 __gnu_Unwind_Find_exidx (_Unwind_Ptr, int *);
@@ -1107,6 +1112,7 @@  __gnu_unwind_pr_common (_Unwind_State st
 		      _uw rtti;
 		      bool is_reference = (data[0] & uint32_highbit) != 0;
 		      void *matched;
+		      enum __cxa_type_match_result match_type;
 
 		      /* Check for no-throw areas.  */
 		      if (data[1] == (_uw) -2)
@@ -1118,17 +1124,31 @@  __gnu_unwind_pr_common (_Unwind_State st
 			{
 			  /* Match a catch specification.  */
 			  rtti = _Unwind_decode_target2 ((_uw) &data[1]);
-			  if (!__cxa_type_match (ucbp, (type_info *) rtti,
-						 is_reference,
-						 &matched))
-			    matched = (void *)0;
+			  match_type = __cxa_type_match (ucbp,
+							 (type_info *) rtti,
+							 is_reference,
+							 &matched);
 			}
+		      else
+			match_type = ctm_succeeded;
 
-		      if (matched)
+		      if (match_type)
 			{
 			  ucbp->barrier_cache.sp =
 			    _Unwind_GetGR (context, R_SP);
-			  ucbp->barrier_cache.bitpattern[0] = (_uw) matched;
+			  // ctm_succeeded_with_ptr_to_base really
+			  // means _c_t_m indirected the pointer
+			  // object.  We have to reconstruct the
+			  // additional pointer layer by using a temporary.
+			  if (match_type == ctm_succeeded_with_ptr_to_base)
+			    {
+			      ucbp->barrier_cache.bitpattern[2]
+				= (_uw) matched;
+			      ucbp->barrier_cache.bitpattern[0]
+				= (_uw) &ucbp->barrier_cache.bitpattern[2];
+			    }
+			  else
+			    ucbp->barrier_cache.bitpattern[0] = (_uw) matched;
 			  ucbp->barrier_cache.bitpattern[1] = (_uw) data;
 			  return _URC_HANDLER_FOUND;
 			}