Fix GH-21691: OPcache CFG optimizer eliminates QM_ASSIGN feeding JMPZ with VAR operand#21696
Fix GH-21691: OPcache CFG optimizer eliminates QM_ASSIGN feeding JMPZ with VAR operand#21696iliaal wants to merge 1 commit intophp:masterfrom
Conversation
|
Confirm it fixes running test for Drupal, used as patch and tests pass https://git.drupalcode.org/project/drupal/-/jobs/9303090 |
dstogov
left a comment
There was a problem hiding this comment.
The fix is right but incomplete.
The similar checks have to be added at least for ZEND_BOOL_NOT above and for ZEND_JMPN?Z_EX below.
I would add these IS_VAR checks into the conditions chains to avoid additional break. (but this is really not critical, and both decisions may have their advantages).
@iluuu1994 please also take a look. This is relate to ""VAR|TMP overhaul (GH-20628)"".
…MPZ_EX with IS_VAR The CFG optimizer (pass 5) substituted a QM_ASSIGN's source operand into its consumer without checking the source's operand type. When ASSIGN_REF produces IS_VAR and a QM_ASSIGN converts it to IS_TMP_VAR before JMPZ/JMPNZ or JMPZ_EX/JMPNZ_EX, eliminating the QM_ASSIGN leaves an IS_VAR operand on a consumer whose handler is declared CONST|TMP|CV, producing "Invalid opcode 43/4/0" (or 46/4/0 for the _EX variants). Guard both substitution sites with src->op1_type != IS_VAR, folded into the enclosing condition (per dstogov's review) so the BOOL_NOT branch is defensively covered as well. Test exercises both JMPZ and JMPZ_EX paths. Closes phpGH-21691
90f1c24 to
94bad2a
Compare
|
Confirmed.
All 880 opcache tests pass. @iluuu1994, worth a look at whether the broader invariant from #20628 warrants auditing other optimizer substitutions. |
Fixes #21691
The CFG optimizer (pass 5) removed a
QM_ASSIGNthat convertedIS_VARtoIS_TMP_VARbeforeJMPZ. SinceJMPZhas no handler forIS_VARoperands, this produced "Invalid opcode 43/4/0." The pattern occurs whenASSIGN_REF(which producesIS_VAR) feeds into a conditional viaQM_ASSIGN.Skips the
QM_ASSIGNelimination when the source operand isIS_VAR.