Fix

Require GC for waitqueues

Fix destructor issue

Author: stevenfontanella

src/tools/fuzzing/fuzzing.cpp

@@ -2894,7 +2894,8 @@ Expression* TranslateToFuzzReader::_makeConcrete(Type type) {
                 &Self::makeStringEq,
                 &Self::makeStringMeasure,
                 &Self::makeStringGet);
-    options.add(FeatureSet::ReferenceTypes | FeatureSet::SharedEverything,
+    options.add(FeatureSet::ReferenceTypes | FeatureSet::GC |
+                  FeatureSet::SharedEverything,
                 &Self::makeWaitqueueNotify);
     if (!structWaitFields.empty()) {
       options.add(FeatureSet::ReferenceTypes | FeatureSet::GC |

src/tools/wasm-ctor-eval.cpp

@@ -956,6 +956,8 @@ struct CtorEvalExternalInterface : EvallingModuleRunner::ExternalInterface {
       } else if (heapType.isArray()) {
         // TODO: for repeated identical values, can use ArrayNew
         init = builder.makeArrayNewFixed(heapType, args);
+      } else if (heapType == HeapTypes::sharedWaitqueue) {
+        init = builder.makeWaitqueueNew();
       } else {
         WASM_UNREACHABLE("bad gc type");
       }

src/wasm-type.h

@@ -1194,8 +1194,8 @@ inline bool HeapType::isFunction() const {
 
 inline bool HeapType::isData() const {
   auto kind = getKind();
-  return isMaybeShared(string) || kind == HeapTypeKind::Struct ||
-         kind == HeapTypeKind::Array;
+  return isMaybeShared(string) || isMaybeShared(waitqueue) ||
+         kind == HeapTypeKind::Struct || kind == HeapTypeKind::Array;
 }
 
 inline bool HeapType::isSignature() const {

src/wasm/literal.cpp

@@ -73,6 +73,9 @@ Literal::Literal(Type type) : type(type) {
 
   if (type.isRef() && type.getHeapType().isMaybeShared(HeapType::waitqueue)) {
     assert(type.isNonNullable());
+    // See ~Literal and isData(). isData() is true for waitqueues so we expect
+    // gcData to be present even if null.
+    new (&gcData) std::shared_ptr<GCData>();
     return;
   }
 
@@ -113,7 +116,6 @@ Literal::Literal(std::shared_ptr<GCData> gcData, HeapType type)
   assert((isData() && gcData) ||
          (type.isMaybeShared(HeapType::ext) && gcData) ||
          (type.isMaybeShared(HeapType::string) && gcData) ||
-         (type.isMaybeShared(HeapType::waitqueue) && gcData) ||
          (type.isMaybeShared(HeapType::any) && gcData) ||
          (type.isBottom() && !gcData));
 }
@@ -189,7 +191,6 @@ Literal::Literal(const Literal& other) : type(other.type) {
       return;
     case HeapType::ext:
     case HeapType::any:
-    case HeapType::waitqueue:
       // Externalized or internalized reference/payload.
       new (&gcData) std::shared_ptr<GCData>(other.gcData);
       return;
@@ -205,6 +206,7 @@ Literal::Literal(const Literal& other) : type(other.type) {
     case HeapType::cont:
     case HeapType::struct_:
     case HeapType::array:
+    case HeapType::waitqueue:
       WASM_UNREACHABLE("invalid type");
     case HeapType::string:
       WASM_UNREACHABLE("TODO: string literals");
@@ -371,10 +373,8 @@ std::shared_ptr<FuncData> Literal::getFuncData() const {
 }
 
 std::shared_ptr<GCData> Literal::getGCData() const {
-  assert(
-    isNull() || isData() ||
-    (type.isRef() && (type.getHeapType().isMaybeShared(HeapType::ext) ||
-                      type.getHeapType().isMaybeShared(HeapType::waitqueue))));
+  assert(isNull() || isData() ||
+         (type.isRef() && (type.getHeapType().isMaybeShared(HeapType::ext))));
   return gcData;
 }
 

src/wasm/wasm-type.cpp

@@ -1294,7 +1294,8 @@ FeatureSet HeapType::getFeatures() const {
             return;
           case HeapType::waitqueue:
           case HeapType::nowaitqueue:
-            feats |= FeatureSet::ReferenceTypes | FeatureSet::SharedEverything;
+            feats |= FeatureSet::ReferenceTypes | FeatureSet::GC |
+                     FeatureSet::SharedEverything;
             return;
           case HeapType::noext:
           case HeapType::nofunc:

src/wasm/wasm-validator.cpp

@@ -3696,10 +3696,13 @@ void FunctionValidator::visitStructCmpxchg(StructCmpxchg* curr) {
 }
 
 void FunctionValidator::visitStructWait(StructWait* curr) {
-  shouldBeTrue(
-    !getModule() || getModule()->features.hasSharedEverything(),
-    curr,
-    "struct.wait requires shared-everything [--enable-shared-everything]");
+  FeatureSet expected =
+    FeatureSet::GC | FeatureSet::SharedEverything | FeatureSet::ReferenceTypes;
+  if (!shouldBeTrue(expected.isSubsetOf(getModule()->features),
+                    curr,
+                    "struct.wait requires additional features")) {
+    getStream() << getMissingFeaturesList(*getModule(), expected) << '\n';
+  }
 
   shouldBeSubType(curr->waitqueue->type,
                   Type(HeapTypes::sharedWaitqueue, Nullable),
@@ -3747,17 +3750,23 @@ void FunctionValidator::visitStructWait(StructWait* curr) {
 }
 
 void FunctionValidator::visitWaitqueueNew(WaitqueueNew* curr) {
-  shouldBeTrue(
-    !getModule() || getModule()->features.hasSharedEverything(),
-    curr,
-    "waitqueue.new requires shared-everything [--enable-shared-everything]");
+  FeatureSet expected =
+    FeatureSet::GC | FeatureSet::SharedEverything | FeatureSet::ReferenceTypes;
+  if (!shouldBeTrue(expected.isSubsetOf(getModule()->features),
+                    curr,
+                    "waitqueue.new requires additional features")) {
+    getStream() << getMissingFeaturesList(*getModule(), expected) << '\n';
+  }
 }
 
 void FunctionValidator::visitWaitqueueNotify(WaitqueueNotify* curr) {
-  shouldBeTrue(
-    !getModule() || getModule()->features.hasSharedEverything(),
-    curr,
-    "waitqueue.notify requires shared-everything [--enable-shared-everything]");
+  FeatureSet expected =
+    FeatureSet::GC | FeatureSet::SharedEverything | FeatureSet::ReferenceTypes;
+  if (!shouldBeTrue(expected.isSubsetOf(getModule()->features),
+                    curr,
+                    "waitqueue.notify requires additional features")) {
+    getStream() << getMissingFeaturesList(*getModule(), expected) << '\n';
+  }
 
   shouldBeSubType(
     curr->waitqueue->type,

test/lit/validation/waitqueue.wast

@@ -1,14 +1,21 @@
-;; RUN: not wasm-opt --enable-reference-types --enable-gc %s 2>&1 | filecheck %s
+;; RUN: not wasm-opt %s 2>&1 | filecheck %s
 
 ;; Tests feature-related validations.
 ;; Other validations are in the spec test spec/waitqueue.wast.
 (module
   (type $struct (struct (field i32)))
-  ;; CHECK:      waitqueue.new requires shared-everything [--enable-shared-everything]
+  ;; CHECK: waitqueue.new requires additional features
+  ;; CHECK: [--enable-reference-types --enable-gc --enable-shared-everything]
   (func $new
     (drop (waitqueue.new))
   )
-  ;; CHECK:      struct.wait requires shared-everything [--enable-shared-everything]
+  ;; CHECK: waitqueue.notify requires additional features
+  ;; CHECK: [--enable-reference-types --enable-gc --enable-shared-everything]
+  (func $notify (param $wq (ref null (shared waitqueue)))
+    (drop (waitqueue.notify (local.get $wq) (i32.const 1)))
+  )
+  ;; CHECK: struct.wait requires additional features
+  ;; CHECK: [--enable-reference-types --enable-gc --enable-shared-everything]
   (func $wait-no-feature (param $ref (ref $struct)) (param $wq (ref null (shared waitqueue)))
     (drop (struct.wait $struct 0 (local.get $ref) (local.get $wq) (i32.const 0) (i64.const 0)))
   )