Skip to content
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

Nuance of ASYNCIFY_IGNORE_INDIRECT #4225

Closed
andrewevstyukhin opened this issue Oct 8, 2021 · 10 comments
Closed

Nuance of ASYNCIFY_IGNORE_INDIRECT #4225

andrewevstyukhin opened this issue Oct 8, 2021 · 10 comments

Comments

@andrewevstyukhin
Copy link

Hi,

I try to better tune asyncify but any my attempt fails when I use ASYNCIFY_IGNORE_INDIRECT. From Emscripten side it influences on

shared.settings.ASYNCIFY_IMPORTS += ['invoke_*']

Simple case breaks big program: -s ASYNCIFY_IGNORE_INDIRECT -s ASYNCIFY_IMPORTS=['invoke_*'].

ModuleAnalyzer first marks functions from the remove-list as info.inRemoveList = true; and info.canChangeState = false;.
Then ModuleAnalyzer applies onlyListInput and addListInput as info.addedFromList = true; and info.canChangeState = true;.
What will happen if REMOVE + ADD applies simultaneously to the same function?
I think info.addedFromList = true; assumes info.inRemoveList = false;.
Suspicious places are 1 and 2.

Also I'm in doubt about the moment of scanner.propagateBack. Looks like there is no opportunity to set up async sources precisely before robust async propagation. imho Dual lists can help: remove and add before propagation, remove and add after propagation.

@andrewevstyukhin
Copy link
Author

I try to limit asyncified imports and there is no other option to disable ASYNCIFY_IMPORTS += ['invoke_*'].
Maybe ASYNCIFY_IMPORTS_REMOVE can be a solution at Emscripten side to adjust ASYNCIFY_IMPORTS after DEFAULT_ASYNCIFY_IMPORTS have been added.

@kripken
Copy link
Member

kripken commented Oct 19, 2021

-s ASYNCIFY_IGNORE_INDIRECT -s ASYNCIFY_IMPORTS=['invoke_*']

The invoke methods are not imports - perhaps that is the issue? They are exports from the wasm. Try adding them to ASYNCIFY_ADD.

You may also need to add the actual imports that call into JS that eventually ends up calling an invoke method.

@andrewevstyukhin
Copy link
Author

emcc.py treats invoke_* as imports https://github.com/emscripten-core/emscripten/blob/da842597941f425e92df0b902d3af53f1bcc2713/emcc.py#L2294

-s ASSERTIONS=1 setting tells the same thing:
Uncaught import invoke_vii was not in ASYNCIFY_IMPORTS, but changed the state

When I add invoke_vii into ASYNCIFY_ADD such error occurs:
[PassRunner] running pass: asyncify... Fatal: Asyncify addlist contained an imported function name (use the import list for imports): invoke_vii

So I add fully spelled invokes into imports (to prevent other runtime errors) and I add dynCall_* into ASYNCIFY_ADD.

exception thrown: RuntimeError: table index is out of bounds,RuntimeError: table index is out of bounds
at dynCall_v (Example.wasm)
at ret. (Example.js)
at Object.finishContextSwitch (Example.js)
at Object.trampoline (Example.js)
at Object.maybeStopUnwind (Example.js)
at ret. (Example.js)
at Example.js
at browserIterationFunc (Example.js)
at Object.runIter (Example.js)
at Browser_mainLoop_runner

I have working ASYNCIFY_REMOVE+ASYNCIFY_ADD, so I can't understand what to add more for ASYNCIFY_IGNORE_INDIRECT...

@andrewevstyukhin
Copy link
Author

andrewevstyukhin commented Oct 20, 2021

I removed locally ASYNCIFY_IMPORTS += ['invoke_*'] from emcc.py and deleted -s ASYNCIFY_IGNORE_INDIRECT setting.

exception thrown: RuntimeError: null function or function signature mismatch,RuntimeError: null function or function signature mismatch
at dynCall_v (Example.wasm)
at ret. (Example.js)
at Object.finishContextSwitch (Example.js)
at Object.trampoline (Example.js)
at Object.maybeStopUnwind (Example.js)
at ret. (Example.js)
at Example.js
at browserIterationFunc (Example.js)
at Object.runIter (Example.js)
at Browser_mainLoop_runner

Firefox tells more:

exception thrown: RuntimeError: indirect call to null,[email protected]
instrumentWasmExports/</ret[x]@Example.js
[email protected]
[email protected]
[email protected]
instrumentWasmExports/</ret[x]@Example.js
createExportWrapper/<@Example.js
[email protected]
[email protected]
[email protected]

The last chance is about to add all invoke_* and legalimport$invoke_* from ASYNCIFY_ADVISE=1 into imports...

This code insertion into function createExportWrapper can help to diagnose stacks:
if (name === 'asyncify_start_unwind') console.log(_emscripten_get_callstack_js(16));

And such patch of instrumentWasmImports::
finally{if(Asyncify.state!==originalAsyncifyState&&ASYNCIFY_IMPORTS.indexOf(x)<0&&!(x.startsWith("invoke_")&&true)){throw"import "+x+" was not in ASYNCIFY_IMPORTS, but changed the state"}}
goes to
finally{if(Asyncify.state!==originalAsyncifyState&&ASYNCIFY_IMPORTS.indexOf(x)<0){console.log(x);console.log(_emscripten_get_callstack_js(16));}}

@andrewevstyukhin
Copy link
Author

andrewevstyukhin commented Oct 20, 2021

Now I have working solution with a patched emcc.py:

-s ASYNCIFY=1 -s ASYNCIFY_ADVISE=1
-s ASYNCIFY_IMPORTS=['invoke_iij','legalstub$dynCall_iij','invoke_vi','invoke_iii','invoke_vii']
-s ASYNCIFY_REMOVE=['','A','B*','C*','D*','E*','F*','G*','H*','I*','J*','K*','L*','M*','N*','O*','P*','Q*','R*','S*','T*','U*','V*','W*','X*','Y*','Z*','a*','b*','c*','d*','f*','g*','h*','j*','k*','m*','n*','o*','p*','q*','r*','s*','t*','u*','v*','w*','x*','y*','z*']
-s ASYNCIFY_ADD=['legalfunc$invoke_iij','dynCall
','std::__2::__function::__func','MyApp::','boost::thread_specific_ptr']

But if I write -s ASYNCIFY_IGNORE_INDIRECT all breaks:

exception thrown: RuntimeError: table index is out of bounds,RuntimeError: table index is out of bounds
at dynCall_v (Example.wasm)
at ret. (Example.js)
at Object.finishContextSwitch

So there is something related to a problem in Binaryen not in Emscripten.

dynCall_v is used for emscripten_set_main_loop callback.

function _emscripten_set_main_loop(func, fps, simulateInfiniteLoop) {
var browserIterationFunc = (function() { dynCall_v.call(null, func); });
setMainLoop(browserIterationFunc, fps, simulateInfiniteLoop);
}

@andrewevstyukhin
Copy link
Author

andrewevstyukhin commented Oct 20, 2021

Even -g breaks execution:

exception thrown: RuntimeError: null function or function signature mismatch,RuntimeError: null function or function signature mismatch
at dynCall_v (Example.wasm)
at ret. (Example.js)

But -g2 works fine!

Then I stripped out debug info, but sizes differ too much:
Example.wasmG2 16 451 357
Example.wasmG3 18 015 373

Only first stack differs:
-g2

emscripten_fiber_swap
at imports.
at mine
! at legalstub$dynCall_iij (Example.wasm)
at ret.
! at Example.js
! at invoke_iij (Example.js)

-g3

emscripten_fiber_swap
at imports.
at mine
! at dynCall_iij (Example.wasm)
! at legalstub$dynCall_iij (Example.wasm)
at ret.
! at Example.js

I patched instrumentWasmImports: with this PoC:

} finally {
if (Asyncify.state !== Asyncify.State.Normal) {
console.log(x); console.log(Asyncify.exportCallStack); console.log(_emscripten_get_callstack_js(16));
}
}

and saw mismatches between -g2 and -g3 versions:
Working -g2 has empty Asyncify.exportCallStack.
Broken -g3 always has 0: "dynCall_iiii" record for legalized invoke_iij and finally breaks at exit from invoke_v.

@kripken
Copy link
Member

kripken commented Oct 20, 2021

Oh right, sorry, I had things reversed with invoke* being imports.

I'm not sure why debug info would affect things here.

If you can get small testcases for these issues that would be good.

@andrewevstyukhin
Copy link
Author

emscripten-core/emscripten#10941 has the same crash at dynCall_v.
emscripten-core/emscripten#11736 tells about debug magic with fibers.

I believe #2913

This PR makes the add-list and only-lists fully instrument the functions
mentioned in them: both themselves, and indirect calls from them.

@andrewevstyukhin
Copy link
Author

Hi,
I updated emscripten up to version 2.0.32 from version 2.0.14 and disabled C++ exceptions then ASYNCIFY_IGNORE_INDIRECT started to work like a charm. Of course I use -s [email protected].

With -fexceptions I still have crashes:

Uncaught RuntimeError: null function or function signature mismatch
at dynCall_v

imho -g3 preserves call stacks and prohibits function inlining, so release with dwarf always comes bigger & slower - funcs.txt told me about that.

@andrewevstyukhin
Copy link
Author

I don't use asyncify any more ;)

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

2 participants