Skip to content

fix: use unsigned compares in wasm SIMD table-lookup - #88

Open
ptweezy wants to merge 1 commit into
nodejs:mainfrom
ptweezy:fix-wasm-simd-unsigned-compare
Open

ptweezy wants to merge 1 commit into
nodejs:mainfrom
ptweezy:fix-wasm-simd-unsigned-compare

Conversation

@ptweezy

@ptweezy ptweezy commented Nov 1, 2026

Copy link
Copy Markdown

Before: the wasm SIMD byte-range test is emitted with signed ops (wasm_i8x16_ge/le). For a range that crosses the 0x7f/0x80 boundary — e.g. [0x0e, 0xff] — wasm_i8x16_le(input, 0xff) compares against 0xff as signed -1, so every byte with the high bit clear fails the test. In a self-looping table-lookup node the vector block then advances zero bytes and falls through to the node's otherwise edge; where otherwise routes back to the same node (as llhttp's header_value_relaxed state does in 9.4.x), the parser spins without consuming input.

After: range comparisons use wasm_u8x16_ge/le. Bytes are unsigned, so crossing ranges match correctly and the vector loop advances. NEON (vcgeq_u8/vcleq_u8) and SSE (_SIDD_UBYTE_OPS) were already unsigned; this aligns wasm with them.

Repro: build llhttp 9.4.3 to wasm with -msimd128, enable llhttp_set_lenient_header_value_relaxed, and parse a ≥16-byte relaxed header value — the SIMD build hangs while the scalar build returns HPE_OK. An exhaustive check over all ranges × all bytes shows 2,113,536 mismatches with the signed ops and 0 with unsigned.

How: two-token change in buildWASM (table-lookup.ts). Added a compiler test with a [0x0e, 0xff] self-loop that fails on the wasm build before this change and passes after. The wasm variant is only built when WASM is set in the fixture env, so the new test is exercised with WASM=clang WASM_CFLAGS="--target=wasm32-unknown-wasi" npm run mocha.

The wasm SIMD byte-range test used signed `wasm_i8x16_ge/le`, so a
range crossing 0x7f/0x80 (e.g. [0x0e, 0xff]) matched no bytes. A
self-looping table-lookup node then advanced zero bytes and took its
`otherwise` edge, which can spin when that edge leads back to the node.

Use `wasm_u8x16_ge/le`, matching the NEON and SSE paths.

This branch has not been deployed

No deployments
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.

1 participant