Skip to content

[NFC] Send the closed-world flag to TranslateToFuzzReader#7136

Merged
kripken merged 2 commits intoWebAssembly:mainfrom
kripken:fuzz.closed.flag
Dec 5, 2024
Merged

[NFC] Send the closed-world flag to TranslateToFuzzReader#7136
kripken merged 2 commits intoWebAssembly:mainfrom
kripken:fuzz.closed.flag

Conversation

@kripken
Copy link
Copy Markdown
Member

@kripken kripken commented Dec 4, 2024

This sends --closed-world to wasm-opt from the fuzzer, when we use that
flag (before we just used it on optimizations, but not fuzz generation). And
TranslateToFuzzReader now stores a boolean about whether we are in closed-
world mode or not.

This has no effect so far, but will be needed in a later PR, where we must
generate code differently based on whether we are in closed-world mode
or not.

@kripken kripken requested a review from tlively December 4, 2024 20:58
Copy link
Copy Markdown
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.

should we add the closedWorld bool along with the code that uses it?

@kripken
Copy link
Copy Markdown
Member Author

kripken commented Dec 4, 2024

Sorry, I'm not sure what you're asking?

Comment thread src/tools/fuzzing.h
Comment on lines +84 to +85
// Whether the module will be tested in a closed-world environment.
bool closedWorld;
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.

This isn't actually used in this PR, so should we move it to a later PR that uses it?

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 put it here because it receives the info that is sent in the other half of the PR, so it shows both parts.

And the other half of the PR isn't "used" either, it has no effect 😄

But happy to do it either way, what do you prefer?

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.

But the new constructor parameter is never passed anywhere, so this is always false, right? Am I missing something that connects the new flag passed by fuzz_opt.py to the new constructor parameter that sets this member variable?

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.

Good point, I was missing the line in wasm-opt.cpp to send it. I pulled it from the next PR to here now.

Copy link
Copy Markdown
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.

Thanks! This makes more sense now.

@kripken kripken merged commit 06e06ec into WebAssembly:main Dec 5, 2024
@kripken kripken deleted the fuzz.closed.flag branch December 5, 2024 17:59
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