Bug 165977

Summary: WebAssembly: test imports and exports with 16-bit characters
Product: WebKit Reporter: JF Bastien <jfbastien>
Component: JavaScriptCoreAssignee: JF Bastien <jfbastien>
Status: RESOLVED FIXED    
Severity: Normal CC: buildbot, commit-queue, ggaren, jfbastien, keith_miller, mark.lam, msaboff, saam, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 161709    
Attachments:
Description Flags
patch
saam: review+, saam: commit-queue-
patch none

JF Bastien
Reported 2016-12-16 16:35:16 PST
The Builder should already handle these, but the parser uses consumeUTF8String which is suspicious.
Attachments
patch (36.32 KB, patch)
2017-06-07 17:13 PDT, JF Bastien 		saam: review+
saam: commit-queue-
DetailsFormatted DiffDiff
patch (36.32 KB, patch)
2017-06-07 21:04 PDT, JF Bastien 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Saam Barati
Comment 1 2016-12-16 16:36:30 PST
*** Bug 165976 has been marked as a duplicate of this bug. ***
Saam Barati
Comment 2 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.
Saam Barati
Comment 3 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.
Radar WebKit Bug Importer
Comment 4 2016-12-20 14:20:25 PST
<rdar://problem/29760130>
JF Bastien
Comment 5 2017-06-07 17:13:49 PDT
Created attachment 312256 [details] patch
Saam Barati
Comment 6 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
JF Bastien
Comment 7 2017-06-07 21:04:34 PDT
Created attachment 312275 [details] patch Use makeString.
WebKit Commit Bot
Comment 8 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>
WebKit Commit Bot
Comment 9 2017-06-07 21:43:56 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.