[asm.js] Fix heap access validation of shift expressions.

This makes sure that shift expressions (not wrapped in parentheses) can
appear as part of the index in a valid heap access expression. Only the
last operand of a sequence of shift expressions is taken into account
when validating the heap access.

R=jarin@chromium.org
TEST=mjsunit/regress/regress-6700
BUG=v8:6700,chromium:754751

Change-Id: Icc7a71bd64461da4d3daea41b995964e3dfc6dc6
Reviewed-on: https://chromium-review.googlesource.com/623811
Commit-Queue: Michael Starzinger <mstarzinger@chromium.org>
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Reviewed-by: Andreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47497}

src/asmjs/asm-parser.cc

@@ -1781,13 +1781,44 @@ AsmType* AsmJsParser::AdditiveExpression() {
 AsmType* AsmJsParser::ShiftExpression() {
   AsmType* a = nullptr;
   RECURSEn(a = AdditiveExpression());
+  heap_access_shift_position_ = kNoHeapAccessShift;
+  // TODO(bradnelson): Implement backtracking to avoid emitting code
+  // for the x >>> 0 case (similar to what's there for |0).
   for (;;) {
     switch (scanner_.Token()) {
-// TODO(bradnelson): Implement backtracking to avoid emitting code
-// for the x >>> 0 case (similar to what's there for |0).
+      case TOK(SAR): {
+        EXPECT_TOKENn(TOK(SAR));
+        heap_access_shift_position_ = kNoHeapAccessShift;
+        // Remember position allowing this shift-expression to be used as part
+        // of a heap access operation expecting `a >> n:NumericLiteral`.
+        bool imm = false;
+        size_t old_pos;
+        size_t old_code;
+        uint32_t shift_imm;
+        if (a->IsA(AsmType::Intish()) && CheckForUnsigned(&shift_imm)) {
+          old_pos = scanner_.Position();
+          old_code = current_function_builder_->GetPosition();
+          scanner_.Rewind();
+          imm = true;
+        }
+        AsmType* b = nullptr;
+        RECURSEn(b = AdditiveExpression());
+        // Check for `a >> n:NumericLiteral` pattern.
+        if (imm && old_pos == scanner_.Position()) {
+          heap_access_shift_position_ = old_code;
+          heap_access_shift_value_ = shift_imm;
+        }
+        if (!(a->IsA(AsmType::Intish()) && b->IsA(AsmType::Intish()))) {
+          FAILn("Expected intish for operator >>.");
+        }
+        current_function_builder_->Emit(kExprI32ShrS);
+        a = AsmType::Signed();
+        continue;
+      }
 #define HANDLE_CASE(op, opcode, name, result)                        \
   case TOK(op): {                                                    \
     EXPECT_TOKENn(TOK(op));                                          \
+    heap_access_shift_position_ = kNoHeapAccessShift;                \
     AsmType* b = nullptr;                                            \
     RECURSEn(b = AdditiveExpression());                              \
     if (!(a->IsA(AsmType::Intish()) && b->IsA(AsmType::Intish()))) { \
@@ -1797,9 +1828,8 @@ AsmType* AsmJsParser::ShiftExpression() {
     a = AsmType::result();                                           \
     continue;                                                        \
   }
-      HANDLE_CASE(SHL, I32Shl, "<<", Signed);
-      HANDLE_CASE(SAR, I32ShrS, ">>", Signed);
-      HANDLE_CASE(SHR, I32ShrU, ">>>", Unsigned);
+        HANDLE_CASE(SHL, I32Shl, "<<", Signed);
+        HANDLE_CASE(SHR, I32ShrU, ">>>", Unsigned);
 #undef HANDLE_CASE
       default:
         return a;
@@ -2353,18 +2383,18 @@ void AsmJsParser::ValidateHeapAccess() {
       info->type->IsA(AsmType::Uint8Array())) {
     RECURSE(index_type = Expression(nullptr));
   } else {
-    RECURSE(index_type = AdditiveExpression());
-    EXPECT_TOKEN(TOK(SAR));
-    uint32_t shift;
-    if (!CheckForUnsigned(&shift)) {
+    RECURSE(index_type = ShiftExpression());
+    if (heap_access_shift_position_ == kNoHeapAccessShift) {
       FAIL("Expected shift of word size");
     }
-    if (shift > 3) {
+    if (heap_access_shift_value_ > 3) {
       FAIL("Expected valid heap access shift");
     }
-    if ((1 << shift) != size) {
+    if ((1 << heap_access_shift_value_) != size) {
       FAIL("Expected heap access shift to match heap view");
     }
+    // Delete the code of the actual shift operation.
+    current_function_builder_->DeleteCodeAfter(heap_access_shift_position_);
     // Mask bottom bits to match asm.js behavior.
     current_function_builder_->EmitI32Const(~(size - 1));
     current_function_builder_->Emit(kExprI32And);

src/asmjs/asm-parser.h

@@ -208,6 +208,14 @@ class AsmJsParser {
   // aforementioned {call_coercion_deferred} is allowed.
   size_t call_coercion_deferred_position_;
 
+  // The code position of the last heap access shift by an immediate value.
+  // For `heap[expr >> value:NumericLiteral]` this indicates from where to
+  // delete code when the expression is used as part of a valid heap access.
+  // Will be set to {kNoHeapAccessShift} if heap access shift wasn't matched.
+  size_t heap_access_shift_position_;
+  uint32_t heap_access_shift_value_;
+  static const size_t kNoHeapAccessShift = -1;
+
   // Used to track the last label we've seen so it can be matched to later
   // statements it's attached to.
   AsmJsScanner::token_t pending_label_;

test/mjsunit/regress/regress-6700.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 TestLeftRight() {
+  function Module(stdlib, foreign, heap) {
+    "use asm";
+    var HEAP32 = new stdlib.Int32Array(heap);
+    function f(i) {
+      i = i | 0;
+      return HEAP32[i << 2 >> 2] | 0;
+    }
+    return { f:f }
+  }
+  var buffer = new ArrayBuffer(1024);
+  var module = new Module(this, {}, buffer);
+  assertTrue(%IsAsmWasmCode(Module));
+  new Int32Array(buffer)[42] = 23;
+  assertEquals(23, module.f(42));
+})();
+
+(function TestRightRight() {
+  function Module(stdlib, foreign, heap) {
+    "use asm";
+    var HEAP32 = new stdlib.Int32Array(heap);
+    function f(i) {
+      i = i | 0;
+      return HEAP32[i >> 2 >> 2] | 0;
+    }
+    return { f:f }
+  }
+  var buffer = new ArrayBuffer(1024);
+  var module = new Module(this, {}, buffer)
+  assertTrue(%IsAsmWasmCode(Module));
+  new Int32Array(buffer)[42 >> 4] = 23;
+  assertEquals(23, module.f(42));
+})();
+
+(function TestRightLeft() {
+  function Module(stdlib, foreign, heap) {
+    "use asm";
+    var HEAP32 = new stdlib.Int32Array(heap);
+    function f(i) {
+      i = i | 0;
+      return HEAP32[i >> 2 << 2] | 0;
+    }
+    return { f:f }
+  }
+  var buffer = new ArrayBuffer(1024);
+  var module = new Module(this, {}, buffer)
+  assertFalse(%IsAsmWasmCode(Module));
+  new Int32Array(buffer)[42 & 0xfc] = 23;
+  assertEquals(23, module.f(42));
+})();
+
+(function TestRightButNotImmediate() {
+  function Module(stdlib, foreign, heap) {
+    "use asm";
+    var HEAP32 = new stdlib.Int32Array(heap);
+    function f(i) {
+      i = i | 0;
+      return HEAP32[i >> 2 + 1] | 0;
+    }
+    return { f:f }
+  }
+  var buffer = new ArrayBuffer(1024);
+  var module = new Module(this, {}, buffer)
+  assertFalse(%IsAsmWasmCode(Module));
+  new Int32Array(buffer)[42 >> 3] = 23;
+  assertEquals(23, module.f(42));
+})();
+
+(function TestLeftOnly() {
+  function Module(stdlib, foreign, heap) {
+    "use asm";
+    var HEAP32 = new stdlib.Int32Array(heap);
+    function f(i) {
+      i = i | 0;
+      return HEAP32[i << 2] | 0;
+    }
+    return { f:f }
+  }
+  var buffer = new ArrayBuffer(1024);
+  var module = new Module(this, {}, buffer)
+  assertFalse(%IsAsmWasmCode(Module));
+  new Int32Array(buffer)[42 << 2] = 23;
+  assertEquals(23, module.f(42));
+})();