-
Notifications
You must be signed in to change notification settings - Fork 747
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
Consider functions in elems of public tables to be public #6660
base: main
Are you sure you want to change the base?
Conversation
This expands the definition of a public type from one which appears in a module's own external type to any type that is externalized during runtime (including during instantiation). I think this is a bridge we do not want to cross because the set of externalized types is undecidable. How exactly did this come up in fuzzing? |
As a concrete example see the testcase in this PR: the type |
Ah, right, the "correct" fix is to make it so that any subtype of a public type is also public when running with an open world. That's more conservative that what is implemented here, but importantly it is also decidable. (This was a known bug at some point but I never fixed it because we weren't working on open-world optimizations. There may be other related bugs that I've forgotten about...) |
Hmm, we already consider declared supertypes of public types to be public. Adding subtypes as well seems to suggest that huge swaths of the type tree would quickly become public? E.g. in Java, a single public subtype of Maybe that's just how limited open world is. In that case maybe it makes sense to just say that wasm-split isn't compatible with such optimizations? |
I don't think subtypes of abstract heap types like |
If not, then how would it solve the testcase in this PR? Or were you not suggesting that it would be enough to fix that, and additional work would be needed? |
Good point. I guess we either have to be extra conservative and make subtypes of abstract heap types (i.e. all types) public or we would have to add a |
Yeah, makes sense. Well, for now I think this is not urgent as it doesn't impact our ability to optimize closed world, which matters a lot more. I'll mark this PR as draft/not-to-land. |
If a table is public (imported or exported) then any function added to it can be called
from the outside, so any change to the type might cause observable differences.
Noticed while fuzzing wasm-split (unsubtyping considered the type of such a function
to be private, modified it, and things broke).