Bug 165977 - WebAssembly: test imports and exports with 16-bit characters
Summary: WebAssembly: test imports and exports with 16-bit characters
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: JF Bastien
URL:
Keywords: InRadar
: 165976 (view as bug list)
Depends on:
Blocks: 161709
  Show dependency treegraph
 
Reported: 2016-12-16 16:35 PST by JF Bastien
Modified: 2017-06-07 21:43 PDT (History)
9 users (show)

See Also:


Attachments
patch (36.32 KB, patch)
2017-06-07 17:13 PDT, JF Bastien
saam: review+
saam: commit-queue-
Details | Formatted Diff | Diff
patch (36.32 KB, patch)
2017-06-07 21:04 PDT, JF Bastien
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description JF Bastien 2016-12-16 16:35:16 PST
The Builder should already handle these, but the parser uses consumeUTF8String which is suspicious.
Comment 1 Saam Barati 2016-12-16 16:36:30 PST
*** Bug 165976 has been marked as a duplicate of this bug. ***
Comment 2 Saam Barati 2016-12-16 16:38:52 PST
(In reply to comment #0)
> The Builder should already handle these, but the parser uses
> consumeUTF8String which is suspicious.

To elaborate, the implementation of consumeUTF8String looks wrong. Not the actual call to it. The implementation skips m_offset stringLength forward, which is wrong for 16 bit strings.
Comment 3 Saam Barati 2016-12-16 16:40:13 PST
(In reply to comment #2)
> (In reply to comment #0)
> > The Builder should already handle these, but the parser uses
> > consumeUTF8String which is suspicious.
> 
> To elaborate, the implementation of consumeUTF8String looks wrong. Not the
> actual call to it. The implementation skips m_offset stringLength forward,
> which is wrong for 16 bit strings.

Nevermind. We're treating a field of number of bytes from wasm as the *string length*. This is obviously wrong for 16 bit strings.
Comment 4 Radar WebKit Bug Importer 2016-12-20 14:20:25 PST
<rdar://problem/29760130>
Comment 5 JF Bastien 2017-06-07 17:13:49 PDT
Created attachment 312256 [details]
patch
Comment 6 Saam Barati 2017-06-07 17:22:33 PDT
Comment on attachment 312256 [details]
patch

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

r=me

> Source/JavaScriptCore/wasm/js/JSWebAssemblyInstance.cpp:141
> +        return before + String(" ") + String::fromUTF8(import.module) + ":" + String::fromUTF8(import.field) + " " + after;

you should use makeString here
Comment 7 JF Bastien 2017-06-07 21:04:34 PDT
Created attachment 312275 [details]
patch

Use makeString.
Comment 8 WebKit Commit Bot 2017-06-07 21:43:54 PDT
Comment on attachment 312275 [details]
patch

Clearing flags on attachment: 312275

Committed r217921: <http://trac.webkit.org/changeset/217921>
Comment 9 WebKit Commit Bot 2017-06-07 21:43:56 PDT
All reviewed patches have been landed.  Closing bug.