From 169044f23fbad5e4aa97892769650254ac118eeb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= <tim@tideways-gmbh.com>
Date: Thu, 13 Jun 2024 13:47:18 +0200
Subject: [PATCH 1/6] zend_compile: Rename `string_placeholder_count` to
 `placeholder_count` in `zend_compile_func_sprintf()`

This is intended to make the diff of a follow-up commit smaller.
---
 Zend/zend_compile.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c
index 65051223c157d..82278ae7eea75 100644
--- a/Zend/zend_compile.c
+++ b/Zend/zend_compile.c
@@ -4739,9 +4739,9 @@ static zend_result zend_compile_func_sprintf(znode *result, zend_ast_list *args)
 
 	char *p;
 	char *end;
-	uint32_t string_placeholder_count;
+	uint32_t placeholder_count;
 	
-	string_placeholder_count = 0;
+	placeholder_count = 0;
 	p = Z_STRVAL_P(format_string);
 	end = p + Z_STRLEN_P(format_string);
 
@@ -4758,7 +4758,7 @@ static zend_result zend_compile_func_sprintf(znode *result, zend_ast_list *args)
 
 		switch (*q) {
 		case 's':
-			string_placeholder_count++;
+			placeholder_count++;
 			break;
 		case '%':
 			break;
@@ -4771,7 +4771,7 @@ static zend_result zend_compile_func_sprintf(znode *result, zend_ast_list *args)
 	}
 
 	/* Bail out if the number of placeholders does not match the number of values. */
-	if (string_placeholder_count != (args->children - 1)) {
+	if (placeholder_count != (args->children - 1)) {
 		return FAILURE;
 	}
 
@@ -4785,14 +4785,14 @@ static zend_result zend_compile_func_sprintf(znode *result, zend_ast_list *args)
 
 	znode *elements = NULL;
 
-	if (string_placeholder_count > 0) {
-		elements = safe_emalloc(sizeof(*elements), string_placeholder_count, 0);
+	if (placeholder_count > 0) {
+		elements = safe_emalloc(sizeof(*elements), placeholder_count, 0);
 	}
 
 	/* Compile the value expressions first for error handling that is consistent
 	 * with a function call: Values that fail to convert to a string may emit errors.
 	 */
-	for (uint32_t i = 0; i < string_placeholder_count; i++) {
+	for (uint32_t i = 0; i < placeholder_count; i++) {
 		zend_compile_expr(elements + i, args->child[1 + i]);
 		if (elements[i].op_type == IS_CONST) {
 			if (Z_TYPE(elements[i].u.constant) != IS_ARRAY) {
@@ -4805,7 +4805,7 @@ static zend_result zend_compile_func_sprintf(znode *result, zend_ast_list *args)
 	uint32_t rope_init_lineno = -1;
 	zend_op *opline = NULL;
 
-	string_placeholder_count = 0;
+	placeholder_count = 0;
 	p = Z_STRVAL_P(format_string);
 	end = p + Z_STRLEN_P(format_string);
 	char *offset = p;
@@ -4841,17 +4841,17 @@ static zend_result zend_compile_func_sprintf(znode *result, zend_ast_list *args)
 			/* Perform the cast of constant arrays when actually evaluating corresponding placeholder
 			 * for correct error reporting.
 			 */
-			if (elements[string_placeholder_count].op_type == IS_CONST) {
-				if (Z_TYPE(elements[string_placeholder_count].u.constant) == IS_ARRAY) {
-					zend_emit_op_tmp(&elements[string_placeholder_count], ZEND_CAST, &elements[string_placeholder_count], NULL)->extended_value = IS_STRING;
+			if (elements[placeholder_count].op_type == IS_CONST) {
+				if (Z_TYPE(elements[placeholder_count].u.constant) == IS_ARRAY) {
+					zend_emit_op_tmp(&elements[placeholder_count], ZEND_CAST, &elements[placeholder_count], NULL)->extended_value = IS_STRING;
 				}
 			}
 			if (rope_elements == 0) {
 				rope_init_lineno = get_next_op_number();
 			}
-			opline = zend_compile_rope_add(result, rope_elements++, &elements[string_placeholder_count]);
+			opline = zend_compile_rope_add(result, rope_elements++, &elements[placeholder_count]);
 
-			string_placeholder_count++;
+			placeholder_count++;
 		}
 
 		p = q;

From f4f2ebcdffc7d0b3ff860612e04a29066be82467 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= <tim@tideways-gmbh.com>
Date: Wed, 12 Jun 2024 12:06:32 +0200
Subject: [PATCH 2/6] zend_compile: Add support for `%d` to `sprintf()`
 optimization
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This extends the existing `sprintf()` optimization by support for the `%d`
placeholder, which effectively equivalent to an `(int)` cast followed by a
`(string)` cast.

For a synthetic test using:

    <?php

    $a = 'foo';
    $b = 42;

    for ($i = 0; $i < 100_000_000; $i++) {
        sprintf("%s-%d", $a, $b);
    }

This optimization yields a 1.3× performance improvement:

    $ hyperfine 'sapi/cli/php -d zend_extension=php-src/modules/opcache.so -d opcache.enable_cli=1 test.php' \
          '/tmp/unoptimized -d zend_extension=php-src/modules/opcache.so -d opcache.enable_cli=1 test.php'
    Benchmark 1: sapi/cli/php -d zend_extension=php-src/modules/opcache.so -d opcache.enable_cli=1 test.php
      Time (mean ± σ):      3.296 s ±  0.094 s    [User: 3.287 s, System: 0.005 s]
      Range (min … max):    3.213 s …  3.527 s    10 runs

    Benchmark 2: /tmp/unoptimized -d zend_extension=php-src/modules/opcache.so -d opcache.enable_cli=1 test.php
      Time (mean ± σ):      4.300 s ±  0.025 s    [User: 4.290 s, System: 0.007 s]
      Range (min … max):    4.266 s …  4.334 s    10 runs

    Summary
      sapi/cli/php -d zend_extension=php-src/modules/opcache.so -d opcache.enable_cli=1 test.php ran
        1.30 ± 0.04 times faster than /tmp/unoptimized -d zend_extension=php-src/modules/opcache.so -d opcache.enable_cli=1 test.php
---
 Zend/zend_compile.c                           |  33 +++--
 .../sprintf_rope_optimization_001.phpt        |   6 +
 .../sprintf_rope_optimization_003.phpt        | 127 ++++++++++++++++++
 3 files changed, 153 insertions(+), 13 deletions(-)
 create mode 100644 ext/standard/tests/strings/sprintf_rope_optimization_003.phpt

diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c
index 82278ae7eea75..2f618e513b07e 100644
--- a/Zend/zend_compile.c
+++ b/Zend/zend_compile.c
@@ -4758,6 +4758,7 @@ static zend_result zend_compile_func_sprintf(znode *result, zend_ast_list *args)
 
 		switch (*q) {
 		case 's':
+		case 'd':
 			placeholder_count++;
 			break;
 		case '%':
@@ -4794,11 +4795,6 @@ static zend_result zend_compile_func_sprintf(znode *result, zend_ast_list *args)
 	 */
 	for (uint32_t i = 0; i < placeholder_count; i++) {
 		zend_compile_expr(elements + i, args->child[1 + i]);
-		if (elements[i].op_type == IS_CONST) {
-			if (Z_TYPE(elements[i].u.constant) != IS_ARRAY) {
-				convert_to_string(&elements[i].u.constant);
-			}
-		}
 	}
 
 	uint32_t rope_elements = 0;
@@ -4817,7 +4813,7 @@ static zend_result zend_compile_func_sprintf(znode *result, zend_ast_list *args)
 
 		char *q = p + 1;
 		ZEND_ASSERT(q < end);
-		ZEND_ASSERT(*q == 's' || *q == '%');
+		ZEND_ASSERT(*q == 's' || *q == 'd' || *q == '%');
 
 		if (*q == '%') {
 			/* Optimization to not create a dedicated rope element for the literal '%':
@@ -4837,15 +4833,26 @@ static zend_result zend_compile_func_sprintf(znode *result, zend_ast_list *args)
 			opline = zend_compile_rope_add(result, rope_elements++, &const_node);
 		}
 
-		if (*q == 's') {
-			/* Perform the cast of constant arrays when actually evaluating corresponding placeholder
-			 * for correct error reporting.
-			 */
-			if (elements[placeholder_count].op_type == IS_CONST) {
-				if (Z_TYPE(elements[placeholder_count].u.constant) == IS_ARRAY) {
-					zend_emit_op_tmp(&elements[placeholder_count], ZEND_CAST, &elements[placeholder_count], NULL)->extended_value = IS_STRING;
+		if (*q != '%') {
+			switch (*q) {
+			case 's':
+				/* Perform the cast of constants when actually evaluating the corresponding placeholder
+				 * for correct error reporting.
+				 */
+				if (elements[placeholder_count].op_type == IS_CONST) {
+					if (Z_TYPE(elements[placeholder_count].u.constant) == IS_ARRAY) {
+						zend_emit_op_tmp(&elements[placeholder_count], ZEND_CAST, &elements[placeholder_count], NULL)->extended_value = IS_STRING;
+					} else {
+						convert_to_string(&elements[placeholder_count].u.constant);
+					}
 				}
+				break;
+			case 'd':
+				zend_emit_op_tmp(&elements[placeholder_count], ZEND_CAST, &elements[placeholder_count], NULL)->extended_value = IS_LONG;
+				break;
+			EMPTY_SWITCH_DEFAULT_CASE();
 			}
+
 			if (rope_elements == 0) {
 				rope_init_lineno = get_next_op_number();
 			}
diff --git a/ext/standard/tests/strings/sprintf_rope_optimization_001.phpt b/ext/standard/tests/strings/sprintf_rope_optimization_001.phpt
index 91fdddfe029d4..634e575729d6f 100644
--- a/ext/standard/tests/strings/sprintf_rope_optimization_001.phpt
+++ b/ext/standard/tests/strings/sprintf_rope_optimization_001.phpt
@@ -100,6 +100,10 @@ try {
 	var_dump(sprintf());
 } catch (\Throwable $e) {echo $e, PHP_EOL; } echo PHP_EOL;
 
+try {
+	var_dump(sprintf('%s-%s-%s', true, false, true));
+} catch (\Throwable $e) {echo $e, PHP_EOL; } echo PHP_EOL;
+
 echo "Done";
 ?>
 --EXPECTF--
@@ -173,4 +177,6 @@ Stack trace:
 #0 %s(97): sprintf()
 #1 {main}
 
+string(4) "1--1"
+
 Done
diff --git a/ext/standard/tests/strings/sprintf_rope_optimization_003.phpt b/ext/standard/tests/strings/sprintf_rope_optimization_003.phpt
new file mode 100644
index 0000000000000..840eb23e1d7ab
--- /dev/null
+++ b/ext/standard/tests/strings/sprintf_rope_optimization_003.phpt
@@ -0,0 +1,127 @@
+--TEST--
+Test sprintf() function : Rope Optimization for '%d'.
+--FILE--
+<?php
+function func($num) {
+	return $num + 1;
+}
+function sideeffect() {
+	echo "Called!\n";
+	return "foo";
+}
+class Foo {
+	public function __construct() {
+		echo "Called\n";
+	}
+}
+
+$a = 42;
+$b = -1337;
+$c = 3.14;
+$d = new stdClass();
+
+try {
+	var_dump(sprintf("%d", $a));
+} catch (\Throwable $e) {echo $e, PHP_EOL; } echo PHP_EOL;
+
+try {
+	var_dump(sprintf("%d/%d", $a, $b));
+} catch (\Throwable $e) {echo $e, PHP_EOL; } echo PHP_EOL;
+
+try {
+	var_dump(sprintf("%d/%d/%d", $a, $b, $c));
+} catch (\Throwable $e) {echo $e, PHP_EOL; } echo PHP_EOL;
+
+try {
+	var_dump(sprintf("%d/%d/%d/%d", $a, $b, $c, $d));
+} catch (\Throwable $e) {echo $e, PHP_EOL; } echo PHP_EOL;
+
+try {
+	var_dump(sprintf("%d/", func(0)));
+} catch (\Throwable $e) {echo $e, PHP_EOL; } echo PHP_EOL;
+
+try {
+	var_dump(sprintf("/%d", func(0)));
+} catch (\Throwable $e) {echo $e, PHP_EOL; } echo PHP_EOL;
+
+try {
+	var_dump(sprintf("/%d/", func(0)));
+} catch (\Throwable $e) {echo $e, PHP_EOL; } echo PHP_EOL;
+
+try {
+	var_dump(sprintf("%d/%d/%d/%d", $a, $b, func(0), $a));
+} catch (\Throwable $e) {echo $e, PHP_EOL; } echo PHP_EOL;
+
+try {
+	var_dump(sprintf("%d/%d/%d/%d", __FILE__, __LINE__, 1, M_PI));
+} catch (\Throwable $e) {echo $e, PHP_EOL; } echo PHP_EOL;
+
+try {
+	var_dump(sprintf("%d/%d/%d", new Foo(), new Foo(), new Foo(), ));
+} catch (\Throwable $e) {echo $e, PHP_EOL; } echo PHP_EOL;
+
+try {
+	var_dump(sprintf('%d/%d/%d', [], [], []));
+} catch (\Throwable $e) {echo $e, PHP_EOL; } echo PHP_EOL;
+
+try {
+	var_dump(sprintf('%d/%d/%d', PHP_INT_MAX, 0, PHP_INT_MIN));
+} catch (\Throwable $e) {echo $e, PHP_EOL; } echo PHP_EOL;
+
+try {
+	var_dump(sprintf('%d/%d/%d', true, false, true));
+} catch (\Throwable $e) {echo $e, PHP_EOL; } echo PHP_EOL;
+
+try {
+	var_dump(sprintf("%d/%d", true, 'foo'));
+} catch (\Throwable $e) {echo $e, PHP_EOL; } echo PHP_EOL;
+
+try {
+	var_dump(sprintf("%d", 'foo'));
+} catch (\Throwable $e) {echo $e, PHP_EOL; } echo PHP_EOL;
+
+echo "Done";
+?>
+--EXPECTF--
+string(2) "42"
+
+string(8) "42/-1337"
+
+string(10) "42/-1337/3"
+
+
+Warning: Object of class stdClass could not be converted to int in %s on line 33
+string(12) "42/-1337/3/1"
+
+string(2) "1/"
+
+string(2) "/1"
+
+string(3) "/1/"
+
+string(13) "42/-1337/1/42"
+
+string(8) "0/53/1/3"
+
+Called
+Called
+Called
+
+Warning: Object of class Foo could not be converted to int in %s on line 57
+
+Warning: Object of class Foo could not be converted to int in %s on line 57
+
+Warning: Object of class Foo could not be converted to int in %s on line 57
+string(5) "1/1/1"
+
+string(5) "0/0/0"
+
+string(42) "9223372036854775807/0/-9223372036854775808"
+
+string(5) "1/0/1"
+
+string(3) "1/0"
+
+string(1) "0"
+
+Done

From 744c53bb58286e34fb4303e569250af6e0e3a9c9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= <tim@tideways-gmbh.com>
Date: Thu, 13 Jun 2024 14:27:31 +0200
Subject: [PATCH 3/6] Fix sprintf_rope_optimization_003.phpt test expecation
 for 32-bit integers

---
 .../tests/strings/sprintf_rope_optimization_003.phpt     | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/ext/standard/tests/strings/sprintf_rope_optimization_003.phpt b/ext/standard/tests/strings/sprintf_rope_optimization_003.phpt
index 840eb23e1d7ab..9a0d819b2771e 100644
--- a/ext/standard/tests/strings/sprintf_rope_optimization_003.phpt
+++ b/ext/standard/tests/strings/sprintf_rope_optimization_003.phpt
@@ -65,7 +65,13 @@ try {
 } catch (\Throwable $e) {echo $e, PHP_EOL; } echo PHP_EOL;
 
 try {
-	var_dump(sprintf('%d/%d/%d', PHP_INT_MAX, 0, PHP_INT_MIN));
+	if (PHP_INT_SIZE == 8) {
+		var_dump(sprintf('%d/%d/%d', PHP_INT_MAX, 0, PHP_INT_MIN));
+		var_dump("2147483647/0/-2147483648");
+	} else {
+		var_dump("9223372036854775807/0/-9223372036854775808");
+		var_dump(sprintf('%d/%d/%d', PHP_INT_MAX, 0, PHP_INT_MIN));
+	}
 } catch (\Throwable $e) {echo $e, PHP_EOL; } echo PHP_EOL;
 
 try {
@@ -117,6 +123,7 @@ string(5) "1/1/1"
 string(5) "0/0/0"
 
 string(42) "9223372036854775807/0/-9223372036854775808"
+string(24) "2147483647/0/-2147483648"
 
 string(5) "1/0/1"
 

From 9da37d7b7db58e9b5c62198a2c4ca02e9abe023a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= <tim@tideways-gmbh.com>
Date: Mon, 17 Jun 2024 09:17:46 +0200
Subject: [PATCH 4/6] zend_compile: Indent switch-case labels in
 zend_compile_func_sprintf()

---
 Zend/zend_compile.c | 46 ++++++++++++++++++++++-----------------------
 1 file changed, 23 insertions(+), 23 deletions(-)

diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c
index 2f618e513b07e..6f934f7a3fd91 100644
--- a/Zend/zend_compile.c
+++ b/Zend/zend_compile.c
@@ -4757,14 +4757,14 @@ static zend_result zend_compile_func_sprintf(znode *result, zend_ast_list *args)
 		}
 
 		switch (*q) {
-		case 's':
-		case 'd':
-			placeholder_count++;
-			break;
-		case '%':
-			break;
-		default:
-			return FAILURE;
+			case 's':
+			case 'd':
+				placeholder_count++;
+				break;
+			case '%':
+				break;
+			default:
+				return FAILURE;
 		}
 
 		p = q;
@@ -4835,22 +4835,22 @@ static zend_result zend_compile_func_sprintf(znode *result, zend_ast_list *args)
 
 		if (*q != '%') {
 			switch (*q) {
-			case 's':
-				/* Perform the cast of constants when actually evaluating the corresponding placeholder
-				 * for correct error reporting.
-				 */
-				if (elements[placeholder_count].op_type == IS_CONST) {
-					if (Z_TYPE(elements[placeholder_count].u.constant) == IS_ARRAY) {
-						zend_emit_op_tmp(&elements[placeholder_count], ZEND_CAST, &elements[placeholder_count], NULL)->extended_value = IS_STRING;
-					} else {
-						convert_to_string(&elements[placeholder_count].u.constant);
+				case 's':
+					/* Perform the cast of constants when actually evaluating the corresponding placeholder
+					 * for correct error reporting.
+					 */
+					if (elements[placeholder_count].op_type == IS_CONST) {
+						if (Z_TYPE(elements[placeholder_count].u.constant) == IS_ARRAY) {
+							zend_emit_op_tmp(&elements[placeholder_count], ZEND_CAST, &elements[placeholder_count], NULL)->extended_value = IS_STRING;
+						} else {
+							convert_to_string(&elements[placeholder_count].u.constant);
+						}
 					}
-				}
-				break;
-			case 'd':
-				zend_emit_op_tmp(&elements[placeholder_count], ZEND_CAST, &elements[placeholder_count], NULL)->extended_value = IS_LONG;
-				break;
-			EMPTY_SWITCH_DEFAULT_CASE();
+					break;
+				case 'd':
+					zend_emit_op_tmp(&elements[placeholder_count], ZEND_CAST, &elements[placeholder_count], NULL)->extended_value = IS_LONG;
+					break;
+				EMPTY_SWITCH_DEFAULT_CASE();
 			}
 
 			if (rope_elements == 0) {

From 104a0915fc61a4760cd928b58bea2cceefd0449e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= <tim@tideways-gmbh.com>
Date: Mon, 17 Jun 2024 17:02:14 +0200
Subject: [PATCH 5/6] Add GMP test to sprintf() rope optimization

---
 .../sprintf_rope_optimization_004.phpt        | 21 +++++++++++++++++++
 1 file changed, 21 insertions(+)
 create mode 100644 ext/standard/tests/strings/sprintf_rope_optimization_004.phpt

diff --git a/ext/standard/tests/strings/sprintf_rope_optimization_004.phpt b/ext/standard/tests/strings/sprintf_rope_optimization_004.phpt
new file mode 100644
index 0000000000000..bec2eabb78330
--- /dev/null
+++ b/ext/standard/tests/strings/sprintf_rope_optimization_004.phpt
@@ -0,0 +1,21 @@
+--TEST--
+Test sprintf() function : Rope Optimization for '%d' with GMP objects
+--EXTENSIONS--
+gmp
+--FILE--
+<?php
+
+$a = new GMP("42");
+$b = new GMP("-1337");
+$c = new GMP("999999999999999999999999999999999");
+
+try {
+	var_dump(sprintf("%d/%d/%d", $a, $b, $c));
+} catch (\Throwable $e) {echo $e, PHP_EOL; } echo PHP_EOL;
+
+echo "Done";
+?>
+--EXPECTF--
+string(28) "42/-1337/4089650035136921599"
+
+Done

From b012cf874242a523f4fd01f7c458c4d7a2e6808e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= <tim@tideways-gmbh.com>
Date: Mon, 17 Jun 2024 17:07:19 +0200
Subject: [PATCH 6/6] Add `%s` test case to sprintf() GMP test

---
 ext/standard/tests/strings/sprintf_rope_optimization_004.phpt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ext/standard/tests/strings/sprintf_rope_optimization_004.phpt b/ext/standard/tests/strings/sprintf_rope_optimization_004.phpt
index bec2eabb78330..a1c937aeb2c2e 100644
--- a/ext/standard/tests/strings/sprintf_rope_optimization_004.phpt
+++ b/ext/standard/tests/strings/sprintf_rope_optimization_004.phpt
@@ -10,12 +10,12 @@ $b = new GMP("-1337");
 $c = new GMP("999999999999999999999999999999999");
 
 try {
-	var_dump(sprintf("%d/%d/%d", $a, $b, $c));
+	var_dump(sprintf("%d/%d/%d/%s", $a, $b, $c, $c + 1));
 } catch (\Throwable $e) {echo $e, PHP_EOL; } echo PHP_EOL;
 
 echo "Done";
 ?>
 --EXPECTF--
-string(28) "42/-1337/4089650035136921599"
+string(63) "42/-1337/4089650035136921599/1000000000000000000000000000000000"
 
 Done