[Webkit-unassigned] [Bug 219297] [WASM-References] Add table.init

[bugzilla-daemon at webkit.org]

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

Yusuke Suzuki <ysuzuki at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #415747|review?                     |review-
              Flags|                            |

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

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

Nice. I'm putting r- since I found several bugs.

> Source/JavaScriptCore/wasm/WasmFunctionParser.h:621
> +            WASM_PARSER_FAIL_IF(!parseVarUInt32(elementIndex), "can't parse element index");
> +            WASM_VALIDATOR_FAIL_IF(elementIndex >= m_info.elementCount(), "element index ", elementIndex, " is invalid, limit is ", m_info.elementCount());
> +
> +            unsigned tableIndex;
> +            WASM_PARSER_FAIL_IF(!parseVarUInt32(tableIndex), "can't parse element index");
> +            WASM_VALIDATOR_FAIL_IF(tableIndex >= m_info.tableCount(), "table index ", tableIndex, " is invalid, limit is ", m_info.tableCount());

This parsing behavior is different from the other table ext operations. This means that we need to have special handling in `FunctionParser<Context>::parseUnreachableExpression()` side too.
The purpose of `FunctionParser<Context>::parseUnreachableExpression()` is skip all wasm stack pushing/poping, but validates imm etc. for unreachable wasm opcodes.
Can you add them?
I think we need TableCopy one too.
And, can you add tests for these cases? I think we can just emit these opcodes after return, and ensure we are stressing table opcodes in parseUnreachableExpression.

> Source/JavaScriptCore/wasm/WasmInstance.cpp:73
> +    for (unsigned elementIdx = 0; elementIdx < m_passiveElements.size(); ++elementIdx) {

Rename idx to index.

> Source/JavaScriptCore/wasm/WasmInstance.cpp:183
> +    for (uint32_t idx = 0; idx < length; ++idx) {

Rename it to index.

> Source/JavaScriptCore/wasm/WasmInstance.cpp:187
> +        JSWebAssemblyInstance* jsInstance = owner<JSWebAssemblyInstance>();
> +        JSWebAssemblyTable* jsTable = jsInstance->table(tableIndex);

We should put them out of the loop.

> Source/JavaScriptCore/wasm/WasmInstance.cpp:199
> +        JSGlobalObject* globalObject = jsInstance->globalObject();
> +        VM& vm = globalObject->vm();

Ditto. Let's put them out of the loop.

> Source/JavaScriptCore/wasm/WasmInstance.cpp:246
> +void Instance::tableInit(uint32_t dstOffset, uint32_t srcOffset, uint32_t len, uint32_t elementIndex, uint32_t tableIndex)

len => length.

-- 
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/20201210/f5fd0d2c/attachment-0001.htm>