Fix #75932: zend_user_opcode_handlers crash

[johnstevenson]

Only call a user_opcode_handler if it has been set, otherwise call the
original opcode handler. When using opcache, a user handler will not be
set if the cache was created by an extension that used its own handlers,
then hit later by a process not using the extension. Also fixes #75886.

[johnstevenson]

@krakjoe @nikic Not wanting to hassle, but has this been forgotten or is there anything I need to do? Or is it unsuitable and perhaps too naive a fix?

[nikic]

Probably @dstogov is the best person to look at this. I think the actual solution here might be to include the fact that user opcodes are used in the system hash, to ensure that cached opcodes are not reused at all in this case. (Actually we might want to check all loaded extensions -- I remember we already have a bunch of optimizations disabled on windows because we can't rely on the same exts being loaded there.)

[dstogov]

User opcode handlers should be set once and forever (at MINIT).
Alternatively, you may disable opcache for requests, that are going to set user opcode handlers.

[krakjoe]

So this doesn't look like the best thing to do ... closing ... sorry about the delay ...

[johnstevenson]

@krakjoe No worries. I should have replied to the feedback I asked for, but I got a bit behind. Is it possible that this could be re-opened so it could be discussed a little more?

Alternatively, you may disable opcache for requests, that are going to set user opcode handlers.

@dstogov Sure, but how do folk know that they must do that. I don't think it is correct behaviour to crash. It is quite common for people to disable an extension like xdebug (for performance reasons). IDEs offer this functionality and so do CI services.

And if opcache file-caching is being used then there could be a crash if xdebug is run first and the opcode handlers are cached. See https://bugs.php.net/bug.php?id=75932

My use case is for composer/xdebug-handler, which is part of Composer and is used in a few other big projects. This restarts a process without xdebug loaded and will crash on Windows regardless of whether file-caching it being used. See https://bugs.php.net/bug.php?id=75886

xdebug-handler works round this issue by disabling opcache cli in the restarted process.

It seems inherently wrong to me that PHP should happily use a nil pointer without checking .

I think the actual solution here might be to include the fact that user opcodes are used in the system hash, to ensure that cached opcodes are not reused at all in this case.

@nikic My solution (if it is realistic) allows opcodes that exist to run, rather than not using any at all.

[dstogov]

If you change user opcode handlers at run-time, you introduce race-conditions in ZTS build and get crash soon or later.

[johnstevenson]

Thanks @dstogov , but how are the opcode handlers are being changed at runtime?

All I have suggested doing is returning ZEND_USER_OPCODE_DISPATCH if the handler is not found in the zend_user_opcodes array (which is something an extension could do anyway) so that the original opcode is used.

[cmb69]

I think the actual solution here might be to include the fact that user opcodes are used in the system hash, […]

I agree, but currently the system hash is calculated during OPcache startup, and the user opcodes are not necessarily set then (depends on loading order). Not sure, if and how that could be solved.