Stub out all imports in ctor-eval #8115
Conversation
|
|
||
| ModuleUtils::iterImports( | ||
| wasm, | ||
| [&modules](std::variant<Memory*, Table*, Global*, Function*, Tag*> import) { |
There was a problem hiding this comment.
What is the advantage of this variant-based visiting, replacing iterImported* functions with one iterImports? (I'm not opposed to it, necessarily, I just don't see an obvious reason so I think I may be missing something.)
There was a problem hiding this comment.
It's not a big difference, I mainly did this because there is some work in common for all imports (the first few lines of this function). If we were to use the iterImported* functions, we'd need to introduce a helper function and call it in the body of each one. Also this way forces us to deal with all types of imports which makes it clear to the reader which ones are handled (e.g. Tags are not handled which was implicit before but is explicit now which I think is a small plus).
There was a problem hiding this comment.
Why don't we want to handle tags? That seems like it was probably an accidental omission before. (Or do we not have a hook for aborting the execution if an imported tag is used? In that case maybe aborting early in the presence of imported tags is intentional.)
There was a problem hiding this comment.
Good question about tags @tlively , but @stevenfontanella otherwise that all sounds good, thanks for the explanation.
There was a problem hiding this comment.
My impression is that ctor-eval won't work at all on any ctor that references an imported tag: https://github.com/WebAssembly/binaryen/blob/main/src/tools/wasm-ctor-eval.cpp#L327. I guess this is the best we can do unless the tag is somehow referenced in a way that doesn't matter at all? Or as you mentioned, if the same tag is used to throw and catch, then it doesn't matter what the imported value was.
|
|
||
| ModuleUtils::iterImports( | ||
| wasm, | ||
| [&modules](std::variant<Memory*, Table*, Global*, Function*, Tag*> import) { |
There was a problem hiding this comment.
Why don't we want to handle tags? That seems like it was probably an accidental omission before. (Or do we not have a hook for aborting the execution if an imported tag is used? In that case maybe aborting early in the presence of imported tags is intentional.)
ctor-eval doesn't have access to imports and previously didn't have any special handling for this. Globals would fail to import, which would prevent further evaluation. This is why global-get-init.wast didn't optimize away even though it does nothing.
We already stubbed out imports to "env". Change the code to create a stub module for all modules named in imports, so that instantiation is valid and evaluation can continue. This also unblocks #8086 which checks imports and requires them to exist during instantiation.