Bug 226468 – ANGLE Metal translator should clamp array access in MSL

• Status: RESOLVED
• Resolution: CONFIGURATION CHANGED
• Component: ANGLE

[Kimmo Kinnunen]

ANGLE Metal translator should clamp array access in MSL

[Kirtikumar Anandrao Ramchandani]

Looks like this issue was created after that, yes?

[Kimmo Kinnunen]

Created attachment 430200 [details]
Patch

[EWS Watchlist]

Note that there are important steps to take when updating ANGLE. See https://trac.webkit.org/wiki/UpdatingANGLE

[Kimmo Kinnunen]

(In reply to Kirtikumar Anandrao Ramchandani from comment #1)
> Looks like this issue was created after that, yes?

This bug report was created based on bug 226397 and after that, yes.

[Dean Jackson]

Can we write a test for this?

[Kirtikumar Anandrao Ramchandani]

We can write a test case to reach that particular piece of code, yes.

[Kimmo Kinnunen]

(In reply to Dean Jackson from comment #5)
> Can we write a test for this?

At the moment it's a bit cumbersome.
For manual inspection of shader text output we'd either need to:
a) add a text-dumping translator tester to ANGLE 
b) implement (the long overdue) per-gpu filtering possibility to RWT and use the layout test to dump the GL/MTL shader output to expected files

For draw inspection-based test it's hard to do since we don't know what to expect in the failure case. In practice we can't know that a success would indicate lack of bug.

This is tested by conformance/uniforms/out-of-bounds-uniform-array-access.html but it's unclear to what extent we can observe the bug. Obviously we cannot, as the test did not trigger.

I'll check if we can later turn on validating layer for Metal on Debug builds, but I doubt it will detect this either.

I'll put testing this in the "back burner" and take the liberty to merge without, as we have bigger issues in testing than this.

[Kimmo Kinnunen]

> b) implement (the long overdue) per-gpu filtering possibility to RWT and use the layout test to dump the GL/MTL shader output to expected files


Except, maybe it's doable since this the "MSL or GLSL or HLSL" decision is per platform in WebKit at the moment. I'll try to write the test.

[Radar WebKit Bug Importer]

<rdar://problem/78944685>

[Ahmad Saleem]

It seems this r+ patch didn't landed. Is it something still needed or being worked on? Thanks!

[Kimmo Kinnunen]

Thanks for the heads. I think I need to upstream this.

[Kimmo Kinnunen]

Already fixed in  GenMetalTraverser::visitBinary(Visit, TIntermBinary *binaryNode)