[wasm] Change the memory access offset to pointer size

TurboFan expects the offset input of a Load or Store node to be a
pointer-size input, i.e. an int32 input on 32-bit platforms, and int64
on 64-bit platforms. In WebAssembly we always provided 32-bit offset
though, which caused problems when the high word of the register which
contained the offset was not empty.

With this CL we change the offset input to int64 on 64-bit platforms.
In addition we also change the type of the memory_size_ node to int64,
so that that we do not have to adjust the type of the memory size at
every memory load.

This CL will cause performance regressions but is necessary for
correctness and to avoid crashes.

R=titzer@chromium.org

Bug: chromium:766666
Change-Id: I5301e108d05e125258d2a06d500c1b75e91697b8
Reviewed-on: https://chromium-review.googlesource.com/723379Reviewed-by: Ben Titzer <titzer@chromium.org>
Commit-Queue: Andreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48689}

src/compiler/wasm-compiler.cc

@@ -3215,6 +3215,10 @@ Node* WasmGraphBuilder::LoadMemSize() {
           static_cast<int32_t>(offsetof(WasmContext, mem_size))),
       *effect_, *control_);
   *effect_ = mem_size;
+  if (jsgraph()->machine()->Is64()) {
+    mem_size = graph()->NewNode(jsgraph()->machine()->ChangeUint32ToUint64(),
+                                mem_size);
+  }
   return mem_size;
 }
 
@@ -3262,8 +3266,13 @@ Node* WasmGraphBuilder::CurrentMemoryPages() {
   // CurrentMemoryPages can not be called from asm.js.
   DCHECK_EQ(wasm::kWasmOrigin, env_->module->origin());
   DCHECK_NOT_NULL(*mem_size_);
+  Node* mem_size = *mem_size_;
+  if (jsgraph()->machine()->Is64()) {
+    mem_size = graph()->NewNode(jsgraph()->machine()->TruncateInt64ToInt32(),
+                                mem_size);
+  }
   return graph()->NewNode(
-      jsgraph()->machine()->Word32Shr(), *mem_size_,
+      jsgraph()->machine()->Word32Shr(), mem_size,
       jsgraph()->Int32Constant(WhichPowerOf2(wasm::WasmModule::kPageSize)));
 }
 
@@ -3405,15 +3414,21 @@ void WasmGraphBuilder::BoundsCheckMem(MachineType memtype, Node* index,
     // The end offset is larger than the smallest memory.
     // Dynamically check the end offset against the actual memory size, which
     // is not known at compile time.
-    Node* cond = graph()->NewNode(
-        jsgraph()->machine()->Uint32LessThanOrEqual(),
-        jsgraph()->IntPtrConstant(static_cast<uintptr_t>(end_offset)),
+    Node* cond;
+    if (jsgraph()->machine()->Is32()) {
+      cond = graph()->NewNode(jsgraph()->machine()->Uint32LessThanOrEqual(),
+                              jsgraph()->Int32Constant(end_offset), *mem_size_);
+    } else {
+      cond = graph()->NewNode(
+          jsgraph()->machine()->Uint64LessThanOrEqual(),
+          jsgraph()->Int64Constant(static_cast<int64_t>(end_offset)),
           *mem_size_);
+    }
     TrapIfFalse(wasm::kTrapMemOutOfBounds, cond, position);
   } else {
     // The end offset is within the bounds of the smallest memory, so only
     // one check is required. Check to see if the index is also a constant.
-    Uint32Matcher m(index);
+    UintPtrMatcher m(index);
     if (m.HasValue()) {
       uint64_t index_val = m.Value();
       if ((index_val + offset + access_size) <= min_size) {
@@ -3424,12 +3439,22 @@ void WasmGraphBuilder::BoundsCheckMem(MachineType memtype, Node* index,
     }
   }
 
-  Node* effective_size =
+  Node* effective_size;
+  if (jsgraph()->machine()->Is32()) {
+    effective_size =
         graph()->NewNode(jsgraph()->machine()->Int32Sub(), *mem_size_,
                          jsgraph()->Int32Constant(end_offset - 1));
+  } else {
+    effective_size = graph()->NewNode(
+        jsgraph()->machine()->Int64Sub(), *mem_size_,
+        jsgraph()->Int64Constant(static_cast<int64_t>(end_offset - 1)));
+  }
 
-  Node* cond = graph()->NewNode(jsgraph()->machine()->Uint32LessThan(), index,
-                                effective_size);
+  const Operator* less = jsgraph()->machine()->Is32()
+                             ? jsgraph()->machine()->Uint32LessThan()
+                             : jsgraph()->machine()->Uint64LessThan();
+
+  Node* cond = graph()->NewNode(less, index, effective_size);
   TrapIfFalse(wasm::kTrapMemOutOfBounds, cond, position);
 }
 
@@ -3483,6 +3508,10 @@ Node* WasmGraphBuilder::LoadMem(wasm::ValueType type, MachineType memtype,
                                 wasm::WasmCodePosition position) {
   Node* load;
 
+  if (jsgraph()->machine()->Is64()) {
+    index =
+        graph()->NewNode(jsgraph()->machine()->ChangeUint32ToUint64(), index);
+  }
   // Wasm semantics throw on OOB. Introduce explicit bounds check.
   if (!FLAG_wasm_trap_handler || !V8_TRAP_HANDLER_SUPPORTED) {
     BoundsCheckMem(memtype, index, offset, position);
@@ -3538,6 +3567,10 @@ Node* WasmGraphBuilder::StoreMem(MachineType memtype, Node* index,
                                  wasm::ValueType type) {
   Node* store;
 
+  if (jsgraph()->machine()->Is64()) {
+    index =
+        graph()->NewNode(jsgraph()->machine()->ChangeUint32ToUint64(), index);
+  }
   // Wasm semantics throw on OOB. Introduce explicit bounds check.
   if (!FLAG_wasm_trap_handler || !V8_TRAP_HANDLER_SUPPORTED) {
     BoundsCheckMem(memtype, index, offset, position);
@@ -3584,6 +3617,10 @@ Node* WasmGraphBuilder::BuildAsmjsLoadMem(MachineType type, Node* index) {
   // asm.js semantics use CheckedLoad (i.e. OOB reads return 0ish).
   DCHECK_NOT_NULL(*mem_size_);
   DCHECK_NOT_NULL(*mem_start_);
+  if (jsgraph()->machine()->Is64()) {
+    index =
+        graph()->NewNode(jsgraph()->machine()->ChangeUint32ToUint64(), index);
+  }
   const Operator* op = jsgraph()->machine()->CheckedLoad(type);
   Node* load =
       graph()->NewNode(op, *mem_start_, index, *mem_size_, *effect_, *control_);
@@ -3597,6 +3634,10 @@ Node* WasmGraphBuilder::BuildAsmjsStoreMem(MachineType type, Node* index,
   // asm.js semantics use CheckedStore (i.e. ignore OOB writes).
   DCHECK_NOT_NULL(*mem_size_);
   DCHECK_NOT_NULL(*mem_start_);
+  if (jsgraph()->machine()->Is64()) {
+    index =
+        graph()->NewNode(jsgraph()->machine()->ChangeUint32ToUint64(), index);
+  }
   const Operator* op =
       jsgraph()->machine()->CheckedStore(type.representation());
   Node* store = graph()->NewNode(op, *mem_start_, index, *mem_size_, val,
@@ -3617,7 +3658,7 @@ Node* WasmGraphBuilder::String(const char* string) {
 Graph* WasmGraphBuilder::graph() { return jsgraph()->graph(); }
 
 void WasmGraphBuilder::LowerInt64() {
-  if (!jsgraph()->machine()->Is32()) return;
+  if (jsgraph()->machine()->Is64()) return;
   Int64Lowering r(jsgraph()->graph(), jsgraph()->machine(), jsgraph()->common(),
                   jsgraph()->zone(), sig_);
   r.LowerGraph();

test/mjsunit/wasm/bounds-check-64bit.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.
+
+load("test/mjsunit/wasm/wasm-constants.js");
+load("test/mjsunit/wasm/wasm-module-builder.js");
+
+const builder = new WasmModuleBuilder();
+builder.addMemory(1, kV8MaxPages, false);
+builder.addFunction('load', kSig_i_ii)
+    .addBody([
+        kExprGetLocal, 0,
+        kExprI64SConvertI32,
+        kExprGetLocal, 1,
+        kExprI64SConvertI32,
+        kExprI64Shl,
+        kExprI32ConvertI64,
+    kExprI32LoadMem, 0, 0])
+    .exportFunc();
+
+const module = builder.instantiate();
+let start = 12;
+let address = start;
+for (i = 1; i < 64; i++) {
+  // This is the address which will be accessed in the code. We cannot use
+  // shifts to calculate the address because JS shifts work on 32-bit integers.
+  address = (address * 2) % 4294967296;
+  if (address < kPageSize) {
+    assertEquals(0, module.exports.load(start, i));
+  } else {
+    assertTraps(kTrapMemOutOfBounds, _ => { module.exports.load(start, i);});
+  }
+}