Commit 7cf29d8d authored by Eric Holk's avatar Eric Holk Committed by Commit Bot

Revert "[wasm] always allocate memory when guard regions are needed"

This reverts commit 1f99c66b.

Reason for revert: Test timeouts on Win64 Debug: https://build.chromium.org/p/client.v8/builders/V8%20Win64%20-%20debug/builds/19226

Original change's description:
> [wasm] always allocate memory when guard regions are needed
> 
> When using trap handlers, memory references do not get any checks inserted. This
> means there is no check for a null memory as happens when the memory size is
> 0. Normally this would be correctly caught as an out of bounds access, since the
> low memory addresses are not normally mapped. However, if they were mapped for
> some reason, we would not catch the out of bounds access.
> 
> The fix is to ensure WebAssembly instances always have a guard region even if
> the memory is size 0.
> 
> Bug: chromium:769637
> Change-Id: I2d0f8c107563236c3780eb7746c2f820e319c65f
> Reviewed-on: https://chromium-review.googlesource.com/693137
> Reviewed-by: Mircea Trofin <mtrofin@chromium.org>
> Commit-Queue: Eric Holk <eholk@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#48240}

TBR=gdeepti@chromium.org,mtrofin@chromium.org,eholk@chromium.org

Change-Id: I4065b367c6cfffe8dd601b67cd53ad54759ae96a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: chromium:769637
Reviewed-on: https://chromium-review.googlesource.com/692918Reviewed-by: 's avatarEric Holk <eholk@chromium.org>
Commit-Queue: Eric Holk <eholk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48242}
parent 658daa65
......@@ -1806,10 +1806,7 @@ MaybeHandle<WasmInstanceObject> InstanceBuilder::Build() {
DCHECK_IMPLIES(EnableGuardRegions(),
module_->is_asm_js() || memory->has_guard_region());
} else if (initial_pages > 0 || EnableGuardRegions()) {
// We need to unconditionally create a guard region if using trap handlers,
// even when the size is zero to prevent null-derence issues
// (e.g. https://crbug.com/769637).
} else if (initial_pages > 0) {
memory_ = AllocateMemory(initial_pages);
if (memory_.is_null()) return {}; // failed to allocate memory
}
......
......@@ -138,11 +138,11 @@ Handle<JSArrayBuffer> NewArrayBuffer(Isolate* isolate, size_t size,
void* allocation_base = nullptr; // Set by TryAllocateBackingStore
size_t allocation_length = 0; // Set by TryAllocateBackingStore
// Normally we would avoid calling TryAllocateBackingStore at all for
// zero-sized memories. This is tricky with guard pages. Instead, this logic
// for when to allocate lives inside TryAllocateBackingStore.
void* memory = TryAllocateBackingStore(isolate, size, enable_guard_regions,
allocation_base, allocation_length);
// Do not reserve memory till non zero memory is encountered.
void* memory =
(size == 0) ? nullptr
: TryAllocateBackingStore(isolate, size, enable_guard_regions,
allocation_base, allocation_length);
if (size > 0 && memory == nullptr) {
return Handle<JSArrayBuffer>::null();
......
// 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');
let builder = new WasmModuleBuilder();
builder
.addMemory()
.addFunction("main", kSig_v_v)
.addBody([kExprI32Const, 4,
kExprI32Const, 8,
kExprI32StoreMem, 0, 16])
.exportAs("main");
let instance = builder.instantiate();
assertTraps(kTrapMemOutOfBounds, instance.exports.main);
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment