Make local.tee's type its local's type (#2511)

According to the current spec, `local.tee`'s return type should be the
same as its local's type. (Discussions on whether we should change this
rule is going on in WebAssembly/reference-types#55, but here I will
assume this spec does not change. If this changes, we should change many
parts of Binaryen transformation anyway...)

But currently in Binaryen `local.tee`'s type is computed from its
value's type. This didn't make any difference in the MVP, but after we
have subtype relationship in #2451, this can become a problem. For
example:
```
(func $test (result funcref) (local $0 anyref)
  (local.tee $0
    (ref.func $test)
  )
)
```
This shouldn't validate in the spec, but this will pass Binaryen
validation with the current `local.tee` implementation.

This makes `local.tee`'s type computed from the local's type, and makes
`LocalSet::makeTee` get a type parameter, to which we should pass the
its corresponding local's type. We don't embed the local type in the
class `LocalSet` because it may increase memory size.

This also fixes the type of `local.get` to be the local type where
`local.get` and `local.set` pair is created from `local.tee`.

5 files changed, 26 insertions, 24 deletions

diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp
index d86eea8f5..2787725a6 100644
--- a/src/wasm/wasm-binary.cpp
+++ b/src/wasm/wasm-binary.cpp
@@ -2459,8 +2459,11 @@ void WasmBinaryBuilder::visitLocalSet(LocalSet* curr, uint8_t code) {
     throwError("bad local.set index");
   }
   curr->value = popNonVoidExpression();
-  curr->type = curr->value->type;
-  curr->setTee(code == BinaryConsts::LocalTee);
+  if (code == BinaryConsts::LocalTee) {
+    curr->makeTee(currFunction->getLocalType(curr->index));
+  } else {
+    curr->makeSet();
+  }
   curr->finalize();
 }
 
diff --git a/src/wasm/wasm-emscripten.cpp b/src/wasm/wasm-emscripten.cpp
index 9a0b145da..be4e51b9e 100644
--- a/src/wasm/wasm-emscripten.cpp
+++ b/src/wasm/wasm-emscripten.cpp
@@ -112,7 +112,7 @@ inline Expression* stackBoundsCheck(Builder& builder,
   auto check =
     builder.makeIf(builder.makeBinary(
                      BinaryOp::LtUInt32,
-                     builder.makeLocalTee(newSP, value),
+                     builder.makeLocalTee(newSP, value, stackPointer->type),
                      builder.makeGlobalGet(stackLimit->name, stackLimit->type)),
                    builder.makeCall(handler, {}, none));
   // (global.set $__stack_pointer (local.get $newSP))
@@ -172,7 +172,7 @@ void EmscriptenGlueGenerator::generateStackAllocFunction() {
   const static uint32_t bitMask = bitAlignment - 1;
   Const* subConst = builder.makeConst(Literal(~bitMask));
   Binary* maskedSub = builder.makeBinary(AndInt32, sub, subConst);
-  LocalSet* teeStackLocal = builder.makeLocalTee(1, maskedSub);
+  LocalSet* teeStackLocal = builder.makeLocalTee(1, maskedSub, i32);
   Expression* storeStack = generateStoreStackPointer(function, teeStackLocal);
 
   Block* block = builder.makeBlock();
diff --git a/src/wasm/wasm-s-parser.cpp b/src/wasm/wasm-s-parser.cpp
index 10b6aead7..1319d80fc 100644
--- a/src/wasm/wasm-s-parser.cpp
+++ b/src/wasm/wasm-s-parser.cpp
@@ -997,7 +997,7 @@ Expression* SExpressionWasmBuilder::makeLocalTee(Element& s) {
   auto ret = allocator.alloc<LocalSet>();
   ret->index = getLocalIndex(*s[1]);
   ret->value = parseExpression(s[2]);
-  ret->setTee(true);
+  ret->makeTee(currFunction->getLocalType(ret->index));
   ret->finalize();
   return ret;
 }
@@ -1006,7 +1006,7 @@ Expression* SExpressionWasmBuilder::makeLocalSet(Element& s) {
   auto ret = allocator.alloc<LocalSet>();
   ret->index = getLocalIndex(*s[1]);
   ret->value = parseExpression(s[2]);
-  ret->setTee(false);
+  ret->makeSet();
   ret->finalize();
   return ret;
 }
diff --git a/src/wasm/wasm-validator.cpp b/src/wasm/wasm-validator.cpp
index c6de444c1..0efc8cf4f 100644
--- a/src/wasm/wasm-validator.cpp
+++ b/src/wasm/wasm-validator.cpp
@@ -718,15 +718,15 @@ void FunctionValidator::visitLocalSet(LocalSet* curr) {
                    "local.set index must be small enough")) {
     if (curr->value->type != unreachable) {
       if (curr->type != none) { // tee is ok anyhow
-        shouldBeEqualOrFirstIsUnreachable(curr->value->type,
-                                          curr->type,
-                                          curr,
-                                          "local.set type must be correct");
+        shouldBeEqual(getFunction()->getLocalType(curr->index),
+                      curr->type,
+                      curr,
+                      "local.set type must be correct");
       }
-      shouldBeEqual(getFunction()->getLocalType(curr->index),
-                    curr->value->type,
+      shouldBeEqual(curr->value->type,
+                    getFunction()->getLocalType(curr->index),
                     curr,
-                    "local.set type must match function");
+                    "local.set's value type must be correct");
     }
   }
 }
diff --git a/src/wasm/wasm.cpp b/src/wasm/wasm.cpp
index 83829418e..783d51e0f 100644
--- a/src/wasm/wasm.cpp
+++ b/src/wasm/wasm.cpp
@@ -442,24 +442,23 @@ void CallIndirect::finalize() {
   }
 }
 
-bool LocalSet::isTee() { return type != none; }
+bool LocalSet::isTee() const { return type != none; }
 
-void LocalSet::setTee(bool is) {
-  if (is) {
-    type = value->type;
-  } else {
-    type = none;
-  }
+// Changes to local.tee. The type of the local should be given.
+void LocalSet::makeTee(Type type_) {
+  type = type_;
+  finalize(); // type may need to be unreachable
+}
+
+// Changes to local.set.
+void LocalSet::makeSet() {
+  type = none;
   finalize(); // type may need to be unreachable
 }
 
 void LocalSet::finalize() {
   if (value->type == unreachable) {
     type = unreachable;
-  } else if (isTee()) {
-    type = value->type;
-  } else {
-    type = none;
   }
 }