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 4839577225a..4ce09e4479e 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, @@ -1432,7 +1438,19 @@ 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 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"; + } + + // 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; } } @@ -1552,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); + wasm.features = features; + ModuleReader reader; + reader.read(options.extra["infile"], wasm); + } + if (!WasmValidator().validate(wasm)) { std::cout << wasm << '\n'; Fatal() << "error in validating output"; diff --git a/test/lit/ctor-eval/gc-cycle.wast b/test/lit/ctor-eval/gc-cycle.wast index 949a3c3852c..0af9fbf10b3 100644 --- a/test/lit/ctor-eval/gc-cycle.wast +++ b/test/lit/ctor-eval/gc-cycle.wast @@ -1291,3 +1291,59 @@ ;; CHECK-NEXT: (global.get $ctor-eval$global) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) +(module + ;; 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. + + ;; 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 $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) + ;; 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)) + + ;; 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) + ) + + ;; 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) + ) + ) +) +