Bug 173349 – REGRESSION: 15 new jsc failures in WPE and GTK+

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Claudio Saavedra]

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

[JF Bastien]

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]

$ ../../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]

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]

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]

(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]

Yes, I'm doing that.

[JF Bastien]

(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]

Created attachment 312902 [details]
Patch

[Claudio Saavedra]

(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 on attachment 312902 [details]
Patch

Great, thanks!

[JF Bastien]

Ossy, could you wipe WasmOps.h from all Linux bots? They'll fail until at least this file is rebuilt.

[WebKit Commit Bot]

Comment on attachment 312902 [details]
Patch

Clearing flags on attachment: 312902

Committed r218272: <http://trac.webkit.org/changeset/218272>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Claudio Saavedra]

(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]

(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.