From 336d8a828c36a5b8ae6f31a54ae660fc5774e8cd Mon Sep 17 00:00:00 2001
From: David Chisnall <github@theravensnest.org>
Date: Wed, 31 Jul 2019 10:38:43 +0100
Subject: [PATCH] Fix test failure.

This bug is also present in the original version:

When removing a WeakRef from the map, we use its obj field to find the
key, but the obj field has already been zeroed by this point and so we
end up leaving dangling pointers in the map.
---
 arc.mm | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/arc.mm b/arc.mm
index 4070017..4873fa8 100644
--- a/arc.mm
+++ b/arc.mm
@@ -629,9 +629,21 @@ namespace {
 struct WeakRef
 {
 	void *isa = &weakref_class;
-	id obj = nullptr;
-	size_t weak_count = 1;
-	WeakRef(id o) : obj(o) {}
+	id object;
+	struct
+	{
+		size_t weak_count:((sizeof(size_t) * 8) - 1);
+		bool isDeleted:1;
+	};
+	id obj()
+	{
+		if (isDeleted)
+		{
+			return nil;
+		}
+		return object;
+	}
+	WeakRef(id o) : object(o), weak_count(1), isDeleted(false) {}
 };
 
 template<typename T>
@@ -709,7 +721,7 @@ static BOOL loadWeakPointer(id *addr, id *obj, WeakRef **ref)
 	if (classForObject(oldObj) == (Class)&weakref_class)
 	{
 		*ref = (WeakRef*)oldObj;
-		*obj = (*ref)->obj;
+		*obj = (*ref)->obj();
 		return YES;
 	}
 	*ref = NULL;
@@ -723,7 +735,7 @@ static inline BOOL weakRefRelease(WeakRef *ref)
 	ref->weak_count--;
 	if (ref->weak_count == 0)
 	{
-		weakRefs().erase(ref->obj);
+		weakRefs().erase(ref->object);
 		delete ref;
 		return YES;
 	}
@@ -786,7 +798,7 @@ WeakRef *incrementWeakRefCount(id obj)
 	}
 	else
 	{
-		assert(ref->obj == obj);
+		assert(ref->obj() == obj);
 		ref->weak_count++;
 	}
 	return ref;
@@ -862,7 +874,7 @@ extern "C" OBJC_PUBLIC BOOL objc_delete_weak_refs(id obj)
 		// accesses from loading from this.  This must be done after
 		// removing the ref from the table, because the compare operation
 		// tests the obj field.
-		oldRef->obj = nil;
+		oldRef->isDeleted = true;
 		// If the weak reference count is zero, then we should have
 		// already removed this.
 		assert(oldRef->weak_count > 0);