MarkJSCalled pass#8733
Conversation
| FindAll<Call> calls(func->body); | ||
| for (auto* call : calls.list) { | ||
| if (intrinsics.isConfigureAll(call)) { | ||
| for (auto name : intrinsics.getConfigureAllFunctions(call)) { |
There was a problem hiding this comment.
It would be good to produce a warning or fatal error when there are function operands passed to configureAll that we are not able to analyze. As a follow-up to this change we should remove getConfigureAllFunctions and all its uses in favor of explicitly annotated js-called functions anyway, so I think we can just inline the logic from getConfigureAllFunctions here.
In the future, if folks have different usage patterns and are running into the warning or fatal error, we can add support for more usage patterns here as well. (array.new_fixed usage comes to mind as low-hanging fruit.)
There was a problem hiding this comment.
so I think we can just inline the logic from getConfigureAllFunctions here.
I'd rather not do that in this PR, so that it is NFC aside from adding a new pass? I don't want to change behavior for users.
But I agree on the path after this PR, yes, it would be nice to unify and simplify this.
(Given we shouldn't unify it yet, I don't think it's worth adding detailed warnings here - that would lead to a bunch of duplicated code.)
There was a problem hiding this comment.
Sure, maybe a TODO about adding a warning/error, then?
2 participants
This adds
@binaryen.js.calledto functions called from anyconfigureAll,even one not from the start function.
Addresses the issue in #8727, where fuzzing the start function can lead to
situations where a function is referred to from configureAll but not marked
as js-called, which can break optimizations. By using this in the fuzzer, we
can fuzz even files with interesting configureAll calls.