Crash in Memory64Lowering

[sbc100]

trafficstars

When working on getting this emscripten test to pass ( ./tests/runner wasm64l.test_stack_placement_pic) I ran into this binaryen crash.

$ gdb --args wasm-opt --strip-dwarf --memory64-lowering --strip-debug --strip-producers test_stack_placement.wasm -o test_stack_placement.wasm -g --mvp-features --enable-memory64 --enable-mutable-globals
wasm-opt: /usr/local/google/home/sbc/dev/wasm/binaryen/src/wasm.h:713: T* wasm::Expression::cast() [with T = wasm::Const]: Assertion `int(_id) == int(T::SpecificId)' failed.
--Type <RET> for more, q to quit, c to continue without paging-- 

Thread 1 "wasm-opt" received signal SIGABRT, Aborted.
__GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:49
49	../sysdeps/unix/sysv/linux/raise.c: No such file or directory.
(gdb) bt
#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:49
#1  0x00007ffff55c8546 in __GI_abort () at abort.c:79
#2  0x00007ffff55c842f in __assert_fail_base (
    fmt=0x7ffff573edf8 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n", 
    assertion=0x5555557177b0 "int(_id) == int(T::SpecificId)", 
    file=0x555555717778 "/usr/local/google/home/sbc/dev/wasm/binaryen/src/wasm.h", 
    line=713, function=<optimized out>) at assert.c:92
#3  0x00007ffff55d7222 in __GI___assert_fail (
    assertion=0x5555557177b0 "int(_id) == int(T::SpecificId)", 
    file=0x555555717778 "/usr/local/google/home/sbc/dev/wasm/binaryen/src/wasm.h", 
    line=713, 
    function=0x555555717740 "T* wasm::Expression::cast() [with T = wasm::Const]")
    at assert.c:101
#4  0x000055555566fcdf in wasm::Expression::cast<wasm::Const> (this=0x5555560d8148)
    at /usr/local/google/home/sbc/dev/wasm/binaryen/src/wasm.h:713
#5  0x00007ffff7185c4a in wasm::Memory64Lowering::visitDataSegment (this=0x555555927de0, 
    segment=0x555555fc6880)
    at /usr/local/google/home/sbc/dev/wasm/binaryen/src/passes/Memory64Lowering.cpp:111
#6  0x00007ffff7186c49 in wasm::Walker<wasm::Memory64Lowering, wasm::Visitor<wasm::Memory64Lowering, void> >::walkDataSegment (this=0x555555927e08, segment=0x555555fc6880)
    at /usr/local/google/home/sbc/dev/wasm/binaryen/src/wasm-traversal.h:212
#7  0x00007ffff71868e4 in wasm::Walker<wasm::Memory64Lowering, wasm::Visitor<wasm::Memory64Lowering, void> >::doWalkModule (this=0x555555927e08, module=0x7fffffffbe70)
    at /usr/local/google/home/sbc/dev/wasm/binaryen/src/wasm-traversal.h:263
#8  0x00007ffff71862e6 in wasm::Walker<wasm::Memory64Lowering, wasm::Visitor<wasm::Memory64Lowering, void> >::walkModule (this=0x555555927e08, module=0x7fffffffbe70)
    at /usr/local/google/home/sbc/dev/wasm/binaryen/src/wasm-traversal.h:222
#9  0x00007ffff7186004 in wasm::WalkerPass<wasm::PostWalker<wasm::Memory64Lowering, wasm::Visitor<wasm::Memory64Lowering, void> > >::run (this=0x555555927de0, 
    runner=0x7fffffffb9c0, module=0x7fffffffbe70)
    at /usr/local/google/home/sbc/dev/wasm/binaryen/src/pass.h:423
#10 0x00007ffff71856b9 in wasm::Memory64Lowering::run (this=0x555555927de0, 
    runner=0x7fffffffb9c0, module=0x7fffffffbe70)
    at /usr/local/google/home/sbc/dev/wasm/binaryen/src/passes/Memory64Lowering.cpp:34
#11 0x00007ffff6e939d8 in wasm::PassRunner::runPass (this=0x7fffffffb9c0, 
    pass=0x555555927de0)
    at /usr/local/google/home/sbc/dev/wasm/binaryen/src/passes/pass.cpp:877
#12 0x00007ffff6e93650 in wasm::PassRunner::run (this=0x7fffffffb9c0)
    at /usr/local/google/home/sbc/dev/wasm/binaryen/src/passes/pass.cpp:749
#13 0x00005555556d2f2d in wasm::OptimizationOptions::runPasses (this=0x7fffffffc1a0, 
    wasm=...)

[sbc100]

test_stack_placement.zip

Attached is a copy of the failing input wasm.

[sbc100]

Looks like the proximate fix is to not assume that segments have const offsets.

[kripken]

Reduced testcase:

(module
 (import "env" "memory" (memory $mimport$0 i64 256 256))
 (import "env" "__memory_base" (global $gimport$0 i64))
 (data (global.get $gimport$0) "")
)

The problem is the segment offset is not a const, correct.

But what should we do here? We can't wrap the 64-bit global into 32 bits since i64.wrap is not allowed in global locations. Should we create another import of the same global but with i32? Not sure that would work.

[sbc100]

I think we already import __table_base32 .. so I guess we should create new import called __memory_base32?

[kripken]

Sgtm. Would be easy to make this pass switch to use the 32-bit global based on the import name.

[kripken]

Would the pass create the new import, or the caller? (The caller needs to actually pass in the new import, so maybe it should do both?)

[sbc100]

I think this pass has to create the new import and then it will be up to emscripten to provide it... trying to remember how to write binaryen code now :)

[kripken]

Sgtm.

I could do this if you prefer?

[sbc100]

I can give it go.. I need to learn more binaryen codebase stuff..

[sbc100]

I can give it go.. I need to learn more binaryen codebase stuff..