Improve locking for properties.

This fixes the issue where two addresses that hash to the same address
could deadlock and also fixes a performance issue due to using spinlocks
with no OS-mediated sleep.

Initially this makes the spinlocks.h consumers compile as C++ so that
they can use C++ abstractions over futexes and similar.  A lot of this
code called calloc, so now has some nicer C++ wrappers for that.

This also includes some fixes to work around older C++ standard
libraries.

Also includes some small tweaks to Cirrus, though Cirrus is going away
soon so we'll need an alternative for FreeBSD CI.

Author: davidchisnall

.cirrus.yml

@@ -1,11 +1,9 @@
 libcxxrt_freebsd_task:
   matrix:
   - freebsd_instance:
-     image_family: freebsd-13-5
+     image_family: freebsd-15-0-amd64-zfs
   - freebsd_instance:
-     image_family: freebsd-15-0-snap
-  - freebsd_instance:
-     image_family: freebsd-14-2
+     image_family: freebsd-14-3
 
   install_script: pkg install -y cmake ninja git
 
@@ -31,7 +29,7 @@ libcxxrt_freebsd_task:
 
 libcxxrt_master_task:
   freebsd_instance:
-    image_family: freebsd-14-2
+    image_family: freebsd-14-3
   install_script: pkg install -y cmake ninja git
 
   clone_script: |

CMakeLists.txt

@@ -55,13 +55,13 @@ add_compile_definitions(GNUSTEP __OBJC_RUNTIME_INTERNAL__=1 __OBJC_BOOL)
 set(libobjc_ASM_SRCS 
 	objc_msgSend.S)
 set(libobjc_OBJCXX_SRCS 
+	properties.mm
 	arc.mm
+	associate.mm
 	)
 set(libobjc_OBJC_SRCS 
 	NSBlocks.m
-	associate.m
-	blocks_runtime_np.m
-	properties.m)
+	blocks_runtime_np.m)
 set(libobjc_C_SRCS 
 	alias_table.c
 	builtin_classes.c
@@ -257,7 +257,7 @@ endif ()
 
 add_library(objc SHARED ${libobjc_C_SRCS} ${libobjc_ASM_SRCS} ${libobjc_OBJC_SRCS} ${libobjc_OBJCXX_SRCS} ${libobjc_ASM_OBJS})
 target_compile_options(objc PRIVATE "$<$<OR:$<COMPILE_LANGUAGE:OBJC>,$<COMPILE_LANGUAGE:OBJCXX>>:-Wno-gnu-folding-constant;-Wno-deprecated-objc-isa-usage;-Wno-objc-root-class;-fobjc-runtime=gnustep-2.0>$<$<COMPILE_LANGUAGE:C>:-Xclang;-fexceptions;-Wno-gnu-folding-constant>")
-target_compile_features(objc PRIVATE cxx_std_17)
+target_compile_features(objc PRIVATE cxx_std_20)
 
 list(APPEND libobjc_CXX_SRCS ${libobjcxx_CXX_SRCS})
 target_sources(objc PRIVATE ${libobjc_CXX_SRCS})

associate.m associate.mmassociate.m renamed to associate.mm

@@ -11,6 +11,7 @@
 #include "selector.h"
 #include "lock.h"
 #include "gc_ops.h"
+#include "helpers.hh"
 
 /**
  * A single associative reference.  Contains the key, value, and association
@@ -105,7 +106,7 @@ static void cleanupReferenceList(struct reference_list *list)
 				// Full barrier - ensure that we've zero'd the key before doing
 				// this!
 				__sync_synchronize();
-				objc_release(r->object);
+				objc_release((id)r->object);
 			}
 			r->object = 0;
 			r->policy = 0;
@@ -117,7 +118,7 @@ static void freeReferenceList(struct reference_list *l)
 {
 	if (NULL == l) { return; }
 	freeReferenceList(l->next);
-	gc->free(l);
+	free(l);
 }
 
 static void setReference(struct reference_list *list,
@@ -135,44 +136,45 @@ static void setReference(struct reference_list *list,
 			break;
 		case OBJC_ASSOCIATION_RETAIN_NONATOMIC:
 		case OBJC_ASSOCIATION_RETAIN:
-			obj = objc_retain(obj);
+			obj = objc_retain((id)obj);
 		case OBJC_ASSOCIATION_ASSIGN:
 			break;
 	}
 	// While inserting into the list, we need to lock it temporarily.
-	volatile int *lock = lock_for_pointer(list);
-	lock_spinlock(lock);
 	struct reference *r = findReference(list, key);
-	// If there's an existing reference, then we can update it, otherwise we
-	// have to install a new one
-	if (NULL == r)
 	{
-		// Search for an unused slot
-		r = findReference(list, 0);
+		auto lock = acquire_locks_for_pointers(list);
+		// If there's an existing reference, then we can update it, otherwise we
+		// have to install a new one
 		if (NULL == r)
 		{
-			struct reference_list *l = list;
+			// Search for an unused slot
+			r = findReference(list, 0);
+			if (NULL == r)
+			{
+				struct reference_list *l = list;
 
-			while (NULL != l->next) { l = l->next; }
+				while (NULL != l->next) { l = l->next; }
 
-			l->next = gc->malloc(sizeof(struct reference_list));
-			r = &l->next->list[0];
+				l->next = allocate_zeroed<struct reference_list>();
+				r = &l->next->list[0];
+			}
+			r->key = key;
 		}
-		r->key = key;
 	}
-	unlock_spinlock(lock);
 	// Now we only need to lock if the old or new property is atomic
 	BOOL needLock = isAtomic(r->policy) || isAtomic(policy);
+	ThinLock *lock;
 	if (needLock)
 	{
 		lock = lock_for_pointer(r);
-		lock_spinlock(lock);
+		lock->lock();
 	}
 	@try
 	{
 		if (OBJC_ASSOCIATION_ASSIGN != r->policy)
 		{
-			objc_release(r->object);
+			objc_release((id)r->object);
 		}
 	}
 	@finally
@@ -182,7 +184,7 @@ static void setReference(struct reference_list *list,
 	}
 	if (needLock)
 	{
-		unlock_spinlock(lock);
+		lock->unlock();
 	}
 }
 
@@ -201,8 +203,8 @@ static inline Class findHiddenClass(id obj)
 
 static Class allocateHiddenClass(Class superclass)
 {
-	Class newClass =
-		calloc(1, sizeof(struct objc_class) + sizeof(struct reference_list));
+	struct objc_class *newClass =
+		allocate_zeroed<struct objc_class>(sizeof(struct reference_list));
 
 	if (Nil == newClass) { return Nil; }
 
@@ -221,9 +223,9 @@ static Class allocateHiddenClass(Class superclass)
 
 	LOCK_RUNTIME_FOR_SCOPE();
 	newClass->sibling_class = superclass->subclass_list;
-	superclass->subclass_list = newClass;
+	superclass->subclass_list = (Class)newClass;
 
-	return newClass;
+	return (Class)newClass;
 }
 
 static inline Class initHiddenClassForObject(id obj)
@@ -248,7 +250,7 @@ static void deallocHiddenClass(id obj, SEL _cmd)
 	Class hiddenClass = findHiddenClass(obj);
 	// After calling [super dealloc], the object will no longer exist.
 	// Free the hidden class.
-	struct reference_list *list = object_getIndexedIvars(hiddenClass);
+	struct reference_list *list = static_cast<struct reference_list *>(object_getIndexedIvars(hiddenClass));
 	DESTROY_LOCK(&list->lock);
 	cleanupReferenceList(list);
 	freeReferenceList(list->next);
@@ -289,38 +291,33 @@ static void deallocHiddenClass(id obj, SEL _cmd)
 		Class cls = (Class)object;
 		if ((NULL == cls->extra_data) && create)
 		{
-			volatile int *lock = lock_for_pointer(cls);
-			struct reference_list *list = gc->malloc(sizeof(struct reference_list));
-			lock_spinlock(lock);
+			struct reference_list *list = allocate_zeroed<struct reference_list>();
+			auto guard = acquire_locks_for_pointers(cls);
 			if (NULL == cls->extra_data)
 			{
 				INIT_LOCK(list->lock);
 				cls->extra_data = list;
-				unlock_spinlock(lock);
 			}
 			else
 			{
-				unlock_spinlock(lock);
-				gc->free(list);
+				free(list);
 			}
 		}
 		return cls->extra_data;
 	}
 	Class hiddenClass = findHiddenClass(object);
 	if ((NULL == hiddenClass) && create)
 	{
-		volatile int *lock = lock_for_pointer(object);
-		lock_spinlock(lock);
+		auto guard = acquire_locks_for_pointers(object);
 		hiddenClass = findHiddenClass(object);
 		if (NULL == hiddenClass)
 		{
 			hiddenClass = initHiddenClassForObject(object);
-			struct reference_list *list = object_getIndexedIvars(hiddenClass);
+			struct reference_list *list = static_cast<struct reference_list *>(object_getIndexedIvars(hiddenClass));
 			INIT_LOCK(list->lock);
 		}
-		unlock_spinlock(lock);
 	}
-	return hiddenClass ? object_getIndexedIvars(hiddenClass) : NULL;
+	return hiddenClass ? static_cast<struct reference_list*>(object_getIndexedIvars(hiddenClass)) : nullptr;
 }
 
 void objc_setAssociatedObject(id object,
@@ -345,9 +342,9 @@ id objc_getAssociatedObject(id object, const void *key)
 		// Apple's objc4 retains and autoreleases the object under these policies
 		if (r->policy & OBJC_ASSOCIATION_RETAIN_NONATOMIC)
 		{
-			objc_retainAutorelease(r->object);
+			objc_retainAutorelease((id)r->object);
 		}
-		return r->object;
+		return (id)r->object;
 	}
 	if (class_isMetaClass(object->isa))
 	{
@@ -363,7 +360,7 @@ id objc_getAssociatedObject(id object, const void *key)
 		}
 		if (Nil != cls)
 		{
-			struct reference_list *next_list = object_getIndexedIvars(cls);
+			struct reference_list *next_list = static_cast<struct reference_list *>(object_getIndexedIvars(cls));
 			if (list != next_list)
 			{
 				list = next_list;
@@ -372,9 +369,9 @@ id objc_getAssociatedObject(id object, const void *key)
 				{
 					if (r->policy & OBJC_ASSOCIATION_RETAIN_NONATOMIC)
 					{
-						objc_retainAutorelease(r->object);
+						objc_retainAutorelease((id)r->object);
 					}
-					return r->object;
+					return (id)r->object;
 				}
 			}
 			cls = class_getSuperclass(cls);
@@ -433,16 +430,14 @@ static Class hiddenClassForObject(id object)
 	Class hiddenClass = findHiddenClass(object);
 	if (NULL == hiddenClass)
 	{
-		volatile int *lock = lock_for_pointer(object);
-		lock_spinlock(lock);
+		auto guard = acquire_locks_for_pointers(object);
 		hiddenClass = findHiddenClass(object);
 		if (NULL == hiddenClass)
 		{
 			hiddenClass = initHiddenClassForObject(object);
-			struct reference_list *list = object_getIndexedIvars(hiddenClass);
+			struct reference_list *list = static_cast<struct reference_list*>(object_getIndexedIvars(hiddenClass));
 			INIT_LOCK(list->lock);
 		}
-		unlock_spinlock(lock);
 	}
 	return hiddenClass;
 }
@@ -464,13 +459,13 @@ id object_clone_np(id object)
 	// Make sure that the prototype has a hidden class, so that methods added
 	// to it will appear in the clone.
 	referenceListForObject(object, YES);
-	id new = class_createInstance(object->isa, 0);
-	Class hiddenClass = initHiddenClassForObject(new);
-	struct reference_list *list = object_getIndexedIvars(hiddenClass);
+	id newInstance = class_createInstance(object->isa, 0);
+	Class hiddenClass = initHiddenClassForObject(newInstance);
+	struct reference_list *list = static_cast<struct reference_list*>(object_getIndexedIvars(hiddenClass));
 	INIT_LOCK(list->lock);
-	objc_setAssociatedObject(new, &prototypeKey, object,
+	objc_setAssociatedObject(newInstance, &prototypeKey, object,
 			OBJC_ASSOCIATION_RETAIN_NONATOMIC);
-	return new;
+	return newInstance;
 }
 
 id object_getPrototype_np(id object)

class.h

@@ -2,6 +2,7 @@
 #define __OBJC_CLASS_H_INCLUDED
 #include "visibility.h"
 #include "objc/runtime.h"
+#include "sarray2.h"
 #include <stdint.h>
 
 #ifdef __cplusplus
@@ -96,7 +97,7 @@ struct objc_class
 	 * The dispatch table for this class.  Intialized and maintained by the
 	 * runtime.
 	 */
-	void                      *dtable;
+	SparseArray               *dtable;
 	/**
 	 * A pointer to the first subclass for this class.  Filled in by the
 	 * runtime.

dtable.c

@@ -696,8 +696,8 @@ static void remove_dtable(InitializingDtable* meta_buffer)
 	LOCK(&initialize_lock);
 	InitializingDtable *buffer = meta_buffer->next;
 	// Install the dtable:
-	meta_buffer->class->dtable = meta_buffer->dtable;
-	buffer->class->dtable = buffer->dtable;
+	meta_buffer->owner->dtable = meta_buffer->dtable;
+	buffer->owner->dtable = buffer->dtable;
 	// Remove the look-aside buffer entry.
 	if (temporary_dtables == meta_buffer)
 	{
@@ -706,7 +706,7 @@ static void remove_dtable(InitializingDtable* meta_buffer)
 	else
 	{
 		InitializingDtable *prev = temporary_dtables;
-		while (prev->next->class != meta_buffer->class)
+		while (prev->next->owner != meta_buffer->owner)
 		{
 			prev = prev->next;
 		}

dtable.h

@@ -6,12 +6,14 @@
 #include <stdint.h>
 #include <stdio.h>
 
-#ifdef __OBJC_LOW_MEMORY__
-typedef struct objc_dtable* dtable_t;
-struct objc_slot* objc_dtable_lookup(dtable_t dtable, uint32_t uid);
-#else
 typedef SparseArray* dtable_t;
-#	define objc_dtable_lookup SparseArrayLookup
+#define objc_dtable_lookup SparseArrayLookup
+
+typedef struct objc_class *Class;
+
+#ifdef __cplusplus
+extern "C"
+{
 #endif
 
 /**
@@ -27,7 +29,7 @@ PRIVATE extern dtable_t uninstalled_dtable;
 typedef struct _InitializingDtable
 {
 	/** The class that owns the dtable. */
-	Class class;
+	struct objc_class *owner;
 	/** The dtable for this class. */
 	dtable_t dtable;
 	/** The next uninstalled dtable in the list. */
@@ -50,12 +52,13 @@ OBJC_PUBLIC
 int objc_sync_enter(id object);
 OBJC_PUBLIC
 int objc_sync_exit(id object);
+
 /**
  * Returns the dtable for a given class.  If we are currently in an +initialize
  * method then this will block if called from a thread other than the one
  * running the +initialize method.  
  */
-static inline dtable_t dtable_for_class(Class cls)
+static inline dtable_t dtable_for_class(struct objc_class *cls)
 {
 	if (classHasInstalledDtable(cls))
 	{
@@ -77,7 +80,7 @@ static inline dtable_t dtable_for_class(Class cls)
 		InitializingDtable *buffer = temporary_dtables;
 		while (NULL != buffer)
 		{
-			if (buffer->class == cls)
+			if (buffer->owner == cls)
 			{
 				dtable = buffer->dtable;
 				break;
@@ -139,3 +142,7 @@ void free_dtable(dtable_t dtable);
  * is installed.
  */
 void checkARCAccessorsSlow(Class cls);
+
+#ifdef __cplusplus
+}
+#endif