rollup / rollup-plugin-wasm

This module has moved and is now available at @rollup/plugin-wasm / https://github.com/rollup/plugins/blob/master/packages/wasm
76 stars 13 forks source link

base64 encoding/decoding is broken #17

Open darionco opened 5 years ago

darionco commented 5 years ago

Take the following program

#include <emscripten.h>
void EMSCRIPTEN_KEEPALIVE putNumber(unsigned int *ptr) {
    ptr[0] = 666;
}

compiled as follows:

emcc mini.c -O3 -s WASM=1 -o mini.wasm -s TOTAL_MEMORY=65536 -s TOTAL_STACK=0

results in the following WAT:

(module
  (type (;0;) (func (param i32)))
  (import "env" "memory" (memory (;0;) 1 1))
  (func (;0;) (type 0) (param i32)
    local.get 0
    i32.const 666
    i32.store)
  (export "_putNumber" (func 0)))

loaded with the plugin:

import wasm from './mini.wasm';

const memory = new WebAssembly.Memory({initial: 1, maximum: 1});
wasm({ env: { memory }}).then(result => {
    const view = new DataView(memory.buffer);
    result.instance.exports._putNumber(4);
    console.log(view.getUint32(4, true));
});

prints out:

765

when loaded using fetch:

const memory = new WebAssembly.Memory({initial: 1, maximum: 1});
const imports = { env: { memory }};
fetch('./mini.wasm').then(result => result.arrayBuffer()).then(buffer => {
    WebAssembly.compile(buffer).then(module => WebAssembly.instantiate(module, imports)).then(instance => {
        const view = new DataView(memory.buffer);
        instance.exports._putNumber(4);
        console.log(view.getUint32(4, true));
    });
});

correctly prints:

666

I tracked down the issue to the base64 encoding/decoding as loading the wasm file using rollup-plugin-url works just fine. I believe you are suffering from this bug

jamen commented 5 years ago

I think people should use WebAssembly.instantiateStreaming(fetch(...)), or use rollup-plugin-url to inline then load it, and support for WebAssembly in Rollup core is greenlit (https://github.com/rollup/rollup/issues/2099), so that'll be the ideal solution. I think those are better ways, but I'm open to ideas for fixing this in the meantime.

Edit: I think this was fixed by #12 or #15, but its not published because of bug #21

darionco commented 5 years ago

15 definitely fixes it, unfortunately I can't comment on #21 since I don't use travis

shellscape commented 5 years ago

@darionco we're moving this plugin to a new rollup monorepo and will move your issue there once the move is done. Please stand by.