From b0c80631a8c99169b9b563bbe11e688a41afb4fa Mon Sep 17 00:00:00 2001 From: David Chisnall Date: Sat, 2 May 2020 12:56:04 +0100 Subject: [PATCH] More EH fixes. libsupc++ is more aggressive about internal consistency checks than libcxxrt, so we need to be more careful in the interop. The tests from PR #138 now pass for me on Debian with libsupc++. --- eh_personality.c | 100 +++++++++++++++++++++++++++++++---------------- objcxx_eh.cc | 1 - 2 files changed, 67 insertions(+), 34 deletions(-) diff --git a/eh_personality.c b/eh_personality.c index eaf00602..8a27bc5e 100644 --- a/eh_personality.c +++ b/eh_personality.c @@ -22,6 +22,13 @@ void test_cxx_eh_implementation(); +// Weak references to C++ runtime functions. We don't bother testing that +// these are 0 before calling them, because if they are not resolved then we +// should not be in a code path that involves a C++ exception. +__attribute__((weak)) void *__cxa_begin_catch(void *e); +__attribute__((weak)) void __cxa_end_catch(void); +__attribute__((weak)) void __cxa_rethrow(void); + /** * Class of exceptions to distinguish between this and other exception types. @@ -75,6 +82,35 @@ typedef enum handler_class } handler_type; +enum exception_type +{ + NONE, + CXX, + OBJC, + FOREIGN, + BOXED_FOREIGN +}; +struct thread_data +{ + enum exception_type current_exception_type; + id lastThrownObject; + BOOL cxxCaughtException; + struct objc_exception *caughtExceptions; +}; + +static __thread struct thread_data thread_data; + +static struct thread_data *get_thread_data(void) +{ + return &thread_data; +} + +static struct thread_data *get_thread_data_fast(void) +{ + return &thread_data; +} + + /** * Saves the result of the landing pad that we have found. For ARM, this is * stored in the generic unwind structure, while on other platforms it is @@ -147,6 +183,9 @@ static void cleanup(_Unwind_Reason_Code reason, struct _Unwind_Exception *e) unwindHeader))); */ } + +void objc_exception_rethrow(struct _Unwind_Exception *e); + /** * Throws an Objective-C exception. This function is, unfortunately, used for * rethrowing caught exceptions too, even in @finally() blocks. Unfortunately, @@ -154,6 +193,25 @@ static void cleanup(_Unwind_Reason_Code reason, struct _Unwind_Exception *e) */ void objc_exception_throw(id object) { + struct thread_data *td = get_thread_data(); + fprintf(stderr, "Throwing %p, in flight exception: %p\n", object, td->lastThrownObject); + fprintf(stderr, "Exception caught by C++: %d\n", td->cxxCaughtException); + // If C++ caught the exception, then we may need to make C++ rethrow it if + // we want to preserve exception state. Rethrows should be handled with + // objc_exception_rethrow, but clang appears to do the wrong thing for some + // cases. + if (td->cxxCaughtException) + { + // For catchalls, we may result in our being passed the pointer to the + // object, not the object. + if ((object == td->lastThrownObject) || + ((object != nil) && + !isSmallObject(object) && + (*(id*)object == td->lastThrownObject))) + { + __cxa_rethrow(); + } + } SEL rethrow_sel = sel_registerName("rethrow"); if ((nil != object) && @@ -175,6 +233,9 @@ void objc_exception_throw(id object) ex->object = object; + td->lastThrownObject = object; + td->cxxCaughtException = NO; + _Unwind_Reason_Code err = _Unwind_RaiseException(&ex->unwindHeader); free(ex); if (_URC_END_OF_STACK == err && 0 != _objc_unexpected_exception) @@ -480,6 +541,7 @@ static inline _Unwind_Reason_Code internal_objc_personality(int version, _Unwind_SetGR(context, __builtin_eh_return_data_regno(1), selector); DEBUG_LOG("Installing context, selector %d\n", (int)selector); + get_thread_data()->cxxCaughtException = NO; return _URC_INSTALL_CONTEXT; } @@ -513,48 +575,20 @@ BEGIN_PERSONALITY_FUNCTION(__gnustep_objcxx_personality_v0) int ret = CALL_PERSONALITY_FUNCTION(__gxx_personality_v0); exceptionObject->private_1 = ex->cxx_exception->private_1; exceptionObject->private_2 = ex->cxx_exception->private_2; + if (ret == _URC_INSTALL_CONTEXT) + { + get_thread_data()->cxxCaughtException = YES; + } return ret; } return CALL_PERSONALITY_FUNCTION(__gxx_personality_v0); } -// Weak references to C++ runtime functions. We don't bother testing that -// these are 0 before calling them, because if they are not resolved then we -// should not be in a code path that involves a C++ exception. -__attribute__((weak)) void *__cxa_begin_catch(void *e); -__attribute__((weak)) void __cxa_end_catch(void); -__attribute__((weak)) void __cxa_rethrow(void); - -enum exception_type -{ - NONE, - CXX, - OBJC, - FOREIGN, - BOXED_FOREIGN -}; -struct thread_data -{ - enum exception_type current_exception_type; - struct objc_exception *caughtExceptions; -}; - -static __thread struct thread_data thread_data; - -static struct thread_data *get_thread_data(void) -{ - return &thread_data; -} - -static struct thread_data *get_thread_data_fast(void) -{ - return &thread_data; -} - id objc_begin_catch(struct _Unwind_Exception *exceptionObject) { struct thread_data *td = get_thread_data(); DEBUG_LOG("Beginning catch %p\n", exceptionObject); + td->cxxCaughtException = NO; if (exceptionObject->exception_class == objc_exception_class) { td->current_exception_type = OBJC; diff --git a/objcxx_eh.cc b/objcxx_eh.cc index a60c8ced..bc4740b4 100644 --- a/objcxx_eh.cc +++ b/objcxx_eh.cc @@ -266,7 +266,6 @@ namespace gnustep const __class_type_info *target, void **thrown_object) const { - assert(0); return false; }; };