-
Notifications
You must be signed in to change notification settings - Fork 833
Fuzzer: Do not emit huge and possibly non-validating tables #6288
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
Conversation
src/tools/fuzzing/fuzzing.cpp
Outdated
| // segment has an offset at an address larger than kMaxSize, which traps at | ||
| // startup but would be a non-validating module for us to emit. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am confused by this comment. Would the segment fail to validate or trap? It can't do both at the same time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A segment with a huge but valid offset that is larger than kMaxSize will trap. But we fail to validate if we raise initial to such sizes. This is due to the max size being 32 bits but Address being 64 bits. Perhaps another way to fix this is to refactor Address to be 32 bits in wasm32, but I think we had a reason for the current approach.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, so big segment offsets lead to traps and big tables lead to validation errors. Makes sense. I guess I don't understand the connection between them, though. Is the fuzzer generating segment offsets and table sizes based off of each other?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the code adjacent to this (right above it) increases initial based on the segments it sees, that is, it is trying to avoid a trap by ensuring a big-enough initial size.
|
Any other thoughts here or is this ok to land? |
tlively
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be good to clarify the language in the comment, but otherwise LGTM.
|
Ok, I rewrote the comment. |
tlively
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this makes sense to me!
No description provided.