diff --git a/src/tools/execution-results.h b/src/tools/execution-results.h index fb618500109..16a7cba3df4 100644 --- a/src/tools/execution-results.h +++ b/src/tools/execution-results.h @@ -20,9 +20,11 @@ #include #include +#include #include "ir/import-names.h" #include "ir/import-utils.h" +#include "ir/module-utils.h" #include "shell-interface.h" #include "support/utilities.h" #include "wasm-type.h" @@ -437,9 +439,18 @@ struct ExecutionResults { // If set, we should ignore this and not compare it to anything. bool ignore = false; + std::unordered_set publicTypes; + // Execute a module and collect the results. Optionally, provide a second // module to link with it (like fuzz_shell's second module). void collect(Module& wasm, Module* second = nullptr) { + auto publicVec = ModuleUtils::getPublicHeapTypes(wasm); + publicTypes.insert(publicVec.begin(), publicVec.end()); + if (second) { + auto secondPublicVec = ModuleUtils::getPublicHeapTypes(*second); + publicTypes.insert(secondPublicVec.begin(), secondPublicVec.end()); + } + try { // Instantiate the first module. LoggingExternalInterface interface(loggings, wasm); @@ -537,13 +548,22 @@ struct ExecutionResults { } bool areEqual(Literal a, Literal b) { + // Values may be recursive, so naively comparing their structures might + // recurse forever. Instead we need to check coinductive equality, which + // means we assume two values are equivalent until proven otherwise and do + // not recursively compare pairs of values we have already seen. + std::unordered_set> compared; + return areEqualImpl(a, b, compared); + } + + bool areEqualImpl(Literal a, + Literal b, + std::unordered_set>& compared) { // Only compare some references. In general the optimizer may change // identities and structures of functions, types, and GC values in ways that // are not externally observable. We must therefore limit ourselves to - // comparing information that _is_ externally observable. - // - // TODO: We could compare more information when we know it will be - // externally visible, for example when the type of the value is public. + // comparing information that _is_ externally observable. This includes + // fields that are part of public types as well as configured JS prototypes. if (!a.type.isRef() || !b.type.isRef()) { return a == b; } @@ -581,22 +601,85 @@ struct ExecutionResults { // However, we have no way of comparing pointer identities across // executions, so just recursively look for externally observable // differences in the prototypes. - if (!areEqual(a.getJSPrototype(), b.getJSPrototype())) { + if (!areEqualImpl(a.getJSPrototype(), b.getJSPrototype(), compared)) { return false; } + // Check for public struct or array content. + auto publicType = getClosestPublicAncestor(htA); + if (publicType != getClosestPublicAncestor(htB)) { + // Since public types are externally observable, having different public + // types is an observable difference. + return false; + } + if (publicType && publicType->isData()) { + auto* dataA = a.getGCData().get(); + auto* dataB = b.getGCData().get(); + if (dataA == dataB) { + return true; + } + if (!compared.insert({dataA, dataB}).second) { + // We are already comparing these values. Assume they are equivalent + // (until possibly proven otherwise later) and do not recurse further. + return true; + } + compared.insert({dataA, dataB}); + + if (publicType->isStruct()) { + auto& fields = publicType->getStruct().fields; + auto& valuesA = dataA->values; + auto& valuesB = dataB->values; + for (Index i = 0; i < fields.size(); i++) { + if (!areEqualImpl(valuesA[i], valuesB[i], compared)) { + return false; + } + } + } else if (publicType->isArray()) { + auto& valuesA = dataA->values; + auto& valuesB = dataB->values; + if (valuesA.size() != valuesB.size()) { + return false; + } + for (Index i = 0; i < valuesA.size(); i++) { + if (!areEqualImpl(valuesA[i], valuesB[i], compared)) { + return false; + } + } + } + } + // Other differences are not observable, so conservatively consider the // values equal. return true; } + std::optional getClosestPublicAncestor(HeapType ht) { + while (true) { + if (publicTypes.count(ht)) { + return ht; + } + if (auto super = ht.getDeclaredSuperType()) { + ht = *super; + } else { + return std::nullopt; + } + } + } + bool areEqual(Literals a, Literals b) { + std::unordered_set> compared; + return areEqualImpl(a, b, compared); + } + + bool areEqualImpl(Literals a, + Literals b, + std::unordered_set>& compared) { if (a.size() != b.size()) { std::cout << "literal counts not identical! " << a << " != " << b << '\n'; return false; } for (Index i = 0; i < a.size(); i++) { - if (!areEqual(a[i], b[i])) { + if (!areEqualImpl(a[i], b[i], compared)) { std::cout << "values not identical! " << a[i] << " != " << b[i] << '\n'; return false; } diff --git a/test/gtest/CMakeLists.txt b/test/gtest/CMakeLists.txt index 058766f58e4..0d57d6a006d 100644 --- a/test/gtest/CMakeLists.txt +++ b/test/gtest/CMakeLists.txt @@ -12,6 +12,7 @@ set(unittest_SOURCES dataflow.cpp dfa_minimization.cpp disjoint_sets.cpp + execution-results.cpp glbs.cpp interpreter.cpp intervals.cpp diff --git a/test/gtest/execution-results.cpp b/test/gtest/execution-results.cpp new file mode 100644 index 00000000000..5db818e11d9 --- /dev/null +++ b/test/gtest/execution-results.cpp @@ -0,0 +1,154 @@ + +#include "tools/execution-results.h" +#include "test/gtest/type-test.h" +#include "wasm-type.h" +#include "wasm.h" +#include "gtest/gtest.h" + +using namespace wasm; + +struct ExecutionResultsTest : public TypeTest { + Module wasm; +}; + +TEST_F(ExecutionResultsTest, PublicVsPrivate) { + TypeBuilder builder(2); + // Public struct + builder[0] = Struct({Field(Type::i32, Mutable)}); + // Private struct + builder[1] = Struct({Field(Type::f64, Mutable)}); + auto result = builder.build(); + ASSERT_TRUE(result); + auto& types = *result; + HeapType publicHT = types[0]; + HeapType privateHT = types[1]; + + ExecutionResults results; + results.publicTypes.insert(publicHT); + + auto data42 = std::make_shared(Literals{Literal(int32_t(42))}); + auto data43 = std::make_shared(Literals{Literal(int32_t(43))}); + + auto dataF42 = std::make_shared(Literals{Literal(double(42.0))}); + auto dataF43 = std::make_shared(Literals{Literal(double(43.0))}); + + Literal public42(data42, publicHT); + // Same data pointer + Literal public42_2(data42, publicHT); + // Same content, different pointer + Literal public42_3(std::make_shared(Literals{Literal(int32_t(42))}), + publicHT); + Literal public43(data43, publicHT); + + Literal private42(dataF42, privateHT); + Literal private43(dataF43, privateHT); + + // Public types are compared by content + EXPECT_TRUE(results.areEqual(public42, public42_2)); + EXPECT_TRUE(results.areEqual(public42, public42_3)); + EXPECT_FALSE(results.areEqual(public42, public43)); + + // Private types are always equal + EXPECT_TRUE(results.areEqual(private42, private43)); + + // Public and private are different + EXPECT_FALSE(results.areEqual(public42, private42)); +} + +TEST_F(ExecutionResultsTest, RecursivePublic) { + TypeBuilder builder(1); + builder[0] = + Struct({Field(builder.getTempRefType(builder[0], Nullable), Mutable)}); + auto result = builder.build(); + ASSERT_TRUE(result); + auto& types = *result; + HeapType ht = types[0]; + + ExecutionResults results; + results.publicTypes.insert(ht); + + // Create recursive data: A -> A + auto dataA = std::make_shared(Literals{Literal::makeNull(ht)}); + Literal litA(dataA, ht); + dataA->values[0] = litA; + + // Create another identical recursive data: B -> B + auto dataB = std::make_shared(Literals{Literal::makeNull(ht)}); + Literal litB(dataB, ht); + dataB->values[0] = litB; + + EXPECT_TRUE(results.areEqual(litA, litB)); + + // Create a different recursive data: C -> D -> C + auto dataC = std::make_shared(Literals{Literal::makeNull(ht)}); + auto dataD = std::make_shared(Literals{Literal::makeNull(ht)}); + Literal litC(dataC, ht); + Literal litD(dataD, ht); + dataC->values[0] = litD; + dataD->values[0] = litC; + + // They are actually all equivalent as infinite structures of just this one + // type. + EXPECT_TRUE(results.areEqual(litA, litC)); +} + +TEST_F(ExecutionResultsTest, SubtypeOfPublic) { + TypeBuilder builder(2); + // Public supertype + builder[0] = Struct({Field(Type::i32, Mutable)}); + builder[0].setOpen(); + // Subtype with extra field + builder[1] = Struct({Field(Type::i32, Mutable), Field(Type::f64, Mutable)}); + builder[1].subTypeOf(builder[0]); + auto result = builder.build(); + ASSERT_TRUE(result); + auto& types = *result; + HeapType superHT = types[0]; + HeapType subHT = types[1]; + + ExecutionResults results; + results.publicTypes.insert(superHT); + + // sub42_0 and sub42_1 have the same public field but different private + // fields. + auto data42_0 = std::make_shared( + Literals{Literal(int32_t(42)), Literal(double(0.0))}); + auto data42_1 = std::make_shared( + Literals{Literal(int32_t(42)), Literal(double(1.0))}); + auto data43_0 = std::make_shared( + Literals{Literal(int32_t(43)), Literal(double(0.0))}); + + Literal sub42_0(data42_0, subHT); + Literal sub42_1(data42_1, subHT); + Literal sub43_0(data43_0, subHT); + + auto data42_super = std::make_shared(Literals{Literal(int32_t(42))}); + Literal super42(data42_super, superHT); + + // Subtype of public type should be compared by content (of the public + // ancestor) Extra fields in the subtype should be ignored. + EXPECT_TRUE(results.areEqual(sub42_0, sub42_1)); + EXPECT_TRUE(results.areEqual(sub42_0, super42)); + EXPECT_FALSE(results.areEqual(sub42_0, sub43_0)); +} + +TEST_F(ExecutionResultsTest, PrivateTypes) { + TypeBuilder builder(1); + builder[0] = Struct({Field(Type::i32, Mutable)}); + auto result = builder.build(); + ASSERT_TRUE(result); + auto& types = *result; + HeapType ht = types[0]; + + // publicTypes is empty + ExecutionResults results; + + auto data42 = std::make_shared(Literals{Literal(int32_t(42))}); + auto data43 = std::make_shared(Literals{Literal(int32_t(43))}); + + Literal lit42(data42, ht); + Literal lit43(data43, ht); + + // This should not crash and should return true. + EXPECT_TRUE(results.areEqual(lit42, lit43)); +} diff --git a/test/lit/exec/fuzzing-api.wast b/test/lit/exec/fuzzing-api.wast index eb3088b5f0d..ac24ef3730e 100644 --- a/test/lit/exec/fuzzing-api.wast +++ b/test/lit/exec/fuzzing-api.wast @@ -29,6 +29,11 @@ (type $i32 (struct i32)) + (rec + (type $rec-A (struct (field (mut (ref null $rec-B))))) + (type $rec-B (struct (field (mut (ref null $rec-A))))) + ) + (table $table 10 20 funcref) ;; Note that the exported table appears first here, but in the binary and in @@ -482,7 +487,6 @@ ;; CHECK: [fuzz-exec] export return-externref-exception ;; CHECK-NEXT: [fuzz-exec] note result: return-externref-exception => jserror - ;; CHECK-NEXT: warning: no passes specified, not doing any work (func $return-externref-exception (export "return-externref-exception") (result externref) ;; Call JS table.set in a way that throws (on out of bounds). The JS exception ;; is caught and returned from the function, so we can see what it looks like @@ -497,6 +501,21 @@ (unreachable) ) ) + + ;; CHECK: [fuzz-exec] export recursive-public-data + ;; CHECK-NEXT: [fuzz-exec] note result: recursive-public-data => object(null) + ;; CHECK-NEXT: warning: no passes specified, not doing any work + (func $recursive-public-data (export "recursive-public-data") (result (ref null $rec-A)) + ;; We should not infinitely recurse when comparing whether this recursive + ;; structure is equivalent in the two executions. + (local $a (ref null $rec-A)) + (local $b (ref null $rec-B)) + (local.set $a (struct.new_default $rec-A)) + (local.set $b (struct.new $rec-B (local.get $a))) + (struct.set $rec-A 0 (local.get $a) (local.get $b)) + (local.get $a) + ) + ) ;; CHECK: [fuzz-exec] export logging ;; CHECK-NEXT: [LoggingExternalInterface logging 42] @@ -613,12 +632,16 @@ ;; CHECK: [fuzz-exec] export return-externref-exception ;; CHECK-NEXT: [fuzz-exec] note result: return-externref-exception => jserror + +;; CHECK: [fuzz-exec] export recursive-public-data +;; CHECK-NEXT: [fuzz-exec] note result: recursive-public-data => object(null) ;; CHECK-NEXT: [fuzz-exec] comparing catch-js-tag ;; CHECK-NEXT: [fuzz-exec] comparing do-sleep ;; CHECK-NEXT: [fuzz-exec] comparing export.calling ;; CHECK-NEXT: [fuzz-exec] comparing export.calling.catching ;; CHECK-NEXT: [fuzz-exec] comparing export.calling.rethrow ;; CHECK-NEXT: [fuzz-exec] comparing logging +;; CHECK-NEXT: [fuzz-exec] comparing recursive-public-data ;; CHECK-NEXT: [fuzz-exec] comparing ref.calling ;; CHECK-NEXT: [fuzz-exec] comparing ref.calling.catching ;; CHECK-NEXT: [fuzz-exec] comparing ref.calling.illegal