diff --git a/src/support/topological_sort.h b/src/support/topological_sort.h index 91353dd3727..3594617ebd5 100644 --- a/src/support/topological_sort.h +++ b/src/support/topological_sort.h @@ -27,7 +27,7 @@ namespace wasm { // CRTP utility that provides an iterator through arbitrary directed acyclic // graphs of data that will visit the data in a topologically sorted order // (https://en.wikipedia.org/wiki/Topological_sorting). In other words, the -// iterator will produce each item only after all that items predecessors have +// iterator will produce each item only after all that item's predecessors have // been produced. // // Subclasses should call `push` on all the root items in their constructors and diff --git a/src/tools/fuzzing.h b/src/tools/fuzzing.h index 78652723627..75e3a2a9a4e 100644 --- a/src/tools/fuzzing.h +++ b/src/tools/fuzzing.h @@ -106,6 +106,8 @@ class TranslateToFuzzReader { std::unordered_map> globalsByType; std::unordered_map> mutableGlobalsByType; + std::unordered_map> immutableGlobalsByType; + std::unordered_map> importedImmutableGlobalsByType; std::vector loggableTypes; diff --git a/src/tools/fuzzing/fuzzing.cpp b/src/tools/fuzzing/fuzzing.cpp index 1b1d2cb0172..f4f059deb43 100644 --- a/src/tools/fuzzing/fuzzing.cpp +++ b/src/tools/fuzzing/fuzzing.cpp @@ -379,26 +379,46 @@ void TranslateToFuzzReader::setupGlobals() { } } + auto useGlobalLater = [&](Global* global) { + auto type = global->type; + auto name = global->name; + globalsByType[type].push_back(name); + if (global->mutable_) { + mutableGlobalsByType[type].push_back(name); + } else { + immutableGlobalsByType[type].push_back(name); + if (global->imported()) { + importedImmutableGlobalsByType[type].push_back(name); + } + } + }; + // Randomly assign some globals from initial content to be ignored for the // fuzzer to use. Such globals will only be used from initial content. This is // important to preserve some real-world patterns, like the "once" pattern in // which a global is used in one function only. (If we randomly emitted gets // and sets of such globals, we'd with very high probability end up breaking // that pattern, and not fuzzing it at all.) - // - // Pick a percentage of initial globals to ignore later down when we decide - // which to allow uses from. - auto numInitialGlobals = wasm.globals.size(); - unsigned percentIgnoredInitialGlobals = 0; - if (numInitialGlobals) { - // Only generate this random number if it will be used. - percentIgnoredInitialGlobals = upTo(100); + if (!wasm.globals.empty()) { + unsigned percentUsedInitialGlobals = upTo(100); + for (auto& global : wasm.globals) { + if (upTo(100) < percentUsedInitialGlobals) { + useGlobalLater(global.get()); + } + } } // Create new random globals. for (size_t index = upTo(MAX_GLOBALS); index > 0; --index) { auto type = getConcreteType(); - auto* init = makeConst(type); + // Prefer immutable ones as they can be used in global.gets in other + // globals, for more interesting patterns. + auto mutability = oneIn(3) ? Builder::Mutable : Builder::Immutable; + + // We can only make something trivial (like a constant) in a global + // initializer. + auto* init = makeTrivial(type); + if (!FindAll(init).list.empty()) { // When creating this initial value we ended up emitting a RefAs, which // means we had to stop in the middle of an overly-nested struct or array, @@ -407,27 +427,16 @@ void TranslateToFuzzReader::setupGlobals() { // validate in a global. Switch to something safe instead. type = getMVPType(); init = makeConst(type); + } else if (type.isTuple() && !init->is()) { + // For now we disallow anything but tuple.make at the top level of tuple + // globals (see details in wasm-binary.cpp). In the future we may allow + // global.get or other things here. + init = makeConst(type); + assert(init->is()); } - auto mutability = oneIn(2) ? Builder::Mutable : Builder::Immutable; auto global = builder.makeGlobal( Names::getValidGlobalName(wasm, "global$"), type, init, mutability); - wasm.addGlobal(std::move(global)); - } - - // Set up data structures for picking globals later for get/set operations. - for (Index i = 0; i < wasm.globals.size(); i++) { - auto& global = wasm.globals[i]; - - // Apply the chance for initial globals to be ignored, see above. - if (i < numInitialGlobals && upTo(100) < percentIgnoredInitialGlobals) { - continue; - } - - // This is a global we can use later, note it. - globalsByType[global->type].push_back(global->name); - if (global->mutable_) { - mutableGlobalsByType[global->type].push_back(global->name); - } + useGlobalLater(wasm.addGlobal(std::move(global))); } } @@ -1477,9 +1486,12 @@ Expression* TranslateToFuzzReader::makeTrivial(Type type) { // using it before it was set, which would trap. if (funcContext && oneIn(type.isNonNullable() ? 10 : 2)) { return makeLocalGet(type); - } else { - return makeConst(type); } + + // Either make a const, or a global.get (which may fail to find a suitable + // global, especially in a non-function context, and if so it will make a + // constant instead). + return oneIn(2) ? makeConst(type) : makeGlobalGet(type); } else if (type == Type::none) { return makeNop(type); } @@ -1900,9 +1912,17 @@ Expression* TranslateToFuzzReader::makeLocalSet(Type type) { } Expression* TranslateToFuzzReader::makeGlobalGet(Type type) { - auto it = globalsByType.find(type); - if (it == globalsByType.end() || it->second.empty()) { - return makeTrivial(type); + // In a non-function context, like in another global, we can only get from an + // immutable global. Whether GC is enabled also matters, as it allows getting + // from a non-import. + auto& relevantGlobals = + funcContext ? globalsByType + : (wasm.features.hasGC() ? immutableGlobalsByType + : importedImmutableGlobalsByType); + auto it = relevantGlobals.find(type); + // If we have no such relevant globals give up and emit a constant instead. + if (it == relevantGlobals.end() || it->second.empty()) { + return makeConst(type); } auto name = pick(it->second); diff --git a/src/tools/fuzzing/parameters.h b/src/tools/fuzzing/parameters.h index 51531681b39..eede193a7f3 100644 --- a/src/tools/fuzzing/parameters.h +++ b/src/tools/fuzzing/parameters.h @@ -30,7 +30,7 @@ constexpr int MAX_PARAMS = 10; constexpr int MAX_VARS = 20; // The maximum number of globals in a module. -constexpr int MAX_GLOBALS = 20; +constexpr int MAX_GLOBALS = 30; // The maximum number of tuple elements. constexpr int MAX_TUPLE_SIZE = 6; diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp index befafde599f..4888310b831 100644 --- a/src/wasm/wasm-binary.cpp +++ b/src/wasm/wasm-binary.cpp @@ -572,8 +572,23 @@ void WasmBinaryWriter::writeGlobals() { o << U32LEB(global->mutable_); if (global->type.size() == 1) { writeExpression(global->init); + } else if (auto* make = global->init->dynCast()) { + // Emit the proper lane for this global. + writeExpression(make->operands[i]); } else { - writeExpression(global->init->cast()->operands[i]); + // For now tuple globals must contain tuple.make. We could perhaps + // support more operations, like global.get, but the code would need to + // look something like this: + // + // auto parentIndex = getGlobalIndex(get->name); + // o << int8_t(BinaryConsts::GlobalGet) << U32LEB(parentIndex + i); + // + // That is, we must emit the instruction here, and not defer to + // writeExpression, as writeExpression writes an entire expression at a + // time (and not just one of the lanes). As emitting an instruction here + // is less clean, and there is no important use case for global.get of + // one tuple global to another, we disallow this. + WASM_UNREACHABLE("unsupported tuple global operation"); } o << int8_t(BinaryConsts::End); ++i; diff --git a/test/passes/fuzz_metrics_noprint.bin.txt b/test/passes/fuzz_metrics_noprint.bin.txt index f6af82a5ad0..c5dab17e75b 100644 --- a/test/passes/fuzz_metrics_noprint.bin.txt +++ b/test/passes/fuzz_metrics_noprint.bin.txt @@ -1,34 +1,34 @@ total - [exports] : 18 - [funcs] : 21 + [exports] : 23 + [funcs] : 34 [globals] : 9 [imports] : 4 [memories] : 1 [memory-data] : 2 - [table-data] : 7 + [table-data] : 6 [tables] : 1 [tags] : 0 - [total] : 11685 - [vars] : 64 - Binary : 848 - Block : 1846 - Break : 456 - Call : 275 - CallIndirect : 117 - Const : 1952 - Drop : 86 - GlobalGet : 844 - GlobalSet : 679 - If : 625 - Load : 236 - LocalGet : 1050 - LocalSet : 764 - Loop : 301 - Nop : 143 - RefFunc : 7 - Return : 103 - Select : 77 - Store : 111 - Switch : 3 - Unary : 835 - Unreachable : 327 + [total] : 4303 + [vars] : 100 + Binary : 355 + Block : 684 + Break : 149 + Call : 219 + CallIndirect : 23 + Const : 643 + Drop : 50 + GlobalGet : 367 + GlobalSet : 258 + If : 206 + Load : 78 + LocalGet : 339 + LocalSet : 236 + Loop : 93 + Nop : 41 + RefFunc : 6 + Return : 45 + Select : 41 + Store : 36 + Switch : 1 + Unary : 304 + Unreachable : 129 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 4ead6265533..57650cb2475 100644 --- a/test/passes/translate-to-fuzz_all-features_metrics_noprint.txt +++ b/test/passes/translate-to-fuzz_all-features_metrics_noprint.txt @@ -1,47 +1,59 @@ total - [exports] : 5 - [funcs] : 7 - [globals] : 14 + [exports] : 3 + [funcs] : 4 + [globals] : 24 [imports] : 5 [memories] : 1 [memory-data] : 20 - [table-data] : 2 + [table-data] : 3 [tables] : 1 [tags] : 1 - [total] : 467 - [vars] : 40 - ArrayGet : 1 - ArrayLen : 1 - ArrayNew : 6 + [total] : 750 + [vars] : 30 + ArrayCopy : 1 + ArrayGet : 2 + ArrayLen : 5 + ArrayNew : 24 ArrayNewFixed : 1 - Binary : 67 - Block : 44 - Break : 5 - Call : 21 - Const : 106 - Drop : 7 - GlobalGet : 20 - GlobalSet : 18 - If : 14 - Load : 17 - LocalGet : 45 - LocalSet : 28 - Loop : 3 - MemoryFill : 1 - Nop : 5 - RefAs : 1 - RefCast : 1 + ArraySet : 1 + AtomicCmpxchg : 1 + AtomicFence : 1 + AtomicRMW : 1 + Binary : 84 + Block : 58 + Break : 12 + Call : 13 + Const : 175 + Drop : 2 + GlobalGet : 45 + GlobalSet : 20 + I31Get : 2 + If : 21 + Load : 20 + LocalGet : 70 + LocalSet : 46 + Loop : 7 + MemoryCopy : 1 + Nop : 11 + Pop : 3 + RefAs : 7 RefEq : 1 - RefFunc : 3 - RefI31 : 4 - RefNull : 3 - Return : 3 - Select : 2 + RefFunc : 5 + RefI31 : 7 + RefIsNull : 3 + RefNull : 19 + RefTest : 3 + Return : 2 + SIMDTernary : 1 + Select : 3 Store : 1 - StringConst : 3 + StringConst : 8 StringEncode : 1 - StringEq : 1 - StructNew : 5 - TupleMake : 5 - Unary : 13 + StringMeasure : 1 + StringWTF16Get : 1 + StructGet : 1 + StructNew : 21 + Try : 3 + TupleMake : 6 + Unary : 19 Unreachable : 10