Bug 173349

Summary: REGRESSION: 15 new jsc failures in WPE and GTK+
Product: WebKit Reporter: Claudio Saavedra <csaavedra>
Component: JavaScriptCoreAssignee: Claudio Saavedra <csaavedra>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, jfbastien, ossy
Priority: P2    
Version: WebKit Local Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Claudio Saavedra
Reported 2017-06-14 00:26:32 PDT
The following tests started failing recently: wasm.yaml/wasm/function-tests/memory-alignment.js.default-wasm wasm.yaml/wasm/function-tests/memory-alignment.js.wasm-eager-jettison wasm.yaml/wasm/function-tests/memory-alignment.js.wasm-no-call-ic wasm.yaml/wasm/function-tests/memory-alignment.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/function-tests/memory-alignment.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/align.wast.js.default-wasm wasm.yaml/wasm/spec-tests/align.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/align.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/align.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/align.wast.js.wasm-no-tls-context wasm.yaml/wasm/spec-tests/memory.wast.js.default-wasm wasm.yaml/wasm/spec-tests/memory.wast.js.wasm-eager-jettison wasm.yaml/wasm/spec-tests/memory.wast.js.wasm-no-call-ic wasm.yaml/wasm/spec-tests/memory.wast.js.wasm-no-cjit-yes-tls-context wasm.yaml/wasm/spec-tests/memory.wast.js.wasm-no-tls-context The failures seem to have been introduced after https://trac.webkit.org/changeset/218216/webkit
Attachments
Patch (2.90 KB, patch)
2017-06-14 10:13 PDT, Claudio Saavedra 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
JF Bastien
Comment 1 2017-06-14 09:23:24 PDT
Interesting. What happens when you run the tests manually? Something like: cd JSTests/wasm && jsc -m function-tests/memory-alignment.js I definitely fixed alignment checking in that patch, but it should all pass!
Claudio Saavedra
Comment 2 2017-06-14 09:27:27 PDT
$ ../../WebKitBuild/Release/bin/jsc -m function-tests/memory-alignment.js Exception: Error: Expected to throw a CompileError with message "WebAssembly.Module doesn't parse at byte 5 / 8: byte alignment 2 exceeds load's natural alignment 1, in function at index 0" _fail@/home/claudio/git/metrological/WebKit/JSTests/wasm/assert.js:27:20 _throws@/home/claudio/git/metrological/WebKit/JSTests/wasm/assert.js:135:10 module code@/home/claudio/git/metrological/WebKit/JSTests/wasm/function-tests/memory-alignment.js:45:26 evaluate@[native code] moduleEvaluation@[native code] [native code] promiseReactionJob@[native code]
JF Bastien
Comment 3 2017-06-14 09:37:50 PDT
That's odd, this part of the change should handle that failing case: https://trac.webkit.org/changeset/218216/webkit#file66 Maybe the bots didn't re-generateWasmOps.h during incremental build? Maybe the cmake build is missing a dependency on generateWasm.py.
Claudio Saavedra
Comment 4 2017-06-14 09:44:13 PDT
I removed WasmOps.h and after building, the generated file was different. Now running the same test has no output, so I assume it's passing. You're probably right that a dependency is missing.
JF Bastien
Comment 5 2017-06-14 09:52:35 PDT
(In reply to Claudio Saavedra from comment #4) > I removed WasmOps.h and after building, the generated file was different. > Now running the same test has no output, so I assume it's passing. You're > probably right that a dependency is missing. Ah good. Could you take a look at the cmake dependency issue? The system I'm on right now is unhappy with cmake so I can't test it.
Claudio Saavedra
Comment 6 2017-06-14 09:59:06 PDT
Yes, I'm doing that.
JF Bastien
Comment 7 2017-06-14 10:05:44 PDT
(In reply to Claudio Saavedra from comment #6) > Yes, I'm doing that. Actually it looks like it's not only a cmake problem: https://bugs.webkit.org/show_bug.cgi?id=173287 I think Source/JavaScriptCore/DerivedSources.make has an issue (but maybe Source/JavaScriptCore/CMakeLists.txt does too).
Claudio Saavedra
Comment 8 2017-06-14 10:13:02 PDT
Created attachment 312902 [details] Patch
Claudio Saavedra
Comment 9 2017-06-14 10:14:01 PDT
(In reply to JF Bastien from comment #7) > (In reply to Claudio Saavedra from comment #6) > > Yes, I'm doing that. > > I think Source/JavaScriptCore/DerivedSources.make has an issue (but maybe > Source/JavaScriptCore/CMakeLists.txt does too). The above patch seems enough to fix the issue in CMake.
JF Bastien
Comment 10 2017-06-14 10:17:35 PDT
Comment on attachment 312902 [details] Patch Great, thanks!
JF Bastien
Comment 11 2017-06-14 11:06:23 PDT
Ossy, could you wipe WasmOps.h from all Linux bots? They'll fail until at least this file is rebuilt.
WebKit Commit Bot
Comment 12 2017-06-14 11:09:39 PDT
Comment on attachment 312902 [details] Patch Clearing flags on attachment: 312902 Committed r218272: <http://trac.webkit.org/changeset/218272>
WebKit Commit Bot
Comment 13 2017-06-14 11:09:40 PDT
All reviewed patches have been landed. Closing bug.
Claudio Saavedra
Comment 14 2017-06-14 12:32:07 PDT
(In reply to JF Bastien from comment #11) > Ossy, could you wipe WasmOps.h from all Linux bots? They'll fail until at > least this file is rebuilt. We did for the GTK+ and WPE bots already.
Csaba Osztrogonác
Comment 15 2017-06-14 23:24:48 PDT
(In reply to JF Bastien from comment #11) > Ossy, could you wipe WasmOps.h from all Linux bots? They'll fail until at > least this file is rebuilt. I triggered clean build on the AArch64 bot, it is happy now, thanks.
Note You need to log in before you can comment on or make changes to this bug.