From 2c4563003cdaa7342cd26a279d9fa89268922741 Mon Sep 17 00:00:00 2001 From: gdeepti Date: Fri, 14 Oct 2016 15:42:47 -0700 Subject: [PATCH] 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} --- src/compiler/wasm-compiler.cc | 17 +++--- src/wasm/wasm-module.cc | 44 +++++++++++++-- src/wasm/wasm-module.h | 5 ++ test/mjsunit/wasm/grow-memory.js | 89 +----------------------------- test/mjsunit/wasm/import-memory.js | 31 ----------- 5 files changed, 57 insertions(+), 129 deletions(-) diff --git a/src/compiler/wasm-compiler.cc b/src/compiler/wasm-compiler.cc index 484a0d9a50..4392896c64 100644 --- a/src/compiler/wasm-compiler.cc +++ b/src/compiler/wasm-compiler.cc @@ -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(offset) + memsize)) { + if (size == 0) { + effective_size = 0; + } else if (offset >= size || + (static_cast(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(size), + static_cast(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); diff --git a/src/wasm/wasm-module.cc b/src/wasm/wasm-module.cc index c0432179dc..abbc2ba85d 100644 --- a/src/wasm/wasm-module.cc +++ b/src/wasm/wasm-module.cc @@ -2056,6 +2056,38 @@ Handle wasm::GetDebugInfo(Handle wasm) { return new_info; } +bool wasm::UpdateWasmModuleMemory(Handle 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 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::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 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 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 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); } diff --git a/src/wasm/wasm-module.h b/src/wasm/wasm-module.h index 1376b97088..25a50ed68e 100644 --- a/src/wasm/wasm-module.h +++ b/src/wasm/wasm-module.h @@ -515,6 +515,11 @@ Handle