From aa93e6ca95123ccd176c392a2b5b8772ee5eb5c4 Mon Sep 17 00:00:00 2001 From: ahaas Date: Wed, 5 Oct 2016 02:11:54 -0700 Subject: [PATCH] [wasm] Call a runtime function for a MemorySize instruction. The implementation of MemorySize with RelocatableInt32Constants is problematic if MemorySize is placed close to a GrowMemory instruction in the code. The use of a runtime function guarantees that the order in which MemorySize and GrowMemory is executed is correct. R=titzer@chromium.org BUG=chromium:651961 TEST=mjsunit/regress/wasm/regression-651961 Committed: https://crrev.com/2c12a9a42d454a36fcd2931fa458d72832eeb689 Review-Url: https://codereview.chromium.org/2386183004 Cr-Original-Commit-Position: refs/heads/master@{#39972} Cr-Commit-Position: refs/heads/master@{#39980} --- src/compiler/wasm-compiler.cc | 23 ++++++++++++-- src/runtime/runtime-wasm.cc | 21 +++++++++++++ src/runtime/runtime.h | 1 + src/wasm/wasm-module.cc | 11 +++++++ src/wasm/wasm-module.h | 2 ++ test/cctest/wasm/test-run-wasm-module.cc | 15 ++++++++++ test/cctest/wasm/test-run-wasm.cc | 16 ---------- .../mjsunit/regress/wasm/regression-651961.js | 24 +++++++++++++++ test/mjsunit/wasm/memory-size.js | 30 +++++++++++++++++++ 9 files changed, 125 insertions(+), 18 deletions(-) create mode 100644 test/mjsunit/regress/wasm/regression-651961.js create mode 100644 test/mjsunit/wasm/memory-size.js diff --git a/src/compiler/wasm-compiler.cc b/src/compiler/wasm-compiler.cc index dac9eec7a7..b003e9968a 100644 --- a/src/compiler/wasm-compiler.cc +++ b/src/compiler/wasm-compiler.cc @@ -2759,8 +2759,27 @@ Node* WasmGraphBuilder::MemBuffer(uint32_t offset) { } Node* WasmGraphBuilder::CurrentMemoryPages() { - return graph()->NewNode(jsgraph()->machine()->Word32Shr(), MemSize(0), - jsgraph()->Int32Constant(16)); + Runtime::FunctionId function_id = Runtime::kWasmMemorySize; + const Runtime::Function* function = Runtime::FunctionForId(function_id); + CallDescriptor* desc = Linkage::GetRuntimeCallDescriptor( + jsgraph()->zone(), function_id, function->nargs, Operator::kNoThrow, + CallDescriptor::kNoFlags); + wasm::ModuleEnv* module = module_; + Node* inputs[] = { + jsgraph()->CEntryStubConstant(function->result_size), // C entry + jsgraph()->ExternalConstant( + ExternalReference(function_id, jsgraph()->isolate())), // ref + jsgraph()->Int32Constant(function->nargs), // arity + jsgraph()->HeapConstant(module->instance->context), // context + *effect_, + *control_}; + Node* call = graph()->NewNode(jsgraph()->common()->Call(desc), + static_cast(arraysize(inputs)), inputs); + + Node* result = BuildChangeSmiToInt32(call); + + *effect_ = call; + return result; } Node* WasmGraphBuilder::MemSize(uint32_t offset) { diff --git a/src/runtime/runtime-wasm.cc b/src/runtime/runtime-wasm.cc index 4be8fa5718..ab69046c45 100644 --- a/src/runtime/runtime-wasm.cc +++ b/src/runtime/runtime-wasm.cc @@ -18,6 +18,27 @@ namespace v8 { namespace internal { +RUNTIME_FUNCTION(Runtime_WasmMemorySize) { + HandleScope scope(isolate); + DCHECK_EQ(0, args.length()); + + Handle module_instance; + { + // Get the module JSObject + DisallowHeapAllocation no_allocation; + const Address entry = Isolate::c_entry_fp(isolate->thread_local_top()); + Address pc = + Memory::Address_at(entry + StandardFrameConstants::kCallerPCOffset); + Code* code = + isolate->inner_pointer_to_code_cache()->GetCacheEntry(pc)->code; + Object* owning_instance = wasm::GetOwningWasmInstance(code); + CHECK_NOT_NULL(owning_instance); + module_instance = handle(JSObject::cast(owning_instance), isolate); + } + return *isolate->factory()->NewNumberFromInt( + wasm::GetInstanceMemorySize(isolate, module_instance)); +} + RUNTIME_FUNCTION(Runtime_WasmGrowMemory) { HandleScope scope(isolate); DCHECK_EQ(1, args.length()); diff --git a/src/runtime/runtime.h b/src/runtime/runtime.h index b834186486..cbdaf0f033 100644 --- a/src/runtime/runtime.h +++ b/src/runtime/runtime.h @@ -917,6 +917,7 @@ namespace internal { #define FOR_EACH_INTRINSIC_WASM(F) \ F(WasmGrowMemory, 1, 1) \ + F(WasmMemorySize, 0, 1) \ F(WasmThrowTypeError, 0, 1) \ F(WasmThrow, 2, 1) \ F(WasmGetCaughtExceptionValue, 1, 1) diff --git a/src/wasm/wasm-module.cc b/src/wasm/wasm-module.cc index 884f651aeb..f4cf505f5a 100644 --- a/src/wasm/wasm-module.cc +++ b/src/wasm/wasm-module.cc @@ -1809,6 +1809,17 @@ void SetInstanceMemory(Handle instance, JSArrayBuffer* buffer) { module->set_ptr_to_heap(buffer); } +int32_t GetInstanceMemorySize(Isolate* isolate, Handle instance) { + MaybeHandle maybe_mem_buffer = + GetInstanceMemory(isolate, instance); + Handle buffer; + if (!maybe_mem_buffer.ToHandle(&buffer)) { + return 0; + } else { + return buffer->byte_length()->Number() / WasmModule::kPageSize; + } +} + int32_t GrowInstanceMemory(Isolate* isolate, Handle instance, uint32_t pages) { Address old_mem_start = nullptr; diff --git a/src/wasm/wasm-module.h b/src/wasm/wasm-module.h index bf6961b914..ac75042392 100644 --- a/src/wasm/wasm-module.h +++ b/src/wasm/wasm-module.h @@ -534,6 +534,8 @@ uint32_t GetNumImportedFunctions(Handle wasm_object); // Returns nullptr on failing to get owning instance. Object* GetOwningWasmInstance(Code* code); +int32_t GetInstanceMemorySize(Isolate* isolate, Handle instance); + int32_t GrowInstanceMemory(Isolate* isolate, Handle instance, uint32_t pages); diff --git a/test/cctest/wasm/test-run-wasm-module.cc b/test/cctest/wasm/test-run-wasm-module.cc index 766f5d03db..394a723677 100644 --- a/test/cctest/wasm/test-run-wasm-module.cc +++ b/test/cctest/wasm/test-run-wasm-module.cc @@ -253,6 +253,21 @@ TEST(Run_WasmModule_Serialization) { } } +TEST(MemorySize) { + // Initial memory size is 16, see wasm-module-builder.cc + static const int kExpectedValue = 16; + TestSignatures sigs; + v8::internal::AccountingAllocator allocator; + Zone zone(&allocator); + + WasmModuleBuilder* builder = new (&zone) WasmModuleBuilder(&zone); + WasmFunctionBuilder* f = builder->AddFunction(sigs.i_v()); + ExportAsMain(f); + byte code[] = {WASM_MEMORY_SIZE}; + f->EmitCode(code, sizeof(code)); + TestModule(&zone, builder, kExpectedValue); +} + TEST(Run_WasmModule_MemSize_GrowMem) { // Initial memory size = 16 + GrowMemory(10) static const int kExpectedValue = 26; diff --git a/test/cctest/wasm/test-run-wasm.cc b/test/cctest/wasm/test-run-wasm.cc index 422ced9be9..38c4769b69 100644 --- a/test/cctest/wasm/test-run-wasm.cc +++ b/test/cctest/wasm/test-run-wasm.cc @@ -79,22 +79,6 @@ WASM_EXEC_TEST(Int32Const_many) { } } -WASM_EXEC_TEST(MemorySize1) { - TestingModule module(execution_mode); - WasmRunner r(&module); - module.AddMemory(WasmModule::kPageSize * 1); - BUILD(r, kExprMemorySize); - CHECK_EQ(1, r.Call()); -} - -WASM_EXEC_TEST(MemorySize2) { - TestingModule module(execution_mode); - WasmRunner r(&module); - module.AddMemory(WasmModule::kPageSize * 3); - BUILD(r, kExprMemorySize); - CHECK_EQ(3, r.Call()); -} - WASM_EXEC_TEST(Int32Param0) { WasmRunner r(execution_mode, MachineType::Int32()); // return(local[0]) diff --git a/test/mjsunit/regress/wasm/regression-651961.js b/test/mjsunit/regress/wasm/regression-651961.js new file mode 100644 index 0000000000..abdec98358 --- /dev/null +++ b/test/mjsunit/regress/wasm/regression-651961.js @@ -0,0 +1,24 @@ +// Copyright 2016 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. + +// Flags: --expose-wasm + +load("test/mjsunit/wasm/wasm-constants.js"); +load("test/mjsunit/wasm/wasm-module-builder.js"); + +(function() { + var builder = new WasmModuleBuilder(); + builder.addMemory(1, 1, false); + builder.addFunction("foo", kSig_i_v) + .addBody([ + kExprMemorySize, + kExprI32Const, 0x10, + kExprGrowMemory, + kExprI32Mul, + ]) + .exportFunc(); + var module = builder.instantiate(); + var result = module.exports.foo(); + assertEquals(1, result); +})(); diff --git a/test/mjsunit/wasm/memory-size.js b/test/mjsunit/wasm/memory-size.js new file mode 100644 index 0000000000..197059eb49 --- /dev/null +++ b/test/mjsunit/wasm/memory-size.js @@ -0,0 +1,30 @@ +// Copyright 2016 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. + +// Flags: --expose-wasm + +load("test/mjsunit/wasm/wasm-constants.js"); +load("test/mjsunit/wasm/wasm-module-builder.js"); + +(function testMemorySizeZero() { + print("testMemorySizeZero()"); + var builder = new WasmModuleBuilder(); + builder.addFunction("memory_size", kSig_i_v) + .addBody([kExprMemorySize]) + .exportFunc(); + var module = builder.instantiate(); + assertEquals(0, module.exports.memory_size()); +})(); + +(function testMemorySizeNonZero() { + print("testMemorySizeNonZero()"); + var builder = new WasmModuleBuilder(); + var size = 11; + builder.addMemory(size, size, false); + builder.addFunction("memory_size", kSig_i_v) + .addBody([kExprMemorySize]) + .exportFunc(); + var module = builder.instantiate(); + assertEquals(size, module.exports.memory_size()); +})();