From 1dbefce43d2fe9f6d9b821baa41275719ae6aee4 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 6 Jan 2026 09:34:51 -0800 Subject: [PATCH 01/13] move --- src/{ => support}/mixed_arena.h | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename src/{ => support}/mixed_arena.h (100%) diff --git a/src/mixed_arena.h b/src/support/mixed_arena.h similarity index 100% rename from src/mixed_arena.h rename to src/support/mixed_arena.h From 4cb0b00a6169a6d875b92551bf41830e84d9faef Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 6 Jan 2026 09:37:11 -0800 Subject: [PATCH 02/13] update --- src/emscripten-optimizer/simple_ast.h | 2 +- src/parsing.h | 1 - src/pass.h | 2 +- src/passes/TrapMode.cpp | 2 +- src/wasm.h | 2 +- src/wasm2js.h | 1 - 6 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/emscripten-optimizer/simple_ast.h b/src/emscripten-optimizer/simple_ast.h index 5920ceee5d5..a2c541ee6e5 100644 --- a/src/emscripten-optimizer/simple_ast.h +++ b/src/emscripten-optimizer/simple_ast.h @@ -33,9 +33,9 @@ #include #include -#include "mixed_arena.h" #include "parser.h" #include "snprintf.h" +#include "support/mixed_arena.h" #include "support/safe_integer.h" #define errv(str, ...) fprintf(stderr, str "\n", __VA_ARGS__); diff --git a/src/parsing.h b/src/parsing.h index 80d4db9fb27..b83a7aca5f3 100644 --- a/src/parsing.h +++ b/src/parsing.h @@ -22,7 +22,6 @@ #include #include -#include "mixed_arena.h" #include "shared-constants.h" #include "support/colors.h" #include "support/utilities.h" diff --git a/src/pass.h b/src/pass.h index e19af92743e..1f78cf1beb0 100644 --- a/src/pass.h +++ b/src/pass.h @@ -19,7 +19,7 @@ #include -#include "mixed_arena.h" +#include "support/mixed_arena.h" #include "support/utilities.h" #include "wasm-traversal.h" #include "wasm.h" diff --git a/src/passes/TrapMode.cpp b/src/passes/TrapMode.cpp index 8a2b8f8c952..8438b7407df 100644 --- a/src/passes/TrapMode.cpp +++ b/src/passes/TrapMode.cpp @@ -22,8 +22,8 @@ #include "asmjs/shared-constants.h" #include "ir/trapping.h" -#include "mixed_arena.h" #include "pass.h" +#include "support/mixed_arena.h" #include "support/name.h" #include "wasm-builder.h" #include "wasm-type.h" diff --git a/src/wasm.h b/src/wasm.h index 3e08a99e3d2..ee34522a1bf 100644 --- a/src/wasm.h +++ b/src/wasm.h @@ -34,8 +34,8 @@ #include #include "literal.h" -#include "mixed_arena.h" #include "support/index.h" +#include "support/mixed_arena.h" #include "support/name.h" #include "wasm-features.h" #include "wasm-type.h" diff --git a/src/wasm2js.h b/src/wasm2js.h index 5725b3b0d76..dba3dca308c 100644 --- a/src/wasm2js.h +++ b/src/wasm2js.h @@ -40,7 +40,6 @@ #include "ir/names.h" #include "ir/table-utils.h" #include "ir/utils.h" -#include "mixed_arena.h" #include "passes/passes.h" #include "support/base64.h" #include "support/file.h" From bf6c1aab222fb08b120730b976ca9dbc0579de56 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 6 Jan 2026 09:42:01 -0800 Subject: [PATCH 03/13] fix --- test/gtest/arena.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/gtest/arena.cpp b/test/gtest/arena.cpp index 7453ea9f615..320c2f17d5c 100644 --- a/test/gtest/arena.cpp +++ b/test/gtest/arena.cpp @@ -1,4 +1,4 @@ -#include "mixed_arena.h" +#include "support/mixed_arena.h" #include "gtest/gtest.h" using ArenaTest = ::testing::Test; From 136baefde0c56cfd8153cb6f3e85601cb99337e5 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 6 Jan 2026 13:00:37 -0800 Subject: [PATCH 04/13] gfix --- src/support/topological_sort.h | 8 +++++- src/tools/wasm-ctor-eval.cpp | 9 ++++++- test/lit/ctor-eval/gc-cycle.wast | 45 ++++++++++++++++++++++++++++++++ 3 files changed, 60 insertions(+), 2 deletions(-) diff --git a/src/support/topological_sort.h b/src/support/topological_sort.h index cb78da90622..c0e1a93712e 100644 --- a/src/support/topological_sort.h +++ b/src/support/topological_sort.h @@ -33,6 +33,9 @@ namespace wasm { namespace TopologicalSort { +// Thrown when no valid topological sort exists due to a cycle. +struct CycleException {}; + // An adjacency list containing edges from vertices to their successors. Uses // `Index` because we are primarily sorting elements of Wasm modules. If we ever // need to sort signficantly larger objects, we might need to switch to @@ -220,7 +223,10 @@ template struct TopologicalOrdersImpl { // Select the next available vertex, decrement in-degrees, and update the // sequence of available vertices. Return the Selector for the next vertex. Selector select(TopologicalOrdersImpl& ctx) { - assert(count >= 1); + if (count == 0) { + // No choices remain, indicating a cycle. + throw TopologicalSort::CycleException(); + } assert(start + count <= ctx.buf.size()); if constexpr (TopologicalOrdersImpl::useMinHeap) { ctx.buf[start] = ctx.popChoice(); diff --git a/src/tools/wasm-ctor-eval.cpp b/src/tools/wasm-ctor-eval.cpp index 455c247ccd7..cafbf577013 100644 --- a/src/tools/wasm-ctor-eval.cpp +++ b/src/tools/wasm-ctor-eval.cpp @@ -1451,7 +1451,14 @@ void evalCtors(Module& wasm, std::cout << " ...stopping since could not create module instance: " << fail.why << "\n"; } - return; + } catch (TopologicalSort::CycleException e) { + // We use a topological sort for GC globals. If there is a non-breakable + // cycle there, we will hit an error (we can break cycles in nullable fields + // by setting a null and filling in the value later, etc., but there is + // nothing we can do for non-nullable, immutable fields). + if (!quiet) { + std::cout << " ...stopping since global sorting hit a cycle\n"; + } } } diff --git a/test/lit/ctor-eval/gc-cycle.wast b/test/lit/ctor-eval/gc-cycle.wast index 949a3c3852c..c555c79161e 100644 --- a/test/lit/ctor-eval/gc-cycle.wast +++ b/test/lit/ctor-eval/gc-cycle.wast @@ -1271,6 +1271,10 @@ ) ) ) + +;; A circular reference using a non-nullable, immutable field. Unlike the cases +;; above, we cannot break up such cycles, and must give up. We should at least +;; not error. ;; CHECK: (global $ctor-eval$global (ref (exact $A)) (struct.new $A ;; CHECK-NEXT: (ref.null none) ;; CHECK-NEXT: )) @@ -1291,3 +1295,44 @@ ;; CHECK-NEXT: (global.get $ctor-eval$global) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) +(module + ;; CHECK: (type $struct (struct (field (mut (ref any))))) + + ;; CHECK: (type $1 (func)) + + ;; CHECK: (type $array (array i8)) + (type $array (array i8)) + (type $struct (struct (field (mut (ref any))))) + + ;; CHECK: (export "test" (func $test)) + + ;; CHECK: (func $test (type $1) + ;; CHECK-NEXT: (local $temp (ref $struct)) + ;; CHECK-NEXT: (local.set $temp + ;; CHECK-NEXT: (struct.new $struct + ;; CHECK-NEXT: (array.new_fixed $array 0) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (struct.set $struct 0 + ;; CHECK-NEXT: (local.get $temp) + ;; CHECK-NEXT: (local.get $temp) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $test (export "test") + (local $temp (ref $struct)) + + ;; Start with the struct referring to an array. + (local.set $temp + (struct.new $struct + (array.new_fixed $array 0) + ) + ) + + ;; Make the struct refer to itself, circularly. + (struct.set $struct 0 + (local.get $temp) + (local.get $temp) + ) + ) +) + From 92a09ebd8a56ddea73ddcd0f34b572e0de2a0499 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 6 Jan 2026 13:32:14 -0800 Subject: [PATCH 05/13] try --- src/tools/wasm-ctor-eval.cpp | 33 ++++++++++++++++++++++---------- test/lit/ctor-eval/gc-cycle.wast | 8 ++++++++ 2 files changed, 31 insertions(+), 10 deletions(-) diff --git a/src/tools/wasm-ctor-eval.cpp b/src/tools/wasm-ctor-eval.cpp index cafbf577013..3f5b31c0bde 100644 --- a/src/tools/wasm-ctor-eval.cpp +++ b/src/tools/wasm-ctor-eval.cpp @@ -694,25 +694,38 @@ struct CtorEvalExternalInterface : EvallingModuleRunner::ExternalInterface { } if (!mustBeBefore.empty()) { - auto oldGlobals = std::move(wasm->globals); - // After clearing the globals vector, clear the map as well. - wasm->updateMaps(); - std::unordered_map globalIndexes; - for (Index i = 0; i < oldGlobals.size(); i++) { - globalIndexes[oldGlobals[i]->name] = i; + for (Index i = 0; i < wasm->globals.size(); i++) { + globalIndexes[wasm->globals[i]->name] = i; } + + // Compute the new order for the globals. As we go, track which ones we + // have placed in the new order already. + std::vector newOrder; + std::unordered_set inNewOrder; + // Add the globals that had an important ordering, in the right order. for (auto global : TopologicalSort::sortOf(mustBeBefore.begin(), mustBeBefore.end())) { - wasm->addGlobal(std::move(oldGlobals[globalIndexes[global]])); + newOrder.push_back(globalIndexes[global]); + inNewOrder.insert(global); } // Add all other globals after them. - for (auto& global : oldGlobals) { - if (global) { - wasm->addGlobal(std::move(global)); + for (auto& global : wasm->globals) { + if (!inNewOrder.count(global->name)) { + newOrder.push_back(globalIndexes[global->name]); } } + + // Modify wasm->globals. We must do this all at once, and only if the + // topological sort succeeds (if it fails, we must not leave wasm->globals + // in a mixed state). + auto oldGlobals = std::move(wasm->globals); + // After clearing the globals vector, clear the map as well. + wasm->updateMaps(); + for (auto index : newOrder) { + wasm->addGlobal(std::move(oldGlobals[index])); + } } // After sorting (*), perform the fixups that we need, that is, replace the diff --git a/test/lit/ctor-eval/gc-cycle.wast b/test/lit/ctor-eval/gc-cycle.wast index c555c79161e..3dedb7281b5 100644 --- a/test/lit/ctor-eval/gc-cycle.wast +++ b/test/lit/ctor-eval/gc-cycle.wast @@ -1304,6 +1304,8 @@ (type $array (array i8)) (type $struct (struct (field (mut (ref any))))) + (global $global (mut i32) (i32.const 42)) + ;; CHECK: (export "test" (func $test)) ;; CHECK: (func $test (type $1) @@ -1333,6 +1335,12 @@ (local.get $temp) (local.get $temp) ) + + ;; Use the global, to keep it alive. If we error during global processing + ;; (failing to sort the new globals, which have a cycle), we could error here. + (drop + (global.get $global) + ) ) ) From a87631a3f9a78e2850900b1cfad0493910bd712b Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 6 Jan 2026 13:32:40 -0800 Subject: [PATCH 06/13] untry --- src/tools/wasm-ctor-eval.cpp | 33 ++++++++++----------------------- 1 file changed, 10 insertions(+), 23 deletions(-) diff --git a/src/tools/wasm-ctor-eval.cpp b/src/tools/wasm-ctor-eval.cpp index 3f5b31c0bde..cafbf577013 100644 --- a/src/tools/wasm-ctor-eval.cpp +++ b/src/tools/wasm-ctor-eval.cpp @@ -694,38 +694,25 @@ struct CtorEvalExternalInterface : EvallingModuleRunner::ExternalInterface { } if (!mustBeBefore.empty()) { + auto oldGlobals = std::move(wasm->globals); + // After clearing the globals vector, clear the map as well. + wasm->updateMaps(); + std::unordered_map globalIndexes; - for (Index i = 0; i < wasm->globals.size(); i++) { - globalIndexes[wasm->globals[i]->name] = i; + for (Index i = 0; i < oldGlobals.size(); i++) { + globalIndexes[oldGlobals[i]->name] = i; } - - // Compute the new order for the globals. As we go, track which ones we - // have placed in the new order already. - std::vector newOrder; - std::unordered_set inNewOrder; - // Add the globals that had an important ordering, in the right order. for (auto global : TopologicalSort::sortOf(mustBeBefore.begin(), mustBeBefore.end())) { - newOrder.push_back(globalIndexes[global]); - inNewOrder.insert(global); + wasm->addGlobal(std::move(oldGlobals[globalIndexes[global]])); } // Add all other globals after them. - for (auto& global : wasm->globals) { - if (!inNewOrder.count(global->name)) { - newOrder.push_back(globalIndexes[global->name]); + for (auto& global : oldGlobals) { + if (global) { + wasm->addGlobal(std::move(global)); } } - - // Modify wasm->globals. We must do this all at once, and only if the - // topological sort succeeds (if it fails, we must not leave wasm->globals - // in a mixed state). - auto oldGlobals = std::move(wasm->globals); - // After clearing the globals vector, clear the map as well. - wasm->updateMaps(); - for (auto index : newOrder) { - wasm->addGlobal(std::move(oldGlobals[index])); - } } // After sorting (*), perform the fixups that we need, that is, replace the From 2fd378fb6eb28cb75a5a84f6ae958fcb8f77a4e5 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 8 Jan 2026 08:44:11 -0800 Subject: [PATCH 07/13] test --- test/lit/ctor-eval/gc-cycle.wast | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/lit/ctor-eval/gc-cycle.wast b/test/lit/ctor-eval/gc-cycle.wast index 3dedb7281b5..9ef5375f976 100644 --- a/test/lit/ctor-eval/gc-cycle.wast +++ b/test/lit/ctor-eval/gc-cycle.wast @@ -1323,6 +1323,10 @@ (func $test (export "test") (local $temp (ref $struct)) + (global.set $global + (i32.const 1337) + ) + ;; Start with the struct referring to an array. (local.set $temp (struct.new $struct From 97dba9a7bdfaf43873c14f524635c11c0b6d3258 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 8 Jan 2026 15:41:32 -0800 Subject: [PATCH 08/13] fix --- src/tools/wasm-ctor-eval.cpp | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/tools/wasm-ctor-eval.cpp b/src/tools/wasm-ctor-eval.cpp index 70772e3f138..ae881352aaf 100644 --- a/src/tools/wasm-ctor-eval.cpp +++ b/src/tools/wasm-ctor-eval.cpp @@ -1328,6 +1328,12 @@ EvalCtorOutcome evalCtor(EvallingModuleRunner& instance, } } +// This is set when we find a situation we cannot handle in the middle of our +// work. In that case we give up, throwing away the invalid state in memory. +// TODO: Handle all those situations more gracefully, if that becomes useful +// enough at some point. +static bool invalidState = false; + // Eval all ctors in a module. void evalCtors(Module& wasm, std::vector& ctors, @@ -1440,6 +1446,11 @@ void evalCtors(Module& wasm, if (!quiet) { std::cout << " ...stopping since global sorting hit a cycle\n"; } + + // We found the cycle during the processing of globals, making wasm->globals + // invalid. Rather than refactor the code heavily to handle this very rare + // case, just give up entirely, and avoid writing out the current state. + invalidState = true; } } @@ -1559,6 +1570,17 @@ int main(int argc, const char* argv[]) { if (canEval(wasm)) { evalCtors(wasm, ctors, keptExports); + if (invalidState) { + // We ended up in a state that cannot be written out. Forget about all of + // it, by re-reading the input and continuing from there (with nothing at + // all evalled, effectively). + auto features = wasm.features; + ModuleUtils::clearModule(wasm); + ModuleReader reader; + reader.read(options.extra["infile"], wasm); + wasm.features = features; + } + if (!WasmValidator().validate(wasm)) { std::cout << wasm << '\n'; Fatal() << "error in validating output"; From ba8d3d00ba5097aaeb38b08f5975140c1a5f922c Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 8 Jan 2026 15:42:38 -0800 Subject: [PATCH 09/13] owkr --- test/lit/ctor-eval/gc-cycle.wast | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/test/lit/ctor-eval/gc-cycle.wast b/test/lit/ctor-eval/gc-cycle.wast index 9ef5375f976..8530152e7c0 100644 --- a/test/lit/ctor-eval/gc-cycle.wast +++ b/test/lit/ctor-eval/gc-cycle.wast @@ -1296,20 +1296,23 @@ ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) (module - ;; CHECK: (type $struct (struct (field (mut (ref any))))) - - ;; CHECK: (type $1 (func)) - ;; CHECK: (type $array (array i8)) (type $array (array i8)) + ;; CHECK: (type $struct (struct (field (mut (ref any))))) (type $struct (struct (field (mut (ref any))))) + ;; CHECK: (type $2 (func)) + + ;; CHECK: (global $global (mut i32) (i32.const 42)) (global $global (mut i32) (i32.const 42)) ;; CHECK: (export "test" (func $test)) - ;; CHECK: (func $test (type $1) + ;; CHECK: (func $test (type $2) ;; CHECK-NEXT: (local $temp (ref $struct)) + ;; CHECK-NEXT: (global.set $global + ;; CHECK-NEXT: (i32.const 1337) + ;; CHECK-NEXT: ) ;; CHECK-NEXT: (local.set $temp ;; CHECK-NEXT: (struct.new $struct ;; CHECK-NEXT: (array.new_fixed $array 0) @@ -1323,6 +1326,8 @@ (func $test (export "test") (local $temp (ref $struct)) + ;; Set the global. This will not get written out, as we will cancel all our + ;; work when we hit the cycle below. (TODO: improve that) (global.set $global (i32.const 1337) ) From 94a2181409a945517bf46faba579f82eaf788e23 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 8 Jan 2026 15:44:20 -0800 Subject: [PATCH 10/13] finish --- test/lit/ctor-eval/gc-cycle.wast | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/test/lit/ctor-eval/gc-cycle.wast b/test/lit/ctor-eval/gc-cycle.wast index 8530152e7c0..59c0e86790f 100644 --- a/test/lit/ctor-eval/gc-cycle.wast +++ b/test/lit/ctor-eval/gc-cycle.wast @@ -1272,9 +1272,6 @@ ) ) -;; A circular reference using a non-nullable, immutable field. Unlike the cases -;; above, we cannot break up such cycles, and must give up. We should at least -;; not error. ;; CHECK: (global $ctor-eval$global (ref (exact $A)) (struct.new $A ;; CHECK-NEXT: (ref.null none) ;; CHECK-NEXT: )) @@ -1296,6 +1293,10 @@ ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) (module + ;; A circular reference using a non-nullable, immutable field. Unlike the cases + ;; above, we cannot break up such cycles, and must give up. We should at least + ;; not error. + ;; CHECK: (type $array (array i8)) (type $array (array i8)) ;; CHECK: (type $struct (struct (field (mut (ref any))))) @@ -1344,12 +1345,6 @@ (local.get $temp) (local.get $temp) ) - - ;; Use the global, to keep it alive. If we error during global processing - ;; (failing to sort the new globals, which have a cycle), we could error here. - (drop - (global.get $global) - ) ) ) From 22b30ba789a6f4d073c6373df91f801e309d04e2 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 8 Jan 2026 15:51:50 -0800 Subject: [PATCH 11/13] fix --- test/lit/ctor-eval/gc-cycle.wast | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/lit/ctor-eval/gc-cycle.wast b/test/lit/ctor-eval/gc-cycle.wast index 59c0e86790f..0af9fbf10b3 100644 --- a/test/lit/ctor-eval/gc-cycle.wast +++ b/test/lit/ctor-eval/gc-cycle.wast @@ -1271,7 +1271,6 @@ ) ) ) - ;; CHECK: (global $ctor-eval$global (ref (exact $A)) (struct.new $A ;; CHECK-NEXT: (ref.null none) ;; CHECK-NEXT: )) @@ -1293,7 +1292,7 @@ ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) (module - ;; A circular reference using a non-nullable, immutable field. Unlike the cases + ;; A circular reference using a non-nullable (but mutable) field. Unlike cases ;; above, we cannot break up such cycles, and must give up. We should at least ;; not error. From 7a99bdc4e65e8a123d95f3651d6b36747790ae06 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 8 Jan 2026 15:52:25 -0800 Subject: [PATCH 12/13] fix --- src/tools/wasm-ctor-eval.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/tools/wasm-ctor-eval.cpp b/src/tools/wasm-ctor-eval.cpp index ae881352aaf..03691a040f9 100644 --- a/src/tools/wasm-ctor-eval.cpp +++ b/src/tools/wasm-ctor-eval.cpp @@ -1440,9 +1440,9 @@ void evalCtors(Module& wasm, } } catch (TopologicalSort::CycleException e) { // We use a topological sort for GC globals. If there is a non-breakable - // cycle there, we will hit an error (we can break cycles in nullable fields - // by setting a null and filling in the value later, etc., but there is - // nothing we can do for non-nullable, immutable fields). + // cycle there, we will hit an error (we can break cycles in nullable and + // mutable fields by setting a null and filling in the value later, but + // other situations are a problem). if (!quiet) { std::cout << " ...stopping since global sorting hit a cycle\n"; } From 92e84ebd0088c04e53431114917b27ab56827f42 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 8 Jan 2026 16:18:52 -0800 Subject: [PATCH 13/13] oops we need features when reading the module --- src/tools/wasm-ctor-eval.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/wasm-ctor-eval.cpp b/src/tools/wasm-ctor-eval.cpp index 03691a040f9..4ce09e4479e 100644 --- a/src/tools/wasm-ctor-eval.cpp +++ b/src/tools/wasm-ctor-eval.cpp @@ -1576,9 +1576,9 @@ int main(int argc, const char* argv[]) { // all evalled, effectively). auto features = wasm.features; ModuleUtils::clearModule(wasm); + wasm.features = features; ModuleReader reader; reader.read(options.extra["infile"], wasm); - wasm.features = features; } if (!WasmValidator().validate(wasm)) {