Select instr. with multiple results is invalid (#1582)

The reference-types proposal adds a select instruction with a type vector, but any number greater than 1 is invalid. Fixes #1577.
WebAssembly · Dec 4, 2020 · 215279f · 215279f
1 parent f6a833f
commit 215279f
Show file tree

Hide file tree

Showing 14 changed files with 78 additions and 37 deletions.
diff --git a/src/binary-reader-ir.cc b/src/binary-reader-ir.cc
@@ -189,7 +189,7 @@ class BinaryReaderIR : public BinaryReaderNop {
   Result OnNopExpr() override;
   Result OnRethrowExpr() override;
   Result OnReturnExpr() override;
-  Result OnSelectExpr(Type result_type) override;
+  Result OnSelectExpr(Index result_count, Type* result_types) override;
   Result OnStoreExpr(Opcode opcode,
                      Address alignment_log2,
                      Address offset) override;
@@ -937,8 +937,10 @@ Result BinaryReaderIR::OnReturnExpr() {
   return AppendExpr(MakeUnique<ReturnExpr>());
 }
 
-Result BinaryReaderIR::OnSelectExpr(Type result_type) {
-  return AppendExpr(MakeUnique<SelectExpr>(result_type.GetInlineVector()));
+Result BinaryReaderIR::OnSelectExpr(Index result_count, Type* result_types) {
+  TypeVector results;
+  results.assign(result_types, result_types + result_count);
+  return AppendExpr(MakeUnique<SelectExpr>(results));
 }
 
 Result BinaryReaderIR::OnGlobalSetExpr(Index global_index) {

diff --git a/src/binary-reader-logging.cc b/src/binary-reader-logging.cc
@@ -375,9 +375,12 @@ Result BinaryReaderLogging::OnLoopExpr(Type sig_type) {
   return reader_->OnLoopExpr(sig_type);
 }
 
-Result BinaryReaderLogging::OnSelectExpr(Type return_type) {
-  LOGF("OnSelectExpr(return_type: %s)\n", return_type.GetName());
-  return reader_->OnSelectExpr(return_type);
+Result BinaryReaderLogging::OnSelectExpr(Index result_count,
+                                         Type* result_types) {
+  LOGF("OnSelectExpr(return_type: ");
+  LogTypes(result_count, result_types);
+  LOGF_NOINDENT(")\n");
+  return reader_->OnSelectExpr(result_count, result_types);
 }
 
 Result BinaryReaderLogging::OnTryExpr(Type sig_type) {

diff --git a/src/binary-reader-logging.h b/src/binary-reader-logging.h
@@ -209,7 +209,7 @@ class BinaryReaderLogging : public BinaryReaderDelegate {
   Result OnReturnCallExpr(Index func_index) override;
   Result OnReturnCallIndirectExpr(Index sig_index, Index table_index) override;
   Result OnReturnExpr() override;
-  Result OnSelectExpr(Type result_type) override;
+  Result OnSelectExpr(Index result_count, Type* result_types) override;
   Result OnStoreExpr(Opcode opcode,
                      Address alignment_log2,
                      Address offset) override;

diff --git a/src/binary-reader-nop.h b/src/binary-reader-nop.h
@@ -286,7 +286,9 @@ class BinaryReaderNop : public BinaryReaderDelegate {
   Result OnReturnCallExpr(Index sig_index) override { return Result::Ok; }
   Result OnReturnCallIndirectExpr(Index sig_index, Index table_index) override { return Result::Ok; }
   Result OnReturnExpr() override { return Result::Ok; }
-  Result OnSelectExpr(Type result_type) override { return Result::Ok; }
+  Result OnSelectExpr(Index result_count, Type* result_types) override {
+    return Result::Ok;
+  }
   Result OnStoreExpr(Opcode opcode,
                      Address alignment_log2,
                      Address offset) override {

diff --git a/src/binary-reader.cc b/src/binary-reader.cc
@@ -686,21 +686,28 @@ Result BinaryReader::ReadFunctionBody(Offset end_offset) {
         break;
 
       case Opcode::SelectT: {
-        Index count;
-        CHECK_RESULT(ReadCount(&count, "num result types"));
-        if (count != 1) {
-          PrintError("invalid arity in select instrcution: %u", count);
-          return Result::Error;
+        Index num_results;
+        CHECK_RESULT(ReadCount(&num_results, "num result types"));
+
+        result_types_.resize(num_results);
+        for (Index i = 0; i < num_results; ++i) {
+          Type result_type;
+          CHECK_RESULT(ReadType(&result_type, "select result type"));
+          ERROR_UNLESS(IsConcreteType(result_type),
+                       "expected valid select result type (got " PRItypecode
+                       ")",
+                       WABT_PRINTF_TYPE_CODE(result_type));
+          result_types_[i] = result_type;
         }
-        Type result_type;
-        CHECK_RESULT(ReadType(&result_type, "select result type"));
-        CALLBACK(OnSelectExpr, result_type);
+
+        Type* result_types = num_results ? result_types_.data() : nullptr;
+        CALLBACK(OnSelectExpr, num_results, result_types);
         CALLBACK0(OnOpcodeBare);
         break;
       }
 
       case Opcode::Select:
-        CALLBACK(OnSelectExpr, Type::Void);
+        CALLBACK(OnSelectExpr, 0, nullptr);
         CALLBACK0(OnOpcodeBare);
         break;
 

diff --git a/src/binary-reader.h b/src/binary-reader.h
@@ -276,7 +276,7 @@ class BinaryReaderDelegate {
   virtual Result OnReturnCallExpr(Index func_index) = 0;
   virtual Result OnReturnCallIndirectExpr(Index sig_index,
                                           Index table_index) = 0;
-  virtual Result OnSelectExpr(Type result_type) = 0;
+  virtual Result OnSelectExpr(Index result_count, Type* result_types) = 0;
   virtual Result OnStoreExpr(Opcode opcode,
                              Address alignment_log2,
                              Address offset) = 0;

diff --git a/src/interp/binary-reader-interp.cc b/src/interp/binary-reader-interp.cc
@@ -195,7 +195,7 @@ class BinaryReaderInterp : public BinaryReaderNop {
   Result OnRefIsNullExpr() override;
   Result OnNopExpr() override;
   Result OnReturnExpr() override;
-  Result OnSelectExpr(Type result_type) override;
+  Result OnSelectExpr(Index result_count, Type* result_types) override;
   Result OnStoreExpr(Opcode opcode,
                      Address alignment_log2,
                      Address offset) override;
@@ -1260,8 +1260,9 @@ Result BinaryReaderInterp::OnReturnExpr() {
   return Result::Ok;
 }
 
-Result BinaryReaderInterp::OnSelectExpr(Type result_type) {
-  CHECK_RESULT(validator_.OnSelect(loc, result_type));
+Result BinaryReaderInterp::OnSelectExpr(Index result_count,
+                                        Type* result_types) {
+  CHECK_RESULT(validator_.OnSelect(loc, result_count, result_types));
   istream_.Emit(Opcode::Select);
   return Result::Ok;
 }

diff --git a/src/shared-validator.cc b/src/shared-validator.cc
@@ -1036,10 +1036,18 @@ Result SharedValidator::OnReturn(const Location& loc) {
   return result;
 }
 
-Result SharedValidator::OnSelect(const Location& loc, Type result_type) {
+Result SharedValidator::OnSelect(const Location& loc,
+                                 Index result_count,
+                                 Type* result_types) {
   Result result = Result::Ok;
   expr_loc_ = &loc;
-  result |= typechecker_.OnSelect(result_type);
+  if (result_count > 1) {
+    result |=
+        PrintError(loc, "invalid arity in select instruction: %" PRIindex ".",
+                   result_count);
+  } else {
+    result |= typechecker_.OnSelect(ToTypeVector(result_count, result_types));
+  }
   return result;
 }
 

diff --git a/src/shared-validator.h b/src/shared-validator.h
@@ -156,7 +156,7 @@ class SharedValidator {
   Result OnReturnCall(const Location&, Var func_var);
   Result OnReturnCallIndirect(const Location&, Var sig_var, Var table_var);
   Result OnReturn(const Location&);
-  Result OnSelect(const Location&, Type result_type);
+  Result OnSelect(const Location&, Index result_count, Type* result_types);
   Result OnSimdLaneOp(const Location&, Opcode, uint64_t lane_idx);
   Result OnSimdShuffleOp(const Location&, Opcode, v128 lane_idx);
   Result OnStore(const Location&, Opcode, Address align);

diff --git a/src/type-checker.cc b/src/type-checker.cc
@@ -730,24 +730,25 @@ Result TypeChecker::OnReturn() {
   return result;
 }
 
-Result TypeChecker::OnSelect(Type expected) {
+Result TypeChecker::OnSelect(const TypeVector& expected) {
   Result result = Result::Ok;
   Type type1 = Type::Any;
   Type type2 = Type::Any;
   Type result_type = Type::Any;
   result |= PeekAndCheckType(0, Type::I32);
   result |= PeekType(1, &type1);
   result |= PeekType(2, &type2);
-  if (expected == Type::Void) {
+  if (expected.empty()) {
     if (type1.IsRef() || type2.IsRef()) {
       result = Result::Error;
     } else {
       result |= CheckType(type1, type2);
       result_type = type1;
     }
   } else {
-    result |= CheckType(type1, expected);
-    result |= CheckType(type2, expected);
+    assert(expected.size() == 1);
+    result |= CheckType(type1, expected[0]);
+    result |= CheckType(type2, expected[0]);
   }
   PrintStackIfFailed(result, "select", result_type, result_type, Type::I32);
   result |= DropTypes(3);

diff --git a/src/type-checker.h b/src/type-checker.h
@@ -113,7 +113,7 @@ class TypeChecker {
   Result OnRefIsNullExpr();
   Result OnRethrow();
   Result OnReturn();
-  Result OnSelect(Type expected);
+  Result OnSelect(const TypeVector& result_types);
   Result OnSimdLaneOp(Opcode, uint64_t);
   Result OnSimdShuffleOp(Opcode, v128);
   Result OnStore(Opcode, const Limits& limits);

diff --git a/src/validator.cc b/src/validator.cc
@@ -464,13 +464,8 @@ Result Validator::OnReturnCallIndirectExpr(ReturnCallIndirectExpr* expr) {
 }
 
 Result Validator::OnSelectExpr(SelectExpr* expr) {
-  Type result_type;
-  if (expr->result_type.empty()) {
-    result_type = Type::Void;
-  } else {
-    result_type = expr->result_type[0];
-  }
-  result_ |= validator_.OnSelect(expr->loc, result_type);
+  result_ |= validator_.OnSelect(expr->loc, expr->result_type.size(),
+                                 expr->result_type.data());
   // TODO: Existing behavior fails when select fails.
 #if 0
   return Result::Ok;

diff --git a/test/parse/expr/bad-select-multi.txt b/test/parse/expr/bad-select-multi.txt
@@ -0,0 +1,21 @@
+;;; TOOL: wat2wasm
+;;; ARGS*: --enable-reference-types
+;;; ERROR: 1
+(module
+  (func
+    i32.const 0
+    i32.const 0
+    i32.const 0
+    i32.const 0
+    i32.const 0
+    select (result i32 i32)
+    unreachable
+    ))
+(;; STDERR ;;;
+out/test/parse/expr/bad-select-multi.txt:11:5: error: invalid arity in select instruction: 2.
+    select (result i32 i32)
+    ^^^^^^
+out/test/parse/expr/bad-select-multi.txt:11:5: error: type mismatch in function, expected [] but got [... i32, i32, i32, i32]
+    select (result i32 i32)
+    ^^^^^^
+;;; STDERR ;;)
diff --git a/test/spec/reference-types/select.txt b/test/spec/reference-types/select.txt
@@ -15,7 +15,8 @@ out/test/spec/reference-types/select.wast:346: assert_invalid passed:
   error: type mismatch in select, expected [any, any, i32] but got [i32]
   000001c: error: OnSelectExpr callback failed
 out/test/spec/reference-types/select.wast:350: assert_invalid passed:
-  0000025: error: invalid arity in select instrcution: 2
+  error: invalid arity in select instruction: 2.
+  0000027: error: OnSelectExpr callback failed
 out/test/spec/reference-types/select.wast:362: assert_invalid passed:
   error: type mismatch in select, expected [any, any, i32] but got [externref, externref, i32]
   000001f: error: OnSelectExpr callback failed