Binaryen.js breakage when compiling with Emscripten tot

[dcodeIO]

Seeing an issue that appears to be a problem when compiling Binaryen to JS with Emscripten newer than 2.0.2.

Works with:

• Emscripten 2.0.1, building Binaryen with CMAKE_BUILD_TYPE=Release
• Emscripten 2.0.2, building Binaryen with CMAKE_BUILD_TYPE=Release
• Emscripten tot, building Binaryen with CMAKE_BUILD_TYPE=Debug

Does not work with:

• Emscripten tot, building Binaryen with CMAKE_BUILD_TYPE=Release

Yielding the following cryptic error when attempting to compile the bootstrapped AssemblyScript compiler:

Assertion failed: *task.currp, at: /path/to/binaryen/src/wasm-traversal.h,702,walk
Error: abort(Assertion failed: *task.currp, at: /path/to/binaryen/src/wasm-traversal.h,702,walk). Build with -s ASSERTIONS=1 for more info.
... gibberish ...

Unfortunately, I haven't figured out yet how to debug this properly since it happens right after code generation in AssemblyScript, during emit of the Wasm binary (no fixture yet), doesn't seem to be a problem in Binaryen itself and does not show up in debug builds. Now I know this probably doesn't help much, but I thought I let you know in case you have an idea where this might be coming from?

[dcodeIO]

What I can provide, if it sounds useful, is a fork of the AssemblyScript compiler that exhibits the problem and allows to easily switch out the binaryen.js build. Doubting how useful that'll ultimately be, but let me know if you'd like to give it a shot.

[kripken]

Hmm, no idea what that could be. But since it's so recent, bisection could be the quickest thing, see #12064

May also help to build with sanitizers, both asan and ubsan.

[dcodeIO]

According to bisection on tot builds so far I found

• Last working: https://chromium.googlesource.com/emscripten-releases/+/a1f38ad4ba4a35a85aa09884e11f5ccf74e4363b
• First broken: https://chromium.googlesource.com/emscripten-releases/+/ba1c10b6ca88321b862a3573e6cfbb529af7dba5

indicating that the problem started with one of these four Binaryen commits:

• 2020-09-03 Fix Wasm2JSBuilder leaking temporary Function (Fix Wasm2JSBuilder leaking temporary Function WebAssembly/binaryen#3098)
• 2020-09-02 Fix LegalizeJSInterface leaking duplicate stub Functions (Fix LegalizeJSInterface leaking duplicate stub Functions WebAssembly/binaryen#3095)
• 2020-09-02 MinifyImportsAndExports: Minify the memory and table as well. (MinifyImportsAndExports: Minify the memory and table as well. WebAssembly/binaryen#3089)
• 2020-09-02 StackCheck: Check both under and overflow (StackCheck: Check both under and overflow WebAssembly/binaryen#3091)

[dcodeIO]

Bisecting further, I found that WebAssembly/binaryen#3089 introduces the problem. More precisely, building the respective merge commit with a known good revision of Emscripten to JS yields a binaryen.js that works in AssemblyScript (which is expected because AS doesn't utilize the import/export minification), but as soon as I copied the respective libbinaryen.so (built with the known good revision of Emscripten, one Binaryen commit earlier) over the file in the known good revision of Emscripten, the problem appeared in the generated binaryen.js. Also explains why the problem doesn't show in debug builds I guess, which I assume don't do the minification.

Last working and first broken binaryen.js.zip

[dcodeIO]

Just double-checked, confident that this is the commit.

[kripken]

Because of closure the diff between the two is very large (even after prettifying whitespace). If the bug still happens without closure in both those builds that could be easier to debug.

[dcodeIO]

binaryen.js-noclosure.zip

• Last working commit: WebAssembly/binaryen@89020a0
• First broken commit: WebAssembly/binaryen@7438b6c

The working file is the last working commit compiled using Binaryen as of the last working commit injected into Emscripten.
The broken file is the last working commit compiled using Binaryen as of the first broken commit injected into Emscripten.

[kripken]

Thanks @dcodeIO , with that the only difference is indeed in the minified imports, after cleaning up whitespace.

Probably worth confirming that whitespace isn't the issue - when you build with -g1 does the bug go away?

If the bug is still there, then the import minification is wrong somehow. I'm having a hard time thinking of what could be wrong there. But first thing I would check this happens in more than one JS VM, as maybe it's a VM bug? (very unlikely) If not that, then to debug it you can build with -s LIBARY_DEBUG. That will log out each call to an import, including parameters and return values, so you can then diff the logs to see where execution diverges.

[dcodeIO]

The problem is still present when compiling with -g1. Comparing those two files, I also see how the only difference is minified names now. Interesting :)

binaryen.js-noclosure-g1.zip

Looking at the library debug logs, these start out the same for 842 lines, and then diverge (first line is the last that equals below):

Working

[library call:emscripten_realloc_buffer: 0x1340000 (20185088)]
  [     return:0x1 (1)
  [     return:0x1 (1)
[library call:_emscripten_memcpy_big: 0xff3778 (16725880),0xfb3770 (16463728),0x40000 (262144)]
[library call:_emscripten_memcpy_big: 0xf6a268 (16163432),0xf94c80 (16338048),0x1800 (6144)]
[library call:_emscripten_memcpy_big: 0xf6d270 (16175728),0xf92bd8 (16329688),0x200 (512)]
[library call:_emscripten_memcpy_big: 0xf6d270 (16175728),0xf92bd8 (16329688),0x200 (512)]
[library call:_emscripten_memcpy_big: 0xf6d678 (16176760),0xf6d270 (16175728),0x400 (1024)]
...ok...

Broken

[library call:emscripten_realloc_buffer: 0x1340000 (20185088)]
[library call:emscripten_realloc_buffer: 0x11a0000 (18481152)]
[library call:emscripten_realloc_buffer: 0x10d0000 (17629184)]
  [     return:0
[library call:_emscripten_memcpy_big: 0xff3778 (16725880),0xfb3770 (16463728),0x40000 (262144)]
[library call:_emscripten_memcpy_big: 0xf6a268 (16163432),0xf94c80 (16338048),0x1800 (6144)]
[library call:_emscripten_memcpy_big: 0xf6d270 (16175728),0xf92bd8 (16329688),0x200 (512)]
[library call:___assert_fail: 0x2a9ed (174573),0x2a9b8 (174520),0x2be (702),0x2a9e8 (174568)]
Assertion failed: *task.currp, at: /path/to/binaryen/src/wasm-traversal.h,702,walk

binaryen.js-noclosure-g1-library_debug-logs.zip (truncated the working log with ...ok...)

[dcodeIO]

Regarding multiple JS VMs, that's tricky, because asc requires a node.js runtime. Can confirm though that it happens with node 14.6.0 and 14.10.0 alike, but not sure how useful that is.

[kripken]

library call:emscripten_realloc_buffer

A surprising place for the change to happen in... But it should be easy to figure out at least!

First, it looks like the problem is we run out of memory in the VM, and fail to grow the wasm. To confirm, you can add some console.logs in emscripten_realloc_buffer, but likely it just fails in the try-catch in that function (so logging in the catch should show the error).

Why one build runs out of memory earlier is a good question, given the small difference. That sounds like a potential VM oddity. But likely it is right at the edge of running out anyhow.

Second, the bigger issue is that we probably don't handle malloc(..) returning 0 properly. Putting a console.log(new Error().stack) in the catch in that function should show the full stack trace from the malloc. With that we can find where in binaryen we are allocating, and decide what to do.

Also, if it looks like there should be memory available, then this might be a JS VM bug we should file. Is this on a 64-bit system with lots of free memory? And how big is the memory before the growth, and how much is it trying to grow to?

[dcodeIO]

The full error is:

grow wasmMemory.buffer.byteLength=16777216 by size=20185088
TypeError: Cannot set property 'buffer' of undefined
    at Va (\path\to\assemblyscript\lib\binaryen.js:400280:14)
    at Nbc (\path\to\assemblyscript\lib\binaryen.js:385412:9)
    at Memory.grow (\path\to\assemblyscript\lib\binaryen.js:339:14)
    at \path\to\assemblyscript\lib\binaryen.js:404971:15
    at emscripten_realloc_buffer (\path\to\assemblyscript\lib\binaryen.js:404977:4)
    at \path\to\assemblyscript\lib\binaryen.js:404996:22
    at _emscripten_resize_heap (\path\to\assemblyscript\lib\binaryen.js:405002:4)
    at bi (\path\to\assemblyscript\lib\binaryen.js:31563:21)
    at Ib (\path\to\assemblyscript\lib\binaryen.js:332939:8)
    at Cfa (\path\to\assemblyscript\lib\binaryen.js:337543:7)
    at Zaa (\path\to\assemblyscript\lib\binaryen.js:339593:6)
    at Ov (\path\to\assemblyscript\lib\binaryen.js:369779:2)
    at QIb (\path\to\assemblyscript\lib\binaryen.js:324715:2)
    at Nn (\path\to\assemblyscript\lib\binaryen.js:355744:2)
    at VG (\path\to\assemblyscript\lib\binaryen.js:307402:3)
    at WJb (\path\to\assemblyscript\lib\binaryen.js:192171:5)
    at Daa (\path\to\assemblyscript\lib\binaryen.js:346594:2)
    at Th (\path\to\assemblyscript\lib\binaryen.js:383420:2)
    at qKb (\path\to\assemblyscript\lib\binaryen.js:311465:5)
    at rKb (\path\to\assemblyscript\lib\binaryen.js:189781:7)
    at Vf (\path\to\assemblyscript\lib\binaryen.js:370467:2)
    at Gaa (\path\to\assemblyscript\lib\binaryen.js:344499:4)
    at YJb (\path\to\assemblyscript\lib\binaryen.js:200895:4)
    at rKb (\path\to\assemblyscript\lib\binaryen.js:189917:5)
    at Vf (\path\to\assemblyscript\lib\binaryen.js:370467:2)
    at rKb (\path\to\assemblyscript\lib\binaryen.js:189809:7)
    at Vf (\path\to\assemblyscript\lib\binaryen.js:370467:2)
    at Mv (\path\to\assemblyscript\lib\binaryen.js:294563:5)
    at dJb (\path\to\assemblyscript\lib\binaryen.js:358761:2)
    at invoke_vi (\path\to\assemblyscript\lib\binaryen.js:407585:23)

And yeah, a 64-bit machine with 32gb of memory (20gb free).

[dcodeIO]

Looks like the l (supposed to be env.memory) in function Va is undefined:

  function Va(pagesToAdd) {
   pagesToAdd = pagesToAdd | 0;
   var Wa = Ua() | 0;
   var Xa = Wa + pagesToAdd | 0;
   if (Wa < Xa && Xa < 65536) {
    var Ya = new ArrayBuffer(v(Xa, 65536));
    var Za = new global.Int8Array(Ya);
    Za.set(n);
    n = Za;
    n = new global.Int8Array(Ya);
    o = new global.Int16Array(Ya);
    p = new global.Int32Array(Ya);
    q = new global.Uint8Array(Ya);
    r = new global.Uint16Array(Ya);
    s = new global.Uint32Array(Ya);
    t = new global.Float32Array(Ya);
    u = new global.Float64Array(Ya);
    buffer = Ya;
    l.buffer = Ya;
   }
   return Wa;
  }

assigned in

 function k(global, env, buffer) {
  var l = env.memory;
  var m = wasmTable;
  var n = new global.Int8Array(buffer);
  var o = new global.Int16Array(buffer);
  ...

and otherwise not accessed according to a "find all usages" search.

In the file, k is called exactly once:

 return k({
  "Int8Array": Int8Array,
  "Int16Array": Int16Array,
  "Int32Array": Int32Array,
  "Uint8Array": Uint8Array,
  "Uint16Array": Uint16Array,
  "Uint32Array": Uint32Array,
  "Float32Array": Float32Array,
  "Float64Array": Float64Array,
  "NaN": NaN,
  "Infinity": Infinity,
  "Math": Math
 }, asmLibraryArg, wasmMemory.buffer);

and asmLibraryArg looks like

var asmLibraryArg = {
 "R": __Exit,
 "i": ___assert_fail,
 "p": ___cxa_allocate_exception,
 "u": ___cxa_begin_catch,
 ...

with no key memory, but a "a": wasmMemory,.

If I change

 function k(global, env, buffer) {
- var l = env.memory;
+ var l = env.a;
  var m = wasmTable;
  var n = new global.Int8Array(buffer);
  var o = new global.Int16Array(buffer);

it works. Looks like the memory in env.memory doesn't become minified, yet Binaryen assumes it to be.

[kripken]

Thanks @dcodeIO , great analysis!

That does indeed look like the issue, and I think I have a small testcase locally showing it's a minification bug in -O3+, looking into it now.