diff options
| -rw-r--r-- | src/tools/fuzzing.h | 6 | ||||
| -rw-r--r-- | src/tools/fuzzing/fuzzing.cpp | 73 | ||||
| -rw-r--r-- | test/passes/translate-to-fuzz_all-features_metrics_noprint.txt | 63 |
3 files changed, 87 insertions, 55 deletions
diff --git a/src/tools/fuzzing.h b/src/tools/fuzzing.h index c0f6ab335..abc0a4248 100644 --- a/src/tools/fuzzing.h +++ b/src/tools/fuzzing.h @@ -302,6 +302,7 @@ private: Expression* makeLoad(Type type); Expression* makeNonAtomicStore(Type type); Expression* makeStore(Type type); + // Makes a small change to a constant value. Literal tweak(Literal value); Literal makeLiteral(Type type); @@ -318,6 +319,11 @@ private: Expression* makeBasicRef(Type type); Expression* makeCompoundRef(Type type); + // Similar to makeBasic/CompoundRef, but indicates that this value will be + // used in a place that will trap on null. For example, the reference of a + // struct.get or array.set would use this. + Expression* makeTrappingRefUse(HeapType type); + Expression* buildUnary(const UnaryArgs& args); Expression* makeUnary(Type type); Expression* buildBinary(const BinaryArgs& args); diff --git a/src/tools/fuzzing/fuzzing.cpp b/src/tools/fuzzing/fuzzing.cpp index 062e66260..c88b2f13d 100644 --- a/src/tools/fuzzing/fuzzing.cpp +++ b/src/tools/fuzzing/fuzzing.cpp @@ -2316,11 +2316,19 @@ Expression* TranslateToFuzzReader::makeCompoundRef(Type type) { return builder.makeRefNull(heapType); } - // If the type is non-nullable, and we've run out of input, emit a cast to - // make this validate, but it will trap at runtime which is not ideal. This at - // least avoids infinite recursion here, and we emit a valid (but not that - // useful) wasm. + // If the type is non-nullable, but we've run out of input, then we need to do + // something here to avoid infinite recursion. In the worse case we'll emit a + // cast to non-null of a null, which validates, but it will trap at runtime + // which is not ideal. if (type.isNonNullable() && (random.finished() || nesting >= LIMIT)) { + // If we have a function context then we can at least emit a local.get, + // perhaps, which is less bad. Note that we need to check typeLocals + // manually here to avoid infinite recursion (as makeLocalGet will fall back + // to us, if there is no local). + // TODO: we could also look for locals containing subtypes + if (funcContext && !funcContext->typeLocals[type].empty()) { + return makeLocalGet(type); + } return builder.makeRefAs(RefAsNonNull, builder.makeRefNull(heapType)); } @@ -2369,6 +2377,43 @@ Expression* TranslateToFuzzReader::makeCompoundRef(Type type) { } } +Expression* TranslateToFuzzReader::makeTrappingRefUse(HeapType type) { + auto percent = upTo(100); + // Only give a low probability to emit a nullable reference. + if (percent < 5) { + return make(Type(type, Nullable)); + } + // Otherwise, usually emit a non-nullable one. + auto nonNull = Type(type, NonNullable); + if (percent < 70 || !funcContext) { + return make(nonNull); + } + // With significant probability, try to use an existing value. it is better to + // have patterns like this: + // + // (local.set $ref (struct.new $.. + // (struct.get (local.get $ref)) + // + // Rather than constantly operating on new data each time: + // + // (local.set $ref (struct.new $.. + // (struct.get (struct.new $.. + // + // By using local values more, we get more coverage of interesting sequences + // of reads and writes to the same objects. + auto& typeLocals = funcContext->typeLocals[nonNull]; + if (!typeLocals.empty()) { + return builder.makeLocalGet(pick(typeLocals), nonNull); + } + // Add a new local and tee it, so later operations can use it. + auto index = builder.addVar(funcContext->func, nonNull); + // Note we must create the child ref here before adding the local to + // typeLocals (or else we might end up using it prematurely). + auto* tee = builder.makeLocalTee(index, make(nonNull), nonNull); + funcContext->typeLocals[nonNull].push_back(index); + return tee; +} + Expression* TranslateToFuzzReader::buildUnary(const UnaryArgs& args) { return builder.makeUnary(args.a, args.b); } @@ -3270,12 +3315,7 @@ Expression* TranslateToFuzzReader::makeStructGet(Type type) { auto& structFields = typeStructFields[type]; assert(!structFields.empty()); auto [structType, fieldIndex] = pick(structFields); - // TODO: also nullable ones? that would increase the risk of traps - // TODO: Ensure a good chance to use a local.get or tee here, as we want to - // test the same reference having multiple sets/gets on it, and not - // gets/sets of struct.news everywhere. Also in struct.set, array.get, - // array.set. - auto* ref = make(Type(structType, NonNullable)); + auto* ref = makeTrappingRefUse(structType); // TODO: fuzz signed and unsigned return builder.makeStructGet(fieldIndex, ref, type); } @@ -3287,8 +3327,7 @@ Expression* TranslateToFuzzReader::makeStructSet(Type type) { } auto [structType, fieldIndex] = pick(mutableStructFields); auto fieldType = structType.getStruct().fields[fieldIndex].type; - // TODO: also nullable ones? that would increase the risk of traps - auto* ref = make(Type(structType, NonNullable)); + auto* ref = makeTrappingRefUse(structType); auto* value = make(fieldType); return builder.makeStructSet(fieldIndex, ref, value); } @@ -3321,8 +3360,7 @@ Expression* TranslateToFuzzReader::makeArrayGet(Type type) { auto& arrays = typeArrays[type]; assert(!arrays.empty()); auto arrayType = pick(arrays); - // TODO: also nullable ones? that would increase the risk of traps - auto* ref = make(Type(arrayType, NonNullable)); + auto* ref = makeTrappingRefUse(arrayType); auto* index = make(Type::i32); // Only rarely emit a plain get which might trap. See related logic in // ::makePointer(). @@ -3348,8 +3386,7 @@ Expression* TranslateToFuzzReader::makeArraySet(Type type) { auto arrayType = pick(mutableArrays); auto elementType = arrayType.getArray().element.type; auto* index = make(Type::i32); - // TODO: also nullable ones? that would increase the risk of traps - auto* ref = make(Type(arrayType, NonNullable)); + auto* ref = makeTrappingRefUse(arrayType); auto* value = make(elementType); // Only rarely emit a plain get which might trap. See related logic in // ::makePointer(). @@ -3369,9 +3406,7 @@ Expression* TranslateToFuzzReader::makeArraySet(Type type) { Expression* TranslateToFuzzReader::makeI31Get(Type type) { assert(type == Type::i32); assert(wasm.features.hasReferenceTypes() && wasm.features.hasGC()); - // TODO: Maybe this should be nullable? - // https://github.com/WebAssembly/gc/issues/312 - auto* i31 = make(Type(HeapType::i31, NonNullable)); + auto* i31 = makeTrappingRefUse(HeapType::i31); return builder.makeI31Get(i31, bool(oneIn(2))); } 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 92b1fc834..e9a93af72 100644 --- a/test/passes/translate-to-fuzz_all-features_metrics_noprint.txt +++ b/test/passes/translate-to-fuzz_all-features_metrics_noprint.txt @@ -1,45 +1,36 @@ total [exports] : 2 - [funcs] : 10 + [funcs] : 7 [globals] : 16 [imports] : 5 [memories] : 1 [memory-data] : 20 - [table-data] : 5 + [table-data] : 1 [tables] : 1 [tags] : 2 - [total] : 500 - [vars] : 9 - ArrayNew : 5 - AtomicCmpxchg : 1 - AtomicFence : 2 - AtomicNotify : 2 - Binary : 66 - Block : 49 - Break : 5 - Call : 4 - Const : 121 - Drop : 1 - GlobalGet : 25 - GlobalSet : 24 - I31New : 2 - If : 16 - Load : 17 + [total] : 350 + [vars] : 10 + ArrayLen : 1 + ArrayNew : 2 + ArraySet : 1 + Binary : 61 + Block : 27 + Const : 87 + GlobalGet : 16 + GlobalSet : 16 + I31New : 1 + If : 11 + Load : 18 LocalGet : 40 - LocalSet : 28 - Loop : 3 - Nop : 9 - RefAs : 2 - RefEq : 2 - RefFunc : 10 - RefNull : 8 - RefTest : 1 - Return : 2 - SIMDExtract : 2 - Select : 4 - StructNew : 8 - StructSet : 1 - TupleExtract : 1 - TupleMake : 8 - Unary : 16 - Unreachable : 15 + LocalSet : 21 + Loop : 2 + Nop : 1 + RefFunc : 6 + RefIsNull : 1 + RefNull : 4 + Return : 4 + Store : 1 + StructNew : 2 + TupleMake : 7 + Unary : 9 + Unreachable : 11 |