Commit 8ee06838 authored by Enrico Bacis's avatar Enrico Bacis Committed by Commit Bot

[wasm] Fix grow-memory on exported memory

The WASM spec maximum memory size is higher than internal V8 maximum object
size. When a memory object grows above this limit (and only in that case), we
should signal an error.

This worked for not-exported memory; however when growing exported memory, the
code was comparing the V8 memory limit with the maximum number of pages defined
in the module, instead of the current number of pages + the number of new
required pages. This lead to signaling errors even when growing exported memory
below the V8 limit if the maximum number of pages specified in the module was
higher than the V8 limit.

GrowMemoryBuffer already checks that we do not grow a memory buffer past the
maximum size specified as parameter, so we can pass it the minimum between the
the V8 limit and the maximum number of pages specified in the module.

This CL introduces a test in test/mjsunit/wasm/import-memory.js that triggers
the problematic path and a patch to fix it.

R=ahaas@chromium.org,clemensh@chromium.org,gdeepti@chromium.org

Change-Id: I5a8da420418b394d61e1ba3cdf4408c3c09e61b6
Reviewed-on: https://chromium-review.googlesource.com/600217Reviewed-by: 's avatarDeepti Gandluri <gdeepti@chromium.org>
Reviewed-by: 's avatarAndreas Haas <ahaas@chromium.org>
Reviewed-by: 's avatarClemens Hammacher <clemensh@chromium.org>
Commit-Queue: Enrico Bacis <enricobacis@google.com>
Cr-Commit-Position: refs/heads/master@{#47395}
parent 21da12a9
......@@ -387,8 +387,8 @@ int32_t WasmMemoryObject::Grow(Isolate* isolate,
uint32_t maximum_pages;
if (memory_object->has_maximum_pages()) {
maximum_pages = static_cast<uint32_t>(memory_object->maximum_pages());
if (FLAG_wasm_max_mem_pages < maximum_pages) return -1;
maximum_pages = Min(FLAG_wasm_max_mem_pages,
static_cast<uint32_t>(memory_object->maximum_pages()));
} else {
maximum_pages = FLAG_wasm_max_mem_pages;
}
......
......@@ -8,7 +8,6 @@ load("test/mjsunit/wasm/wasm-constants.js");
load("test/mjsunit/wasm/wasm-module-builder.js");
var kPageSize = 0x10000;
var kV8MaxPages = 32767;
function genGrowMemoryBuilder() {
var builder = new WasmModuleBuilder();
......@@ -479,9 +478,8 @@ testGrowMemoryDeclaredMaxTraps();
function testGrowMemoryDeclaredSpecMaxTraps() {
// The spec maximum is higher than the internal V8 maximum. This test only
// checks that grow_memory does not grow past the internally defined maximum
// to reflect the currentl implementation.
// to reflect the current implementation.
var builder = genGrowMemoryBuilder();
var kSpecMaxPages = 65535;
builder.addMemory(1, kSpecMaxPages, false);
var module = builder.instantiate();
function poke(value) { return module.exports.store(offset, value); }
......
......@@ -149,7 +149,6 @@ load("test/mjsunit/wasm/wasm-module-builder.js");
(function TestGrowMemoryZeroInitialMemory() {
print("ZeroInitialMemory");
let kV8MaxPages = 32767;
let memory = new WebAssembly.Memory({initial: 0});
assertEquals(0, memory.buffer.byteLength);
let i32 = new Int32Array(memory.buffer);
......@@ -367,6 +366,36 @@ load("test/mjsunit/wasm/wasm-module-builder.js");
}
})();
(function TestExportImportedMemoryGrowPastV8Maximum() {
// The spec maximum is higher than the internal V8 maximum. This test only
// checks that grow_memory does not grow past the internally defined maximum
// to reflect the current implementation even when the memory is exported.
print("TestExportImportedMemoryGrowPastV8Maximum");
var instance_1, instance_2;
{
let builder = new WasmModuleBuilder();
builder.addMemory(1, kSpecMaxPages, true);
builder.exportMemoryAs("exported_mem");
builder.addFunction("grow", kSig_i_i)
.addBody([kExprGetLocal, 0, kExprGrowMemory, kMemoryZero])
.exportFunc();
instance_1 = builder.instantiate();
}
{
let builder = new WasmModuleBuilder();
builder.addImportedMemory("doo", "imported_mem");
builder.addFunction("grow", kSig_i_i)
.addBody([kExprGetLocal, 0, kExprGrowMemory, kMemoryZero])
.exportFunc();
instance_2 = builder.instantiate({
doo: {imported_mem: instance_1.exports.exported_mem}});
}
assertEquals(1, instance_1.exports.grow(20));
assertEquals(21, instance_2.exports.grow(20));
assertEquals(-1, instance_1.exports.grow(kV8MaxPages - 40));
assertEquals(-1, instance_2.exports.grow(kV8MaxPages - 40));
})();
(function TestExportGrow() {
print("TestExportGrow");
let builder = new WasmModuleBuilder();
......
......@@ -15,6 +15,9 @@ function bytes() {
return buffer;
}
// V8 internal constants
var kV8MaxPages = 32767;
// Header declaration constants
var kWasmH0 = 0;
var kWasmH1 = 0x61;
......@@ -28,6 +31,7 @@ var kWasmV3 = 0;
var kHeaderSize = 8;
var kPageSize = 65536;
var kSpecMaxPages = 65535;
function bytesWithHeader() {
var buffer = new ArrayBuffer(kHeaderSize + arguments.length);
......
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