Skip to content

Commit

Permalink
Merged: [wasm] Change the memory access offset to pointer size
Browse files Browse the repository at this point in the history
Revision: 50ecc42cc744b49d9f26ad8b92325c0a36775407

Please take a careful look at this back-merge, because the original
patch does not apply properly. The patch was created after Enrico's
changes, but branch 6.2 is before Enrico's changes.

Differences to the original patch:
(1) This CL does not do a 64-bit bounds check. The reason is that in
    6.2 we still use RelocInfo::WASM_MEMORY_SIZE_REFERENCE, which
    assumes that the memory size is a 32-bit constant.
(2) Because of (1) I apply the ChangeUint32ToUint64 after the bounds
    check. The reason is that the 32-bit bounds check cannot deal
    with 64 bit nodes.

BUG=chromium:766666
LOG=N
NOTRY=true
NOPRESUBMIT=true
NOTREECHECKS=true
R=titzer@chromium.org

Change-Id: Ia76519538840c562acf7b64e9946e20c5f987ef9
Reviewed-on: https://chromium-review.googlesource.com/735350
Commit-Queue: Andreas Haas <ahaas@chromium.org>
Reviewed-by: Ben Titzer <titzer@chromium.org>
Cr-Commit-Position: refs/branch-heads/6.2@{#72}
Cr-Branched-From: efa2ac4-refs/heads/6.2.414@{#1}
Cr-Branched-From: a861ebb-refs/heads/master@{#47693}
  • Loading branch information
gahaas authored and Commit Bot committed Oct 25, 2017
1 parent 5eb0505 commit 4c977da
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 0 deletions.
16 changes: 16 additions & 0 deletions src/compiler/wasm-compiler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3215,6 +3215,10 @@ Node* WasmGraphBuilder::LoadMem(wasm::ValueType type, MachineType memtype,
BoundsCheckMem(memtype, index, offset, position);
}

if (jsgraph()->machine()->Is64()) {
index =
graph()->NewNode(jsgraph()->machine()->ChangeUint32ToUint64(), index);
}
if (memtype.representation() == MachineRepresentation::kWord8 ||
jsgraph()->machine()->UnalignedLoadSupported(memtype.representation())) {
if (FLAG_wasm_trap_handler && V8_TRAP_HANDLER_SUPPORTED) {
Expand Down Expand Up @@ -3267,6 +3271,10 @@ Node* WasmGraphBuilder::StoreMem(MachineType memtype, Node* index,
BoundsCheckMem(memtype, index, offset, position);
}

if (jsgraph()->machine()->Is64()) {
index =
graph()->NewNode(jsgraph()->machine()->ChangeUint32ToUint64(), index);
}
#if defined(V8_TARGET_BIG_ENDIAN)
val = BuildChangeEndiannessStore(val, memtype, type);
#endif
Expand Down Expand Up @@ -3301,6 +3309,10 @@ Node* WasmGraphBuilder::StoreMem(MachineType memtype, Node* index,
Node* WasmGraphBuilder::BuildAsmjsLoadMem(MachineType type, Node* index) {
// TODO(turbofan): fold bounds checks for constant asm.js loads.
// asm.js semantics use CheckedLoad (i.e. OOB reads return 0ish).
if (jsgraph()->machine()->Is64()) {
index =
graph()->NewNode(jsgraph()->machine()->ChangeUint32ToUint64(), index);
}
const Operator* op = jsgraph()->machine()->CheckedLoad(type);
Node* load =
graph()->NewNode(op, MemBuffer(0), index, MemSize(), *effect_, *control_);
Expand All @@ -3312,6 +3324,10 @@ Node* WasmGraphBuilder::BuildAsmjsStoreMem(MachineType type, Node* index,
Node* val) {
// TODO(turbofan): fold bounds checks for constant asm.js stores.
// asm.js semantics use CheckedStore (i.e. ignore OOB writes).
if (jsgraph()->machine()->Is64()) {
index =
graph()->NewNode(jsgraph()->machine()->ChangeUint32ToUint64(), index);
}
const Operator* op =
jsgraph()->machine()->CheckedStore(type.representation());
Node* store = graph()->NewNode(op, MemBuffer(0), index, MemSize(), val,
Expand Down
33 changes: 33 additions & 0 deletions test/mjsunit/wasm/bounds-check-64bit.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// 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);});
}
}

0 comments on commit 4c977da

Please sign in to comment.