diff --git a/src/ir/intrinsics.h b/src/ir/intrinsics.h index 9be2caebc7d..d2e99460685 100644 --- a/src/ir/intrinsics.h +++ b/src/ir/intrinsics.h @@ -130,6 +130,16 @@ class Intrinsics { return getAnnotations(nullptr, func); } + void setAnnotations(Function* func, + Expression* curr, + const CodeAnnotation& value) { + func->codeAnnotations[curr] = value; + } + + void setAnnotations(Function* func, const CodeAnnotation& value) { + setAnnotations(func, nullptr, value); + } + // Given a call in a function, return all the annotations for it. The call may // be annotated itself (which takes precedence), or the function it calls be // annotated. diff --git a/src/tools/fuzzing.h b/src/tools/fuzzing.h index f12735636a2..8befd5a9def 100644 --- a/src/tools/fuzzing.h +++ b/src/tools/fuzzing.h @@ -143,6 +143,7 @@ class TranslateToFuzzReader { bool closedWorld; Builder builder; Random random; + Intrinsics intrinsics; // Whether to emit memory operations like loads and stores. bool allowMemory = true; @@ -218,6 +219,9 @@ class TranslateToFuzzReader { // All tags that are valid as exception tags (which cannot have results). std::vector exceptionTags; + // All functions marked jsCalled. + std::vector jsCalled; + Index numAddedFunctions = 0; // The name of an empty tag. @@ -393,7 +397,9 @@ class TranslateToFuzzReader { } void recombine(Function* func); void mutate(Function* func); - // Fix up the IR after recombination and mutation. + // Fix up the IR for closed world. + void fixClosedWorld(Function* func); + // Fix up the IR after recombination and mutation (which may break the IR). void fixAfterChanges(Function* func); void modifyInitialFunctions(); @@ -568,6 +574,12 @@ class TranslateToFuzzReader { Name getTargetName(Expression* target); Type getTargetType(Expression* target); + // Checks if a function is valid to take a ref.func of. + bool isValidRefFuncTarget(Name func); + + // Checks if a function is a callRef* import (call-ref or call-ref-catch). + bool isCallRefImport(Name func); + // statistical distributions // 0 to the limit, logarithmic scale diff --git a/src/tools/fuzzing/fuzzing.cpp b/src/tools/fuzzing/fuzzing.cpp index 0e7bab677f4..f0dac5827fb 100644 --- a/src/tools/fuzzing/fuzzing.cpp +++ b/src/tools/fuzzing/fuzzing.cpp @@ -64,7 +64,7 @@ TranslateToFuzzReader::TranslateToFuzzReader(Module& wasm, std::vector&& input, bool closedWorld) : wasm(wasm), closedWorld(closedWorld), builder(wasm), - random(std::move(input), wasm.features), + random(std::move(input), wasm.features), intrinsics(wasm), loggableTypes(getLoggableTypes(wasm.features)), atomicMemoryOrders(getMemoryOrders(wasm.features)), @@ -1000,25 +1000,6 @@ void TranslateToFuzzReader::addImportCallingSupport() { return; } - if (wasm.features.hasReferenceTypes() && closedWorld) { - // In closed world mode we must *remove* the call-ref* imports, if they - // exist in the initial content. These are not valid to call in closed-world - // mode as they call function references. (Another solution here would be to - // make closed-world issue validation errors on these imports, but that - // would require changes to the general-purpose validator.) - for (auto& func : wasm.functions) { - if (func->imported() && func->module == "fuzzing-support" && - func->base.startsWith("call-ref")) { - // Make it non-imported, and with a simple body. - func->module = func->base = Name(); - func->type = func->type.with(Exact); - auto results = func->getResults(); - func->body = - results.isConcrete() ? makeConst(results) : makeNop(Type::none); - } - } - } - // Only add these some of the time, as they inhibit some fuzzing (things like // wasm-ctor-eval and wasm-merge are sensitive to the wasm being able to call // its own exports, and to care about the indexes of the exports). @@ -1058,9 +1039,7 @@ void TranslateToFuzzReader::addImportCallingSupport() { wasm.addFunction(std::move(func)); } - // If the wasm will be used for closed-world testing, we cannot use the - // call-ref variants, as mentioned before. - if (wasm.features.hasReferenceTypes() && !closedWorld) { + if (wasm.features.hasReferenceTypes()) { if (choice & 4) { // Given an funcref, call it from JS. callRefImportName = Names::getValidFunctionName(wasm, "call-ref"); @@ -1638,6 +1617,16 @@ void TranslateToFuzzReader::processFunctions() { } } } + + // Also fix up closed world, if we need to. We must do this at the end, so + // nothing can break the closed world assumptions after. + if (closedWorld) { + for (auto& func : wasm.functions) { + if (!func->imported()) { + fixClosedWorld(func.get()); + } + } + } } // TODO: return std::unique_ptr @@ -1695,8 +1684,8 @@ Function* TranslateToFuzzReader::addFunction() { func->body = make(bodyType); } - // Add hang limit checks after all other operations on the function body. wasm.addFunction(std::move(allocation)); + // Export some functions, but not all (to allow inlining etc.). Try to export // at least one, though, to keep each testcase interesting. Avoid non- // nullable params, as those cannot be constructed by the fuzzer on the @@ -1713,7 +1702,10 @@ Function* TranslateToFuzzReader::addFunction() { wasm.addExport( Builder::makeExport(func->name, func->name, ExternalKind::Function)); } - // add some to an elem segment TODO we could do this for imported funcs too + + // Add some to an elem segment TODO we could do this for imported funcs too, + // but in closed world must be careful of callRef*, see the jsCalled logic + // and isValidRefFuncTarget. while (oneIn(3) && !random.finished()) { auto type = func->type; std::vector compatibleSegments; @@ -1725,6 +1717,23 @@ Function* TranslateToFuzzReader::addFunction() { auto& randomElem = compatibleSegments[upTo(compatibleSegments.size())]; randomElem->data.push_back(builder.makeRefFunc(func->name)); } + + // Mark some functions as jsCalled. This allows them to be called by + // reference even in closed world, using the callRef* imports. + // TODO: We could do this, and exporting above, to initial content too. + if (oneIn(4)) { + auto annotations = intrinsics.getAnnotations(func); + annotations.jsCalled = true; + intrinsics.setAnnotations(func, annotations); + + // We cannot actually use this as jsCalled if it does not have a type + // compatible with the callRef* imports. They send a funcref, so we must + // only send non-shared functions. + if (!func->type.getHeapType().isShared()) { + jsCalled.push_back(func->name); + } + } + numAddedFunctions++; return func; } @@ -2125,6 +2134,60 @@ void TranslateToFuzzReader::mutate(Function* func) { modder.walkFunctionInModule(func, &wasm); } +void TranslateToFuzzReader::fixClosedWorld(Function* func) { + assert(closedWorld); + + struct Fixer + : public ExpressionStackWalker> { + TranslateToFuzzReader& parent; + + Fixer(TranslateToFuzzReader& parent) : parent(parent) {} + + void visitCall(Call* curr) { + // In closed world, the callRef* imports can cause misoptimization later: + // they send a funcref to JS to call, and in closed world we assume such + // calls do not happen unless the function is annotated as jsCalled. We + // must therefore ensure that calls to these imports only send a jsCalled + // method and nothing else. + if (!parent.isCallRefImport(curr->target)) { + return; + } + + // These imports take a funcref as the first param. + assert(!curr->operands.empty()); + if (curr->operands[0]->type == Type::unreachable) { + // This call is not executed anyhow, and we shouldn't replace it, as + // that could change the type. + return; + } + + if (parent.jsCalled.empty()) { + // There is nothing valid to call at all. Keep the children (we may + // need them to validate, e.g. if there is a `pop`), but remove the + // call. + std::vector list; + for (auto* child : curr->operands) { + list.push_back(parent.builder.makeDrop(child)); + } + list.push_back(parent.makeTrivial(curr->type)); + replaceCurrent(parent.builder.makeBlock(list)); + return; + } + + // Set something valid. As above, we must keep the old child for + // validation reasons. + auto old = parent.builder.makeDrop(curr->operands[0]); + auto target = parent.pick(parent.jsCalled); + assert(parent.isValidRefFuncTarget(target)); + auto new_ = parent.builder.makeRefFunc(target); + curr->operands[0] = parent.builder.makeSequence(old, new_); + } + } fixer(*this); + + FunctionCreationContext context(*this, func); + fixer.walk(func->body); +} + void TranslateToFuzzReader::fixAfterChanges(Function* func) { struct Fixer : public ExpressionStackWalker> { @@ -3002,6 +3065,10 @@ Expression* TranslateToFuzzReader::makeCallRef(Type type) { target = wasm.functions[upTo(wasm.functions.size())].get(); isReturn = type == Type::unreachable && wasm.features.hasTailCall() && funcContext->func->getResults() == target->getResults(); + if (!isValidRefFuncTarget(target->name)) { + i++; + continue; + } if (target->getResults() == type || isReturn) { break; } @@ -3638,7 +3705,9 @@ Expression* TranslateToFuzzReader::makeRefFuncConst(Type type) { funcContext->func->type.getHeapType().getShared() == share && !oneIn(4)) { auto* target = funcContext->func; - return builder.makeRefFunc(target->name, target->type); + if (isValidRefFuncTarget(target->name)) { + return builder.makeRefFunc(target->name, target->type); + } } } // Look for a proper function starting from a random location, and loop from @@ -3648,7 +3717,8 @@ Expression* TranslateToFuzzReader::makeRefFuncConst(Type type) { Index i = start; do { auto& func = wasm.functions[i]; - if (Type::isSubType(func->type, type)) { + if (Type::isSubType(func->type, type) && + isValidRefFuncTarget(func->name)) { return builder.makeRefFunc(func->name); } i = (i + 1) % wasm.functions.size(); @@ -3688,6 +3758,7 @@ Expression* TranslateToFuzzReader::makeRefFuncConst(Type type) { Type(heapType, NonNullable, Exact), {}, body)); + assert(isValidRefFuncTarget(func->name)); return builder.makeRefFunc(func->name); } @@ -6028,4 +6099,31 @@ Type TranslateToFuzzReader::getTargetType(Expression* target) { WASM_UNREACHABLE("unexpected expr type"); } +bool TranslateToFuzzReader::isValidRefFuncTarget(Name func) { + // In closed world, we must not take callRef* by reference: if we do, then we + // might end up calling them indirectly, and with any possible function + // reference, but in that mode we must only pass in jsCalled functions. We + // handle direct calls in fixClosedWorld, but cannot handle indirect ones + // easily, so just disallow taking references of those functions. + if (!closedWorld) { + return true; + } + return !isCallRefImport(func); +} + +bool TranslateToFuzzReader::isCallRefImport(Name target) { + // Check the import module and base. Note we do not just compare to + // callRefImportName / callRefCatchImportName because those are the names of + // the methods we added, but the initial content may import those methods + // under other internal names. + auto* func = wasm.getFunctionOrNull(target); + if (!func) { + // We are called while a function is still being constructed. Such a new + // defined function can never be an import. + return false; + } + return func->imported() && func->module == "fuzzing-support" && + func->base.startsWith("call-ref"); +} + } // namespace wasm diff --git a/test/passes/fuzz_metrics_noprint.bin.txt b/test/passes/fuzz_metrics_noprint.bin.txt index 126178d95e2..aa0bf5135ce 100644 --- a/test/passes/fuzz_metrics_noprint.bin.txt +++ b/test/passes/fuzz_metrics_noprint.bin.txt @@ -1,35 +1,35 @@ Metrics total - [exports] : 139 - [funcs] : 217 + [exports] : 85 + [funcs] : 123 [globals] : 7 [imports] : 6 [memories] : 1 [memory-data] : 23 - [table-data] : 71 + [table-data] : 44 [tables] : 1 [tags] : 0 - [total] : 15937 - [vars] : 628 - Binary : 1172 - Block : 2747 - Break : 485 - Call : 628 - CallIndirect : 146 - Const : 2648 - Drop : 197 - GlobalGet : 1418 - GlobalSet : 1052 - If : 866 - Load : 259 - LocalGet : 1011 - LocalSet : 708 - Loop : 309 - Nop : 227 - RefFunc : 71 - Return : 159 - Select : 83 - Store : 105 - Switch : 1 - Unary : 1112 - Unreachable : 533 + [total] : 13644 + [vars] : 404 + Binary : 996 + Block : 2154 + Break : 426 + Call : 468 + CallIndirect : 135 + Const : 2312 + Drop : 113 + GlobalGet : 1167 + GlobalSet : 818 + If : 765 + Load : 231 + LocalGet : 1187 + LocalSet : 698 + Loop : 286 + Nop : 169 + RefFunc : 44 + Return : 134 + Select : 119 + Store : 92 + Switch : 4 + Unary : 910 + Unreachable : 416 diff --git a/test/passes/fuzz_metrics_passes_noprint.bin.txt b/test/passes/fuzz_metrics_passes_noprint.bin.txt index dc93446ceb3..4a3f7455c85 100644 --- a/test/passes/fuzz_metrics_passes_noprint.bin.txt +++ b/test/passes/fuzz_metrics_passes_noprint.bin.txt @@ -1,35 +1,35 @@ Metrics total - [exports] : 33 - [funcs] : 57 + [exports] : 54 + [funcs] : 84 [globals] : 4 [imports] : 4 [memories] : 1 [memory-data] : 20 - [table-data] : 14 + [table-data] : 20 [tables] : 1 [tags] : 0 - [total] : 8611 - [vars] : 182 - Binary : 562 - Block : 1467 - Break : 279 - Call : 266 - CallIndirect : 51 - Const : 1345 - Drop : 118 - GlobalGet : 715 - GlobalSet : 508 - If : 493 - Load : 138 - LocalGet : 635 - LocalSet : 564 - Loop : 197 - Nop : 115 - RefFunc : 14 - Return : 98 - Select : 87 - Store : 67 - Switch : 2 - Unary : 634 - Unreachable : 256 + [total] : 9213 + [vars] : 294 + Binary : 632 + Block : 1619 + Break : 312 + Call : 298 + CallIndirect : 47 + Const : 1427 + Drop : 90 + GlobalGet : 825 + GlobalSet : 606 + If : 507 + Load : 134 + LocalGet : 609 + LocalSet : 528 + Loop : 219 + Nop : 147 + RefFunc : 20 + Return : 91 + Select : 65 + Store : 62 + Switch : 1 + Unary : 667 + Unreachable : 307 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 dc3c5e52c93..8625c6781a1 100644 --- a/test/passes/translate-to-fuzz_all-features_metrics_noprint.txt +++ b/test/passes/translate-to-fuzz_all-features_metrics_noprint.txt @@ -1,54 +1,56 @@ Metrics total - [exports] : 12 + [exports] : 14 [funcs] : 8 [globals] : 26 [imports] : 14 [memories] : 1 [memory-data] : 16 - [table-data] : 2 + [table-data] : 1 [tables] : 2 [tags] : 1 - [total] : 581 - [vars] : 38 - ArrayNewFixed : 10 - AtomicCmpxchg : 1 - Binary : 28 - Block : 86 - BrOn : 2 - Break : 10 + [total] : 614 + [vars] : 30 + ArrayNewFixed : 8 + AtomicCmpxchg : 2 + AtomicFence : 1 + AtomicNotify : 1 + Binary : 32 + Block : 91 + BrOn : 1 + Break : 18 Call : 21 - CallIndirect : 1 - CallRef : 1 - Const : 105 + CallRef : 2 + Const : 120 ContNew : 1 DataDrop : 1 - Drop : 3 - GlobalGet : 44 + Drop : 8 + GlobalGet : 47 GlobalSet : 34 - If : 25 - Load : 7 + I31Get : 1 + If : 34 + Load : 5 LocalGet : 30 - LocalSet : 18 + LocalSet : 14 Loop : 9 - Nop : 9 - Pop : 2 - RefAs : 5 - RefCast : 5 - RefEq : 3 - RefFunc : 6 - RefI31 : 4 - RefNull : 10 - Return : 2 + MemoryInit : 1 + Nop : 11 + RefAs : 3 + RefFunc : 11 + RefI31 : 1 + RefNull : 9 + RefTest : 1 + Return : 5 SIMDExtract : 2 - Select : 5 - StringConst : 6 - StringEncode : 1 - StringWTF16Get : 1 - StructNew : 6 - Try : 3 - TryTable : 9 - TupleExtract : 8 - TupleMake : 12 - Unary : 28 + Select : 4 + Store : 3 + StringConst : 7 + StringEncode : 3 + StringEq : 1 + StringMeasure : 1 + StructNew : 3 + Try : 6 + TryTable : 3 + TupleMake : 7 + Unary : 34 Unreachable : 17