Skip to content

Fixed HASH/PACKED array inference through MAY_BE_ARRAY_EMPTY flag #12591

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 2 commits into from
Nov 2, 2023
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
4 changes: 2 additions & 2 deletions Zend/Optimizer/ssa_integrity.c
Original file line number Diff line number Diff line change
Expand Up @@ -170,10 +170,10 @@ void ssa_verify_integrity(zend_op_array *op_array, zend_ssa *ssa, const char *ex
}
} FOREACH_PHI_USE_END();

if ((type & MAY_BE_ARRAY_KEY_ANY) && !(type & MAY_BE_ARRAY_OF_ANY)) {
if ((type & (MAY_BE_ARRAY_KEY_ANY-MAY_BE_ARRAY_EMPTY)) && !(type & MAY_BE_ARRAY_OF_ANY)) {
FAIL("var " VARFMT " has array key type but not value type\n", VAR(i));
}
if ((type & MAY_BE_ARRAY_OF_ANY) && !(type & MAY_BE_ARRAY_KEY_ANY)) {
if ((type & MAY_BE_ARRAY_OF_ANY) && !(type & (MAY_BE_ARRAY_KEY_ANY-MAY_BE_ARRAY_EMPTY))) {
FAIL("var " VARFMT " has array value type but not key type\n", VAR(i));
}
if ((type & MAY_BE_REF) && ssa->var_info[i].ce) {
Expand Down
33 changes: 23 additions & 10 deletions Zend/Optimizer/zend_dump.c
Original file line number Diff line number Diff line change
Expand Up @@ -244,21 +244,34 @@ static void zend_dump_type_info(uint32_t info, zend_class_entry *ce, int is_inst
}
if (info & MAY_BE_ARRAY) {
if (first) first = 0; else fprintf(stderr, ", ");
if (!(info & MAY_BE_ARRAY_KEY_STRING) || (info & MAY_BE_PACKED_GUARD)) {
if (MAY_BE_PACKED_ONLY(info)) {
if (info & MAY_BE_PACKED_GUARD) {
fprintf(stderr, "!");
}
if (info & MAY_BE_PACKED_GUARD) {
fprintf(stderr, "!");
}
if (MAY_BE_EMPTY_ONLY(info)) {
fprintf(stderr, "empty ");
} else if (MAY_BE_PACKED_ONLY(info)) {
fprintf(stderr, "packed ");
} else if (MAY_BE_HASH_ONLY(info)) {
fprintf(stderr, "hash ");
} else if ((info & MAY_BE_ARRAY_KEY_ANY) != MAY_BE_ARRAY_KEY_ANY && (info & MAY_BE_ARRAY_KEY_ANY) != 0) {
bool afirst = 1;
fprintf(stderr, "[");
if (info & MAY_BE_ARRAY_EMPTY) {
if (afirst) afirst = 0; else fprintf(stderr, ", ");
fprintf(stderr, "empty ");
}
if (MAY_BE_PACKED(info)) {
if (afirst) afirst = 0; else fprintf(stderr, ", ");
fprintf(stderr, "packed ");
} else if (MAY_BE_HASH_ONLY(info)) {
if (info & MAY_BE_PACKED_GUARD) {
fprintf(stderr, "!");
}
}
if (MAY_BE_HASH(info)) {
if (afirst) afirst = 0; else fprintf(stderr, ", ");
fprintf(stderr, "hash ");
}
fprintf(stderr, "] ");
}
fprintf(stderr, "array");
if ((info & MAY_BE_ARRAY_KEY_ANY) != 0 &&
if ((info & (MAY_BE_ARRAY_KEY_LONG|MAY_BE_ARRAY_KEY_STRING)) != 0 &&
((info & MAY_BE_ARRAY_KEY_LONG) == 0 ||
(info & MAY_BE_ARRAY_KEY_STRING) == 0)) {
bool afirst = 1;
Expand Down
11 changes: 8 additions & 3 deletions Zend/Optimizer/zend_func_info.c
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ static uint32_t zend_range_info(const zend_call_info *call_info, const zend_ssa
uint32_t t2 = _ssa_op1_info(op_array, ssa, call_info->arg_info[1].opline,
&ssa->ops[call_info->arg_info[1].opline - op_array->opcodes]);
uint32_t t3 = 0;
uint32_t tmp = MAY_BE_RC1 | MAY_BE_ARRAY;
uint32_t tmp = MAY_BE_RC1 | MAY_BE_ARRAY | MAY_BE_ARRAY_EMPTY;

if (call_info->num_args == 3) {
t3 = _ssa_op1_info(op_array, ssa, call_info->arg_info[2].opline,
Expand All @@ -87,7 +87,7 @@ static uint32_t zend_range_info(const zend_call_info *call_info, const zend_ssa
return tmp;
} else {
/* May throw */
return MAY_BE_RC1 | MAY_BE_ARRAY | MAY_BE_ARRAY_PACKED | MAY_BE_ARRAY_OF_LONG | MAY_BE_ARRAY_OF_DOUBLE | MAY_BE_ARRAY_OF_STRING;
return MAY_BE_RC1 | MAY_BE_ARRAY | MAY_BE_ARRAY_EMPTY | MAY_BE_ARRAY_PACKED | MAY_BE_ARRAY_OF_LONG | MAY_BE_ARRAY_OF_DOUBLE | MAY_BE_ARRAY_OF_STRING;
}
}

Expand Down Expand Up @@ -120,7 +120,12 @@ uint32_t zend_get_internal_func_info(
if (info->info_func) {
return call_info ? info->info_func(call_info, ssa) : 0;
} else {
return info->info;
uint32_t ret = info->info;

if (ret & MAY_BE_ARRAY) {
ret |= MAY_BE_ARRAY_EMPTY;
}
return ret;
}
}

Expand Down
32 changes: 20 additions & 12 deletions Zend/Optimizer/zend_inference.c
Original file line number Diff line number Diff line change
Expand Up @@ -2105,18 +2105,22 @@ ZEND_API uint32_t ZEND_FASTCALL zend_array_type_info(const zval *zv)
tmp |= MAY_BE_RCN;
}

ZEND_HASH_FOREACH_STR_KEY_VAL(ht, str, val) {
if (str) {
tmp |= MAY_BE_ARRAY_KEY_STRING;
} else {
tmp |= MAY_BE_ARRAY_KEY_LONG;
}
tmp |= 1 << (Z_TYPE_P(val) + MAY_BE_ARRAY_SHIFT);
} ZEND_HASH_FOREACH_END();
if (HT_IS_PACKED(ht)) {
tmp &= ~(MAY_BE_ARRAY_NUMERIC_HASH|MAY_BE_ARRAY_STRING_HASH);
if (zend_hash_num_elements(ht) == 0) {
tmp |= MAY_BE_ARRAY_EMPTY;
} else if (HT_IS_PACKED(ht)) {
tmp |= MAY_BE_ARRAY_PACKED;
ZEND_HASH_PACKED_FOREACH_VAL(ht, val) {
tmp |= 1 << (Z_TYPE_P(val) + MAY_BE_ARRAY_SHIFT);
} ZEND_HASH_FOREACH_END();
} else {
tmp &= ~MAY_BE_ARRAY_PACKED;
ZEND_HASH_MAP_FOREACH_STR_KEY_VAL(ht, str, val) {
if (str) {
tmp |= MAY_BE_ARRAY_STRING_HASH;
} else {
tmp |= MAY_BE_ARRAY_NUMERIC_HASH;
}
tmp |= 1 << (Z_TYPE_P(val) + MAY_BE_ARRAY_SHIFT);
} ZEND_HASH_FOREACH_END();
}
return tmp;
}
Expand Down Expand Up @@ -2224,6 +2228,7 @@ static uint32_t assign_dim_array_result_type(
if (tmp & MAY_BE_ARRAY_KEY_ANY) {
tmp |= (value_type & MAY_BE_ANY) << MAY_BE_ARRAY_SHIFT;
}
tmp &= ~MAY_BE_ARRAY_EMPTY;
return tmp;
}

Expand Down Expand Up @@ -3661,6 +3666,9 @@ static zend_always_inline zend_result _zend_update_type_info(
break;
}
}
if (opline->opcode != ZEND_FETCH_DIM_FUNC_ARG) {
tmp &= ~MAY_BE_ARRAY_EMPTY;
}
}
if (((tmp & MAY_BE_ARRAY) && (tmp & MAY_BE_ARRAY_KEY_ANY))
|| opline->opcode == ZEND_FETCH_DIM_FUNC_ARG
Expand Down Expand Up @@ -3828,7 +3836,7 @@ static zend_always_inline zend_result _zend_update_type_info(
UPDATE_SSA_TYPE(MAY_BE_LONG, ssa_op->result_def);
break;
case ZEND_FUNC_GET_ARGS:
UPDATE_SSA_TYPE(MAY_BE_RC1|MAY_BE_RCN| MAY_BE_ARRAY | MAY_BE_ARRAY_PACKED | MAY_BE_ARRAY_OF_ANY, ssa_op->result_def);
UPDATE_SSA_TYPE(MAY_BE_RC1|MAY_BE_RCN|MAY_BE_ARRAY|MAY_BE_ARRAY_EMPTY|MAY_BE_ARRAY_PACKED|MAY_BE_ARRAY_OF_ANY, ssa_op->result_def);
break;
case ZEND_GET_CLASS:
case ZEND_GET_CALLED_CLASS:
Expand Down
1 change: 0 additions & 1 deletion Zend/Optimizer/zend_inference.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
#define MAY_BE_PACKED_GUARD (1<<27) /* needs packed array guard */
#define MAY_BE_CLASS_GUARD (1<<27) /* needs class guard */
#define MAY_BE_GUARD (1<<28) /* needs type guard */
//#define MAY_BE_IN_REG (1<<29) /* deprecated and not used */

#define MAY_HAVE_DTOR \
(MAY_BE_OBJECT|MAY_BE_RESOURCE \
Expand Down
8 changes: 5 additions & 3 deletions Zend/zend_type_info.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,15 +59,17 @@
#define MAY_BE_ARRAY_PACKED (1<<21)
#define MAY_BE_ARRAY_NUMERIC_HASH (1<<22) /* hash with numeric keys */
#define MAY_BE_ARRAY_STRING_HASH (1<<23) /* hash with string keys */
#define MAY_BE_ARRAY_EMPTY (1<<29)

#define MAY_BE_ARRAY_KEY_LONG (MAY_BE_ARRAY_PACKED | MAY_BE_ARRAY_NUMERIC_HASH)
#define MAY_BE_ARRAY_KEY_STRING MAY_BE_ARRAY_STRING_HASH
#define MAY_BE_ARRAY_KEY_ANY (MAY_BE_ARRAY_KEY_LONG | MAY_BE_ARRAY_KEY_STRING)
#define MAY_BE_ARRAY_KEY_ANY (MAY_BE_ARRAY_KEY_LONG | MAY_BE_ARRAY_KEY_STRING | MAY_BE_ARRAY_EMPTY)

#define MAY_BE_PACKED(t) ((t) & MAY_BE_ARRAY_PACKED)
#define MAY_BE_HASH(t) ((t) & (MAY_BE_ARRAY_NUMERIC_HASH | MAY_BE_ARRAY_KEY_STRING))
#define MAY_BE_PACKED_ONLY(t) (MAY_BE_PACKED(t) && !MAY_BE_HASH(t))
#define MAY_BE_HASH_ONLY(t) (MAY_BE_HASH(t) && !MAY_BE_PACKED(t))
#define MAY_BE_PACKED_ONLY(t) (((t) & MAY_BE_ARRAY_KEY_ANY) == MAY_BE_ARRAY_PACKED)
#define MAY_BE_HASH_ONLY(t) (MAY_BE_HASH(t) && !((t) & (MAY_BE_ARRAY_PACKED|MAY_BE_ARRAY_EMPTY)))
#define MAY_BE_EMPTY_ONLY(t) (((t) & MAY_BE_ARRAY_KEY_ANY) == MAY_BE_ARRAY_EMPTY)

#define MAY_BE_CLASS (1<<24)
#define MAY_BE_INDIRECT (1<<25)
Expand Down
28 changes: 28 additions & 0 deletions ext/opcache/tests/jit/gh12527.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
--TEST--
GH-12527: Incorrect hash/packed inference
--INI--
opcache.enable=1
opcache.enable_cli=1
--FILE--
<?php
function foo(array $a1) {
$a2 = [];
foreach ($a1 as $key => $val) {
if (!$val) {
$a2["bad"][] = $key;
} else {
$a2[0][] = $key;
}
}
foreach ($a2 as $key => $val) {
var_dump($key);
}
}
foo([1, 2, 3]);
foo([1, 2, 3]);
foo([0, 0]);
?>
--EXPECT--
int(0)
int(0)
string(3) "bad"