From: Alex Dowad <alexinbeijing@gmail.com>
Date: Thu, 29 Oct 2020 21:10:04 +0000 (+0200)
Subject: Convert numeric string array keys to integers correctly in JITted code
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=d44235acaeb4;p=php

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`.
---

diff --git a/Zend/tests/dim_assign_001.phpt b/Zend/tests/dim_assign_001.phpt
new file mode 100644
index 0000000000..86a0f19186
--- /dev/null
+++ b/Zend/tests/dim_assign_001.phpt
@@ -0,0 +1,26 @@
+--TEST--
+JIT - Assigning to arrays using string key which parses to an integer
+--SKIPIF--
+<?php require_once('skipif.inc'); ?>
+--FILE--
+<?php
+/* We are going to store a value in an array, using the key "1"
+ * PHP should always convert such strings to integers when storing or retrieving
+ * values from an array
+ *
+ * We'll do it in a loop, so that if JIT is enabled, the code will be JIT'd
+ * (Because this test was originally added as a regression test for a JIT bug)
+ *
+ * Also, the test will do this in a way which guarantees PHP won't be able to
+ * predict whether the (string) key will be a numeric string or not */
+$fp = fopen(realpath(__DIR__ . '/dim_assign_001.txt'), 'r+');
+$array = array();
+while ($line = fgets($fp, 256)) {
+  sscanf($line, '%x', $char);
+  $char = chr($char);
+  $array[$char] = "Values can be stored correctly using numeric string keys";
+}
+var_dump($array['1']);
+?>
+--EXPECT--
+string(56) "Values can be stored correctly using numeric string keys"
diff --git a/Zend/tests/dim_assign_001.txt b/Zend/tests/dim_assign_001.txt
new file mode 100644
index 0000000000..f14f5c0cbe
--- /dev/null
+++ b/Zend/tests/dim_assign_001.txt
@@ -0,0 +1,2 @@
+0x30
+0x31
diff --git a/ext/opcache/jit/zend_jit_helpers.c b/ext/opcache/jit/zend_jit_helpers.c
index 11912e3a70..5d5a30541b 100644
--- a/ext/opcache/jit/zend_jit_helpers.c
+++ b/ext/opcache/jit/zend_jit_helpers.c
@@ -437,7 +437,7 @@ static int ZEND_FASTCALL zend_jit_undefined_op_helper_write(HashTable *ht, uint3
 
 static void ZEND_FASTCALL zend_jit_fetch_dim_r_helper(zend_array *ht, zval *dim, zval *result)
 {
-	zend_long hval;
+	zend_ulong hval;
 	zend_string *offset_key;
 	zval *retval;
 
@@ -478,6 +478,9 @@ static void ZEND_FASTCALL zend_jit_fetch_dim_r_helper(zend_array *ht, zval *dim,
 	}
 
 str_index:
+	if (ZEND_HANDLE_NUMERIC(offset_key, hval)) {
+		goto num_index;
+	}
 	retval = zend_hash_find(ht, offset_key);
 	if (retval) {
 		/* support for $GLOBALS[...] */
@@ -509,7 +512,7 @@ num_undef:
 
 static void ZEND_FASTCALL zend_jit_fetch_dim_is_helper(zend_array *ht, zval *dim, zval *result)
 {
-	zend_long hval;
+	zend_ulong hval;
 	zend_string *offset_key;
 	zval *retval;
 
@@ -550,6 +553,9 @@ static void ZEND_FASTCALL zend_jit_fetch_dim_is_helper(zend_array *ht, zval *dim
 	}
 
 str_index:
+	if (ZEND_HANDLE_NUMERIC(offset_key, hval)) {
+		goto num_index;
+	}
 	retval = zend_hash_find(ht, offset_key);
 	if (retval) {
 		/* support for $GLOBALS[...] */
@@ -578,7 +584,7 @@ num_undef:
 
 static int ZEND_FASTCALL zend_jit_fetch_dim_isset_helper(zend_array *ht, zval *dim)
 {
-	zend_long hval;
+	zend_ulong hval;
 	zend_string *offset_key;
 	zval *retval;
 
@@ -618,6 +624,9 @@ static int ZEND_FASTCALL zend_jit_fetch_dim_isset_helper(zend_array *ht, zval *d
 	}
 
 str_index:
+	if (ZEND_HANDLE_NUMERIC(offset_key, hval)) {
+		goto num_index;
+	}
 	retval = zend_hash_find(ht, offset_key);
 	if (retval) {
 		/* support for $GLOBALS[...] */
@@ -645,7 +654,7 @@ num_undef:
 
 static zval* ZEND_FASTCALL zend_jit_fetch_dim_rw_helper(zend_array *ht, zval *dim)
 {
-	zend_long hval;
+	zend_ulong hval;
 	zend_string *offset_key;
 	zval *retval;
 
@@ -688,6 +697,9 @@ static zval* ZEND_FASTCALL zend_jit_fetch_dim_rw_helper(zend_array *ht, zval *di
 	}
 
 str_index:
+	if (ZEND_HANDLE_NUMERIC(offset_key, hval)) {
+		goto num_index;
+	}
 	retval = zend_hash_find(ht, offset_key);
 	if (retval) {
 		/* support for $GLOBALS[...] */
@@ -726,7 +738,7 @@ num_undef:
 
 static zval* ZEND_FASTCALL zend_jit_fetch_dim_w_helper(zend_array *ht, zval *dim)
 {
-	zend_long hval;
+	zend_ulong hval;
 	zend_string *offset_key;
 	zval *retval;
 
@@ -769,6 +781,9 @@ static zval* ZEND_FASTCALL zend_jit_fetch_dim_w_helper(zend_array *ht, zval *dim
 	}
 
 str_index:
+	if (ZEND_HANDLE_NUMERIC(offset_key, hval)) {
+		goto num_index;
+	}
 	retval = zend_hash_find(ht, offset_key);
 	if (retval) {
 		/* support for $GLOBALS[...] */