Skip to content

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

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

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

break;
}
case IS_UNDEF:
zend_jit_undefined_op_helper(EG(current_execute_data)->opline->op2.var);

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 undefined-variable warning here can free str the same way: with global $s; return $s[$undef] ?? 1; and an error handler doing $GLOBALS['s'] = null, function JIT then reads the freed string (valgrind; PHP-8.4 has it without this PR too). Could the new guard cover this case as well, like the VM's IS_UNDEF branch does? The VM's own trailing-data warning in zend_fetch_dimension_address_read() has no guard either and shows the same invalid read without opcache.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I created separate PR for this bug fix: #25329

It's valid bug, but I would like to keep all fixes separately. It's better for clearer git history.

@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
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