Repository navigation
Conversation
There was a problem hiding this comment.
I don't understand why you decrement and increment op_array->T back and force.
It may be modified only by two Optimizer passes - optimize_temp_vars_5.c and compact_vars.c.
Most probably these passes need to be updated to take OBSERVER into account.
There was a problem hiding this comment.
I agree. I wasn't really aware that only these two passes touch it - that makes much more sense then :-)
dstogov
left a comment
There was a problem hiding this comment.
Please try to remove all the op_array->T observer related modifications in Optimizer and add incrementation only in optimize_temp_vars_5.c and compact_vars.c
Instead of fixing up temporaries count in between observer steps, just take ZEND_ACC_DONE_PASS_TWO into account during stack_size calculation. Introducing zend_vm_calc_ct_used_stack for that use case. This should be much less susceptible to forgetting to handle the ZEND_OBSERVER_ENABLED temporary explicitly.
|
What's the process for having this merged in? |
|
@bwoebi are you going to merge this? |
|
Yes, sorry, forgot about this PR :-) |
|
Curious - will this also be merged into the 8.3 branch, or is that one unaffected? |
|
@scottbuscemi The fix landed in PHP 8.2 and 8.3 as well. Don't ask me why github doesn't display the PHP-8.2 and PHP-8.3 branches on the commit though... |
|
The change logs don't reflect this fix having made it into either 8.2.22 or 8.3.10 (which were release a couple days after the above). Is this because this PR was too close to the release date, and they're going to be published in the next versions? |
Instead of fixing up temporaries count in between observer steps, just take ZEND_ACC_DONE_PASS_TWO into account during stack_size calculation. Introducing zend_vm_calc_ct_used_stack for that use case.
This should be much less susceptible to forgetting to handle the ZEND_OBSERVER_ENABLED temporary explicitly.