-
Notifications
You must be signed in to change notification settings - Fork 715
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
Limits #2522
Comments
Hmm, I'm not sure I totally follow. The BinaryReaderIR does impose limits like kMaxNestingDepth, kMaxFunctionLocals, kMaxFunctionParams, and kMaxFunctionResults, which match the values in V8 (https://chromium.googlesource.com/v8/v8/+/refs/heads/main/src/wasm/wasm-limits.h) and hopefully the other big engines. You're saying WABT should limit other kinds of things? And/or the values should be configurable at runtime? I think prevailing practice so far has just been to sort of handshake agree on reasonable values for the various "implementation limitations" (https://webassembly.github.io/spec/core/appendix/implementation.html) across major implementations. |
right, so Lines 94 to 97 in ea193b4
... we would never have found these if you hadn't mentioned them. (... why aren't they in a header??? or, alternatively, configurable by the user.) it is worth noting that these only apply to the IR reader, they do not apply to text format or - more importantly - to interpreter. |
Making them configurable seems like maybe unnecessary. Putting them in a header seems pretty reasonable. Maybe just in the existing header for the binary file stuff. |
we feel like making them configurable might be useful given the whole wabt is primarily a developer tool (tho having defaults probably wouldn't hurt), but we mean... maybe. we also need to get these to apply to the interpreter and the text format and whatnot tho. |
Hopefully only the parsers / validators need to enforce these limits. The interpreter can assume all its inputs have been validated. There may be some exceptions but I can't think of any |
hmm... we'll think about it. |
wabt currently applies no limits anywhere ever.
we would like to introduce a limits.json file that can be passed in the CLI. (we think a json would be a better fit than putting it all in the CLI.) like
--limits limits.json
.the hard part is enforcing the limits tbh... but anyway.
The text was updated successfully, but these errors were encountered: