Bug 166616

Summary: WebAssembly: Some loads don't take into account the offset
Product: WebKit Reporter: Saam Barati <saam>
Component: JavaScriptCoreAssignee: Saam Barati <saam>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, fpizlo, ggaren, gskachkov, jfbastien, keith_miller, mark.lam, msaboff, oliver, ticaiolima, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
patch
none
patch keith_miller: review+

Description Saam Barati 2016-12-29 14:20:50 PST 
...
Comment 1 Saam Barati 2016-12-29 16:51:15 PST 
This can be seen my looking at emitLoadOp. This is part of the reason we can't unity. However, there are still other errors I'm hitting after fixing this locally.
Comment 2 Radar WebKit Bug Importer 2017-01-02 11:46:13 PST 
<rdar://problem/29841541>
Comment 3 Saam Barati 2017-01-02 12:13:28 PST 
Created attachment 297906 [details]
patch
Comment 4 JF Bastien 2017-01-02 14:06:09 PST 
Comment on attachment 297906 [details]
patch

lgtm
Comment 5 JF Bastien 2017-01-02 14:06:26 PST 
Forgot to ask: are there new spec tests that pass?
Comment 6 Saam Barati 2017-01-02 14:24:51 PST 
(In reply to comment #5)
> Forgot to ask: are there new spec tests that pass?

I didn't try all of them, but memory.wast.js still fails. I can check all of them.
Comment 7 Saam Barati 2017-01-02 15:40:46 PST 
Created attachment 297914 [details]
patch

Two more tests pass for me locally, but I'm not convinced they're related to this patch. They could be tests we forgot to update.
Comment 8 Keith Miller 2017-01-02 15:47:18 PST 
Comment on attachment 297914 [details]
patch

r=me if you add a test for each load.
Comment 9 Saam Barati 2017-01-02 16:33:46 PST 
landed in:
https://trac.webkit.org/changeset/210228
with tests for all loads that were missing offset.