Skip to content

Let a JIT'd isset() reach __isset() on an unset property - #33

Open
EdmondDantes wants to merge 1 commit into
true-asyncfrom
223-jit-fetch-obj-is-undef
Open

Let a JIT'd isset() reach __isset() on an unset property#33
EdmondDantes wants to merge 1 commit into
true-asyncfrom
223-jit-fetch-obj-is-undef

Conversation

@EdmondDantes

Copy link
Copy Markdown

Fixes true-async/php-async#223.

What answers wrongly

isset($obj->prop[$key]) returns false for a key the array holds, when prop is a
declared property removed by unset() and served by &__get()/__isset(), and the
tracing JIT has compiled the accessor. opcache.jit=function and no JIT answer
correctly. Found in Laravel's Blade under laravel-spawn: the view factory keeps its
per-request render state exactly this way, so @once blocks were emitted twice under
concurrent renders.

Neither coroutines nor fibers are needed. ext/opcache/tests/jit/fetch_obj_is_unset_prop.phpt,
added here, is 40 lines of plain PHP and fails on every run before this change.

Why

With a known prop_info the JIT reads the property slot at its offset and, on a hot
trace, defers the IS_UNDEF check to the result type guard that follows. That guard
admits IS_UNDEF only for ZEND_FETCH_OBJ_IS with a NULL result, and its
deoptimization exits at opline + 1 with the slot copied into the result rather than
re-running the fetch. A property served by __get() traces to the type the handler
returns, so an IS_UNDEF slot fails the guard, the deoptimization answers from the
empty slot, and the handler is never called. The trace dump shows both halves:

0001 #2.T2 [!rc1, rcn, array of [any, ref]] = FETCH_OBJ_IS THIS string("marks") ; val(undef)
...
---- TRACE 4 TSSA start (side trace 3/1) Holder::has()
0002 #3.T1 [!false] = ISSET_ISEMPTY_DIM_OBJ (isset) #2.T2 [!null] #0.CV0($key)

The recorded slot is undef while the inferred result is array; the side trace picks
up at ISSET_ISEMPTY_DIM_OBJ with the value already turned into null.

The change

Emit the IS_UNDEF guard before the fetch where the result type guard cannot stand in
for it, which is ZEND_FETCH_OBJ_IS with a non-NULL traced result. The guard exits at
opline, so the VM re-runs the fetch and reaches the magic handler.

ZEND_FETCH_OBJ_R defers the same check and its deoptimization has the same shape, but
no script made it answer wrongly here, so its condition is untouched.

Upstream

php/php-src master carries the identical condition at
ext/opcache/jit/zend_jit_ir.c:14534, so this is an upstream defect rather than a fork
regression. Sending it there is a separate step.

Checks

Run from this tree, ZTS debug build:

  • ext/opcache/tests/jit/fetch_obj_is_unset_prop.phpt — fails on the same tree with this
    commit reverted and the JIT rebuilt, passes with it.
  • ext/opcache/tests/jit — 488 passed, 0 failed.
  • ext/opcache/tests — 908 passed, 1 failed: zend_version.phpt, which fails on the
    reverted build too.
  • Zend/tests with opcache.jit=tracing — 5414 passed, 2 failed:
    arginfo_zpp_mismatch.phpt and arginfo_zpp_mismatch_strict.phpt, both of which fail
    with the JIT off as well.

The tracing JIT knows the offset of a declared property and reads its slot
directly, deferring the IS_UNDEF check to the result type guard that follows.
The guard admits IS_UNDEF only for ZEND_FETCH_OBJ_IS with a NULL result, and
its deoptimization resumes at the next opline with the slot copied into the
result rather than re-running the fetch. A property removed by unset() and
served by __isset()/__get() traces to the type those return, so IS_UNDEF fails
the guard and isset($obj->prop[$key]) answers false for a key the array holds,
with the magic handler never called. Check IS_UNDEF before the fetch where the
guard cannot stand in for it.

FETCH_OBJ_R defers the same check and its deoptimization has the same shape,
but no script made it answer wrongly, so its condition is left alone.

Reported as true-async/php-async#223, where Laravel's Blade emitted an @once
block twice under concurrent renders. Reproduces on upstream master, which
carries the same condition.

php#223
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.

1 participant