rx::angle::Result ProgramMtl::getSpecializedShader() leaks MTLFunctionConstantValues object due early return when an error occurs in ANGLE_TRY() macro. The early return occurs before the object is autoreleased: // Create Metal shader object ANGLE_MTL_OBJC_SCOPE { ANGLE_TRY(CreateMslShader(context, translatedMslInfo->metalLibrary, SHADER_ENTRY_NAME, funcConstants, &shaderVariant->metalShader)); [funcConstants ANGLE_MTL_AUTORELEASE]; } Found by clang static analyzer. Regressed with this commit: Roll ANGLE to include upstreamed Metal backend <https://bugs.webkit.org/show_bug.cgi?id=220896> <rdar://problem/73539682> <https://commits.webkit.org/r286603>
<rdar://problem/87656387>
(In reply to David Kilzer (:ddkilzer) from comment #0) > Regressed with this commit: > > Roll ANGLE to include upstreamed Metal backend > <https://bugs.webkit.org/show_bug.cgi?id=220896> > <rdar://problem/73539682> > <https://commits.webkit.org/r286603> Huh, that commit actually reverted some changes I made in r281160 to fix this leak earlier! [Metal ANGLE] Fix over-autorelease of rx::DisplayMtl::getMetalDeviceMatchingAttribute() and various Objective-C leak s <https://webkit.org/b/229128> <rdar://problem/81964007> <https://commits.webkit.org/r281160>
Created attachment 449288 [details] Patch v1
Note that there are important steps to take when updating ANGLE. See https://trac.webkit.org/wiki/UpdatingANGLE
Comment on attachment 449288 [details] Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=449288&action=review We can all agree that autorelease is the worst > Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/ProgramMtl.mm:643 > + [funcConstants.get() setConstantValue:&emulateDiscard I don’t think this .get() is needed, same for the 5 more below, unless mtl::AutoObjCObj is different from RetainPtr in this respect.
Created attachment 449341 [details] Patch for landing
Comment on attachment 449288 [details] Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=449288&action=review >> Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/ProgramMtl.mm:643 >> + [funcConstants.get() setConstantValue:&emulateDiscard > > I don’t think this .get() is needed, same for the 5 more below, unless mtl::AutoObjCObj is different from RetainPtr in this respect. Yes! Will remove and land a follow-up fix for Bug 235278.
Committed r288105 (246119@main): <https://commits.webkit.org/246119@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 449341 [details].
David, Darin - FYI Kyle, myself and several other colleagues are actively working on resolving divergence between WebKit's copy of ANGLE, and upstream ANGLE. Filed: Upstream fix for MTLFunctionConstantValues leak in rx::ProgramMtl::getSpecializedShader() https://bugs.chromium.org/p/angleproject/issues/detail?id=6920