Skip to content
This repository has been archived by the owner on Aug 4, 2021. It is now read-only.

base64 encoding/decoding is broken #17

Open
darionco opened this issue Mar 26, 2019 · 3 comments
Open

base64 encoding/decoding is broken #17

darionco opened this issue Mar 26, 2019 · 3 comments

Comments

@darionco
Copy link

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
Copy link
Contributor

jamen commented Oct 8, 2019

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 (rollup/rollup#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
Copy link
Author

darionco commented Oct 8, 2019

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

@shellscape
Copy link
Contributor

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants