From 661fe5e72987e97d5f6db0d4cbefc73c1e166e36 Mon Sep 17 00:00:00 2001
From: Thomas Lively <tlively@google.com>
Date: Fri, 3 Mar 2023 17:53:53 -0600
Subject: [PATCH] Skip function references when detecting uninhabitable types
 (#5545)

Function references are always inhabitable because functions can be created with
any function type, even types that refer to uninhabitable types. Take advantage
of this by skipping function references when finding non-nullable reference
cycles that cause uninhabitability.
---
 src/tools/fuzzing/heap-types.cpp | 35 +++++++++++++++++++-------------
 src/tools/wasm-fuzz-types.cpp    | 18 +++++++---------
 2 files changed, 28 insertions(+), 25 deletions(-)

diff --git a/src/tools/fuzzing/heap-types.cpp b/src/tools/fuzzing/heap-types.cpp
index cb33a65ed24..174f3fd891d 100644
--- a/src/tools/fuzzing/heap-types.cpp
+++ b/src/tools/fuzzing/heap-types.cpp
@@ -685,10 +685,8 @@ struct Inhabitator {
   //
   // An invariant field of a heaptype must have the same type in subtypes of
   // that heaptype. A covariant field of a heaptype must be typed with a subtype
-  // of its original type in subtypes of the heaptype. A contravariant field of
-  // a heap type must be typed with a supertype of its original type in subtypes
-  // of the heaptype.
-  enum Variance { Invariant, Covariant, Contravariant };
+  // of its original type in subtypes of the heaptype.
+  enum Variance { Invariant, Covariant };
 
   // The input types.
   const std::vector<HeapType>& types;
@@ -712,7 +710,7 @@ struct Inhabitator {
 
 Inhabitator::Variance Inhabitator::getVariance(FieldPos field) {
   auto [type, idx] = field;
-  assert(!type.isBasic());
+  assert(!type.isBasic() && !type.isSignature());
   if (type.isStruct()) {
     if (type.getStruct().fields[idx].mutable_ == Mutable) {
       return Invariant;
@@ -727,13 +725,6 @@ Inhabitator::Variance Inhabitator::getVariance(FieldPos field) {
       return Covariant;
     }
   }
-  if (type.isSignature()) {
-    if (idx < type.getSignature().params.size()) {
-      return Contravariant;
-    } else {
-      return Covariant;
-    }
-  }
   WASM_UNREACHABLE("unexpected kind");
 }
 
@@ -768,8 +759,6 @@ void Inhabitator::markNullable(FieldPos field) {
           curr = *super;
         }
       }
-      [[fallthrough]];
-    case Contravariant:
       // Mark the field nullable in all subtypes. If the subtype field is
       // already nullable, that's ok and this will have no effect. TODO: Remove
       // this extra `index` variable once we have C++20. It's a workaround for
@@ -784,6 +773,11 @@ void Inhabitator::markNullable(FieldPos field) {
 
 void Inhabitator::markBottomRefsNullable() {
   for (auto type : types) {
+    if (type.isSignature()) {
+      // Functions can always be instantiated, even if their types refer to
+      // uninhabitable types.
+      continue;
+    }
     auto children = type.getTypeChildren();
     for (size_t i = 0; i < children.size(); ++i) {
       auto child = children[i];
@@ -801,6 +795,11 @@ void Inhabitator::markExternRefsNullable() {
   // TODO: Remove this once the fuzzer imports externref globals or gets some
   // other way to instantiate externrefs.
   for (auto type : types) {
+    if (type.isSignature()) {
+      // Functions can always be instantiated, even if their types refer to
+      // uninhabitable types.
+      continue;
+    }
     auto children = type.getTypeChildren();
     for (size_t i = 0; i < children.size(); ++i) {
       auto child = children[i];
@@ -863,6 +862,14 @@ void Inhabitator::breakNonNullableCycles() {
           ++idx;
           continue;
         }
+        // Skip references to function types. Functions types can always be
+        // instantiated since functions can be created even with uninhabitable
+        // params or results. Function references therefore break cycles that
+        // would otherwise produce uninhabitability.
+        if (heapType.isSignature()) {
+          ++idx;
+          continue;
+        }
         // If this ref forms a cycle, break the cycle by marking it nullable and
         // continue.
         if (auto it = visiting.find(heapType); it != visiting.end()) {
diff --git a/src/tools/wasm-fuzz-types.cpp b/src/tools/wasm-fuzz-types.cpp
index dd85223dc68..0e66d48ca68 100644
--- a/src/tools/wasm-fuzz-types.cpp
+++ b/src/tools/wasm-fuzz-types.cpp
@@ -502,9 +502,14 @@ static std::optional<HeapType>
 findUninhabitable(HeapType type,
                   std::unordered_set<HeapType>& visited,
                   std::unordered_set<HeapType>& visiting) {
-  if (type.isBasic() || visited.count(type)) {
+  if (type.isBasic()) {
     return std::nullopt;
-  } else if (type.isBasic()) {
+  }
+  if (type.isSignature()) {
+    // Function types are always inhabitable.
+    return std::nullopt;
+  }
+  if (visited.count(type)) {
     return std::nullopt;
   }
 
@@ -523,15 +528,6 @@ findUninhabitable(HeapType type,
           type, type.getArray().element.type, visited, visiting)) {
       return t;
     }
-  } else if (type.isSignature()) {
-    auto sig = type.getSignature();
-    for (auto types : {sig.params, sig.results}) {
-      for (auto child : types) {
-        if (auto t = findUninhabitable(type, child, visited, visiting)) {
-          return t;
-        }
-      }
-    }
   } else {
     WASM_UNREACHABLE("unexpected type kind");
   }