-
Notifications
You must be signed in to change notification settings - Fork 733
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
Binary reader options and intended use cases #2537
Comments
Regarding As a corollary to this I don't think the interpreter should have any runtime checks for things that are validation errors (perhaps a asserts are OK, but not real runtime checks). If you know of any such checks I think we should remove them or make them into asserts. |
Yes, agree. However, there's another issue: Fuzzing issues like https://issues.oss-fuzz.com/issues/378159149 seem to be related with continuing to build interpreter state for invalid modules after finding out they're invalid. These issues cannot be triggered with a regular wasm-interp. To us, these don't really seem worth chasing, so it would be kinda nice to eliminate them. wabt itself never uses |
Currently, we have the following binary reader options:
Now,
features
andlog_stream
are... fairly universal, we wanna be able to select which post-MVP features to enable and disable and oftentimes also enable verbose logging.However, it's another story with these other options. Most of these aren't available when running the provided tools, but API users can bring their own
ReadBinaryOptions
. In theory, this leads to use-cases like:among other, potentially questionable ideas. However, we would like to highlight one issue in particular, and it's that our interpreter is designed to be... pretty fast. It's not trying to be the fastest thing in the world, but it does little run-time checking, instead relying entirely on the
SharedValidator
to do its job. As such, it's a dangerous idea to run (maliciously crafted) broken modules in the interpreter.Would it make sense to more strictly enforce that many of these options are purely for use with the objdump reader? Ofc, we can still support use-cases like those in #2534 / #2535, but we (Soni) don't believe the API should look like this.
The text was updated successfully, but these errors were encountered: