Add Features enum to IR (#1250)

This enum describes which wasm features the IR is expected to include. The
validator should reject operations which require excluded features, and passes
should avoid producing IR which requires excluded features.

This makes it easier to catch possible errors in Binaryen producers (e.g.
emscripten). Asm2wasm has a flag to enable or disable atomics. Other
tools currently just accept all features (as, dis and opt are just for
inspecting or modifying existing modules, so it would be annoying to have to use
flags with those tools and I expect the risk of accidentally introducing
atomics to be low).

1 files changed, 10 insertions, 1 deletions

diff --git a/src/wasm/wasm-validator.cpp b/src/wasm/wasm-validator.cpp
index 780d74c2d..4359b4e13 100644
--- a/src/wasm/wasm-validator.cpp
+++ b/src/wasm/wasm-validator.cpp
@@ -49,6 +49,7 @@ inline std::ostream& printModuleComponent(Expression* curr, std::ostream& stream
 struct ValidationInfo {
   bool validateWeb;
   bool validateGlobally;
+  FeatureSet features;
   bool quiet;
 
   std::atomic<bool> valid;
@@ -483,6 +484,7 @@ void FunctionValidator::visitSetLocal(SetLocal *curr) {
 }
 
 void FunctionValidator::visitLoad(Load *curr) {
+  if (curr->isAtomic) shouldBeTrue(info.features & Feature::Atomics, curr, "Atomic operation (atomics are disabled)");
   shouldBeFalse(curr->isAtomic && !getModule()->memory.shared, curr, "Atomic operation with non-shared memory");
   validateMemBytes(curr->bytes, curr->type, curr);
   validateAlignment(curr->align, curr->type, curr->bytes, curr->isAtomic, curr);
@@ -491,6 +493,7 @@ void FunctionValidator::visitLoad(Load *curr) {
 }
 
 void FunctionValidator::visitStore(Store *curr) {
+  if (curr->isAtomic) shouldBeTrue(info.features & Feature::Atomics, curr, "Atomic operation (atomics are disabled)");
   shouldBeFalse(curr->isAtomic && !getModule()->memory.shared, curr, "Atomic operation with non-shared memory");
   validateMemBytes(curr->bytes, curr->valueType, curr);
   validateAlignment(curr->align, curr->type, curr->bytes, curr->isAtomic, curr);
@@ -500,6 +503,7 @@ void FunctionValidator::visitStore(Store *curr) {
 }
 
 void FunctionValidator::visitAtomicRMW(AtomicRMW* curr) {
+  shouldBeTrue(info.features & Feature::Atomics, curr, "Atomic operation (atomics are disabled)");
   shouldBeFalse(!getModule()->memory.shared, curr, "Atomic operation with non-shared memory");
   validateMemBytes(curr->bytes, curr->type, curr);
   shouldBeEqualOrFirstIsUnreachable(curr->ptr->type, i32, curr, "AtomicRMW pointer type must be i32");
@@ -508,6 +512,7 @@ void FunctionValidator::visitAtomicRMW(AtomicRMW* curr) {
 }
 
 void FunctionValidator::visitAtomicCmpxchg(AtomicCmpxchg* curr) {
+  shouldBeTrue(info.features & Feature::Atomics, curr, "Atomic operation (atomics are disabled)");
   shouldBeFalse(!getModule()->memory.shared, curr, "Atomic operation with non-shared memory");
   validateMemBytes(curr->bytes, curr->type, curr);
   shouldBeEqualOrFirstIsUnreachable(curr->ptr->type, i32, curr, "cmpxchg pointer type must be i32");
@@ -520,6 +525,7 @@ void FunctionValidator::visitAtomicCmpxchg(AtomicCmpxchg* curr) {
 }
 
 void FunctionValidator::visitAtomicWait(AtomicWait* curr) {
+  shouldBeTrue(info.features & Feature::Atomics, curr, "Atomic operation (atomics are disabled)");
   shouldBeFalse(!getModule()->memory.shared, curr, "Atomic operation with non-shared memory");
   shouldBeEqualOrFirstIsUnreachable(curr->type, i32, curr, "AtomicWait must have type i32");
   shouldBeEqualOrFirstIsUnreachable(curr->ptr->type, i32, curr, "AtomicWait pointer type must be i32");
@@ -529,6 +535,7 @@ void FunctionValidator::visitAtomicWait(AtomicWait* curr) {
 }
 
 void FunctionValidator::visitAtomicWake(AtomicWake* curr) {
+  shouldBeTrue(info.features & Feature::Atomics, curr, "Atomic operation (atomics are disabled)");
   shouldBeFalse(!getModule()->memory.shared, curr, "Atomic operation with non-shared memory");
   shouldBeEqualOrFirstIsUnreachable(curr->type, i32, curr, "AtomicWake must have type i32");
   shouldBeEqualOrFirstIsUnreachable(curr->ptr->type, i32, curr, "AtomicWake pointer type must be i32");
@@ -957,6 +964,7 @@ static void validateMemory(Module& module, ValidationInfo& info) {
   info.shouldBeFalse(curr.initial > curr.max, "memory", "memory max >= initial");
   info.shouldBeTrue(curr.max <= Memory::kMaxSize, "memory", "max memory must be <= 4GB");
   info.shouldBeTrue(!curr.shared || curr.hasMax(), "memory", "shared memory must have max size");
+  if (curr.shared) info.shouldBeTrue(info.features & Feature::Atomics, "memory", "memory is shared, but atomics are disabled");
   Index mustBeGreaterOrEqual = 0;
   for (auto& segment : curr.segments) {
     if (!info.shouldBeEqual(segment.offset->type, i32, segment.offset, "segment offset should be i32")) continue;
@@ -998,10 +1006,11 @@ static void validateModule(Module& module, ValidationInfo& info) {
 // TODO: If we want the validator to be part of libwasm rather than libpasses, then
 // Using PassRunner::getPassDebug causes a circular dependence. We should fix that,
 // perhaps by moving some of the pass infrastructure into libsupport.
-bool WasmValidator::validate(Module& module, Flags flags) {
+bool WasmValidator::validate(Module& module, FeatureSet features, Flags flags) {
   ValidationInfo info;
   info.validateWeb = flags & Web;
   info.validateGlobally = flags & Globally;
+  info.features = features;
   info.quiet = flags & Quiet;
   // parallel wasm logic validation
   PassRunner runner(&module);