Introduce Zend guard recursion protection

This PR introduces a new way of recursion protection in JSON, var_dump
and friends. It fixes issue in master for __debugInfo and also improves
perf for jsonSerializable in some cases. More info can be found in
GH-10020.

Closes GH-11812

Author: bukka

Diff for: NEWS

@@ -4,6 +4,8 @@ PHP                                                                        NEWS
 
 - Core:
   . Fixed bug GH-11937 (Constant ASTs containing objects). (ilutov)
+  . Introduced Zend guard recursion protection to fix __debugInfo issue.
+    (Jakub Zelenka)
 
 17 Aug 2023, PHP 8.3.0beta3
 

Diff for: Zend/zend.c

@@ -546,6 +546,7 @@ static void zend_print_zval_r_to_buf(smart_str *buf, zval *expr, int indent) /*
 				HashTable *properties;
 
 				zend_object *zobj = Z_OBJ_P(expr);
+				uint32_t *guard = zend_get_recursion_guard(zobj);
 				zend_string *class_name = Z_OBJ_HANDLER_P(expr, get_class_name)(zobj);
 				smart_str_appends(buf, ZSTR_VAL(class_name));
 				zend_string_release_ex(class_name, 0);
@@ -561,7 +562,7 @@ static void zend_print_zval_r_to_buf(smart_str *buf, zval *expr, int indent) /*
 					smart_str_appendc(buf, '\n');
 				}
 
-				if (GC_IS_RECURSIVE(Z_OBJ_P(expr))) {
+				if (ZEND_GUARD_OR_GC_IS_RECURSIVE(guard, DEBUG, zobj)) {
 					smart_str_appends(buf, " *RECURSION*");
 					return;
 				}
@@ -571,9 +572,9 @@ static void zend_print_zval_r_to_buf(smart_str *buf, zval *expr, int indent) /*
 					break;
 				}
 
-				GC_PROTECT_RECURSION(Z_OBJ_P(expr));
+				ZEND_GUARD_OR_GC_PROTECT_RECURSION(guard, DEBUG, zobj);
 				print_hash(buf, properties, indent, 1);
-				GC_UNPROTECT_RECURSION(Z_OBJ_P(expr));
+				ZEND_GUARD_OR_GC_UNPROTECT_RECURSION(guard, DEBUG, zobj);
 
 				zend_release_properties(properties);
 				break;

Diff for: Zend/zend_API.c

@@ -2746,6 +2746,7 @@ ZEND_API void zend_add_magic_method(zend_class_entry *ce, zend_function *fptr, z
 		ce->__tostring = fptr;
 	} else if (zend_string_equals_literal(lcname, ZEND_DEBUGINFO_FUNC_NAME)) {
 		ce->__debugInfo = fptr;
+		ce->ce_flags |= ZEND_ACC_USE_GUARDS;
 	} else if (zend_string_equals_literal(lcname, "__serialize")) {
 		ce->__serialize = fptr;
 	} else if (zend_string_equals_literal(lcname, "__unserialize")) {

Diff for: Zend/zend_object_handlers.c

@@ -36,10 +36,10 @@
 #define ZEND_WRONG_PROPERTY_OFFSET   0
 
 /* guard flags */
-#define IN_GET		(1<<0)
-#define IN_SET		(1<<1)
-#define IN_UNSET	(1<<2)
-#define IN_ISSET	(1<<3)
+#define IN_GET		ZEND_GUARD_PROPERTY_GET
+#define IN_SET		ZEND_GUARD_PROPERTY_SET
+#define IN_UNSET	ZEND_GUARD_PROPERTY_UNSET
+#define IN_ISSET	ZEND_GUARD_PROPERTY_ISSET
 
 /*
   __X accessors explanation:
@@ -542,30 +542,36 @@ static void zend_property_guard_dtor(zval *el) /* {{{ */ {
 }
 /* }}} */
 
+static zend_always_inline zval *zend_get_guard_value(zend_object *zobj)
+{
+	return zobj->properties_table + zobj->ce->default_properties_count;
+}
+
 ZEND_API uint32_t *zend_get_property_guard(zend_object *zobj, zend_string *member) /* {{{ */
 {
 	HashTable *guards;
 	zval *zv;
 	uint32_t *ptr;
 
+
 	ZEND_ASSERT(zobj->ce->ce_flags & ZEND_ACC_USE_GUARDS);
-	zv = zobj->properties_table + zobj->ce->default_properties_count;
+	zv = zend_get_guard_value(zobj);
 	if (EXPECTED(Z_TYPE_P(zv) == IS_STRING)) {
 		zend_string *str = Z_STR_P(zv);
 		if (EXPECTED(str == member) ||
 		    /* str and member don't necessarily have a pre-calculated hash value here */
 		    EXPECTED(zend_string_equal_content(str, member))) {
-			return &Z_PROPERTY_GUARD_P(zv);
-		} else if (EXPECTED(Z_PROPERTY_GUARD_P(zv) == 0)) {
+			return &Z_GUARD_P(zv);
+		} else if (EXPECTED(Z_GUARD_P(zv) == 0)) {
 			zval_ptr_dtor_str(zv);
 			ZVAL_STR_COPY(zv, member);
-			return &Z_PROPERTY_GUARD_P(zv);
+			return &Z_GUARD_P(zv);
 		} else {
 			ALLOC_HASHTABLE(guards);
 			zend_hash_init(guards, 8, NULL, zend_property_guard_dtor, 0);
 			/* mark pointer as "special" using low bit */
 			zend_hash_add_new_ptr(guards, str,
-				(void*)(((uintptr_t)&Z_PROPERTY_GUARD_P(zv)) | 1));
+				(void*)(((uintptr_t)&Z_GUARD_P(zv)) | 1));
 			zval_ptr_dtor_str(zv);
 			ZVAL_ARR(zv, guards);
 		}
@@ -579,8 +585,8 @@ ZEND_API uint32_t *zend_get_property_guard(zend_object *zobj, zend_string *membe
 	} else {
 		ZEND_ASSERT(Z_TYPE_P(zv) == IS_UNDEF);
 		ZVAL_STR_COPY(zv, member);
-		Z_PROPERTY_GUARD_P(zv) = 0;
-		return &Z_PROPERTY_GUARD_P(zv);
+		Z_GUARD_P(zv) &= ~ZEND_GUARD_PROPERTY_MASK;
+		return &Z_GUARD_P(zv);
 	}
 	/* we have to allocate uint32_t separately because ht->arData may be reallocated */
 	ptr = (uint32_t*)emalloc(sizeof(uint32_t));
@@ -589,6 +595,15 @@ ZEND_API uint32_t *zend_get_property_guard(zend_object *zobj, zend_string *membe
 }
 /* }}} */
 
+ZEND_API uint32_t *zend_get_recursion_guard(zend_object *zobj)
+{
+	if (!(zobj->ce->ce_flags & ZEND_ACC_USE_GUARDS)) {
+		return NULL;
+	}
+	zval *zv = zend_get_guard_value(zobj);
+	return &Z_GUARD_P(zv);
+}
+
 ZEND_API zval *zend_std_read_property(zend_object *zobj, zend_string *name, int type, void **cache_slot, zval *rv) /* {{{ */
 {
 	zval *retval;

Diff for: Zend/zend_object_handlers.h

@@ -241,6 +241,10 @@ ZEND_API zend_function *zend_get_call_trampoline_func(const zend_class_entry *ce
 
 ZEND_API uint32_t *zend_get_property_guard(zend_object *zobj, zend_string *member);
 
+ZEND_API uint32_t *zend_get_property_guard(zend_object *zobj, zend_string *member);
+
+ZEND_API uint32_t *zend_get_recursion_guard(zend_object *zobj);
+
 /* Default behavior for get_properties_for. For use as a fallback in custom
  * get_properties_for implementations. */
 ZEND_API HashTable *zend_std_get_properties_for(zend_object *obj, zend_prop_purpose purpose);

Diff for: Zend/zend_objects.c

@@ -35,7 +35,9 @@ static zend_always_inline void _zend_object_std_init(zend_object *object, zend_c
 	object->properties = NULL;
 	zend_objects_store_put(object);
 	if (UNEXPECTED(ce->ce_flags & ZEND_ACC_USE_GUARDS)) {
-		ZVAL_UNDEF(object->properties_table + object->ce->default_properties_count);
+		zval *guard_value = object->properties_table + object->ce->default_properties_count;
+		ZVAL_UNDEF(guard_value);
+		Z_GUARD_P(guard_value) = 0;
 	}
 }
 

Diff for: Zend/zend_types.h

@@ -345,7 +345,7 @@ struct _zval_struct {
 		uint32_t     num_args;             /* arguments number for EX(This) */
 		uint32_t     fe_pos;               /* foreach position */
 		uint32_t     fe_iter_idx;          /* foreach iterator index */
-		uint32_t     property_guard;       /* single property guard */
+		uint32_t     guard;                /* recursion and single property guard */
 		uint32_t     constant_flags;       /* constant flags */
 		uint32_t     extra;                /* not further specified */
 	} u2;
@@ -619,6 +619,22 @@ struct _zend_ast_ref {
 #define _IS_BOOL					18
 #define _IS_NUMBER					19
 
+/* guard flags */
+#define ZEND_GUARD_PROPERTY_GET		(1<<0)
+#define ZEND_GUARD_PROPERTY_SET		(1<<1)
+#define ZEND_GUARD_PROPERTY_UNSET	(1<<2)
+#define ZEND_GUARD_PROPERTY_ISSET	(1<<3)
+#define ZEND_GUARD_PROPERTY_MASK	15
+#define ZEND_GUARD_RECURSION_DEBUG	(1<<4)
+#define ZEND_GUARD_RECURSION_EXPORT	(1<<5)
+#define ZEND_GUARD_RECURSION_JSON	(1<<6)
+
+#define ZEND_GUARD_RECURSION_TYPE(t) ZEND_GUARD_RECURSION_ ## t
+
+#define ZEND_GUARD_IS_RECURSIVE(pg, t)			((*pg & ZEND_GUARD_RECURSION_TYPE(t)) != 0)
+#define ZEND_GUARD_PROTECT_RECURSION(pg, t)		*pg |= ZEND_GUARD_RECURSION_TYPE(t)
+#define ZEND_GUARD_UNPROTECT_RECURSION(pg, t)	*pg &= ~ZEND_GUARD_RECURSION_TYPE(t)
+
 static zend_always_inline uint8_t zval_get_type(const zval* pz) {
 	return pz->u1.v.type;
 }
@@ -659,8 +675,8 @@ static zend_always_inline uint8_t zval_get_type(const zval* pz) {
 #define Z_FE_ITER(zval)				(zval).u2.fe_iter_idx
 #define Z_FE_ITER_P(zval_p)			Z_FE_ITER(*(zval_p))
 
-#define Z_PROPERTY_GUARD(zval)		(zval).u2.property_guard
-#define Z_PROPERTY_GUARD_P(zval_p)	Z_PROPERTY_GUARD(*(zval_p))
+#define Z_GUARD(zval)				(zval).u2.guard
+#define Z_GUARD_P(zval_p)			Z_GUARD(*(zval_p))
 
 #define Z_CONSTANT_FLAGS(zval)		(zval).u2.constant_flags
 #define Z_CONSTANT_FLAGS_P(zval_p)	Z_CONSTANT_FLAGS(*(zval_p))
@@ -859,6 +875,25 @@ static zend_always_inline uint32_t zval_gc_info(uint32_t gc_type_info) {
 #define Z_PROTECT_RECURSION_P(zv)   Z_PROTECT_RECURSION(*(zv))
 #define Z_UNPROTECT_RECURSION_P(zv) Z_UNPROTECT_RECURSION(*(zv))
 
+#define ZEND_GUARD_OR_GC_IS_RECURSIVE(pg, t, zobj) \
+	(pg ? ZEND_GUARD_IS_RECURSIVE(pg, t) : GC_IS_RECURSIVE(zobj))
+
+#define ZEND_GUARD_OR_GC_PROTECT_RECURSION(pg, t, zobj) do { \
+		if (pg) { \
+			ZEND_GUARD_PROTECT_RECURSION(pg, t); \
+		} else { \
+			GC_PROTECT_RECURSION(zobj); \
+		} \
+	} while(0)
+
+#define ZEND_GUARD_OR_GC_UNPROTECT_RECURSION(pg, t, zobj) do { \
+		if (pg) { \
+			ZEND_GUARD_UNPROTECT_RECURSION(pg, t); \
+		} else { \
+			GC_UNPROTECT_RECURSION(zobj); \
+		} \
+	} while(0)
+
 /* All data types < IS_STRING have their constructor/destructors skipped */
 #define Z_CONSTANT(zval)			(Z_TYPE(zval) == IS_CONSTANT_AST)
 #define Z_CONSTANT_P(zval_p)		Z_CONSTANT(*(zval_p))

Diff for: ext/json/json.c

@@ -37,10 +37,17 @@ PHP_JSON_API zend_class_entry *php_json_exception_ce;
 
 PHP_JSON_API ZEND_DECLARE_MODULE_GLOBALS(json)
 
+static int php_json_implement_json_serializable(zend_class_entry *interface, zend_class_entry *class_type)
+{
+	class_type->ce_flags |= ZEND_ACC_USE_GUARDS;
+	return SUCCESS;
+}
+
 /* {{{ MINIT */
 static PHP_MINIT_FUNCTION(json)
 {
 	php_json_serializable_ce = register_class_JsonSerializable();
+	php_json_serializable_ce->interface_gets_implemented = php_json_implement_json_serializable;
 
 	php_json_exception_ce = register_class_JsonException(zend_ce_exception);
 

Diff for: ext/json/json_encoder.c

@@ -531,19 +531,22 @@ zend_result php_json_escape_string(
 static zend_result php_json_encode_serializable_object(smart_str *buf, zval *val, int options, php_json_encoder *encoder) /* {{{ */
 {
 	zend_class_entry *ce = Z_OBJCE_P(val);
-	HashTable* myht = Z_OBJPROP_P(val);
+	zend_object *obj = Z_OBJ_P(val);
+	uint32_t *guard = zend_get_recursion_guard(obj);
 	zval retval, fname;
 	zend_result return_code;
 
-	if (myht && GC_IS_RECURSIVE(myht)) {
+	ZEND_ASSERT(guard != NULL);
+
+	if (ZEND_GUARD_IS_RECURSIVE(guard, JSON)) {
 		encoder->error_code = PHP_JSON_ERROR_RECURSION;
 		if (options & PHP_JSON_PARTIAL_OUTPUT_ON_ERROR) {
 			smart_str_appendl(buf, "null", 4);
 		}
 		return FAILURE;
 	}
 
-	PHP_JSON_HASH_PROTECT_RECURSION(myht);
+	ZEND_GUARD_PROTECT_RECURSION(guard, JSON);
 
 	ZVAL_STRING(&fname, "jsonSerialize");
 
@@ -556,7 +559,7 @@ static zend_result php_json_encode_serializable_object(smart_str *buf, zval *val
 		if (options & PHP_JSON_PARTIAL_OUTPUT_ON_ERROR) {
 			smart_str_appendl(buf, "null", 4);
 		}
-		PHP_JSON_HASH_UNPROTECT_RECURSION(myht);
+		ZEND_GUARD_UNPROTECT_RECURSION(guard, JSON);
 		return FAILURE;
 	}
 
@@ -568,19 +571,19 @@ static zend_result php_json_encode_serializable_object(smart_str *buf, zval *val
 		if (options & PHP_JSON_PARTIAL_OUTPUT_ON_ERROR) {
 			smart_str_appendl(buf, "null", 4);
 		}
-		PHP_JSON_HASH_UNPROTECT_RECURSION(myht);
+		ZEND_GUARD_UNPROTECT_RECURSION(guard, JSON);
 		return FAILURE;
 	}
 
 	if ((Z_TYPE(retval) == IS_OBJECT) &&
 		(Z_OBJ(retval) == Z_OBJ_P(val))) {
 		/* Handle the case where jsonSerialize does: return $this; by going straight to encode array */
-		PHP_JSON_HASH_UNPROTECT_RECURSION(myht);
+		ZEND_GUARD_UNPROTECT_RECURSION(guard, JSON);
 		return_code = php_json_encode_array(buf, &retval, options, encoder);
 	} else {
 		/* All other types, encode as normal */
 		return_code = php_json_encode_zval(buf, &retval, options, encoder);
-		PHP_JSON_HASH_UNPROTECT_RECURSION(myht);
+		ZEND_GUARD_UNPROTECT_RECURSION(guard, JSON);
 	}
 
 	zval_ptr_dtor(&retval);

Diff for: ext/json/tests/json_encode_recursion_01.phpt

@@ -0,0 +1,26 @@
+--TEST--
+json_encode() Recursion test with just JsonSerializable
+--FILE--
+<?php
+
+class SerializingTest implements JsonSerializable
+{
+    public $a = 1;
+
+    private $b = 'hide';
+
+    protected $c = 'protect';
+
+    public function jsonSerialize(): mixed
+    {
+        $result = json_encode($this);
+        var_dump($result);
+        return $this;
+    }
+}
+
+var_dump(json_encode(new SerializingTest()));
+?>
+--EXPECT--
+bool(false)
+string(7) "{"a":1}"

Diff for: ext/json/tests/json_encode_recursion_02.phpt

@@ -0,0 +1,25 @@
+--TEST--
+json_encode() Recursion test with JsonSerializable and var_dump simple
+--FILE--
+<?php
+
+class SerializingTest implements JsonSerializable
+{
+    public $a = 1;
+
+    public function jsonSerialize(): mixed
+    {
+        var_dump($this);
+        return $this;
+    }
+}
+
+var_dump(json_encode(new SerializingTest()));
+
+?>
+--EXPECT--
+object(SerializingTest)#1 (1) {
+  ["a"]=>
+  int(1)
+}
+string(7) "{"a":1}"

Diff for: ext/json/tests/json_encode_recursion_03.phpt

@@ -0,0 +1,38 @@
+--TEST--
+json_encode() Recursion test with JsonSerializable and __debugInfo
+--FILE--
+<?php
+
+class SerializingTest implements JsonSerializable
+{
+    public $a = 1;
+
+    public function __debugInfo()
+    {
+        return [ 'result' => json_encode($this) ];
+    }
+
+    public function jsonSerialize(): mixed
+    {
+        var_dump($this);
+        return $this;
+    }
+}
+
+var_dump(json_encode(new SerializingTest()));
+echo "---------\n";
+var_dump(new SerializingTest());
+
+?>
+--EXPECT--
+object(SerializingTest)#1 (1) {
+  ["result"]=>
+  bool(false)
+}
+string(7) "{"a":1}"
+---------
+*RECURSION*
+object(SerializingTest)#1 (1) {
+  ["result"]=>
+  string(7) "{"a":1}"
+}