[Webkit-unassigned] [Bug 147293] Implement WebAssembly module parser

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Jul 27 22:46:25 PDT 2015


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

--- Comment #7 from Sukolsak Sakshuwong <sukolsak at gmail.com> ---
Thank you for the review.

(In reply to comment #3)
> Comment on attachment 257558 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=257558&action=review
> 
> r=me with some mixups
> 
> > Source/JavaScriptCore/parser/SourceProvider.h:90
> > +#if ENABLE(WEBASSEMBLY)
> > +        virtual bool isStringSource() const override
> > +        {
> > +            return true;
> > +        }
> > +#endif
> 
> SourceProvider has a subclass named "StringSourceProvider". This function is
> too easily confused with that class. 
> 
> For now, I think you should remove this function. Instead of performing an
> isStringSource() check, I think you should just go with the [WebAssembly
> Source] source string you've provided. In the future, we'll have to see what
> the spec says about toString on a WebAssembly function.

I originally named it isStringSourceProvider() but renamed it to isStringSource() after realizing that OpaqueJSScript was another subclass of SourceProvider that should have this method return true.

The reason I used this method was that SourceCode::toUTF8() returns "m_provider->source().impl()->utf8ForRange(m_startChar, m_endChar - m_startChar)". This will crash if source() is "[WebAssembly source]" and m_startChar and m_endChar are out of range. Come to think of it, maybe I shouldn't have m_startChar and m_endChar out of range in the first place.

> > Source/JavaScriptCore/wasm/WASMFormat.h:33
> > +static const uint32_t wasmMagicNumber = 0x6d736177;
> 
> This file should be named WASMMagicNumber.h.

This file will have more constants and enums that define the WASM file format. It will be very similar to <https://github.com/WebAssembly/polyfill-prototype-1/blob/master/src/shared.h> Should I rename it to WASMMagicNumber.h for now? Or maybe WASMConstants.h?

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20150728/2939e991/attachment-0001.html>


More information about the webkit-unassigned mailing list