Repository navigation
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## next #3128 +/- ##
=======================================
Coverage ? 61.33%
=======================================
Files ? 244
Lines ? 56931
Branches ? 10122
=======================================
Hits ? 34919
Misses ? 16769
Partials ? 5243 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| // | code_len: real code len count from varint | ||
| // | leng: return value, means length of varint. -1 means error | ||
| // return | varint, sign-extended and stored as two's complement | ||
| static uint32_t get_varint32(const uint8_t *code, size_t code_len, size_t *leng) |
There was a problem hiding this comment.
This is the LEB128 decode?
There was a problem hiding this comment.
Yes, this is the signed LEB128 decoder for the i32.const immediate. It follows the existing get_varuint32 shape, with a length check against code_len and a check that the unused bits of the 5th byte match the sign. The shared LEB128.h only has decodeULEB128, which is unsigned and doesn't bound reads by the buffer length, so I kept this next to the other WASM readers.
There was a problem hiding this comment.
Be so kind and implement it in LEB128.h (with the bounds checks of course).
There is a good chance we have to reuse it again.
This way you can also add some unit tests for the decoding.
There was a problem hiding this comment.
Done in 0c83428. decodeSLEB128 now lives in LEB128.h next to decodeULEB128, with the same signature as the LLVM version (p, n, end, error) and it stops reading at end. The WASM readers call it and only add the spec limits on top: at most 5 bytes plus an int32 range check for i32.const, at most 10 bytes for i64.const. Unit tests are in tests/unit/leb128.c and cover the 32/64-bit limits, non-canonical padding, truncated input and values that don't fit into int64.
|
The red jobs were all 32-bit or Windows targets. The new i64.const case printed 0xffffff80 there because WASM_printInst used %lx for the uint64_t varuint64, and long is 32 bits on those targets. I've switched it to PRIx64 in fd546f7. tests/details passes locally; I can't run the 32-bit targets here, so CI will need to confirm those. |
Move the signed LEB128 decoding into LEB128.h as decodeSLEB128 with a bounds check on the input buffer, use it for the i32.const and i64.const immediates and add unit tests for the decoder.
This branch has not been deployed
2 participants
Your checklist for this pull request
Detailed description
The WebAssembly spec encodes the immediates of i32.const and i64.const as signed LEB128, but Capstone was decoding them as unsigned LEB128. As a result, negative constants were disassembled incorrectly:
41 7f was shown as 0x7f (127) instead of -1
41 80 7f was shown as 16256 instead of -128
Some valid encodings of negative values were rejected as invalid
Test plan
This PR switches both instructions to signed LEB128 decoding and adds a test checking that 41 7f disassembles to i32.const -1.
Closing issues
This fix was found and prepared with AI assistance and reviewed by me before submitting.