[Webkit-unassigned] [Bug 219943] [WASM-References] Add support for memory.copy, memory.init and data.drop

[bugzilla-daemon at webkit.org]

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

Yusuke Suzuki <ysuzuki at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ysuzuki at apple.com

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

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

> Source/JavaScriptCore/ChangeLog:12
> +        Add support for memory.copy [dstAddress, srcAddress, length] -> [],
> +        memory.init data_index [dstAddress, srcAddress, length] -> []
> +        and for data.drop data_index.
> +        https://webassembly.github.io/reference-types/core/binary/instructions.html#memory-instructions.
> +        Overview: https://github.com/WebAssembly/bulk-memory-operations/blob/master/proposals/bulk-memory-operations/Overview.md.

Can you describe what each opcode does in ChangeLog?
ChangeLog is very important to know what is implemented, and what is supposed to be done in the patch. While URL can be broken, ChangeLog exists in the tree. And we can know the rationale behind the patch :).

> Source/JavaScriptCore/wasm/WasmInstance.cpp:186
> +    const uint32_t segmentSizeInBytes = m_passiveDataSegments.quickGet(dataSegmentIndex) ? segment->sizeInBytes : 0u;

We use `0U`.

> Source/JavaScriptCore/wasm/WasmMemory.cpp:654
> +    if (offset + length > m_handle->size())

We should use sumOverflows

> Source/JavaScriptCore/wasm/WasmSectionParser.cpp:640
> +auto SectionParser::parseI32InitExpr(Optional<I32InitExpr>& initExpr, const char* failMessage) -> PartialResult

Let's make `const char*` to ASCIILiteral. And use ASCIILiteral to pass string to this function by using `"STRING"_s`.

> Source/JavaScriptCore/wasm/WasmSections.h:92
> +    const auto previousKnownSectionId = static_cast<uint8_t>(previousKnown);
> +    const auto nextSectionId = static_cast<uint8_t>(next);

This is not necessary.

> Source/JavaScriptCore/wasm/WasmSections.h:95
> +        && previousKnownSectionId == static_cast<uint8_t>(Section::DataCount)

It should be `previousKnown == Section::DataCount`

> Source/JavaScriptCore/wasm/WasmSections.h:96
> +        && nextSectionId == static_cast<uint8_t>(Section::Code)) {

It should be `next == Section::Code`

-- 
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/20201216/bb082bcf/attachment-0001.htm>