Allow looser heap accesses historically emitted by Emscripten.

Older versions of Emscripten appear to emit Asm.js containing:
HEAP8[x] with x in int
As opposed to the spec legal construct:
HEAP8[x>>0] with x in int

As older programs and even benchmarks such as Embenchen
include these constructs, support them for compatibility.

BUG= https://code.google.com/p/v8/issues/detail?id=4203
TEST=test-asm-validator,mjsunit/asm-wasm
R=aseemgarg@chromium.org,titzer@chromium.org
LOG=N

Review URL: https://codereview.chromium.org/1692713006

Cr-Commit-Position: refs/heads/master@{#33964}

src/typing-asm.cc

@@ -765,24 +765,28 @@ void AsmTyper::VisitHeapAccess(Property* expr, bool assigning,
       RECURSE(VisitWithExpectation(literal, cache_.kAsmSigned,
                                    "array index expected to be integer"));
     } else {
-      BinaryOperation* bin = expr->key()->AsBinaryOperation();
-      if (bin == NULL || bin->op() != Token::SAR) {
-        FAIL(expr->key(), "expected >> in heap access");
-      }
-      RECURSE(VisitWithExpectation(bin->left(), cache_.kAsmSigned,
-                                   "array index expected to be integer"));
-      Literal* right = bin->right()->AsLiteral();
-      if (right == NULL || right->raw_value()->ContainsDot()) {
-        FAIL(right, "heap access shift must be integer");
-      }
-      RECURSE(VisitWithExpectation(bin->right(), cache_.kAsmSigned,
-                                   "array shift expected to be integer"));
-      int n = static_cast<int>(right->raw_value()->AsNumber());
       int expected_shift = ElementShiftSize(type);
-      if (expected_shift < 0 || n != expected_shift) {
-        FAIL(right, "heap access shift must match element size");
+      if (expected_shift == 0) {
+        RECURSE(Visit(expr->key()));
+      } else {
+        BinaryOperation* bin = expr->key()->AsBinaryOperation();
+        if (bin == NULL || bin->op() != Token::SAR) {
+          FAIL(expr->key(), "expected >> in heap access");
+        }
+        RECURSE(VisitWithExpectation(bin->left(), cache_.kAsmSigned,
+                                     "array index expected to be integer"));
+        Literal* right = bin->right()->AsLiteral();
+        if (right == NULL || right->raw_value()->ContainsDot()) {
+          FAIL(right, "heap access shift must be integer");
+        }
+        RECURSE(VisitWithExpectation(bin->right(), cache_.kAsmSigned,
+                                     "array shift expected to be integer"));
+        int n = static_cast<int>(right->raw_value()->AsNumber());
+        if (expected_shift < 0 || n != expected_shift) {
+          FAIL(right, "heap access shift must match element size");
+        }
       }
-      bin->set_bounds(Bounds(cache_.kAsmSigned));
+      expr->key()->set_bounds(Bounds(cache_.kAsmSigned));
     }
     Type* result_type;
     if (type->Is(cache_.kAsmIntArrayElement)) {
@@ -900,7 +904,8 @@ void AsmTyper::VisitProperty(Property* expr) {
 
   // Only recurse at this point so that we avoid needing
   // stdlib.Math to have a real type.
-  RECURSE(VisitWithExpectation(expr->obj(), Type::Any(), "bad propety object"));
+  RECURSE(
+      VisitWithExpectation(expr->obj(), Type::Any(), "bad property object"));
 
   // For heap view or function table access.
   if (computed_type_->IsArray()) {

src/wasm/asm-wasm-builder.cc

@@ -725,29 +725,38 @@ class AsmWasmBuilderImpl : public AstVisitor {
         WasmOpcodes::LoadStoreOpcodeOf(mtype, is_set_op_),
         WasmOpcodes::LoadStoreAccessOf(false));
     is_set_op_ = false;
-    Literal* value = expr->key()->AsLiteral();
-    if (value) {
-      DCHECK(value->raw_value()->IsNumber());
-      DCHECK(kAstI32 == TypeOf(value));
-      int val = static_cast<int>(value->raw_value()->AsNumber());
-      byte code[] = {WASM_I32(val * size)};
-      current_function_builder_->EmitCode(code, sizeof(code));
-      return;
-    }
-    BinaryOperation* binop = expr->key()->AsBinaryOperation();
-    if (binop) {
-      DCHECK(Token::SAR == binop->op());
-      DCHECK(binop->right()->AsLiteral()->raw_value()->IsNumber());
-      DCHECK(kAstI32 == TypeOf(binop->right()->AsLiteral()));
-      DCHECK(size ==
-             1 << static_cast<int>(
-                 binop->right()->AsLiteral()->raw_value()->AsNumber()));
-      // Mask bottom bits to match asm.js behavior.
-      current_function_builder_->Emit(kExprI32And);
-      byte code[] = {WASM_I8(~(size - 1))};
-      current_function_builder_->EmitCode(code, sizeof(code));
-      RECURSE(Visit(binop->left()));
+    if (size == 1) {
+      // Allow more general expression in byte arrays than the spec
+      // strictly permits.
+      // Early versions of Emscripten emit HEAP8[HEAP32[..]|0] in
+      // places that strictly should be HEAP8[HEAP32[..]>>0].
+      RECURSE(Visit(expr->key()));
       return;
+    } else {
+      Literal* value = expr->key()->AsLiteral();
+      if (value) {
+        DCHECK(value->raw_value()->IsNumber());
+        DCHECK(kAstI32 == TypeOf(value));
+        int val = static_cast<int>(value->raw_value()->AsNumber());
+        byte code[] = {WASM_I32(val * size)};
+        current_function_builder_->EmitCode(code, sizeof(code));
+        return;
+      }
+      BinaryOperation* binop = expr->key()->AsBinaryOperation();
+      if (binop) {
+        DCHECK(Token::SAR == binop->op());
+        DCHECK(binop->right()->AsLiteral()->raw_value()->IsNumber());
+        DCHECK(kAstI32 == TypeOf(binop->right()->AsLiteral()));
+        DCHECK(size ==
+               1 << static_cast<int>(
+                   binop->right()->AsLiteral()->raw_value()->AsNumber()));
+        // Mask bottom bits to match asm.js behavior.
+        current_function_builder_->Emit(kExprI32And);
+        byte code[] = {WASM_I8(~(size - 1))};
+        current_function_builder_->EmitCode(code, sizeof(code));
+        RECURSE(Visit(binop->left()));
+        return;
+      }
     }
     UNREACHABLE();
   }

test/cctest/test-asm-validator.cc

@@ -1326,7 +1326,7 @@ TEST(Load1) {
           CHECK_EXPR(Property, Bounds(cache.kAsmInt)) {
             CHECK_VAR(i8, Bounds(cache.kInt8Array));
             CHECK_EXPR(BinaryOperation, Bounds(cache.kAsmSigned)) {
-              CHECK_VAR(x, Bounds(cache.kAsmSigned));
+              CHECK_VAR(x, Bounds(cache.kAsmInt));
               CHECK_EXPR(Literal, Bounds(cache.kAsmFixnum));
             }
           }
@@ -1386,7 +1386,7 @@ TEST(Store1) {
         CHECK_EXPR(Property, Bounds::Unbounded()) {
           CHECK_VAR(i8, Bounds(cache.kInt8Array));
           CHECK_EXPR(BinaryOperation, Bounds(cache.kAsmSigned)) {
-            CHECK_VAR(x, Bounds(cache.kAsmSigned));
+            CHECK_VAR(x, Bounds(cache.kAsmInt));
             CHECK_EXPR(Literal, Bounds(cache.kAsmFixnum));
           }
         }
@@ -1750,6 +1750,28 @@ TEST(ForeignFunction) {
   CHECK_FUNC_TYPES_END_2()
 }
 
+TEST(ByteArray) {
+  // Forbidden by asm.js spec, present in embenchen.
+  CHECK_FUNC_TYPES_BEGIN(
+      "function bar() { var x = 0; i8[x] = 2; }\n"
+      "function foo() { bar(); }") {
+    CHECK_EXPR(FunctionLiteral, FUNC_V_TYPE) {
+      CHECK_EXPR(Assignment, Bounds(cache.kAsmInt)) {
+        CHECK_VAR(x, Bounds(cache.kAsmInt));
+        CHECK_EXPR(Literal, Bounds(cache.kAsmFixnum));
+      }
+      CHECK_EXPR(Assignment, Bounds(cache.kAsmInt)) {
+        CHECK_EXPR(Property, Bounds::Unbounded()) {
+          CHECK_VAR(i8, Bounds(cache.kInt8Array));
+          CHECK_VAR(x, Bounds(cache.kAsmSigned));
+        }
+        CHECK_EXPR(Literal, Bounds(cache.kAsmFixnum));
+      }
+    }
+    CHECK_SKIP();
+  }
+  CHECK_FUNC_TYPES_END
+}
 
 TEST(BadExports) {
   HARNESS_PREAMBLE()
@@ -1768,7 +1790,14 @@ TEST(BadExports) {
 
 TEST(NestedHeapAssignment) {
   CHECK_FUNC_ERROR(
-      "function bar() { var x = 0; i8[x = 1] = 2; }\n"
+      "function bar() { var x = 0; i16[x = 1] = 2; }\n"
+      "function foo() { bar(); }",
+      "asm: line 39: expected >> in heap access\n");
+}
+
+TEST(BadOperatorHeapAssignment) {
+  CHECK_FUNC_ERROR(
+      "function bar() { var x = 0; i16[x & 1] = 2; }\n"
       "function foo() { bar(); }",
       "asm: line 39: expected >> in heap access\n");
 }

test/mjsunit/wasm/asm-wasm.js

@@ -1170,3 +1170,52 @@ function TestForeignVariables() {
 }
 
 TestForeignVariables();
+
+
+(function() {
+  function TestByteHeapAccessCompat(stdlib, foreign, buffer) {
+    "use asm";
+
+    var HEAP8 = new stdlib.Uint8Array(buffer);
+    var HEAP32 = new stdlib.Int32Array(buffer);
+
+    function store(i, v) {
+      i = i | 0;
+      v = v | 0;
+      HEAP32[i >> 2] = v;
+    }
+
+    function storeb(i, v) {
+      i = i | 0;
+      v = v | 0;
+      HEAP8[i | 0] = v;
+    }
+
+    function load(i) {
+      i = i | 0;
+      return HEAP8[i] | 0;
+    }
+
+    function iload(i) {
+      i = i | 0;
+      return HEAP8[HEAP32[i >> 2] | 0] | 0;
+    }
+
+    return {load: load, iload: iload, store: store, storeb: storeb};
+  }
+
+  var m = _WASMEXP_.instantiateModuleFromAsm(
+      TestByteHeapAccessCompat.toString());
+  m.store(0, 20);
+  m.store(4, 21);
+  m.store(8, 22);
+  m.storeb(20, 123);
+  m.storeb(21, 42);
+  m.storeb(22, 77);
+  assertEquals(123, m.load(20));
+  assertEquals(42, m.load(21));
+  assertEquals(77, m.load(22));
+  assertEquals(123, m.iload(0));
+  assertEquals(42, m.iload(4));
+  assertEquals(77, m.iload(8));
+})();