[GIT-PULLS] [php-src] PR #22824: Zend: handle IS_UNDEF in the generic conversion helpers that still lack it
[email protected] (brzuchal) Mon, 20 Jul 2026 10:12:03 +0000
| Newsgroups | php.git-pulls |
|---|---|
| Message-ID | <[email protected]> |
Pull Request: https://github.com/php/php-src/pull/22824 Author: brzuchal ### Up front: no userland reproducer This is a consistency fix and I could not construct a PHP-level trigger: every VM handler normalizes `IS_UNDEF` to `IS_NULL` before calling these helpers. I would rather state that than have it asked. The argument is the asymmetry below plus the fact that this exact bug class has already been fixed twice upstream, both times by adding the missing `case`. If the view is that a reproducer is required, I am happy for this to be closed. ### The asymmetry These are `ZEND_API` entry points taking an arbitrary zval, and `IS_UNDEF` is a tag internal code legitimately holds, most obviously from an object property table, where uninitialized typed properties are `IS_UNDEF`. Four helpers account for it, five do not: | handles `IS_UNDEF` | does not | |---|---| | `zendi_try_get_long()` | `_zendi_try_convert_scalar_to_number()` | | `zval_get_long_func()` | `zval_get_double_func()` | | `__zval_get_string_func()` | `convert_to_long()` | | `_convert_to_string()` | `convert_to_double()` | | | `convert_to_boolean()` | `zval_get_long(undef)` returns `0`; `zval_get_double(undef)` reaches `ZEND_UNREACHABLE()`, which is an assertion failure in a debug build and undefined behaviour in a release build. Sibling APIs, same contract, and the difference does not look designed. ### Why it looks like this Two point fixes, each scoped to the single function that had been reported: - [`2b383848a73`](https://github.com/php/php-src/commit/2b383848a738eda02bc0f5bf116b021a6e42c24a) - *Fix handling of references in `zval_try_get_long()`* ([#18761](https://github.com/php/php-src/pull/18761)), adding `IS_REFERENCE`. Its commit message is the precedent: "This API can't handle references, yet everyone keeps forgetting that it can't and that you should DEREF upfront." - [`9e1b285f65a`](https://github.com/php/php-src/commit/9e1b285f65a5df3fa5366e6c8241152a4ceb88d4) - *Fix GH-22142: Assertion failure in `zendi_try_get_long()` on IS_UNDEF* ([#22142](https://github.com/php/php-src/issues/22142), fixed by [#22143](https://github.com/php/php-src/pull/22143)), found by fuzzing, adding `IS_UNDEF`. Neither time were the neighbouring helpers revisited. This finishes that sweep. ### What it does Adds `case IS_UNDEF:` to five helpers, each joining an arm that already exists: `IS_UNDEF` behaves as `IS_NULL` for the cast helpers, and yields a normal `TypeError` for the operator helper, matching the bitwise operators, which are already safe through `zendi_try_get_long()`. ### What it deliberately does not do `increment_function()` and `decrement_function()` are left alone. `IS_UNDEF` reaching them is a genuine invariant violation, not an input to coerce: every VM handler normalizes it before the call (`zend_vm_def.h`), and the JIT asserts the precondition outright (`ZEND_ASSERT(Z_TYPE_P(var_ptr) != IS_UNDEF)`, four sites in `zend_jit_helpers.c`). Accepting it there would weaken a check another subsystem relies on. No catch-all `default:` is added either. `IS_CONSTANT_AST`, `IS_INDIRECT`, `IS_PTR`, `IS_ALIAS_PTR` and `_IS_ERROR` indicate a caller bug and must keep failing loudly. ### Related Companion fix, same bug class at a call site rather than in the helper: [#22822](https://github.com/php/php-src/pull/22822). Independent, either can merge first. ### Behaviour and tests No behaviour change for any tag that works today. `Zend/tests`, `ext/date/tests`, `ext/zlib/tests`: 6039 pass, no failures attributable to this change. No test is added: without a userland path reaching these helpers with `IS_UNDEF`, the only way to write one would be a test-only API for fabricating zval tags, which seems a worse trade than shipping the fix untested.