Skip to content

Language: preserve bound closure calledClass across CFG edges (#24335) - #24354

Merged
PurHur merged 1 commit into
masterfrom
agent/issue-24335-closure-bind-coalesce
Jul 28, 2026
Merged

Language: preserve bound closure calledClass across CFG edges (#24335)#24354
PurHur merged 1 commit into
masterfrom
agent/issue-24335-closure-bind-coalesce

Conversation

@PurHur

@PurHur PurHur commented Jul 28, 2026

Copy link
Copy Markdown
Owner

Summary

  • Block::getFrame() now copies calledClass onto CFG branch frames (?? / if / try), matching how closureCall was already preserved.
  • Top-level Closure::bind(..., A::class) lost bound scope on those edges, so A::$private ?? $default (and if/try private static reads) threw while a single-block direct read worked.
  • Methods were unaffected because visibility falls back to func->class; bound top-level closures only have calledClass.

php-src: Zend/zend_closures.c (bind scope) + coalesce/static-prop fetch under bound scope.

Test plan

  • Issue repro (VM):
./script/docker-exec.sh -- bash -lc 'php bin/vm.php test/repro/issue_24335_closure_bind_coalesce.php'
# coalesce=7 / direct=7 / if=7 / try=7
  • Zend matches for those four lines.
  • Unit: vendor/bin/phpunit --filter testVmBoundClosurePrivateStaticCoalesceAndIf test/unit/ClosureCompileTest.php — OK
  • Full ClosureCompileTest + ClosureBindInternalClassVMTest — OK
  • JIT/AOT: pre-existing segfault on master for even Closure::bind + private static direct read (php bin/jit.php exit 139) — not introduced by this change; out of scope for this VM frame fix.

Pillar 1 snapshot (pre-claim): inventory OK 6614/6614; north-star5-fast ok; user_release_ready: no on helper-runtime fingerprint (#24302, claimed elsewhere).

Closes #24335

Made with Cursor

Closure::bind scope was lost on ?? / if / try branch frames, so private
static reads threw even when direct single-block access worked.

Co-authored-by: Cursor <cursoragent@cursor.com>
@PurHur
PurHur merged commit 84565e3 into master Jul 28, 2026
1 check passed
@PurHur
PurHur deleted the agent/issue-24335-closure-bind-coalesce branch July 28, 2026 17:11
PurHur added a commit that referenced this pull request Jul 28, 2026
#24354)

Closure::bind scope was lost on ?? / if / try branch frames, so private
static reads threw even when direct single-block access worked.

Co-authored-by: PurHur <PurHur@users.noreply.github.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
PurHur added a commit that referenced this pull request Jul 28, 2026
* json_decode: native HT runtime materialization for assoc arrays (#24137)

Replace int-wire JsonDecodeJitHelper with tag-dispatched native __hashtable__*
building via phpc_native_ht_* (ParseStr #13827 shape), wired through a single
__compiler_json_decode bridge with ensureNativeHtInternalProxies before NestedJIT.

AOT green for runtime literal JSON strings; json_encode→decode roundtrip still
blocked (encode boxes __string__* in __value__* — separate issue path).

Co-authored-by: Cursor <cursoragent@cursor.com>

* json_decode: raise shrink line budget; add encode→decode AOT diag (#24137)

StringJsonDecode grew to 409 lines with native HT materialization; bump the
shrink ceiling. Add a small AOT diag repro for the NestedJIT heap-string
param gap that still blocks json_encode→json_decode.

Co-authored-by: Cursor <cursoragent@cursor.com>

* AOT json_decode: box assoc HT in __value__* (#24137)

Raw __hashtable__* cast to __value__* was misread as int/NULL after
NestedJIT native HT fill. Peer #24298 boxHashtableValue shape.

Co-authored-by: Cursor <cursoragent@cursor.com>

* Foundation: refresh helper-runtime prelink (35 stale → 0) (#24336) (#24349)

DOM and introspection helper units drifted after #24321; release-readiness
--strict failed with 258 fresh / 35 stale. Re-emit via --prelink in the
pinned Docker env so the committed cache matches live fingerprints again.

Co-authored-by: PurHur <PurHur@users.noreply.github.com>
Co-authored-by: Cursor <cursoragent@cursor.com>

* DOM: getNamedItem matches Attr local name, not QName (#24332) (#24350)

php-src namednodemap.c uses xmlHasProp (local name) for attribute maps.
Wire JIT/AOT instance-method proxies for getNamedItem(NS) so :object
attributes receivers bind the same path.

Co-authored-by: PurHur <PurHur@users.noreply.github.com>
Co-authored-by: Cursor <cursoragent@cursor.com>

* AOT: skip the corpus warmup when the committed prelink cache is current (#24302) (#24351)

Compiling <?php echo "hi\n"; on a clean checkout took ~9 minutes. Second compile: 5 seconds.

warmForUserAotBuild() decided whether to emit the entire helper corpus based on one thing — whether
a marker file exists:

    $marker = self::coreMarkerPath();     // build/helper-runtime-cache/<prefix><fingerprint>.ok
    if (is_file($marker)) { return; }
    ...
    $rc = self::runWarmupCommand($cmd);   // shells out to emit-helper-runtime-object.php

coreMarkerPath() lives under cacheDir() = build/helper-runtime-cache, which is gitignored
(.gitignore:29, 0 files tracked). A clean checkout can therefore NEVER have the marker, so the first
user AOT build re-emitted the whole corpus regardless of whether prelinked/helper-runtime/ was
present and current.

That is also why #24321 (refreshing the committed cache) did not help: it fixes what the CONSUMPTION
path can use, while the warmup decision never looked at it. Measured before 533s, after 517s — ~16s
of ~530.

Found by profiling, not by guessing: a SIGALRM sampler on the real bin/compile.php collected only 5
ticks in 441s, top path HelperRuntimeCache::runWarmupCommand < warmForUserAotBuild. The parent is
blocked on a subprocess, which is why parent-side sampling sees nothing — the near-empty profile was
itself the signal.

Fix: skip the warmup when the committed per-arch cache exists, carries a core_fingerprint equal to
the live one, and actually contains units. The consumption path already reads prelinkedUnitsDir()
alongside unitsDir(), so in that state there is nothing to emit and warming is pure waste.

  first compile, clean checkout:  517s -> 5s

committedCacheIsCurrent() is deliberately conservative: missing manifest, unreadable JSON,
mismatched fingerprint or an empty units directory all fall through to the warmup, so a stale or
partial committed cache is still emitted rather than silently producing a build with missing
helpers.

Gate (no CI on lib/, and AOT is informational in compiler-gate.yml), branch vs a clean master
checkout, run sequentially, both with build/helper-runtime-cache removed first so the measurement is
COLD:

  cold aot-smoke : 8/8 both sides
  VM  --repeat 2 : 110/110 both sides, exit 0
  AOT --repeat 3 : master 16 failing, branch 16 failing
                   regressions (branch-only): NONE
                   fixed (master-only)      : NONE

Note on method: my first attempt at that comparison read master's sweep while it was still running
(73 of 110 cases) and showed four phantom regressions — j05, j06, k02, m02 — purely because those
cases had not been reached yet. The numbers above are from the completed run.

Co-authored-by: PurHur <tedyyyyy@gmail.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Stdlib: ftp_connect Reflection/named args use Zend hostname (#23644) (#24352)

InternalArgInfo still advertises host; override BuiltinParamNames so
Reflection and named hostname:/port:/timeout: match ext/ftp/ftp.stub.php.

Co-authored-by: PurHur <PurHur@users.noreply.github.com>
Co-authored-by: Cursor <cursoragent@cursor.com>

* Language: UnhandledMatchError formats variable subjects (#24329) (#24353)

Snapshot the match subject into a fresh temp before the message helper so
ARG_SEND cannot bind the null match-result seed. Also refuse remapping
unhandled-match helpers / named CVs onto that seed.

Co-authored-by: PurHur <PurHur@users.noreply.github.com>
Co-authored-by: Cursor <cursoragent@cursor.com>

* Language: preserve bound closure calledClass across CFG edges (#24335) (#24354)

Closure::bind scope was lost on ?? / if / try branch frames, so private
static reads threw even when direct single-block access worked.

Co-authored-by: PurHur <PurHur@users.noreply.github.com>
Co-authored-by: Cursor <cursoragent@cursor.com>

* AOT: persist disableRefcount typeinfo store (#24137)

compile{} expand of `$ref.typeinfo = …` discarded insertValue (#24226 sibling),
so disableRefcount was a no-op and NestedJIT heap-string delref still freed
payloads. Hand-store the cleared REFCOUNTED bit; pin json_decode bridge copies.

Co-authored-by: Cursor <cursoragent@cursor.com>

* Stdlib: AOT json_decode assoc via decodeInto + LLVM tag peek (#24137)

NestedJIT cannot use VmJsonFormat or return HT pointers; bridge allocates
the HT and peeks the first JSON byte in LLVM (resultTag ARG_RECV emptied
string formals). Skip NestedJIT string ARG_RECV rebind that UAF'd payload
bytes. Parse into phpc_native_ht_* with ord-based ints and substr slices
for nested arrays.

Co-authored-by: Cursor <cursoragent@cursor.com>

---------

Co-authored-by: PurHur <PurHur@users.noreply.github.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: PurHur <tedyyyyy@gmail.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Regression: Closure::bind scope + private static ?? throws — Zend coalesces (Zend/zend_closures.c)

1 participant