Encode i32.const immediates as signed LEB128 in the PostScript Wasm compiler#21601
Conversation
|
If this affects rendering, then a ref-test would seem appropriate as well. |
8fa4d4a to
4e1ef7a
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #21601 +/- ##
=======================================
Coverage 89.85% 89.85%
=======================================
Files 263 263
Lines 66763 66765 +2
=======================================
+ Hits 59990 59995 +5
+ Misses 6773 6770 -3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Good idea — added one in 7f2d3ef. It's a minimal PDF that renders an axial shading whose colour function is a Type 4 function with nine outputs, so it exercises the exact path that was broken: the 9th output's store offset is 64, which is the first value the old unsigned-LEB128 encoding mangled. Without the fix the shading traps and drops out (blank); with it, it renders. Ready for a snapshot run whenever you'd like, and happy to rename it if you'd prefer an issue-number filename. |
7f2d3ef to
c26f72e
Compare
Snuffleupagus
changed the title
From: Bot.io (Linux m4)SuccessFull output at http://54.241.84.105:8877/2b0fd8f4dc17b34/output.txt Total script time: 18.16 mins
|
…ompiler The Type-4 PostScript -> Wasm compiler emitted i32.const immediates with the unsigned LEB128 encoder (_emitULEB128). Wasm decodes i32.const as a *signed* LEB128, so any immediate whose final 7-bit group has bit 0x40 set is mis-decoded (e.g. 64 -> single byte 0x40 -> read back as -64). The output-store address for the i-th result is emitted as i32.const (i*8). For a function with >= 9 outputs the 9th offset is 64, which decodes as -64, so f64.store targets 0xFFFFFFC0 and traps (memory access out of bounds). The module still validates and instantiates, so the JS fallback in function.js is not engaged; the trap only surfaces at render time (the affected shading / tint transform silently fails to render). The same mis-encoding affects the constant bitshift amount. Add a signed-LEB128 emitter (_emitSLEB128) and use it for the three i32.const immediate sites; local/import/type indices remain unsigned (correct). Add a unit test covering functions with 9+ outputs, and a reference test rendering a shading whose colour function has 9 outputs.
c26f72e to
778aa45
Compare
From: Bot.io (Windows)SuccessFull output at http://54.193.163.58:8877/1cee000a7a50cf6/output.txt Total script time: 25.69 mins
|
|
/botio makeref |
From: Bot.io (Windows)ReceivedCommand cmd_makeref from @calixteman received. Current queue size: 0 Live output at: http://54.193.163.58:8877/43437eb7fde0171/output.txt |
From: Bot.io (Linux m4)ReceivedCommand cmd_makeref from @calixteman received. Current queue size: 0 Live output at: http://54.241.84.105:8877/ca8fde1cc215923/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.241.84.105:8877/ca8fde1cc215923/output.txt Total script time: 18.21 mins
|
From: Bot.io (Windows)SuccessFull output at http://54.193.163.58:8877/43437eb7fde0171/output.txt Total script time: 25.68 mins
|
|
Thanks for the merge, and for handling the makeref run. |
|
Thanks for merging this, and for running the bots to get the refs in. |
6 participants
While poking at the new PostScript Type 4 → Wasm compiler I noticed that functions with 9 or more outputs blow up at render time with RuntimeError: memory access out of bounds, and the shading/tint-transform that uses them just silently fails to draw.
The cause is a small encoding bug. Each output's result is stored with f64.store at address i * 8, and that address is emitted as i32.const (i * 8). i32.const immediates are signed LEB128, but the compiler was writing them with the unsigned encoder (_emitULEB128). The two encodings only agree while the value stays ≤ 63 — as soon as you hit 64 (the 9th output), the unsigned encoder emits the single byte 0x40, which Wasm reads back as -64. So the store ends up targeting 0xFFFFFFC0, goes out of bounds, and traps. The module itself still validates and instantiates fine, so the JS fallback in function.js never kicks in and the problem only shows up when the function is actually called. The same mis-encoding also affects the constant bitshift amount (there it just produces a wrong shift).
The fix is to add a proper signed-LEB128 emitter (_emitSLEB128) and use it for the three i32.const immediate sites. The local/import/type indices stay on the unsigned encoder, since those really are unsigned in Wasm. The existing hand-written -1 case (emitted directly as 0x7f) already hinted the signed encoding was needed here.
I added a regression test in postscript_spec.js that compiles functions with 9/10/16/20 outputs through the existing compileAndRun helper — since that helper instantiates and calls the function, it fails on the old code and passes now. Also checked the fix against the JS evaluator across a bunch of programs (dup chains, sin/cos, roll/copy, ifelse, arithmetic) and the outputs match, and the full unit suite is green.
One related thing I ran into while testing, not fixed here: a non-constant-condition if that grows the stack causes the rest of the block to be evaluated once per branch, so a handful of stacked guards makes compile time blow up exponentially (a ~430-byte function can OOM the worker). Since it's a hang/OOM rather than a throw, the function.js try/catch can't recover from it. Might be worth capping the compile-time work and bailing to the JS interpreter — happy to do that in a follow-up if you'd like.