[asm.js] Fix numeric literal bounds checking.

This fixes the bounds checking of "unsigned" numeric literals (those
that do not contains dots) by the parser. In particular this fixes a
bogus truncation to 32-bit in the scanner. It also makes the scanner
more robust by limiting the range of those numeric literals, hence
completely avoiding rounding loss or truncation errors.

R=clemensh@chromium.org
TEST=unittests/AsmJsScannerTest.UnsignedNumbers
BUG=v8:6298

Change-Id: Id31ab3c652e99fa8d3d6663315768e1bfaf3b773
Reviewed-on: https://chromium-review.googlesource.com/486881Reviewed-by: Clemens Hammacher <clemensh@chromium.org>
Commit-Queue: Michael Starzinger <mstarzinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#44890}

src/asmjs/asm-parser.cc

@@ -437,7 +437,7 @@ void AsmJsParser::ValidateModuleVar(bool mutable_variable) {
   }
   EXPECT_TOKEN('=');
   double dvalue = 0.0;
-  uint64_t uvalue = 0;
+  uint32_t uvalue = 0;
   if (CheckForDouble(&dvalue)) {
     DeclareGlobal(info, mutable_variable, AsmType::Double(), kWasmF64,
                   WasmInitExpr(dvalue));
@@ -504,7 +504,7 @@ void AsmJsParser::ValidateModuleVarFromGlobal(VarInfo* info,
     negate = true;
   }
   double dvalue = 0.0;
-  uint64_t uvalue = 0;
+  uint32_t uvalue = 0;
   if (CheckForDouble(&dvalue)) {
     if (negate) {
       dvalue = -dvalue;
@@ -871,7 +871,7 @@ void AsmJsParser::ValidateFunctionLocals(
       // Store types.
       EXPECT_TOKEN('=');
       double dvalue = 0.0;
-      uint64_t uvalue = 0;
+      uint32_t uvalue = 0;
       if (Check('-')) {
         if (CheckForDouble(&dvalue)) {
           info->kind = VarKind::kLocal;
@@ -1298,7 +1298,7 @@ void AsmJsParser::ValidateCase() {
   if (Check('-')) {
     negate = true;
   }
-  uint64_t uvalue;
+  uint32_t uvalue;
   if (!CheckForUnsigned(&uvalue)) {
     FAIL("Expected numeric literal");
   }
@@ -1359,7 +1359,7 @@ AsmType* AsmJsParser::Expression(AsmType* expected) {
 AsmType* AsmJsParser::NumericLiteral() {
   call_coercion_ = nullptr;
   double dvalue = 0.0;
-  uint64_t uvalue = 0;
+  uint32_t uvalue = 0;
   if (CheckForDouble(&dvalue)) {
     current_function_builder_->EmitF64Const(dvalue);
     return AsmType::Double();
@@ -1519,7 +1519,7 @@ AsmType* AsmJsParser::AssignmentExpression() {
 AsmType* AsmJsParser::UnaryExpression() {
   AsmType* ret;
   if (Check('-')) {
-    uint64_t uvalue;
+    uint32_t uvalue;
     if (CheckForUnsigned(&uvalue)) {
       // TODO(bradnelson): was supposed to be 0x7fffffff, check errata.
       if (uvalue <= 0x80000000) {
@@ -1601,7 +1601,7 @@ AsmType* AsmJsParser::UnaryExpression() {
 
 // 6.8.8 MultaplicativeExpression
 AsmType* AsmJsParser::MultiplicativeExpression() {
-  uint64_t uvalue;
+  uint32_t uvalue;
   if (CheckForUnsignedBelow(0x100000, &uvalue)) {
     if (Check('*')) {
       AsmType* a;
@@ -1634,7 +1634,7 @@ AsmType* AsmJsParser::MultiplicativeExpression() {
   RECURSEn(a = UnaryExpression());
   for (;;) {
     if (Check('*')) {
-      uint64_t uvalue;
+      uint32_t uvalue;
       if (Check('-')) {
         if (CheckForUnsigned(&uvalue)) {
           if (uvalue >= 0x100000) {
@@ -2014,10 +2014,12 @@ AsmType* AsmJsParser::ValidateCall() {
   if (Check('[')) {
     RECURSEn(EqualityExpression());
     EXPECT_TOKENn('&');
-    uint64_t mask = 0;
+    uint32_t mask = 0;
     if (!CheckForUnsigned(&mask)) {
       FAILn("Expected mask literal");
     }
+    // TODO(mstarzinger): Clarify and explain where this limit is coming from,
+    // as it is not mandated by the spec directly.
     if (mask > 0x7fffffff) {
       FAILn("Expected power of 2 mask");
     }
@@ -2326,10 +2328,14 @@ void AsmJsParser::ValidateHeapAccess() {
   VarInfo* info = GetVarInfo(Consume());
   int32_t size = info->type->ElementSizeInBytes();
   EXPECT_TOKEN('[');
-  uint64_t offset;
+  uint32_t offset;
   if (CheckForUnsigned(&offset)) {
     // TODO(bradnelson): Check more things.
-    if (offset > 0x7fffffff || offset * size > 0x7fffffff) {
+    // TODO(mstarzinger): Clarify and explain where this limit is coming from,
+    // as it is not mandated by the spec directly.
+    if (offset > 0x7fffffff ||
+        static_cast<uint64_t>(offset) * static_cast<uint64_t>(size) >
+            0x7fffffff) {
       FAIL("Heap access out of range");
     }
     if (Check(']')) {
@@ -2349,7 +2355,7 @@ void AsmJsParser::ValidateHeapAccess() {
   } else {
     RECURSE(index_type = AdditiveExpression());
     EXPECT_TOKEN(TOK(SAR));
-    uint64_t shift;
+    uint32_t shift;
     if (!CheckForUnsigned(&shift)) {
       FAIL("Expected shift of word size");
     }
@@ -2430,7 +2436,7 @@ void AsmJsParser::GatherCases(std::vector<int32_t>* cases) {
     } else if (depth == 1 && Peek(TOK(case))) {
       scanner_.Next();
       int32_t value;
-      uint64_t uvalue;
+      uint32_t uvalue;
       if (Check('-')) {
         if (!CheckForUnsigned(&uvalue)) {
           break;

src/asmjs/asm-parser.h

@@ -192,7 +192,7 @@ class AsmJsParser {
     }
   }
 
-  inline bool CheckForUnsigned(uint64_t* value) {
+  inline bool CheckForUnsigned(uint32_t* value) {
     if (scanner_.IsUnsigned()) {
       *value = scanner_.AsUnsigned();
       scanner_.Next();
@@ -202,7 +202,7 @@ class AsmJsParser {
     }
   }
 
-  inline bool CheckForUnsignedBelow(uint64_t limit, uint64_t* value) {
+  inline bool CheckForUnsignedBelow(uint32_t limit, uint32_t* value) {
     if (scanner_.IsUnsigned() && scanner_.AsUnsigned() < limit) {
       *value = scanner_.AsUnsigned();
       scanner_.Next();

src/asmjs/asm-scanner.cc

@@ -72,7 +72,7 @@ void AsmJsScanner::Next() {
     if (Token() == kDouble) {
       PrintF("%lf ", AsDouble());
     } else if (Token() == kUnsigned) {
-      PrintF("%" PRIu64 " ", AsUnsigned());
+      PrintF("%" PRIu32 " ", AsUnsigned());
     } else {
       std::string name = Name(Token());
       PrintF("%s ", name.c_str());
@@ -308,9 +308,8 @@ void AsmJsScanner::ConsumeNumber(uc32 ch) {
   UnicodeCache cache;
   double_value_ = StringToDouble(
       &cache,
-      Vector<uint8_t>(
-          const_cast<uint8_t*>(reinterpret_cast<const uint8_t*>(number.data())),
-          static_cast<int>(number.size())),
+      Vector<const uint8_t>(reinterpret_cast<const uint8_t*>(number.data()),
+                            static_cast<int>(number.size())),
       ALLOW_HEX | ALLOW_OCTAL | ALLOW_BINARY | ALLOW_IMPLICIT_OCTAL);
   if (std::isnan(double_value_)) {
     // Check if string to number conversion didn't consume all the characters.
@@ -332,6 +331,11 @@ void AsmJsScanner::ConsumeNumber(uc32 ch) {
   if (has_dot) {
     token_ = kDouble;
   } else {
+    // Exceeding safe integer range is an error.
+    if (double_value_ > static_cast<double>(kMaxUInt32)) {
+      token_ = kParseError;
+      return;
+    }
     unsigned_value_ = static_cast<uint32_t>(double_value_);
     token_ = kUnsigned;
   }

src/asmjs/asm-scanner.h

@@ -92,12 +92,19 @@ class V8_EXPORT_PRIVATE AsmJsScanner {
     return token - kGlobalsStart;
   }
 
-  // Methods to check if the current token is an asm.js "number" (contains a
-  // dot) or an "unsigned" (a number without a dot).
+  // Methods to check if the current token is a numeric literal considered an
+  // asm.js "double" (contains a dot) or an "unsigned" (without a dot). Note
+  // that numbers without a dot outside the [0 .. 2^32) range are errors.
   bool IsUnsigned() const { return Token() == kUnsigned; }
-  uint64_t AsUnsigned() const { return unsigned_value_; }
+  uint32_t AsUnsigned() const {
+    DCHECK(IsUnsigned());
+    return unsigned_value_;
+  }
   bool IsDouble() const { return Token() == kDouble; }
-  double AsDouble() const { return double_value_; }
+  double AsDouble() const {
+    DCHECK(IsDouble());
+    return double_value_;
+  }
 
   // clang-format off
   enum {
@@ -146,7 +153,7 @@ class V8_EXPORT_PRIVATE AsmJsScanner {
   std::unordered_map<std::string, token_t> property_names_;
   int global_count_;
   double double_value_;
-  uint64_t unsigned_value_;
+  uint32_t unsigned_value_;
   bool preceded_by_newline_;
 
   // Consume multiple characters.

test/mjsunit/regress/regress-6298.js

+// Copyright 2017 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: --allow-natives-syntax
+
+function Module(stdlib, imports, buffer) {
+  "use asm";
+  function f() {
+    return (281474976710655 * 1048575) | 0;
+  }
+  return { f:f };
+}
+var m = Module(this);
+assertEquals(-1048576, m.f());
+assertFalse(%IsAsmWasmCode(Module));

test/unittests/asmjs/asm-scanner-unittest.cc

@@ -209,6 +209,26 @@ TEST_F(AsmJsScannerTest, Numbers) {
   CheckForEnd();
 }
 
+TEST_F(AsmJsScannerTest, UnsignedNumbers) {
+  SetupSource("0x7fffffff 0x80000000 0xffffffff 0x100000000");
+
+  CHECK(scanner.IsUnsigned());
+  CHECK_EQ(0x7fffffff, scanner.AsUnsigned());
+  scanner.Next();
+
+  CHECK(scanner.IsUnsigned());
+  CHECK_EQ(0x80000000, scanner.AsUnsigned());
+  scanner.Next();
+
+  CHECK(scanner.IsUnsigned());
+  CHECK_EQ(0xffffffff, scanner.AsUnsigned());
+  scanner.Next();
+
+  // Numeric "unsigned" literals with a payload of more than 32-bit are rejected
+  // by asm.js in all contexts, we hence consider `0x100000000` to be an error.
+  CheckForParseError();
+}
+
 TEST_F(AsmJsScannerTest, BadNumber) {
   SetupSource(".123fe");
   Skip('.');