Fix block pass dropping ZEND_FAST_CALL OP2 definition - #23508
Open
Mrmaxmeier wants to merge 1 commit into
Open
Conversation
For a return from inside a try block that has a finally, ZEND_FAST_CALL names
the temporary holding the pending return value in its OP2. That operand is a
real use: zend_dispatch_try_catch_finally_helper() destroys the value through
it when the finally block throws, and the RETURN behind the FAST_CALL consumes
it afterwards.
The block pass tracked only a single source per temporary and treated the
RETURN as its sole use, so constant propagation removed the QM_ASSIGN that
defines it:
$x = 1;
try {
try {
return true ? 1.5 : 2.5;
} finally {
undef_fn();
}
} catch (Error $e) {
echo $e->getMessage(), "\n";
}
The optimized op_array kept "FAST_CALL finally T3" with T3 no longer defined
anywhere, and unwinding through the finally destroyed an uninitialized
temporary.
Clear the recorded source for the OP2 operand of ZEND_FAST_CALL, the same way
the pass already does for the operands that ZEND_FETCH_LIST_R and friends keep
alive.
Assisted-By: Claude Opus 5 <noreply@anthropic.com>
NickSdot
reviewed
Aug 29, 2026
Comment on lines
+22
to
+48
| } catch (Error $e) { | ||
| echo $e->getMessage(), "\n"; | ||
| } | ||
| return "fallback"; | ||
| } | ||
|
|
||
| function test_refcounted() { | ||
| $x = 1; | ||
| try { | ||
| try { | ||
| return true ? "returned" : "other"; | ||
| } finally { | ||
| undef_fn(); | ||
| } | ||
| } catch (Error $e) { | ||
| echo $e->getMessage(), "\n"; | ||
| } | ||
| return "fallback"; | ||
| } | ||
|
|
||
| var_dump(test_scalar()); | ||
| var_dump(test_refcounted()); | ||
| ?> | ||
| --EXPECT-- | ||
| Call to undefined function undef_fn() | ||
| string(8) "fallback" | ||
| Call to undefined function undef_fn() |
Contributor
There was a problem hiding this comment.
Suggested change
| } catch (Error $e) { | |
| echo $e->getMessage(), "\n"; | |
| } | |
| return "fallback"; | |
| } | |
| function test_refcounted() { | |
| $x = 1; | |
| try { | |
| try { | |
| return true ? "returned" : "other"; | |
| } finally { | |
| undef_fn(); | |
| } | |
| } catch (Error $e) { | |
| echo $e->getMessage(), "\n"; | |
| } | |
| return "fallback"; | |
| } | |
| var_dump(test_scalar()); | |
| var_dump(test_refcounted()); | |
| ?> | |
| --EXPECT-- | |
| Call to undefined function undef_fn() | |
| string(8) "fallback" | |
| Call to undefined function undef_fn() | |
| } catch (Throwable $e) { | |
| echo $e::class, ': ', $e->getMessage(), "\n"; | |
| } | |
| return "fallback"; | |
| } | |
| function test_refcounted() { | |
| $x = 1; | |
| try { | |
| try { | |
| return true ? "returned" : "other"; | |
| } finally { | |
| undef_fn(); | |
| } | |
| } catch (Throwable $e) { | |
| echo $e::class, ': ', $e->getMessage(), "\n"; | |
| } | |
| return "fallback"; | |
| } | |
| var_dump(test_scalar()); | |
| var_dump(test_refcounted()); | |
| ?> | |
| --EXPECT-- | |
| Error: Call to undefined function undef_fn() | |
| string(8) "fallback" | |
| Error: Call to undefined function undef_fn() |
Ref: https://github.com/php/php-src/blob/master/CODING_STANDARDS.md#testing
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
2 participants
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 use of an uninitialized value bug with the
fuzzer-function-jitfuzzing target:Note: MSAN would be the right tool to detect this bug, but oss-fuzz flags this harness as ASAN-only because the JIT doesn't work with MSAN.
ASAN backtrace for reproducer
When a
returncrosses afinallyblock, PHP first stores the return value in a temporary. Both theZEND_FAST_CALLthat enters thefinallyblock and the laterZEND_RETURNrefer to that temporary. The OP2 reference onZEND_FAST_CALLis needed during exception unwinding: if thefinallyblock throws, the VM uses it to find and destroy the pending return value.The block pass did not account for this extra use. After propagating the value into
ZEND_RETURN, it considered theQM_ASSIGNthat initialized the temporary dead and removed it. This left the optimized op_array withbut no opcode that initialized
T3. If thefinallyblock then threw, exception unwinding attempted to destroy the uninitialized temporary.The fix marks the OP2 temporary as ineligible for this source-removal optimization. This uses the same mechanism already used for operands that must remain available to
ZEND_FETCH_LIST_Rand similar opcodes.Thanks!
Found by the CISPA Fandango team while triaging findings in oss-fuzz harnesses.