Repository navigation
Conversation
Free OP_DATA when appending to a full array, and when converting a float or resource key throws, matching the VM.
329267a to
fa6e85c
Compare
iliaal
left a comment
iliaal
left a comment
There was a problem hiding this comment.
Please address the inline double-free.
| free_op_data: | ||
| if (opline->opcode == ZEND_ASSIGN_DIM | ||
| && ((opline+1)->op1_type & (IS_VAR | IS_TMP_VAR))) { | ||
| zval_ptr_dtor_nogc(EX_VAR((opline+1)->op1.var)); |
There was a problem hiding this comment.
This helper is also called from zend_jit_assign_dim_helper() after converting false to an array. If float or resource key conversion throws, OP_DATA is freed here and then freed again by the caller's unconditional jit_FREE_OP().
With $a = false; $a[1.5] = new D; and an error handler that suppresses the false-to-array deprecation but throws on the key-conversion warning, the PR head aborts in a debug build and Valgrind reports invalid reads. The base revision completes cleanly.
Please keep OP_DATA ownership with the caller and perform the missing cleanup in the direct generated fetch path. This also needs false-container variants of the float and resource tests.
There was a problem hiding this comment.
Thanks for catching it. I pushed another commit. Not sure about the new helper approach, whether it's acceptable solution, but it keeps the balance between code size and new instructions.
zend_jit_fetch_dim_w_helper() no longer frees OP_DATA, which zend_jit_assign_dim_helper() frees again after converting false to an array. The generated ASSIGN_DIM path selects dedicated fetch and append-failure helpers at compile time that free OP_DATA, so generated code size is unchanged.
Free OP_DATA when appending to a full array, and when converting a float or resource key throws, matching the VM.