emscripten-core / emsdk

Emscripten SDK
http://emscripten.org
Other
3.03k stars 694 forks source link

Stack is not restored after it is being allocated in callMain #1098

Open denisvmedia opened 2 years ago

denisvmedia commented 2 years ago

Consider the following code in postamble.js:

#if MAIN_READS_PARAMS && STANDALONE_WASM
  mainArgs = [thisProgram].concat(args)
#elif MAIN_READS_PARAMS
  args = args || [];
  args.unshift(thisProgram);

  var argc = args.length;
  var argv = stackAlloc((argc + 1) * {{{ Runtime.POINTER_SIZE }}});
  var argv_ptr = argv >> {{{ POINTER_SHIFT }}};
  args.forEach((arg) => {
    {{{ POINTER_HEAP }}}[argv_ptr++] = {{{ to64('allocateUTF8OnStack(arg)') }}};
  });
  {{{ POINTER_HEAP }}}[argv_ptr] = {{{ to64('0') }}};
#else
  var argc = 0;
  var argv = 0;
#endif // MAIN_READS_PARAMS

Here we call stackAlloc but we never call stackRestore. Thus, if we call callMain many times, the program finally crashes. I created this workaround for the lib I use: https://github.com/denisvmedia/jq-web/commit/ad67bcd48189cea53493ed909e0f505b1d334001

The problem of this solution is that stackRestore and stackSave are not exported by default. And also, it's a user-side code, while I'd expect it to be in the lib core.

P.S. More details on the problem here: https://github.com/paolosimone/jq-wasm/issues/3.

sbc100 commented 2 years ago

Sounds like an easy fix. Would you like to upload at patch to callMain?

However, callMain isn't really designed to be called many times like that. Normally a programs main function expects to be called exactly once, after which heap and stack are not normally reset and are generally not in a defined state. Perhaps you program is set to cleanup nicely as main exists, but this is not true to many programs. Most program expect main to be called each time in a completely separate process with a new heap and stack.

If you want to re-enter you program many times I would suggest exporting a different symbol..something like -sEXPORTED_SYMBOLS=doMainAction. That function that then take more precise args rather than having to fit into the argv model.

denisvmedia commented 2 years ago

My aim is to use jq in frontend. Restarting it every time is probably less efficient. Exporting another symbol can work, but it will require patching or even creating own fork of jq. Which is honestly not, beyond my current plans. I tested multiple calls to its main a lot and it seemed to work smoothly.

As of the patch. I thought of it, but the problem is that stackRestore and stackSave are not available inside postamble.js unless EXPORTED_RUNTIME_METHODS=stackSave,stackRestore,stackAlloc is used. So, I'm not sure how to make it working otherwise.

sbc100 commented 2 years ago

Did you trying using stackSave and stackRestore inside of callMain? As far as I can tell it should work fine. That function already uses stackAlloc and all 3 of these functions get exports from the wasm module by default (node that this is different from being exported to the outside world on the Module object). See:

https://github.com/emscripten-core/emscripten/blob/1f5237433a36a09024f18f290e9248469016c0af/emcc.py#L2132

So I think it should just work.

denisvmedia commented 2 years ago

Ok, I will try and get back with the results.

Kagami commented 11 months ago

I noticed the same. With latest Emscripten you just need to:

withStackSave(() => callMain(args))

in your pre.js.

windowsair commented 10 months ago

For me, tried doing a stack recovery and still get the following error:

Uncaught (in promise) RuntimeError: null function or function signature mismatch
    at test.wasm:0x29443e
    at Object.callMain (test.mjs:8:86207)