Add differential fuzzing against wasmi (a Wasm interpreter). #2453
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
the
fuzzing
cfallin
force-pushed
the
differential-fuzz-interp
branch
from
1f47183
to
98b08e3
Compare
Subscribe to Label Actioncc @fitzgen
This issue or pull request has been labeled: "fuzzing"
Thus the following users have been cc'd because of the following labels:
To subscribe or unsubscribe from this label, edit the |
cfallin
force-pushed
the
differential-fuzz-interp
branch
from
84b205a
to
207ac97
Compare
cfallin
force-pushed
the
differential-fuzz-interp
branch
2 times, most recently
from
0cf19cc
to
d0fe898
Compare
|
Updated, thanks! It turns out that wasmi doesn't canonicalize NaNs (wasmi-labs/wasmi#19) so I've disabled fuzzing of modules with FP ops for now. If it seems worthwhile, we can add a This fails the "publish" check because it's using a crate-source patch in the root |
Sure thing. |
|
Done! |
cfallin
force-pushed
the
differential-fuzz-interp
branch
from
d0fe898
to
04d7f6d
Compare
|
Thanks! Updated. I also just added an |
cfallin
added a commit
to cfallin/wasmtime
that referenced
this pull request
Dec 2, 2020
This issue was found while fuzzing the new backend (bytecodealliance#2453); I suspect that it arises with the new backend because we can sink instructions (e.g. loads or extends) in more interesting ways than before, but I'm not entirely sure. Test coverage will be via the fuzz corpus once bytecodealliance#2453 lands.
github-actions
bot
added
the
wasmtime:api
Subscribe to Label Actioncc @peterhuene
This issue or pull request has been labeled: "wasmtime:api"
Thus the following users have been cc'd because of the following labels:
To subscribe or unsubscribe from this label, edit the |
cfallin
added a commit
to cfallin/wasmtime
that referenced
this pull request
Dec 2, 2020
A dynamic heap address computation may create up to two conditional branches: the usual bounds-check, but also (in some cases) an offset-addition overflow check. The x64 backend had reversed the condition code for this check, resulting in an always-trapping execution for a valid offset. I'm somewhat surprised this has existed so long, but I suppose the particular conditions (large offset, small offset guard, dynamic heap) have been somewhat rare in our testing so far. Found via fuzzing in bytecodealliance#2453.
cfallin
added a commit
to cfallin/wasmtime
that referenced
this pull request
Dec 2, 2020
- Sort by generated-code offset to maintain invariant and avoid gimli panic. - Fix srcloc interaction with branch peephole optimization in MachBuffer: if a srcloc range overlaps with a branch that is truncated, remove that srcloc range. These issues were found while fuzzing the new backend (bytecodealliance#2453); I suspect that they arise with the new backend because we can sink instructions (e.g. loads or extends) in more interesting ways than before, but I'm not entirely sure. Test coverage will be via the fuzz corpus once bytecodealliance#2453 lands.
alexcrichton
approved these changes
Dec 2, 2020
cfallin
added a commit
to cfallin/wasmtime
that referenced
this pull request
Dec 2, 2020
A dynamic heap address computation may create up to two conditional branches: the usual bounds-check, but also (in some cases) an offset-addition overflow check. The x64 backend had reversed the condition code for this check, resulting in an always-trapping execution for a valid offset. I'm somewhat surprised this has existed so long, but I suppose the particular conditions (large offset, small offset guard, dynamic heap) have been somewhat rare in our testing so far. Found via fuzzing in bytecodealliance#2453.
cfallin
added a commit
to cfallin/wasmtime
that referenced
this pull request
Dec 2, 2020
- Sort by generated-code offset to maintain invariant and avoid gimli panic. - Fix srcloc interaction with branch peephole optimization in MachBuffer: if a srcloc range overlaps with a branch that is truncated, remove that srcloc range. These issues were found while fuzzing the new backend (bytecodealliance#2453); I suspect that they arise with the new backend because we can sink instructions (e.g. loads or extends) in more interesting ways than before, but I'm not entirely sure. Test coverage will be via the fuzz corpus once bytecodealliance#2453 lands.
cfallin
added a commit
to cfallin/wasmtime
that referenced
this pull request
Dec 2, 2020
A dynamic heap address computation may create up to two conditional branches: the usual bounds-check, but also (in some cases) an offset-addition overflow check. The x64 backend had reversed the condition code for this check, resulting in an always-trapping execution for a valid offset. I'm somewhat surprised this has existed so long, but I suppose the particular conditions (large offset, small offset guard, dynamic heap) have been somewhat rare in our testing so far. Found via fuzzing in bytecodealliance#2453.
cfallin
added a commit
to cfallin/wasmtime
that referenced
this pull request
Dec 2, 2020
- Sort by generated-code offset to maintain invariant and avoid gimli panic. - Fix srcloc interaction with branch peephole optimization in MachBuffer: if a srcloc range overlaps with a branch that is truncated, remove that srcloc range. These issues were found while fuzzing the new backend (bytecodealliance#2453); I suspect that they arise with the new backend because we can sink instructions (e.g. loads or extends) in more interesting ways than before, but I'm not entirely sure. Test coverage will be via the fuzz corpus once bytecodealliance#2453 lands.
This PR adds a new fuzz target, `differential_wasmi`, that runs a Cranelift-based Wasm backend alongside a simple third-party Wasm interpeter crate (`wasmi`). The fuzzing runs the first function in a given module to completion on each side, and then diffs the return value and linear memory contents. This strategy should provide end-to-end coverage including both the Wasm translation to CLIF (which has seen some subtle and scary bugs at times), the lowering from CLIF to VCode, the register allocation, and the final code emission. This PR also adds a feature `experimental_x64` to the fuzzing crate (and the chain of dependencies down to `cranelift-codegen`) so that we can fuzz the new x86-64 backend as well as the current one.
cfallin
force-pushed
the
differential-fuzz-interp
branch
from
04d7f6d
to
bbdea06
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds a new fuzz target,
differential_wasmi, that runs aCranelift-based Wasm backend alongside a simple third-party Wasm
interpeter crate (
wasmi). The fuzzing runs the first function in agiven module to completion on each side, and then diffs the return value
and linear memory contents.
This strategy should provide end-to-end coverage including both the Wasm
translation to CLIF (which has seen some subtle and scary bugs at
times), the lowering from CLIF to VCode, the register allocation, and
the final code emission.