From ac7f74fa6d778e22c5018425d73a824f129a4ed8 Mon Sep 17 00:00:00 2001 From: Thomas Lively Date: Wed, 18 Mar 2026 21:58:36 -0700 Subject: [PATCH] Compare public fields in execution-results.h When comparing two logged struct or array values, look up their closest public supertypes. If these supertypes are different, then the values are observably different. Otherwise, look at the fields the shared public supertype makes visible and recursively check for differences. Since GC data may in general be recursive, naively recursing may cause the comparison to exhaust the stack or fail to terminate. Instead, perform a coinductive equality check where pairs of values are assumed to be equivalent until proven otherwise. This avoids recursing into the comparison of a pair of values we have already started comparing. Since two execution traces typically differ only when the fuzzer finds a misoptimization, add a new gtest file to test the new comparison logic. --- src/tools/execution-results.h | 95 +++++++++++++++++-- test/gtest/CMakeLists.txt | 1 + test/gtest/execution-results.cpp | 154 +++++++++++++++++++++++++++++++ test/lit/exec/fuzzing-api.wast | 25 ++++- 4 files changed, 268 insertions(+), 7 deletions(-) create mode 100644 test/gtest/execution-results.cpp 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