Skip to content

Commit e72fc12

Browse files
ndosschendossche
committed
Fix GH-10008: Narrowing occurred during type inference of ZEND_ADD_ARRAY_ELEMENT
This test triggers narrowing for two ops: first ZEND_ADD_ARRAY_ELEMENT, and then ZEND_ASSIGN. The type inference happens in the following order: 1) The ZEND_ADD_ARRAY_ELEMENT infers type 0x40e04080 (packed flag is set), arr_type=0 at this point because it hasn't been set by ZEND_INIT_ARRAY yet. 2) The ZEND_INIT_ARRAY infers type 0x40804080 3) The ZEND_ADD_ARRAY_ELEMENT infers type 0x40e04080, arr_type=0x40804080, which does not have the packed flag set while the existing result of ZEND_ADD_ARRAY_ELEMENT has the packed flag set. This seems to occur because of the phi node introduced by the while loop. If I remove the loop the problem goes away. As Arnaud noted, this seems to be caused by a too wide type inference for arr_type==0. We should keep the invariant that if x>=y then key_type(x) >= key_type(y). If we write the possible results down in a table we get: ``` arr_type resulting key type --------------- -------------------------------------------------------------------------- HASH_ONLY -> MAY_BE_ARRAY_NUMERIC_HASH PACKED_ONLY -> MAY_BE_ARRAY_NUMERIC_HASH | MAY_BE_ARRAY_PACKED (== MAY_BE_ARRAY_KEY_LONG) HASH || PACKED -> MAY_BE_ARRAY_NUMERIC_HASH | MAY_BE_ARRAY_PACKED (== MAY_BE_ARRAY_KEY_LONG) 0 -> MAY_BE_ARRAY_NUMERIC_HASH | MAY_BE_ARRAY_PACKED (== MAY_BE_ARRAY_KEY_LONG) ``` As we can see, `HASH_ONLY > 0` but `MAY_BE_ARRAY_NUMERIC_HASH < MAY_BE_ARRAY_NUMERIC_HASH | MAY_BE_ARRAY_PACKED`, which violates the invariant. Instead if we modify the zero case to have MAY_BE_ARRAY_NUMERIC_HASH instead, we get the following table which satisfies the invariant. ``` arr_type resulting key type --------------- -------------------------------------------------------------------------- HASH_ONLY -> MAY_BE_ARRAY_NUMERIC_HASH PACKED_ONLY -> MAY_BE_ARRAY_NUMERIC_HASH | MAY_BE_ARRAY_PACKED (== MAY_BE_ARRAY_KEY_LONG) HASH || PACKED -> MAY_BE_ARRAY_NUMERIC_HASH | MAY_BE_ARRAY_PACKED (== MAY_BE_ARRAY_KEY_LONG) 0 -> MAY_BE_ARRAY_NUMERIC_HASH ``` Broke in 1ffbb73. Closes GH-10294.
1 parent 2a7f23e commit e72fc12

File tree

3 files changed

+53
-9
lines changed

3 files changed

+53
-9
lines changed

‎NEWS‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ PHP NEWS
88
. Fixed bug GH-12215 (Module entry being overwritten causes type errors in
99
ext/dom). (nielsdos)
1010
. Fixed bug GH-12273 (__builtin_cpu_init check). (Freaky)
11+
. Fixed bug GH-10008 (Narrowing occurred during type inference of
12+
ZEND_ADD_ARRAY_ELEMENT). (nielsdos, arnaud-lb)
1113

1214
- CType:
1315
. Fixed bug GH-11997 (ctype_alnum 5 times slower in PHP 8.1 or greater).

‎Zend/Optimizer/zend_inference.c‎

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1926,6 +1926,21 @@ ZEND_API uint32_t zend_array_element_type(uint32_t t1, zend_uchar op_type, int w
19261926
return tmp;
19271927
}
19281928

1929+
static zend_always_inline uint32_t assign_long_dim_array_result_type(uint32_t arr_type)
1930+
{
1931+
/* Rules:
1932+
* HASH_ONLY -> MAY_BE_ARRAY_NUMERIC_HASH
1933+
* PACKED_ONLY -> MAY_BE_ARRAY_NUMERIC_HASH | MAY_BE_ARRAY_PACKED (== MAY_BE_ARRAY_KEY_LONG)
1934+
* HASH || PACKED -> MAY_BE_ARRAY_NUMERIC_HASH | MAY_BE_ARRAY_PACKED (== MAY_BE_ARRAY_KEY_LONG)
1935+
* 0 -> MAY_BE_ARRAY_NUMERIC_HASH
1936+
*/
1937+
if (MAY_BE_PACKED(arr_type)) {
1938+
return MAY_BE_ARRAY_KEY_LONG;
1939+
} else {
1940+
return MAY_BE_ARRAY_NUMERIC_HASH;
1941+
}
1942+
}
1943+
19291944
static uint32_t assign_dim_array_result_type(
19301945
uint32_t arr_type, uint32_t dim_type, uint32_t value_type, zend_uchar dim_op_type) {
19311946
uint32_t tmp = 0;
@@ -1939,13 +1954,13 @@ static uint32_t assign_dim_array_result_type(
19391954
if (arr_type & (MAY_BE_UNDEF|MAY_BE_NULL|MAY_BE_FALSE)) {
19401955
tmp |= MAY_BE_ARRAY_PACKED;
19411956
}
1942-
tmp |= MAY_BE_HASH_ONLY(arr_type) ? MAY_BE_ARRAY_NUMERIC_HASH : MAY_BE_ARRAY_KEY_LONG;
1957+
tmp |= assign_long_dim_array_result_type(arr_type);
19431958
} else {
19441959
if (dim_type & (MAY_BE_LONG|MAY_BE_FALSE|MAY_BE_TRUE|MAY_BE_RESOURCE|MAY_BE_DOUBLE)) {
19451960
if (arr_type & (MAY_BE_UNDEF|MAY_BE_NULL|MAY_BE_FALSE)) {
19461961
tmp |= MAY_BE_ARRAY_PACKED;
19471962
}
1948-
tmp |= MAY_BE_HASH_ONLY(arr_type) ? MAY_BE_ARRAY_NUMERIC_HASH : MAY_BE_ARRAY_KEY_LONG;
1963+
tmp |= assign_long_dim_array_result_type(arr_type);
19491964
}
19501965
if (dim_type & MAY_BE_STRING) {
19511966
tmp |= MAY_BE_ARRAY_KEY_STRING;
@@ -1954,7 +1969,7 @@ static uint32_t assign_dim_array_result_type(
19541969
if (arr_type & (MAY_BE_UNDEF|MAY_BE_NULL|MAY_BE_FALSE)) {
19551970
tmp |= MAY_BE_ARRAY_PACKED;
19561971
}
1957-
tmp |= MAY_BE_HASH_ONLY(arr_type) ? MAY_BE_ARRAY_NUMERIC_HASH : MAY_BE_ARRAY_KEY_LONG;
1972+
tmp |= assign_long_dim_array_result_type(arr_type);
19581973
}
19591974
}
19601975
if (dim_type & (MAY_BE_UNDEF|MAY_BE_NULL)) {
@@ -3254,17 +3269,15 @@ static zend_always_inline int _zend_update_type_info(
32543269
key_type |= MAY_BE_ARRAY_PACKED;
32553270
}
32563271
if (t1 & MAY_BE_ARRAY) {
3257-
key_type |= MAY_BE_HASH_ONLY(t1) ?
3258-
MAY_BE_ARRAY_NUMERIC_HASH : MAY_BE_ARRAY_KEY_LONG;
3272+
key_type |= assign_long_dim_array_result_type(t1);
32593273
}
32603274
} else {
32613275
if (t2 & (MAY_BE_LONG|MAY_BE_FALSE|MAY_BE_TRUE|MAY_BE_RESOURCE|MAY_BE_DOUBLE)) {
32623276
if (t1 & (MAY_BE_UNDEF|MAY_BE_NULL|MAY_BE_FALSE)) {
32633277
key_type |= MAY_BE_ARRAY_PACKED;
32643278
}
32653279
if (t1 & MAY_BE_ARRAY) {
3266-
key_type |= MAY_BE_HASH_ONLY(t1) ?
3267-
MAY_BE_ARRAY_NUMERIC_HASH : MAY_BE_ARRAY_KEY_LONG;
3280+
key_type |= assign_long_dim_array_result_type(t1);
32683281
}
32693282
}
32703283
if (t2 & MAY_BE_STRING) {
@@ -3275,8 +3288,7 @@ static zend_always_inline int _zend_update_type_info(
32753288
key_type |= MAY_BE_ARRAY_PACKED;
32763289
}
32773290
if (t1 & MAY_BE_ARRAY) {
3278-
key_type |= MAY_BE_HASH_ONLY(t1) ?
3279-
MAY_BE_ARRAY_NUMERIC_HASH : MAY_BE_ARRAY_KEY_LONG;
3291+
key_type |= assign_long_dim_array_result_type(t1);
32803292
}
32813293
}
32823294
}

‎ext/opcache/tests/opt/gh10008.phpt‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
--TEST--
2+
GH-10008 (Narrowing occurred during type inference of ZEND_ADD_ARRAY_ELEMENT)
3+
--INI--
4+
opcache.enable=1
5+
opcache.enable_cli=1
6+
opcache.optimization_level=0x20
7+
--EXTENSIONS--
8+
opcache
9+
--FILE--
10+
<?php
11+
12+
function test()
13+
{
14+
$bool = true;
15+
for ($i = 0; $i < 10; $i++) {
16+
if ($bool !== true) {
17+
$string_key = "a";
18+
// The following line triggers narrowing during type inference of ZEND_ADD_ARRAY_ELEMENT.
19+
$array = ["b" => $bool, $string_key => 123];
20+
}
21+
22+
$bool = false;
23+
}
24+
}
25+
26+
echo "Done\n";
27+
28+
?>
29+
--EXPECT--
30+
Done

0 commit comments

Comments
 (0)