Validation for AtomicRMW and cmpxchg (#1092)

Also fix cases where fail() had the arguments backwards. Wasn't an error because lol templates.
Also fix printModuleComponent template to SFINAE on Expression* so we properly get the specialized version.

2 files changed, 37 insertions, 10 deletions

diff --git a/src/wasm-validator.h b/src/wasm-validator.h
index 2ec530476..f778e6f24 100644
--- a/src/wasm-validator.h
+++ b/src/wasm-validator.h
@@ -46,13 +46,15 @@
 namespace wasm {
 
 // Print anything that can be streamed to an ostream
-template <typename T>
+template <typename T,
+  typename std::enable_if<
+    !std::is_base_of<Expression, typename std::remove_pointer<T>::type>::value
+  >::type* = nullptr>
 inline std::ostream& printModuleComponent(T curr, std::ostream& stream) {
   stream << curr << std::endl;
   return stream;
 }
-// Specialization for Expressions to print type info too
-template <>
+// Extra overload for Expressions, to print type info too
 inline std::ostream& printModuleComponent(Expression* curr, std::ostream& stream) {
   WasmPrinter::printExpression(curr, stream, false, true) << std::endl;
   return stream;
@@ -162,13 +164,13 @@ public:
   // helpers
  private:
   template <typename T, typename S>
-  std::ostream& fail(T curr, S text);
+  std::ostream& fail(S text, T curr);
   std::ostream& printFailureHeader();
 
   template<typename T>
   bool shouldBeTrue(bool result, T curr, const char* text) {
     if (!result) {
-      fail(curr, "unexpected false: " + std::string(text));
+      fail("unexpected false: " + std::string(text), curr);
       return false;
     }
     return result;
@@ -176,7 +178,7 @@ public:
   template<typename T>
   bool shouldBeFalse(bool result, T curr, const char* text) {
     if (result) {
-      fail(curr, "unexpected true: " + std::string(text));
+      fail("unexpected true: " + std::string(text), curr);
       return false;
     }
     return result;
@@ -187,7 +189,7 @@ public:
     if (left != right) {
       std::ostringstream ss;
       ss << left << " != " << right << ": " << text;
-      fail(curr, ss.str());
+      fail(ss.str(), curr);
       return false;
     }
     return true;
@@ -198,7 +200,7 @@ public:
     if (left != unreachable && left != right) {
       std::ostringstream ss;
       ss << left << " != " << right << ": " << text;
-      fail(curr, ss.str());
+      fail(ss.str(), curr);
       return false;
     }
     return true;
@@ -209,12 +211,13 @@ public:
     if (left == right) {
       std::ostringstream ss;
       ss << left << " == " << right << ": " << text;
-      fail(curr, ss.str());
+      fail(ss.str(), curr);
       return false;
     }
     return true;
   }
 
+  void shouldBeIntOrUnreachable(WasmType ty, Expression* curr, const char* text);
   void validateAlignment(size_t align, WasmType type, Index bytes, bool isAtomic,
                          Expression* curr);
   void validateMemBytes(uint8_t bytes, WasmType ty, Expression* curr);
diff --git a/src/wasm/wasm-validator.cpp b/src/wasm/wasm-validator.cpp
index c1f44a68b..1b328ec19 100644
--- a/src/wasm/wasm-validator.cpp
+++ b/src/wasm/wasm-validator.cpp
@@ -220,22 +220,46 @@ void WasmValidator::visitSetLocal(SetLocal *curr) {
   }
 }
 void WasmValidator::visitLoad(Load *curr) {
+  if (curr->isAtomic && !getModule()->memory.shared) fail("Atomic operation with non-shared memory", curr);
   validateMemBytes(curr->bytes, curr->type, curr);
   validateAlignment(curr->align, curr->type, curr->bytes, curr->isAtomic, curr);
   shouldBeEqualOrFirstIsUnreachable(curr->ptr->type, i32, curr, "load pointer type must be i32");
 }
 void WasmValidator::visitStore(Store *curr) {
+  if (curr->isAtomic && !getModule()->memory.shared) fail("Atomic operation with non-shared memory", curr);
   validateMemBytes(curr->bytes, curr->valueType, curr);
   validateAlignment(curr->align, curr->type, curr->bytes, curr->isAtomic, curr);
   shouldBeEqualOrFirstIsUnreachable(curr->ptr->type, i32, curr, "store pointer type must be i32");
   shouldBeUnequal(curr->value->type, none, curr, "store value type must not be none");
   shouldBeEqualOrFirstIsUnreachable(curr->value->type, curr->valueType, curr, "store value type must match");
 }
+void WasmValidator::shouldBeIntOrUnreachable(WasmType ty, Expression* curr, const char* text) {
+  switch (ty) {
+    case i32:
+    case i64:
+    case unreachable: {
+      break;
+    }
+    default: fail(text, curr);
+  }
+}
 void WasmValidator::visitAtomicRMW(AtomicRMW* curr) {
+  if (!getModule()->memory.shared) fail("Atomic operation with non-shared memory", curr);
   validateMemBytes(curr->bytes, curr->type, curr);
+  shouldBeEqualOrFirstIsUnreachable(curr->ptr->type, i32, curr, "AtomicRMW pointer type must be i32");
+  shouldBeEqualOrFirstIsUnreachable(curr->value->type, curr->type, curr, "AtomicRMW result type must match operand");
+  shouldBeIntOrUnreachable(curr->type, curr, "Atomic operations are only valid on int types");
 }
 void WasmValidator::visitAtomicCmpxchg(AtomicCmpxchg* curr) {
+  if (!getModule()->memory.shared) fail("Atomic operation with non-shared memory", curr);
   validateMemBytes(curr->bytes, curr->type, curr);
+  shouldBeEqualOrFirstIsUnreachable(curr->ptr->type, i32, curr, "cmpxchg pointer type must be i32");
+  if (curr->expected->type != unreachable && curr->replacement->type != unreachable) {
+    shouldBeEqual(curr->expected->type, curr->replacement->type, curr, "cmpxchg operand types must match");
+  }
+  shouldBeEqualOrFirstIsUnreachable(curr->expected->type, curr->type, curr, "Cmpxchg result type must match expected");
+  shouldBeEqualOrFirstIsUnreachable(curr->replacement->type, curr->type, curr, "Cmpxchg result type must match replacement");
+  shouldBeIntOrUnreachable(curr->expected->type, curr, "Atomic operations are only valid on int types");
 }
 void WasmValidator::validateMemBytes(uint8_t bytes, WasmType ty, Expression* curr) {
   switch (bytes) {
@@ -655,7 +679,7 @@ void WasmValidator::validateBinaryenIR(Module& wasm) {
 }
 
 template <typename T, typename S>
-std::ostream& WasmValidator::fail(T curr, S text) {
+std::ostream& WasmValidator::fail(S text, T curr) {
   valid = false;
   auto& ret = printFailureHeader() << text << ", on \n";
   return printModuleComponent(curr, ret);