fix(qwen36): a truncated multibyte tail made the tokenizer read past the prompt - #1557
Merged
Merged
Conversation
Edo771977
added a commit
to Edo771977/colibri
that referenced
this pull request
Sep 16, 2026
Integrazione dev (riuso KV Qwen3.6 JustVugg#1553) + JustVugg#1552 Qwen3.8 + JustVugg#1557 + JustVugg#1549
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
serve_read_reqreads exactlyplenbytes intomalloc(plen+1)andserve_onehands them toencode_textas raw prompt text. Nothing between the socket and the tokenizer checks that those bytes are well-formed UTF-8, so a prompt whose tail is a truncated multibyte sequence reaches the pre-tokenizer with a lead byte whose continuation bytes are not there.utf8_decodereads[i]without boundingi, and reported the lead byte's full length even when the continuation bytes were absent.utf8_advcalled it withn = 0x7fffffff, which disabled the one bound that was there.pretok_endthen returned a piece end past the prompt, andencode_textread that many bytes out of the allocation.Under a sanitizer that is an out-of-bounds read. Without one the engine silently tokenizes whatever follows the prompt in memory, so the model is fed a token stream that nobody can see is wrong.
qwen36andqwen38carry the same pre-tokenizer. qwen38's copy already treats a truncated sequence as one invalid unit, andtests/test_qwen38_tokenizer.cgates it under the comment "A serving payload is byte-counted and may end in a truncated multibyte sequence. Treat that byte as one invalid unit without reading past it." qwen36's copy never got that port. This change makes the two agree and adds the same gate here.The fix is qwen38's
utf8_decodeverbatim, plus passing the real length toutf8_advand clamping the piece end inencode_text. Six lines of logic; the rest is the test and its build rule.Validation
make -C c check— exit 0,OK (skipped=85), macOS arm64 (clang + libomp). Run in a clean worktree at this branch, and again atdev(87c54a9) for comparison: both pass, so this adds no failures.make -C c cuda-test(if applicable) — n/a, no CUDA changeOut-of-bounds read, before and after. Compiling the same 12-line probe against each tree with
-fsanitize=address,undefined, using only signatures that are identical on both sides, and asserting qwen38's own three values for a lone0xE2lead withn = 1:No change on well-formed input. Every
(codepoint, advance)tuple and everypretok_endboundary over a 20-entry corpus (ASCII, contractions, Latin-1, Cyrillic, CJK, kana, 4-byte emoji, URLs, JSON, whitespace) is byte-identical todev: 452 lines, zero differences.Note on the new test.
tests/test_qwen36_tok_truncated.ccallsutf8_advwith the length argument this change adds, so it does not compile against unpatcheddevand is not by itself a fail-before/pass-after control. The sanitizer probe above is the before/after evidence. The test exists to keep the behaviour pinned going forward, and it carries qwen38's assertions so the two engines cannot drift apart again.Compatibility
utf8_advgains a length parameter. It isstaticinqwen36.cand has exactly two references, its definition and one call inencode_text, both updated here.qwen38.chas its own independentstaticcopy and is untouched.