Skip to content

zend_compile: Add support for %d to sprintf() optimization #14561

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jun 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 37 additions & 30 deletions Zend/zend_compile.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -4757,21 +4757,22 @@ static zend_result zend_compile_func_sprintf(znode *result, zend_ast_list *args)
}

switch (*q) {
case 's':
string_placeholder_count++;
break;
case '%':
break;
default:
return FAILURE;
case 's':
case 'd':
placeholder_count++;
break;
case '%':
break;
default:
return FAILURE;
}

p = q;
p++;
}

/* 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;
}

Expand All @@ -4785,27 +4786,22 @@ 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) {
convert_to_string(&elements[i].u.constant);
}
}
}

uint32_t rope_elements = 0;
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;
Expand All @@ -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 '%':
Expand All @@ -4837,21 +4833,32 @@ 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[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 (*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();
}
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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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--
Expand Down Expand Up @@ -173,4 +177,6 @@ Stack trace:
#0 %s(97): sprintf()
#1 {main}

string(4) "1--1"

Done
134 changes: 134 additions & 0 deletions ext/standard/tests/strings/sprintf_rope_optimization_003.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
--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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Memo to myself, it might make sense to change the sprintf family of functions to emit the deprecation about implicit truncation for floats.

$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 {
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 {
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(24) "2147483647/0/-2147483648"

string(5) "1/0/1"

string(3) "1/0"

string(1) "0"

Done
21 changes: 21 additions & 0 deletions ext/standard/tests/strings/sprintf_rope_optimization_004.phpt
Original file line number Diff line number Diff line change
@@ -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/%s", $a, $b, $c, $c + 1));
} catch (\Throwable $e) {echo $e, PHP_EOL; } echo PHP_EOL;

echo "Done";
?>
--EXPECTF--
string(63) "42/-1337/4089650035136921599/1000000000000000000000000000000000"

Done
Loading