opcache: set ZEND_ACC_PRELOADED for compile-only preloaded enums - #23103
Open
Nibbler999 wants to merge 1 commit into
Open
opcache: set ZEND_ACC_PRELOADED for compile-only preloaded enums#23103Nibbler999 wants to merge 1 commit into
Nibbler999 wants to merge 1 commit into
Conversation
phpGH-17835 made zend_enum_register_func() tag an enum's generated cases()/from()/tryFrom() with ZEND_ACC_PRELOADED, so that zend_persist_class_method() gives them a static run_time_cache map_ptr, which survives into every request. The tag is set inside the if (EG(active)) branch. An enum that preload reaches through opcache_compile_file() rather than by executing its file is linked from preload_link(), which runs after the preload request has ended, so EG(active) is already false and the tag is never applied. Those functions get a non-static map_ptr that nothing initialises again, so RUN_TIME_CACHE() is NULL in a fresh request and the first observed call dereferences NULL + the observer handle: #0 zend_observer_fcall_begin_specialized Zend/zend_observer.h:92 php#1 ZEND_DO_FCALL_SPEC_OBSERVER_HANDLER Zend/zend_vm_execute.h:2281 Move the tag out of the EG(active) branch so both linking paths set it. The compile-only case then takes exactly the same route as the already fixed declared case.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
GH-17835 fixed GH-17715 for enums that preload declares. It does not cover enums that preload only compiles, and those still crash on 8.4 and 8.5.
Cause
zend_enum_register_func()setsZEND_ACC_PRELOADEDinside theif (EG(active))branch.zend_persist_class_method()keys on that flag to chooseZEND_MAP_PTR_NEW_STATIC()overZEND_MAP_PTR_NEW(), i.e. a run_time_cache slot that survives into every request — which is what a class living in SHM permanently needs.An enum that preload reaches via
opcache_compile_file()is not linked while the preload script executes. It is linked frompreload_link(), which runs after the preload request has ended, soEG(active)is already false, theelsebranch is taken and the flag is never set.gdb on the crash, on an unpatched 8.5.9:
ZEND_OBSERVER_DATA()computesRUN_TIME_CACHE(...) + handle=NULL + 0and dereferences it — the same null deref, and the sameZend/zend_observer.h:92, that GH-17715's UBSAN output reports.Fix
Move the tag out of the
EG(active)branch so both linking paths set it. The compile-only case then follows the identical, already-proven route as the declared case; no new mechanism is introduced.Test
ext/opcache/tests/preload_enum_observed_compile_only.phptmirrors the existingpreload_enum_observed.phptand reuses itspreload_enum.incfixture, changing only how preload reaches it.