Skip to content

Commit

Permalink
wasi: use WasmMemoryObject handle for perf (#43544)
Browse files Browse the repository at this point in the history 
PR-URL: #43544
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
  • Loading branch information
@devsnek @targos
devsnek authored and targos committed Jul 20, 2022
1 parent b8d36e5 commit 7d7573f
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 129 deletions.
16 changes: 0 additions & 16 deletions lib/wasi.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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);
}
Expand Down
30 changes: 12 additions & 18 deletions src/node_wasi.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -89,7 +87,7 @@ using v8::Object;
using v8::String;
using v8::Uint32;
using v8::Value;

using v8::WasmMemoryObject;

static MaybeLocal<Value> WASIException(Local<Context> context,
int errorno,
Expand Down Expand Up @@ -1642,26 +1640,22 @@ void WASI::SockShutdown(const FunctionCallbackInfo<Value>& args) {

void WASI::_SetMemory(const FunctionCallbackInfo<Value>& 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<Object>());
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<WasmMemoryObject>());
}


uvwasi_errno_t WASI::backingStore(char** store, size_t* byte_length) {
Environment* env = this->env();
Local<Object> memory = PersistentToLocal::Strong(this->memory_);
Local<Value> prop;

if (!memory->Get(env->context(), env->buffer_string()).ToLocal(&prop))
return UVWASI_EINVAL;

if (!prop->IsArrayBuffer())
return UVWASI_EINVAL;

Local<ArrayBuffer> ab = prop.As<ArrayBuffer>();
std::shared_ptr<BackingStore> backing_store = ab->GetBackingStore();
Local<WasmMemoryObject> memory = PersistentToLocal::Strong(this->memory_);
std::shared_ptr<BackingStore> backing_store =
memory->Buffer()->GetBackingStore();
*byte_length = backing_store->ByteLength();
*store = static_cast<char*>(backing_store->Data());
CHECK_NOT_NULL(*store);
Expand Down
2 changes: 1 addition & 1 deletion src/node_wasi.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<v8::Object> memory_;
v8::Global<v8::WasmMemoryObject> memory_;
uvwasi_mem_t alloc_info_;
size_t current_uvwasi_memory_ = 0;
};
Expand Down
53 changes: 6 additions & 47 deletions test/wasi/test-wasi-initialize-validation.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -70,7 +73,7 @@ const bufferSource = fixtures.readSync('simple.wasm');
return {
_start() {},
_initialize() {},
memory: new Uint8Array(),
memory: new WebAssembly.Memory({ initial: 1 }),
};
}
});
Expand All @@ -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({});
Expand Down
50 changes: 3 additions & 47 deletions test/wasi/test-wasi-start-validation.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -70,7 +70,7 @@ const bufferSource = fixtures.readSync('simple.wasm');
return {
_start() {},
_initialize() {},
memory: new Uint8Array(),
memory: new WebAssembly.Memory({ initial: 1 }),
};
}
});
Expand All @@ -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({});
Expand Down

0 comments on commit 7d7573f

Please sign in to comment.