Skip to content

Commit d44235a

Browse files
committed
Convert numeric string array keys to integers correctly in JITted code
While fixing bugs in mbstring, one of my new test cases failed with a strange error message stating: 'Warning: Undefined array key 1...', when clearly the array key had been set properly. GDB'd that sucker and found that JIT'd PHP code was calling directly into `zend_hash_add_new` (which was not converting the numeric string key to an integer properly). But where was that code coming from? I examined the disasm, looked up symbols to figure out where call instructions were going, then grepped the codebase for those function names. It soon became clear that the disasm I was looking at was compiled from `zend_jit_fetch_dim_w_helper`.
1 parent 904c1b6 commit d44235a

File tree

3 files changed

+48
-5
lines changed

3 files changed

+48
-5
lines changed

Zend/tests/dim_assign_001.phpt

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
--TEST--
2+
JIT - Assigning to arrays using string key which parses to an integer
3+
--SKIPIF--
4+
<?php require_once('skipif.inc'); ?>
5+
--FILE--
6+
<?php
7+
/* We are going to store a value in an array, using the key "1"
8+
* PHP should always convert such strings to integers when storing or retrieving
9+
* values from an array
10+
*
11+
* We'll do it in a loop, so that if JIT is enabled, the code will be JIT'd
12+
* (Because this test was originally added as a regression test for a JIT bug)
13+
*
14+
* Also, the test will do this in a way which guarantees PHP won't be able to
15+
* predict whether the (string) key will be a numeric string or not */
16+
$fp = fopen(realpath(__DIR__ . '/dim_assign_001.txt'), 'r+');
17+
$array = array();
18+
while ($line = fgets($fp, 256)) {
19+
sscanf($line, '%x', $char);
20+
$char = chr($char);
21+
$array[$char] = "Values can be stored correctly using numeric string keys";
22+
}
23+
var_dump($array['1']);
24+
?>
25+
--EXPECT--
26+
string(56) "Values can be stored correctly using numeric string keys"

Zend/tests/dim_assign_001.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
0x30
2+
0x31

ext/opcache/jit/zend_jit_helpers.c

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -437,7 +437,7 @@ static int ZEND_FASTCALL zend_jit_undefined_op_helper_write(HashTable *ht, uint3
437437

438438
static void ZEND_FASTCALL zend_jit_fetch_dim_r_helper(zend_array *ht, zval *dim, zval *result)
439439
{
440-
zend_long hval;
440+
zend_ulong hval;
441441
zend_string *offset_key;
442442
zval *retval;
443443

@@ -478,6 +478,9 @@ static void ZEND_FASTCALL zend_jit_fetch_dim_r_helper(zend_array *ht, zval *dim,
478478
}
479479

480480
str_index:
481+
if (ZEND_HANDLE_NUMERIC(offset_key, hval)) {
482+
goto num_index;
483+
}
481484
retval = zend_hash_find(ht, offset_key);
482485
if (retval) {
483486
/* support for $GLOBALS[...] */
@@ -509,7 +512,7 @@ static void ZEND_FASTCALL zend_jit_fetch_dim_r_helper(zend_array *ht, zval *dim,
509512

510513
static void ZEND_FASTCALL zend_jit_fetch_dim_is_helper(zend_array *ht, zval *dim, zval *result)
511514
{
512-
zend_long hval;
515+
zend_ulong hval;
513516
zend_string *offset_key;
514517
zval *retval;
515518

@@ -550,6 +553,9 @@ static void ZEND_FASTCALL zend_jit_fetch_dim_is_helper(zend_array *ht, zval *dim
550553
}
551554

552555
str_index:
556+
if (ZEND_HANDLE_NUMERIC(offset_key, hval)) {
557+
goto num_index;
558+
}
553559
retval = zend_hash_find(ht, offset_key);
554560
if (retval) {
555561
/* support for $GLOBALS[...] */
@@ -578,7 +584,7 @@ static void ZEND_FASTCALL zend_jit_fetch_dim_is_helper(zend_array *ht, zval *dim
578584

579585
static int ZEND_FASTCALL zend_jit_fetch_dim_isset_helper(zend_array *ht, zval *dim)
580586
{
581-
zend_long hval;
587+
zend_ulong hval;
582588
zend_string *offset_key;
583589
zval *retval;
584590

@@ -618,6 +624,9 @@ static int ZEND_FASTCALL zend_jit_fetch_dim_isset_helper(zend_array *ht, zval *d
618624
}
619625

620626
str_index:
627+
if (ZEND_HANDLE_NUMERIC(offset_key, hval)) {
628+
goto num_index;
629+
}
621630
retval = zend_hash_find(ht, offset_key);
622631
if (retval) {
623632
/* support for $GLOBALS[...] */
@@ -645,7 +654,7 @@ static int ZEND_FASTCALL zend_jit_fetch_dim_isset_helper(zend_array *ht, zval *d
645654

646655
static zval* ZEND_FASTCALL zend_jit_fetch_dim_rw_helper(zend_array *ht, zval *dim)
647656
{
648-
zend_long hval;
657+
zend_ulong hval;
649658
zend_string *offset_key;
650659
zval *retval;
651660

@@ -688,6 +697,9 @@ static zval* ZEND_FASTCALL zend_jit_fetch_dim_rw_helper(zend_array *ht, zval *di
688697
}
689698

690699
str_index:
700+
if (ZEND_HANDLE_NUMERIC(offset_key, hval)) {
701+
goto num_index;
702+
}
691703
retval = zend_hash_find(ht, offset_key);
692704
if (retval) {
693705
/* support for $GLOBALS[...] */
@@ -726,7 +738,7 @@ static zval* ZEND_FASTCALL zend_jit_fetch_dim_rw_helper(zend_array *ht, zval *di
726738

727739
static zval* ZEND_FASTCALL zend_jit_fetch_dim_w_helper(zend_array *ht, zval *dim)
728740
{
729-
zend_long hval;
741+
zend_ulong hval;
730742
zend_string *offset_key;
731743
zval *retval;
732744

@@ -769,6 +781,9 @@ static zval* ZEND_FASTCALL zend_jit_fetch_dim_w_helper(zend_array *ht, zval *dim
769781
}
770782

771783
str_index:
784+
if (ZEND_HANDLE_NUMERIC(offset_key, hval)) {
785+
goto num_index;
786+
}
772787
retval = zend_hash_find(ht, offset_key);
773788
if (retval) {
774789
/* support for $GLOBALS[...] */

0 commit comments

Comments
 (0)