Skip to content
Open
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
2 changes: 2 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ PHP NEWS
function triggered by bailout in php_output_lock_error()). (timwolla)
. Fix OSS-Fuzz #471533782 (Infinite loop in GC destructor fiber). (ilutov)
. Fix OSS-Fuzz #472563272 (Borked block_pass JMP[N]Z optimization). (ilutov)
. Fix deprecation now showing when accessing null key of an array with JIT.
(alexandre-daubois)

- MbString:
. Fixed bug GH-20833 (mb_str_pad() divide by zero if padding string is
Expand Down
6 changes: 4 additions & 2 deletions Zend/Optimizer/sccp.c
Original file line number Diff line number Diff line change
Expand Up @@ -363,8 +363,7 @@ static inline zend_result zval_to_string_offset(zend_long *result, zval *op) {
static inline zend_result fetch_array_elem(zval **result, zval *op1, zval *op2) {
switch (Z_TYPE_P(op2)) {
case IS_NULL:
*result = zend_hash_find(Z_ARR_P(op1), ZSTR_EMPTY_ALLOC());
return SUCCESS;
return FAILURE;
case IS_FALSE:
*result = zend_hash_index_find(Z_ARR_P(op1), 0);
return SUCCESS;
Expand Down Expand Up @@ -428,6 +427,9 @@ static inline zend_result ct_eval_isset_isempty(zval *result, uint32_t extended_
}

static inline zend_result ct_eval_isset_dim(zval *result, uint32_t extended_value, zval *op1, zval *op2) {
if (Z_TYPE_P(op2) == IS_NULL) {
return FAILURE;
}
Copy link
Member

Choose a reason for hiding this comment

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

Is this still necessary? Shouldn't fetch_array_elem() already handle it?

Copy link
Member Author

Choose a reason for hiding this comment

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

I would say it is. If op1 is not an array (e.g. isset($a[null])), the condition below this one (if (Z_TYPE_P(op1) == IS_ARRAY || IS_PARTIAL_ARRAY(op1)) {) would be false and we would completely ditch any check on a null key.

Adding this early return avoids compile-time checks that would silence this kind of operation. If I get it right 😄

Copy link
Member

Choose a reason for hiding this comment

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

The deprecation was only added for arrays though, as far as I understand.

if (Z_TYPE_P(op1) == IS_ARRAY || IS_PARTIAL_ARRAY(op1)) {
zval *value;
if (fetch_array_elem(&value, op1, op2) == FAILURE) {
Expand Down
4 changes: 3 additions & 1 deletion ext/opcache/tests/jit/fetch_dim_r_001.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,16 @@ function foo() {
}
foo();
?>
--EXPECT--
--EXPECTF--
int(1)
int(3)
int(2)
int(1)
int(3)
int(1)
int(2)

Deprecated: Using null as an array offset is deprecated, use an empty string instead in %s on line %d
int(4)
int(5)
int(5)
Expand Down
Loading