Validate unique local names, and use validation in wasm2js. Fixes #1885 (#1886)

* Also fixes some bugs in wasm2js tests that did not validate.
* Rename FeatureOptions => ToolOptions, as they now contain all the basic stuff each tool needs for commandline options (validation yes or no, and which features if so).

Author: kripken

src/tools/optimization-options.h

@@ -14,20 +14,20 @@
  * limitations under the License.
  */
 
-#include "feature-options.h"
+#include "tool-options.h"
 
 //
 // Shared optimization options for commandline tools
 //
 
 namespace wasm {
 
-struct OptimizationOptions : public FeatureOptions {
+struct OptimizationOptions : public ToolOptions {
   static constexpr const char* DEFAULT_OPT_PASSES = "O";
 
   std::vector<std::string> passes;
 
-  OptimizationOptions(const std::string& command, const std::string& description) : FeatureOptions(command, description) {
+  OptimizationOptions(const std::string& command, const std::string& description) : ToolOptions(command, description) {
     (*this).add("", "-O", "execute default optimization passes",
                 Options::Arguments::Zero,
                 [this](Options*, const std::string&) {
@@ -92,11 +92,6 @@ struct OptimizationOptions : public FeatureOptions {
                 [this](Options* o, const std::string& argument) {
                   passOptions.shrinkLevel = atoi(argument.c_str());
                 })
-           .add("--no-validation", "-n", "Disables validation, assumes inputs are correct",
-                Options::Arguments::Zero,
-                [this](Options* o, const std::string& argument) {
-                  passOptions.validate = false;
-                })
            .add("--ignore-implicit-traps", "-iit", "Optimize under the helpful assumption that no surprising traps occur (from load, div/mod, etc.)",
                 Options::Arguments::Zero,
                 [this](Options*, const std::string&) {

src/tools/feature-options.h renamed to src/tools/tool-options.h

@@ -23,10 +23,10 @@
 
 namespace wasm {
 
-struct FeatureOptions : public Options {
+struct ToolOptions : public Options {
   PassOptions passOptions;
 
-  FeatureOptions(const std::string& command, const std::string& description)
+  ToolOptions(const std::string& command, const std::string& description)
       : Options(command, description) {
     (*this)
         .add("--mvp-features", "-mvp", "Disable all non-MVP features",
@@ -83,6 +83,11 @@ struct FeatureOptions : public Options {
              [this](Options *o, const std::string& arguments) {
                passOptions.features.setSIMD(false);
              })
+        .add("--no-validation", "-n", "Disables validation, assumes inputs are correct",
+             Options::Arguments::Zero,
+             [this](Options* o, const std::string& argument) {
+               passOptions.validate = false;
+             });
         ;
   }
 

src/tools/wasm-shell.cpp

@@ -23,14 +23,14 @@
 #include <memory>
 
 #include "execution-results.h"
-#include "feature-options.h"
 #include "pass.h"
 #include "shell-interface.h"
 #include "support/file.h"
 #include "wasm-interpreter.h"
 #include "wasm-printing.h"
 #include "wasm-s-parser.h"
 #include "wasm-validator.h"
+#include "tool-options.h"
 
 using namespace cashew;
 using namespace wasm;
@@ -233,7 +233,7 @@ int main(int argc, const char* argv[]) {
   Name entry;
   std::set<size_t> skipped;
 
-  FeatureOptions options("wasm-shell", "Execute .wast files");
+  ToolOptions options("wasm-shell", "Execute .wast files");
   options
       .add(
           "--entry", "-e", "Call the entry point after parsing the module",

src/tools/wasm2js.cpp

@@ -23,13 +23,14 @@
 #include "support/file.h"
 #include "wasm-s-parser.h"
 #include "wasm2js.h"
+#include "tool-options.h"
 
 using namespace cashew;
 using namespace wasm;
 
 int main(int argc, const char *argv[]) {
   Wasm2JSBuilder::Flags builderFlags;
-  Options options("wasm2js", "Transform .wasm/.wast files to asm.js");
+  ToolOptions options("wasm2js", "Transform .wasm/.wast files to asm.js");
   options
       .add("--output", "-o", "Output file (stdout if not specified)",
            Options::Arguments::One,
@@ -97,6 +98,13 @@ int main(int argc, const char *argv[]) {
     Fatal() << "error in building module, std::bad_alloc (possibly invalid request for silly amounts of memory)";
   }
 
+  if (options.passOptions.validate) {
+    if (!WasmValidator().validate(wasm, options.getFeatures())) {
+      WasmPrinter::printModule(&wasm);
+      Fatal() << "error in validating input";
+    }
+  }
+
   if (options.debug) std::cerr << "asming..." << std::endl;
   Wasm2JSBuilder wasm2js(builderFlags);
   asmjs = wasm2js.processWasm(&wasm);

src/wasm/wasm-validator.cpp

@@ -1072,6 +1072,12 @@ void FunctionValidator::visitFunction(Function* curr) {
   if (curr->imported()) {
     shouldBeTrue(curr->type.is(), curr->name, "imported functions must have a function type");
   }
+  // validate optional local names
+  std::set<Name> seen;
+  for (auto& pair : curr->localNames) {
+    Name name = pair.second;
+    shouldBeTrue(seen.insert(name).second, name, "local names must be unique");
+  }
 }
 
 static bool checkOffset(Expression* curr, Address add, Address max) {

test/passes/optimize-instructions.txt

@@ -1241,7 +1241,7 @@
    (local.get $0)
   )
  )
- (func $almost-sign-ext (; 12 ;) (type $5) (param $0 i32) (param $0 i32)
+ (func $almost-sign-ext (; 12 ;) (type $6) (param $0 i32)
   (drop
    (i32.shr_s
     (i32.shl

test/passes/optimize-instructions.wast

@@ -1377,7 +1377,7 @@
       )
     )
   )
-  (func $almost-sign-ext (param $0 i32) (param $0 i32)
+  (func $almost-sign-ext (param $0 i32)
     (drop
       (i32.shr_s
         (i32.shl

test/wasm2js/grow-memory-tricky.wast

@@ -7,14 +7,14 @@
  (export "f2" (func $1))
 
  (func $0 (result i32)
-  (block
+  (block (result i32)
     (i32.store (i32.const 0) (grow_memory (i32.const 1)))
     (i32.load (i32.const 0))
   )
  )
 
  (func $1 (result i32)
-  (block
+  (block (result i32)
     (i32.store (i32.const 0) (call $grow))
     (i32.load (i32.const 0))
   )

test/wasm2js/i64-select.wast

@@ -2,7 +2,7 @@
 
 (module
  (func $p (param $i i32) (result i32) (local.get $i))
- (func (param i32) (result i64)
+ (func (param i32) (result i32)
   (return
    (select
     (call $p (i32.const -1))