Fix zend_optimizer_replace_by_const() leaving stale OP2 uses behind - #23546
Open
Mrmaxmeier wants to merge 1 commit into
Open
Fix zend_optimizer_replace_by_const() leaving stale OP2 uses behind#23546Mrmaxmeier wants to merge 1 commit into
Mrmaxmeier wants to merge 1 commit into
Conversation
For opcodes that do not consume their OP1 operand, the constant is propagated
into every use of the temporary rather than just the first one. That loop only
ever looked at OP1, so a use of the same temporary as an OP2 operand was left
referring to a temporary that no longer has a definition:
function test($v) {
switch ($v) {
case [$x] = (int)1.5:
break;
}
}
The ZEND_CAST is folded and rewritten into the ZEND_FETCH_LIST_R of the list
assignment, but the comparison emitted for the case expression uses the same
temporary as its OP2 and keeps the dangling reference, which makes SSA
construction fail its integrity check.
An OP2 operand is always consumed (ZEND_FE_FETCH_R/RW, where OP2 is a def
rather than a use, is the sole exception), so such a use is the last one and
the loop can stop there.
Assisted-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hi,
we ran into a failing SSA integrity check with the
fuzzer-function-jitfuzzing target:Assertion backtrace for reproducer
For opcodes that do not consume their OP1 operand (
ZEND_FETCH_LIST_Rand friends), the constant is propagated into every use of the temporary rather than just the first one, because the value stays alive past the first reader. That loop only ever looked at OP1 though, so a use of the same temporary as an OP2 operand was skipped and left referring to a temporary that no longer has a definition.In the reproducer the
ZEND_CASTis folded and rewritten into theZEND_FETCH_LIST_Rof the list assignment, but the comparison emitted for thecaseexpression uses the same temporary as its OP2. That use keeps the dangling reference, and SSA construction then fails its integrity check.The fix handles the OP2 case in the same loop. An OP2 operand is always consumed, so such a use is necessarily the last one and the loop can stop there.
ZEND_FE_FETCH_R/ZEND_FE_FETCH_RWare the sole exception, where OP2 is a def rather than a use, so they are excluded.Thanks!
Found by the CISPA Fandango team while triaging findings in oss-fuzz harnesses.