Page MenuHomePhabricator

[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.

Diff Detail

Repository
rL LLVM

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 ↗(On Diff #209392)

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

lld/wasm/Writer.cpp
661 ↗(On Diff #209392)

Isn't OutputSegment::startVA already a uint32_t?

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 ↗(On Diff #209392)

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 ↗(On Diff #209392)

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 ↗(On Diff #209392)

+1

lld/wasm/Writer.cpp
661 ↗(On Diff #209392)

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

This revision was automatically updated to reflect the committed changes.