Skip to content

Best practices for Module['buffer']? #18474

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
willcohen opened this issue Jan 5, 2023 · 5 comments
Closed

Best practices for Module['buffer']? #18474

willcohen opened this issue Jan 5, 2023 · 5 comments

Comments

@willcohen
Copy link

Moving WebAssembly/binaryen#5390 here.

New emscripten-specific question
It looks like my attempts to use Module['buffer'] in pre-js are ill-advised, per #18452. For environments without WebAssembly, what is the preferred method to pass an external ArrayBuffer?

I suppose I'd be happy to use the internal glue code's mechanism as well, but something about the graaljs environment seems unable right now to create a memory or wasmMemory object on its own to emscripten's satisfaction. That said, I can absolutely confirm that I can create ArrayBuffers, and that they seem truthy enough on an ad-hoc basis.

Original text from binaryen issue
In attempting to load a wasm2js-generated js file on graaljs, I hit the following error:

failed to compile wasm module: TypeError: Cannot read property 'buffer' of undefined
Execution error (PolyglotException) at <js>/asmFunc (src.js:493).
TypeError: Cannot read property 'buffer' of undefined

It's referring to these lines in the transpiled js, implying that the env.memory section is undefined.

function asmFunc(env) {
 var memory = env.memory;
 var buffer = memory.buffer;

This error occurs when using the shell environment, which I suppose graal isn't, but running the transpilation without setting an env fails with Aborted(Assertion failed: shell environment detected but not enabled at build time. Add 'shell' to -sENVIRONMENT to enable.)

Should I be using a different environment, does this mean that the shell environment needs something else to generate a WebAssembly.Memory, or does there need to be an entirely new graal environment?

@willcohen
Copy link
Author

@sbc100
Based on #18454, does this mean that the path forward for JS-only users should be to create something in pre-js like:

var Module = {
  wasmMemory = { buffer: new ArrayBuffer() }
};

@kripken
Copy link
Member

kripken commented Jan 5, 2023

I think that's right but @sbc100 might be aware of something I forgot.

Note that you also need -sIMPORTED_MEMORY so that you can provide a memory like that, which will then be imported into the wasm (as opposed to the default where it is defined in the wasm).

@sbc100
Copy link
Collaborator

sbc100 commented Jan 5, 2023 via email

@willcohen
Copy link
Author

willcohen commented Jan 6, 2023

Other way around, actually -- no working examples. I was trying to figure out why graaljs kept insisting that memory.buffer was undefined, which led me to try providing an imported one, assuming the issue was one of me using the wrong flags or needing to stick some pre-js in. Based on #18452 I tried replacing the uses of env.memory and memory.buffer inside the generated -O0 code itself with a direct usage of Memory['wasmMemory'] and it loaded successfully with graal, at least with emscripten 3.1.24 (stuck there for now because of LLVM 14).

I've gone ahead and backported #18452 and #18454 locally and anything else that touched runtime_init_memory since 3.1.24 to see if that clears it up, rather than trying to trace the older code more. With that, missingGlobal is now triggered on buffer -- looks like what @kripken had noted in WebAssembly/binaryen#5390. graaljs is falling through the cracks between shell and however emscripten expects it to behave for wasmMemory.

@willcohen
Copy link
Author

Problem solved. #18452 and #18454 indeed solve the problem, suggesting that this is similar to #17842. That wasn't clear to me yesterday because in trying to debug I'd added 'wasmMemory' to EXPORTED_RUNTIME_METHODS, which needed to be removed for the missingGlobal to go away. Many thanks and sorry for the bother.

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

No branches or pull requests

3 participants