Validation fixes for #1317 (#1347)

* add get_global/set_global validation 
* validate get_local index
* update builds
* fix tests

Author: kripken

bin/binaryen.js

@@ -228,6 +228,8 @@ struct FunctionValidator : public WalkerPass<PostWalker<FunctionValidator>> {
   void visitCallIndirect(CallIndirect *curr);
   void visitGetLocal(GetLocal* curr);
   void visitSetLocal(SetLocal *curr);
+  void visitGetGlobal(GetGlobal* curr);
+  void visitSetGlobal(SetGlobal *curr);
   void visitLoad(Load *curr);
   void visitStore(Store *curr);
   void visitAtomicRMW(AtomicRMW *curr);
@@ -470,6 +472,7 @@ void FunctionValidator::visitCallIndirect(CallIndirect *curr) {
 }
 
 void FunctionValidator::visitGetLocal(GetLocal* curr) {
+  shouldBeTrue(curr->index < getFunction()->getNumLocals(), curr, "get_local index must be small enough");
   shouldBeTrue(isConcreteWasmType(curr->type), curr, "get_local must have a valid type - check what you provided when you constructed the node");
 }
 
@@ -483,6 +486,19 @@ void FunctionValidator::visitSetLocal(SetLocal *curr) {
   }
 }
 
+void FunctionValidator::visitGetGlobal(GetGlobal* curr) {
+  if (!info.validateGlobally) return;
+  shouldBeTrue(getModule()->getGlobalOrNull(curr->name) || getModule()->getImportOrNull(curr->name), curr, "get_global name must be valid");
+}
+
+void FunctionValidator::visitSetGlobal(SetGlobal *curr) {
+  if (!info.validateGlobally) return;
+  auto* global = getModule()->getGlobalOrNull(curr->name);
+  shouldBeTrue(global, curr, "set_global name must be valid (and not an import; imports can't be modified)");
+  shouldBeTrue(global->mutable_, curr, "set_global global must be mutable");
+  shouldBeEqualOrFirstIsUnreachable(curr->value->type, global->type, curr, "set_global value must have right type");
+}
+
 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");

bin/wasm.js

@@ -0,0 +1,28 @@
+(function() {
+  var mod = new Binaryen.Module();
+  var funcType = mod.addFunctionType("v", Binaryen.void, []);
+  var func = mod.addFunction("test", funcType, [],
+    mod.block("", [
+      mod.drop(
+        mod.getGlobal("missing", Binaryen.i32)
+      )
+    ])
+  );
+  mod.addExport("test", func);
+  console.log(mod.validate())
+})();
+
+(function() {
+  var mod = new Binaryen.Module();
+  var funcType = mod.addFunctionType("v", Binaryen.void, []);
+  var func = mod.addFunction("test", funcType, [],
+    mod.block("", [
+      mod.drop(
+        mod.getLocal(0, Binaryen.i32)
+      )
+    ])
+  );
+  mod.addFunctionExport("test", "test", func);
+  console.log(mod.validate())
+})();
+

src/wasm/wasm-validator.cpp

@@ -0,0 +1,6 @@
+[wasm-validator error in function $test] unexpected false: get_global name must be valid, on 
+[i32] (get_global $missing)
+0
+[wasm-validator error in function $test] unexpected false: get_local index must be small enough, on 
+[i32] (get_local $0)
+0

test/binaryen.js/validation_errors.js

@@ -461,7 +461,7 @@
  )
  (func $replace-block-changes-later-when-if-goes (; 26 ;) (type $1)
   (block $top
-   (set_global $global$0
+   (set_global $x
     (i32.const 0)
    )
    (block $inner

test/binaryen.js/validation_errors.js.txt

@@ -676,7 +676,7 @@
  )
  (func $replace-block-changes-later-when-if-goes
   (block $top ;; and so should this
-   (set_global $global$0
+   (set_global $x
     (i32.const 0)
    )
    (drop

test/passes/dce.txt

@@ -3,6 +3,7 @@
  (type $1 (func (result i32)))
  (type $2 (func))
  (type $3 (func (result f64)))
+ (global $global$0 (mut i32) (i32.const 0))
  (memory $0 0)
  (func $x (; 0 ;) (type $0) (param $x i32)
   (call $x

test/passes/dce.wast

@@ -1,6 +1,7 @@
 (module
   (memory 0)
   (type $0 (func (param i32)))
+  (global $global$0 (mut i32) (i32.const 0))
   (func $x (type $0) (param $x i32)
     (call $x
       (i32.add