Skip to content

[Fuzzing] Use initial contents in ClusterFuzz#7192

Merged
kripken merged 50 commits intoWebAssembly:mainfrom
kripken:initial.cluster
Jan 7, 2025
Merged

[Fuzzing] Use initial contents in ClusterFuzz#7192
kripken merged 50 commits intoWebAssembly:mainfrom
kripken:initial.cluster

Conversation

@kripken
Copy link
Copy Markdown
Member

@kripken kripken commented Jan 7, 2025

The ClusterFuzz bundler now looks through all of our test suites and
packages all testcases that are suitable for ClusterFuzz to use. This adds
more variety to the wasm files we fuzz there, as the test suite has
corner cases that the main fuzzer is unlikely to generate.

This adds a comment in the JS whenever it uses initial content, to
make debugging easier, something like

[10, 20, 30] /* using initial content 17.wasm */

(this is the reason for the change to extract_wasms.py).

@kripken kripken requested a review from tlively January 7, 2025 19:12

from test import fuzzing # noqa
from test import shared # noqa
from test import support # noqa
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What lints are we skipping here?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The one that imports must be on top (which we can't do because of lines 83-89, which I can't think of a nice alternative to).

Comment on lines +105 to +109
features = [
'-all',
'--disable-shared-everything',
'--disable-fp16',
]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Would it be feasible to deduplicate these into a shared location?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Not trivially, since run.py is not part of the normal "tree". We never run it as part of our own code, only after being packaged for ClusterFuzz. So we don't have the normal shared locations. I suppose we could have a file that we import locally and copy over for ClusterFuzz, but given this is just a few lines, that seems a bit excessive to me.

Comment on lines +168 to +169
# Write initial/num.txt which contains the number of testcases in that
# directory (saves run.py from needing to listdir each time).
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Does this make a measurable performance difference?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I can't measure this on ClusterFuzz itself, which is where it really matters. But imagine that ClusterFuzz might start to move to calling run.py more times for fewer testcases (rather than once for 1,000, it could do 1000 for single testcases each), then this constant overhead could matter.


# Replace the wasm files and write them out.
js = re.sub(r'var \w+ = new Uint8Array\(\[([\d,]+)\]\);', repl, js)
js = re.sub(r'var \w+ = new Uint8Array\(\[([\d,]+)\]\)', repl, js)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why no more semicolon?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Because now we can have one of the new comments between this and the semicolon. Detecting that would make this complicated for no useful reason (and adding the comment after the semicolon would be a little annoying in run.py).

(Sorry if this wasn't clear enough in the description.)

Comment thread test/unit/test_cluster_fuzz.py Outdated
kripken and others added 3 commits January 7, 2025 13:33
Co-authored-by: Thomas Lively <tlively123@gmail.com>
@kripken kripken merged commit 8d0f662 into WebAssembly:main Jan 7, 2025
@kripken kripken deleted the initial.cluster branch January 7, 2025 22:36
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