[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
----------------------------------------------------------------------------
                 CC|                            |ysuzuki at apple.com
 Attachment #415626|review?                     |review-
              Flags|                            |

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

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

The direction looks good, but I found several issues.

> Source/JavaScriptCore/wasm/WasmAirIRGenerator.cpp:1163
> +    auto result = tmpForType(Type::I32);
> +    emitCCall(
> +        &operationWasmElemDrop, result, instanceValue(),
> +        addConstant(Type::I32, elementIndex));
> +
> +    emitCheck([&] {
> +        return Inst(BranchTest32, nullptr, Arg::resCond(MacroAssembler::Zero), result, result);
> +    }, [=] (CCallHelpers& jit, const B3::StackmapGenerationParams&) {
> +        this->emitThrowException(jit, ExceptionType::OutOfBoundsTableAccess);
> +    });
> +
> +    return { };

Do we need exception check here? Looks like elem_drop instruction never returns false.

> Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp:787
> +    auto result = m_currentBlock->appendNew<CCallValue>(
> +        m_proc, toB3Type(I32), origin(),
> +        m_currentBlock->appendNew<ConstPtrValue>(m_proc, origin(), tagCFunction<OperationPtrTag>(operationWasmElemDrop)),
> +        instanceValue(),
> +        m_currentBlock->appendNew<Const32Value>(m_proc, origin(), elementIndex));
> +
> +    {
> +        CheckValue* check = m_currentBlock->appendNew<CheckValue>(m_proc, Check, origin(),
> +            m_currentBlock->appendNew<Value>(m_proc, Equal, origin(), result, m_currentBlock->appendNew<Const32Value>(m_proc, origin(), 0)));
> +
> +        check->setGenerator([=] (CCallHelpers& jit, const B3::StackmapGenerationParams&) {
> +            this->emitExceptionCheck(jit, ExceptionType::OutOfBoundsTableAccess);
> +        });
> +    }
> +

Ditto.

> Source/JavaScriptCore/wasm/WasmInstance.cpp:168
> +const Element* Instance::elem(unsigned i) const

Let's name it `elementAt`.

> Source/JavaScriptCore/wasm/wasm.json:72
> +        "table.init":          { "category": "exttable",   "value":  252, "return": ["i32"],                         "parameter": ["i32", "i32", "i32"],          "immediate": [{"name": "element_index",  "type": "varuint32"}, {"name": "table_index","type": "varuint32"}],"description": "initialize table from a given element", "extendedOp": 12 },

Does `table.init` return "i32" value? Can you point at the spec what value should be returned?

> Source/JavaScriptCore/wasm/wasm.json:73
> +        "elem.drop":           { "category": "exttable",   "value":  252, "return": ["i32"],                         "parameter": [],                             "immediate": [{"name": "element_index",  "type": "varuint32"}],                                             "description": "prevents further use of a passive element segment", "extendedOp": 13 },

Ditto for elem.drop. And can you check the other table instructions too?

-- 
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/20201208/12ab780c/attachment.htm>