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).

11 files changed, 41 insertions, 12 deletions

diff --git a/src/asm2wasm.h b/src/asm2wasm.h
index c02376963..a030a2024 100644
--- a/src/asm2wasm.h
+++ b/src/asm2wasm.h
@@ -379,7 +379,6 @@ public:
 
   Asm2WasmPreProcessor& preprocessor;
   bool debug;
-    
   TrapMode trapMode;
   TrappingFunctionContainer trappingFunctions;
   PassOptions passOptions;
@@ -1286,6 +1285,7 @@ void Asm2WasmBuilder::processAsm(Ref ast) {
   };
 
   PassRunner passRunner(&wasm);
+  passRunner.setFeatures(passOptions.features);
   if (debug) {
     passRunner.setDebug(true);
     passRunner.setValidateGlobally(false);
diff --git a/src/pass.h b/src/pass.h
index 20384241c..236878a1a 100644
--- a/src/pass.h
+++ b/src/pass.h
@@ -62,6 +62,7 @@ struct PassOptions {
   int shrinkLevel = 0;   // 0, 1, 2 correspond to -O0, -Os, -Oz
   bool ignoreImplicitTraps = false; // optimize assuming things like div by 0, bad load/store, will not trap
   bool debugInfo = false; // whether to try to preserve debug info through, which are special calls
+  FeatureSet features = Feature::MVP; // Which wasm features to accept, and be allowed to use
 };
 
 //
@@ -87,6 +88,9 @@ struct PassRunner {
   void setValidateGlobally(bool validate) {
     options.validateGlobally = validate;
   }
+  void setFeatures(FeatureSet features) {
+    options.features = features;
+  }
 
   void add(std::string passName) {
     auto pass = PassRegistry::get()->createPass(passName);
diff --git a/src/passes/pass.cpp b/src/passes/pass.cpp
index 4e105f71d..d92a4a80c 100644
--- a/src/passes/pass.cpp
+++ b/src/passes/pass.cpp
@@ -231,7 +231,7 @@ void PassRunner::run() {
       totalTime += diff;
       // validate, ignoring the time
       std::cerr << "[PassRunner]   (validating)\n";
-      if (!WasmValidator().validate(*wasm, validationFlags)) {
+      if (!WasmValidator().validate(*wasm, options.features, validationFlags)) {
         if (passDebug >= 2) {
           std::cerr << "Last pass (" << pass->name << ") broke validation. Here is the module before: \n" << moduleBefore.str() << "\n";
         } else {
@@ -246,7 +246,7 @@ void PassRunner::run() {
     std::cerr << "[PassRunner] passes took " << totalTime.count() << " seconds." << std::endl;
     // validate
     std::cerr << "[PassRunner] (final validation)\n";
-    if (!WasmValidator().validate(*wasm, validationFlags)) {
+    if (!WasmValidator().validate(*wasm, options.features, validationFlags)) {
       std::cerr << "final module does not validate\n";
       abort();
     }
diff --git a/src/tools/asm2wasm.cpp b/src/tools/asm2wasm.cpp
index 455410cd6..4ec68283f 100644
--- a/src/tools/asm2wasm.cpp
+++ b/src/tools/asm2wasm.cpp
@@ -115,6 +115,9 @@ int main(int argc, const char *argv[]) {
       .add("--emit-text", "-S", "Emit text instead of binary for the output file",
            Options::Arguments::Zero,
            [&](Options *o, const std::string &argument) { emitBinary = false; })
+      .add("--enable-threads", "-a", "Enable the Atomics wasm feature",
+           Options::Arguments::Zero,
+           [&](Options *o, const std::string &argument) { options.passOptions.features |= Feature::Atomics; })
       .add_positional("INFILE", Options::Arguments::One,
                       [](Options *o, const std::string &argument) {
                         o->extra["infile"] = argument;
@@ -176,6 +179,7 @@ int main(int argc, const char *argv[]) {
     wasm.memory.segments.emplace_back(init, data);
     if (options.runningDefaultOptimizationPasses()) {
       PassRunner runner(&wasm);
+      runner.setFeatures(options.features);
       runner.add("memory-packing");
       runner.run();
     }
@@ -202,7 +206,7 @@ int main(int argc, const char *argv[]) {
     }
   }
 
-  if (!WasmValidator().validate(wasm)) {
+  if (!WasmValidator().validate(wasm, options.passOptions.features)) {
     Fatal() << "error in validating output";
   }
 
diff --git a/src/tools/optimization-options.h b/src/tools/optimization-options.h
index a15024151..c2f7f44af 100644
--- a/src/tools/optimization-options.h
+++ b/src/tools/optimization-options.h
@@ -25,6 +25,7 @@ struct OptimizationOptions : public Options {
 
   std::vector<std::string> passes;
   PassOptions passOptions;
+  FeatureSet features = Feature::Atomics;
 
   OptimizationOptions(const std::string &command, const std::string &description) : Options(command, description) {
     (*this).add("", "-O", "execute default optimization passes",
@@ -118,6 +119,7 @@ struct OptimizationOptions : public Options {
   void runPasses(Module& wasm) {
     PassRunner passRunner(&wasm, passOptions);
     if (debug) passRunner.setDebug(true);
+    passRunner.setFeatures(features);
     for (auto& pass : passes) {
       if (pass == DEFAULT_OPT_PASSES) {
         passRunner.addDefaultOptimizationPasses();
@@ -130,4 +132,3 @@ struct OptimizationOptions : public Options {
 };
 
 } // namespace wasm
-
diff --git a/src/tools/wasm-as.cpp b/src/tools/wasm-as.cpp
index 956ad213f..d456f43fd 100644
--- a/src/tools/wasm-as.cpp
+++ b/src/tools/wasm-as.cpp
@@ -92,7 +92,7 @@ int main(int argc, const char *argv[]) {
 
   if (options.extra["validate"] != "none") {
     if (options.debug) std::cerr << "Validating..." << std::endl;
-    if (!wasm::WasmValidator().validate(wasm,
+    if (!wasm::WasmValidator().validate(wasm, Feature::All,
          WasmValidator::Globally | (options.extra["validate"] == "web" ? WasmValidator::Web : 0))) {
       Fatal() << "Error: input module is not valid.\n";
     }
diff --git a/src/tools/wasm-opt.cpp b/src/tools/wasm-opt.cpp
index b813b5dd3..7c2a3a35a 100644
--- a/src/tools/wasm-opt.cpp
+++ b/src/tools/wasm-opt.cpp
@@ -110,6 +110,9 @@ int main(int argc, const char* argv[]) {
   options.parse(argc, argv);
 
   Module wasm;
+  // It should be safe to just always enable atomics in wasm-opt, because we
+  // don't expect any passes to accidentally generate atomic ops
+  FeatureSet features = Feature::Atomics;
 
   if (options.debug) std::cerr << "reading...\n";
 
@@ -125,14 +128,14 @@ int main(int argc, const char* argv[]) {
       Fatal() << "error in building module, std::bad_alloc (possibly invalid request for silly amounts of memory)";
     }
 
-    if (!WasmValidator().validate(wasm)) {
+    if (!WasmValidator().validate(wasm, features)) {
       Fatal() << "error in validating input";
     }
   } else {
     // translate-to-fuzz
     TranslateToFuzzReader reader(wasm);
     reader.read(options.extra["infile"]);
-    if (!WasmValidator().validate(wasm)) {
+    if (!WasmValidator().validate(wasm, features)) {
       std::cerr << "translate-to-fuzz must always generate a valid module";
       abort();
     }
@@ -173,7 +176,7 @@ int main(int argc, const char* argv[]) {
   if (options.runningPasses()) {
     if (options.debug) std::cerr << "running passes...\n";
     options.runPasses(wasm);
-    assert(WasmValidator().validate(wasm));
+    assert(WasmValidator().validate(wasm, features));
   }
 
   if (fuzzExec) {
@@ -189,7 +192,7 @@ int main(int argc, const char* argv[]) {
       auto input = buffer.getAsChars();
       WasmBinaryBuilder parser(second, input, false);
       parser.read();
-      assert(WasmValidator().validate(second));
+      assert(WasmValidator().validate(second, features));
     }
     results.check(*compare);
   }
diff --git a/src/wasm-printing.h b/src/wasm-printing.h
index c8b44e394..3b5b94cc2 100644
--- a/src/wasm-printing.h
+++ b/src/wasm-printing.h
@@ -27,6 +27,7 @@ namespace wasm {
 struct WasmPrinter {
   static std::ostream& printModule(Module* module, std::ostream& o) {
     PassRunner passRunner(module);
+    passRunner.setFeatures(Feature::All);
     passRunner.setIsNested(true);
     passRunner.add<Printer>(&o);
     passRunner.run();
diff --git a/src/wasm-validator.h b/src/wasm-validator.h
index 9f9cddb02..81d65a6bd 100644
--- a/src/wasm-validator.h
+++ b/src/wasm-validator.h
@@ -57,7 +57,7 @@ struct WasmValidator {
   };
   typedef uint32_t Flags;
 
-  bool validate(Module& module, Flags flags = Globally);
+  bool validate(Module& module, FeatureSet features = MVP, Flags flags = Globally);
 };
 
 } // namespace wasm
diff --git a/src/wasm.h b/src/wasm.h
index c31aac9c1..485afade1 100644
--- a/src/wasm.h
+++ b/src/wasm.h
@@ -56,6 +56,13 @@
 
 namespace wasm {
 
+enum Feature : uint32_t {
+  MVP = 0,
+  Atomics = 1 << 0,
+  All = 0xffffffff,
+};
+typedef uint32_t FeatureSet;
+
 // An index in a wasm module
 typedef uint32_t Index;
 
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);