From 20b892b5a0402afcd0ef6780170b7361e6963234 Mon Sep 17 00:00:00 2001 From: Jakob Kummerow Date: Fri, 28 Feb 2020 11:57:06 +0100 Subject: [PATCH] [wasm] Fix memory growth to >2GB There were a few places that still checked against the limit for initial memory size rather than the limit for memory size after growth (which was recently separated from the former). Bug: v8:7881 Change-Id: Id17d86e2f7a5dfa4f1dd35153b0cefc01f72ed33 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2078574 Commit-Queue: Jakob Kummerow Reviewed-by: Andreas Haas Cr-Commit-Position: refs/heads/master@{#66496} --- src/objects/backing-store.cc | 18 +++++++-- src/wasm/module-instantiate.cc | 2 +- src/wasm/wasm-js.cc | 4 +- src/wasm/wasm-limits.h | 2 +- src/wasm/wasm-objects.cc | 16 ++++---- test/mjsunit/mjsunit.status | 1 + test/mjsunit/wasm/grow-huge-memory.js | 35 +++++++++++++++++ test/mjsunit/wasm/grow-memory.js | 48 ++++++++---------------- test/mjsunit/wasm/import-memory.js | 2 +- test/mjsunit/wasm/wasm-module-builder.js | 2 +- 10 files changed, 79 insertions(+), 51 deletions(-) create mode 100644 test/mjsunit/wasm/grow-huge-memory.js diff --git a/src/objects/backing-store.cc b/src/objects/backing-store.cc index 29498711a0..4dee39c10d 100644 --- a/src/objects/backing-store.cc +++ b/src/objects/backing-store.cc @@ -9,6 +9,7 @@ #include "src/execution/isolate.h" #include "src/handles/global-handles.h" #include "src/logging/counters.h" +#include "src/wasm/wasm-constants.h" #include "src/wasm/wasm-engine.h" #include "src/wasm/wasm-limits.h" #include "src/wasm/wasm-objects-inl.h" @@ -313,9 +314,11 @@ std::unique_ptr BackingStore::TryAllocateWasmMemory( // Compute size of reserved memory. - size_t engine_max_pages = wasm::max_initial_mem_pages(); - size_t byte_capacity = - std::min(engine_max_pages, maximum_pages) * wasm::kWasmPageSize; + size_t engine_max_pages = wasm::max_maximum_mem_pages(); + maximum_pages = std::min(engine_max_pages, maximum_pages); + CHECK_LE(maximum_pages, + std::numeric_limits::max() / wasm::kWasmPageSize); + size_t byte_capacity = maximum_pages * wasm::kWasmPageSize; size_t reservation_size = GetReservationSize(guards, byte_capacity); //-------------------------------------------------------------------------- @@ -408,7 +411,7 @@ std::unique_ptr BackingStore::AllocateWasmMemory( DCHECK_EQ(0, wasm::kWasmPageSize % AllocatePageSize()); // Enforce engine limitation on the maximum number of pages. - if (initial_pages > wasm::max_initial_mem_pages()) return nullptr; + if (initial_pages > wasm::kV8MaxWasmMemoryPages) return nullptr; auto backing_store = TryAllocateWasmMemory(isolate, initial_pages, maximum_pages, shared); @@ -422,6 +425,13 @@ std::unique_ptr BackingStore::AllocateWasmMemory( std::unique_ptr BackingStore::CopyWasmMemory(Isolate* isolate, size_t new_pages) { + // Trying to allocate 4 GiB on a 32-bit platform is guaranteed to fail. + // We don't lower the official max_maximum_mem_pages() limit because that + // would be observable upon instantiation; this way the effective limit + // on 32-bit platforms is defined by the allocator. + if (new_pages > std::numeric_limits::max() / wasm::kWasmPageSize) { + return {}; + } DCHECK_GE(new_pages * wasm::kWasmPageSize, byte_length_); // Note that we could allocate uninitialized to save initialization cost here, // but since Wasm memories are allocated by the page allocator, the zeroing diff --git a/src/wasm/module-instantiate.cc b/src/wasm/module-instantiate.cc index 93e7422054..1e530bb760 100644 --- a/src/wasm/module-instantiate.cc +++ b/src/wasm/module-instantiate.cc @@ -1405,7 +1405,7 @@ bool InstanceBuilder::AllocateMemory() { uint32_t initial_pages = module_->initial_pages; uint32_t maximum_pages = module_->has_maximum_pages ? module_->maximum_pages - : wasm::max_initial_mem_pages(); + : wasm::max_maximum_mem_pages(); if (initial_pages > max_initial_mem_pages()) { thrower_->RangeError("Out of memory: wasm memory too large"); return false; diff --git a/src/wasm/wasm-js.cc b/src/wasm/wasm-js.cc index 435c8d1a42..84f43f6ded 100644 --- a/src/wasm/wasm-js.cc +++ b/src/wasm/wasm-js.cc @@ -1698,8 +1698,8 @@ void WebAssemblyMemoryGrow(const v8::FunctionCallbackInfo& args) { } uint64_t max_size64 = receiver->maximum_pages(); - if (max_size64 > uint64_t{i::wasm::max_initial_mem_pages()}) { - max_size64 = i::wasm::max_initial_mem_pages(); + if (max_size64 > uint64_t{i::wasm::max_maximum_mem_pages()}) { + max_size64 = i::wasm::max_maximum_mem_pages(); } i::Handle old_buffer(receiver->array_buffer(), i_isolate); diff --git a/src/wasm/wasm-limits.h b/src/wasm/wasm-limits.h index 547d0b5f81..02303fb69d 100644 --- a/src/wasm/wasm-limits.h +++ b/src/wasm/wasm-limits.h @@ -67,7 +67,7 @@ V8_EXPORT_PRIVATE uint32_t max_maximum_mem_pages(); uint32_t max_table_init_entries(); inline uint64_t max_mem_bytes() { - return uint64_t{max_initial_mem_pages()} * kWasmPageSize; + return uint64_t{max_maximum_mem_pages()} * kWasmPageSize; } } // namespace wasm diff --git a/src/wasm/wasm-objects.cc b/src/wasm/wasm-objects.cc index 013d1a9902..16e64651b1 100644 --- a/src/wasm/wasm-objects.cc +++ b/src/wasm/wasm-objects.cc @@ -879,26 +879,22 @@ int32_t WasmMemoryObject::Grow(Isolate* isolate, if (old_buffer->is_asmjs_memory()) return -1; // Checks for maximum memory size. - uint32_t maximum_pages = wasm::max_initial_mem_pages(); + uint32_t maximum_pages = wasm::max_maximum_mem_pages(); if (memory_object->has_maximum_pages()) { maximum_pages = std::min( maximum_pages, static_cast(memory_object->maximum_pages())); } - CHECK_GE(wasm::max_initial_mem_pages(), maximum_pages); + DCHECK_GE(wasm::max_maximum_mem_pages(), maximum_pages); size_t old_size = old_buffer->byte_length(); - CHECK_EQ(0, old_size % wasm::kWasmPageSize); + DCHECK_EQ(0, old_size % wasm::kWasmPageSize); size_t old_pages = old_size / wasm::kWasmPageSize; - CHECK_GE(wasm::max_initial_mem_pages(), old_pages); - if ((pages > maximum_pages - old_pages) || // exceeds remaining - (pages > wasm::max_initial_mem_pages() - old_pages)) { // exceeds limit - return -1; - } + CHECK_GE(wasm::max_maximum_mem_pages(), old_pages); + if (pages > maximum_pages - old_pages) return -1; std::shared_ptr backing_store = old_buffer->GetBackingStore(); if (!backing_store) return -1; // Compute new size. size_t new_pages = old_pages + pages; - size_t new_byte_length = new_pages * wasm::kWasmPageSize; // Try to handle shared memory first. if (old_buffer->is_shared()) { @@ -909,6 +905,8 @@ int32_t WasmMemoryObject::Grow(Isolate* isolate, new_pages); // Broadcasting the update should update this memory object too. CHECK_NE(*old_buffer, memory_object->array_buffer()); + // If the allocation succeeded, then this can't possibly overflow: + size_t new_byte_length = new_pages * wasm::kWasmPageSize; // This is a less than check, as it is not guaranteed that the SAB // length here will be equal to the stashed length above as calls to // grow the same memory object can come in from different workers. diff --git a/test/mjsunit/mjsunit.status b/test/mjsunit/mjsunit.status index 240cea6f0d..19d577cff0 100644 --- a/test/mjsunit/mjsunit.status +++ b/test/mjsunit/mjsunit.status @@ -429,6 +429,7 @@ # 32-bit platforms ['arch in (ia32, arm, mips, mipsel)', { # Needs >2GB of available contiguous memory. + 'wasm/grow-huge-memory': [SKIP], 'wasm/huge-memory': [SKIP], 'wasm/huge-typedarray': [SKIP], }], # 'arch in (ia32, arm, mips, mipsel)' diff --git a/test/mjsunit/wasm/grow-huge-memory.js b/test/mjsunit/wasm/grow-huge-memory.js new file mode 100644 index 0000000000..5fe8f5dccf --- /dev/null +++ b/test/mjsunit/wasm/grow-huge-memory.js @@ -0,0 +1,35 @@ +// Copyright 2020 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. + +// Save some memory on Linux; other platforms ignore this flag. +// Flags: --multi-mapped-mock-allocator + +// Test that we can grow memories to sizes beyond 2GB. + +load("test/mjsunit/wasm/wasm-module-builder.js"); + +function GetMemoryPages(memory) { + return memory.buffer.byteLength >>> 16; +} + +(function TestGrowFromJS() { + let mem = new WebAssembly.Memory({initial: 200}); + mem.grow(40000); + assertEquals(40200, GetMemoryPages(mem)); +})(); + +(function TestGrowFromWasm() { + let builder = new WasmModuleBuilder(); + builder.addMemory(200, 50000, true); + builder.addFunction("grow", kSig_i_v) + .addBody([ + ...wasmI32Const(40000), // Number of pages to grow by. + kExprMemoryGrow, kMemoryZero, // Grow memory. + kExprDrop, // Drop result of grow (old pages). + kExprMemorySize, kMemoryZero // Get the memory size. + ]).exportFunc(); + let instance = builder.instantiate(); + assertEquals(40200, instance.exports.grow()); + assertEquals(40200, GetMemoryPages(instance.exports.memory)); +})(); diff --git a/test/mjsunit/wasm/grow-memory.js b/test/mjsunit/wasm/grow-memory.js index 6d0e7e5c5f..b7bc1090a8 100644 --- a/test/mjsunit/wasm/grow-memory.js +++ b/test/mjsunit/wasm/grow-memory.js @@ -37,7 +37,7 @@ function genMemoryGrowBuilder() { } // V8 internal memory size limit. -var kV8MaxPages = 32767; +var kV8MaxPages = 65536; // TODO(gdeepti): Generate tests programatically for all the sizes instead of @@ -477,23 +477,18 @@ function testMemoryGrowDeclaredMaxTraps() { testMemoryGrowDeclaredMaxTraps(); -function testMemoryGrowDeclaredSpecMaxTraps() { - // 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. +(function testMemoryGrowInternalMaxTraps() { + // This test checks that grow_memory does not grow past the internally + // defined maximum memory size. var builder = genMemoryGrowBuilder(); builder.addMemory(1, kSpecMaxPages, false); var module = builder.instantiate(); - function poke(value) { return module.exports.store(offset, value); } function growMem(pages) { return module.exports.grow_memory(pages); } assertEquals(1, growMem(20)); assertEquals(-1, growMem(kV8MaxPages - 20)); -} +})(); -testMemoryGrowDeclaredSpecMaxTraps(); - -function testMemoryGrow2Gb() { - print("testMemoryGrow2Gb"); +(function testMemoryGrow4Gb() { var builder = genMemoryGrowBuilder(); builder.addMemory(1, undefined, false); var module = builder.instantiate(); @@ -502,36 +497,27 @@ function testMemoryGrow2Gb() { function poke(value) { return module.exports.store(offset, value); } function growMem(pages) { return module.exports.grow_memory(pages); } - for(offset = 0; offset <= (kPageSize - 4); offset+=4) { + for (offset = 0; offset <= (kPageSize - 4); offset += 4) { poke(100000 - offset); assertEquals(100000 - offset, peek()); } let result = growMem(kV8MaxPages - 1); - if (result == 1 ){ - for(offset = 0; offset <= (kPageSize - 4); offset+=4) { + if (result == 1) { + for (offset = 0; offset <= (kPageSize - 4); offset += 4) { assertEquals(100000 - offset, peek()); } - // Bounds check for large mem size - for(offset = (kV8MaxPages - 1) * kPageSize; - offset <= (kV8MaxPages * kPageSize - 4); offset+=4) { + // Bounds check for large mem size. + let kMemSize = (kV8MaxPages * kPageSize); + let kLastValidOffset = kMemSize - 4; // Accommodate a 4-byte read/write. + for (offset = kMemSize - kPageSize; offset <= kLastValidOffset; + offset += 4) { poke(0xaced); assertEquals(0xaced, peek()); } - for (offset = kV8MaxPages * kPageSize - 3; - offset <= kV8MaxPages * kPageSize + 4; offset++) { - assertTraps(kTrapMemOutOfBounds, poke); - } - - // Check traps around 3GB/4GB boundaries - let offset_3gb = 49152 * kPageSize; - let offset_4gb = 2 * kV8MaxPages * kPageSize; - for (offset = offset_3gb - 5; offset < offset_3gb + 4; offset++) { - assertTraps(kTrapMemOutOfBounds, poke); - } - for (offset = offset_4gb - 5; offset < offset_4gb; offset++) { + for (offset = kLastValidOffset + 1; offset < kMemSize; offset++) { assertTraps(kTrapMemOutOfBounds, poke); } } else { @@ -539,6 +525,4 @@ function testMemoryGrow2Gb() { // bit platforms. When grow_memory fails, expected result is -1. assertEquals(-1, result); } -} - -testMemoryGrow2Gb(); +})(); diff --git a/test/mjsunit/wasm/import-memory.js b/test/mjsunit/wasm/import-memory.js index 08100efabd..bac6d8f624 100644 --- a/test/mjsunit/wasm/import-memory.js +++ b/test/mjsunit/wasm/import-memory.js @@ -7,7 +7,7 @@ load("test/mjsunit/wasm/wasm-module-builder.js"); // V8 internal memory size limit. -var kV8MaxPages = 32767; +var kV8MaxPages = 65536; (function TestOne() { print("TestOne"); diff --git a/test/mjsunit/wasm/wasm-module-builder.js b/test/mjsunit/wasm/wasm-module-builder.js index c315b5a853..8f30a39a26 100644 --- a/test/mjsunit/wasm/wasm-module-builder.js +++ b/test/mjsunit/wasm/wasm-module-builder.js @@ -45,7 +45,7 @@ var kWasmV3 = 0; var kHeaderSize = 8; var kPageSize = 65536; -var kSpecMaxPages = 65535; +var kSpecMaxPages = 65536; var kMaxVarInt32Size = 5; var kMaxVarInt64Size = 10;