Fix GH-21639: Protect frameless args during __toString() reentry

[prateekbhujel]

Fixes GH-21639.

Frameless internal calls can borrow CV operands directly from the caller frame. If one of those arguments is converted through __toString(), userland can mutate the borrowed values while the frameless handler is still reading them.

This keeps the fix on the actual reentry path:

• when __toString() runs from a frameless call, keep copies of string/array CV operands for that active frameless opcode
• release those copies after the frameless handler returns, so the borrowed operands stay valid for the whole handler
• use the same post-handler cleanup point for JIT frameless calls

Tests run:

make -j8
git diff --check HEAD~3 HEAD
sapi/cli/php run-tests.php -q Zend/tests/gh21639.phpt ext/pcre/tests/gh21639.phpt ext/standard/tests/strings/bug22224.phpt ext/standard/tests/strings/implode_variation.phpt ext/standard/tests/array/in_array/in_array_variation1_mixed.phpt ext/standard/tests/strings/strtr_basic.phpt ext/standard/tests/strings/str_replace_basic.phpt ext/pcre/tests/preg_replace.phpt
sapi/cli/php -d zend_extension=$PWD/modules/opcache.so -d opcache.enable_cli=1 -d opcache.jit=tracing -d opcache.protect_memory=1 -d opcache.jit_buffer_size=64M run-tests.php -q Zend/tests/gh21639.phpt ext/pcre/tests/gh21639.phpt Zend/tests/frameless_undefined_var.phpt

[iluuu1994]

Thanks for the PR. implode() is just one example, this applies to:

• preg_replace
• in_array (with strict: false)
• strtr
• str_replace

I'm afraid this will need a more general solution. My hope was to avoid overhead in the VM, but it might be unavoidable for a proper fix, even if this is a largely artificial issue.

[iluuu1994]

The Symfony benchmark IC increases by 0.64%, the Wordpress one by 1.15%. This removes much of the benefit of frameless calls, so I'm not to keen on solving this issue in that way. As long as other engine bugs aren't solved (#20001 and #20018) I don't think this needs to be rushed.

[bwoebi]

The other alternative would be checking inside the tostring handler whether the parent frame is currently at a frameless opcode and then safely copy its CV args to a buffer, set EG(vm_interrupt) and free them on the next EG(vm_interrupt), completely moving the overhead off the main paths and be a truly generic solution.
(Not necessarily vm_interrupt, it can rather be like the delayed error handler solution.)

Obviously comes at a small tostring handler cost, but I'd really rather see overhead there...?

[prateekbhujel]

@bwoebi Yeah, agreed. That cost belongs on the __toString() side, not on every frameless call.

I pushed a follow-up in that direction. It checks from __toString() whether the parent frame is currently on a frameless opcode, then keeps copies of the string/array CV operands for that opcode.

I did not keep the cleanup on EG(vm_interrupt): in the failing implode() cases it can run before the parent frameless handler has returned, so the parent frame still looks like it is sitting on the same frameless opline and the cleanup can keep treating the copies as live. I moved the release to the frameless handler return instead, with the same cleanup point for JIT frameless calls.

So the copy/dtor work is on actual __toString() reentry. The ordinary frameless path only has the cold pending-copy check after the handler returns, which seemed like the safer version of this approach.