[webkit-reviews] review granted: [Bug 220914] [JSC] WebAssembly.Global should support Funcref and Externref : [Attachment 418433] Patch

[bugzilla-daemon at webkit.org]

Yusuke Suzuki <ysuzuki at apple.com> has granted  review:
Bug 220914: [JSC] WebAssembly.Global should support Funcref and Externref
https://bugs.webkit.org/show_bug.cgi?id=220914

Attachment 418433: Patch

https://bugs.webkit.org/attachment.cgi?id=418433&action=review




--- Comment #3 from Yusuke Suzuki <ysuzuki at apple.com> ---
Comment on attachment 418433
  --> https://bugs.webkit.org/attachment.cgi?id=418433
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=418433&action=review

r=me with comments.

> Source/JavaScriptCore/wasm/js/WebAssemblyGlobalConstructor.cpp:146
> +    RETURN_IF_EXCEPTION(throwScope, encodedJSValue());

Use `RETURN_IF_EXCEPTION(throwScope, {});` in newer code.

> Source/JavaScriptCore/wasm/js/WebAssemblyGlobalConstructor.cpp:151
> +	   if (type == Wasm::Type::Funcref && argument.isUndefined())
> +	       argument = jsNull();

Can we share this "default value" part with WasmTable etc.?

> Source/JavaScriptCore/wasm/js/WebAssemblyGlobalConstructor.cpp:152
> +	   jsWebAssemblyGlobal->global()->set(globalObject, argument);

Insert `RETURN_IF_EXCEPTION(throwScope, { });` after calling a function which
takes `globalObject` as a first parameter (this is a rough indicator of "it can
throw").

> Source/JavaScriptCore/wasm/js/WebAssemblyGlobalConstructor.cpp:154
> +    RELEASE_AND_RETURN(throwScope, JSValue::encode(jsWebAssemblyGlobal));

RELEASE_AND_RETURN is not necessary if the evaluated part will not throw. Let's
use `return`.

> JSTests/wasm/references/globals.js:42
> +  assert.throws(() => new WebAssembly.Global({ value: "anyfunc", mutable:
true }, new Pelmen(calories)), WebAssembly.RuntimeError, "Funcref must be an
exported wasm function (evaluating 'new WebAssembly.Global({ value:
\"anyfunc\", mutable: true }, new Pelmen(calories))')");

Let's add a test, setting a wrong value via `.value = xxx`.