Skip to content

ext/standard: Skip mbrlen() for ASCII bytes in php_mblen() - #24207

Open
ArtUkrainskiy wants to merge 2 commits into
php:masterfrom
ArtUkrainskiy:php-mblen-ascii
Open

ArtUkrainskiy wants to merge 2 commits into
php:masterfrom
ArtUkrainskiy:php-mblen-ascii

Conversation

@ArtUkrainskiy

@ArtUkrainskiy ArtUkrainskiy commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

fgetcsv(), str_getcsv(), escapeshellarg() and escapeshellcmd() call php_mblen() while scanning their input; for ASCII input this means one call per byte. With glibc this goes through mbrlen() into gconv, accounting for about 80% of the instructions in a 10-column fgetcsv() loop (callgrind).

When Zend classifies the current locale as using ASCII characters as singletons (CG(ascii_compatible_locale), the same flag php_basename() already relies on), a non-NUL byte below 0x80 is one character and can be counted without calling mbrlen().

php_mb_reset(), which callers run at the start of a string, records that flag; escapeshellarg() and escapeshellcmd() now call it too, like fgetcsv() and basename(). Other locales continue through mbrlen() with conversion state preserved across the string, as ZTS builds already did; NTS builds previously used mblen() and now share the same explicit-state path.

Some glibc converters (BIG5-HKSCS, CP1255, JIS X 0213, TSCII, ...) can flush a buffered character without consuming the current input byte, causing mbrlen() to return 0 for a non-NUL byte. In that case php_mblen() calls mbrlen() again with the updated state and the same input byte, so the caller gets a consumed length and can make progress.

Benchmark

Release build, ns per call:

Benchmark master this
fgetcsv(), 10 columns 4,217 515
fgetcsv(), 50 columns 20,775 2,241
str_getcsv(), 10 columns 4,131 458
escapeshellarg(), 24-byte path 310 48
escapeshellarg(), 128 bytes, ja_JP.SJIS (old path) 1,400 1,368

Differential testing

I compared every caller against master NTS:

  • str_getcsv() with three dialects
  • fgetcsv()
  • escapeshellarg()
  • escapeshellcmd()
  • basename()
  • pathinfo()

The test used ~6,500 inputs mixing lead bytes, ASCII-special bytes in trail-byte positions, and lone high bytes across 19 glibc locales, including C, UTF-8, Shift_JIS, GBK, GB18030, EUC-JP/KR/TW, Big5, Big5-HKSCS, CP1255, JIS X 0213, TSCII and IBM1047, built with localedef.

16/19 locales produced identical output in both the new NTS and ZTS builds.

The three differences are explainable:

  • TCVN5712-1 and CP1258: glibc's decoders can buffer an ASCII letter and combine it with a following tone mark. On master NTS this can make e.g. a| appear as one multibyte character, allowing | to pass through escapeshellcmd(). CP1258 is classified by Zend as a single-byte locale, so the ASCII letter is now counted immediately and | is escaped. For TCVN, NTS now preserves conversion state across the string, matching the behavior the ZTS path already had.
  • IBM1047/EBCDIC: this is single-byte and therefore falls under Zend's existing ascii_compatible_locale classification, even though the encoding itself is not ASCII-compatible (localedef warns about this). Bytes below 0x80 that glibc previously reported as invalid are consequently counted as single bytes by the fast path.

ext/standard and ext/spl tests pass on both NTS and ZTS builds.

Benchmark and differential-test scripts:

https://github.com/ArtUkrainskiy/php-src-bench/tree/main/reports/fgetcsv-mblen

UPD (Oct 9): the zero-consumption retry is now one retry from the initial state (the same-state retry would spin on ESC ( B NUL in ISO-2022-JP); UPGRADING/UPGRADING.INTERNALS added. Separate commit, will squash on request.

fgetcsv(), str_getcsv(), escapeshellarg() and escapeshellcmd() call
php_mblen() on every byte, and with glibc mbrlen() is most of their
time. When the locale uses ASCII characters as singletons
(CG(ascii_compatible_locale)), a byte below 0x80 is one character and
is counted without the call. php_mb_reset() records that at the start
of a string; escapeshellarg() and escapeshellcmd() now call it like the
other callers do. Other locales keep going through mbrlen() with the
state held across the string, and a decoder that flushes a buffered
character without consuming input is called again for the same byte.

About 8x on fgetcsv() and 6x on escapeshellarg() with ASCII input.
@ArtUkrainskiy
ArtUkrainskiy marked this pull request as ready for review October 8, 2026 22:28
@ArtUkrainskiy
ArtUkrainskiy requested a review from bukka as a code owner October 8, 2026 22:28

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

How is this going to work str_getcsv("\x1b(B\0", ...) in ISO-2022-JP?
Please add UPGRADING.

Retrying mbrlen() with the same state loops on a shift sequence
followed by NUL in a stateful encoding; decode the byte from the
initial state instead, which ends after one retry.
@ArtUkrainskiy

Copy link
Copy Markdown
Contributor Author

@LamentXU123 Fixed in 05e9146.

What was wrong: for a decoder that flushes a buffered character without consuming input, php_mblen() retried mbrlen() with the same state until it consumed something. In an ISO-2022 locale ESC ( B NUL returns 0 on every call, so that loop never ends. (The fast path itself is off there, ascii_compatible_locale is false.)

What it is now: one retry from the initial state. A flushed character leaves nothing behind, and a 0 from the initial state means a NUL, so the call ends after two mbrlen() and returns 0 — the end of the string for every caller, which is what master does on that input.

Alternative I considered: keep the same-state retry but bound it (glibc's TSCII flushes up to three characters per byte). Rejected: a magic number in generic code, and if some decoder buffered more it would truncate the string; the reset has no such limit. Downside of the reset: a decoder that both keeps a shift state and flushes without consuming would lose that state — no libc has one.

Checked: 19-locale differential run unchanged (HKSCS/CP1255/JIS X 0213/TSCII still identical to master NTS), a stub decoder that always returns 0 ends after two calls. No ISO-2022 LC_CTYPE available here (only NetBSD ships one), so that case is reasoned, not run.

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