diff options
author | Alon Zakai <azakai@google.com> | 2024-02-20 16:49:37 -0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-02-20 16:49:37 -0800 |
commit | 1441bcbb33a5354432daf1edc4bc3a72d0bd7687 (patch) | |
tree | e14d7aee926edad11d315ac4ebbc0988243abad7 | |
parent | 0ecea77e9fd3dd42b5cd269ce64b6072691cfb52 (diff) | |
download | binaryen-1441bcbb33a5354432daf1edc4bc3a72d0bd7687.tar.gz binaryen-1441bcbb33a5354432daf1edc4bc3a72d0bd7687.tar.bz2 binaryen-1441bcbb33a5354432daf1edc4bc3a72d0bd7687.zip |
Fuzzer: Add a pass to prune illegal imports and exports for JS (#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).
-rwxr-xr-x | scripts/fuzz_opt.py | 6 | ||||
-rw-r--r-- | scripts/fuzz_shell.js | 5 | ||||
-rw-r--r-- | src/passes/LegalizeJSInterface.cpp | 86 | ||||
-rw-r--r-- | src/passes/pass.cpp | 3 | ||||
-rw-r--r-- | src/passes/passes.h | 1 | ||||
-rw-r--r-- | test/lit/help/wasm-opt.test | 3 | ||||
-rw-r--r-- | test/lit/help/wasm2js.test | 3 | ||||
-rw-r--r-- | test/lit/passes/legalize-and-prune-js-interface.wast | 212 |
8 files changed, 316 insertions, 3 deletions
diff --git a/scripts/fuzz_opt.py b/scripts/fuzz_opt.py index 8dc87cb7a..c5498822e 100755 --- a/scripts/fuzz_opt.py +++ b/scripts/fuzz_opt.py @@ -188,7 +188,7 @@ def randomize_fuzz_settings(): FUZZ_OPTS += ['--no-fuzz-oob'] if random.random() < 0.5: LEGALIZE = True - FUZZ_OPTS += ['--legalize-js-interface'] + FUZZ_OPTS += ['--legalize-and-prune-js-interface'] else: LEGALIZE = False @@ -926,7 +926,7 @@ class CompareVMs(TestCaseHandler): compare(before[vm], after[vm], 'CompareVMs between before and after: ' + vm.name) def can_run_on_feature_opts(self, feature_opts): - return all_disallowed(['simd', 'multivalue', 'multimemory']) + return True # Check for determinism - the same command must have the same output. @@ -957,7 +957,7 @@ class Wasm2JS(TestCaseHandler): # later make sense (if we don't do this, the wasm may have i64 exports). # after applying other necessary fixes, we'll recreate the after wasm # from scratch. - run([in_bin('wasm-opt'), before_wasm, '--legalize-js-interface', '-o', before_wasm_temp] + FEATURE_OPTS) + run([in_bin('wasm-opt'), before_wasm, '--legalize-and-prune-js-interface', '-o', before_wasm_temp] + FEATURE_OPTS) compare_before_to_after = random.random() < 0.5 compare_to_interpreter = compare_before_to_after and random.random() < 0.5 if compare_before_to_after: diff --git a/scripts/fuzz_shell.js b/scripts/fuzz_shell.js index 743e838b5..736110751 100644 --- a/scripts/fuzz_shell.js +++ b/scripts/fuzz_shell.js @@ -73,6 +73,11 @@ var imports = { 'log-i64': logValue, 'log-f32': logValue, 'log-f64': logValue, + // 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, }, 'env': { 'setTempRet0': function(x) { tempRet0 = x }, diff --git a/src/passes/LegalizeJSInterface.cpp b/src/passes/LegalizeJSInterface.cpp index 3ca6b7095..0082ba7ab 100644 --- a/src/passes/LegalizeJSInterface.cpp +++ b/src/passes/LegalizeJSInterface.cpp @@ -29,6 +29,12 @@ // 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 +// the logic to legalize them, removing those imports/exports still allows us to +// fuzz all the legal imports/exports. (Note that multivalue is supported in +// exports in newer VMs - node 16+ - so that part is only needed for older VMs.) +// #include "asmjs/shared-constants.h" #include "ir/element-utils.h" @@ -43,6 +49,8 @@ namespace wasm { +namespace { + // These are aliases for getTempRet0/setTempRet0 which emscripten defines in // compiler-rt and exports under these names. static Name GET_TEMP_RET_EXPORT("__get_temp_ret"); @@ -358,10 +366,88 @@ private: } }; +struct LegalizeAndPruneJSInterface : public LegalizeJSInterface { + // Legalize fully (true) and add pruning on top. + LegalizeAndPruneJSInterface() : LegalizeJSInterface(true) {} + + void run(Module* module) override { + LegalizeJSInterface::run(module); + + prune(module); + } + + void prune(Module* module) { + // For each function name, the exported id it is exported with. For + // example, + // + // (func $foo (export "bar") + // + // Would have exportedFunctions["foo"] = "bar"; + std::unordered_map<Name, Name> exportedFunctions; + for (auto& exp : module->exports) { + if (exp->kind == ExternalKind::Function) { + exportedFunctions[exp->value] = exp->name; + } + } + + for (auto& func : module->functions) { + // If the function is neither exported nor imported, no problem. + auto imported = func->imported(); + auto exported = exportedFunctions.count(func->name); + if (!imported && !exported) { + continue; + } + + // The params are allowed to be multivalue, but not the results. Otherwise + // look for SIMD. + auto sig = func->type.getSignature(); + auto illegal = isIllegal(sig.results); + illegal = + illegal || std::any_of(sig.params.begin(), + sig.params.end(), + [&](const Type& t) { return isIllegal(t); }); + if (!illegal) { + continue; + } + + // Prune an import by implementing it in a trivial manner. + if (imported) { + func->module = func->base = Name(); + + Builder builder(*module); + if (sig.results == Type::none) { + func->body = builder.makeNop(); + } else { + func->body = + builder.makeConstantExpression(Literal::makeZeros(sig.results)); + } + } + + // Prune an export by just removing it. + if (exported) { + module->removeExport(exportedFunctions[func->name]); + } + } + + // TODO: globals etc. + } + + bool isIllegal(Type type) { + auto features = type.getFeatures(); + return features.hasSIMD() || features.hasMultivalue(); + } +}; + +} // anonymous namespace + Pass* createLegalizeJSInterfacePass() { return new LegalizeJSInterface(true); } Pass* createLegalizeJSInterfaceMinimallyPass() { return new LegalizeJSInterface(false); } +Pass* createLegalizeAndPruneJSInterfacePass() { + return new LegalizeAndPruneJSInterface(); +} + } // namespace wasm diff --git a/src/passes/pass.cpp b/src/passes/pass.cpp index 1fe81cbfc..6faf77276 100644 --- a/src/passes/pass.cpp +++ b/src/passes/pass.cpp @@ -221,6 +221,9 @@ void PassRegistry::registerPasses() { "legalizes i64 types on the import/export boundary in a minimal " "manner, only on things only JS will call", createLegalizeJSInterfaceMinimallyPass); + registerPass("legalize-and-prune-js-interface", + "legalizes the import/export boundary and prunes when needed", + createLegalizeAndPruneJSInterfacePass); registerPass("local-cse", "common subexpression elimination inside basic blocks", createLocalCSEPass); diff --git a/src/passes/passes.h b/src/passes/passes.h index c3ab7773f..2f188a689 100644 --- a/src/passes/passes.h +++ b/src/passes/passes.h @@ -67,6 +67,7 @@ Pass* createInliningPass(); Pass* createInliningOptimizingPass(); Pass* createJSPIPass(); Pass* createJ2CLOptsPass(); +Pass* createLegalizeAndPruneJSInterfacePass(); Pass* createLegalizeJSInterfacePass(); Pass* createLegalizeJSInterfaceMinimallyPass(); Pass* createLimitSegmentsPass(); diff --git a/test/lit/help/wasm-opt.test b/test/lit/help/wasm-opt.test index 5e3a322ed..0cf76e52e 100644 --- a/test/lit/help/wasm-opt.test +++ b/test/lit/help/wasm-opt.test @@ -225,6 +225,9 @@ ;; CHECK-NEXT: --jspi wrap imports and exports for ;; CHECK-NEXT: JavaScript promise integration ;; CHECK-NEXT: +;; CHECK-NEXT: --legalize-and-prune-js-interface legalizes the import/export +;; CHECK-NEXT: boundary and prunes when needed +;; CHECK-NEXT: ;; CHECK-NEXT: --legalize-js-interface legalizes i64 types on the ;; CHECK-NEXT: import/export boundary ;; CHECK-NEXT: diff --git a/test/lit/help/wasm2js.test b/test/lit/help/wasm2js.test index aadefe8fc..9804718a8 100644 --- a/test/lit/help/wasm2js.test +++ b/test/lit/help/wasm2js.test @@ -179,6 +179,9 @@ ;; CHECK-NEXT: --jspi wrap imports and exports for ;; CHECK-NEXT: JavaScript promise integration ;; CHECK-NEXT: +;; CHECK-NEXT: --legalize-and-prune-js-interface legalizes the import/export +;; CHECK-NEXT: boundary and prunes when needed +;; CHECK-NEXT: ;; CHECK-NEXT: --legalize-js-interface legalizes i64 types on the ;; CHECK-NEXT: import/export boundary ;; CHECK-NEXT: diff --git a/test/lit/passes/legalize-and-prune-js-interface.wast b/test/lit/passes/legalize-and-prune-js-interface.wast new file mode 100644 index 000000000..1683818a2 --- /dev/null +++ b/test/lit/passes/legalize-and-prune-js-interface.wast @@ -0,0 +1,212 @@ +;; NOTE: Assertions have been generated by update_lit_checks.py --all-items and should not be edited. +;; NOTE: This test was ported using port_passes_tests_to_lit.py and could be cleaned up. + +;; RUN: foreach %s %t wasm-opt -all --legalize-and-prune-js-interface -S -o - | filecheck %s + +(module + (import "env" "imported-64" (func $imported-64 (param i32 f64) (result i64))) + + (import "env" "imported-v128" (func $imported-v128 (result v128))) + + (import "env" "imported-mv" (func $imported-mv (result i32 f64))) + + (import "env" "imported-v128-param" (func $imported-v128-param (param v128) (result i32))) + + (import "env" "imported-v128-param-noresult" (func $imported-v128-param-noresult (param v128))) + + ;; CHECK: (type $0 (func (result v128))) + + ;; CHECK: (type $1 (func (result i32 f64))) + + ;; CHECK: (type $2 (func (param v128) (result i32))) + + ;; CHECK: (type $3 (func (param v128))) + + ;; CHECK: (type $4 (func)) + + ;; CHECK: (type $5 (func (result i32))) + + ;; CHECK: (type $6 (func (param i32 f64) (result i64))) + + ;; CHECK: (type $7 (func (param i32 f64) (result i32))) + + ;; CHECK: (import "env" "getTempRet0" (func $getTempRet0 (type $5) (result i32))) + + ;; CHECK: (import "env" "imported-64" (func $legalimport$imported-64 (type $7) (param i32 f64) (result i32))) + + ;; CHECK: (func $imported-v128 (type $0) (result v128) + ;; CHECK-NEXT: (v128.const i32x4 0x00000000 0x00000000 0x00000000 0x00000000) + ;; CHECK-NEXT: ) + + ;; CHECK: (func $imported-mv (type $1) (result i32 f64) + ;; CHECK-NEXT: (tuple.make 2 + ;; CHECK-NEXT: (i32.const 0) + ;; CHECK-NEXT: (f64.const 0) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + + ;; CHECK: (func $imported-v128-param (type $2) (param $0 v128) (result i32) + ;; CHECK-NEXT: (i32.const 0) + ;; CHECK-NEXT: ) + + ;; CHECK: (func $imported-v128-param-noresult (type $3) (param $0 v128) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: ) + + ;; CHECK: (func $call-64 (type $4) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (call $legalfunc$imported-64 + ;; CHECK-NEXT: (i32.const 0) + ;; CHECK-NEXT: (f64.const 1.2) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (call $imported-v128) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (tuple.drop 2 + ;; CHECK-NEXT: (call $imported-mv) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (call $imported-v128-param + ;; CHECK-NEXT: (v128.const i32x4 0x00000000 0x00000000 0x00000000 0x00000000) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (call $imported-v128-param-noresult + ;; CHECK-NEXT: (v128.const i32x4 0x00000000 0x00000000 0x00000000 0x00000000) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $call-64 + ;; This import has an i64 which will be legalized, the same as in the + ;; normal --legalize-js-interface pass. Note how the multiple params are not + ;; an issue for us. + (drop (call $imported-64 + (i32.const 0) + (f64.const 1.2) + )) + + ;; This import uses SIMD, which we must prune from the list of imports. + ;; We'll define it in a trivial manner inside the module instead. + (drop (call $imported-v128)) + + ;; Ditto, but with multivalue. + (tuple.drop 2 (call $imported-mv)) + + ;; Ditto, but now the illegal thing is a param. + (drop (call $imported-v128-param + (v128.const i32x4 0 0 0 0) + )) + + ;; Ditto, but no result this time. + (call $imported-v128-param-noresult + (v128.const i32x4 0 0 0 0) + ) + ) +) + +;; CHECK: (func $legalfunc$imported-64 (type $6) (param $0 i32) (param $1 f64) (result i64) +;; CHECK-NEXT: (i64.or +;; CHECK-NEXT: (i64.extend_i32_u +;; CHECK-NEXT: (call $legalimport$imported-64 +;; CHECK-NEXT: (local.get $0) +;; CHECK-NEXT: (local.get $1) +;; CHECK-NEXT: ) +;; CHECK-NEXT: ) +;; CHECK-NEXT: (i64.shl +;; CHECK-NEXT: (i64.extend_i32_u +;; CHECK-NEXT: (call $getTempRet0) +;; CHECK-NEXT: ) +;; CHECK-NEXT: (i64.const 32) +;; CHECK-NEXT: ) +;; CHECK-NEXT: ) +;; CHECK-NEXT: ) +(module + ;; CHECK: (type $0 (func (param i64) (result i64))) + + ;; CHECK: (type $1 (func (param v128))) + + ;; CHECK: (type $2 (func (result v128))) + + ;; CHECK: (type $3 (func (result i32 i32))) + + ;; CHECK: (type $4 (func (param i32))) + + ;; CHECK: (type $5 (func (param i32 i32) (result i32))) + + ;; CHECK: (import "env" "setTempRet0" (func $setTempRet0 (type $4) (param i32))) + + ;; CHECK: (export "export-64" (func $legalstub$export-64)) + + ;; CHECK: (func $export-64 (type $0) (param $x i64) (result i64) + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: ) + (func $export-64 (export "export-64") (param $x i64) (result i64) + ;; This can be legalized. Note we have two params, but that's no problem. + (unreachable) + ) + + ;; CHECK: (func $export-v128 (type $1) (param $x v128) + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: ) + (func $export-v128 (export "export-v128") (param $x v128) + ;; This will be pruned. + (unreachable) + ) + + ;; CHECK: (func $export-v128-result (type $2) (result v128) + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: ) + (func $export-v128-result (export "export-v128-result") (result v128) + ;; This will be pruned. + (unreachable) + ) + + ;; CHECK: (func $export-mv (type $3) (result i32 i32) + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: ) + (func $export-mv (export "export-mv") (result i32 i32) + ;; This will be pruned. + (unreachable) + ) +) + +;; CHECK: (func $legalstub$export-64 (type $5) (param $0 i32) (param $1 i32) (result i32) +;; CHECK-NEXT: (local $2 i64) +;; CHECK-NEXT: (local.set $2 +;; CHECK-NEXT: (call $export-64 +;; CHECK-NEXT: (i64.or +;; CHECK-NEXT: (i64.extend_i32_u +;; CHECK-NEXT: (local.get $0) +;; CHECK-NEXT: ) +;; CHECK-NEXT: (i64.shl +;; CHECK-NEXT: (i64.extend_i32_u +;; CHECK-NEXT: (local.get $1) +;; CHECK-NEXT: ) +;; CHECK-NEXT: (i64.const 32) +;; CHECK-NEXT: ) +;; CHECK-NEXT: ) +;; CHECK-NEXT: ) +;; CHECK-NEXT: ) +;; CHECK-NEXT: (call $setTempRet0 +;; CHECK-NEXT: (i32.wrap_i64 +;; CHECK-NEXT: (i64.shr_u +;; CHECK-NEXT: (local.get $2) +;; CHECK-NEXT: (i64.const 32) +;; CHECK-NEXT: ) +;; CHECK-NEXT: ) +;; CHECK-NEXT: ) +;; CHECK-NEXT: (i32.wrap_i64 +;; CHECK-NEXT: (local.get $2) +;; CHECK-NEXT: ) +;; CHECK-NEXT: ) +(module + (import "env" "imported-v128" (func $imported-v128 (result v128))) + + ;; The import is also exported. We will both implement it with a trivial body + ;; and also prune the export, so it remains neither an import nor an export. + (export "imported-v128" (func $imported-v128)) +) +;; CHECK: (type $0 (func (result v128))) + +;; CHECK: (func $imported-v128 (type $0) (result v128) +;; CHECK-NEXT: (v128.const i32x4 0x00000000 0x00000000 0x00000000 0x00000000) +;; CHECK-NEXT: ) |