Fuzzer: Add a pass to prune illegal imports and exports for JS #6312
Conversation
| // JS cannot log v128 values (we trap on the boundary), but we must still | ||
| // provide an import so that we do not trap during linking. (Alternatively, | ||
| // we could avoid running JS on code with SIMD in it, but it is useful to | ||
| // fuzz such code as much as we can.) | ||
| 'log-v128': logValue, |
There was a problem hiding this comment.
Then what does it print for v128?
There was a problem hiding this comment.
It will trap atm if it is called. But this is forward-looking in that if the JS-wasm spec some day allows this without trapping - maybe it becomes a JS array of 4 elements? - then it would print that JS value.
| @@ -29,6 +29,11 @@ | |||
| // across modules, we still want to legalize dynCalls so JS can call into the | |||
| // tables even to a signature that is not legal. | |||
| // | |||
| // Another variation also "prunes" imports and exports that we cannot yet | |||
| // legalize, like exports and imports with SIMD or multivalue. Until we write | |||
There was a problem hiding this comment.
Why would we need to legalize or prune out multivalue? We should only be testing on JS engines that fully support it, right?
| func->body = | ||
| builder.makeConstantExpression(Literal::makeZeros(sig.results)); |
There was a problem hiding this comment.
What if the result type is not defaultable?
There was a problem hiding this comment.
Then we'd be in trouble, but we already avoid this situation in the fuzzer because it is a problem in general: We can't send a non-null reference from JS. So the fuzzer fixes that up.
We need to prune it because JS will trap if you call an export returning a multivalue like |
|
Looks like multivalue support for exports was added in node 16. I guess we could limit ourselves to only fuzzing that and onwards, but it seems simpler for now to just fix up things to run in all node. I added a comment. |
|
Agreed the benefit isn't very large. Though without this PR I now realize we'd also need to add code to properly receive multivalues on the JS side, compare them between VMs, etc., so this PR allows deferring some work. Worth doing all that but my main goal atm is GC fuzzing. |
…sembly#6312) We already have passes to legalize i64 imports and exports, which the fuzzer will run so that we can run wasm files in JS VMs. SIMD and multivalue also pose a problem as they trap on the boundary. In principle we could legalize them as well, but that is substantial effort, so instead just prune them: given a wasm module, remove any imports or exports that use SIMD or multivalue (or anything else that is not legal for JS). Running this in the fuzzer will allow us to not skip running v8 on any testcase we enable SIMD and multivalue for. (Multivalue is allowed in newer VMs, so that part of this PR could be removed eventually.) Also remove the limitation on running v8 with multimemory (v8 now supports that).
We already have passes to legalize i64 imports and exports, which the fuzzer will
run so that we can run wasm files in JS VMs. SIMD and multivalue also pose a
problem as they trap on the boundary. In principle we could legalize them as well,
but that is substantial effort, so instead just prune them: given a wasm module,
remove any imports or exports that use SIMD or multivalue (or anything else that
is not legal for JS).
Running this in the fuzzer will allow us to not skip running v8 on any testcase we
enable SIMD and multivalue for.
Also remove the limitation on running v8 with multimemory (v8 now supports
that).