From 234af00bcb93e0be1d9b17ab3f2d366b09a99611 Mon Sep 17 00:00:00 2001
From: Nikita Popov <nikita.ppv@gmail.com>
Date: Thu, 18 Mar 2021 10:14:32 +0100
Subject: [PATCH] Destroy constant values before object store

Now that constants can contain objects (currently only enums),
we should destroy them before we free the object store, otherwise
there will be false positive leak reports.

This doesn't affect the fast_shutdown sequence.
---
 Zend/zend_execute_API.c | 44 +++++++++++++++++++++++++++--------------
 Zend/zend_objects_API.c |  5 +----
 2 files changed, 30 insertions(+), 19 deletions(-)

diff --git a/Zend/zend_execute_API.c b/Zend/zend_execute_API.c
index f61a865ddcb..ce51ef7960e 100644
--- a/Zend/zend_execute_API.c
+++ b/Zend/zend_execute_API.c
@@ -278,6 +278,24 @@ void shutdown_executor(void) /* {{{ */
 	if (!fast_shutdown) {
 		zend_hash_graceful_reverse_destroy(&EG(symbol_table));
 
+		/* Constants may contain objects, destroy them before the object store. */
+		if (EG(full_tables_cleanup)) {
+			zend_hash_reverse_apply(EG(zend_constants), clean_non_persistent_constant_full);
+		} else {
+			ZEND_HASH_REVERSE_FOREACH_STR_KEY_VAL(EG(zend_constants), key, zv) {
+				zend_constant *c = Z_PTR_P(zv);
+				if (_idx == EG(persistent_constants_count)) {
+					break;
+				}
+				zval_ptr_dtor_nogc(&c->value);
+				if (c->name) {
+					zend_string_release_ex(c->name, 0);
+				}
+				efree(c);
+				zend_string_release_ex(key, 0);
+			} ZEND_HASH_FOREACH_END_DEL();
+		}
+
 		/* Release static properties and static variables prior to the final GC run,
 		 * as they may hold GC roots. */
 		ZEND_HASH_REVERSE_FOREACH_VAL(EG(function_table), zv) {
@@ -302,6 +320,15 @@ void shutdown_executor(void) /* {{{ */
 
 			if (ZEND_MAP_PTR(ce->mutable_data) && ZEND_MAP_PTR_GET_IMM(ce->mutable_data)) {
 				zend_cleanup_mutable_class_data(ce);
+			} else if (ce->type == ZEND_USER_CLASS && !(ce->ce_flags & ZEND_ACC_IMMUTABLE)) {
+				/* Constants may contain objects, destroy the values before the object store. */
+				zend_class_constant *c;
+				ZEND_HASH_FOREACH_PTR(&ce->constants_table, c) {
+					if (c->ce == ce) {
+						zval_ptr_dtor_nogc(&c->value);
+						ZVAL_UNDEF(&c->value);
+					}
+				} ZEND_HASH_FOREACH_END();
 			}
 
 			if (ce->ce_flags & ZEND_HAS_STATIC_IN_METHODS) {
@@ -340,6 +367,8 @@ void shutdown_executor(void) /* {{{ */
 			gc_collect_cycles();
 		}
 #endif
+	} else {
+		zend_hash_discard(EG(zend_constants), EG(persistent_constants_count));
 	}
 
 	zend_objects_store_free_object_storage(&EG(objects_store), fast_shutdown);
@@ -356,7 +385,6 @@ void shutdown_executor(void) /* {{{ */
 		 * Zend Memory Manager frees memory by its own. We don't have to free
 		 * each allocated block separately.
 		 */
-		zend_hash_discard(EG(zend_constants), EG(persistent_constants_count));
 		zend_hash_discard(EG(function_table), EG(persistent_functions_count));
 		zend_hash_discard(EG(class_table), EG(persistent_classes_count));
 		zend_cleanup_internal_classes();
@@ -364,23 +392,9 @@ void shutdown_executor(void) /* {{{ */
 		zend_vm_stack_destroy();
 
 		if (EG(full_tables_cleanup)) {
-			zend_hash_reverse_apply(EG(zend_constants), clean_non_persistent_constant_full);
 			zend_hash_reverse_apply(EG(function_table), clean_non_persistent_function_full);
 			zend_hash_reverse_apply(EG(class_table), clean_non_persistent_class_full);
 		} else {
-			ZEND_HASH_REVERSE_FOREACH_STR_KEY_VAL(EG(zend_constants), key, zv) {
-				zend_constant *c = Z_PTR_P(zv);
-				if (_idx == EG(persistent_constants_count)) {
-					break;
-				}
-				zval_ptr_dtor_nogc(&c->value);
-				if (c->name) {
-					zend_string_release_ex(c->name, 0);
-				}
-				efree(c);
-				zend_string_release_ex(key, 0);
-			} ZEND_HASH_FOREACH_END_DEL();
-
 			ZEND_HASH_REVERSE_FOREACH_STR_KEY_VAL(EG(function_table), key, zv) {
 				zend_function *func = Z_PTR_P(zv);
 				if (_idx == EG(persistent_functions_count)) {
diff --git a/Zend/zend_objects_API.c b/Zend/zend_objects_API.c
index b0e9d2f8a8c..104cda61412 100644
--- a/Zend/zend_objects_API.c
+++ b/Zend/zend_objects_API.c
@@ -113,10 +113,7 @@ ZEND_API void ZEND_FASTCALL zend_objects_store_free_object_storage(zend_objects_
 			if (IS_OBJ_VALID(obj)) {
 				if (!(OBJ_FLAGS(obj) & IS_OBJ_FREE_CALLED)) {
 					GC_ADD_FLAGS(obj, IS_OBJ_FREE_CALLED);
-					// FIXME: This causes constant objects to leak
-					if (!(obj->ce->ce_flags & ZEND_ACC_ENUM)) {
-						GC_ADDREF(obj);
-					}
+					GC_ADDREF(obj);
 					obj->handlers->free_obj(obj);
 				}
 			}