Skip to content

fix(qwen36): a truncated multibyte tail made the tokenizer read past the prompt - #1557

Merged
JustVugg merged 1 commit into
JustVugg:devfrom
Ti-03:fix/qwen36-truncated-utf8
Sep 17, 2026
Merged

JustVugg merged 1 commit into
JustVugg:devfrom
Ti-03:fix/qwen36-truncated-utf8

Conversation

@Ti-03

@Ti-03 Ti-03 commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Summary

serve_read_req reads exactly plen bytes into malloc(plen+1) and serve_one hands them to encode_text as 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_decode read s[i] without bounding i, and reported the lead byte's full length even when the continuation bytes were absent. utf8_adv called it with n = 0x7fffffff, which disabled the one bound that was there. pretok_end then returned a piece end past the prompt, and encode_text read 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.

qwen36 and qwen38 carry the same pre-tokenizer. qwen38's copy already treats a truncated sequence as one invalid unit, and tests/test_qwen38_tokenizer.c gates 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_decode verbatim, plus passing the real length to utf8_adv and clamping the piece end in encode_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 at dev (87c54a9) for comparison: both pass, so this adds no failures.
  • CUDA changes were tested with make -C c cuda-test (if applicable) — n/a, no CUDA change
  • Performance claims include hardware, commands, and repeatable measurements — n/a, no performance claim
  • Performance claims include a validated experiment manifest with raw evidence — n/a

Out-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 lone 0xE2 lead with n = 1:

UNPATCHED (dev @ 87c54a9)
  ERROR: AddressSanitizer: stack-buffer-overflow
  READ of size 1
      #0 utf8_decode qwen36.c:153
      #1 pretok_end  qwen36.c:206
  'lead3' is a 2-byte object; the access is at offset 35 in a [32, 34) frame

PATCHED
  utf8_decode -> 0xe2   (qwen38 requires 0xe2)
  adv         -> 1      (qwen38 requires 1)
  pretok_end  -> 1      (qwen38 requires 1)
  VERDICT: PASS

No change on well-formed input. Every (codepoint, advance) tuple and every pretok_end boundary over a 20-entry corpus (ASCII, contractions, Latin-1, Cyrillic, CJK, kana, 4-byte emoji, URLs, JSON, whitespace) is byte-identical to dev: 452 lines, zero differences.

Note on the new test. tests/test_qwen36_tok_truncated.c calls utf8_adv with the length argument this change adds, so it does not compile against unpatched dev and 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

  • The default CPU build remains dependency-free
  • No model files, generated binaries, or benchmark artifacts are included

utf8_adv gains a length parameter. It is static in qwen36.c and has exactly two references, its definition and one call in encode_text, both updated here. qwen38.c has its own independent static copy and is untouched.

Edo771977 added a commit to Edo771977/colibri that referenced this pull request Sep 16, 2026
@JustVugg
JustVugg merged commit 033eb61 into JustVugg:dev Sep 17, 2026
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants