From 5f01917a498951e883a91c86d02d7d201b05a0bd Mon Sep 17 00:00:00 2001 From: Zenny Chen Date: Sun, 19 Nov 2017 04:27:25 +0800 Subject: [PATCH 01/26] Update blocks_runtime.h (#45) Update blocks_runtime.h to be compatible with Apple / LLVM version. --- blocks_runtime.m | 12 ++++++------ objc/blocks_runtime.h | 10 +++++----- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/blocks_runtime.m b/blocks_runtime.m index 0ba7961..844d487 100644 --- a/blocks_runtime.m +++ b/blocks_runtime.m @@ -42,9 +42,9 @@ static void *_HeapBlockByRef = (void*)1; /** * Returns the Objective-C type encoding for the block. */ -const char *block_getType_np(void *b) +const char *block_getType_np(const void *b) { - struct Block_layout *block = b; + const struct Block_layout *block = b; if ((NULL == block) || !(block->flags & BLOCK_HAS_SIGNATURE)) { return NULL; @@ -231,10 +231,10 @@ void _Block_object_dispose(const void *object, const int flags) // Copy a block to the heap if it's still on the stack or increments its retain count. -void *_Block_copy(void *src) +void *_Block_copy(const void *src) { if (NULL == src) { return NULL; } - struct Block_layout *self = src; + struct Block_layout *self = (struct Block_layout*)src; struct Block_layout *ret = self; extern void _NSConcreteStackBlock; @@ -265,10 +265,10 @@ void *_Block_copy(void *src) } // Release a block and frees the memory when the retain count hits zero. -void _Block_release(void *src) +void _Block_release(const void *src) { if (NULL == src) { return; } - struct Block_layout *self = src; + struct Block_layout *self = (struct Block_layout*)src; extern void _NSConcreteStackBlock; extern void _NSConcreteMallocBlock; diff --git a/objc/blocks_runtime.h b/objc/blocks_runtime.h index e5dbdea..3834f54 100644 --- a/objc/blocks_runtime.h +++ b/objc/blocks_runtime.h @@ -13,9 +13,9 @@ #define BLOCKS_EXPORT extern #endif -BLOCKS_EXPORT void *_Block_copy(void *); -BLOCKS_EXPORT void _Block_release(void *); -BLOCKS_EXPORT const char *block_getType_np(void *b) OBJC_NONPORTABLE; +BLOCKS_EXPORT void *_Block_copy(const void *); +BLOCKS_EXPORT void _Block_release(const void *); +BLOCKS_EXPORT const char *block_getType_np(const void *b) OBJC_NONPORTABLE; -#define Block_copy(x) ((__typeof(x))_Block_copy((void *)(x))) -#define Block_release(x) _Block_release((void *)(x)) +#define Block_copy(x) ((__typeof(x))_Block_copy((const void *)(x))) +#define Block_release(x) _Block_release((const void *)(x)) From 6451d35e929c80fe4249761959186b382c7c5558 Mon Sep 17 00:00:00 2001 From: David Chisnall Date: Mon, 27 Nov 2017 16:32:54 +0000 Subject: [PATCH 02/26] Fix block refcounts to use saturating arithmetic. --- blocks_runtime.m | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/blocks_runtime.m b/blocks_runtime.m index 844d487..74e2163 100644 --- a/blocks_runtime.m +++ b/blocks_runtime.m @@ -60,8 +60,10 @@ static int increment24(int *ref) { int old = *ref; int val = old & BLOCK_REFCOUNT_MASK; - // FIXME: We should gracefully handle refcount overflow, but for now we - // just give up + if (val == BLOCK_REFCOUNT_MASK) + { + return val; + } assert(val < BLOCK_REFCOUNT_MASK); if (!__sync_bool_compare_and_swap(ref, old, old+1)) { @@ -74,8 +76,10 @@ static int decrement24(int *ref) { int old = *ref; int val = old & BLOCK_REFCOUNT_MASK; - // FIXME: We should gracefully handle refcount overflow, but for now we - // just give up + if (val == BLOCK_REFCOUNT_MASK) + { + return val; + } assert(val > 0); if (!__sync_bool_compare_and_swap(ref, old, old-1)) { From a2a80ac5dc099bf15900e9b55148961750d115dc Mon Sep 17 00:00:00 2001 From: David Chisnall Date: Tue, 28 Nov 2017 12:42:29 +0000 Subject: [PATCH 03/26] Remove debugging printf. --- associate.m | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/associate.m b/associate.m index 3db8495..37b9c26 100644 --- a/associate.m +++ b/associate.m @@ -247,7 +247,7 @@ static void deallocHiddenClass(id obj, SEL _cmd) DESTROY_LOCK(&list->lock); cleanupReferenceList(list); freeReferenceList(list->next); - fprintf(stderr, "Deallocating dtable %p\n", hiddenClass->dtable); + //fprintf(stderr, "Deallocating dtable %p\n", hiddenClass->dtable); free_dtable(hiddenClass->dtable); // We shouldn't have any subclasses left at this point assert(hiddenClass->subclass_list == 0); From af4e0719ff73d737bd40507d688533fdb11ad641 Mon Sep 17 00:00:00 2001 From: David Chisnall Date: Tue, 12 Dec 2017 16:01:53 +0000 Subject: [PATCH 04/26] Checkpoint work on improving weak reference efficiency. --- arc.m | 158 +++++++++++++++++++++++++++++++++++++++--------- objc/objc-arc.h | 4 +- 2 files changed, 134 insertions(+), 28 deletions(-) diff --git a/arc.m b/arc.m index b7a3b3b..24f8b7c 100644 --- a/arc.m +++ b/arc.m @@ -1,4 +1,5 @@ #include +#include #include #import "stdio.h" #import "objc/runtime.h" @@ -163,6 +164,17 @@ extern BOOL FastARCAutorelease; static BOOL useARCAutoreleasePool; +/** + * We use the top bit of the reference count to indicate whether an object has + * ever had a weak reference taken. This lets us avoid acquiring the weak + * table lock for most objects on deallocation. + */ +static const long weak_mask = ((size_t)1)<<((sizeof(size_t)*8)-1); +/** + * All of the bits other than the top bit are the real reference count. + */ +static const long refcount_mask = ~weak_mask; + static inline id retain(id obj) { if (isSmallObject(obj)) { return obj; } @@ -174,14 +186,40 @@ static inline id retain(id obj) } if (objc_test_class_flag(cls, objc_class_flag_fast_arc)) { - intptr_t *refCount = ((intptr_t*)obj) - 1; - // Note: this should be an atomic read, so that a sufficiently clever - // compiler doesn't notice that there's no happens-before relationship - // here. - if (*refCount >= 0) - { - __sync_add_and_fetch(refCount, 1); - } + uintptr_t *refCount = ((uintptr_t*)obj) - 1; + uintptr_t refCountVal = __sync_fetch_and_add(refCount, 0); + uintptr_t newVal = refCountVal; + do { + refCountVal = newVal; + long realCount = refCountVal & refcount_mask; + // If this object's reference count is already less than 0, then + // this is a spurious retain. This can happen when one thread is + // attempting to acquire a strong reference from a weak reference + // and the other thread is attempting to destroy it. The + // deallocating thread will decrement the reference count with no + // locks held and will then acquire the weak ref table lock and + // attempt to zero the weak references. The caller of this will be + // `objc_loadWeakRetained`, which will also hold the lock. If the + // serialisation is such that the locked retain happens after the + // decrement, then we return nil here so that the weak-to-strong + // transition doesn't happen and the object is actually destroyed. + // If the serialisation happens the other way, then the locked + // check of the reference count will happen after we've referenced + // this and we don't zero the references or deallocate. + if (realCount < 0) + { + return nil; + } + // If the reference count is saturated, don't increment it. + if (realCount == refcount_mask) + { + return obj; + } + realCount++; + realCount |= refCountVal & weak_mask; + uintptr_t updated = (uintptr_t)realCount; + newVal = __sync_val_compare_and_swap(refCount, refCountVal, updated); + } while (newVal != refCountVal); return obj; } return [obj retain]; @@ -203,12 +241,37 @@ static inline void release(id obj) } if (objc_test_class_flag(cls, objc_class_flag_fast_arc)) { - intptr_t *refCount = ((intptr_t*)obj) - 1; + uintptr_t *refCount = ((uintptr_t*)obj) - 1; + uintptr_t refCountVal = __sync_fetch_and_add(refCount, 0); + uintptr_t newVal = refCountVal; + bool isWeak; + bool shouldFree; + do { + refCountVal = newVal; + size_t realCount = refCountVal & refcount_mask; + // If the reference count is saturated, don't decrement it. + if (realCount == refcount_mask) + { + return; + } + realCount--; + isWeak = (refCountVal & weak_mask) == weak_mask; + shouldFree = realCount == -1; + realCount |= refCountVal & weak_mask; + uintptr_t updated = (uintptr_t)realCount; + newVal = __sync_val_compare_and_swap(refCount, refCountVal, updated); + } while (newVal != refCountVal); // We allow refcounts to run into the negative, but should only // deallocate once. - if (__sync_sub_and_fetch(refCount, 1) == -1) + if (shouldFree) { - objc_delete_weak_refs(obj); + if (isWeak) + { + if (!objc_delete_weak_refs(obj)) + { + return; + } + } [obj dealloc]; } return; @@ -524,6 +587,7 @@ void* block_load_weak(void *block); id objc_storeWeak(id *addr, id obj) { + LOCK_FOR_SCOPE(&weakRefLock); id old = *addr; BOOL isGlobalObject = (obj == nil) || isSmallObject(obj); @@ -538,16 +602,44 @@ id objc_storeWeak(id *addr, id obj) isGlobalObject = YES; } } - if (cls && objc_test_class_flag(cls, objc_class_flag_fast_arc)) + if (obj && cls && objc_test_class_flag(cls, objc_class_flag_fast_arc)) { - intptr_t *refCount = ((intptr_t*)obj) - 1; - if (obj && *refCount < 0) + uintptr_t *refCount = ((uintptr_t*)obj) - 1; + if (obj) { - obj = nil; - cls = Nil; + uintptr_t refCountVal = __sync_fetch_and_add(refCount, 0); + uintptr_t newVal = refCountVal; + do { + refCountVal = newVal; + long realCount = refCountVal & refcount_mask; + // If this object has already been deallocated (or is in the + // process of being deallocated) then don't bother storing it. + if (realCount < 0) + { + obj = nil; + cls = Nil; + break; + } + // The weak ref flag is monotonic (it is set, never cleared) so + // don't bother trying to re-set it. + if ((refCountVal & weak_mask) == weak_mask) + { + break; + } + // Set the flag in the reference count to indicate that a weak + // reference has been taken. + // + // We currently hold the weak ref lock, so another thread + // racing to deallocate this object will have to wait to do so + // if we manage to do the reference count update first. This + // shouldn't be possible, because `obj` should be a strong + // reference and so it shouldn't be possible to deallocate it + // while we're assigning it. + uintptr_t updated = ((uintptr_t)realCount | weak_mask); + newVal = __sync_val_compare_and_swap(refCount, refCountVal, updated); + } while (newVal != refCountVal); } } - LOCK_FOR_SCOPE(&weakRefLock); if (nil != old) { WeakRef *oldRef = weak_ref_table_get(weakRefs, old); @@ -583,7 +675,8 @@ id objc_storeWeak(id *addr, id obj) } else if (objc_test_class_flag(cls, objc_class_flag_fast_arc)) { - if ((*(((intptr_t*)obj) - 1)) < 0) + uintptr_t *refCount = ((uintptr_t*)obj) - 1; + if ((long)((__sync_fetch_and_add(refCount, 0) & refcount_mask)) < 0) { return nil; } @@ -648,20 +741,38 @@ static void zeroRefs(WeakRef *ref, BOOL shouldFree) } } -void objc_delete_weak_refs(id obj) +BOOL objc_delete_weak_refs(id obj) { LOCK_FOR_SCOPE(&weakRefLock); + if (objc_test_class_flag(classForObject(obj), objc_class_flag_fast_arc)) + { + // If another thread has done a load of a weak reference, then it will + // have incremented the reference count with the lock held. It may + // have done so in between this thread's decrementing the reference + // count and its acquiring the lock. In this case, report failure. + uintptr_t *refCount = ((uintptr_t*)obj) - 1; + if ((long)((__sync_fetch_and_add(refCount, 0) & refcount_mask)) < 0) + { + return NO; + } + } WeakRef *oldRef = weak_ref_table_get(weakRefs, obj); if (0 != oldRef) { zeroRefs(oldRef, NO); weak_ref_remove(weakRefs, obj); } + return YES; } id objc_loadWeakRetained(id* addr) { LOCK_FOR_SCOPE(&weakRefLock); + // *addr can only be zeroed by another thread if it holds the weakRefLock. + // It is possible for another thread to zero the reference count here, but + // it will then acquire the weak ref lock prior to zeroing the weak + // references and deallocating the object. If this thread has incremented + // the reference count, then it will skip deallocating. id obj = *addr; if (nil == obj) { return nil; } // Small objects don't need reference count modification @@ -674,14 +785,7 @@ id objc_loadWeakRetained(id* addr) { obj = block_load_weak(obj); } - else if (objc_test_class_flag(cls, objc_class_flag_fast_arc)) - { - if ((*(((intptr_t*)obj) - 1)) < 0) - { - return nil; - } - } - else + else if (!objc_test_class_flag(cls, objc_class_flag_fast_arc)) { obj = _objc_weak_load(obj); } diff --git a/objc/objc-arc.h b/objc/objc-arc.h index 0da4030..203cfef 100644 --- a/objc/objc-arc.h +++ b/objc/objc-arc.h @@ -94,9 +94,11 @@ void objc_release(id obj); * weak pointers will return 0. This function should be called in -release, * before calling [self dealloc]. * + * This will return `YES` if the weak references were deleted, `NO` otherwise. + * * Nonstandard extension. */ -void objc_delete_weak_refs(id obj); +BOOL objc_delete_weak_refs(id obj); /** * Returns the total number of objects in the ARC-managed autorelease pool. */ From fe579e3f1a99c1e47a86038b5b8b9ab0da058961 Mon Sep 17 00:00:00 2001 From: David Chisnall Date: Wed, 13 Dec 2017 14:07:19 +0000 Subject: [PATCH 05/26] Simplify the weak code. Rather than tracking all of the locations of weak pointers, keep a weak refcount. This should also make it easier to add finer-grained locking to weak references. --- arc.m | 205 ++++++++++++++++++++++++++-------------------------------- 1 file changed, 90 insertions(+), 115 deletions(-) diff --git a/arc.m b/arc.m index 24f8b7c..8875bcc 100644 --- a/arc.m +++ b/arc.m @@ -530,17 +530,19 @@ id objc_storeStrong(id *addr, id value) // Weak references //////////////////////////////////////////////////////////////////////////////// +static int weakref_class; + typedef struct objc_weak_ref { + void *isa; id obj; - id *ref[4]; - struct objc_weak_ref *next; + size_t weak_count; } WeakRef; -static int weak_ref_compare(const id obj, const WeakRef weak_ref) +static int weak_ref_compare(const id obj, const WeakRef *weak_ref) { - return obj == weak_ref.obj; + return obj == weak_ref->obj; } static uint32_t ptr_hash(const void *ptr) @@ -549,23 +551,14 @@ static uint32_t ptr_hash(const void *ptr) // always be 0, which is not so useful for a hash value return ((uintptr_t)ptr >> 4) | ((uintptr_t)ptr << ((sizeof(id) * 8) - 4)); } -static int weak_ref_hash(const WeakRef weak_ref) -{ - return ptr_hash(weak_ref.obj); -} -static int weak_ref_is_null(const WeakRef weak_ref) +static int weak_ref_hash(const WeakRef *weak_ref) { - return weak_ref.obj == NULL; + return ptr_hash(weak_ref->obj); } -const static WeakRef NullWeakRef; #define MAP_TABLE_NAME weak_ref #define MAP_TABLE_COMPARE_FUNCTION weak_ref_compare #define MAP_TABLE_HASH_KEY ptr_hash #define MAP_TABLE_HASH_VALUE weak_ref_hash -#define MAP_TABLE_VALUE_TYPE struct objc_weak_ref -#define MAP_TABLE_VALUE_NULL weak_ref_is_null -#define MAP_TABLE_VALUE_PLACEHOLDER NullWeakRef -#define MAP_TABLE_ACCESS_BY_REFERENCE 1 #define MAP_TABLE_SINGLE_THREAD 1 #define MAP_TABLE_NO_LOCK 1 @@ -583,13 +576,56 @@ PRIVATE void init_arc(void) #endif } +/** + * Load from a weak pointer and return whether this really was a weak + * reference or a strong (not deallocatable) object in a weak pointer. The + * object will be stored in `obj` and the weak reference in `ref`, if one + * exists. + */ +__attribute__((always_inline)) +static BOOL loadWeakPointer(id *addr, id *obj, WeakRef **ref) +{ + id oldObj = *addr; + if (oldObj == nil) + { + *ref = NULL; + *obj = nil; + return NO; + } + if (classForObject(oldObj) == (Class)&weakref_class) + { + *ref = (WeakRef*)oldObj; + *obj = (*ref)->obj; + return YES; + } + *ref = NULL; + *obj = oldObj; + return NO; +} + +__attribute__((always_inline)) +static inline void weakRefRelease(WeakRef *ref) +{ + ref->weak_count--; + if (ref->weak_count == 0) + { + free(ref); + } +} + void* block_load_weak(void *block); id objc_storeWeak(id *addr, id obj) { LOCK_FOR_SCOPE(&weakRefLock); - id old = *addr; - + WeakRef *oldRef; + id old; + loadWeakPointer(addr, &old, &oldRef); + // If the old and new values are the same, then we don't need to do anything. + if (old == obj) + { + return obj; + } BOOL isGlobalObject = (obj == nil) || isSmallObject(obj); Class cls = Nil; if (!isGlobalObject) @@ -640,23 +676,13 @@ id objc_storeWeak(id *addr, id obj) } while (newVal != refCountVal); } } - if (nil != old) + // If we old ref exists, decrement its reference count. This may also + // delete the weak reference control block. + if (oldRef != NULL) { - WeakRef *oldRef = weak_ref_table_get(weakRefs, old); - while (NULL != oldRef) - { - for (int i=0 ; i<4 ; i++) - { - if (oldRef->ref[i] == addr) - { - oldRef->ref[i] = 0; - oldRef = 0; - break; - } - } - oldRef = (oldRef == NULL) ? NULL : oldRef->next; - } + weakRefRelease(oldRef); } + // If we're storing nil, then just write a null pointer. if (nil == obj) { *addr = obj; @@ -665,82 +691,30 @@ id objc_storeWeak(id *addr, id obj) if (isGlobalObject) { // If this is a global object, it's never deallocated, so secretly make - // this a strong reference + // this a strong reference. *addr = obj; return obj; } - if (&_NSConcreteMallocBlock == cls) - { - obj = block_load_weak(obj); - } - else if (objc_test_class_flag(cls, objc_class_flag_fast_arc)) - { - uintptr_t *refCount = ((uintptr_t*)obj) - 1; - if ((long)((__sync_fetch_and_add(refCount, 0) & refcount_mask)) < 0) - { - return nil; - } - } - else - { - obj = _objc_weak_load(obj); - } if (nil != obj) { WeakRef *ref = weak_ref_table_get(weakRefs, obj); - while (NULL != ref) + if (ref == NULL) { - for (int i=0 ; i<4 ; i++) - { - if (0 == ref->ref[i]) - { - ref->ref[i] = addr; - *addr = obj; - return obj; - } - } - if (ref->next == NULL) - { - break; - } - ref = ref->next; - } - if (NULL != ref) - { - ref->next = calloc(sizeof(WeakRef), 1); - ref->next->ref[0] = addr; + ref = calloc(1, sizeof(WeakRef)); + ref->isa = (Class)&weakref_class; + ref->obj = obj; + ref->weak_count = 1; + weak_ref_insert(weakRefs, ref); } else { - WeakRef newRef = {0}; - newRef.obj = obj; - newRef.ref[0] = addr; - weak_ref_insert(weakRefs, newRef); + ref->weak_count++; } + *addr = (id)ref; } - *addr = obj; return obj; } -static void zeroRefs(WeakRef *ref, BOOL shouldFree) -{ - if (NULL != ref->next) - { - zeroRefs(ref->next, YES); - } - for (int i=0 ; i<4 ; i++) - { - if (0 != ref->ref[i]) - { - *ref->ref[i] = 0; - } - } - if (shouldFree) - { - free(ref); - } -} - BOOL objc_delete_weak_refs(id obj) { LOCK_FOR_SCOPE(&weakRefLock); @@ -759,8 +733,16 @@ BOOL objc_delete_weak_refs(id obj) WeakRef *oldRef = weak_ref_table_get(weakRefs, obj); if (0 != oldRef) { - zeroRefs(oldRef, NO); + // Zero the object pointer. This prevents any other weak + // accesses from loading from this. + oldRef->obj = nil; + // The address of obj is likely to be reused, so remove it from + // the table so that we don't accidentally alias weak + // references weak_ref_remove(weakRefs, obj); + // If the weak reference count is zero, then we should have + // already removed this. + assert(oldRef->weak_count > 0); } return YES; } @@ -768,18 +750,21 @@ BOOL objc_delete_weak_refs(id obj) id objc_loadWeakRetained(id* addr) { LOCK_FOR_SCOPE(&weakRefLock); - // *addr can only be zeroed by another thread if it holds the weakRefLock. - // It is possible for another thread to zero the reference count here, but - // it will then acquire the weak ref lock prior to zeroing the weak - // references and deallocating the object. If this thread has incremented - // the reference count, then it will skip deallocating. - id obj = *addr; - if (nil == obj) { return nil; } - // Small objects don't need reference count modification - if (isSmallObject(obj)) + id obj; + WeakRef *ref; + // If this is really a strong reference (nil, or an non-deallocatable + // object), just return it. + if (!loadWeakPointer(addr, &obj, &ref)) { return obj; } + // The object cannot be deallocated while we hold the lock (release + // will acquire the lock before attempting to deallocate) + if (obj == nil) + { + weakRefRelease(ref); + return nil; + } Class cls = classForObject(obj); if (&_NSConcreteMallocBlock == cls) { @@ -808,20 +793,10 @@ void objc_moveWeak(id *dest, id *src) // the object can't be deallocated, so we just move the value and update // the weak reference table entry to indicate the new address. LOCK_FOR_SCOPE(&weakRefLock); + WeakRef *oldRef = weak_ref_table_get(weakRefs, *dest); *dest = *src; *src = nil; - WeakRef *oldRef = weak_ref_table_get(weakRefs, *dest); - while (NULL != oldRef) - { - for (int i=0 ; i<4 ; i++) - { - if (oldRef->ref[i] == src) - { - oldRef->ref[i] = dest; - return; - } - } - } + weakRefRelease(oldRef); } void objc_destroyWeak(id* obj) From bdc20a8babf3f3b350abae01557cfa937c720e79 Mon Sep 17 00:00:00 2001 From: David Chisnall Date: Wed, 13 Dec 2017 15:38:20 +0000 Subject: [PATCH 06/26] Add some debugging help for ARC-compilance. --- dtable.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/dtable.c b/dtable.c index 4e39562..4aa63e0 100644 --- a/dtable.c +++ b/dtable.c @@ -56,6 +56,13 @@ static BOOL ownsMethod(Class cls, SEL sel) return NO; } + +#ifdef DEBUG_ARC_COMPAT +#define ARC_DEBUG_LOG(...) fprintf(stderr, __VA_LIST__) +#else +#define ARC_DEBUG_LOG(...) do {} while(0) +#endif + /** * Checks whether the class implements memory management methods, and whether * they are safe to use with ARC. @@ -73,18 +80,21 @@ static void checkARCAccessors(Class cls) struct objc_slot *slot = objc_get_slot(cls, retain); if ((NULL != slot) && !ownsMethod(slot->owner, isARC)) { + ARC_DEBUG_LOG("%s does not support ARC correctly (implements retain)\n", cls->name); objc_clear_class_flag(cls, objc_class_flag_fast_arc); return; } slot = objc_get_slot(cls, release); if ((NULL != slot) && !ownsMethod(slot->owner, isARC)) { + ARC_DEBUG_LOG("%s does not support ARC correctly (implements release)\n", cls->name); objc_clear_class_flag(cls, objc_class_flag_fast_arc); return; } slot = objc_get_slot(cls, autorelease); if ((NULL != slot) && !ownsMethod(slot->owner, isARC)) { + ARC_DEBUG_LOG("%s does not support ARC correctly (implements autorelease)\n", cls->name); objc_clear_class_flag(cls, objc_class_flag_fast_arc); return; } From 458bd3c7a2b4e9b57a906b09bdc56138a6523011 Mon Sep 17 00:00:00 2001 From: David Chisnall Date: Wed, 13 Dec 2017 16:09:12 +0000 Subject: [PATCH 07/26] Add interfaces for better integration with Foundation. --- arc.m | 154 +++++++++++++++++++++++++++--------------------- objc/objc-arc.h | 26 ++++++++ 2 files changed, 112 insertions(+), 68 deletions(-) diff --git a/arc.m b/arc.m index 8875bcc..d12a044 100644 --- a/arc.m +++ b/arc.m @@ -175,6 +175,45 @@ static const long weak_mask = ((size_t)1)<<((sizeof(size_t)*8)-1); */ static const long refcount_mask = ~weak_mask; +id objc_retain_fast_np(id obj) +{ + uintptr_t *refCount = ((uintptr_t*)obj) - 1; + uintptr_t refCountVal = __sync_fetch_and_add(refCount, 0); + uintptr_t newVal = refCountVal; + do { + refCountVal = newVal; + long realCount = refCountVal & refcount_mask; + // If this object's reference count is already less than 0, then + // this is a spurious retain. This can happen when one thread is + // attempting to acquire a strong reference from a weak reference + // and the other thread is attempting to destroy it. The + // deallocating thread will decrement the reference count with no + // locks held and will then acquire the weak ref table lock and + // attempt to zero the weak references. The caller of this will be + // `objc_loadWeakRetained`, which will also hold the lock. If the + // serialisation is such that the locked retain happens after the + // decrement, then we return nil here so that the weak-to-strong + // transition doesn't happen and the object is actually destroyed. + // If the serialisation happens the other way, then the locked + // check of the reference count will happen after we've referenced + // this and we don't zero the references or deallocate. + if (realCount < 0) + { + return nil; + } + // If the reference count is saturated, don't increment it. + if (realCount == refcount_mask) + { + return obj; + } + realCount++; + realCount |= refCountVal & weak_mask; + uintptr_t updated = (uintptr_t)realCount; + newVal = __sync_val_compare_and_swap(refCount, refCountVal, updated); + } while (newVal != refCountVal); + return obj; +} + static inline id retain(id obj) { if (isSmallObject(obj)) { return obj; } @@ -186,43 +225,55 @@ static inline id retain(id obj) } if (objc_test_class_flag(cls, objc_class_flag_fast_arc)) { - uintptr_t *refCount = ((uintptr_t*)obj) - 1; - uintptr_t refCountVal = __sync_fetch_and_add(refCount, 0); - uintptr_t newVal = refCountVal; - do { - refCountVal = newVal; - long realCount = refCountVal & refcount_mask; - // If this object's reference count is already less than 0, then - // this is a spurious retain. This can happen when one thread is - // attempting to acquire a strong reference from a weak reference - // and the other thread is attempting to destroy it. The - // deallocating thread will decrement the reference count with no - // locks held and will then acquire the weak ref table lock and - // attempt to zero the weak references. The caller of this will be - // `objc_loadWeakRetained`, which will also hold the lock. If the - // serialisation is such that the locked retain happens after the - // decrement, then we return nil here so that the weak-to-strong - // transition doesn't happen and the object is actually destroyed. - // If the serialisation happens the other way, then the locked - // check of the reference count will happen after we've referenced - // this and we don't zero the references or deallocate. - if (realCount < 0) - { - return nil; - } - // If the reference count is saturated, don't increment it. - if (realCount == refcount_mask) + return objc_retain_fast_np(obj); + } + return [obj retain]; +} + +BOOL objc_release_fast_no_destroy_np(id obj) +{ + uintptr_t *refCount = ((uintptr_t*)obj) - 1; + uintptr_t refCountVal = __sync_fetch_and_add(refCount, 0); + uintptr_t newVal = refCountVal; + bool isWeak; + bool shouldFree; + do { + refCountVal = newVal; + size_t realCount = refCountVal & refcount_mask; + // If the reference count is saturated, don't decrement it. + if (realCount == refcount_mask) + { + return NO; + } + realCount--; + isWeak = (refCountVal & weak_mask) == weak_mask; + shouldFree = realCount == -1; + realCount |= refCountVal & weak_mask; + uintptr_t updated = (uintptr_t)realCount; + newVal = __sync_val_compare_and_swap(refCount, refCountVal, updated); + } while (newVal != refCountVal); + // We allow refcounts to run into the negative, but should only + // deallocate once. + if (shouldFree) + { + if (isWeak) + { + if (!objc_delete_weak_refs(obj)) { - return obj; + return NO; } - realCount++; - realCount |= refCountVal & weak_mask; - uintptr_t updated = (uintptr_t)realCount; - newVal = __sync_val_compare_and_swap(refCount, refCountVal, updated); - } while (newVal != refCountVal); - return obj; + } + return YES; + } + return NO; +} + +void objc_release_fast_np(id obj) +{ + if (objc_release_fast_no_destroy_np(obj)) + { + [obj dealloc]; } - return [obj retain]; } static inline void release(id obj) @@ -241,39 +292,7 @@ static inline void release(id obj) } if (objc_test_class_flag(cls, objc_class_flag_fast_arc)) { - uintptr_t *refCount = ((uintptr_t*)obj) - 1; - uintptr_t refCountVal = __sync_fetch_and_add(refCount, 0); - uintptr_t newVal = refCountVal; - bool isWeak; - bool shouldFree; - do { - refCountVal = newVal; - size_t realCount = refCountVal & refcount_mask; - // If the reference count is saturated, don't decrement it. - if (realCount == refcount_mask) - { - return; - } - realCount--; - isWeak = (refCountVal & weak_mask) == weak_mask; - shouldFree = realCount == -1; - realCount |= refCountVal & weak_mask; - uintptr_t updated = (uintptr_t)realCount; - newVal = __sync_val_compare_and_swap(refCount, refCountVal, updated); - } while (newVal != refCountVal); - // We allow refcounts to run into the negative, but should only - // deallocate once. - if (shouldFree) - { - if (isWeak) - { - if (!objc_delete_weak_refs(obj)) - { - return; - } - } - [obj dealloc]; - } + objc_release_fast_np(obj); return; } [obj release]; @@ -375,7 +394,6 @@ unsigned long objc_arc_autorelease_count_for_object_np(id obj) return count; } - void *objc_autoreleasePoolPush(void) { initAutorelease(); diff --git a/objc/objc-arc.h b/objc/objc-arc.h index 203cfef..75dde84 100644 --- a/objc/objc-arc.h +++ b/objc/objc-arc.h @@ -32,6 +32,12 @@ id objc_loadWeakRetained(id* obj); * Retains the argument. Equivalent to [obj retain]. */ id objc_retain(id obj); +/** + * Retains the argument, assuming that the argument is a normal object and has + * its reference count managed by the runtime. + * This is intended to implement `-retain` in ARC-compatible root classes. + */ +id objc_retain_fast_np(id obj) OBJC_NONPORTABLE; /** * Retains and autoreleases an object. Equivalent to [[obj retain] autorelease]. */ @@ -85,6 +91,26 @@ void objc_destroyWeak(id* addr); * Equivalent to objc_copyWeak(), but may also set src to nil. */ void objc_moveWeak(id *dest, id *src); +/** + * Releases the argument, assuming that the argument is a normal object and has + * its reference count managed by the runtime. If the retain count reaches + * zero then all weak references will be zeroed and the object will be + * destroyed. + * + * This is intended to implement `-release` in ARC-compatible root + * classes. + */ +void objc_release_fast_np(id obj) OBJC_NONPORTABLE; +/** + * Releases the argument, assuming that the argument is a normal object and has + * its reference count managed by the runtime. If the retain count reaches + * zero then all weak references will be zeroed but the object will *NOT* be + * destroyed. + * + * This is intended to implement `NSDecrementExtraRefCountWasZero` for use with + * ARC-compatible classes. + */ +BOOL objc_release_fast_no_destroy_np(id obj) OBJC_NONPORTABLE; /** * Releases an object. Equivalent to [obj release]. */ From e5b4468867c9eb576adbae45e4521fbd848a923e Mon Sep 17 00:00:00 2001 From: David Chisnall Date: Wed, 13 Dec 2017 16:28:22 +0000 Subject: [PATCH 08/26] Improve copying and moving weak refs. Fixes a bug introduced (replacing a similar bug) in the last rewrite of objc_moveWeak. This version should now be correct. objc_copyWeak was implemented in a naive way, which had a lot more overhead than required. --- arc.m | 29 ++++++++++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/arc.m b/arc.m index d12a044..397265e 100644 --- a/arc.m +++ b/arc.m @@ -802,7 +802,24 @@ id objc_loadWeak(id* object) void objc_copyWeak(id *dest, id *src) { - objc_release(objc_initWeak(dest, objc_loadWeakRetained(src))); + // Don't retain or release. While the weak ref lock is held, we know that + // the object can't be deallocated, so we just move the value and update + // the weak reference table entry to indicate the new address. + LOCK_FOR_SCOPE(&weakRefLock); + id obj; + WeakRef *srcRef; + WeakRef *dstRef; + loadWeakPointer(dest, &obj, &dstRef); + loadWeakPointer(src, &obj, &srcRef); + *dest = *src; + if (srcRef) + { + srcRef->weak_count++; + } + if (dstRef) + { + weakRefRelease(dstRef); + } } void objc_moveWeak(id *dest, id *src) @@ -811,10 +828,16 @@ void objc_moveWeak(id *dest, id *src) // the object can't be deallocated, so we just move the value and update // the weak reference table entry to indicate the new address. LOCK_FOR_SCOPE(&weakRefLock); - WeakRef *oldRef = weak_ref_table_get(weakRefs, *dest); + id obj; + WeakRef *oldRef; + // If the destination is a weak ref, free it. + loadWeakPointer(dest, &obj, &oldRef); *dest = *src; *src = nil; - weakRefRelease(oldRef); + if (oldRef != NULL) + { + weakRefRelease(oldRef); + } } void objc_destroyWeak(id* obj) From 12b820cb996e7c928d8cad1403cb81ef0e27b223 Mon Sep 17 00:00:00 2001 From: David Chisnall Date: Wed, 13 Dec 2017 17:22:22 +0000 Subject: [PATCH 09/26] Add refcount accessor function. --- arc.m | 7 +++++++ objc/objc-arc.h | 4 ++++ 2 files changed, 11 insertions(+) diff --git a/arc.m b/arc.m index 397265e..4775beb 100644 --- a/arc.m +++ b/arc.m @@ -175,6 +175,13 @@ static const long weak_mask = ((size_t)1)<<((sizeof(size_t)*8)-1); */ static const long refcount_mask = ~weak_mask; +size_t object_getRetainCount_np(id obj) +{ + uintptr_t *refCount = ((uintptr_t*)obj) - 1; + uintptr_t refCountVal = __sync_fetch_and_add(refCount, 0); + return (((size_t)refCountVal) & refcount_mask) + 1; +} + id objc_retain_fast_np(id obj) { uintptr_t *refCount = ((uintptr_t*)obj) - 1; diff --git a/objc/objc-arc.h b/objc/objc-arc.h index 75dde84..32ecb7a 100644 --- a/objc/objc-arc.h +++ b/objc/objc-arc.h @@ -111,6 +111,10 @@ void objc_release_fast_np(id obj) OBJC_NONPORTABLE; * ARC-compatible classes. */ BOOL objc_release_fast_no_destroy_np(id obj) OBJC_NONPORTABLE; +/** + * Returns the retain count of an object. + */ +size_t object_getRetainCount_np(id obj) OBJC_NONPORTABLE; /** * Releases an object. Equivalent to [obj release]. */ From 6d9f2a6586f2370baf7a20d98e3700c0bd606c37 Mon Sep 17 00:00:00 2001 From: David Chisnall Date: Thu, 14 Dec 2017 09:54:53 +0000 Subject: [PATCH 10/26] Add some release announcement text. --- ANNOUNCE | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/ANNOUNCE b/ANNOUNCE index 572626f..e5aa402 100644 --- a/ANNOUNCE +++ b/ANNOUNCE @@ -24,6 +24,24 @@ Highlights of this release include: Thumb-2 code. This will also generate Thumb-2 message send functions, improving instruction cache usage. +- Significant improvements to ARC, including + + * The runtime no longer acquires a global lock on every object deallocation (a + global lock is still used for objects that have weak references). + + * Weak references use a scheme closer to C++ `std::weak_pointer` and are + lazily zeroed on access. This reduces the space overheads for weak + references. + + * Some additional helper functions are added for use in `NSObject` and other + root classes, which simplifies the layering between the runtime and the + Foundation (or equivalent) implementation. + +- Improvements to how the runtime handles layout of ivars with strong alignment + requirements, which should fix issues relating to using vector types in + Objective-C objects. + + You may obtain the code for this release from git and use the 1.x branch: https://github.com/gnustep/libobjc2.git From 2542638b23c955b8a1b5b618b2f2ba783fa45180 Mon Sep 17 00:00:00 2001 From: David Chisnall Date: Sun, 17 Dec 2017 10:59:00 +0000 Subject: [PATCH 11/26] Add a warning to the announcement. --- ANNOUNCE | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ANNOUNCE b/ANNOUNCE index e5aa402..6dfdef2 100644 --- a/ANNOUNCE +++ b/ANNOUNCE @@ -27,7 +27,9 @@ Highlights of this release include: - Significant improvements to ARC, including * The runtime no longer acquires a global lock on every object deallocation (a - global lock is still used for objects that have weak references). + global lock is still used for objects that have weak references). *NOTE:* + This is incompatible with other code directly inspecting the reference + count and will break with older versions of GNUstep Base! * Weak references use a scheme closer to C++ `std::weak_pointer` and are lazily zeroed on access. This reduces the space overheads for weak From 44d05e58a6cae3e2b985f8fd4a7e850e288a2f02 Mon Sep 17 00:00:00 2001 From: David Lobron Date: Fri, 22 Dec 2017 10:14:17 -0500 Subject: [PATCH 12/26] Minimal reproducing test case. This creates a test number 47 in the unit test suite, which fails with an exception. The exception reproduces the bug. --- Test/CMakeLists.txt | 29 ++++++++++++---------------- Test/MyException.h | 16 +++++++++++++++ Test/MyException.m | 47 +++++++++++++++++++++++++++++++++++++++++++++ Test/minRep.mm | 17 ++++++++++++++++ Test/minRep1.h | 9 +++++++++ Test/minRep1.mm | 17 ++++++++++++++++ Test/minRep2.h | 6 ++++++ Test/minRep2.mm | 24 +++++++++++++++++++++++ Test/minRepM.h | 10 ++++++++++ Test/minRepM.m | 26 +++++++++++++++++++++++++ 10 files changed, 184 insertions(+), 17 deletions(-) create mode 100644 Test/MyException.h create mode 100644 Test/MyException.m create mode 100644 Test/minRep.mm create mode 100644 Test/minRep1.h create mode 100644 Test/minRep1.mm create mode 100644 Test/minRep2.h create mode 100644 Test/minRep2.mm create mode 100644 Test/minRepM.h create mode 100644 Test/minRepM.m diff --git a/Test/CMakeLists.txt b/Test/CMakeLists.txt index 4baf428..f295235 100644 --- a/Test/CMakeLists.txt +++ b/Test/CMakeLists.txt @@ -5,7 +5,7 @@ # List of single-file tests. set(TESTS - alignTest.m + #alignTest.m AllocatePair.m AssociatedObject.m AssociatedObject2.m @@ -19,53 +19,48 @@ set(TESTS NestedExceptions.m PropertyAttributeTest.m PropertyIntrospectionTest.m - PropertyIntrospectionTest2_arc.m + PropertyIntrospectionTest2.m ProtocolCreation.m ResurrectInDealloc_arc.m RuntimeTest.m - WeakBlock_arc.m WeakReferences_arc.m - ivar_arc.m - IVarOverlap.m objc_msgSend.m msgInterpose.m NilException.m MethodArguments.m - zeroSizedIVar.m - exchange.m ) # Function for adding a test. This takes the name of the test and the list of # source files as arguments. -function(addtest_flags TEST_NAME FLAGS TEST_SOURCE) - if (${TEST_NAME} MATCHES ".*_arc") +function(addtest_flags TEST FLAGS TEST_SOURCE) + if (TEST MATCHES ".*_arc") # Only compile the main file with ARC set_source_files_properties(${TEST_SOURCE} COMPILE_FLAGS "-fobjc-arc") # Add the ARC-incompatible definitions of the test class. list(APPEND TEST_SOURCE "Test.m") endif() - add_executable(${TEST_NAME} ${TEST_SOURCE}) - add_test(${TEST_NAME} ${TEST_NAME}) + add_executable(${TEST} ${TEST_SOURCE}) + add_test(${TEST} ${TEST}) set(ARC "") - set_target_properties(${TEST_NAME} PROPERTIES + set_target_properties(${TEST} PROPERTIES INCLUDE_DIRECTORIES "${CMAKE_SOURCE_DIR}" COMPILE_FLAGS "-fobjc-runtime=gnustep-1.7 -fblocks ${FLAGS}" LINKER_LANGUAGE C ) - set_property(TEST ${TEST_NAME} PROPERTY + set_property(TEST ${TEST} PROPERTY ENVIRONMENT "LD_LIBRARY_PATH=" ) - target_link_libraries(${TEST_NAME} objc) + target_link_libraries(${TEST} objc) endfunction(addtest_flags) foreach(TEST_SOURCE ${TESTS}) get_filename_component(TEST ${TEST_SOURCE} NAME_WE) - addtest_flags(${TEST} "-O0 -UNDEBUG" ${TEST_SOURCE}) - addtest_flags("${TEST}_optimised" "-O3 -UNDEBUG" ${TEST_SOURCE}) + addtest_flags(${TEST} "-O0" ${TEST_SOURCE}) + addtest_flags("${TEST}_optimised" "-O3" ${TEST_SOURCE}) endforeach() # Tests that are more than a single file. addtest_flags(CXXExceptions "-O0" "CXXException.m;CXXException.cc") addtest_flags(CXXExceptions_optimised "-O3" "CXXException.m;CXXException.cc") - +addtest_flags(MinRep "-O0" "minRep1.mm;minRep2.mm;minRep.mm;minRepM.m;ModTest.m;MyException.m") diff --git a/Test/MyException.h b/Test/MyException.h new file mode 100644 index 0000000..8ca3d4c --- /dev/null +++ b/Test/MyException.h @@ -0,0 +1,16 @@ +#include "ModTest.h" + +@interface MyException : Test { +@private + char *_name; + char *_reason; +} ++ (void) raise: (char *)name + format: (char *)reason; +- (void) raise; +- (char *)name; +- (char *)reason; +- (MyException *)initWithName:(char *)name + reason:(char *)reason; + +@end diff --git a/Test/MyException.m b/Test/MyException.m new file mode 100644 index 0000000..316f5cf --- /dev/null +++ b/Test/MyException.m @@ -0,0 +1,47 @@ +#import "MyException.h" + +@implementation MyException + +- (MyException *)initWithName:(char *)name + reason:(char *)reason +{ + if ((self = [super init]) != nil) { + _name = name; + _reason = reason; + } + return self; +} + ++ (void) raise: (char *)name + format: (char *)reason +{ + MyException *e = [[[MyException alloc] initWithName:name + reason:reason] autorelease]; + [e raise]; +} + +- (void) dealloc +{ + if (_name) { + free(_name); + } + if (_reason) { + free(_reason); + } +} + +- (void) raise { + @throw self; +} + +- (char*)name +{ + return _name; +} + +- (char*)reason +{ + return _reason; +} + +@end diff --git a/Test/minRep.mm b/Test/minRep.mm new file mode 100644 index 0000000..6d75f60 --- /dev/null +++ b/Test/minRep.mm @@ -0,0 +1,17 @@ +#import "ModTest.h" + +#import "minRepM.h" + +#import "stdio.h" + +int main (int iArgc, const char *iArgv[]) +{ + @autoreleasepool { + MinRepM *mrm = [MinRepM new]; + printf("Poking\n"); + [mrm poke]; + printf("Poked\n"); + } + + return 0; +} diff --git a/Test/minRep1.h b/Test/minRep1.h new file mode 100644 index 0000000..b92e360 --- /dev/null +++ b/Test/minRep1.h @@ -0,0 +1,9 @@ +#import "ModTest.h" + +#import "minRep2.h" + +@interface MinRep1 : Test { +} +- (void)poke; + +@end diff --git a/Test/minRep1.mm b/Test/minRep1.mm new file mode 100644 index 0000000..88a2ad4 --- /dev/null +++ b/Test/minRep1.mm @@ -0,0 +1,17 @@ +#import "ModTest.h" + +#import "minRep1.h" + +#import "stdio.h" + +@implementation MinRep1 + +- (void)poke +{ + MinRep2 *mr2 = [MinRep2 new]; + printf("Poking from minRep1\n"); + [mr2 poke]; + [mr2 release]; +} + +@end diff --git a/Test/minRep2.h b/Test/minRep2.h new file mode 100644 index 0000000..3522281 --- /dev/null +++ b/Test/minRep2.h @@ -0,0 +1,6 @@ +#import "ModTest.h" + +@interface MinRep2 : Test { +} +- (void)poke; +@end diff --git a/Test/minRep2.mm b/Test/minRep2.mm new file mode 100644 index 0000000..592e98e --- /dev/null +++ b/Test/minRep2.mm @@ -0,0 +1,24 @@ +#import "ModTest.h" + +#import "minRep2.h" +#import "MyException.h" + +#import "stdio.h" + +@implementation MinRep2 + +- (void)poke { + @try { + printf("Raising MyException\n"); + MyException *e = [MyException new]; + @throw e; + } @catch (MyException *localException) { + printf("Caught - re-raising\n"); + [localException retain]; + [[localException autorelease] raise]; + } @catch(...) { + printf("Caught in general block\n"); + } +} + +@end diff --git a/Test/minRepM.h b/Test/minRepM.h new file mode 100644 index 0000000..da5631e --- /dev/null +++ b/Test/minRepM.h @@ -0,0 +1,10 @@ +#import "ModTest.h" + +#import "minRep1.h" + +@interface MinRepM : Test { + MinRep1 *_mr1; +} +- (void)poke; + +@end; diff --git a/Test/minRepM.m b/Test/minRepM.m new file mode 100644 index 0000000..eaa011d --- /dev/null +++ b/Test/minRepM.m @@ -0,0 +1,26 @@ +#import "ModTest.h" + +#import "minRepM.h" +#import "minRep1.h" + +#import "MyException.h" + +#import "stdio.h" + +@implementation MinRepM + +- (void)poke { + _mr1 = [MinRep1 new]; + @try { + printf("Poking from minRepM\n"); + if (_mr1) { + [_mr1 poke]; + } + printf("Poked from minRepM\n"); + } @catch (MyException *localException) { + printf("In NS_HANDLER block, %s %s\n", + [localException name], [localException reason]); + } +} + +@end From c036265ecee4eda632bd91e6bbc9b49a33bebb3e Mon Sep 17 00:00:00 2001 From: David Lobron Date: Fri, 22 Dec 2017 10:44:27 -0500 Subject: [PATCH 13/26] Remove changes introduced accidentally because I was using an older version of this file (sync with 0b5f66393a6d81e86028c4d91f7866d7482acd1c). --- Test/CMakeLists.txt | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/Test/CMakeLists.txt b/Test/CMakeLists.txt index f295235..074827f 100644 --- a/Test/CMakeLists.txt +++ b/Test/CMakeLists.txt @@ -5,7 +5,7 @@ # List of single-file tests. set(TESTS - #alignTest.m + alignTest.m AllocatePair.m AssociatedObject.m AssociatedObject2.m @@ -19,45 +19,50 @@ set(TESTS NestedExceptions.m PropertyAttributeTest.m PropertyIntrospectionTest.m - PropertyIntrospectionTest2.m + PropertyIntrospectionTest2_arc.m ProtocolCreation.m ResurrectInDealloc_arc.m RuntimeTest.m + WeakBlock_arc.m WeakReferences_arc.m + ivar_arc.m + IVarOverlap.m objc_msgSend.m msgInterpose.m NilException.m MethodArguments.m + zeroSizedIVar.m + exchange.m ) # Function for adding a test. This takes the name of the test and the list of # source files as arguments. -function(addtest_flags TEST FLAGS TEST_SOURCE) - if (TEST MATCHES ".*_arc") +function(addtest_flags TEST_NAME FLAGS TEST_SOURCE) + if (${TEST_NAME} MATCHES ".*_arc") # Only compile the main file with ARC set_source_files_properties(${TEST_SOURCE} COMPILE_FLAGS "-fobjc-arc") # Add the ARC-incompatible definitions of the test class. list(APPEND TEST_SOURCE "Test.m") endif() - add_executable(${TEST} ${TEST_SOURCE}) - add_test(${TEST} ${TEST}) + add_executable(${TEST_NAME} ${TEST_SOURCE}) + add_test(${TEST_NAME} ${TEST_NAME}) set(ARC "") - set_target_properties(${TEST} PROPERTIES + set_target_properties(${TEST_NAME} PROPERTIES INCLUDE_DIRECTORIES "${CMAKE_SOURCE_DIR}" COMPILE_FLAGS "-fobjc-runtime=gnustep-1.7 -fblocks ${FLAGS}" LINKER_LANGUAGE C ) - set_property(TEST ${TEST} PROPERTY + set_property(TEST ${TEST_NAME} PROPERTY ENVIRONMENT "LD_LIBRARY_PATH=" ) - target_link_libraries(${TEST} objc) + target_link_libraries(${TEST_NAME} objc) endfunction(addtest_flags) foreach(TEST_SOURCE ${TESTS}) get_filename_component(TEST ${TEST_SOURCE} NAME_WE) - addtest_flags(${TEST} "-O0" ${TEST_SOURCE}) - addtest_flags("${TEST}_optimised" "-O3" ${TEST_SOURCE}) + addtest_flags(${TEST} "-O0 -UNDEBUG" ${TEST_SOURCE}) + addtest_flags("${TEST}_optimised" "-O3 -UNDEBUG" ${TEST_SOURCE}) endforeach() # Tests that are more than a single file. From 24f3713a4bc48fbdd987cfab7fec19e9d8ebff10 Mon Sep 17 00:00:00 2001 From: David Chisnall Date: Sun, 24 Dec 2017 10:16:36 +0000 Subject: [PATCH 14/26] Fix a bug in weak ref handling. Weak refs were being left as dangling pointers after being deleted. The load that caused the deallocation would return nil, but then the next one would dereference a dangling pointer. --- arc.m | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/arc.m b/arc.m index 4775beb..e18872b 100644 --- a/arc.m +++ b/arc.m @@ -629,13 +629,15 @@ static BOOL loadWeakPointer(id *addr, id *obj, WeakRef **ref) } __attribute__((always_inline)) -static inline void weakRefRelease(WeakRef *ref) +static inline BOOL weakRefRelease(WeakRef *ref) { ref->weak_count--; if (ref->weak_count == 0) { free(ref); + return YES; } + return NO; } void* block_load_weak(void *block); @@ -787,7 +789,11 @@ id objc_loadWeakRetained(id* addr) // will acquire the lock before attempting to deallocate) if (obj == nil) { - weakRefRelease(ref); + // If we've destroyed this weak ref, then make sure that we also deallocate the object. + if (weakRefRelease(ref)) + { + *addr = nil; + } return nil; } Class cls = classForObject(obj); From 1f2d35006b8f1c7cf7ae87ed8a6068331584d611 Mon Sep 17 00:00:00 2001 From: David Chisnall Date: Sun, 24 Dec 2017 16:19:01 +0000 Subject: [PATCH 15/26] Make the ObjC++ interop test compile --- Test/CMakeLists.txt | 9 ++++++--- Test/MyException.h | 2 +- Test/MyException.m | 10 +++++----- Test/Test.h | 11 ++++++++--- Test/Test.m | 36 +++--------------------------------- Test/minRep.mm | 2 +- Test/minRep1.h | 2 +- Test/minRep1.mm | 2 +- Test/minRep2.h | 2 +- Test/minRep2.mm | 2 +- Test/minRepM.h | 2 +- Test/minRepM.m | 2 +- 12 files changed, 30 insertions(+), 52 deletions(-) diff --git a/Test/CMakeLists.txt b/Test/CMakeLists.txt index 074827f..f6d6eba 100644 --- a/Test/CMakeLists.txt +++ b/Test/CMakeLists.txt @@ -63,9 +63,12 @@ foreach(TEST_SOURCE ${TESTS}) get_filename_component(TEST ${TEST_SOURCE} NAME_WE) addtest_flags(${TEST} "-O0 -UNDEBUG" ${TEST_SOURCE}) addtest_flags("${TEST}_optimised" "-O3 -UNDEBUG" ${TEST_SOURCE}) + target_compile_definitions(${TEST} PRIVATE SINGLE_FILE_TEST=1) + target_compile_definitions("${TEST}_optimised" PRIVATE SINGLE_FILE_TEST=1) endforeach() # Tests that are more than a single file. -addtest_flags(CXXExceptions "-O0" "CXXException.m;CXXException.cc") -addtest_flags(CXXExceptions_optimised "-O3" "CXXException.m;CXXException.cc") -addtest_flags(MinRep "-O0" "minRep1.mm;minRep2.mm;minRep.mm;minRepM.m;ModTest.m;MyException.m") +addtest_flags(CXXExceptions "-O0" "CXXException.m;CXXException.cc;Test.m") +addtest_flags(CXXExceptions_optimised "-O3" "CXXException.m;CXXException.cc;Test.m") +addtest_flags(ObjCXXEHInterop "-O0" "minRep1.mm;minRep2.mm;minRep.mm;minRepM.m;MyException.m;Test.m") +addtest_flags(ObjCXXEHInterop_opttimised "-O3" "minRep1.mm;minRep2.mm;minRep.mm;minRepM.m;MyException.m;Test.m") diff --git a/Test/MyException.h b/Test/MyException.h index 8ca3d4c..fb7596e 100644 --- a/Test/MyException.h +++ b/Test/MyException.h @@ -1,4 +1,4 @@ -#include "ModTest.h" +#include "Test.h" @interface MyException : Test { @private diff --git a/Test/MyException.m b/Test/MyException.m index 316f5cf..2b548fd 100644 --- a/Test/MyException.m +++ b/Test/MyException.m @@ -1,21 +1,20 @@ #import "MyException.h" +#include @implementation MyException - (MyException *)initWithName:(char *)name reason:(char *)reason { - if ((self = [super init]) != nil) { - _name = name; - _reason = reason; - } + _name = name; + _reason = reason; return self; } + (void) raise: (char *)name format: (char *)reason { - MyException *e = [[[MyException alloc] initWithName:name + MyException *e = [[[MyException new] initWithName:name reason:reason] autorelease]; [e raise]; } @@ -28,6 +27,7 @@ if (_reason) { free(_reason); } + [super dealloc]; } - (void) raise { diff --git a/Test/Test.h b/Test/Test.h index 6ca0261..e455aaa 100644 --- a/Test/Test.h +++ b/Test/Test.h @@ -23,6 +23,11 @@ __attribute__((objc_root_class)) #endif @end +@interface NSAutoreleasePool : Test +@end + +#ifdef SINGLE_FILE_TEST + #if !__has_feature(objc_arc) @implementation Test + (Class)class { return self; } @@ -48,10 +53,7 @@ __attribute__((objc_root_class)) } - (void)_ARCCompliantRetainRelease {} @end -#endif -@interface NSAutoreleasePool : Test -@end @implementation NSAutoreleasePool - (void)_ARCCompatibleAutoreleasePool {} + (void)addObject:(id)anObject @@ -59,3 +61,6 @@ __attribute__((objc_root_class)) objc_autorelease(anObject); } @end +#endif + +#endif diff --git a/Test/Test.m b/Test/Test.m index 83a4c3d..87bfcae 100644 --- a/Test/Test.m +++ b/Test/Test.m @@ -4,37 +4,7 @@ #undef NDEBUG #endif #include - -#ifndef __has_attribute -#define __has_attribute(x) 0 +#ifndef SINGLE_FILE_TEST +#define SINGLE_FILE_TEST 1 #endif - -#if __has_attribute(objc_root_class) -__attribute__((objc_root_class)) -#endif -@interface Test { id isa; } -@end -@implementation Test -+ (Class)class { return self; } -+ (id)new -{ - return class_createInstance(self, 0); -} -- (void)dealloc -{ - object_dispose(self); -} -- (id)autorelease -{ - return objc_autorelease(self); -} -- (id)retain -{ - return objc_retain(self); -} -- (void)release -{ - objc_release(self); -} -- (void)_ARCCompliantRetainRelease {} -@end +#include "Test.h" diff --git a/Test/minRep.mm b/Test/minRep.mm index 6d75f60..7daf632 100644 --- a/Test/minRep.mm +++ b/Test/minRep.mm @@ -1,4 +1,4 @@ -#import "ModTest.h" +#import "Test.h" #import "minRepM.h" diff --git a/Test/minRep1.h b/Test/minRep1.h index b92e360..aa07f16 100644 --- a/Test/minRep1.h +++ b/Test/minRep1.h @@ -1,4 +1,4 @@ -#import "ModTest.h" +#import "Test.h" #import "minRep2.h" diff --git a/Test/minRep1.mm b/Test/minRep1.mm index 88a2ad4..8d6e212 100644 --- a/Test/minRep1.mm +++ b/Test/minRep1.mm @@ -1,4 +1,4 @@ -#import "ModTest.h" +#import "Test.h" #import "minRep1.h" diff --git a/Test/minRep2.h b/Test/minRep2.h index 3522281..bff182f 100644 --- a/Test/minRep2.h +++ b/Test/minRep2.h @@ -1,4 +1,4 @@ -#import "ModTest.h" +#import "Test.h" @interface MinRep2 : Test { } diff --git a/Test/minRep2.mm b/Test/minRep2.mm index 592e98e..00fc588 100644 --- a/Test/minRep2.mm +++ b/Test/minRep2.mm @@ -1,4 +1,4 @@ -#import "ModTest.h" +#import "Test.h" #import "minRep2.h" #import "MyException.h" diff --git a/Test/minRepM.h b/Test/minRepM.h index da5631e..16915c6 100644 --- a/Test/minRepM.h +++ b/Test/minRepM.h @@ -1,4 +1,4 @@ -#import "ModTest.h" +#import "Test.h" #import "minRep1.h" diff --git a/Test/minRepM.m b/Test/minRepM.m index eaa011d..7b3b821 100644 --- a/Test/minRepM.m +++ b/Test/minRepM.m @@ -1,4 +1,4 @@ -#import "ModTest.h" +#import "Test.h" #import "minRepM.h" #import "minRep1.h" From 8f03e62a84007af1d176a48cae4b26114e7bb6b4 Mon Sep 17 00:00:00 2001 From: David Chisnall Date: Sun, 24 Dec 2017 16:37:44 +0000 Subject: [PATCH 16/26] Delete a lot of redundant code. --- Test/CMakeLists.txt | 4 ++-- Test/MyException.h | 16 --------------- Test/MyException.m | 47 --------------------------------------------- Test/minRep.mm | 17 ---------------- Test/minRep1.mm | 4 +--- Test/minRep2.h | 13 +++++++++---- Test/minRep2.mm | 16 +++++++-------- Test/minRepM.m | 24 +++++++++++------------ 8 files changed, 30 insertions(+), 111 deletions(-) delete mode 100644 Test/MyException.h delete mode 100644 Test/MyException.m delete mode 100644 Test/minRep.mm diff --git a/Test/CMakeLists.txt b/Test/CMakeLists.txt index f6d6eba..a1bb34e 100644 --- a/Test/CMakeLists.txt +++ b/Test/CMakeLists.txt @@ -70,5 +70,5 @@ endforeach() # Tests that are more than a single file. addtest_flags(CXXExceptions "-O0" "CXXException.m;CXXException.cc;Test.m") addtest_flags(CXXExceptions_optimised "-O3" "CXXException.m;CXXException.cc;Test.m") -addtest_flags(ObjCXXEHInterop "-O0" "minRep1.mm;minRep2.mm;minRep.mm;minRepM.m;MyException.m;Test.m") -addtest_flags(ObjCXXEHInterop_opttimised "-O3" "minRep1.mm;minRep2.mm;minRep.mm;minRepM.m;MyException.m;Test.m") +addtest_flags(ObjCXXEHInterop "-O0" "minRep2.mm;minRepM.m;Test.m") +addtest_flags(ObjCXXEHInterop_opttimised "-O3" "minRep2.mm;minRepM.m;Test.m") diff --git a/Test/MyException.h b/Test/MyException.h deleted file mode 100644 index fb7596e..0000000 --- a/Test/MyException.h +++ /dev/null @@ -1,16 +0,0 @@ -#include "Test.h" - -@interface MyException : Test { -@private - char *_name; - char *_reason; -} -+ (void) raise: (char *)name - format: (char *)reason; -- (void) raise; -- (char *)name; -- (char *)reason; -- (MyException *)initWithName:(char *)name - reason:(char *)reason; - -@end diff --git a/Test/MyException.m b/Test/MyException.m deleted file mode 100644 index 2b548fd..0000000 --- a/Test/MyException.m +++ /dev/null @@ -1,47 +0,0 @@ -#import "MyException.h" -#include - -@implementation MyException - -- (MyException *)initWithName:(char *)name - reason:(char *)reason -{ - _name = name; - _reason = reason; - return self; -} - -+ (void) raise: (char *)name - format: (char *)reason -{ - MyException *e = [[[MyException new] initWithName:name - reason:reason] autorelease]; - [e raise]; -} - -- (void) dealloc -{ - if (_name) { - free(_name); - } - if (_reason) { - free(_reason); - } - [super dealloc]; -} - -- (void) raise { - @throw self; -} - -- (char*)name -{ - return _name; -} - -- (char*)reason -{ - return _reason; -} - -@end diff --git a/Test/minRep.mm b/Test/minRep.mm deleted file mode 100644 index 7daf632..0000000 --- a/Test/minRep.mm +++ /dev/null @@ -1,17 +0,0 @@ -#import "Test.h" - -#import "minRepM.h" - -#import "stdio.h" - -int main (int iArgc, const char *iArgv[]) -{ - @autoreleasepool { - MinRepM *mrm = [MinRepM new]; - printf("Poking\n"); - [mrm poke]; - printf("Poked\n"); - } - - return 0; -} diff --git a/Test/minRep1.mm b/Test/minRep1.mm index 8d6e212..ab87270 100644 --- a/Test/minRep1.mm +++ b/Test/minRep1.mm @@ -8,10 +8,8 @@ - (void)poke { - MinRep2 *mr2 = [MinRep2 new]; printf("Poking from minRep1\n"); - [mr2 poke]; - [mr2 release]; + poke_objcxx(); } @end diff --git a/Test/minRep2.h b/Test/minRep2.h index bff182f..60a88bd 100644 --- a/Test/minRep2.h +++ b/Test/minRep2.h @@ -1,6 +1,11 @@ #import "Test.h" -@interface MinRep2 : Test { -} -- (void)poke; -@end +#ifdef __cplusplus +#define EXTERN_C extern "C" +#else +#define EXTERN_C +#endif + +EXTERN_C void rethrow(id); +EXTERN_C void poke_objcxx(void); + diff --git a/Test/minRep2.mm b/Test/minRep2.mm index 00fc588..3657102 100644 --- a/Test/minRep2.mm +++ b/Test/minRep2.mm @@ -1,24 +1,22 @@ #import "Test.h" #import "minRep2.h" -#import "MyException.h" #import "stdio.h" -@implementation MinRep2 - -- (void)poke { +void poke_objcxx(void) +{ @try { printf("Raising MyException\n"); - MyException *e = [MyException new]; + Test *e = [Test new]; @throw e; - } @catch (MyException *localException) { + } @catch (Test *localException) { printf("Caught - re-raising\n"); [localException retain]; - [[localException autorelease] raise]; + localException = [localException autorelease];; + rethrow(localException); } @catch(...) { - printf("Caught in general block\n"); + printf("Caught in catchall\n"); } } -@end diff --git a/Test/minRepM.m b/Test/minRepM.m index 7b3b821..920f7b0 100644 --- a/Test/minRepM.m +++ b/Test/minRepM.m @@ -1,26 +1,24 @@ #import "Test.h" #import "minRepM.h" -#import "minRep1.h" - -#import "MyException.h" +#import "minRep2.h" #import "stdio.h" -@implementation MinRepM +void rethrow(id x) +{ + @throw x; +} -- (void)poke { - _mr1 = [MinRep1 new]; +int main(void) +{ @try { printf("Poking from minRepM\n"); - if (_mr1) { - [_mr1 poke]; - } + poke_objcxx(); printf("Poked from minRepM\n"); - } @catch (MyException *localException) { - printf("In NS_HANDLER block, %s %s\n", - [localException name], [localException reason]); + } @catch (Test *localException) { + printf("In NS_HANDLER block, %p\n", localException); + } } -@end From 9115a909b147203c4b7a320ded867740da04c36d Mon Sep 17 00:00:00 2001 From: David Chisnall Date: Sun, 24 Dec 2017 16:41:13 +0000 Subject: [PATCH 17/26] Delete more code. The reduced test case is now fairly close to minimal. --- Test/minRep1.h | 9 --------- Test/minRep2.h | 11 ----------- Test/minRep2.mm | 6 +++--- Test/minRepM.h | 10 ---------- Test/minRepM.m | 5 ++--- 5 files changed, 5 insertions(+), 36 deletions(-) delete mode 100644 Test/minRep1.h delete mode 100644 Test/minRep2.h delete mode 100644 Test/minRepM.h diff --git a/Test/minRep1.h b/Test/minRep1.h deleted file mode 100644 index aa07f16..0000000 --- a/Test/minRep1.h +++ /dev/null @@ -1,9 +0,0 @@ -#import "Test.h" - -#import "minRep2.h" - -@interface MinRep1 : Test { -} -- (void)poke; - -@end diff --git a/Test/minRep2.h b/Test/minRep2.h deleted file mode 100644 index 60a88bd..0000000 --- a/Test/minRep2.h +++ /dev/null @@ -1,11 +0,0 @@ -#import "Test.h" - -#ifdef __cplusplus -#define EXTERN_C extern "C" -#else -#define EXTERN_C -#endif - -EXTERN_C void rethrow(id); -EXTERN_C void poke_objcxx(void); - diff --git a/Test/minRep2.mm b/Test/minRep2.mm index 3657102..48fb7c6 100644 --- a/Test/minRep2.mm +++ b/Test/minRep2.mm @@ -1,10 +1,10 @@ #import "Test.h" +#import "stdio.h" -#import "minRep2.h" +extern "C" void rethrow(id); -#import "stdio.h" -void poke_objcxx(void) +extern "C" void poke_objcxx(void) { @try { printf("Raising MyException\n"); diff --git a/Test/minRepM.h b/Test/minRepM.h deleted file mode 100644 index 16915c6..0000000 --- a/Test/minRepM.h +++ /dev/null @@ -1,10 +0,0 @@ -#import "Test.h" - -#import "minRep1.h" - -@interface MinRepM : Test { - MinRep1 *_mr1; -} -- (void)poke; - -@end; diff --git a/Test/minRepM.m b/Test/minRepM.m index 920f7b0..ef02d53 100644 --- a/Test/minRepM.m +++ b/Test/minRepM.m @@ -1,10 +1,9 @@ #import "Test.h" -#import "minRepM.h" -#import "minRep2.h" - #import "stdio.h" +void poke_objcxx(void); + void rethrow(id x) { @throw x; From 760bb785aa9ddc2d62f14ccf742f66da4ded0bfe Mon Sep 17 00:00:00 2001 From: David Chisnall Date: Sun, 24 Dec 2017 16:42:52 +0000 Subject: [PATCH 18/26] Rename files. minRep is not actually an informative name for a test case. --- Test/CMakeLists.txt | 4 ++-- Test/{minRepM.m => ObjCXXEHInterop.m} | 0 Test/{minRep2.mm => ObjCXXEHInterop.mm} | 0 3 files changed, 2 insertions(+), 2 deletions(-) rename Test/{minRepM.m => ObjCXXEHInterop.m} (100%) rename Test/{minRep2.mm => ObjCXXEHInterop.mm} (100%) diff --git a/Test/CMakeLists.txt b/Test/CMakeLists.txt index a1bb34e..79a75cd 100644 --- a/Test/CMakeLists.txt +++ b/Test/CMakeLists.txt @@ -70,5 +70,5 @@ endforeach() # Tests that are more than a single file. addtest_flags(CXXExceptions "-O0" "CXXException.m;CXXException.cc;Test.m") addtest_flags(CXXExceptions_optimised "-O3" "CXXException.m;CXXException.cc;Test.m") -addtest_flags(ObjCXXEHInterop "-O0" "minRep2.mm;minRepM.m;Test.m") -addtest_flags(ObjCXXEHInterop_opttimised "-O3" "minRep2.mm;minRepM.m;Test.m") +addtest_flags(ObjCXXEHInterop "-O0" "ObjCXXEHInterop.mm;ObjCXXEHInterop.m;Test.m") +addtest_flags(ObjCXXEHInterop_optimised "-O3" "ObjCXXEHInterop.mm;ObjCXXEHInterop.m;Test.m") diff --git a/Test/minRepM.m b/Test/ObjCXXEHInterop.m similarity index 100% rename from Test/minRepM.m rename to Test/ObjCXXEHInterop.m diff --git a/Test/minRep2.mm b/Test/ObjCXXEHInterop.mm similarity index 100% rename from Test/minRep2.mm rename to Test/ObjCXXEHInterop.mm From f6709519fa8413d2653afa4c6041736ad48060a0 Mon Sep 17 00:00:00 2001 From: David Chisnall Date: Mon, 25 Dec 2017 08:52:02 +0000 Subject: [PATCH 19/26] Simplify test case some more. It appears that the problem occurs when throwing an Objective-C exception through and Objective-C++ frame. --- Test/ObjCXXEHInterop.m | 1 - Test/ObjCXXEHInterop.mm | 2 -- 2 files changed, 3 deletions(-) diff --git a/Test/ObjCXXEHInterop.m b/Test/ObjCXXEHInterop.m index ef02d53..8d4d275 100644 --- a/Test/ObjCXXEHInterop.m +++ b/Test/ObjCXXEHInterop.m @@ -17,7 +17,6 @@ int main(void) printf("Poked from minRepM\n"); } @catch (Test *localException) { printf("In NS_HANDLER block, %p\n", localException); - } } diff --git a/Test/ObjCXXEHInterop.mm b/Test/ObjCXXEHInterop.mm index 48fb7c6..768e507 100644 --- a/Test/ObjCXXEHInterop.mm +++ b/Test/ObjCXXEHInterop.mm @@ -15,8 +15,6 @@ extern "C" void poke_objcxx(void) [localException retain]; localException = [localException autorelease];; rethrow(localException); - } @catch(...) { - printf("Caught in catchall\n"); } } From 7bd78e5b4660d3c8c885fe45e484f0aea532381c Mon Sep 17 00:00:00 2001 From: David Chisnall Date: Mon, 25 Dec 2017 09:12:40 +0000 Subject: [PATCH 20/26] Fix a bug in ObjC++ EH. Throwing an Objective-C exception through a C++ catch block was broken. This was because the C++ code inserts a cleanup handler to make sure that it invokes `__cxa_end_catch`. Unwinding through this catchup transformed the Objective-C exception into a C++ one. This case should have been handled, except for two bugs: 1. A typo (`#ifdef` instead of `#ifndef`) meant that we were not extracting the Objective-C exception from the C++ object. 2. We were skipping everything except catchalls after the search phase, because we lose some information in the transformation. Fixes #49 --- eh_personality.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/eh_personality.c b/eh_personality.c index 31aecc1..90afbbb 100644 --- a/eh_personality.c +++ b/eh_personality.c @@ -330,7 +330,7 @@ static inline _Unwind_Reason_Code internal_objc_personality(int version, // The object to return void *object = NULL; -#ifdef NO_OBJCXX +#ifndef NO_OBJCXX if (exceptionClass == cxx_exception_class) { int objcxx; @@ -421,7 +421,7 @@ static inline _Unwind_Reason_Code internal_objc_personality(int version, // If this is not a cleanup, ignore it and keep unwinding. //if (check_action_record(context, foreignException, &lsda, //action.action_record, thrown_class, &selector) != handler_cleanup) - if (handler != handler_cleanup) + if ((handler != handler_cleanup) && !objcxxException) { DEBUG_LOG("Ignoring handler! %d\n",handler); return continueUnwinding(exceptionObject, context); From 82b3abf93e3287c362721227b747bcaf4fcb5306 Mon Sep 17 00:00:00 2001 From: David Chisnall Date: Mon, 25 Dec 2017 09:29:25 +0000 Subject: [PATCH 21/26] Fix test warning on GNU platforms. --- Test/objc_msgSend.m | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Test/objc_msgSend.m b/Test/objc_msgSend.m index 612734d..f9b82aa 100644 --- a/Test/objc_msgSend.m +++ b/Test/objc_msgSend.m @@ -1,3 +1,5 @@ +// Needed with glibc to expose vasprintf +#define _GNU_SOURCE #include #include #include From fefb333b01745b935571c0fa9d24ae4fc472bba1 Mon Sep 17 00:00:00 2001 From: David Chisnall Date: Mon, 25 Dec 2017 09:29:37 +0000 Subject: [PATCH 22/26] Remove the separate libobjcxx. If we don't find a separate dynamically linkable C++ runtime, then depend on the C++ standard library implementation. --- CMakeLists.txt | 28 ++++++---------------------- 1 file changed, 6 insertions(+), 22 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index aa3e1dc..1a0e15c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -73,7 +73,7 @@ set(libobjc_HDRS objc/slot.h ) -set(libobjcxx_CXX_SRCS objcxx_eh.cc) +set(libobjc_CXX_SRCS objcxx_eh.cc) # For release builds, we disable spamming the terminal with warnings about # selector type mismatches @@ -187,31 +187,16 @@ if (ENABLE_OBJCXX) test_cxx_runtime CMAKE_FLAGS "-DCXX_RUNTIME=${CXX_RUNTIME}") message(STATUS "Is runtime useable? ${USERUNTIME}") - if (${FORCE_LIBOBJCXX} OR NOT ${USERUNTIME}) - message(STATUS "Forcing build of stand-alone libobjcxx") - add_library(objcxx SHARED ${libobjcxx_CXX_SRCS}) - set_target_properties(objcxx PROPERTIES - LINKER_LANGUAGE C - SOVERSION ${libobjc_VERSION} - ) - target_link_libraries(objcxx ${CXX_RUNTIME}) - set(CXX_RUNTIME "") - list(APPEND INSTALL_TARGETS objcxx) + if (NOT ${USERUNTIME}) + message(STATUS "libobjc will depend on C++ standard library") else () - set(libobjc_CXX_SRCS ${libobjcxx_CXX_SRCS}) # We don't want to link the STL implementation (e.g. libstdc++) if # we have a separate C++ runtime. set(CMAKE_CXX_IMPLICIT_LINK_LIBRARIES "") + target_link_libraries(objc ${CXX_RUNTIME}) endif () else () - message(STATUS "No C++ runtime library found") - add_library(objcxx SHARED ${libobjcxx_CXX_SRCS}) - set_target_properties(objcxx PROPERTIES - LINKER_LANGUAGE C - SOVERSION ${libobjc_VERSION} - ) - set(CXX_RUNTIME "") - list(APPEND INSTALL_TARGETS objcxx) + message(STATUS "libobjc will depend on C++ standard library") endif () endif (ENABLE_OBJCXX) @@ -248,8 +233,7 @@ endif () -# Explicitly link the C++ runtime and libgc if we are compiling with gc support. -target_link_libraries(objc ${CXX_RUNTIME}) +# Explicitly link libgc if we are compiling with gc support. if (LIBGC) target_link_libraries(objc ${LIBGC}) endif () From dc9b8313a4af686254ea93f622ecf74e533f2ea0 Mon Sep 17 00:00:00 2001 From: David Chisnall Date: Tue, 26 Dec 2017 17:21:48 +0000 Subject: [PATCH 23/26] Rework C++ library detection. macOS ships with libc++abi, which doesn't provide the hooks required for interop, so this was causing all of the tests to fail to link in the Travis macOS test. We're now correctly detecting that it won't work on macOS and disabling those tests. --- CMake/CMakeLists.txt | 8 +++-- CMake/typeinfo_test.cc | 3 +- CMakeLists.txt | 70 ++++++++++++++++++++++++++++++------------ Test/CMakeLists.txt | 2 ++ 4 files changed, 60 insertions(+), 23 deletions(-) diff --git a/CMake/CMakeLists.txt b/CMake/CMakeLists.txt index 899bb0e..9cbdd09 100644 --- a/CMake/CMakeLists.txt +++ b/CMake/CMakeLists.txt @@ -2,6 +2,8 @@ cmake_minimum_required(VERSION 2.8) add_executable(test_cxx_runtime typeinfo_test.cc) set(CMAKE_CXX_IMPLICIT_LINK_LIBRARIES "") -target_link_libraries(test_cxx_runtime ${CXX_RUNTIME}) -set_target_properties(test_cxx_runtime PROPERTIES - LINKER_LANGUAGE C) +if (CXX_RUNTIME) + target_link_libraries(test_cxx_runtime ${CXX_RUNTIME}) + set_target_properties(test_cxx_runtime PROPERTIES + LINKER_LANGUAGE C) +endif() diff --git a/CMake/typeinfo_test.cc b/CMake/typeinfo_test.cc index b0b7b77..e11629a 100644 --- a/CMake/typeinfo_test.cc +++ b/CMake/typeinfo_test.cc @@ -57,5 +57,6 @@ bool type_info2::__is_pointer_p() const { return true; } int main() { - return 0; + type_info2 s; + return s.__is_pointer_p(); } diff --git a/CMakeLists.txt b/CMakeLists.txt index 1a0e15c..2d0d463 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -73,7 +73,7 @@ set(libobjc_HDRS objc/slot.h ) -set(libobjc_CXX_SRCS objcxx_eh.cc) +set(libobjcxx_CXX_SRCS objcxx_eh.cc) # For release builds, we disable spamming the terminal with warnings about # selector type mismatches @@ -162,18 +162,43 @@ set_source_files_properties( # C++ Runtime interaction # + +function(test_cxx CXX_RUNTIME_NAME IS_STDLIB) + set(CXX_RUNTIME_NAME "${CMAKE_SHARED_LIBRARY_PREFIX}${CXX_RUNTIME_NAME}${CMAKE_SHARED_LIBRARY_SUFFIX}") + message(STATUS "Testing ${CXX_RUNTIME_NAME} as the C++ runtime library") + find_library(CXX_RUNTIME NAMES libsupc++.so) + if (CXX_RUNTIME) + message(STATUS "Testing ${CXX_RUNTIME} as the C++ runtime library") + try_compile(USERUNTIME + "${CMAKE_BINARY_DIR}/CMake" + "${CMAKE_SOURCE_DIR}/CMake" + test_cxx_runtime + CMAKE_FLAGS "-DCXX_RUNTIME=${CXX_RUNTIME} -DTEST_LINKER_LANGUAGE=C") + if (${USERUNTIME}) + set(CXX_RUNTIME ${CXX_RUNTIME} PARENT_SCOPE) + endif() + endif() +endfunction() + set(ENABLE_OBJCXX true CACHE BOOL "Enable support for Objective-C++") -set(FORCE_LIBOBJCXX false CACHE BOOL - "Force building a separate Objective-C++ runtime library") + +set(CXXRT_IS_STDLIB false) + +add_library(objc SHARED ${libobjc_C_SRCS} ${libobjc_ASM_SRCS} ${libobjc_OBJC_SRCS}) + if (ENABLE_OBJCXX) + message(STATUS "Testing C++ interop") # Try to find libcxxrt.so. We can link to this to provide the C++ ABI # layer, if it exists. - find_library(CXX_RUNTIME NAMES libcxxrt.so) + test_cxx(cxxrt false) # If it doesn't, then look for GNU libsupc++.so instead (either works, # they're ABI compatible). if (NOT CXX_RUNTIME) - find_library(CXX_RUNTIME NAMES libsupc++.so) + test_cxx(supc++ false) + endif (NOT CXX_RUNTIME) + if (NOT CXX_RUNTIME) + test_cxx(c++abi false) endif (NOT CXX_RUNTIME) # If we have a C++ ABI library, then we can produce a single libobjc that @@ -181,26 +206,34 @@ if (ENABLE_OBJCXX) # a separate libobjcxx. if (CXX_RUNTIME) message(STATUS "Using ${CXX_RUNTIME} as the C++ runtime library") - try_compile( USERUNTIME + else() + message(STATUS "Testing C++ standard library") + try_compile(USERUNTIME "${CMAKE_BINARY_DIR}/CMake" "${CMAKE_SOURCE_DIR}/CMake" - test_cxx_runtime - CMAKE_FLAGS "-DCXX_RUNTIME=${CXX_RUNTIME}") - message(STATUS "Is runtime useable? ${USERUNTIME}") - if (NOT ${USERUNTIME}) + test_cxx_runtime) + if (${USERUNTIME}) message(STATUS "libobjc will depend on C++ standard library") - else () - # We don't want to link the STL implementation (e.g. libstdc++) if - # we have a separate C++ runtime. - set(CMAKE_CXX_IMPLICIT_LINK_LIBRARIES "") - target_link_libraries(objc ${CXX_RUNTIME}) - endif () - else () - message(STATUS "libobjc will depend on C++ standard library") + set(CXXRT_IS_STDLIB true) + else() + message(STATUS "No useable C++ runtime found") + set(ENABLE_OBJCXX false) + endif() endif () endif (ENABLE_OBJCXX) +if (ENABLE_OBJCXX) + if (NOT CXXRT_IS_STDLIB) + # We don't want to link the STL implementation (e.g. libstdc++) if + # we have a separate C++ runtime. + set(CMAKE_CXX_IMPLICIT_LINK_LIBRARIES "") + target_link_libraries(objc ${CXX_RUNTIME}) + endif() + set(libobjc_CXX_SRCS ${libobjcxx_CXX_SRCS}) + target_sources(objc ${libobjcxx_CXX_SRCS}) +endif() + # Currently, we actually need pthreads, but we should use the platform's native # threading implementation (we do for everything except thread-local storage) @@ -210,7 +243,6 @@ set(objc_LINK_FLAGS "${objc_LINK_FLAGS} ${CMAKE_THREAD_LIBS_INIT}") -add_library(objc SHARED ${libobjc_C_SRCS} ${libobjc_ASM_SRCS} ${libobjc_OBJC_SRCS} ${libobjc_CXX_SRCS}) set_target_properties(objc PROPERTIES LINKER_LANGUAGE C diff --git a/Test/CMakeLists.txt b/Test/CMakeLists.txt index 79a75cd..7db5183 100644 --- a/Test/CMakeLists.txt +++ b/Test/CMakeLists.txt @@ -70,5 +70,7 @@ endforeach() # Tests that are more than a single file. addtest_flags(CXXExceptions "-O0" "CXXException.m;CXXException.cc;Test.m") addtest_flags(CXXExceptions_optimised "-O3" "CXXException.m;CXXException.cc;Test.m") +if (ENABLE_OBJCXX) addtest_flags(ObjCXXEHInterop "-O0" "ObjCXXEHInterop.mm;ObjCXXEHInterop.m;Test.m") addtest_flags(ObjCXXEHInterop_optimised "-O3" "ObjCXXEHInterop.mm;ObjCXXEHInterop.m;Test.m") +endif() From 7832765112af58f5777c22c9b44832a9df44d0e3 Mon Sep 17 00:00:00 2001 From: David Chisnall Date: Tue, 26 Dec 2017 17:32:50 +0000 Subject: [PATCH 24/26] Improve detection of libsupc++ --- CMake/typeinfo_test.cc | 5 +---- CMakeLists.txt | 2 +- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/CMake/typeinfo_test.cc b/CMake/typeinfo_test.cc index e11629a..74802f9 100644 --- a/CMake/typeinfo_test.cc +++ b/CMake/typeinfo_test.cc @@ -21,7 +21,6 @@ namespace std bool operator==(const type_info &) const; bool operator!=(const type_info &) const; bool before(const type_info &) const; - type_info(); private: type_info(const type_info& rhs); type_info& operator= (const type_info& rhs); @@ -44,14 +43,12 @@ namespace std class type_info2 : public std::type_info { public: + type_info2() : type_info("foo") {} virtual bool __is_pointer_p() const; virtual bool __is_function_p() const { return true; } virtual bool __do_catch(const type_info *thrown_type, void **thrown_object, unsigned outer) const { return true; } - virtual bool __do_upcast( - const __class_type_info *target, - void **thrown_object) const { return true; } }; bool type_info2::__is_pointer_p() const { return true; } diff --git a/CMakeLists.txt b/CMakeLists.txt index 2d0d463..8d4255e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -231,7 +231,7 @@ if (ENABLE_OBJCXX) target_link_libraries(objc ${CXX_RUNTIME}) endif() set(libobjc_CXX_SRCS ${libobjcxx_CXX_SRCS}) - target_sources(objc ${libobjcxx_CXX_SRCS}) + target_sources(objc PRIVATE ${libobjcxx_CXX_SRCS}) endif() From bb78abb662e681a75163e20fc5f813469824e602 Mon Sep 17 00:00:00 2001 From: David Chisnall Date: Tue, 26 Dec 2017 17:38:09 +0000 Subject: [PATCH 25/26] Fix copy and paste type. --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 8d4255e..5391b50 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -166,7 +166,7 @@ set_source_files_properties( function(test_cxx CXX_RUNTIME_NAME IS_STDLIB) set(CXX_RUNTIME_NAME "${CMAKE_SHARED_LIBRARY_PREFIX}${CXX_RUNTIME_NAME}${CMAKE_SHARED_LIBRARY_SUFFIX}") message(STATUS "Testing ${CXX_RUNTIME_NAME} as the C++ runtime library") - find_library(CXX_RUNTIME NAMES libsupc++.so) + find_library(CXX_RUNTIME NAMES ${CXX_RUNTIME_NAME}) if (CXX_RUNTIME) message(STATUS "Testing ${CXX_RUNTIME} as the C++ runtime library") try_compile(USERUNTIME From 587cf3f27bb76e270022c7f7835005d1e73135ee Mon Sep 17 00:00:00 2001 From: David Chisnall Date: Tue, 26 Dec 2017 19:08:05 +0000 Subject: [PATCH 26/26] Another try at making CMake work. --- CMakeLists.txt | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 5391b50..3031462 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -165,17 +165,16 @@ set_source_files_properties( function(test_cxx CXX_RUNTIME_NAME IS_STDLIB) set(CXX_RUNTIME_NAME "${CMAKE_SHARED_LIBRARY_PREFIX}${CXX_RUNTIME_NAME}${CMAKE_SHARED_LIBRARY_SUFFIX}") - message(STATUS "Testing ${CXX_RUNTIME_NAME} as the C++ runtime library") - find_library(CXX_RUNTIME NAMES ${CXX_RUNTIME_NAME}) - if (CXX_RUNTIME) - message(STATUS "Testing ${CXX_RUNTIME} as the C++ runtime library") + find_library(CXX_RUNTIME_LIB NAMES ${CXX_RUNTIME_NAME}) + if (CXX_RUNTIME_LIB) + message(STATUS "Testing ${CXX_RUNTIME_LIB} as the C++ runtime library") try_compile(USERUNTIME "${CMAKE_BINARY_DIR}/CMake" "${CMAKE_SOURCE_DIR}/CMake" test_cxx_runtime - CMAKE_FLAGS "-DCXX_RUNTIME=${CXX_RUNTIME} -DTEST_LINKER_LANGUAGE=C") - if (${USERUNTIME}) - set(CXX_RUNTIME ${CXX_RUNTIME} PARENT_SCOPE) + CMAKE_FLAGS "-DCXX_RUNTIME=${CXX_RUNTIME_LIB}") + if (USERUNTIME) + set(CXX_RUNTIME ${CXX_RUNTIME_LIB} PARENT_SCOPE) endif() endif() endfunction()