merge: cascade 8.4 into master - #202
Merged
Merged
Conversation
The property hooks paired setFakeScope() calls by hand around the original engine handler. Any throw from that handler - a throwing __get(), a typed property error, an uninitialized readonly access - skipped the restoring call and left EG(fake_scope) pointing at a foreign class entry for the rest of the request, silently changing visibility checks everywhere. Executor::withFakeScope() now owns the install/restore pair and restores in a finally block; all seven affected hooks (read/write/has/unset property, get property pointer, get properties for, get debug info) go through it, so the unsafe hand-rolled pattern can no longer be written. The baselined argument.type entries for the hook call sites are renamed to the new method - same pre-existing CData typing gap, new callee name. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
Three sites hand-rolled the "null out pDestructor, delete, put it back" dance that HashTable::deleteWithoutDestructor() already performs safely: - ReflectionMethod::fromHookCData() did it inside its finally block WITHOUT a try, so a failing delete() left the publication board's function table with a NULL destructor for the rest of the process - every later bucket removal would then leak. The now-unused getRawValue() handle is dropped too. - HotSwap::unpublishClassEntry() and Core::shutdown() duplicated the same logic; ClassDelta already used the helper, so this just finishes the job. Semantics are unchanged: the destructor is still disabled for the delete, so the payload (a shared zend_function, a rehomed class entry, a function embedded in an immortalized closure) survives the bucket removal. Core::shutdown() keeps its find() pre-check - zend_hash_del() reports an absent key as FAILURE, which HashTable::delete() turns into an exception. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
HashTable::find() returns null for an absent key, and addInterfaces() called
getRawClass() on the result unchecked. interface_exists() is not a sufficient
pre-check: it accepts a leading-backslash name ("\Countable") that strtolower()
never turns into a class-table key, so the lookup misses and the method dies
with a call-on-null instead of a ReflectionException - halfway through, with a
freshly allocated interface buffer already tracked.
Adds the same guard every sibling lookup carries (setParent(), ObjectEntry,
ClosureEntry, ...), which also retires a baseline entry.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
…dant casts Three hot-path reductions in the reflection layer, no behavior change. Instance minting: inside namespace ZEngine\Reflection the unqualified name ReflectionClass resolves to z-engine's own class, whose constructor runs the native parent constructor, a class-table find() and initLowLevelStructures() (four HashTable wrappers) - all discarded when the instance only serves newInstanceWithoutConstructor(). The ten such sites in ReflectionMethod, ReflectionProperty, ReflectionClassConstant and ReflectionFunction now use the aliased native reflector, matching ReflectionClass::fromCData() and the Type\* wrappers. ReflectionMethod::fromHookCData() keeps the z-engine class for the publication board, which genuinely needs getMethodTable(). Class-table walk: FunctionBodySwap::countPublishedShares() built a full ReflectionClass per engine class only to read one type byte and look up one method bucket. Both answers now come off the entry through the owning class via new @internal ReflectionClass::entryIsUserDefined() and ReflectionClass::entryMethodTable() helpers, so no call site pokes the raw zend_class_entry itself. Counting semantics (alias dedup, user-class filter, address match, floor of 1) are unchanged. Redundant casts: four sites passed an already-typed zend_class_entry * through Core::cast('zend_class_entry *', ...), paying a count() probe, a caught FFI exception and a type lookup per call. Each value is a pointer field (zend_object.ce, zend_function_common.scope), never an array needing decay, so the cast is dropped as in ReflectionProperty::getDeclaringClass(). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
initLowLevelStructures() cached a HashTable wrapper over zobj->properties once, but setDynamicPropertiesPointer() rewrites that very field without touching the shadow copy. The only reader, __debugInfo(), therefore dumped a table the object no longer owns - possibly one already freed, i.e. a dangling read from a debug dump. Objects whose table was built after the entry (the common lazy case) were also missing from the dump entirely. The field is removed and the table resolved from the live pointer at dump time, so ObjectEntry keeps exactly one source of truth for zobj->properties. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
Least privilege, run serialization and supply-chain pinning across every workflow, plus two cheap gates and two caches in the quality jobs: - ci.yml gets `permissions: contents: read` (it only builds and tests - merge-up.yml and both generate-*-headers.yml already declared theirs) and a concurrency group that cancels superseded pull-request runs while leaving branch runs to finish. - The two header-generation workflows commit and push to the target branch, so they get a non-cancelling concurrency group keyed on that branch: two overlapping runs would race on the push/rebase, and a cancelled run can leave the artifacts half-refreshed. merge-up.yml gets the same treatment for its "is a merge-up PR already open?" check. - Every non-actions/* action is pinned to a full commit SHA with the tag it resolves as a trailing comment (setup-php v2 -> v2.37.2, composer-install v3 -> v3.2.1, msvc-dev-cmd v1 -> v1.13.0, setup-buildx-action v3 -> v3.12.0). actions/* stay on their major tags. - static-analysis validates composer.json (--no-check-lock: the lock is deliberately not committed) and audits the installed dependencies for known advisories after the install step. - PHPStan gets a deterministic tmpDir (var/phpstan, already covered by the /var/ ignore) and both it and the php-cs-fixer cache are restored across runs, keyed on the tool config plus composer.json with a restore-keys fallback. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
Only composer was covered. Adds a weekly github-actions entry (which also keeps the new SHA pins and their version comments current) and one weekly docker entry per Dockerfile directory - tools/generator/Dockerfile and tools/docker/php-debug.Dockerfile, the two images CI builds inline. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
SECURITY.md points reporters at GitHub private vulnerability reporting (there is no dedicated security contact) and draws the line this project needs: a library that writes to Zend Engine memory by design crashes the process when misused, which is not a vulnerability - a wrong struct layout on a supported build, or corruption reachable through the documented PHP-native surface, is. The pull request template encodes the CONTRIBUTING.md checklist: target the minimum affected version branch, report the exact PHP build used, keep phpstan/cs:check green, and regenerate headers with composer gen-headers instead of hand-editing include/ or stubs/. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
…Hook OpCodeHook::handle() captured a debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS, 10) on EVERY execution of a hooked opcode, only to answer "is z-engine's own code running this opcode?". Hooking a hot opcode (ASSIGN, DO_FCALL, EXT_STMT) paid a full stack walk per operation. The answer is already in the callback argument: the zend_execute_data* IS the frame that executes the opcode, and its zend_function carries the class scope that debug_backtrace() would have reported as the frame's "class". The filter now reads that scope off the frame - two struct reads and one zend_string materialization, no stack walk, no allocation per frame. Frame resolution goes through the owning wrappers, not raw CData at the call site: ExecutionData::getFunctionScopeName() (frame owner) delegates the entry read to ReflectionFunction::getScopeNameOf() (zend_function owner). The latter is a pointer-level static on purpose - ReflectionFunction::fromCData() initializes the native reflection state, which does a function-table lookup and throws (with its own trace capture) for method entries, i.e. exactly the cost being removed. Preserved semantics, case by case: - method/closure frame: scope name is the declaring/bound class, so a "ZEngine..." prefix still dispatches to the default handler - including frames of the test suite itself, whose namespace is also ZEngine\ (autoload-dev maps it to tests/), which is why OpCodeHookTest compiles its probes as global functions; - global function frame: no scope, previously reported by debug_backtrace() as a frame without a 'class' key -> empty class -> user handler runs. Now scope is NULL -> user handler runs; - main-script/pseudo frame: produced no backtrace frame at all -> empty class -> user handler ran. Its op_array carries no scope either -> user handler runs; - no function entry / unresolvable scope (trampolines, internal entries without a scope): conservatively treated as non-ZEngine, i.e. the user handler runs, which is the outcome the backtrace variant produced for those frames. Stacked hooks no longer need frame skipping. The old loop skipped frames whose class was OpCodeHook itself because a hook delegating to its predecessor via $this->originalHandler pushed its own handle() frame between the executing frame and the backtrace root. Delegation passes the SAME execute_data pointer down the chain, so every hook in the chain resolves the identical executing frame directly - there is nothing between them to skip. As a side effect the reentrancy guard got stricter in the right direction: an opcode genuinely executed inside OpCodeHook::handle() now resolves to OpCodeHook's own scope and dispatches, instead of being skipped over in search of a deeper frame. Also in this file: Closure::fromCallable([$this, 'handle']) -> $this->handle(...) (the style AbstractModule already uses), and the now-dead BACKTRACE_LIMIT constant is replaced by the named ENGINE_SCOPE_PREFIX it always served. docs/self-debugging.md described the per-hit backtrace filter as a cost driver and is updated accordingly. No phpstan-baseline entries covered the removed code, so none were pruned; phpstan level max and php-cs-fixer stay clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
…OfType() Core::sizeOfType() is the blessed named API for "how big is this engine type here?" - Core::type() is @internal and must not hand a raw FFI\CType across the boundary just to be measured (AGENTS.md). src/Reflection and src/Type were already migrated; this converts the 58 remaining call sites in OpCache, Memory, EngineExtension, System and the rest of Reflection. Plain struct names go through the generated stub class constant (Core::sizeOfType(zval::class)), which is checked by the analyser and never autoloads; the forms no stub models - pointers ('zend_class_entry *', 'zend_string *') and the two dynamic type names resolved at runtime - keep the C type-name string overload. Byte-for-byte the same size computation, only the API changed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
Every Core::sizeOfType() call re-resolves the C type through FFI, so resolving it per element of a hash/table walk is pure overhead for a value that cannot change inside the loop. - PayloadRelocator::unserializeHash()/serializeHash(): the packed-array branch resolved sizeof(zval) on every bucket of every hash in the payload, while the sibling non-packed branch already hoists $bucketSize - hoist $zvalSize to match. - ClassSpecializer: hoist the trait-alias, trait-precedence, property-info and class-constant struct sizes out of their copy loops (the precedence loop already hoisted its sibling $pointerSize). Behavior is unchanged - the same sizes, computed once per loop instead of once per element. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
The Type layer had no domain exception class: nine failure modes threw a bare
\RuntimeException with a hand-written message at the call site, and six of them
were near-duplicates of each other ("Can not add/store/delete an item with
key/index ..."), so the same wording lived in three files at once.
TypeOperationException now owns every one of those messages behind a named
static constructor, per the convention in AGENTS.md. It extends \RuntimeException
- exactly what each replaced throw threw - so every existing catch keeps
matching: Core::shutdown()'s find() pre-check around HashTable::delete(), the
\RuntimeException expectations in PersistentHashTableTest and
StringEntryOwnershipTest, and any consumer catching the base type.
Messages are carried over verbatim; only their home changed.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
…nstructors AbstractModule had no module exception class at all: register() and startup() threw three bare \RuntimeExceptions with inline messages, while ZEngineModule hand-wrote the very same 'The zengine module has no globals block' text twice. Two classes now own those messages behind named constructors (AGENTS.md): - ModuleRegistrationException (EngineExtension, shaped like the neighbouring ExtensionNotRegisteredException) extends \RuntimeException, exactly what the three replaced throws threw - the module-lifecycle-order fixture's catch (RuntimeException) around a rejected registration keeps matching. - HeapAnchorMissingException extends PersistentHeapException, which is what ZEngineModule already threw there. The anchor slot IS heap state, so the class joins the existing heap hierarchy in ZEngine\Memory next to its siblings rather than moving the failure to an unrelated base type and breaking every catch (PersistentHeapException) around a heap lookup. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
…e_callback preg_replace_callback() returns null when PCRE fails (backtrack or recursion limit), and that null went straight into strtolower() - a deprecation since PHP 8.1 that the baseline had been carrying as "expects string, string|null given". On the error path the class name is now used unsplit instead, which is what the camelCase-to-snake_case conversion degrades to anyway. The freed baseline entry is pruned. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
…onstructors
install() and uninstall() reported the two zend_set_user_opcode_handler()
failures with inline messages that share their whole vocabulary ("Can not
install user opcode handler" / "Can not restore original opcode handler").
Both are engine-domain failures of the same operation, so OpCodeHookException
now owns them behind named constructors, extending \RuntimeException exactly
like the throws it replaces.
The three remaining SPL throws in this file stay native on purpose:
- the Core::isShutdown() guard repeats a message that AbstractHook and
IteratorBridge raise verbatim; unifying it means one factory for all three
sites, which belongs with the hook-consolidation work that owns those files,
not to a partial conversion here.
- the out-of-order uninstall guard and the handler-signature check are
programmer-misuse guards on a public API with stable messages (the latter is
asserted verbatim by OpCodeHookTest); a factory adds no clarity there.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
countPublishedShares() reflects each class again: the framework's model is that consumers hold Reflection objects, and entry-level static utilities on ReflectionClass cut against that. The redundant zend_class_entry* cast removal in applyStaticVariableDefaults() stays. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
…-6pfu7y-reflection-fastpath perf(reflection): mint instances via native reflection and drop redundant casts on hot paths
…-6pfu7y-exception-safety fix: exception-safe engine-state restoration (fake scope, pDestructor, class-table guards)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
…-6pfu7y-ci-hardening ci: permissions, concurrency, dependabot coverage, SHA-pinned actions, caches, composer validate/audit
Bumps [actions/upload-artifact](https://github.com/actions/upload-artifact) from 4 to 7. - [Release notes](https://github.com/actions/upload-artifact/releases) - [Commits](actions/upload-artifact@v4...v7) --- updated-dependencies: - dependency-name: actions/upload-artifact dependency-version: '7' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [actions/download-artifact](https://github.com/actions/download-artifact) from 4 to 8. - [Release notes](https://github.com/actions/download-artifact/releases) - [Commits](actions/download-artifact@v4...v8) --- updated-dependencies: - dependency-name: actions/download-artifact dependency-version: '8' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [actions/checkout](https://github.com/actions/checkout) from 4 to 7. - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](actions/checkout@v4...v7) --- updated-dependencies: - dependency-name: actions/checkout dependency-version: '7' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [docker/setup-buildx-action](https://github.com/docker/setup-buildx-action) from 3 to 4. - [Release notes](https://github.com/docker/setup-buildx-action/releases) - [Commits](docker/setup-buildx-action@v3...v4) --- updated-dependencies: - dependency-name: docker/setup-buildx-action dependency-version: '4' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [actions/cache](https://github.com/actions/cache) from 4 to 6. - [Release notes](https://github.com/actions/cache/releases) - [Changelog](https://github.com/actions/cache/blob/main/RELEASES.md) - [Commits](actions/cache@v4...v6) --- updated-dependencies: - dependency-name: actions/cache dependency-version: '6' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [ramsey/composer-install](https://github.com/ramsey/composer-install) from 3 to 4. - [Release notes](https://github.com/ramsey/composer-install/releases) - [Commits](ramsey/composer-install@v3...v4) --- updated-dependencies: - dependency-name: ramsey/composer-install dependency-version: '4' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
…-6pfu7y-sizeoftype refactor: finish the Core::sizeOfType() migration and hoist loop-invariant sizes
…-6pfu7y-exception-factories refactor: align Type and EngineExtension exceptions with the named-constructor convention
…ions/cache-6 ci: Bump actions/cache from 4 to 6
…sey/composer-install-4 ci: Bump ramsey/composer-install from 3 to 4
…ker/setup-buildx-action-4 ci: Bump docker/setup-buildx-action from 3 to 4
…ions/checkout-7 ci: Bump actions/checkout from 4 to 7
…ions/download-artifact-8 ci: Bump actions/download-artifact from 4 to 8
…ions/upload-artifact-7 ci: Bump actions/upload-artifact from 4 to 7
Re-bases the CI signal on the true merged state: 8.4 gained the exception-factory classes (#201) that also touch OpCodeHook's install-time guards. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
…pers Per maintainer review on #199: no raw zend_function crossing an API boundary and no static entry helpers. ReflectionFunction now overrides the native getClosureScopeClass(), wrapping the scope it already owns; ExecutionData::getScopeClass() connects the frame to its entry with a null-safe chain; OpCodeHook reads the scope name off the returned wrapper. getScopeNameOf()/getFunctionScopeName() are gone. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnoZ7wuGepCTzsmFQ5sKxG
…-6pfu7y-opcodehook-frame perf(system): resolve the executing frame from execute_data instead of debug_backtrace in OpCodeHook
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
2 participants
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.
Automated cascade merge of
8.4intomaster(branch flow defined in.github/branch-flow.json).Resolve conflicts in favour of the newer engine structures where they touch
include/- regenerate headers on the target branch instead of merging them textually. See AGENTS.md.