[Enfiladeguide]

Fixes GH-24661.

Quoted text written with an LLM and checked by a human developer.

Since 980f7bd, ZEND_INIT_FCALL_OFFSET takes an internal function from the offset it had in the function table at compile time. A cached script can run with a different function table: FPM pools apply their own disable_functions and extension after the master has started, and the file cache can hold a script compiled after dl(). The offset then points to another function, which is called instead, and a call to a function that no longer exists fails the assertion in ZEND_INIT_FCALL (a NULL dereference in release builds).

The patch now only changes these two handlers. On a run-time cache miss, the offset is used only if the function there has the expected name, otherwise the function is looked up by name. A missing function throws "Call to undefined function", and so does a user function of that name if the call was compiled for the internal function (op2.extra holds its offset). Cache hits are unchanged.

The file cache cleared the offset until d6541df. It now keeps it, and the system ID, finalized at startup, doesn't change with dl().

Pools still share one opcache (#11723): JIT-compiled code can still call a function that a pool disables, and frameless calls and compile-time evaluation keep what the compiling process saw. On PHP-8.6 and master, ZEND_TYPE_ASSERT reads the same offset when the type doesn't match and would need the same check.

Tests: the file cache with dl() without FPM, a pool with opcache.preload, and two pools with different disable_functions. They fail on PHP-8.4 and pass with this patch, also on PHP-8.5, PHP-8.6 and master.

[ndossche]

I'm not sure this is really a nice solution, it is bolted on top with extra machinery.

[Enfiladeguide]

Quoted text written with an LLM and checked by a human developer.

Thanks, that's fair. I've reworked it to reuse the system ID, which since GH-17733 covers the internal functions and modules, instead of a second hash next to it:

• The part that System entropy does not take into account function order, resulting in wrong functions possibly called in different configurations #17733 added moves to zend_update_system_id(), which zend_finalize_system_id() calls at the end. It works on a copy of the MD5 context, so FPM calls it again where it changes the functions: after zend_disable_functions(), if it removed any, and after loading an extension. Both the pool configuration and PHP_ADMIN_VALUE go through that code.
• The file cache needs no change, as it already uses zend_system_id.
• Shared memory does not use the system ID, so opcache mixes it into root_hash in RINIT when it differs from the one at startup, as for chroot.

The counting in RINIT, the separate hash and the per-thread copy of the system ID are gone, and the opcache part is now 15 lines. The tests are the same apart from one comment, and the branch is rebased onto PHP-8.4.

If you'd prefer this somewhere else, for example in zend_disable_functions() itself rather than in FPM, I'll change it.

[bukka]

I don't think this is the right approach. It's just hacking around the fact that opcache is shared between pools. We have got various related issues and think the proper solution is the pool manager: #11723 (please don't try to ask your LLM to implement it - this needs a proper design and you need to know what you are doing for that).

[bukka]

I was checking this more and I just can't see a clean solution for this in stable branches. The current implementation (especially the FPM part) is just mixing the opcache inner working to fpm which is just not right IMHO. I'm open to ideas but it just cannot be something like this.

[Enfiladeguide]

Quoted text written with an LLM and checked by a human developer.

Thanks. I've dropped the FPM and opcache parts. The patch now only changes the two ZEND_INIT_FCALL handlers: on a run-time cache miss, a stale offset falls back to the lookup by name, and a missing function throws instead of crashing. On the current PHP-8.4 branch the same offsets also go wrong without FPM, with the file cache after dl() (the new opcache test). It doesn't make pools independent, so that stays with #11723.