Revert of [wasm] Fix bounds check for zero initial memory. (patchset #11 id:200001 of https://codereview.chromium.org/2416543002/ )

Reason for revert:
Reverting because of failure on V8 Linux64 GC Stress

http://build.chromium.org/p/client.v8/builders/V8%20Linux64%20GC%20Stress%20-%20custom%20snapshot/builds/8572

Original issue's description:
> [wasm] Fix bounds check for zero initial memory.
>
> Currently when memory size references are updated with zero initial memory during GrowMemory/Relocation of Instance objects, the bounds check does not take into account the size of memtype.
>
> R=titzer@chromium.org, bradnelson@chromium.org
>
> Committed: https://crrev.com/70416a2b360c0d993cffb48284b143d484d1e290
> Cr-Commit-Position: refs/heads/master@{#40326}

TBR=bradnelson@chromium.org,titzer@chromium.org,bradnelson@google.com,mtrofin@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true

Review-Url: https://codereview.chromium.org/2416393002
Cr-Commit-Position: refs/heads/master@{#40328}
This commit is contained in:
gdeepti 2016-10-14 15:42:47 -07:00 committed by Commit bot
parent b122da39d0
commit 2c4563003c
5 changed files with 57 additions and 129 deletions

View File

@ -2859,22 +2859,25 @@ void WasmGraphBuilder::BoundsCheckMem(MachineType memtype, Node* index,
uint32_t size = module_->instance->mem_size;
byte memsize = wasm::WasmOpcodes::MemSize(memtype);
// Check against the effective size.
size_t effective_size;
if (size <= offset || size < (static_cast<uint64_t>(offset) + memsize)) {
if (size == 0) {
effective_size = 0;
} else if (offset >= size ||
(static_cast<uint64_t>(offset) + memsize) > size) {
// Two checks are needed in the case where the offset is statically
// out of bounds; one check for the offset being in bounds, and the next for
// the offset + index being out of bounds for code to be patched correctly
// on relocation.
size_t effective_offset = offset + memsize - 1;
effective_size = size - memsize + 1;
Node* cond = graph()->NewNode(jsgraph()->machine()->Uint32LessThan(),
jsgraph()->IntPtrConstant(effective_offset),
jsgraph()->IntPtrConstant(offset),
jsgraph()->RelocatableInt32Constant(
static_cast<uint32_t>(size),
static_cast<uint32_t>(effective_size),
RelocInfo::WASM_MEMORY_SIZE_REFERENCE));
trap_->AddTrapIfFalse(wasm::kTrapMemOutOfBounds, cond, position);
// For offset > effective size, this relies on check above to fail and
// effective size can be negative, relies on wrap around.
effective_size = size - offset - memsize + 1;
DCHECK(offset >= effective_size);
effective_size = offset - effective_size;
} else {
effective_size = size - offset - memsize + 1;
CHECK(effective_size <= kMaxUInt32);

View File

@ -2056,6 +2056,38 @@ Handle<WasmDebugInfo> wasm::GetDebugInfo(Handle<JSObject> wasm) {
return new_info;
}
bool wasm::UpdateWasmModuleMemory(Handle<JSObject> object, Address old_start,
Address new_start, uint32_t old_size,
uint32_t new_size) {
DisallowHeapAllocation no_allocation;
if (!IsWasmObject(*object)) {
return false;
}
// Get code table associated with the module js_object
Object* obj = object->GetInternalField(kWasmModuleCodeTable);
Handle<FixedArray> code_table(FixedArray::cast(obj));
// Iterate through the code objects in the code table and update relocation
// information
for (int i = 0; i < code_table->length(); ++i) {
obj = code_table->get(i);
Handle<Code> code(Code::cast(obj));
int mode_mask = RelocInfo::ModeMask(RelocInfo::WASM_MEMORY_REFERENCE) |
RelocInfo::ModeMask(RelocInfo::WASM_MEMORY_SIZE_REFERENCE);
for (RelocIterator it(*code, mode_mask); !it.done(); it.next()) {
RelocInfo::Mode mode = it.rinfo()->rmode();
if (RelocInfo::IsWasmMemoryReference(mode) ||
RelocInfo::IsWasmMemorySizeReference(mode)) {
it.rinfo()->update_wasm_memory_reference(old_start, new_start, old_size,
new_size);
}
}
}
return true;
}
Handle<FixedArray> wasm::BuildFunctionTable(Isolate* isolate, uint32_t index,
const WasmModule* module) {
const WasmIndirectFunctionTable* table = &module->function_tables[index];
@ -2207,9 +2239,9 @@ int32_t wasm::GetInstanceMemorySize(Isolate* isolate,
int32_t wasm::GrowInstanceMemory(Isolate* isolate, Handle<JSObject> instance,
uint32_t pages) {
if (!IsWasmObject(*instance)) return false;
if (pages == 0) return GetInstanceMemorySize(isolate, instance);
if (pages == 0) {
return GetInstanceMemorySize(isolate, instance);
}
Address old_mem_start = nullptr;
uint32_t old_size = 0, new_size = 0;
@ -2246,8 +2278,10 @@ int32_t wasm::GrowInstanceMemory(Isolate* isolate, Handle<JSObject> instance,
memcpy(new_mem_start, old_mem_start, old_size);
}
SetInstanceMemory(instance, *buffer);
RelocateInstanceCode(instance, old_mem_start, new_mem_start, old_size,
new_size);
if (!UpdateWasmModuleMemory(instance, old_mem_start, new_mem_start, old_size,
new_size)) {
return -1;
}
DCHECK(old_size % WasmModule::kPageSize == 0);
return (old_size / WasmModule::kPageSize);
}

View File

@ -515,6 +515,11 @@ Handle<Script> GetAsmWasmScript(Handle<JSObject> wasm);
int GetAsmWasmSourcePosition(Handle<JSObject> wasm, int func_index,
int byte_offset);
// Update memory references of code objects associated with the module
bool UpdateWasmModuleMemory(Handle<JSObject> object, Address old_start,
Address new_start, uint32_t old_size,
uint32_t new_size);
// Constructs a single function table as a FixedArray of double size,
// populating it with function signature indices and function indices.
Handle<FixedArray> BuildFunctionTable(Isolate* isolate, uint32_t index,

View File

@ -38,8 +38,6 @@ function genGrowMemoryBuilder() {
return builder;
}
// TODO(gdeepti): Generate tests programatically for all the sizes instead of
// current implementation.
function testGrowMemoryReadWrite32() {
var builder = genGrowMemoryBuilder();
builder.addMemory(1, 1, false);
@ -199,95 +197,14 @@ function testGrowMemoryZeroInitialSize() {
assertEquals(20, peek());
}
for(offset = kPageSize - 3; offset <= kPageSize + 5; offset++) {
assertTraps(kTrapMemOutOfBounds, peek);
}
offset = 3*kPageSize;
for (var i = 1; i < 4; i++) {
assertTraps(kTrapMemOutOfBounds, poke);
assertEquals(i, growMem(1));
}
poke(20);
assertEquals(20, peek());
}
testGrowMemoryZeroInitialSize();
function testGrowMemoryZeroInitialSize32() {
var builder = genGrowMemoryBuilder();
var module = builder.instantiate();
var offset;
function peek() { return module.exports.load(offset); }
function poke(value) { return module.exports.store(offset, value); }
function growMem(pages) { return module.exports.grow_memory(pages); }
assertTraps(kTrapMemOutOfBounds, peek);
assertTraps(kTrapMemOutOfBounds, poke);
assertEquals(0, growMem(1));
for(offset = 0; offset <= kPageSize - 4; offset++) {
poke(20);
assertEquals(20, peek());
}
for(offset = kPageSize - 3; offset <= kPageSize + 5; offset++) {
assertTraps(kTrapMemOutOfBounds, peek);
}
}
testGrowMemoryZeroInitialSize32();
function testGrowMemoryZeroInitialSize16() {
var builder = genGrowMemoryBuilder();
var module = builder.instantiate();
var offset;
function peek() { return module.exports.load16(offset); }
function poke(value) { return module.exports.store16(offset, value); }
function growMem(pages) { return module.exports.grow_memory(pages); }
assertTraps(kTrapMemOutOfBounds, peek);
assertTraps(kTrapMemOutOfBounds, poke);
assertEquals(0, growMem(1));
for(offset = 0; offset <= kPageSize - 2; offset++) {
poke(20);
assertEquals(20, peek());
}
for(offset = kPageSize - 1; offset <= kPageSize + 5; offset++) {
assertTraps(kTrapMemOutOfBounds, peek);
}
}
testGrowMemoryZeroInitialSize16();
function testGrowMemoryZeroInitialSize8() {
var builder = genGrowMemoryBuilder();
var module = builder.instantiate();
var offset;
function peek() { return module.exports.load8(offset); }
function poke(value) { return module.exports.store8(offset, value); }
function growMem(pages) { return module.exports.grow_memory(pages); }
assertTraps(kTrapMemOutOfBounds, peek);
assertTraps(kTrapMemOutOfBounds, poke);
assertEquals(0, growMem(1));
for(offset = 0; offset <= kPageSize - 1; offset++) {
poke(20);
assertEquals(20, peek());
}
//TODO(gdeepti): Fix tests with correct write boundaries
//when runtime function is fixed.
for(offset = kPageSize; offset <= kPageSize + 5; offset++) {
assertTraps(kTrapMemOutOfBounds, peek);
}
}
testGrowMemoryZeroInitialSize8();
testGrowMemoryZeroInitialSize();
function testGrowMemoryTrapMaxPagesZeroInitialMemory() {
var builder = genGrowMemoryBuilder();

View File

@ -80,34 +80,3 @@ load("test/mjsunit/wasm/wasm-module-builder.js");
}
}
})();
(function ValidateBoundsCheck() {
print("Validate bounds check");
let memory = new WebAssembly.Memory({initial: 1, maximum: 5});
assertEquals(kPageSize, memory.buffer.byteLength);
let i32 = new Int32Array(memory.buffer);
let builder = new WasmModuleBuilder();
// builder.addImportedMemory("mine");
builder.addImportedMemory("mine");
builder.addFunction("load", kSig_i_i)
.addBody([kExprGetLocal, 0, kExprI32LoadMem, 0, 0])
.exportFunc();
builder.addFunction("store", kSig_i_ii)
.addBody([kExprGetLocal, 0, kExprGetLocal, 1, kExprI32StoreMem, 0, 0,
kExprGetLocal, 1])
.exportFunc();
var offset;
let instance = builder.instantiate({mine: memory});
function load() { return instance.exports.load(offset); }
function store(value) { return instance.exports.store(offset, value); }
for (offset = 0; offset < kPageSize -3; offset+=4) {
store(offset);
}
for (offset = 0; offset < kPageSize - 3; offset+=4) {
assertEquals(offset, load());
}
for (offset = kPageSize - 3; offset < kPageSize + 4; offset++) {
assertTraps(kTrapMemOutOfBounds, load);
}
})();