[Webkit-unassigned] [Bug 219192] [WASM-References] Add support for active mods in element section

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=219192

--- Comment #8 from Saam Barati <sbarati at apple.com> ---
Comment on attachment 414886
  --> https://bugs.webkit.org/attachment.cgi?id=414886
Patch

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

> Source/JavaScriptCore/ChangeLog:10
> +        Add support for vec(exp) in element section:

can you explain what it used t be and how this is different?

> Source/JavaScriptCore/ChangeLog:12
> +        Just transformed (ref.func idx) -> idx and (ref.null func) -> null.

I don't know what this means.

> Source/JavaScriptCore/ChangeLog:13
> +        For null values call table->clear to zero value.

I don't understand the significance here. Can you explain it more

> Source/JavaScriptCore/wasm/WasmSectionParser.cpp:386
> +            const uint32_t tableIndex = 0;

nit: can be constexpr

> Source/JavaScriptCore/wasm/WasmSectionParser.cpp:412
> +        case 0x01: {
> +            WASM_PARSER_FAIL_IF(!Options::useWebAssemblyReferences(), "references are not enabled");
> +            WASM_PARSER_FAIL_IF(true, "unsupported ", elementNum, "th Element reserved byte");
> +            break;
> +        }

What do we need to do to support this? Maybe we should have a bug?

> Source/JavaScriptCore/wasm/WasmSectionParser.cpp:432
> +                Optional<uint32_t> funcIdx;

Why optional here? Seems like a weird choice since we fail unless we parse a real uint32_t

> Source/JavaScriptCore/wasm/WasmSectionParser.cpp:443
> +        case 0x03: {
> +            WASM_PARSER_FAIL_IF(!Options::useWebAssemblyReferences(), "references are not enabled");
> +            WASM_PARSER_FAIL_IF(true, "unsupported ", elementNum, "th Element reserved byte");
> +            break;

Should we have a bug on implementing this?

> Source/JavaScriptCore/wasm/WasmSectionParser.cpp:472
> +            WASM_PARSER_FAIL_IF(true, "unsupported ", elementNum, "th Element reserved byte");

What do we need to do to support this? Maybe we should have a bug?

> Source/JavaScriptCore/wasm/WasmSectionParser.cpp:487
> +            WASM_PARSER_FAIL_IF(refType == Externref, "reftype in element section should be funcref");

I think the error message and the condition here are a bit worrisome if anyone adds another enum value. Why can't we just directly check if != Funcref?

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20201130/86d5f1b2/attachment-0001.htm>