Bug 223695

Summary: Metal ANGLE crashes LayoutTests/inspector/canvas/updateShader-webgl.html
Product: WebKit Reporter: Kyle Piddington <kpiddington>
Component: ANGLEAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, dino, ews-watchlist, graouts, jdarpinian, kbr, kkinnunen, kondapallykalyan, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
ews-feeder: commit-queue-
Patch none

Description Kyle Piddington 2021-03-24 09:32:24 PDT
Metal ANGLE is crashing in this test (LayoutTests/inspector/canvas/updateShader-webgl.html), due to a bad NSDictionary. 
Switch to using allocated dictionaries, rather than stack constants.
Comment 1 Kyle Piddington 2021-03-24 09:33:58 PDT
Created attachment 424141 [details]
Patch
Comment 2 EWS Watchlist 2021-03-24 09:35:20 PDT
Note that there are important steps to take when updating ANGLE. See https://trac.webkit.org/wiki/UpdatingANGLE
Comment 3 Alexey Proskuryakov 2021-03-24 13:12:45 PDT
Comment on attachment 424141 [details]
Patch

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

> Source/ThirdParty/ANGLE/ChangeLog:4
> +Switch to using allocated dictionaries, rather than stack constants.

What is the exact mechanism for this failure? Literals are not stack constants, and should generally work everywhere.

I wonder if this is covering for an over-release somewhere with a leak.

> Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/ProgramMtl.mm:251
> -        mDefaultSubstitutionDictionary = @{@"TRANSFORM_FEEDBACK_ENABLED": @"0"};
> +        mDefaultSubstitutionDictionary = [[NSDictionary alloc] initWithDictionary:@{@"TRANSFORM_FEEDBACK_ENABLED": @"0"}];

There is a code path above that also initializes mDefaultSubstitutionDictionary with a literal. Worth updating that for consistency?
Comment 4 Kenneth Russell 2021-03-24 13:16:51 PDT
I'd like to defer review to ap@ who clearly understands this code better than me, but happy to study and officially review if needed.
Comment 5 Kenneth Russell 2021-03-24 13:17:30 PDT
Also: possible to add the crashing stack trace as a comment here for posterity?
Comment 6 Alexey Proskuryakov 2021-03-24 13:36:35 PDT
An ASan report would likely be best for understanding what leads to the crash.
Comment 7 Kyle Piddington 2021-03-24 16:46:43 PDT
Created attachment 424200 [details]
Patch
Comment 8 Kyle Piddington 2021-03-24 17:03:58 PDT
Created attachment 424202 [details]
Patch
Comment 9 EWS 2021-03-24 18:06:23 PDT
Committed r274990: <https://commits.webkit.org/r274990>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 424202 [details].
Comment 10 Radar WebKit Bug Importer 2021-03-24 18:07:16 PDT
<rdar://problem/75815100>