Differenciate WeakMaps from bare HashTables used as weak maps for GC purposes

Since cbf67e4, the GC needs to find all WeakMaps referencing a weakly
referenced object. Doing so, it treats all ZEND_WEAKREF_TAG_MAP as WeakMap
instances.

However, a ZEND_WEAKREF_TAG_MAP reference may be a bare HashTable when
zend_weakrefs_hash_add() is used.

Introduce a new tag, ZEND_WEAKREF_TAG_BARE_HT, and use this tag when weakly
referencing an object from a bare HashTable. Ignore such references in GC.

Fixes GH-19543
Closes GH-19544

Co-authored-by: Tim Düsterhus <[email protected]>

Author: arnaud-lb

Zend/tests/gh19543-001.phpt

@@ -0,0 +1,17 @@
+--TEST--
+GH-19543 001: GC treats ZEND_WEAKREF_TAG_MAP references as WeakMap references
+--EXTENSIONS--
+zend_test
+--FILE--
+<?php
+
+$e = new Exception();
+$a = new stdClass();
+zend_weakmap_attach($e, $a);
+unset($a);
+gc_collect_cycles();
+
+?>
+==DONE==
+--EXPECT--
+==DONE==

Zend/tests/gh19543-002.phpt

@@ -0,0 +1,19 @@
+--TEST--
+GH-19543 002: GC treats ZEND_WEAKREF_TAG_MAP references as WeakMap references
+--EXTENSIONS--
+zend_test
+--FILE--
+<?php
+
+$e = new Exception();
+$a = new stdClass();
+zend_weakmap_attach($e, $a);
+unset($a);
+$e2 = $e;
+unset($e2); // add to roots
+gc_collect_cycles();
+
+?>
+==DONE==
+--EXPECT--
+==DONE==

Zend/zend_weakrefs.c

@@ -36,19 +36,21 @@ typedef struct _zend_weakmap_iterator {
 	uint32_t ht_iter;
 } zend_weakmap_iterator;
 
-/* EG(weakrefs) is a map from a key corresponding to a zend_object pointer to all the WeakReference and/or WeakMap entries relating to that pointer.
+/* EG(weakrefs) is a map from a key corresponding to a zend_object pointer to all the WeakReference, WeakMap, and/or bare HashTable entries relating to that pointer.
  *
  * 1. For a single WeakReference,
  *    the HashTable's corresponding value's tag is a ZEND_WEAKREF_TAG_REF and the pointer is a singleton WeakReference instance (zend_weakref *) for that zend_object pointer (from WeakReference::create()).
  * 2. For a single WeakMap, the HashTable's corresponding value's tag is a ZEND_WEAKREF_TAG_MAP and the pointer is a WeakMap instance (zend_weakmap *).
- * 3. For multiple values associated with the same zend_object pointer, the HashTable entry's tag is a ZEND_WEAKREF_TAG_HT with a HashTable mapping
- *    tagged pointers of at most 1 WeakReference and 1 or more WeakMaps to the same tagged pointer.
+ * 3. For a single bare HashTable, the HashTable's corresponding value's tag is a ZEND_WEAKREF_TAG_BARE_HT and the pointer is a HashTable*.
+ * 4. For multiple values associated with the same zend_object pointer, the HashTable entry's tag is a ZEND_WEAKREF_TAG_HT with a HashTable mapping
+ *    tagged pointers of at most 1 WeakReference and 1 or more WeakMap or bare HashTable to the same tagged pointer.
  *
  * ZEND_MM_ALIGNED_OFFSET_LOG2 is at least 2 on supported architectures (pointers to the objects in question are aligned to 4 bytes (1<<2) even on 32-bit systems),
  * i.e. the least two significant bits of the pointer can be used as a tag (ZEND_WEAKREF_TAG_*). */
-#define ZEND_WEAKREF_TAG_REF 0
-#define ZEND_WEAKREF_TAG_MAP 1
-#define ZEND_WEAKREF_TAG_HT  2
+#define ZEND_WEAKREF_TAG_REF     0
+#define ZEND_WEAKREF_TAG_MAP     1
+#define ZEND_WEAKREF_TAG_HT      2
+#define ZEND_WEAKREF_TAG_BARE_HT 3
 #define ZEND_WEAKREF_GET_TAG(p) (((uintptr_t) (p)) & 3)
 #define ZEND_WEAKREF_GET_PTR(p) ((void *) (((uintptr_t) (p)) & ~3))
 #define ZEND_WEAKREF_ENCODE(p, t) ((void *) (((uintptr_t) (p)) | (t)))
@@ -72,8 +74,8 @@ static inline void zend_weakref_unref_single(
 		zend_weakref *wr = ptr;
 		wr->referent = NULL;
 	} else {
-		/* unreferencing WeakMap entry (at ptr) with a key of object. */
-		ZEND_ASSERT(tag == ZEND_WEAKREF_TAG_MAP);
+		/* unreferencing WeakMap or bare HashTable entry (at ptr) with a key of object. */
+		ZEND_ASSERT(tag == ZEND_WEAKREF_TAG_MAP || tag == ZEND_WEAKREF_TAG_BARE_HT);
 		zend_hash_index_del((HashTable *) ptr, zend_object_to_weakref_key(object));
 	}
 }
@@ -166,18 +168,20 @@ static void zend_weakref_unregister(zend_object *object, void *payload, bool wea
 	}
 }
 
+/* Insert 'pData' into bare HashTable 'ht', with the given 'key'. 'key' is
+ * weakly referenced. 'ht' is considered to be a bare HashTable, not a WeakMap. */
 ZEND_API zval *zend_weakrefs_hash_add(HashTable *ht, zend_object *key, zval *pData) {
 	zval *zv = zend_hash_index_add(ht, zend_object_to_weakref_key(key), pData);
 	if (zv) {
-		zend_weakref_register(key, ZEND_WEAKREF_ENCODE(ht, ZEND_WEAKREF_TAG_MAP));
+		zend_weakref_register(key, ZEND_WEAKREF_ENCODE(ht, ZEND_WEAKREF_TAG_BARE_HT));
 	}
 	return zv;
 }
 
 ZEND_API zend_result zend_weakrefs_hash_del(HashTable *ht, zend_object *key) {
 	zval *zv = zend_hash_index_find(ht, zend_object_to_weakref_key(key));
 	if (zv) {
-		zend_weakref_unregister(key, ZEND_WEAKREF_ENCODE(ht, ZEND_WEAKREF_TAG_MAP), 1);
+		zend_weakref_unregister(key, ZEND_WEAKREF_ENCODE(ht, ZEND_WEAKREF_TAG_BARE_HT), 1);
 		return SUCCESS;
 	}
 	return FAILURE;
@@ -520,6 +524,10 @@ HashTable *zend_weakmap_get_object_key_entry_gc(zend_object *object, zval **tabl
 				ZEND_ASSERT(zv);
 				zend_get_gc_buffer_add_ptr(gc_buffer, zv);
 				zend_get_gc_buffer_add_obj(gc_buffer, &wm->std);
+			} else if (ZEND_WEAKREF_GET_TAG(tagged_ptr) == ZEND_WEAKREF_TAG_BARE_HT) {
+				/* Bare HashTables are intentionally ignored, since they are
+				 * intended for internal usage by extensions and might not be
+				 * collectable. */
 			}
 		} ZEND_HASH_FOREACH_END();
 	} else if (tag == ZEND_WEAKREF_TAG_MAP) {
@@ -528,6 +536,8 @@ HashTable *zend_weakmap_get_object_key_entry_gc(zend_object *object, zval **tabl
 		ZEND_ASSERT(zv);
 		zend_get_gc_buffer_add_ptr(gc_buffer, zv);
 		zend_get_gc_buffer_add_obj(gc_buffer, &wm->std);
+	} else if (tag == ZEND_WEAKREF_TAG_BARE_HT) {
+		/* Bare HashTables are intentionally ignored (see above) */
 	}
 
 	zend_get_gc_buffer_use(gc_buffer, table, n);
@@ -554,13 +564,19 @@ HashTable *zend_weakmap_get_object_entry_gc(zend_object *object, zval **table, i
 				zval *zv = zend_hash_index_find(&wm->ht, obj_key);
 				ZEND_ASSERT(zv);
 				zend_get_gc_buffer_add_ptr(gc_buffer, zv);
+			} else if (ZEND_WEAKREF_GET_TAG(tagged_ptr) == ZEND_WEAKREF_TAG_BARE_HT) {
+				/* Bare HashTables are intentionally ignored
+				 * (see zend_weakmap_get_object_key_entry_gc) */
 			}
 		} ZEND_HASH_FOREACH_END();
 	} else if (tag == ZEND_WEAKREF_TAG_MAP) {
 		zend_weakmap *wm = (zend_weakmap*) ptr;
 		zval *zv = zend_hash_index_find(&wm->ht, obj_key);
 		ZEND_ASSERT(zv);
 		zend_get_gc_buffer_add_ptr(gc_buffer, zv);
+	} else if (tag == ZEND_WEAKREF_TAG_BARE_HT) {
+		/* Bare HashTables are intentionally ignored
+		 * (see zend_weakmap_get_object_key_entry_gc) */
 	}
 
 	zend_get_gc_buffer_use(gc_buffer, table, n);

ext/zend_test/php_test.h

@@ -50,7 +50,7 @@ ZEND_BEGIN_MODULE_GLOBALS(zend_test)
 	int observer_fiber_switch;
 	int observer_fiber_destroy;
 	int observer_execute_internal;
-	HashTable global_weakmap;
+	HashTable *global_weakmap;
 	int replace_zend_execute_ex;
 	int register_passes;
 	bool print_stderr_mshutdown;

ext/zend_test/test.c

@@ -340,7 +340,7 @@ static ZEND_FUNCTION(zend_weakmap_attach)
 			Z_PARAM_ZVAL(value)
 	ZEND_PARSE_PARAMETERS_END();
 
-	if (zend_weakrefs_hash_add(&ZT_G(global_weakmap), obj, value)) {
+	if (zend_weakrefs_hash_add(ZT_G(global_weakmap), obj, value)) {
 		Z_TRY_ADDREF_P(value);
 		RETURN_TRUE;
 	}
@@ -355,13 +355,13 @@ static ZEND_FUNCTION(zend_weakmap_remove)
 			Z_PARAM_OBJ(obj)
 	ZEND_PARSE_PARAMETERS_END();
 
-	RETURN_BOOL(zend_weakrefs_hash_del(&ZT_G(global_weakmap), obj) == SUCCESS);
+	RETURN_BOOL(zend_weakrefs_hash_del(ZT_G(global_weakmap), obj) == SUCCESS);
 }
 
 static ZEND_FUNCTION(zend_weakmap_dump)
 {
 	ZEND_PARSE_PARAMETERS_NONE();
-	RETURN_ARR(zend_array_dup(&ZT_G(global_weakmap)));
+	RETURN_ARR(zend_array_dup(ZT_G(global_weakmap)));
 }
 
 static ZEND_FUNCTION(zend_get_current_func_name)
@@ -1311,18 +1311,20 @@ PHP_MSHUTDOWN_FUNCTION(zend_test)
 
 PHP_RINIT_FUNCTION(zend_test)
 {
-	zend_hash_init(&ZT_G(global_weakmap), 8, NULL, ZVAL_PTR_DTOR, 0);
+	ALLOC_HASHTABLE(ZT_G(global_weakmap));
+	zend_hash_init(ZT_G(global_weakmap), 8, NULL, ZVAL_PTR_DTOR, 0);
 	ZT_G(observer_nesting_depth) = 0;
 	return SUCCESS;
 }
 
 PHP_RSHUTDOWN_FUNCTION(zend_test)
 {
 	zend_ulong obj_key;
-	ZEND_HASH_FOREACH_NUM_KEY(&ZT_G(global_weakmap), obj_key) {
-		zend_weakrefs_hash_del(&ZT_G(global_weakmap), zend_weakref_key_to_object(obj_key));
+	ZEND_HASH_FOREACH_NUM_KEY(ZT_G(global_weakmap), obj_key) {
+		zend_weakrefs_hash_del(ZT_G(global_weakmap), zend_weakref_key_to_object(obj_key));
 	} ZEND_HASH_FOREACH_END();
-	zend_hash_destroy(&ZT_G(global_weakmap));
+	zend_hash_destroy(ZT_G(global_weakmap));
+	FREE_HASHTABLE(ZT_G(global_weakmap));
 
 	if (ZT_G(zend_test_heap))  {
 		free(ZT_G(zend_test_heap));