Skip to content

Fuzzer: Separate arguments used to make the fuzz wasm from the opts we run on it #6357

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

Merged
merged 7 commits into from
Feb 27, 2024

Conversation

kripken
Copy link
Member

@kripken kripken commented Feb 27, 2024

Before FUZZ_OPTS was used both when doing --translate-to-fuzz/-ttf to generate the
wasm from the random bytes and also when later running optimizations to generate
a second wasm file for comparison. That is, we ended up doing this, if the opts were -O3:

wasm-opt random.input -ttf -o a.wasm -O3
wasm-opt a.wasm -O3 -o b.wasm

Now we have a pair a.wasm,b.wasm which we can test. However, we have run -O3
on both which is a little silly - the second -O3 might not actually have anything left
to do, which would mean we compare the same wasm to itself.

Worse, this is incorrect, as there are things we need to do only during the
generation phase, like --denan. We need that in order to generate a valid wasm to
test on, but it is "destructive" in itself: when removing NaNs (to avoid nondeterminism)
if replaces them with 0, which is different. As a result, running --denan when
generating the second wasm from the first could lead to different execution in them.
This was always a problem, but became more noticable recently now that DeNaN
modifies SIMD operations, as one optimization we do is to replace a memory.copy
with v128.load + v128.store, and --denan will make sure the loaded value has no
NaNs...

To fix this, separate the generation and optimization phase. Instead of

wasm-opt random.input -ttf -o a.wasm --denan -O3
wasm-opt a.wasm --denan -O3 -o b.wasm

(note how --denan -O3 appears twice), do this:

wasm-opt random.input -ttf -o a.wasm --denan
wasm-opt a.wasm -O3 -o b.wasm

(note how --denan appears in generation, and -O3 in optimization.

@kripken kripken requested review from tlively and aheejin February 27, 2024 18:24
@kripken kripken merged commit a1afe47 into WebAssembly:main Feb 27, 2024
@kripken kripken deleted the fuzz.gen.args branch February 27, 2024 21:58
radekdoulik pushed a commit to dotnet/binaryen that referenced this pull request Jul 12, 2024
…e run on it (WebAssembly#6357)

Before FUZZ_OPTS was used both when doing --translate-to-fuzz/-ttf to generate the
wasm from the random bytes and also when later running optimizations to generate
a second wasm file for comparison. That is, we ended up doing this, if the opts were -O3:

wasm-opt random.input -ttf -o a.wasm -O3
wasm-opt a.wasm -O3 -o b.wasm

Now we have a pair a.wasm,b.wasm which we can test. However, we have run -O3
on both which is a little silly - the second -O3 might not actually have anything left
to do, which would mean we compare the same wasm to itself.

Worse, this is incorrect, as there are things we need to do only during the
generation phase, like --denan. We need that in order to generate a valid wasm to
test on, but it is "destructive" in itself: when removing NaNs (to avoid nondeterminism)
if replaces them with 0, which is different. As a result, running --denan when
generating the second wasm from the first could lead to different execution in them.
This was always a problem, but became more noticable recently now that DeNaN
modifies SIMD operations, as one optimization we do is to replace a memory.copy
with v128.load + v128.store, and --denan will make sure the loaded value has no
NaNs...

To fix this, separate the generation and optimization phase. Instead of

wasm-opt random.input -ttf -o a.wasm --denan -O3
wasm-opt a.wasm --denan -O3 -o b.wasm

(note how --denan -O3 appears twice), do this:

wasm-opt random.input -ttf -o a.wasm --denan
wasm-opt a.wasm -O3 -o b.wasm

(note how --denan appears in generation, and -O3 in optimization).
@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