Skip to content

Fix GH-17140 (Assertion failure in JIT trace exit with ZEND_FETCH_DIM_FUNC_ARG) - #17148

Closed
ndossche wants to merge 2 commits into
php:PHP-8.4from
ndossche:fix-GH-17140
Closed

ndossche wants to merge 2 commits into
php:PHP-8.4from
ndossche:fix-GH-17140

Conversation

@ndossche

@ndossche ndossche commented Dec 13, 2024 •

Copy link
Copy Markdown
Member

ZEND_FETCH_DIM_FUNC_ARG should also be repeated on undefined access, consistent to how ZEND_FETCH_DIM_R is handled. The opcode was just missing from the assertion list.
This doesn't reproduce on 8.3 because there it always re-executes the last opline and doesn't have an assertion.

…DIM_FUNC_ARG)

ZEND_FETCH_DIM_FUNC_ARG should also be repeated on undefined access,
consistent to how ZEND_FETCH_DIM_R is handled. The opcode was just
missing from the assertion list.
@ndossche ndossche linked an issue Dec 13, 2024 that may be closed by this pull request
@ndossche
ndossche marked this pull request as ready for review December 13, 2024 17:53
@ndossche
ndossche requested a review from dstogov as a code owner December 13, 2024 17:53
Comment thread ext/opcache/jit/zend_jit_trace.c Outdated
ZVAL_NULL(EX_VAR_NUM(i));
} else {
assert(op->opcode == ZEND_FETCH_DIM_R || op->opcode == ZEND_FETCH_LIST_R || op->opcode == ZEND_FETCH_OBJ_R);
assert(op->opcode == ZEND_FETCH_DIM_R || op->opcode == ZEND_FETCH_LIST_R || op->opcode == ZEND_FETCH_OBJ_R || op->opcode == ZEND_FETCH_DIM_FUNC_ARG);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ZEND_FETCH_OBJ_FUNC_ARG should be add as well.

<?php
namespace Foo;
class X {
        public $a = 1;
        public $b;
        function __construct() {
                unset($this->b);
        }
}
function test() {
    $a['x'] = new X;
    for ($fusion = 0; $i < 3; $i++) {
        var_dump($a['x']->b);
    }
}
test();
?>

Alsoassert() may be changed to ZEND_ASSERT().

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Right, makes sense, thanks

Co-authored-by: Dmitry Stogov <[email protected]>
@ndossche ndossche closed this in 2104097 Dec 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Assertion failure in JIT trace exit with ZEND_FETCH_DIM_FUNC_ARG

2 participants