Disallow --nominal with GC (#4758)

Nominal types don't make much sense without GC, and in particular trying to emit
them with typed function references but not GC enabled can result in invalid
binaries because nominal types do not respect the type ordering constraints
required by the typed function references proposal. Making this change was
mostly straightforward, but required fixing the fuzzer to use --nominal only
when GC is enabled and required exiting early from nominal-only optimizations
when GC was not enabled.

Fixes #4756.

10 files changed, 41 insertions, 30 deletions

diff --git a/scripts/fuzz_opt.py b/scripts/fuzz_opt.py
index 745065732..01895c585 100755
--- a/scripts/fuzz_opt.py
+++ b/scripts/fuzz_opt.py
@@ -39,7 +39,6 @@ TYPE_SYSTEM_FLAG = '--nominal'
 
 # feature options that are always passed to the tools.
 CONSTANT_FEATURE_OPTS = ['--all-features']
-CONSTANT_FEATURE_OPTS.append(TYPE_SYSTEM_FLAG)
 
 INPUT_SIZE_MIN = 1024
 INPUT_SIZE_MEAN = 40 * 1024
@@ -120,6 +119,9 @@ def randomize_feature_opts():
                 if possible in IMPLIED_FEATURE_OPTS:
                     FEATURE_OPTS.extend(IMPLIED_FEATURE_OPTS[possible])
     print('randomized feature opts:', '\n  ' + '\n  '.join(FEATURE_OPTS))
+    # Type system flags only make sense when GC is enabled
+    if '--disable-gc' not in FEATURE_OPTS:
+        FEATURE_OPTS.append(TYPE_SYSTEM_FLAG)
 
 
 ALL_FEATURE_OPTS = ['--all-features', '-all', '--mvp-features', '-mvp']
@@ -819,8 +821,8 @@ class CheckDeterminism(TestCaseHandler):
         b1 = open('b1.wasm', 'rb').read()
         b2 = open('b2.wasm', 'rb').read()
         if (b1 != b2):
-            run([in_bin('wasm-dis'), 'b1.wasm', '-o', 'b1.wat', TYPE_SYSTEM_FLAG])
-            run([in_bin('wasm-dis'), 'b2.wasm', '-o', 'b2.wat', TYPE_SYSTEM_FLAG])
+            run([in_bin('wasm-dis'), 'b1.wasm', '-o', 'b1.wat', FEATURE_OPTS])
+            run([in_bin('wasm-dis'), 'b2.wasm', '-o', 'b2.wat', FEATURE_OPTS])
             t1 = open('b1.wat', 'r').read()
             t2 = open('b2.wat', 'r').read()
             compare(t1, t2, 'Output must be deterministic.', verbose=False)
@@ -1379,10 +1381,8 @@ on valid wasm files.)
                 with open('reduce.sh', 'w') as reduce_sh:
                     reduce_sh.write('''\
 # check the input is even a valid wasm file
-echo "At least one of the next two values should be 0:"
-%(wasm_opt)s %(typesystem)s --detect-features %(temp_wasm)s
-echo "  " $?
-%(wasm_opt)s %(typesystem)s --all-features %(temp_wasm)s
+echo "The following value should be 0:"
+%(wasm_opt)s %(features)s %(temp_wasm)s
 echo "  " $?
 
 # run the command
@@ -1419,7 +1419,7 @@ echo "  " $?
                          'auto_init': auto_init,
                          'original_wasm': original_wasm,
                          'temp_wasm': os.path.abspath('t.wasm'),
-                         'typesystem': TYPE_SYSTEM_FLAG,
+                         'features': ' '.join(FEATURE_OPTS),
                          'reduce_sh': os.path.abspath('reduce.sh')})
 
                 print('''\
@@ -1441,7 +1441,7 @@ You can reduce the testcase by running this now:
 vvvv
 
 
-%(wasm_reduce)s %(type_system_flag)s %(original_wasm)s '--command=bash %(reduce_sh)s' -t %(temp_wasm)s -w %(working_wasm)s
+%(wasm_reduce)s %(features)s %(original_wasm)s '--command=bash %(reduce_sh)s' -t %(temp_wasm)s -w %(working_wasm)s
 
 
 ^^^^
@@ -1449,9 +1449,8 @@ vvvv
 
 Make sure to verify by eye that the output says something like this:
 
-At least one of the next two values should be 0:
+The following value should be 0:
   0
-  1
 The following value should be 1:
   1
 
@@ -1471,7 +1470,7 @@ After reduction, the reduced file will be in %(working_wasm)s
                        'working_wasm': os.path.abspath('w.wasm'),
                        'wasm_reduce': in_bin('wasm-reduce'),
                        'reduce_sh': os.path.abspath('reduce.sh'),
-                       'type_system_flag': TYPE_SYSTEM_FLAG})
+                       'features': ' '.join(FEATURE_OPTS)})
                 break
         if given_seed is not None:
             break
diff --git a/src/passes/ConstantFieldPropagation.cpp b/src/passes/ConstantFieldPropagation.cpp
index 2b314eecc..d2b08ea30 100644
--- a/src/passes/ConstantFieldPropagation.cpp
+++ b/src/passes/ConstantFieldPropagation.cpp
@@ -176,6 +176,9 @@ struct PCVScanner
 
 struct ConstantFieldPropagation : public Pass {
   void run(PassRunner* runner, Module* module) override {
+    if (!module->features.hasGC()) {
+      return;
+    }
     if (getTypeSystem() != TypeSystem::Nominal) {
       Fatal() << "ConstantFieldPropagation requires nominal typing";
     }
diff --git a/src/passes/GlobalRefining.cpp b/src/passes/GlobalRefining.cpp
index e43f8ef59..6f2f19842 100644
--- a/src/passes/GlobalRefining.cpp
+++ b/src/passes/GlobalRefining.cpp
@@ -32,6 +32,9 @@ namespace {
 
 struct GlobalRefining : public Pass {
   void run(PassRunner* runner, Module* module) override {
+    if (!module->features.hasGC()) {
+      return;
+    }
     if (getTypeSystem() != TypeSystem::Nominal) {
       Fatal() << "GlobalRefining requires nominal typing";
     }
diff --git a/src/passes/GlobalStructInference.cpp b/src/passes/GlobalStructInference.cpp
index c7eec2b94..b2717b850 100644
--- a/src/passes/GlobalStructInference.cpp
+++ b/src/passes/GlobalStructInference.cpp
@@ -62,6 +62,9 @@ struct GlobalStructInference : public Pass {
   std::unordered_map<HeapType, std::vector<Name>> typeGlobals;
 
   void run(PassRunner* runner, Module* module) override {
+    if (!module->features.hasGC()) {
+      return;
+    }
     if (getTypeSystem() != TypeSystem::Nominal) {
       Fatal() << "GlobalStructInference requires nominal typing";
     }
diff --git a/src/passes/GlobalTypeOptimization.cpp b/src/passes/GlobalTypeOptimization.cpp
index 9b2a1f269..865fe8f69 100644
--- a/src/passes/GlobalTypeOptimization.cpp
+++ b/src/passes/GlobalTypeOptimization.cpp
@@ -112,6 +112,9 @@ struct GlobalTypeOptimization : public Pass {
   std::unordered_map<HeapType, std::vector<Index>> indexesAfterRemovals;
 
   void run(PassRunner* runner, Module* module) override {
+    if (!module->features.hasGC()) {
+      return;
+    }
     if (getTypeSystem() != TypeSystem::Nominal) {
       Fatal() << "GlobalTypeOptimization requires nominal typing";
     }
diff --git a/src/passes/SignaturePruning.cpp b/src/passes/SignaturePruning.cpp
index 4f142eb97..ecf6d7e21 100644
--- a/src/passes/SignaturePruning.cpp
+++ b/src/passes/SignaturePruning.cpp
@@ -48,6 +48,9 @@ struct SignaturePruning : public Pass {
   std::unordered_map<HeapType, Signature> newSignatures;
 
   void run(PassRunner* runner, Module* module) override {
+    if (!module->features.hasGC()) {
+      return;
+    }
     if (getTypeSystem() != TypeSystem::Nominal) {
       Fatal() << "SignaturePruning requires nominal typing";
     }
diff --git a/src/passes/SignatureRefining.cpp b/src/passes/SignatureRefining.cpp
index 0574df434..8a8c45872 100644
--- a/src/passes/SignatureRefining.cpp
+++ b/src/passes/SignatureRefining.cpp
@@ -48,6 +48,9 @@ struct SignatureRefining : public Pass {
   std::unordered_map<HeapType, Signature> newSignatures;
 
   void run(PassRunner* runner, Module* module) override {
+    if (!module->features.hasGC()) {
+      return;
+    }
     if (getTypeSystem() != TypeSystem::Nominal) {
       Fatal() << "SignatureRefining requires nominal typing";
     }
diff --git a/src/passes/TypeRefining.cpp b/src/passes/TypeRefining.cpp
index 0f5169f8f..c755c1af6 100644
--- a/src/passes/TypeRefining.cpp
+++ b/src/passes/TypeRefining.cpp
@@ -78,6 +78,9 @@ struct TypeRefining : public Pass {
   StructUtils::StructValuesMap<FieldInfo> finalInfos;
 
   void run(PassRunner* runner, Module* module) override {
+    if (!module->features.hasGC()) {
+      return;
+    }
     if (getTypeSystem() != TypeSystem::Nominal) {
       Fatal() << "TypeRefining requires nominal typing";
     }
diff --git a/src/tools/tool-options.h b/src/tools/tool-options.h
index bc948fc97..c15291c8d 100644
--- a/src/tools/tool-options.h
+++ b/src/tools/tool-options.h
@@ -176,6 +176,12 @@ struct ToolOptions : public Options {
   void applyFeatures(Module& module) const {
     module.features.enable(enabledFeatures);
     module.features.disable(disabledFeatures);
+    // Non-default type systems only make sense with GC enabled. TODO: Error on
+    // non-GC equirecursive types as well once we make isorecursive the default
+    // if we don't remove equirecursive types entirely.
+    if (!module.features.hasGC() && getTypeSystem() == TypeSystem::Nominal) {
+      Fatal() << "Nominal typing is only allowed when GC is enabled";
+    }
   }
 
 private:
diff --git a/test/lit/nominal-no-gc.wast b/test/lit/nominal-no-gc.wast
index 7247caa5a..82f2698d9 100644
--- a/test/lit/nominal-no-gc.wast
+++ b/test/lit/nominal-no-gc.wast
@@ -1,20 +1,5 @@
-;; Write the module with --nominal but without GC
-;; RUN: wasm-opt %s --nominal --disable-gc -g -o %t.wasm
+;; Using --nominal without GC is not allowed.
 
-;; 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
+;; RUN: not wasm-opt %s --nominal --disable-gc -g -o %t.wasm 2>&1 | filecheck %s
 
-;; Also check that we don't get a failure with the default configuration.
-;; RUN: wasm-opt %t.wasm
-
-;; CHECK-NOT: rec
-
-(module
-  (type $foo (func))
-  (type $bar (func))
-
-  (func $f1 (type $foo))
-  (func $f2 (type $bar))
-)
+;; CHECK: Nominal typing is only allowed when GC is enabled