From 4778831e22add430e69933c53c3d8f7f89db9e36 Mon Sep 17 00:00:00 2001 From: Gus Caplan Date: Wed, 22 Jun 2022 23:18:33 -0700 Subject: [PATCH] wasi: use WasmMemoryObject handle for perf MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/43544 Reviewed-By: Tobias Nießen Reviewed-By: Colin Ihrig Reviewed-By: Anna Henningsen --- lib/wasi.js | 16 ------ src/node_wasi.cc | 30 +++++------ src/node_wasi.h | 2 +- test/wasi/test-wasi-initialize-validation.js | 53 +++----------------- test/wasi/test-wasi-start-validation.js | 50 ++---------------- 5 files changed, 22 insertions(+), 129 deletions(-) diff --git a/lib/wasi.js b/lib/wasi.js index 7093953a00739a..2a0770bf6dbbb0 100644 --- a/lib/wasi.js +++ b/lib/wasi.js @@ -10,14 +10,12 @@ const { } = primordials; const { - ERR_INVALID_ARG_TYPE, ERR_WASI_ALREADY_STARTED } = require('internal/errors').codes; const { emitExperimentalWarning, kEmptyObject, } = require('internal/util'); -const { isArrayBuffer } = require('internal/util/types'); const { validateArray, validateBoolean, @@ -39,20 +37,6 @@ function setupInstance(self, instance) { validateObject(instance, 'instance'); validateObject(instance.exports, 'instance.exports'); - // WASI::_SetMemory() in src/node_wasi.cc only expects that |memory| is - // an object. It will try to look up the .buffer property when needed - // and fail with UVWASI_EINVAL when the property is missing or is not - // an ArrayBuffer. Long story short, we don't need much validation here - // but we type-check anyway because it helps catch bugs in the user's - // code early. - validateObject(instance.exports.memory, 'instance.exports.memory'); - if (!isArrayBuffer(instance.exports.memory.buffer)) { - throw new ERR_INVALID_ARG_TYPE( - 'instance.exports.memory.buffer', - ['WebAssembly.Memory'], - instance.exports.memory.buffer); - } - self[kInstance] = instance; self[kSetMemory](instance.exports.memory); } diff --git a/src/node_wasi.cc b/src/node_wasi.cc index 803edf0c91dda1..965a619c8d4acd 100644 --- a/src/node_wasi.cc +++ b/src/node_wasi.cc @@ -72,9 +72,7 @@ inline void Debug(WASI* wasi, Args&&... args) { } \ } while (0) - using v8::Array; -using v8::ArrayBuffer; using v8::BackingStore; using v8::BigInt; using v8::Context; @@ -89,7 +87,7 @@ using v8::Object; using v8::String; using v8::Uint32; using v8::Value; - +using v8::WasmMemoryObject; static MaybeLocal WASIException(Local context, int errorno, @@ -1642,26 +1640,22 @@ void WASI::SockShutdown(const FunctionCallbackInfo& args) { void WASI::_SetMemory(const FunctionCallbackInfo& args) { WASI* wasi; - CHECK_EQ(args.Length(), 1); - CHECK(args[0]->IsObject()); ASSIGN_OR_RETURN_UNWRAP(&wasi, args.This()); - wasi->memory_.Reset(wasi->env()->isolate(), args[0].As()); + CHECK_EQ(args.Length(), 1); + if (!args[0]->IsWasmMemoryObject()) { + return node::THROW_ERR_INVALID_ARG_TYPE( + wasi->env(), + "\"instance.exports.memory\" property must be a WebAssembly.Memory " + "object"); + } + wasi->memory_.Reset(wasi->env()->isolate(), args[0].As()); } uvwasi_errno_t WASI::backingStore(char** store, size_t* byte_length) { - Environment* env = this->env(); - Local memory = PersistentToLocal::Strong(this->memory_); - Local prop; - - if (!memory->Get(env->context(), env->buffer_string()).ToLocal(&prop)) - return UVWASI_EINVAL; - - if (!prop->IsArrayBuffer()) - return UVWASI_EINVAL; - - Local ab = prop.As(); - std::shared_ptr backing_store = ab->GetBackingStore(); + Local memory = PersistentToLocal::Strong(this->memory_); + std::shared_ptr backing_store = + memory->Buffer()->GetBackingStore(); *byte_length = backing_store->ByteLength(); *store = static_cast(backing_store->Data()); CHECK_NOT_NULL(*store); diff --git a/src/node_wasi.h b/src/node_wasi.h index 7a0be60aa645a7..b3814ddc31033a 100644 --- a/src/node_wasi.h +++ b/src/node_wasi.h @@ -94,7 +94,7 @@ class WASI : public BaseObject, inline void writeUInt64(char* memory, uint64_t value, uint32_t offset); uvwasi_errno_t backingStore(char** store, size_t* byte_length); uvwasi_t uvw_; - v8::Global memory_; + v8::Global memory_; uvwasi_mem_t alloc_info_; size_t current_uvwasi_memory_ = 0; }; diff --git a/test/wasi/test-wasi-initialize-validation.js b/test/wasi/test-wasi-initialize-validation.js index 40dfd864d1874e..6506e1ac4a79e3 100644 --- a/test/wasi/test-wasi-initialize-validation.js +++ b/test/wasi/test-wasi-initialize-validation.js @@ -47,7 +47,10 @@ const bufferSource = fixtures.readSync('simple.wasm'); Object.defineProperty(instance, 'exports', { get() { - return { _initialize: 5, memory: new Uint8Array() }; + return { + _initialize: 5, + memory: new WebAssembly.Memory({ initial: 1 }), + }; }, }); assert.throws( @@ -70,7 +73,7 @@ const bufferSource = fixtures.readSync('simple.wasm'); return { _start() {}, _initialize() {}, - memory: new Uint8Array(), + memory: new WebAssembly.Memory({ initial: 1 }), }; } }); @@ -97,55 +100,11 @@ const bufferSource = fixtures.readSync('simple.wasm'); () => { wasi.initialize(instance); }, { code: 'ERR_INVALID_ARG_TYPE', - message: /"instance\.exports\.memory" property must be of type object/ + message: /"instance\.exports\.memory" property must be a WebAssembly\.Memory object/ } ); } - { - // Verify that a non-ArrayBuffer memory.buffer is rejected. - const wasi = new WASI({}); - const wasm = await WebAssembly.compile(bufferSource); - const instance = await WebAssembly.instantiate(wasm); - - Object.defineProperty(instance, 'exports', { - get() { - return { - _initialize() {}, - memory: {}, - }; - } - }); - // The error message is a little white lie because any object - // with a .buffer property of type ArrayBuffer is accepted, - // but 99% of the time a WebAssembly.Memory object is used. - assert.throws( - () => { wasi.initialize(instance); }, - { - code: 'ERR_INVALID_ARG_TYPE', - message: /"instance\.exports\.memory\.buffer" property must be an WebAssembly\.Memory/ - } - ); - } - - { - // Verify that an argument that duck-types as a WebAssembly.Instance - // is accepted. - const wasi = new WASI({}); - const wasm = await WebAssembly.compile(bufferSource); - const instance = await WebAssembly.instantiate(wasm); - - Object.defineProperty(instance, 'exports', { - get() { - return { - _initialize() {}, - memory: { buffer: new ArrayBuffer(0) }, - }; - } - }); - wasi.initialize(instance); - } - { // Verify that a WebAssembly.Instance from another VM context is accepted. const wasi = new WASI({}); diff --git a/test/wasi/test-wasi-start-validation.js b/test/wasi/test-wasi-start-validation.js index 2059ff081e88dd..016479f412f678 100644 --- a/test/wasi/test-wasi-start-validation.js +++ b/test/wasi/test-wasi-start-validation.js @@ -47,7 +47,7 @@ const bufferSource = fixtures.readSync('simple.wasm'); Object.defineProperty(instance, 'exports', { get() { - return { memory: new Uint8Array() }; + return { memory: new WebAssembly.Memory({ initial: 1 }) }; }, }); assert.throws( @@ -70,7 +70,7 @@ const bufferSource = fixtures.readSync('simple.wasm'); return { _start() {}, _initialize() {}, - memory: new Uint8Array(), + memory: new WebAssembly.Memory({ initial: 1 }), }; } }); @@ -97,55 +97,11 @@ const bufferSource = fixtures.readSync('simple.wasm'); () => { wasi.start(instance); }, { code: 'ERR_INVALID_ARG_TYPE', - message: /"instance\.exports\.memory" property must be of type object/ + message: /"instance\.exports\.memory" property must be a WebAssembly\.Memory object/ } ); } - { - // Verify that a non-ArrayBuffer memory.buffer is rejected. - const wasi = new WASI({}); - const wasm = await WebAssembly.compile(bufferSource); - const instance = await WebAssembly.instantiate(wasm); - - Object.defineProperty(instance, 'exports', { - get() { - return { - _start() {}, - memory: {}, - }; - } - }); - // The error message is a little white lie because any object - // with a .buffer property of type ArrayBuffer is accepted, - // but 99% of the time a WebAssembly.Memory object is used. - assert.throws( - () => { wasi.start(instance); }, - { - code: 'ERR_INVALID_ARG_TYPE', - message: /"instance\.exports\.memory\.buffer" property must be an WebAssembly\.Memory/ - } - ); - } - - { - // Verify that an argument that duck-types as a WebAssembly.Instance - // is accepted. - const wasi = new WASI({}); - const wasm = await WebAssembly.compile(bufferSource); - const instance = await WebAssembly.instantiate(wasm); - - Object.defineProperty(instance, 'exports', { - get() { - return { - _start() {}, - memory: { buffer: new ArrayBuffer(0) }, - }; - } - }); - wasi.start(instance); - } - { // Verify that a WebAssembly.Instance from another VM context is accepted. const wasi = new WASI({});