This is an archive of the discontinued LLVM Phabricator instance.

[WebAssembly] i32.const operands should be signed
ClosedPublic

Authored by tlively on Jul 11 2019, 6:13 PM.

Details

Summary

This was causing large addresses to be emitted as negative numbers,
which rightfully caused crashes in binaryen.

Event Timeline

tlively created this revision.Jul 11 2019, 6:13 PM
aheejin accepted this revision.Jul 11 2019, 7:43 PM
aheejin added inline comments.
lld/test/wasm/data-segments.ll
91

It'd be better if we can get disassembly, but it seems to be not fully working yet

lld/wasm/Writer.cpp
661
This revision is now accepted and ready to land.Jul 11 2019, 7:43 PM
sbc100 added inline comments.Jul 12 2019, 1:53 AM
lld/test/wasm/data-segments.ll
44

How does this change test the logic change? Seems like 10000 isn't large enough to trigger the binaryen crash?

tlively marked 3 inline comments as done.Jul 12 2019, 10:54 AM
tlively added inline comments.
lld/test/wasm/data-segments.ll
44

1000 encoded as unsigned leb in hex is 904E, bu as a signed leb it is 90CE00. The need to leave room for a sign bit bumps the encoding up to use the next byte.

91

+1

lld/wasm/Writer.cpp
661

Good catch, this was left over from some previous experimentation.

This revision was automatically updated to reflect the committed changes.