Skip to content

JIT: Fix FETCH_DIM_IS ignoring leading-numeric string offsets - #25298

Closed
jorgsowa wants to merge 2 commits into
php:PHP-8.4from
jorgsowa:fix-jit-fetch-dim-is-string-offset
Closed

jorgsowa wants to merge 2 commits into
php:PHP-8.4from
jorgsowa:fix-jit-fetch-dim-is-string-offset

Conversation

@jorgsowa

@jorgsowa jorgsowa commented Nov 7, 2026

Copy link
Copy Markdown
Contributor

For a leading-numeric string offset such as "1x", the VM warns Illegal string offset "1x" and uses the leading number, also under ??:

function f(string $s) { return $s["1x"] ?? "default"; }
var_dump(f("abc"));
// VM:  Warning: Illegal string offset "1x" + string(1) "b"
// JIT: string(7) "default", no warning

This is the behaviour specified by the Saner Numeric Strings RFC and pinned in Zend/tests/offsets/string_container_offset_behaviour.phpt. The RFC change (b224878) updated the JIT's R helper but not zend_jit_fetch_dim_str_is_helper, which kept a strict parse and silently returned null. The IS helper now parses the offset like the VM, warns on trailing data, and holds a reference to the string in case the error handler releases it.

zend_may_throw_ex() did not count this warning for FETCH_DIM_IS either. With ?? this has no effect, because COALESCE always consumes the result. The inner fetch of a nested isset($s["1x"][0]) / empty() is different:

  • DCE removed it when the result was unused, losing the warning even without JIT.
  • Function JIT skipped saving the call chain around it, leaking already-sent arguments when the error handler throws.
Code VM (no opcache) opcache, JIT off opcache, function JIT
$s["1x"] ?? "d" warning, "b" warning, "b" before: "d", no warning
after: warning, "b"
$x = isset($s["1x"][0]); (result used) warning, true warning, true before: false, no warning
after: warning, true
isset($s["1x"][0]); (result unused) warning before: no warning
after: warning
before: no warning
after: warning
consume($t, isset($s["1x"][0])) with a throwing error handler exception exception before: exception plus a leaked $t
after: exception

Comment thread ext/opcache/jit/zend_jit_helpers.c Resolved

@iliaal iliaal left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new refcount guard has no test: with a handler doing $GLOBALS['s'] = null, $s["1x"] ?? "default" under function JIT reads the freed string without it.

For $str["1x"] ?? $default, the VM warns "Illegal string offset" and
uses the leading number, while the JIT helper returned null silently.
zend_may_throw_ex() did not count this warning either, so DCE removed
the inner fetch of an unused nested isset()/empty() and the JIT skipped
saving the call chain around it.
@jorgsowa
jorgsowa force-pushed the fix-jit-fetch-dim-is-string-offset branch from 04d6025 to 0cf874c Compare November 9, 2026 18:45
@jorgsowa jorgsowa closed this in e8714b0 Nov 9, 2026
@jorgsowa

jorgsowa commented Nov 9, 2026

Copy link
Copy Markdown
Contributor Author

I have merged PR along with new test that you mentioned about fetch_dim_is_string_offset_004.phpt

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants