Bug 198355 – oss-fuzz: jsc: Issue 15016: jsc: Abrt in JSC::Wasm::AirIRGenerator::addLocal (15016)

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

[Justin Michaud]

oss-fuzz: jsc: Issue 15016: jsc: Abrt in JSC::Wasm::AirIRGenerator::addLocal (15016)

[Justin Michaud]

Created attachment 370900 [details]
Patch

[Justin Michaud]

<rdar://51218333>

[Yusuke Suzuki]

Comment on attachment 370900 [details]
Patch

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

r=me

> JSTests/wasm/references/is_null.js:18
> -        .Function("h", { params: ["anyref"], ret: "anyref" })
> +        .Function("h", { params: ["anyref"], ret: "anyref" }, ["anyref"])
>            .GetLocal(0)
> +          .SetLocal(1)
> +          .GetLocal(1)
>          .End()

Can we have a test that ensures anyref local is initialized with null?

[Saam Barati]

Comment on attachment 370900 [details]
Patch

Aren't we missing this in WasmB3IRGenerator too?

[Saam Barati]

Comment on attachment 370900 [details]
Patch

You need to change B3IRGenerator to also not emit zero for this local, and instead, jsNull().

[Justin Michaud]

Created attachment 370911 [details]
Patch

[Saam Barati]

Comment on attachment 370911 [details]
Patch

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

r=me

> JSTests/wasm/references/is_null.js:62
> +assert.eq(instance.exports.local_read(), 1)

This tests WasmB3IRGenerator because we sometimes run that as the lowest tier in our testing, right?

[Saam Barati]

Comment on attachment 370911 [details]
Patch

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

> Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp:537
> +        auto val = (type == Anyref ? JSValue::encode(jsNull()) : 0);

style nit: no need for outer parens here.

[Justin Michaud]

(In reply to Saam Barati from comment #7)
> Comment on attachment 370911 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=370911&action=review
> 
> r=me
> 
> > JSTests/wasm/references/is_null.js:62
> > +assert.eq(instance.exports.local_read(), 1)
> 
> This tests WasmB3IRGenerator because we sometimes run that as the lowest
> tier in our testing, right?

That is correct. I added a pass that runs it with wasmBBQUsesAir=0.

[Justin Michaud]

Created attachment 370914 [details]
Patch

[EWS Watchlist]

Comment on attachment 370914 [details]
Patch

Attachment 370914 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/12325383

New failing tests:
imported/blink/fast/canvas/canvas-state-persistence-no-dirty.html
imported/blink/compositing/layer-creation/iframe-clip-removed.html
http/tests/misc/slow-loading-animated-image.html
storage/indexeddb/index-cursor.html
js/slow-stress/variadic-closure-call.html

[EWS Watchlist]

Created attachment 370925 [details]
Archive of layout-test-results from ews211 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews211  Port: win-future  Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit

[Justin Michaud]

Comment on attachment 370914 [details]
Patch

Test failures look unrelated.

[WebKit Commit Bot]

Comment on attachment 370914 [details]
Patch

Rejecting attachment 370914 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-03', 'validate-changelog', '--check-oops', '--non-interactive', 370914, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

ChangeLog entry in JSTests/ChangeLog contains OOPS!.

Full output: https://webkit-queues.webkit.org/results/12329376

[Justin Michaud]

Created attachment 370950 [details]
Patch

ಠ_ಠ

[WebKit Commit Bot]

Comment on attachment 370950 [details]
Patch

Clearing flags on attachment: 370950

Committed r245895: <https://trac.webkit.org/changeset/245895>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.