Skip to content

Commit 1441bcb

Browse files
authored
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).
1 parent 0ecea77 commit 1441bcb

File tree

8 files changed

+316
-3
lines changed

8 files changed

+316
-3
lines changed

scripts/fuzz_opt.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,7 @@ def randomize_fuzz_settings():
188188
FUZZ_OPTS += ['--no-fuzz-oob']
189189
if random.random() < 0.5:
190190
LEGALIZE = True
191-
FUZZ_OPTS += ['--legalize-js-interface']
191+
FUZZ_OPTS += ['--legalize-and-prune-js-interface']
192192
else:
193193
LEGALIZE = False
194194

@@ -926,7 +926,7 @@ def compare_before_and_after(self, before, after):
926926
compare(before[vm], after[vm], 'CompareVMs between before and after: ' + vm.name)
927927

928928
def can_run_on_feature_opts(self, feature_opts):
929-
return all_disallowed(['simd', 'multivalue', 'multimemory'])
929+
return True
930930

931931

932932
# Check for determinism - the same command must have the same output.
@@ -957,7 +957,7 @@ def handle_pair(self, input, before_wasm, after_wasm, opts):
957957
# later make sense (if we don't do this, the wasm may have i64 exports).
958958
# after applying other necessary fixes, we'll recreate the after wasm
959959
# from scratch.
960-
run([in_bin('wasm-opt'), before_wasm, '--legalize-js-interface', '-o', before_wasm_temp] + FEATURE_OPTS)
960+
run([in_bin('wasm-opt'), before_wasm, '--legalize-and-prune-js-interface', '-o', before_wasm_temp] + FEATURE_OPTS)
961961
compare_before_to_after = random.random() < 0.5
962962
compare_to_interpreter = compare_before_to_after and random.random() < 0.5
963963
if compare_before_to_after:

scripts/fuzz_shell.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,11 @@ var imports = {
7373
'log-i64': logValue,
7474
'log-f32': logValue,
7575
'log-f64': logValue,
76+
// JS cannot log v128 values (we trap on the boundary), but we must still
77+
// provide an import so that we do not trap during linking. (Alternatively,
78+
// we could avoid running JS on code with SIMD in it, but it is useful to
79+
// fuzz such code as much as we can.)
80+
'log-v128': logValue,
7681
},
7782
'env': {
7883
'setTempRet0': function(x) { tempRet0 = x },

src/passes/LegalizeJSInterface.cpp

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,12 @@
2929
// across modules, we still want to legalize dynCalls so JS can call into the
3030
// tables even to a signature that is not legal.
3131
//
32+
// Another variation also "prunes" imports and exports that we cannot yet
33+
// legalize, like exports and imports with SIMD or multivalue. Until we write
34+
// the logic to legalize them, removing those imports/exports still allows us to
35+
// fuzz all the legal imports/exports. (Note that multivalue is supported in
36+
// exports in newer VMs - node 16+ - so that part is only needed for older VMs.)
37+
//
3238

3339
#include "asmjs/shared-constants.h"
3440
#include "ir/element-utils.h"
@@ -43,6 +49,8 @@
4349

4450
namespace wasm {
4551

52+
namespace {
53+
4654
// These are aliases for getTempRet0/setTempRet0 which emscripten defines in
4755
// compiler-rt and exports under these names.
4856
static Name GET_TEMP_RET_EXPORT("__get_temp_ret");
@@ -358,10 +366,88 @@ struct LegalizeJSInterface : public Pass {
358366
}
359367
};
360368

369+
struct LegalizeAndPruneJSInterface : public LegalizeJSInterface {
370+
// Legalize fully (true) and add pruning on top.
371+
LegalizeAndPruneJSInterface() : LegalizeJSInterface(true) {}
372+
373+
void run(Module* module) override {
374+
LegalizeJSInterface::run(module);
375+
376+
prune(module);
377+
}
378+
379+
void prune(Module* module) {
380+
// For each function name, the exported id it is exported with. For
381+
// example,
382+
//
383+
// (func $foo (export "bar")
384+
//
385+
// Would have exportedFunctions["foo"] = "bar";
386+
std::unordered_map<Name, Name> exportedFunctions;
387+
for (auto& exp : module->exports) {
388+
if (exp->kind == ExternalKind::Function) {
389+
exportedFunctions[exp->value] = exp->name;
390+
}
391+
}
392+
393+
for (auto& func : module->functions) {
394+
// If the function is neither exported nor imported, no problem.
395+
auto imported = func->imported();
396+
auto exported = exportedFunctions.count(func->name);
397+
if (!imported && !exported) {
398+
continue;
399+
}
400+
401+
// The params are allowed to be multivalue, but not the results. Otherwise
402+
// look for SIMD.
403+
auto sig = func->type.getSignature();
404+
auto illegal = isIllegal(sig.results);
405+
illegal =
406+
illegal || std::any_of(sig.params.begin(),
407+
sig.params.end(),
408+
[&](const Type& t) { return isIllegal(t); });
409+
if (!illegal) {
410+
continue;
411+
}
412+
413+
// Prune an import by implementing it in a trivial manner.
414+
if (imported) {
415+
func->module = func->base = Name();
416+
417+
Builder builder(*module);
418+
if (sig.results == Type::none) {
419+
func->body = builder.makeNop();
420+
} else {
421+
func->body =
422+
builder.makeConstantExpression(Literal::makeZeros(sig.results));
423+
}
424+
}
425+
426+
// Prune an export by just removing it.
427+
if (exported) {
428+
module->removeExport(exportedFunctions[func->name]);
429+
}
430+
}
431+
432+
// TODO: globals etc.
433+
}
434+
435+
bool isIllegal(Type type) {
436+
auto features = type.getFeatures();
437+
return features.hasSIMD() || features.hasMultivalue();
438+
}
439+
};
440+
441+
} // anonymous namespace
442+
361443
Pass* createLegalizeJSInterfacePass() { return new LegalizeJSInterface(true); }
362444

363445
Pass* createLegalizeJSInterfaceMinimallyPass() {
364446
return new LegalizeJSInterface(false);
365447
}
366448

449+
Pass* createLegalizeAndPruneJSInterfacePass() {
450+
return new LegalizeAndPruneJSInterface();
451+
}
452+
367453
} // namespace wasm

src/passes/pass.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,9 @@ void PassRegistry::registerPasses() {
221221
"legalizes i64 types on the import/export boundary in a minimal "
222222
"manner, only on things only JS will call",
223223
createLegalizeJSInterfaceMinimallyPass);
224+
registerPass("legalize-and-prune-js-interface",
225+
"legalizes the import/export boundary and prunes when needed",
226+
createLegalizeAndPruneJSInterfacePass);
224227
registerPass("local-cse",
225228
"common subexpression elimination inside basic blocks",
226229
createLocalCSEPass);

src/passes/passes.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ Pass* createInliningPass();
6767
Pass* createInliningOptimizingPass();
6868
Pass* createJSPIPass();
6969
Pass* createJ2CLOptsPass();
70+
Pass* createLegalizeAndPruneJSInterfacePass();
7071
Pass* createLegalizeJSInterfacePass();
7172
Pass* createLegalizeJSInterfaceMinimallyPass();
7273
Pass* createLimitSegmentsPass();

test/lit/help/wasm-opt.test

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,9 @@
225225
;; CHECK-NEXT: --jspi wrap imports and exports for
226226
;; CHECK-NEXT: JavaScript promise integration
227227
;; CHECK-NEXT:
228+
;; CHECK-NEXT: --legalize-and-prune-js-interface legalizes the import/export
229+
;; CHECK-NEXT: boundary and prunes when needed
230+
;; CHECK-NEXT:
228231
;; CHECK-NEXT: --legalize-js-interface legalizes i64 types on the
229232
;; CHECK-NEXT: import/export boundary
230233
;; CHECK-NEXT:

test/lit/help/wasm2js.test

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,9 @@
179179
;; CHECK-NEXT: --jspi wrap imports and exports for
180180
;; CHECK-NEXT: JavaScript promise integration
181181
;; CHECK-NEXT:
182+
;; CHECK-NEXT: --legalize-and-prune-js-interface legalizes the import/export
183+
;; CHECK-NEXT: boundary and prunes when needed
184+
;; CHECK-NEXT:
182185
;; CHECK-NEXT: --legalize-js-interface legalizes i64 types on the
183186
;; CHECK-NEXT: import/export boundary
184187
;; CHECK-NEXT:

0 commit comments

Comments
 (0)