Skip to content

Fix GH-24792: Assignment-result writes compile but fail with invalid opcode - #25046

Open
ChrisAbner wants to merge 1 commit into
php:PHP-8.6from
ChrisAbner:fix-gh-24792
Open

ChrisAbner wants to merge 1 commit into
php:PHP-8.6from
ChrisAbner:fix-gh-24792

Conversation

@ChrisAbner

Copy link
Copy Markdown

Fixes GH-24792.

Since the VAR|TMP overhaul (6173a9a), zend_compile_var_inner() compiles ZEND_AST_ASSIGN and ZEND_AST_ASSIGN_REF for every fetch type. When an assignment is the container of a write, such as ($a = [1])[0] = 2, unset(($a = [1])[0]) or $r = &($a = [1])[0], the compiler emits a write fetch on the TMP result of the assignment. That combination has no handler, so the script compiles and then dies with Invalid opcode. PHP 8.5 rejected all of these at compile time with "Cannot use temporary expression in write context". This patch restores that error.

The overhaul added these cases so that a destructuring assignment with references can be passed by reference (f([&$x] = $arr)). It originally had an assertion limiting them to that case, which a01a8e7 removed. The patch keeps compiling assignments when by_ref is set or when the fetch is not a write (W/RW/UNSET), and otherwise falls through to the existing default branch and its error. This puts the check in the one place every write container goes through. Checking in each caller (dim, prop, unset, inc/dec, reference fetch) would have needed one check per caller. As a side effect, ($a =& $b)[0] = 2 and ([&$x] = $arr)[0] = 5 are compile errors again. PHP 8.5 rejected them too; 8.6.0RC4 started executing them. If you would rather keep those working in 8.6, I can narrow the check.

Verification. Debug NTS build of PHP-8.6 (e2fa0c9, --disable-all --enable-cli), compared with the official php:8.5-cli (8.5.12) and php:8.6.0RC4-cli images:

Code 8.5.12 8.6.0RC4 PHP-8.6 + patch
($a = [1])[0] = 2; compile error Invalid opcode 23/2/1 compile error
($a = new stdClass)->p = 1; compile error Invalid opcode 24/2/1 compile error
($a = [1])[0] += 2; compile error Invalid opcode 27/2/1 compile error
($a = [1])[0]++; compile error Invalid opcode 87/2/1 compile error
unset(($a = [1])[0]); compile error Invalid opcode 75/2/1 compile error
$r = &($a = [1])[0]; compile error Invalid opcode 84/2/1 compile error
f(($a = [1])[0]); with f(&$x) compile error Invalid opcode 84/2/1 compile error
($a =& $b)[0] = 2; compile error runs compile error
([&$x] = $arr)[0] = 5; compile error runs compile error
f([&$x] = $arr); / f($a =& $b); with f(&$x) works works works
isset(($a = [1])[0]), ($a = [1])[5] ?? 'd', ($v = ['k' => 'v'])['k'] works works works

Out of 30 variants in total, all 30 match 8.5.12 with the patch, with and without OPcache + tracing JIT. Without the patch, 15 fail with Invalid opcode in 8.6.0RC4 and on master.

Tests Result
New Zend/tests/gh24792*.phpt (9) on 8.5.12 / 8.6.0RC4 / patched 9 pass / 8 fail / 9 pass
Zend/tests + tests, patched 6301 passed, 0 failed
Zend/tests + tests + ext/opcache/tests, patched, opcache.enable_cli=1, opcache.jit=tracing, opcache.protect_memory=1 7275 passed, 0 failed

Not covered: foreach (($a =& $b) as &$v) { $v = 3; } writes through to $b in 8.5 but not in 8.6.0RC4, with or without this patch. It is a separate behaviour change from the same overhaul and is left alone here. I only tested on Linux x86_64 NTS debug builds, not ZTS or ASAN builds of the patch. The bug itself also reproduces on the official ZTS RC4 image.

…id opcode

Since the VAR|TMP overhaul, zend_compile_var_inner() compiles assignment
expressions for any fetch type, so using an assignment as the container of
a write (e.g. ($a = [1])[0] = 2, unset(($a = [1])[0])) emitted a write
fetch on a TMP operand, which has no handler.

Only compile assignments in write context when they are passed by
reference, which is the case the overhaul needed (f([&$x] = $arr)).
Otherwise fall through to the "Cannot use temporary expression in write
context" compile error, as in PHP 8.5.
@anun333

anun333 commented Oct 30, 2026

Copy link
Copy Markdown

Quoted below: written with Claude, an LLM, as CONTRIBUTING.md asks us to mark. The builds and test runs behind it are on my machine; the commands to repeat them are under the fold.

Confirmed on today's PHP-8.6 (524e66be8d4) and master (1a72e5f2542), debug builds, each with this PR cherry-picked. I tried write contexts beyond the description's list too, and with the PR every one matches PHP-8.5 (c673cabdc36):

Code PHP-8.5 PHP-8.6 and master this PR, on both
($a = [1])[] = 2; compile error Invalid opcode 23/2/0 compile error
($a = [[1]])[0][1] = 2; compile error Invalid opcode 84/2/1 compile error
($a = new stdClass)->p->q = 1; compile error Invalid opcode 85/2/1 compile error
($a = ['s'])[0] .= 'x'; compile error Invalid opcode 27/2/1 compile error
($a = new stdClass)->p++; compile error Invalid opcode 132/2/1 compile error
($a = 'str')[0] = 'x'; compile error Invalid opcode 23/2/1 compile error
(($a = [1]))[0] = 2; compile error Invalid opcode 23/2/1 compile error
foreach ([1] as ($a = [1])[0]) {} compile error Invalid opcode 23/2/1 compile error
($a = [1])[0] ??= 2; compile error runs, no error compile error
function () { ($a = [1])[0] = 2; }; (never called) compile error compiles compile error
[($a = [1])[0]] = [5]; "Assignments can only happen to writable values" the same the same
($a .= 'x')[0] = 'y';, ($a ??= [1])[0] = 2; compile error compile error compile error
($a = [1])[0], ($a = new stdClass)->p ?? 'd' (reads) work work work

"compile error" is "Cannot use temporary expression in write context". The ??= row is the one today's PHP-8.6 doesn't fail on at all: it runs. Results are the same with opcache.enable_cli=1 opcache.jit=tracing, and on master the patch applies as is (NEWS aside), so the merge-up carries it. I also see the foreach (($a =& $b) as &$v) difference you mention: [3,3] on 8.5, [1,2] on 8.6 and master with or without the patch. Zend/tests passes on both (5,666; 5,675 with the nine new tests), and eight of the new tests fail on PHP-8.6 without the fix (the ninth checks what already works).

How this was tested

Linux x86_64, gcc 13.3, debug builds of origin/PHP-8.5, origin/PHP-8.6 and origin/master, and of 8.6 and master with this PR's commit cherry-picked:

./buildconf --force
./configure --enable-debug --disable-cgi --disable-phpdbg --disable-fileinfo --enable-pcntl --enable-zend-test --enable-fpm --with-curl --enable-mbstring --disable-mbregex --with-mysqli --with-pdo-mysql --with-openssl --with-bz2
make -j4

Each row is sapi/cli/php -n -r '<code> echo "ran\n";', first line of output kept, then again with -d opcache.enable_cli=1 -d opcache.jit=tracing -d opcache.jit_buffer_size=16M. Suites: TEST_PHP_EXECUTABLE=$PWD/sapi/cli/php sapi/cli/php run-tests.php -n -q -j4 Zend/tests.

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.

2 participants