From 5fc488229d416116236199fb1187e0015a1a9d53 Mon Sep 17 00:00:00 2001 From: Thomas Lively Date: Fri, 17 Jun 2022 09:38:11 -0700 Subject: [PATCH 1/5] Do not emit recursion groups without GC enabled We emit nominal types as a single large recursion group, but this produces invalid modules when --nominal or --hybrid was used without GC enabled. Fix the bug by always emitting types as though they were structural (i.e. without recursion groups) when GC is not enabled. --- src/wasm/wasm-binary.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp index decd78852c8..87712ceb4c8 100644 --- a/src/wasm/wasm-binary.cpp +++ b/src/wasm/wasm-binary.cpp @@ -223,7 +223,12 @@ void WasmBinaryWriter::writeTypes() { // the type section. With nominal typing there is always one group and with // equirecursive typing there is one group per type. size_t numGroups = 0; - switch (getTypeSystem()) { + // MVP types are structural and do not use recursion groups. + TypeSystem typeSystem = getTypeSystem(); + if (!wasm->features.hasGC()) { + typeSystem = TypeSystem::Equirecursive; + } + switch (typeSystem) { case TypeSystem::Equirecursive: numGroups = indexedTypes.types.size(); break; @@ -242,7 +247,7 @@ void WasmBinaryWriter::writeTypes() { BYN_TRACE("== writeTypes\n"); auto start = startSection(BinaryConsts::Section::Type); o << U32LEB(numGroups); - if (getTypeSystem() == TypeSystem::Nominal) { + if (typeSystem == TypeSystem::Nominal) { // The nominal recursion group contains every type. o << S32LEB(BinaryConsts::EncodedType::Rec) << U32LEB(indexedTypes.types.size()); From 96508b9e0bd6d21142df7ff81e9742f148c445b4 Mon Sep 17 00:00:00 2001 From: Thomas Lively Date: Fri, 17 Jun 2022 09:55:00 -0700 Subject: [PATCH 2/5] add missing test --- test/lit/nominal-no-gc.wast | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 test/lit/nominal-no-gc.wast diff --git a/test/lit/nominal-no-gc.wast b/test/lit/nominal-no-gc.wast new file mode 100644 index 00000000000..92decd8564b --- /dev/null +++ b/test/lit/nominal-no-gc.wast @@ -0,0 +1,15 @@ +;; Write the module with --nominal but without GC +;; RUN: wasm-opt %s --nominal --disable-gc -g -o %t.wasm + +;; We should not get any recursion groups even though we used --nominal +;; RUN: wasm-opt %t.wasm --hybrid -all -S -o - | filecheck %s + +;; CHECK-NOT: rec + +(module + (type $foo (func)) + (type $bar (func)) + + (func $f1 (type $foo)) + (func $f2 (type $bar)) +) \ No newline at end of file From 94316baff618b964957f48dcc1ca6c8db201df76 Mon Sep 17 00:00:00 2001 From: Thomas Lively Date: Fri, 17 Jun 2022 12:29:58 -0700 Subject: [PATCH 3/5] add missing line terminator --- test/lit/nominal-no-gc.wast | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/lit/nominal-no-gc.wast b/test/lit/nominal-no-gc.wast index 92decd8564b..717f3a7dfe9 100644 --- a/test/lit/nominal-no-gc.wast +++ b/test/lit/nominal-no-gc.wast @@ -12,4 +12,4 @@ (func $f1 (type $foo)) (func $f2 (type $bar)) -) \ No newline at end of file +) From 0bd705c2896f8101a47c7bfd363872cfd2b0f3d8 Mon Sep 17 00:00:00 2001 From: Thomas Lively Date: Fri, 17 Jun 2022 12:31:27 -0700 Subject: [PATCH 4/5] update comment --- test/lit/nominal-no-gc.wast | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/lit/nominal-no-gc.wast b/test/lit/nominal-no-gc.wast index 717f3a7dfe9..c562e41002c 100644 --- a/test/lit/nominal-no-gc.wast +++ b/test/lit/nominal-no-gc.wast @@ -1,7 +1,9 @@ ;; Write the module with --nominal but without GC ;; RUN: wasm-opt %s --nominal --disable-gc -g -o %t.wasm -;; We should not get any recursion groups even though we used --nominal +;; We should not get any recursion groups even though we used --nominal. We use +;; --hybrid -all here to make sure that any rec groups from the binary will +;; actually show up in the output and cause the test to fail. ;; RUN: wasm-opt %t.wasm --hybrid -all -S -o - | filecheck %s ;; CHECK-NOT: rec From 2a0fe1004626d10bc7abb9297cdcda47a24f004e Mon Sep 17 00:00:00 2001 From: Thomas Lively Date: Fri, 17 Jun 2022 17:00:12 -0700 Subject: [PATCH 5/5] Add test with default flags --- test/lit/nominal-no-gc.wast | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/lit/nominal-no-gc.wast b/test/lit/nominal-no-gc.wast index c562e41002c..7247caa5a04 100644 --- a/test/lit/nominal-no-gc.wast +++ b/test/lit/nominal-no-gc.wast @@ -6,6 +6,9 @@ ;; actually show up in the output and cause the test to fail. ;; RUN: wasm-opt %t.wasm --hybrid -all -S -o - | filecheck %s +;; Also check that we don't get a failure with the default configuration. +;; RUN: wasm-opt %t.wasm + ;; CHECK-NOT: rec (module