[wasm] Making compare and conditionals more correct.

Comparisons were allowing asm 'int' values in places
that require strict 'signed' or 'unsigned' but not both.

Fixes crash when these make it to asm-wasm.

BUG=599413
BUG=v8:4203
R=aseemgarg@chromium.org

Review-Url: https://codereview.chromium.org/2106683003
Cr-Commit-Position: refs/heads/master@{#37353}

src/typing-asm.cc

@@ -540,20 +540,22 @@ void AsmTyper::VisitConditional(Conditional* expr) {
       expr->then_expression(), expected_type_,
       "conditional then branch type mismatch with enclosing expression"));
   Type* then_type = StorageType(computed_type_);
-  if (intish_ != 0 || !then_type->Is(cache_.kAsmComparable)) {
-    FAIL(expr->then_expression(), "invalid type in ? then expression");
-  }
+  int then_intish = intish_;
 
   RECURSE(VisitWithExpectation(
       expr->else_expression(), expected_type_,
       "conditional else branch type mismatch with enclosing expression"));
   Type* else_type = StorageType(computed_type_);
-  if (intish_ != 0 || !else_type->Is(cache_.kAsmComparable)) {
-    FAIL(expr->else_expression(), "invalid type in ? else expression");
-  }
+  int else_intish = intish_;
 
-  if (!then_type->Is(else_type) || !else_type->Is(then_type)) {
-    FAIL(expr, "then and else expressions in ? must have the same type");
+  if (then_intish != 0 || else_intish != 0 ||
+      !((then_type->Is(cache_.kAsmInt) && else_type->Is(cache_.kAsmInt)) ||
+        (then_type->Is(cache_.kAsmFloat) && else_type->Is(cache_.kAsmFloat)) ||
+        (then_type->Is(cache_.kAsmDouble) &&
+         else_type->Is(cache_.kAsmDouble)))) {
+    FAIL(expr,
+         "then and else expressions in ? must have the same type "
+         "and be int, float, or double");
   }
 
   RECURSE(IntersectResult(expr, then_type));
@@ -1379,20 +1381,25 @@ void AsmTyper::VisitCompareOperation(CompareOperation* expr) {
       VisitWithExpectation(expr->left(), Type::Number(),
                            "left comparison operand expected to be number"));
   Type* left_type = computed_type_;
-  if (!left_type->Is(cache_.kAsmComparable)) {
-    FAIL(expr->left(), "bad type on left side of comparison");
-  }
+  int left_intish = intish_;
 
   RECURSE(
       VisitWithExpectation(expr->right(), Type::Number(),
                            "right comparison operand expected to be number"));
   Type* right_type = computed_type_;
-  if (!right_type->Is(cache_.kAsmComparable)) {
-    FAIL(expr->right(), "bad type on right side of comparison");
-  }
-
-  if (!left_type->Is(right_type) && !right_type->Is(left_type)) {
-    FAIL(expr, "left and right side of comparison must match");
+  int right_intish = intish_;
+
+  if (left_intish != 0 || right_intish != 0 ||
+      !((left_type->Is(cache_.kAsmUnsigned) &&
+         right_type->Is(cache_.kAsmUnsigned)) ||
+        (left_type->Is(cache_.kAsmSigned) &&
+         right_type->Is(cache_.kAsmSigned)) ||
+        (left_type->Is(cache_.kAsmFloat) && right_type->Is(cache_.kAsmFloat)) ||
+        (left_type->Is(cache_.kAsmDouble) &&
+         right_type->Is(cache_.kAsmDouble)))) {
+    FAIL(expr,
+         "left and right side of comparison must match type "
+         "and be signed, unsigned, float, or double");
   }
 
   RECURSE(IntersectResult(expr, cache_.kAsmSigned));

src/typing-asm.h

@@ -121,7 +121,7 @@ class AsmTyper : public AstVisitor {
 
   AstTypeBounds bounds_;
 
-  static const int kErrorMessageLimit = 100;
+  static const int kErrorMessageLimit = 150;
   char error_message_[kErrorMessageLimit];
 
   static const int kMaxUncombinedAdditiveSteps = 1 << 20;

test/cctest/test-asm-validator.cc

@@ -1138,7 +1138,8 @@ TEST(TernaryMismatchInt32Float64) {
   CHECK_FUNC_ERROR(
       "function bar() { var x = 1; var y = 0.0; return (1 ? x : y)|0; }\n"
       "function foo() { bar(); }",
-      "asm: line 1: then and else expressions in ? must have the same type\n");
+      "asm: line 1: then and else expressions in ? must have the same type "
+      "and be int, float, or double\n");
 }
 
 
@@ -1146,7 +1147,8 @@ TEST(TernaryMismatchIntish) {
   CHECK_FUNC_ERROR(
       "function bar() { var x = 1; var y = 0; return (1 ? x + x : y)|0; }\n"
       "function foo() { bar(); }",
-      "asm: line 1: invalid type in ? then expression\n");
+      "asm: line 1: then and else expressions in ? must have the same type "
+      "and be int, float, or double\n");
 }
 
 
@@ -1154,7 +1156,8 @@ TEST(TernaryMismatchInt32Float32) {
   CHECK_FUNC_ERROR(
       "function bar() { var x = 1; var y = 2.0; return (x?fround(y):x)|0; }\n"
       "function foo() { bar(); }",
-      "asm: line 1: then and else expressions in ? must have the same type\n");
+      "asm: line 1: then and else expressions in ? must have the same type "
+      "and be int, float, or double\n");
 }
 
 
@@ -1359,7 +1362,8 @@ TEST(CompareToStringLeft) {
   CHECK_FUNC_ERROR(
       "function bar() { var x = 1; return ('hi' > x)|0; }\n"
       "function foo() { bar(); }",
-      "asm: line 1: bad type on left side of comparison\n");
+      "asm: line 1: left and right side of comparison must match type "
+      "and be signed, unsigned, float, or double\n");
 }
 
 
@@ -1367,7 +1371,8 @@ TEST(CompareToStringRight) {
   CHECK_FUNC_ERROR(
       "function bar() { var x = 1; return (x < 'hi')|0; }\n"
       "function foo() { bar(); }",
-      "asm: line 1: bad type on right side of comparison\n");
+      "asm: line 1: left and right side of comparison must match type "
+      "and be signed, unsigned, float, or double\n");
 }
 
 
@@ -1375,7 +1380,8 @@ TEST(CompareMismatchInt32Float64) {
   CHECK_FUNC_ERROR(
       "function bar() { var x = 1; var y = 2.0; return (x < y)|0; }\n"
       "function foo() { bar(); }",
-      "asm: line 1: left and right side of comparison must match\n");
+      "asm: line 1: left and right side of comparison must match type "
+      "and be signed, unsigned, float, or double\n");
 }
 
 
@@ -1383,7 +1389,8 @@ TEST(CompareMismatchInt32Uint32) {
   CHECK_FUNC_ERROR(
       "function bar() { var x = 1; var y = 2; return ((x|0) < (y>>>0))|0; }\n"
       "function foo() { bar(); }",
-      "asm: line 1: left and right side of comparison must match\n");
+      "asm: line 1: left and right side of comparison must match type "
+      "and be signed, unsigned, float, or double\n");
 }
 
 
@@ -1391,7 +1398,8 @@ TEST(CompareMismatchInt32Float32) {
   CHECK_FUNC_ERROR(
       "function bar() { var x = 1; var y = 2.0; return (x < fround(y))|0; }\n"
       "function foo() { bar(); }",
-      "asm: line 1: left and right side of comparison must match\n");
+      "asm: line 1: left and right side of comparison must match type "
+      "and be signed, unsigned, float, or double\n");
 }
 
 TEST(FunctionRepeated) {

test/mjsunit/regress/regress-wasm-crbug-599413.js

+// Copyright 2016 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --expose-wasm
+
+function __f_100() {
+  "use asm";
+  function __f_76() {
+    var __v_39 = 0;
+    outer: while (1) {
+      while (__v_39 == 4294967295) {
+      }
+    }
+  }
+  return {__f_76: __f_76};
+}
+assertThrows(function() {
+  Wasm.instantiateModuleFromAsm(__f_100.toString());
+});

test/mjsunit/wasm/asm-wasm.js

@@ -145,7 +145,7 @@ function TestWhileSimple() {
 
   function caller() {
     var x = 0;
-    while(x < 5) {
+    while((x|0) < 5) {
       x = (x + 1)|0;
     }
     return x|0;
@@ -162,7 +162,7 @@ function TestWhileWithoutBraces() {
 
   function caller() {
     var x = 0;
-    while(x <= 3)
+    while((x|0) <= 3)
       x = (x + 1)|0;
     return x|0;
   }
@@ -178,7 +178,7 @@ function TestReturnInWhile() {
 
   function caller() {
     var x = 0;
-    while(x < 10) {
+    while((x|0) < 10) {
       x = (x + 6)|0;
       return x|0;
     }
@@ -196,7 +196,7 @@ function TestReturnInWhileWithoutBraces() {
 
   function caller() {
     var x = 0;
-    while(x < 5)
+    while((x|0) < 5)
       return 7;
     return x|0;
   }
@@ -318,7 +318,7 @@ function TestBreakInBlock() {
     var x = 0;
     abc: {
       x = 10;
-      if (x == 10) {
+      if ((x|0) == 10) {
         break abc;
       }
       x = 20;
@@ -339,7 +339,7 @@ function TestBreakInNamedWhile() {
     var x = 0;
     outer: while (1) {
       x = (x + 1)|0;
-      while (x == 11) {
+      while ((x|0) == 11) {
         break outer;
       }
     }
@@ -358,9 +358,9 @@ function TestContinue() {
   function caller() {
     var x = 5;
     var ret = 0;
-    while (x >= 0) {
+    while ((x|0) >= 0) {
       x = (x - 1)|0;
-      if (x == 2) {
+      if ((x|0) == 2) {
         continue;
       }
       ret = (ret - 1)|0;
@@ -381,11 +381,11 @@ function TestContinueInNamedWhile() {
     var x = 5;
     var y = 0;
     var ret = 0;
-    outer: while (x > 0) {
+    outer: while ((x|0) > 0) {
       x = (x - 1)|0;
       y = 0;
-      while (y < 5) {
-        if (x == 3) {
+      while ((y|0) < 5) {
+        if ((x|0) == 3) {
           continue outer;
         }
         ret = (ret + 1)|0;
@@ -420,7 +420,7 @@ function TestNotEquals() {
 
   function caller() {
     var a = 3;
-    if (a != 2) {
+    if ((a|0) != 2) {
       return 21;
     }
     return 0;
@@ -458,7 +458,7 @@ function TestMixedAdd() {
     var c = 0;
     c = ((a>>>0) + b)|0;
     if ((c >>> 0) > (0>>>0)) {
-      if (c < 0) {
+      if ((c|0) < 0) {
         return 23;
       }
     }
@@ -733,7 +733,7 @@ function TestForLoop() {
   function caller() {
     var ret = 0;
     var i = 0;
-    for (i = 2; i <= 10; i = (i+1)|0) {
+    for (i = 2; (i|0) <= 10; i = (i+1)|0) {
       ret = (ret + i) | 0;
     }
     return ret|0;
@@ -751,7 +751,7 @@ function TestForLoopWithoutInit() {
   function caller() {
     var ret = 0;
     var i = 0;
-    for (; i < 10; i = (i+1)|0) {
+    for (; (i|0) < 10; i = (i+1)|0) {
       ret = (ret + 10) | 0;
     }
     return ret|0;
@@ -771,7 +771,7 @@ function TestForLoopWithoutCondition() {
     var i = 0;
     for (i=1;; i = (i+1)|0) {
       ret = (ret + i) | 0;
-      if (i == 11) {
+      if ((i|0) == 11) {
         break;
       }
     }
@@ -789,7 +789,7 @@ function TestForLoopWithoutNext() {
 
   function caller() {
     var i = 0;
-    for (i=1; i < 41;) {
+    for (i=1; (i|0) < 41;) {
       i = (i + 1) | 0;
     }
     return i|0;
@@ -806,7 +806,7 @@ function TestForLoopWithoutBody() {
 
   function caller() {
     var i = 0;
-    for (i=1; i < 45 ; i = (i+1)|0) {
+    for (i=1; (i|0) < 45 ; i = (i+1)|0) {
     }
     return i|0;
   }
@@ -826,7 +826,7 @@ function TestDoWhile() {
     do {
       ret = (ret + ret)|0;
       i = (i + 1)|0;
-    } while (i < 2);
+    } while ((i|0) < 2);
     return ret|0;
   }
 
@@ -841,7 +841,7 @@ function TestConditional() {
 
   function caller() {
     var x = 1;
-    return ((x > 0) ? 41 : 71)|0;
+    return (((x|0) > 0) ? 41 : 71)|0;
   }
 
   return {caller:caller};
@@ -911,8 +911,8 @@ function TestFunctionTableMultipleFunctions() {
   }
 
   function caller() {
-    if (function_table[0&1](50) == 51) {
-      if (function_table[1&1](60) == 62) {
+    if ((function_table[0&1](50)|0) == 51) {
+      if ((function_table[1&1](60)|0) == 62) {
         return 73;
       }
     }
@@ -953,9 +953,9 @@ function TestFunctionTable() {
     fun_id = fun_id|0;
     arg1 = arg1|0;
     arg2 = arg2|0;
-    if (table_id == 0) {
+    if ((table_id|0) == 0) {
       return funBin[fun_id&3](arg1, arg2)|0;
-    } else if (table_id == 1) {
+    } else if ((table_id|0) == 1) {
       return fun[fun_id&0](arg1)|0;
     }
     return 0;