feat(core): align AOT and eval Core builtin support by nahime0 · Pull Request #893 · illegalstudio/elephc

nahime0 · 2026-09-04T14:51:09Z

Summary

Aligns the Core builtin contract and its AOT and Magician implementations, including the callable, ownership, and runtime-state regressions found during review. The branch was rebased onto main at 217ff6caa early on; the later main changes (#991, #997, #1003, #1004, #1008) were brought in with a merge commit (3a85bfc2d) because a second rebase of the 700-commit history conflicted with #892's by-ref argument rewrite.

The internal Core inventory is 59/59 on both backends. This is not a claim of complete PHP compatibility: the generated PHP 8.5 comparison is 57/62 functions. The inventory excludes clone, get_error_handler(), and get_exception_handler(); the comparison counts die and exit separately as language constructs.

Changes

Shared contracts, typed EIR lowering, and target-aware runtime support for Core introspection, handlers, and GC controls.
Class introspection through direct calls, statically selected CUF/CUFA, FCC, named arguments, and spread arguments. Mixed array extraction no longer corrupts later object introspection.
Shared resource identity and ownership, last-owner retirement, explicit-close propagation, descriptor-reuse protection, and preservation of standard streams during web request reset.
Error-handler suspension and exception-safe restoration, ordinary-warning dispatch, reporting masks, nullable error_reporting() queries, and unhandled user-error termination.
Native visibility of declarations from the current eval context, shared resource inventory, array-valued constant snapshots, and thin-eval catalog constants including PHP_INT_SIZE.
Reflection fixes for protected visibility, trait properties, declaration order, and enum method spelling.
Backtrace composition across native and eval boundaries, preserving eval markers, options, and limits.
Integration with main's PCNTL callback lifetime, argument planning, and distinct eval status codes.
Focused regression tests, examples, and regenerated builtin documentation.

Deliberate bounds

59/59 measures implemented contract entries, not unrestricted PHP semantics. Class-introspection callable targets remain subject to the documented static-target limits.
Native constant/function inventories append declarations from the current function's eval context; this is not a new process-global registry across unrelated eval contexts.
Resource inventory visibility is shared, but eval fclose() still operates on eval-managed streams. Synchronizing native locals into eval also retains them until the next synchronization or context release.
Constant categories use the documented Core/user grouping. disable_functions is unsupported, so get_defined_functions() accepts the flag without changing the inventory.
get_required_files() remains PHP's alias of get_included_files().
GC compatibility fields such as threshold, buffer_size, and full do not claim Zend collector-buffer parity.

CI stabilization

Fixes landed after the first full CI run, each from a red job on the matrix:

Web handler double free. The global-receiver write-back retired the previous hash after __rt_hash_set, but growth past 16 entries already frees that block; $_GET with more than 16 parameters freed the table twice and the request-end reset walked a reused string as a hash (every web/session test red on all targets). Element-write sites now publish only; mutating builtins keep their retiring write-back.
By-ref variadic SIGSEGV on aarch64. array_to_hash consumes the reference cell's array and the following store_ref_cell retired it again; the freed block was reused by the persisted key string. The cell store now only publishes when fed by that conversion. Reproduced locally with a zig-backed linux-aarch64 build under qemu and with --heap-debug on x86.
Destructor resurrection is gated on the in-progress guard bit, so the collector's sweep of cycle members still frees them (GC cycle tests and web resets).
Nested global declarations. The shared walk now descends into closure bodies and enum methods, so unset() plus a global $x inside a closure keeps the shared symbol and prints PHP's answer; the locals_retype fixtures pin the new behavior, including the vetoed kill being an ordinary retype in both modes.
Eval bridge. Eval-declared property writes and eval closures (Closure::bind on an eval callback adapter) are bridged through runtime hooks; ancestor-private names on eval children are answered dynamically; runtime property names are cast to string.
Checker. A mixed variable is accepted for a by-reference array parameter; container write-backs and scalar releases no longer drop closure facts.
Two EIR ownership-shape tests were updated to the current invariants (last detach before release; a borrowed string reference-place view is not released).
Mutating-builtin receivers on Mixed-widened locals (found while re-checking open issues, not by CI). prepare_consuming_storeback drops the slot's previous box so the receiver load is the sole owner during an in-place sort(), but the store-back then moved that owner into the new box while EIR still releases the load: the sorted array was freed under the box, and an eval() escape walk iterated the reused block until the heap ran out (AOT ReflectionClass::getMethods() followed by eval() reflection on the same class hangs forever #1031's shape, which main only avoids by leaking the old box). The receiver store now retains exactly when a later instruction releases the value; array_multisort, whose receivers EIR never releases, keeps the owner transfer. Regression: test_eval_reflection_after_aot_get_methods_and_sorted_local_terminates.
Eval shard runner (Greptile review). The single-process eval shard kept cache reuse but had dropped nextest's per-test termination and the one retry; it now watches the single pass for progress (stops after 180s without a completed test, or 30 minutes total) and reruns whatever did not pass under nextest, each test in its own process with the configured slow-timeout and --retries 1 --flaky-result pass.

Validation

CI run 35119004532 on abef3186b was green across the full matrix, and run 35221162825 covers the receiver store-back fix on 04024b2b9; run 35231600667 covers the eval shard runner on a3e590e2e (macos-aarch64, linux-aarch64, linux-x86_64 codegen/non-codegen/eval/web/PDO shards, iOS emitter checks, PDO live databases). The linux-aarch64 PDO shard 1/4 needed one rerun after a stuck runner with no log; it passed in its usual six minutes.

Local testing was limited to single tests and small families (the affected web, session, GC, globals, hash, variadic and ownership families), per the stabilization instructions.

Closed issues

Verified by running each issue's reproduction on this branch and on current main (129bf261e), against PHP 8.5.10:

Closes Support func_get_args and func_num_args with default parameters #856 — func_get_args()/func_num_args() in a function with default parameters compile and return the passed arguments (array(0) {} for values()).
Closes Runtime callable invoker mis-marshals array arguments: bare array param receives garbage, loop-grown assoc boxed with the indexed tag #925 — the runtime callable invoker hands a bare array parameter its real payload and boxes a loop-grown assoc with the hash tag; all three probe lines match PHP.
Closes Reading $argc inside a closure body compiles and the binary segfaults (with a by-value capture overwrite) #783 — reading $argc inside a closure with a by-value capture overwrite no longer segfaults; the program prints PHP's 77|1 (the "Undefined variable" warning is still not emitted).
Closes eval() in a parameter default compiles and the binary segfaults #782 — eval() in a parameter default no longer segfaults; the default is evaluated and the eval-local $a stays out of the caller's scope (none|1). PHP rejects the construct at compile time; the issue accepts either outcome.
Closes AOT ReflectionClass::getMethods() followed by eval() reflection on the same class hangs forever #1031 — the AOT getMethods() + eval reflection program no longer spins (macOS, verified on an arm64 host before and after the fix) or exhausts the heap (linux); the receiver store-back kept the sorted array alive under its box. The wrong casing of the printed names stays under Bug: ReflectionMethod::getName() loses declared method casing #571.

Partially addresses #742: engine warnings (Undefined array key, E_WARNING) and E_USER_WARNING now reach set_error_handler(); error_get_last() and the mysqli warning sites are still open.

Known follow-ups (not in this PR)

Direct call to a by-ref variadic leaks one block per call when the arguments are positional only #1062 — a direct call to a by-ref variadic leaks one block per call when the arguments are positional only.
Closure::bind of an eval-declared closure from AOT leaks five blocks per bind #1063 — Closure::bind of an eval-declared closure from AOT leaks five blocks per bind.
The descriptor invoker's hash argument container is never released #1047 / Closure / dynamic-callable invocations leak one heap block per string argument and per mixed-bound argument (SAX handlers exhaust the default heap at ~85k elements) #924 — descriptor-invoker container and dynamic-callable argument leaks, pre-existing on main.

nahime0 · 2026-09-07T10:10:21Z

Sorry @Guikingone, I've found some issues and must postpone your review on this one

A mutating container write-back publishes whatever its helper returned, which is the same pointer whenever copy-on-write found a sole owner and growth did not relocate. Since the ref-cell store began retiring its previous occupant, that released the storage the cell still points at: two `$items[$i] = …` writes through one by-reference variadic freed the collector out from under the next read, and every eval callable form that writes a by-reference variadic returned empty strings. Skip the retirement when nothing was actually replaced, release the cell the attached key-sort separation replaces when its receiver is a raw local, and only retain a key-sort place read when it is a borrow — an element read already hands over an owning temporary.

A named argument the signature does not declare is collected by the variadic tail, so `replace(named: $n)` on `function replace(&...$items)` writes through `$n` exactly like a positional argument in that tail does. Constant propagation only matched declared parameter names, so it kept folding the caller's old value after the call and masked the write-back entirely.

A by-reference `foreach` over an array element now reaches its source through a reference cell, so the sentinel arrives as a boxed Mixed value and `__rt_mixed_unbox` normalizes it to the null tag before the shape dispatch that would touch storage. The statically shaped source still folds the sentinel to zero. Issue #556 asks that no array header be read from a sentinel; assert that one of the two recognitions is present instead of pinning the older shape.

Widening that rule to refcounted payloads dropped `Callable`, which `PhpType::is_refcounted` deliberately excludes, so an extracted descriptor lost its published root slot. Name both shapes.

Two lowering assertions still described the shapes these paths had before the branch changed them: - A scalar array slot handed to a by-reference parameter is now promoted to a managed reference cell instead of exposing a bare interior address. Count both ops, and keep the per-op checks that the exposed reference is a non-heap pointer taken from a boxed parent. - An optimization pass can leave a `Nop` where it removed an instruction, so the union's owned operand release is the next instruction that still does something, not literally the following one.

A declared PHP array reads back as one boxed value, so a by-reference element argument taken from it matched neither concrete receiver branch and skipped the lease that puts its managed cell in the call's unwind ledger. A later argument whose evaluation threw — a destructor firing while a sibling argument detached the array — then left the cell unreleased.

The synthetic alias a by-reference `foreach` binds to a static property was declared Mixed before the binding supplied the slot's actual payload type, and `declare_local` keeps the first declaration. An untyped `public static $rows = [...]` keeps concrete hash storage, so the loop's write-back boxed that hash into a Mixed cell and published it through the symbol — the next `C::$rows[$k]` read then walked a cell as a table and segfaulted.

Widening a concrete-element parent for a `mixed`/`array` by-reference parameter leaves the element in a boxed slot, but the argument was still handed out as a bare interior address. Two aliases of one element — `f($outer[0], $outer[0])` — then disagreed: the second promoted the slot to a reference cell and the first kept pointing at it, so the callee read that cell pointer as the element's value and `count()` rejected an int.

The bump path adds the requested payload size and the 16-byte header straight onto the offset, so a request that is not a multiple of eight — a persisted string of any odd length, the script path among them — misaligned EVERY block carved after it. Generated code reads those blocks with plain word loads and never notices; the eval interpreter dereferences the same blocks as `*mut RuntimeCell` and aborted on Rust's alignment check. That is why the eval by-reference bridge failures looked non-deterministic: they tracked the length of unrelated strings, and any diagnostic that shifted an allocation made them disappear. Round the payload up at the single entry point both the free-list and bump paths share.

A write-back that republishes what __rt_*_ensure_unique returned must not retire the cell's previous occupant: the COW split already dropped the mutator's owner, so the second release freed the argument hash an eval by-reference variadic still shared. RefCellStorePrevious names the two conventions; the mutating builtins keep Retire. Mutating builtins on a global-backed local now resolve a Global receiver place (the _eir_global_* symbol) instead of refusing, and array_reverse on a boxed source no longer keeps the checker's concrete result type.

An eval() in scope widens every local to Mixed, so $o->{$name} reached the backend ladders boxed and was refused. The get, set and unset lowerings cast a non-string name first, as php does.

Release-family ops carry the conservative all() effects, so every release_local_slot looked like an opaque user-code boundary and erased global-backed closure facts. They now answer from the released payload type, unions member by member, and the release is analysed against what the slot still holds while the tracking decision follows the widened slot type.

A destructor that stores $this in a new owner (a closure it creates, a global) must not have its storage freed underneath that owner, and __destruct must never run twice. The free path now checks for surviving owners after the call, clears the guard and marks completion in kind bit 17 exactly like the collector does.

The walk now descends into every expression, so a global written inside a closure literal, an assignment prelude or an enum method binds the top-level name to program storage like a named function's would. The Mixed-array backend gaps that kept this a deliberate blind spot are closed; the checker's unset-kill veto cost is documented.

php resolves a name the layout carries only as a strict ancestor's private slot to a dynamic property outside that ancestor. The eval bridge now falls through to dynamic storage instead of refusing, and a generated Mixed-receiver read asks Magician whether the receiver is an eval-declared child before raising the private-access Error.

A runtime-name write on a Mixed receiver that lands in the stdClass arm now hands an eval-declared object to Magician's setter, under the writer's lexical scope, so private slots and the __set guard hold and a refusal is raised as the same Error php throws. A closure created by eval() can now be invoked and rebound from generated code: the boxed-callable ladders wrap an eval callback into an adapter descriptor, and __rt_closure_bind rebinds $this through a hook Magician installs with the eval context.

The declared PHP-array union lowers to the same boxed Mixed cell a mixed variable already holds, so the reference needs no storage change; only the compile-time contract refused a mixed return value bound to array &$p.

# Conflicts: # src/types/checker/functions/call_validation.rs # src/types/checker/functions/resolution/call.rs # src/types/checker/functions/resolution/resolved.rs # src/types/checker/type_compat/declarations.rs # tests/error_tests/misc/functions.rs

The collector sweeps cycle members that still count peers as owners after running their destructors itself; gate the resurrection path on the in-progress guard bit so those blocks are freed instead of kept. Pin the closure/enum global fixtures to PHP's answers now that the shared walk sees nested bodies, and relax the lease cleanup ownership assertion to the cell's last detach.

Element-write helpers (hash_set, hash_unset, hash_append, hash_spread, the element-cell helpers) split under the ensure_unique convention and free the block they replace on growth. The global-receiver write-back still retired the previous pointer, so $_GET with more than sixteen parameters freed the pre-growth table twice inside the web handler and the request-end reset walked a reused block as a hash. Give global and raw-local receivers a publish-only write-back for those sites; the mutating builtins keep their retiring store-back. A store_ref_cell fed by array_to_hash of the same cell's load only publishes too: the conversion already consumed the cell's owner, and the second release freed a by-reference variadic's argument array under a first-class callable (SIGSEGV on aarch64). Pin the string reference-place fixture to the pruned release.

A mutating builtin's receiver on a Mixed-widened local is an unboxed load that lowering releases after the call. prepare_consuming_storeback drops the slot's previous box so that load is the sole owner during the in-place mutation, and the store-back then moved that owner into the new box: the array ended one owner short, sort($names) freed it under the box the slot kept, and an eval() escape walk iterated the reused block until the heap ran out (issue #1031's linux shape). Retain into the box exactly when a later instruction releases the value; receivers lowering never releases (array_multisort) keep the owner transfer.

greptile-apps · 2026-09-17T13:36:05Z

Confidence Score: 5/5

The PR appears safe to merge based on the reviewed changes, with no outstanding actionable finding.

Summary

This PR aligns the Core builtin contract across AOT and eval, with extensive changes to callable dispatch, runtime introspection, resource ownership, error handlers, GC, reflection, backtraces, and generated builtin documentation.

Changes since the previous review primarily:

Add a watchdog around the shared-process eval test shard.
Rerun unfinished or failed eval tests through nextest with timeout and retry protections.
Wire extracted nextest archives into the eval shard workflow.
Split and parallelize additional CI suites.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Shared Core builtin contract] --> B[AOT EIR lowering]
    A --> C[Magician eval bindings]
    B --> D[Target-aware runtime helpers]
    C --> D
    D --> E[Core introspection and handlers]
    D --> F[Resources and ownership]
    D --> G[GC and reflection]
    D --> H[Backtrace and eval bridge]
    I[Eval codegen tests] --> J[Single shared libtest process]
    J --> K{Completed successfully?}
    K -->|Yes| L[Shard passes]
    K -->|No or watchdog| M[Select unfinished and failed tests]
    M --> N[Nextest timeout and retry fallback]

_{Reviews (2) · Last reviewed commit: "ci(eval): keep per-test termination and ..."}

…ss shard The eval shard runner reuses one libtest process for cache reuse, which dropped nextest's per-test slow-timeout and the retries-1 policy the other shards keep. Watch the single pass for progress and stop it when no test completes for the eval budget (180s) or the shard exceeds 30 minutes, then rerun under nextest only what the first pass did not report as ok, each test in its own process with the configured timeout and retry.

Guikingone · 2026-09-17T16:35:29Z

Consolidated review — 4 independent reviewers

Reviewed at a3e590e2e against merge-base 710125525. Four reviewers read the branch independently, each with its own checkout and tool access (read/diff/grep over the full tree): Claude Opus 5, Kimi K3, GLM 5.3, DeepSeek v4.1 (the last three through Ollama). Every finding below was re-read in the source before being included; the ones that did not survive that check were dropped.

Scope note. 3314 files, +152k/-19.6k, ~2100 of them generated docs. Nobody read all of it, and nothing was built or run here — a full codegen suite is a multi-hour build on this machine. So the ownership arithmetic below is traced from the emitted-code shape, not observed under --heap-debug. Please treat finding 1 as needing a heap-debug repro before it is accepted or dismissed.

The review was deliberately concentrated on what the PR description itself flags as riskiest: the late CI-stabilization fixes, above all item 7 (the conditional retain in the receiver store-back).

1. `sort()` / `rsort()` on a hash in a Mixed-widened local leaks the pre-rebuild table — major

src/codegen/lower_inst/builtins/arrays/sort_dispatch.rs:129 and :162, with src/codegen/context.rs:1006

Found independently by Kimi K3 and GLM 5.3, with the same file and lines and the same mechanism. That convergence is why it leads.

lower_hash_reindexing_sort publishes its receiver twice: once after ensure_unique_hash_sort_source (line 129) and again after __rt_array_slice_to_hash rebuilds the reindexed table (line 162). Both go through ReceiverPlace::store_back → store_receiver_value_to_local.

For a Raw Mixed-widened slot holding a value that EIR releases later, that function now takes the new path (context.rs:1011-1027): it boxes with emit_box_current_value_as_mixed, which retains, and stores the box without retiring the slot's previous occupant. That is correct for the single-store-back builtins, because prepare_consuming_storeback already dropped the old box — but it is called once, and there are two publishes here:

store-back fseek() returns cursor position instead of 0/-1 #1 retains the COW-split table T0 into box B1, slot ← B1;
store-back fgets() on invalid file descriptor hangs instead of returning false #2 retains the rebuilt table T1 into box B2, slot ← B2, and B1 is never released;
the parked __rt_decref_hash at line 169 drops only the load's own lease on T0.

So B1 keeps a live reference on T0: the box and the whole pre-rebuild hash table with its entries leak, once per call, unbounded in a loop. Reachable via sort($h) / rsort($h) where $h is an assoc array in a slot widened to Mixed — sort_receiver_is_hash (builtins/arrays.rs:700) routes exactly that to lower_hash_reindexing_sort.

The two reviewers disagreed on what this path did before the PR: Kimi read it as a 24-byte box leak, GLM as the #1031 use-after-free shape (store-back #2 transferred the owner into B2 and the EIR release then freed T1 under the box still in the slot). GLM's reading looks right, and it does not change the conclusion either way — the fix improved this path but left it leaking. The second store-back needs to retire B1, or the intermediate publish must not allocate a retaining box.

Verified here: the two store_back_value calls on the same SSA value with no retirement between them, and that emit_box_current_value_as_mixed (retains) and emit_box_current_owned_value_as_mixed (transfers) are genuinely different helpers (codegen_support/value_boxing.rs:83 and :228).

2. Doubled `prepare_consuming_storeback` in the new hash-element reference lowering — major

src/codegen/lower_inst/arrays.rs:503 / :514 (AArch64) and :553 / :565 (x86_64)

Found by DeepSeek v4.1. All of lower_hash_elem_ref_cell_* is new in this PR (the whole span is + lines), and both architectures have the identical shape.

On the create_missing miss path, the receiver is prepared twice with a publication in between:

receiver.prepare_consuming_storeback(ctx, hash)?;   // :503 — drops the slot's box
... __rt_hash_to_mixed ...
receiver.store_back_container_writeback(ctx, hash)?; // :507 — publishes a new box into the slot
... __rt_hash_get, key missed ...
if create_missing {
    receiver.prepare_consuming_storeback(ctx, hash)?; // :514 — drops the box published at :507

prepare_consuming_storeback on a Local receiver is release_mutated_source_local_owner, whose guard (context.rs:929) tests the value's static EIR type, which store_result_value does not change — so both calls pass the guard and both emit emit_owned_local_cleanup. If the box at :507 took the container's owner (value_can_own_mixed_box_source returns true for a LoadLocal from a Mixed slot), the second cleanup frees it and with it the hash, and the __rt_hash_set at :524 then writes into freed storage. If the box retained instead, it is a leaked container reference.

Trigger shape: $a["k"] used as a reference target with the key absent, $a in a Mixed-widened local.

Verified here: the call sequence on both arches, and that the whole span is newly added by this PR.

3. The receiver retain is emitted before the user comparator runs, so a throwing comparator leaks it — minor

src/codegen/lower_inst/builtins/arrays/sort_dispatch.rs:278-280 vs the sort_helper call at :312

Found by GLM 5.3. lower_user_sort_static_callback does prepare_consuming_storeback → ensure_unique_sort_source → store_back_value (the +1 retain) and only then calls the helper that invokes the PHP callback. If the comparator throws, the EIR release v sitting after the call never executes. pin_throwing_builtin_operands deliberately excludes by-reference parameters from unwind pinning (ir_lower/expr/call_operand_owners.rs:843 — "caller storage the builtin writes back through, never a temporary this path releases"), so nothing drops that reference on the unwind path: one leaked container reference per throw.

Pre-PR this path was balanced on unwind, because the owner transfer left no extra reference to lose. Non-callback sorts cannot throw, and array_multisort keeps the transfer, so a throwing usort/uasort/uksort comparator is the realistic trigger.

Verified here: the ordering (store-back at :280 precedes the comparator-invoking call). The unwind conclusion rests on GLM's reading of the pin exclusion, which was not independently re-derived.

4. `value_is_released_later` is path-insensitive and inconsistent with its neighbour — robustness, not a proven defect

src/codegen/context.rs:1030

Raised by Claude Opus 5 and Kimi K3. The predicate that decides "retain exactly when a later instruction releases the value" is a flat scan of function.instructions, and it differs from value_can_transfer_ownership_to_consumer (context.rs:1362, ~330 lines below, answering a near-identical question) on two axes:

window — the new one skips to current_inst + 1, the existing one scans the whole function;
operand match — the new one requires operands == [value], the existing one operands.first() == Some(value).

Neither is live today: release_if_owned emits Op::Release with exactly one operand (ir_lower/ownership.rs:70-83), and the release does follow the call in table order. Kimi also traced that ReleaseUnlessAliases is only emitted for nullable method-call results, never for builtin receivers, so the dangerous direction (release runs, retain skipped) has no current producer — the failure direction here is leak-only.

But the invariant this fix rests on is "an Op::Release in the table will execute". Nothing enforces it: a Release in a conditionally-executed block, or on a path an optimization pass relocates, still flips the retain on. Kimi found one already-live instance of the mismatch: lower_release skips the decref when !ownership.may_require_release() (lower_inst/ownership.rs:144), so an EIR Release that codegen no-ops leaves the retained reference with nothing to balance it — one Mixed-box reference per such call.

Worth making the two predicates share one implementation, and worth a comment stating the assumption explicitly, since findings 1 and 3 are both consequences of it being weaker than it reads.

GLM checked the index arithmetic itself and found it correct (InstId::as_raw() is the zero-based table index), while flagging the same optimization-pass caveat.

5. The eval shard runner can silently drop tests and still report green — minor, latent

scripts/ci/run_eval_codegen_shard.sh:50-77 and :136-138

Found by Claude Opus 5. The script's own comment gets the constraint right — "libtest's --skip filters are substring matches" — but the grouping that acts on it only handles prefixes separated by an underscore:

if [[ $test_name == "$candidate"_* ]]; then

A substring relation without that underscore is not grouped. If codegen::eval::…::test_foo and …::test_foobar land in different shards, --skip …test_foo in test_foobar's shard skips test_foobar too, and it runs in no shard at all. Nothing catches that: when pass 1 exits 0 the script exits 0 (:136-138) without ever checking that the number of tests reported ok/ignored matches ${#selected[@]}; the remaining reconciliation only runs when pass 1 already failed.

This is latent, not live — I enumerated the 1142 eval test names across the 7 modules and found 13 substring pairs, all currently _-separated and therefore correctly grouped. It becomes real the first time someone adds test_foo2 next to test_foo, and it fails silently and green, which is the worst way for a CI guard to fail. A count assertion after pass 1 is a two-line fix and closes it regardless of the naming question.

Minor, same file: if the subshell has not written pass1.pid within the fixed sleep 1 (:94-95), test_pid is empty, the watchdog kills nothing and wait "$tee_pid" blocks — the hang the watchdog exists to bound.

6. `ReceiverPlace::Global`'s doc contradicts the code it documents — minor

src/codegen/lower_inst/receiver_place.rs:42-50 vs :260-290

Found by DeepSeek v4.1. The variant doc says "The symbol holds one boxed Mixed cell and owns it", but emit_global_receiver_store_back loads word 0, compares it against the raw container pointer and stores the raw container pointer back — no box is involved. Self-consistent today only because the element-write paths reject a Mixed receiver before reaching it. Given that this is the function the $_GET double-free fix turns on, the comment is worth correcting.

Checked and found clean

Reported so the diff-reading effort is not repeated:

Fix 1, web handler double free. store_container_writeback_to_local publishes only; emit_global_receiver_store_back's same-pointer compare-and-skip is correct on both arches and the Retire/Keep split matches the two helper conventions. Register allocation across that sequence was checked on all four arch × PIC combinations: emit_store_reg_to_symbol picks x9/r11 as its address scratch and never clobbers the old_reg the retire depends on.
Fix 2, by-ref variadic. The RefCell arm of prepare_consuming_storeback (separate incref'd helper owner) pairs with the StoreRefCell retirement; the publish-only conversion path is consistent with the aarch64 fix as described.
Fix 3, destructor resurrection. The bit-31 guard, the and isolating real owners, and the kind bit 17 completion mark were compared instruction by instruction between the AArch64 and x86_64 arms of object_free_deep.rs — they agree, the immediates are encodable on both, and the x86 and/jz correctly relies on the flags the and sets.
array_multisort's owner transfer. All three model reviewers independently confirmed it is properly gated: EIR suppresses the receiver releases via builtin_consumes_mutating_ref_operand, so value_is_released_later is false and the transfer matches the backend's lease consumption, including the duplicate detached-lease retirement for aliased receivers.
Single-store-back mutating builtins — indexed sort/rsort, shuffle, assoc array_pop, the boxed pop/shift/sort family, the hash link sorts: each publishes exactly once, so the new retain is balanced by the single EIR release. Finding 1 is specifically about the one member of the family that publishes twice.
Resource inventory, GC control runtime, error/exception handler install-restore on the interpreter side: append-only nodes with weak-cell invalidation before destruction, reset skipping standard fds, the syscall close mapping, and the owned-vs-borrowed asymmetry between the native and context handler paths (which is intentional — the native path hands back an owned handle, the context path a borrowed one that is retained).

Not covered

Backtrace lowering across the eval boundary, the eval bridge hooks, the checker relaxations in call_validation.rs beyond a reading of by_ref_argument_can_widen_local_to_mixed, per-builtin interpreter/AOT parity for all 59 entries, and the 2100 generated doc files.

The recommendation is to land after finding 1 is resolved — it is a regression in a path the PR touches, in the exact shape the PR set out to fix, and two independent reviewers reached it separately. Findings 2 and 3 deserve a heap-debug check on the named repro shapes before release; 4, 5 and 6 are safe follow-ups.

nahime0 force-pushed the feat/core-align branch from f8d0dcb to c738392 Compare September 5, 2026 20:58

nahime0 requested review from Guikingone and removed request for Guikingone September 7, 2026 09:45

nahime0 marked this pull request as draft September 7, 2026 10:10

nahime0 force-pushed the feat/core-align branch from 551d0ef to 9c4d101 Compare September 7, 2026 11:57

nahime0 changed the title ~~feat(core): complete AOT and eval builtin parity~~ feat(core): align AOT and eval Core builtin support Sep 7, 2026

nahime0 force-pushed the feat/core-align branch from ef4b353 to 743e73e Compare September 7, 2026 18:22

nahime0 force-pushed the feat/core-align branch 5 times, most recently from dd174c5 to 9094735 Compare September 11, 2026 21:47

nahime0 added 20 commits September 16, 2026 10:06

fix(ownership): keep callable unboxing in the owning-temporary rule

e9ec56b

Widening that rule to refcounted payloads dropped `Callable`, which `PhpType::is_refcounted` deliberately excludes, so an extracted descriptor lost its published root slot. Name both shapes.

fix(ir_lower): cast runtime property names to string

123bb29

An eval() in scope widens every local to Mixed, so $o->{$name} reached the backend ladders boxed and was refused. The get, set and unset lowerings cast a non-string name first, as php does.

This was referenced Sep 17, 2026

Direct call to a by-ref variadic leaks one block per call when the arguments are positional only #1062

Open

Closure::bind of an eval-declared closure from AOT leaks five blocks per bind #1063

Open

nahime0 mentioned this pull request Sep 17, 2026

AOT ReflectionClass::getMethods() followed by eval() reflection on the same class hangs forever #1031

Open

4 tasks

nahime0 marked this pull request as ready for review September 17, 2026 13:30

nahime0 requested a review from Guikingone September 17, 2026 13:31

greptile-apps Bot reviewed Sep 17, 2026

View reviewed changes

Comment thread scripts/ci/run_eval_codegen_shard.sh Outdated

Navigation Menu

Uh oh!

feat(core): align AOT and eval Core builtin support - #893

feat(core): align AOT and eval Core builtin support#893
nahime0 wants to merge 714 commits into
mainfrom
feat/core-align

nahime0 commented Sep 4, 2026 •

edited

Loading

Uh oh!

nahime0 commented Sep 7, 2026

Uh oh!

greptile-apps Bot commented Sep 17, 2026 •

edited

Loading

Uh oh!

Uh oh!

Guikingone commented Sep 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

nahime0 commented Sep 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Deliberate bounds

CI stabilization

Validation

Closed issues

Known follow-ups (not in this PR)

Uh oh!

nahime0 commented Sep 7, 2026

Uh oh!

greptile-apps Bot commented Sep 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Confidence Score: 5/5

Summary

Uh oh!

Uh oh!

Guikingone commented Sep 17, 2026

Consolidated review — 4 independent reviewers

1. sort() / rsort() on a hash in a Mixed-widened local leaks the pre-rebuild table — major

2. Doubled prepare_consuming_storeback in the new hash-element reference lowering — major

3. The receiver retain is emitted before the user comparator runs, so a throwing comparator leaks it — minor

4. value_is_released_later is path-insensitive and inconsistent with its neighbour — robustness, not a proven defect

5. The eval shard runner can silently drop tests and still report green — minor, latent

6. ReceiverPlace::Global's doc contradicts the code it documents — minor

Checked and found clean

Not covered

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

nahime0 commented Sep 4, 2026 •

edited

Loading

greptile-apps Bot commented Sep 17, 2026 •

edited

Loading

1. `sort()` / `rsort()` on a hash in a Mixed-widened local leaks the pre-rebuild table — major

2. Doubled `prepare_consuming_storeback` in the new hash-element reference lowering — major

4. `value_is_released_later` is path-insensitive and inconsistent with its neighbour — robustness, not a proven defect

6. `ReceiverPlace::Global`'s doc contradicts the code it documents — minor