Fix Enum::from/tryFrom memory leak in JIT for internal enums

when passing an int to a string enum. Previously, the int was coerced to
a string. The JIT skips parameter clean up when unnecessary. In this
particular case, passing int to from(int|string) normally doesn't cause
a coercion so no dtor for the $value zval is generated.

To circumvent this we avoid coersion by explicitly allowing ints and
converting them to strings ourselves. Then we can free it appropriately.

See GH-8518
Closes GH-8633

Author: iluuu1994

NEWS

@@ -4,6 +4,7 @@ PHP                                                                        NEWS
 
 - Core:
   . Fixed bug GH-8338 (Intel CET is disabled unintentionally). (Chen, Hu)
+  . Fixed leak in Enum::from/tryFrom for internal enums when using JIT (ilutov)
 
 - Date:
   . Fixed bug #72963 (Null-byte injection in CreateFromFormat and related

Zend/tests/enum/internal_enums.phpt

@@ -21,6 +21,9 @@ var_dump(ZendTestStringEnum::Foo->value);
 var_dump($bar = ZendTestStringEnum::from("Test2"));
 var_dump($bar === ZendTestStringEnum::Bar);
 var_dump(ZendTestStringEnum::tryFrom("Test3"));
+var_dump(ZendTestStringEnum::tryFrom(42));
+var_dump(ZendTestStringEnum::tryFrom(43));
+var_dump(ZendTestStringEnum::tryFrom(0));
 var_dump(ZendTestStringEnum::cases());
 
 var_dump($s = serialize($foo));
@@ -47,13 +50,18 @@ string(5) "Test1"
 enum(ZendTestStringEnum::Bar)
 bool(true)
 NULL
-array(3) {
+enum(ZendTestStringEnum::FortyTwo)
+NULL
+NULL
+array(4) {
   [0]=>
   enum(ZendTestStringEnum::Foo)
   [1]=>
   enum(ZendTestStringEnum::Bar)
   [2]=>
   enum(ZendTestStringEnum::Baz)
+  [3]=>
+  enum(ZendTestStringEnum::FortyTwo)
 }
 string(30) "E:22:"ZendTestStringEnum:Foo";"
 enum(ZendTestStringEnum::Foo)

Zend/tests/enum/internal_enums_strict_types.phpt

@@ -0,0 +1,28 @@
+--TEST--
+Internal enums from/tryFrom in strict_types=1
+--EXTENSIONS--
+zend_test
+--FILE--
+<?php
+
+declare(strict_types=1);
+
+var_dump(ZendTestStringEnum::from("Test2"));
+
+try {
+    var_dump(ZendTestStringEnum::from(42));
+} catch (Error $e) {
+    echo $e->getMessage(), "\n";
+}
+
+try {
+    var_dump(ZendTestStringEnum::tryFrom(43));
+} catch (Error $e) {
+    echo $e->getMessage(), "\n";
+}
+
+?>
+--EXPECT--
+enum(ZendTestStringEnum::Bar)
+ZendTestStringEnum::from(): Argument #1 ($value) must be of type string, int given
+ZendTestStringEnum::tryFrom(): Argument #1 ($value) must be of type string, int given

Zend/zend_enum.c

@@ -210,6 +210,7 @@ static ZEND_NAMED_FUNCTION(zend_enum_cases_func)
 static void zend_enum_from_base(INTERNAL_FUNCTION_PARAMETERS, bool try)
 {
 	zend_class_entry *ce = execute_data->func->common.scope;
+	bool release_string = false;
 	zend_string *string_key;
 	zend_long long_key;
 
@@ -221,17 +222,33 @@ static void zend_enum_from_base(INTERNAL_FUNCTION_PARAMETERS, bool try)
 
 		case_name_zv = zend_hash_index_find(ce->backed_enum_table, long_key);
 	} else {
-		ZEND_PARSE_PARAMETERS_START(1, 1)
-			Z_PARAM_STR(string_key)
-		ZEND_PARSE_PARAMETERS_END();
-
 		ZEND_ASSERT(ce->enum_backing_type == IS_STRING);
+
+		if (ZEND_ARG_USES_STRICT_TYPES()) {
+			ZEND_PARSE_PARAMETERS_START(1, 1)
+				Z_PARAM_STR(string_key)
+			ZEND_PARSE_PARAMETERS_END();
+		} else {
+			// We allow long keys so that coercion to string doesn't happen implicitly. The JIT
+			// skips deallocation of params that don't require it. In the case of from/tryFrom
+			// passing int to from(int|string) looks like no coercion will happen, so the JIT
+			// won't emit a dtor call. Thus we allocate/free the string manually.
+			ZEND_PARSE_PARAMETERS_START(1, 1)
+				Z_PARAM_STR_OR_LONG(string_key, long_key)
+			ZEND_PARSE_PARAMETERS_END();
+
+			if (string_key == NULL) {
+				release_string = true;
+				string_key = zend_long_to_str(long_key);
+			}
+		}
+
 		case_name_zv = zend_hash_find(ce->backed_enum_table, string_key);
 	}
 
 	if (case_name_zv == NULL) {
 		if (try) {
-			RETURN_NULL();
+			goto return_null;
 		}
 
 		if (ce->enum_backing_type == IS_LONG) {
@@ -240,7 +257,7 @@ static void zend_enum_from_base(INTERNAL_FUNCTION_PARAMETERS, bool try)
 			ZEND_ASSERT(ce->enum_backing_type == IS_STRING);
 			zend_value_error("\"%s\" is not a valid backing value for enum \"%s\"", ZSTR_VAL(string_key), ZSTR_VAL(ce->name));
 		}
-		RETURN_THROWS();
+		goto throw;
 	}
 
 	// TODO: We might want to store pointers to constants in backed_enum_table instead of names,
@@ -251,11 +268,26 @@ static void zend_enum_from_base(INTERNAL_FUNCTION_PARAMETERS, bool try)
 	zval *case_zv = &c->value;
 	if (Z_TYPE_P(case_zv) == IS_CONSTANT_AST) {
 		if (zval_update_constant_ex(case_zv, c->ce) == FAILURE) {
-			RETURN_THROWS();
+			goto throw;
 		}
 	}
 
-	ZVAL_COPY(return_value, case_zv);
+	if (release_string) {
+		zend_string_release(string_key);
+	}
+	RETURN_COPY(case_zv);
+
+throw:
+	if (release_string) {
+		zend_string_release(string_key);
+	}
+	RETURN_THROWS();
+
+return_null:
+	if (release_string) {
+		zend_string_release(string_key);
+	}
+	RETURN_NULL();
 }
 
 static ZEND_NAMED_FUNCTION(zend_enum_from_func)

ext/zend_test/test.stub.php

@@ -72,6 +72,7 @@ enum ZendTestStringEnum: string {
         case Foo = "Test1";
         case Bar = "Test2";
         case Baz = "Test2\\a";
+        case FortyTwo = "42";
     }
 
     function zend_test_array_return(): array {}

ext/zend_test/test_arginfo.h

@@ -1,5 +1,5 @@
 /* This is a generated file, edit the .stub.php file instead.
- * Stub hash: 7b0abebae0b0eeea0f45dccb04759fceec1096e3 */
+ * Stub hash: 6d9ff0e2263a39420dd157206331a9e897d9e775 */
 
 ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_zend_test_array_return, 0, 0, IS_ARRAY, 0)
 ZEND_END_ARG_INFO()
@@ -417,6 +417,11 @@ static zend_class_entry *register_class_ZendTestStringEnum(void)
 	ZVAL_STR(&enum_case_Baz_value, enum_case_Baz_value_str);
 	zend_enum_add_case_cstr(class_entry, "Baz", &enum_case_Baz_value);
 
+	zval enum_case_FortyTwo_value;
+	zend_string *enum_case_FortyTwo_value_str = zend_string_init("42", sizeof("42") - 1, 1);
+	ZVAL_STR(&enum_case_FortyTwo_value, enum_case_FortyTwo_value_str);
+	zend_enum_add_case_cstr(class_entry, "FortyTwo", &enum_case_FortyTwo_value);
+
 	return class_entry;
 }