From c0acd2f346ab7c1c5f0dda84cce8f2ab1010f1ff Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 25 Apr 2024 10:24:42 -0700 Subject: [PATCH 1/7] work --- src/tools/fuzzing.h | 1 + src/tools/fuzzing/fuzzing.cpp | 12 +++- ...e-to-fuzz_all-features_metrics_noprint.txt | 69 ++++++++++--------- 3 files changed, 47 insertions(+), 35 deletions(-) diff --git a/src/tools/fuzzing.h b/src/tools/fuzzing.h index dc17f7c91dd..017220b3e16 100644 --- a/src/tools/fuzzing.h +++ b/src/tools/fuzzing.h @@ -327,6 +327,7 @@ class TranslateToFuzzReader { Expression* makeStringNewArray(); Expression* makeStringNewCodePoint(); Expression* makeStringConcat(); + Expression* makeStringEq(Type type); Expression* makeStringEncode(Type type); // Similar to makeBasic/CompoundRef, but indicates that this value will be diff --git a/src/tools/fuzzing/fuzzing.cpp b/src/tools/fuzzing/fuzzing.cpp index 541b58b0fec..be7c87519fa 100644 --- a/src/tools/fuzzing/fuzzing.cpp +++ b/src/tools/fuzzing/fuzzing.cpp @@ -1368,7 +1368,8 @@ Expression* TranslateToFuzzReader::_makeConcrete(Type type) { &Self::makeI31Get); options.add(FeatureSet::ReferenceTypes | FeatureSet::GC | FeatureSet::Strings, - &Self::makeStringEncode); + &Self::makeStringEncode, + &Self::makeStringEq); } if (type.isTuple()) { options.add(FeatureSet::Multivalue, &Self::makeTupleMake); @@ -2817,6 +2818,15 @@ Expression* TranslateToFuzzReader::makeStringConcat() { return builder.makeStringConcat(left, right); } +Expression* TranslateToFuzzReader::makeStringEq(Type type) { + assert(type == Type::i32); + + auto* left = make(Type(HeapType::string, getNullability())); + auto* right = make(Type(HeapType::string, getNullability())); + auto op = pick(StringEqEqual, StringEqCompare); + return builder.makeStringEq(op, left, right); +} + Expression* TranslateToFuzzReader::makeTrappingRefUse(HeapType type) { auto percent = upTo(100); // Only give a low probability to emit a nullable reference. diff --git a/test/passes/translate-to-fuzz_all-features_metrics_noprint.txt b/test/passes/translate-to-fuzz_all-features_metrics_noprint.txt index 648cb053b1c..51dafcbdaee 100644 --- a/test/passes/translate-to-fuzz_all-features_metrics_noprint.txt +++ b/test/passes/translate-to-fuzz_all-features_metrics_noprint.txt @@ -1,43 +1,44 @@ total - [exports] : 7 - [funcs] : 12 + [exports] : 4 + [funcs] : 6 [globals] : 14 [imports] : 5 [memories] : 1 [memory-data] : 20 - [table-data] : 6 + [table-data] : 1 [tables] : 1 [tags] : 1 - [total] : 493 - [vars] : 31 - ArrayNew : 1 - ArrayNewFixed : 1 - Binary : 61 - Block : 59 - Break : 2 - Call : 29 - CallRef : 1 - Const : 101 - Drop : 10 - GlobalGet : 28 - GlobalSet : 26 - I31Get : 1 - If : 17 - Load : 16 - LocalGet : 36 - LocalSet : 29 - Loop : 2 - Nop : 5 - RefAs : 1 - RefCast : 2 - RefFunc : 8 - RefI31 : 4 - RefNull : 4 + [total] : 514 + [vars] : 24 + ArrayNew : 11 + ArrayNewFixed : 2 + AtomicFence : 1 + Binary : 79 + Block : 52 + Break : 4 + Call : 8 + Const : 137 + Drop : 5 + GlobalGet : 22 + GlobalSet : 22 + If : 13 + Load : 21 + LocalGet : 42 + LocalSet : 24 + Loop : 6 + Nop : 4 + RefAs : 3 + RefFunc : 2 + RefI31 : 2 + RefIsNull : 1 + RefNull : 5 Return : 3 SIMDExtract : 1 - StringConst : 1 - StructNew : 5 - TupleExtract : 1 - TupleMake : 7 - Unary : 16 - Unreachable : 15 + Store : 1 + StringConst : 2 + StructGet : 2 + StructNew : 9 + Switch : 1 + TupleMake : 6 + Unary : 11 + Unreachable : 12 From 8cd78be854168d9611de39e22b0d08614198be6e Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 25 Apr 2024 11:08:56 -0700 Subject: [PATCH 2/7] fix --- src/ir/effects.h | 10 +++++- test/lit/passes/vacuum-strings.wast | 53 +++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 1 deletion(-) create mode 100644 test/lit/passes/vacuum-strings.wast diff --git a/src/ir/effects.h b/src/ir/effects.h index 3ecb54641ff..6f4ede1a8d8 100644 --- a/src/ir/effects.h +++ b/src/ir/effects.h @@ -989,7 +989,15 @@ class EffectAnalyzer { // traps when an input is null. parent.implicitTrap = true; } - void visitStringEq(StringEq* curr) {} + void visitStringEq(StringEq* curr) { + if (curr->op == StringEqCompare) { + // traps when either input is null. + if (curr->left->type.isNullable() || + curr->right->type.isNullable()) { + parent.implicitTrap = true; + } + } + } void visitStringAs(StringAs* curr) { // traps when ref is null. parent.implicitTrap = true; diff --git a/test/lit/passes/vacuum-strings.wast b/test/lit/passes/vacuum-strings.wast new file mode 100644 index 00000000000..acbd98f2566 --- /dev/null +++ b/test/lit/passes/vacuum-strings.wast @@ -0,0 +1,53 @@ +;; NOTE: Assertions have been generated by update_lit_checks.py and should not be edited. +;; RUN: wasm-opt %s --vacuum -all -S -o - | filecheck %s + +(module + ;; CHECK: (func $0 (type $0) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (string.compare + ;; CHECK-NEXT: (string.const "hello") + ;; CHECK-NEXT: (ref.null none) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (string.compare + ;; CHECK-NEXT: (ref.null none) + ;; CHECK-NEXT: (string.const "world") + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (string.compare + ;; CHECK-NEXT: (ref.null none) + ;; CHECK-NEXT: (ref.null none) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $0 + ;; We cannot vacuum away compares that might trap. + (drop + (string.compare + (string.const "hello") + (string.const "world") + ) + ) + (drop + (string.compare + (string.const "hello") + (ref.null none) + ) + ) + (drop + (string.compare + (ref.null none) + (string.const "world") + ) + ) + (drop + (string.compare + (ref.null none) + (ref.null none) + ) + ) + ) +) + From 0e7ad06d6990b0778e2b6e109244070976b82abf Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 25 Apr 2024 11:21:58 -0700 Subject: [PATCH 3/7] fix --- src/tools/fuzzing.h | 2 +- src/tools/fuzzing/fuzzing.cpp | 30 +++++++++++++++++------------- 2 files changed, 18 insertions(+), 14 deletions(-) diff --git a/src/tools/fuzzing.h b/src/tools/fuzzing.h index 017220b3e16..c60744dea3f 100644 --- a/src/tools/fuzzing.h +++ b/src/tools/fuzzing.h @@ -399,7 +399,7 @@ class TranslateToFuzzReader { Nullability getSuperType(Nullability nullability); HeapType getSuperType(HeapType type); Type getSuperType(Type type); - Type getArrayTypeForString(); + HeapType getArrayTypeForString(); // Utilities Name getTargetName(Expression* target); diff --git a/src/tools/fuzzing/fuzzing.cpp b/src/tools/fuzzing/fuzzing.cpp index be7c87519fa..e681ff2d7a7 100644 --- a/src/tools/fuzzing/fuzzing.cpp +++ b/src/tools/fuzzing/fuzzing.cpp @@ -2756,7 +2756,7 @@ Expression* TranslateToFuzzReader::makeCompoundRef(Type type) { } Expression* TranslateToFuzzReader::makeStringNewArray() { - auto* array = make(getArrayTypeForString()); + auto* array = makeTrappingRefUse(getArrayTypeForString()); auto* start = make(Type::i32); auto* end = make(Type::i32); return builder.makeStringNew(StringNewWTF16Array, array, start, end, false); @@ -2813,18 +2813,24 @@ Expression* TranslateToFuzzReader::makeStringConst() { } Expression* TranslateToFuzzReader::makeStringConcat() { - auto* left = make(Type(HeapType::string, getNullability())); - auto* right = make(Type(HeapType::string, getNullability())); + auto* left = makeTrappingRefUse(HeapType::string); + auto* right = makeTrappingRefUse(HeapType::string); return builder.makeStringConcat(left, right); } Expression* TranslateToFuzzReader::makeStringEq(Type type) { assert(type == Type::i32); - auto* left = make(Type(HeapType::string, getNullability())); - auto* right = make(Type(HeapType::string, getNullability())); - auto op = pick(StringEqEqual, StringEqCompare); - return builder.makeStringEq(op, left, right); + if (oneIn(2)) { + auto* left = make(Type(HeapType::string, getNullability())); + auto* right = make(Type(HeapType::string, getNullability())); + return builder.makeStringEq(StringEqEqual, left, right); + } + + // string.compare may trap if the either input is null. + auto* left = makeTrappingRefUse(HeapType::string); + auto* right = makeTrappingRefUse(HeapType::string); + return builder.makeStringEq(StringEqCompare, left, right); } Expression* TranslateToFuzzReader::makeTrappingRefUse(HeapType type) { @@ -3930,8 +3936,8 @@ Expression* TranslateToFuzzReader::makeArrayBulkMemoryOp(Type type) { Expression* TranslateToFuzzReader::makeStringEncode(Type type) { assert(type == Type::i32); - auto* ref = make(Type(HeapType::string, getNullability())); - auto* array = make(getArrayTypeForString()); + auto* ref = makeTrappingRefUse(HeapType::string); + auto* array = makeTrappingRefUse(getArrayTypeForString()); auto* start = make(Type::i32); // Only rarely emit without a bounds check, which might trap. See related @@ -4317,12 +4323,10 @@ Type TranslateToFuzzReader::getSuperType(Type type) { return superType; } -Type TranslateToFuzzReader::getArrayTypeForString() { +HeapType TranslateToFuzzReader::getArrayTypeForString() { // Emit an array that can be used with JS-style strings, containing 16-bit // elements. For now, this must be a mutable type as that is all V8 accepts. - auto arrayHeapType = HeapType(Array(Field(Field::PackedType::i16, Mutable))); - auto nullability = getNullability(); - return Type(arrayHeapType, nullability); + Return HeapType(Array(Field(Field::PackedType::i16, Mutable))); } Name TranslateToFuzzReader::getTargetName(Expression* target) { From 815298bdb5e6e53c600f01c36aac949a702bf535 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 25 Apr 2024 11:22:35 -0700 Subject: [PATCH 4/7] fix --- src/tools/fuzzing/fuzzing.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/fuzzing/fuzzing.cpp b/src/tools/fuzzing/fuzzing.cpp index e681ff2d7a7..add0494ead8 100644 --- a/src/tools/fuzzing/fuzzing.cpp +++ b/src/tools/fuzzing/fuzzing.cpp @@ -4326,7 +4326,7 @@ Type TranslateToFuzzReader::getSuperType(Type type) { HeapType TranslateToFuzzReader::getArrayTypeForString() { // Emit an array that can be used with JS-style strings, containing 16-bit // elements. For now, this must be a mutable type as that is all V8 accepts. - Return HeapType(Array(Field(Field::PackedType::i16, Mutable))); + return HeapType(Array(Field(Field::PackedType::i16, Mutable))); } Name TranslateToFuzzReader::getTargetName(Expression* target) { From 7b155a763c18f2a7efee36ea8a68912a5bf25af2 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 25 Apr 2024 11:22:51 -0700 Subject: [PATCH 5/7] format --- src/ir/effects.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/ir/effects.h b/src/ir/effects.h index 6f4ede1a8d8..ef9aceb37cd 100644 --- a/src/ir/effects.h +++ b/src/ir/effects.h @@ -992,8 +992,7 @@ class EffectAnalyzer { void visitStringEq(StringEq* curr) { if (curr->op == StringEqCompare) { // traps when either input is null. - if (curr->left->type.isNullable() || - curr->right->type.isNullable()) { + if (curr->left->type.isNullable() || curr->right->type.isNullable()) { parent.implicitTrap = true; } } From 22e5556f62240e146adadc20925e56e10a050e8c Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 25 Apr 2024 11:36:07 -0700 Subject: [PATCH 6/7] test --- test/lit/passes/vacuum-strings.wast | 35 +++++++++++++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/test/lit/passes/vacuum-strings.wast b/test/lit/passes/vacuum-strings.wast index acbd98f2566..6925d52e6a7 100644 --- a/test/lit/passes/vacuum-strings.wast +++ b/test/lit/passes/vacuum-strings.wast @@ -2,7 +2,7 @@ ;; RUN: wasm-opt %s --vacuum -all -S -o - | filecheck %s (module - ;; CHECK: (func $0 (type $0) + ;; CHECK: (func $compare (type $0) ;; CHECK-NEXT: (drop ;; CHECK-NEXT: (string.compare ;; CHECK-NEXT: (string.const "hello") @@ -22,7 +22,7 @@ ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) - (func $0 + (func $compare ;; We cannot vacuum away compares that might trap. (drop (string.compare @@ -49,5 +49,36 @@ ) ) ) + + ;; CHECK: (func $eq (type $0) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: ) + (func $eq + ;; Equals, however, never traps so all these can be removed. + (drop + (string.eq + (string.const "hello") + (string.const "world") + ) + ) + (drop + (string.eq + (string.const "hello") + (ref.null none) + ) + ) + (drop + (string.eq + (ref.null none) + (string.const "world") + ) + ) + (drop + (string.eq + (ref.null none) + (ref.null none) + ) + ) + ) ) From 85f75324836127bf37545e1914fb29fce48f87bc Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 25 Apr 2024 11:37:20 -0700 Subject: [PATCH 7/7] test --- ...e-to-fuzz_all-features_metrics_noprint.txt | 43 ++++++++++--------- 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/test/passes/translate-to-fuzz_all-features_metrics_noprint.txt b/test/passes/translate-to-fuzz_all-features_metrics_noprint.txt index 51dafcbdaee..20b145a95e9 100644 --- a/test/passes/translate-to-fuzz_all-features_metrics_noprint.txt +++ b/test/passes/translate-to-fuzz_all-features_metrics_noprint.txt @@ -8,37 +8,38 @@ total [table-data] : 1 [tables] : 1 [tags] : 1 - [total] : 514 + [total] : 533 [vars] : 24 ArrayNew : 11 ArrayNewFixed : 2 AtomicFence : 1 Binary : 79 - Block : 52 - Break : 4 + Block : 57 + Break : 6 Call : 8 Const : 137 - Drop : 5 - GlobalGet : 22 - GlobalSet : 22 - If : 13 - Load : 21 - LocalGet : 42 - LocalSet : 24 - Loop : 6 + Drop : 1 + GlobalGet : 28 + GlobalSet : 28 + If : 15 + Load : 17 + LocalGet : 40 + LocalSet : 23 + Loop : 9 Nop : 4 - RefAs : 3 + RefAs : 1 RefFunc : 2 RefI31 : 2 RefIsNull : 1 RefNull : 5 - Return : 3 - SIMDExtract : 1 - Store : 1 - StringConst : 2 - StructGet : 2 + Return : 2 + SIMDExtract : 2 + Select : 1 + Store : 2 + StringConst : 4 + StringEq : 1 + StructGet : 1 StructNew : 9 - Switch : 1 - TupleMake : 6 - Unary : 11 - Unreachable : 12 + TupleMake : 4 + Unary : 15 + Unreachable : 15