Skip to content
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

[Strings] Fuzzer: Emit StringConcat #6532

Merged
merged 31 commits into from
Apr 24, 2024
Merged

[Strings] Fuzzer: Emit StringConcat #6532

merged 31 commits into from
Apr 24, 2024

Conversation

kripken
Copy link
Member

@kripken kripken commented Apr 24, 2024

Also refactor the code a little to make it easier to add this.

Diff without whitespace is much smaller.

@kripken kripken requested a review from tlively April 24, 2024 17:23
Comment on lines +2624 to +2626
// We much less frequently make string.concat as it will recursively
// generate two string children, i.e., it can lead to exponential
// growth.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this true of lots of instructions, though? Don't we have some depth limiting mechanism that already takes care of this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do have a limit, yeah, but it is nice to avoid reaching it I think (as reaching it has downsides - we've exhausted input bytes). And this still gives a reasonable chance for concat to happen I think.

RefNull : 5
Return : 3
Store : 1
StringConst : 1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe tweak the input data again? It looks like this single StringConst is the only string instruction generated.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heh, I thought the same and tinkered with it for maybe 10 minutes before giving up. It's just very hard to get it to emit the right stuff here without stopping to emit other stuff (like GC instructions) by chance. Perhaps if we had a very large input, or multiple input files, that would be better... but I'm not sure it's worth it.

Copy link
Member

@tlively tlively left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, lgtm.

@kripken kripken merged commit 22e80de into WebAssembly:main Apr 24, 2024
10 of 13 checks passed
@kripken kripken deleted the fuzz.s4 branch April 24, 2024 18:05
@gkdn gkdn mentioned this pull request Aug 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants