Diff
Modified: trunk/Source/ThirdParty/ANGLE/ChangeLog (281159 => 281160)
--- trunk/Source/ThirdParty/ANGLE/ChangeLog 2021-08-17 20:19:22 UTC (rev 281159)
+++ trunk/Source/ThirdParty/ANGLE/ChangeLog 2021-08-17 20:32:30 UTC (rev 281160)
@@ -1,3 +1,57 @@
+2021-08-17 David Kilzer <[email protected]>
+
+ [Metal ANGLE] Fix over-autorelease of rx::DisplayMtl::getMetalDeviceMatchingAttribute() and various Objective-C leaks
+ <https://webkit.org/b/229128>
+ <rdar://problem/81964007>
+
+ Reviewed by Alex Christensen.
+
+ * src/libANGLE/renderer/metal/DisplayMtl.h:
+ (rx::DisplayMtl::getMetalDeviceMatchingAttribute):
+ - Change to return mtl::AutoObjCPtr<> to make ownership clear.
+ (rx::DisplayMtl::mMetalDevice):
+ - No need to initialize to nil.
+
+ * src/libANGLE/renderer/metal/DisplayMtl.mm:
+ (rx::DisplayMtl::initializeImpl):
+ - Update for changes to getMetalDeviceMatchingAttribute().
+ (rx::DisplayMtl::getMetalDeviceMatchingAttribute):
+ - Change to return mtl::AutoObjCPtr<> to make ownership clear.
+ - Fix leak of `deviceList`, `externalGPUs`, `integratedGPUs`, and
+ `discreteGPUs`.
+ - Use mtl::adoptObjCObj<>() to prevent leak of
+ MTLCreateSystemDefaultDevice().
+
+ * src/libANGLE/renderer/metal/IOSurfaceSurfaceMtl.mm:
+ - Fix leak of `captureDescriptor` in two different if blocks.
+
+ * src/libANGLE/renderer/metal/ProgramMtl.mm:
+ - Fix leak of `funcConstants` in early return on error path.
+
+ * src/libANGLE/renderer/metal/ProvokingVertexHelper.mm:
+ (rx::ProvokingVertexHelper::getSpecializedShader):
+ - Fix leak of `fcValues`.
+
+ * src/libANGLE/renderer/metal/SurfaceMtl.mm:
+ - Fix leak of `captureDescriptor` in two different if blocks.
+
+ * src/libANGLE/renderer/metal/mtl_common.h:
+ (rx::mtl::WrappedObject::retainAssign):
+ - Move statement inside #if/#endif that isn't needed for ARC.
+ (rx::mtl::WrappedObject::unretainAssign): Add.
+ (rx::mtl::AutoObjCPtr::AutoObjCPtr): Add.
+ (rx::mtl::adoptObjCObj): Add.
+ - Add a helper method to adopt an Objective-C object to
+ eliminate the need to autorelease a +1 retained object before
+ an mtl::AutoObjCPtr<> object wraps it. Modeled after
+ WTF::RetainPtr<> in WebKit.
+
+ * src/libANGLE/renderer/metal/mtl_state_cache.mm:
+ (rx::mtl::RenderPipelineCache::createRenderPipelineState):
+ (rx::mtl::ProvokingVertexComputePipelineCache::createComputePipelineState):
+ - Use adoptObjCObj<>() to fix potential leak on the early return
+ path since these methods return an mtl::AutoObjCPtr<>.
+
2021-08-16 David Kilzer <[email protected]>
"make analyze" should run clang static analyzer in deep mode
Modified: trunk/Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/DisplayMtl.h (281159 => 281160)
--- trunk/Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/DisplayMtl.h 2021-08-17 20:19:22 UTC (rev 281159)
+++ trunk/Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/DisplayMtl.h 2021-08-17 20:32:30 UTC (rev 281160)
@@ -175,10 +175,10 @@
void initializeFeatures();
void initializeLimitations();
EGLenum EGLDrawingBufferTextureTarget();
- id<MTLDevice> getMetalDeviceMatchingAttribute(const egl::AttributeMap &attribs);
+ mtl::AutoObjCPtr<id<MTLDevice>> getMetalDeviceMatchingAttribute(const egl::AttributeMap &attribs);
angle::Result initializeShaderLibrary();
- mtl::AutoObjCPtr<id<MTLDevice>> mMetalDevice = nil;
+ mtl::AutoObjCPtr<id<MTLDevice>> mMetalDevice;
uint32_t mMetalDeviceVendorId = 0;
mtl::CommandQueue mCmdQueue;
Modified: trunk/Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/DisplayMtl.mm (281159 => 281160)
--- trunk/Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/DisplayMtl.mm 2021-08-17 20:19:22 UTC (rev 281159)
+++ trunk/Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/DisplayMtl.mm 2021-08-17 20:32:30 UTC (rev 281160)
@@ -139,7 +139,7 @@
{
ANGLE_MTL_OBJC_SCOPE
{
- mMetalDevice = [getMetalDeviceMatchingAttribute(display->getAttributeMap()) ANGLE_MTL_AUTORELEASE];
+ mMetalDevice = getMetalDeviceMatchingAttribute(display->getAttributeMap());
//If we can't create a device, fail initialization.
if(!mMetalDevice.get())
{
@@ -244,14 +244,14 @@
}
-id<MTLDevice> DisplayMtl::getMetalDeviceMatchingAttribute(const egl::AttributeMap &attribs)
+mtl::AutoObjCPtr<id<MTLDevice>> DisplayMtl::getMetalDeviceMatchingAttribute(const egl::AttributeMap &attribs)
{
#if defined(ANGLE_PLATFORM_MACOS) || defined(ANGLE_PLATFORM_MACCATALYST)
const std::string anglePreferredDevice = angle::GetEnvironmentVar(kANGLEPreferredDeviceEnv);
- NSArray<id<MTLDevice>> *deviceList = MTLCopyAllDevices();
+ auto deviceList = mtl::adoptObjCObj(MTLCopyAllDevices());
if (anglePreferredDevice != "")
{
- for (id<MTLDevice> device in deviceList)
+ for (id<MTLDevice> device in deviceList.get())
{
if ([device.name.lowercaseString
containsString:[NSString stringWithUTF8String:anglePreferredDevice.c_str()]
@@ -258,16 +258,16 @@
.lowercaseString])
{
NSLog(@"Using Metal Device: %@", [device name]);
- return device;
+ return device;
break;
}
}
}
- NSMutableArray<id<MTLDevice>> *externalGPUs = [[NSMutableArray alloc] init];
- NSMutableArray<id<MTLDevice>> *integratedGPUs = [[NSMutableArray alloc] init];
- NSMutableArray<id<MTLDevice>> *discreteGPUs = [[NSMutableArray alloc] init];
- for (id <MTLDevice> device in deviceList) {
+ auto externalGPUs = mtl::adoptObjCObj<NSMutableArray<id<MTLDevice>>>([[NSMutableArray alloc] init]);
+ auto integratedGPUs = mtl::adoptObjCObj<NSMutableArray<id<MTLDevice>>>([[NSMutableArray alloc] init]);
+ auto discreteGPUs = mtl::adoptObjCObj<NSMutableArray<id<MTLDevice>>>([[NSMutableArray alloc] init]);
+ for (id <MTLDevice> device in deviceList.get()) {
if (device.removable)
{
[externalGPUs addObject:device];
@@ -286,22 +286,22 @@
if (attribs.get(EGL_POWER_PREFERENCE_ANGLE, EGL_LOW_POWER_ANGLE) == EGL_HIGH_POWER_ANGLE)
{
//Search for a discrete GPU first.
- for (id<MTLDevice> device in discreteGPUs) {
+ for (id<MTLDevice> device in discreteGPUs.get()) {
if(![device isHeadless])
return device;
}
}
//If we've selected a low power device, look through integrated devices.
- for (id<MTLDevice> device in integratedGPUs) {
+ for (id<MTLDevice> device in integratedGPUs.get()) {
if(![device isHeadless])
return device;
}
//If we selected a low power device and there's no low-power devices avaialble, return the first (default) device.
- if(deviceList.count > 0)
+ if (deviceList.get().count > 0)
return deviceList[0];
#endif
//If we can't find anything, or are on a platform that doesn't support power options, create a default device.
- return MTLCreateSystemDefaultDevice();
+ return mtl::adoptObjCObj(MTLCreateSystemDefaultDevice());
}
Modified: trunk/Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/IOSurfaceSurfaceMtl.mm (281159 => 281160)
--- trunk/Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/IOSurfaceSurfaceMtl.mm 2021-08-17 20:19:22 UTC (rev 281159)
+++ trunk/Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/IOSurfaceSurfaceMtl.mm 2021-08-17 20:32:30 UTC (rev 281160)
@@ -168,15 +168,15 @@
# ifdef __MAC_10_15
if (ANGLE_APPLE_AVAILABLE_XCI(10.15, 13.0, 13))
{
- MTLCaptureDescriptor *captureDescriptor = [[MTLCaptureDescriptor alloc] init];
- captureDescriptor.captureObject = metalDevice;
- const std::string filePath = GetMetalCaptureFile();
+ auto captureDescriptor = mtl::adoptObjCObj([[MTLCaptureDescriptor alloc] init]);
+ captureDescriptor.get().captureObject = metalDevice;
+ const std::string filePath = GetMetalCaptureFile();
if (filePath != "")
{
const std::string numberedPath =
filePath + std::to_string(gFrameCaptured - 1) + ".gputrace";
- captureDescriptor.destination = MTLCaptureDestinationGPUTraceDocument;
- captureDescriptor.outputURL =
+ captureDescriptor.get().destination = MTLCaptureDestinationGPUTraceDocument;
+ captureDescriptor.get().outputURL =
[NSURL fileURLWithPath:[NSString stringWithUTF8String:numberedPath.c_str()]
isDirectory:false];
}
@@ -183,11 +183,11 @@
else
{
// This will pause execution only if application is being debugged inside Xcode
- captureDescriptor.destination = MTLCaptureDestinationDeveloperTools;
+ captureDescriptor.get().destination = MTLCaptureDestinationDeveloperTools;
}
NSError *error;
- if (![captureManager startCaptureWithDescriptor:captureDescriptor error:&error])
+ if (![captureManager startCaptureWithDescriptor:captureDescriptor.get() error:&error])
{
NSLog(@"Failed to start capture, error %@", error);
}
@@ -196,11 +196,11 @@
# endif // __MAC_10_15
if (ANGLE_APPLE_AVAILABLE_XCI(10.15, 13.0, 13))
{
- MTLCaptureDescriptor *captureDescriptor = [[MTLCaptureDescriptor alloc] init];
- captureDescriptor.captureObject = metalDevice;
+ auto captureDescriptor = mtl::adoptObjCObj([[MTLCaptureDescriptor alloc] init]);
+ captureDescriptor.get().captureObject = metalDevice;
NSError *error;
- if (![captureManager startCaptureWithDescriptor:captureDescriptor error:&error])
+ if (![captureManager startCaptureWithDescriptor:captureDescriptor.get() error:&error])
{
NSLog(@"Failed to start capture, error %@", error);
}
Modified: trunk/Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/ProgramMtl.mm (281159 => 281160)
--- trunk/Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/ProgramMtl.mm 2021-08-17 20:19:22 UTC (rev 281159)
+++ trunk/Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/ProgramMtl.mm 2021-08-17 20:32:30 UTC (rev 281160)
@@ -598,7 +598,7 @@
mtl::TranslatedShaderInfo *translatedMslInfo = &mMslShaderTranslateInfo[shaderType];
ProgramShaderObjVariantMtl *shaderVariant;
- MTLFunctionConstantValues *funcConstants = nil;
+ mtl::AutoObjCObj<MTLFunctionConstantValues> funcConstants;
if (shaderType == gl::ShaderType::Vertex)
{
@@ -634,7 +634,7 @@
GetRasterizationDiscardEnabledConstName()];
}
- funcConstants = [[MTLFunctionConstantValues alloc] init];
+ funcConstants = mtl::adoptObjCObj([[MTLFunctionConstantValues alloc] init]);
[funcConstants setConstantValue:&emulateDiscard
type:MTLDataTypeBool
withName:discardEnabledStr];
@@ -672,7 +672,7 @@
GetCoverageMaskEnabledConstName()];
}
- funcConstants = [[MTLFunctionConstantValues alloc] init];
+ funcConstants = mtl::adoptObjCObj([[MTLFunctionConstantValues alloc] init]);
[funcConstants setConstantValue:&emulateCoverageMask
type:MTLDataTypeBool
withName:coverageMaskEnabledStr];
@@ -694,9 +694,8 @@
// Create Metal shader object
ANGLE_MTL_OBJC_SCOPE
{
- ANGLE_TRY(CreateMslShader(context, translatedMslInfo->metalLibrary, SHADER_ENTRY_NAME, funcConstants,
+ ANGLE_TRY(CreateMslShader(context, translatedMslInfo->metalLibrary, SHADER_ENTRY_NAME, funcConstants.get(),
&shaderVariant->metalShader));
- [funcConstants ANGLE_MTL_AUTORELEASE];
}
// Store reference to the translated source for easily querying mapped bindings later.
Modified: trunk/Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/ProvokingVertexHelper.mm (281159 => 281160)
--- trunk/Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/ProvokingVertexHelper.mm 2021-08-17 20:19:22 UTC (rev 281159)
+++ trunk/Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/ProvokingVertexHelper.mm 2021-08-17 20:32:30 UTC (rev 281160)
@@ -174,11 +174,12 @@
id<MTLFunction> *shaderOut)
{
uint indexBufferKey = buildIndexBufferKey(pipelineDesc);
- MTLFunctionConstantValues * fcValues = [[MTLFunctionConstantValues alloc]init];
+ auto fcValues = mtl::adoptObjCObj([[MTLFunctionConstantValues alloc] init]);
[fcValues setConstantValue:&indexBufferKey type:MTLDataTypeUInt withName:@"fixIndexBufferKey"];
- return CreateMslShader(context, mProvokingVertexLibrary, @"fixIndexBuffer", fcValues, shaderOut);
+ return CreateMslShader(context, mProvokingVertexLibrary, @"fixIndexBuffer", fcValues.get(), shaderOut);
}
+
//Private command buffer
bool ProvokingVertexHelper::hasSpecializedShader(gl::ShaderType shaderType,
const mtl::ProvokingVertexComputePipelineDesc &renderPipelineDesc)
Modified: trunk/Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/SurfaceMtl.mm (281159 => 281160)
--- trunk/Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/SurfaceMtl.mm 2021-08-17 20:19:22 UTC (rev 281159)
+++ trunk/Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/SurfaceMtl.mm 2021-08-17 20:32:30 UTC (rev 281160)
@@ -177,15 +177,15 @@
# ifdef __MAC_10_15
if (ANGLE_APPLE_AVAILABLE_XCI(10.15, 13.0, 13))
{
- MTLCaptureDescriptor *captureDescriptor = [[MTLCaptureDescriptor alloc] init];
- captureDescriptor.captureObject = metalDevice;
- const std::string filePath = GetMetalCaptureFile();
+ auto captureDescriptor = mtl::adoptObjCObj([[MTLCaptureDescriptor alloc] init]);
+ captureDescriptor.get().captureObject = metalDevice;
+ const std::string filePath = GetMetalCaptureFile();
if (filePath != "")
{
const std::string numberedPath =
filePath + std::to_string(gFrameCaptured - 1) + ".gputrace";
- captureDescriptor.destination = MTLCaptureDestinationGPUTraceDocument;
- captureDescriptor.outputURL =
+ captureDescriptor.get().destination = MTLCaptureDestinationGPUTraceDocument;
+ captureDescriptor.get().outputURL =
[NSURL fileURLWithPath:[NSString stringWithUTF8String:numberedPath.c_str()]
isDirectory:false];
}
@@ -192,11 +192,11 @@
else
{
// This will pause execution only if application is being debugged inside Xcode
- captureDescriptor.destination = MTLCaptureDestinationDeveloperTools;
+ captureDescriptor.get().destination = MTLCaptureDestinationDeveloperTools;
}
NSError *error;
- if (![captureManager startCaptureWithDescriptor:captureDescriptor error:&error])
+ if (![captureManager startCaptureWithDescriptor:captureDescriptor.get() error:&error])
{
NSLog(@"Failed to start capture, error %@", error);
}
@@ -205,11 +205,11 @@
# endif // __MAC_10_15
if (ANGLE_APPLE_AVAILABLE_XCI(10.15, 13.0, 13))
{
- MTLCaptureDescriptor *captureDescriptor = [[MTLCaptureDescriptor alloc] init];
- captureDescriptor.captureObject = metalDevice;
+ auto captureDescriptor = mtl::adoptObjCObj([[MTLCaptureDescriptor alloc] init]);
+ captureDescriptor.get().captureObject = metalDevice;
NSError *error;
- if (![captureManager startCaptureWithDescriptor:captureDescriptor error:&error])
+ if (![captureManager startCaptureWithDescriptor:captureDescriptor.get() error:&error])
{
NSLog(@"Failed to start capture, error %@", error);
}
Modified: trunk/Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/mtl_common.h (281159 => 281160)
--- trunk/Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/mtl_common.h 2021-08-17 20:19:22 UTC (rev 281159)
+++ trunk/Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/mtl_common.h 2021-08-17 20:32:30 UTC (rev 281160)
@@ -208,8 +208,8 @@
void retainAssign(T obj)
{
+#if !__has_feature(objc_arc)
T retained = obj;
-#if !__has_feature(objc_arc)
[retained retain];
#endif
release();
@@ -216,6 +216,12 @@
mMetalObject = obj;
}
+ void unretainAssign(T obj)
+ {
+ release();
+ mMetalObject = obj;
+ }
+
private:
void release()
{
@@ -228,6 +234,21 @@
T mMetalObject = nil;
};
+// Because ARC enablement is a compile-time choice, and we compile this header
+// both ways, we need a separate copy of our code when ARC is enabled.
+#if __has_feature(objc_arc)
+#define adoptObjCObj adoptObjCObjArc
+#endif
+
+template <typename T>
+class AutoObjCPtr;
+
+template <typename T>
+using AutoObjCObj = AutoObjCPtr<T *>;
+
+template <typename U>
+AutoObjCObj<U> adoptObjCObj(U* NS_RELEASES_ARGUMENT) __attribute__((__warn_unused_result__));
+
// This class is similar to WrappedObject, however, it allows changing the
// internal pointer with public methods.
template <typename T>
@@ -293,7 +314,13 @@
using ParentType::retainAssign;
+ template <typename U>
+ friend AutoObjCObj<U> adoptObjCObj(U* NS_RELEASES_ARGUMENT) __attribute__((__warn_unused_result__));
+
private:
+ enum AdoptTag { Adopt };
+ AutoObjCPtr(T src, AdoptTag) { this->unretainAssign(src); }
+
void transfer(AutoObjCPtr &&src)
{
this->retainAssign(std::move(src.get()));
@@ -301,9 +328,19 @@
}
};
-template <typename T>
-using AutoObjCObj = AutoObjCPtr<T *>;
+template <typename U>
+inline AutoObjCObj<U> adoptObjCObj(U* NS_RELEASES_ARGUMENT src)
+{
+#if __has_feature(objc_arc)
+ return src;
+#elif defined(OBJC_NO_GC)
+ return AutoObjCPtr<U*>(src, AutoObjCPtr<U*>::Adopt);
+#else
+#error "ObjC GC not supported."
+#endif
+}
+
// NOTE: SharedEvent is only declared on iOS 12.0+ or mac 10.14+
#if defined(__IPHONE_12_0) || defined(__MAC_10_14)
#define ANGLE_MTL_EVENT_AVAILABLE 1
Modified: trunk/Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/mtl_state_cache.mm (281159 => 281160)
--- trunk/Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/mtl_state_cache.mm 2021-08-17 20:19:22 UTC (rev 281159)
+++ trunk/Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/mtl_state_cache.mm 2021-08-17 20:32:30 UTC (rev 281160)
@@ -1041,8 +1041,8 @@
}
// Create pipeline state
NSError *err = nil;
- id<MTLRenderPipelineState> newState =
- [metalDevice newRenderPipelineStateWithDescriptor:objCDesc error:&err];
+ auto newState =
+ adoptObjCObj([metalDevice newRenderPipelineStateWithDescriptor:objCDesc error:&err]);
if (err)
{
context->handleError(err, __FILE__, ANGLE_FUNCTION, __LINE__);
@@ -1049,7 +1049,7 @@
return nil;
}
- return [newState ANGLE_MTL_AUTORELEASE];
+ return newState;
}
}
@@ -1233,8 +1233,8 @@
// Convert to Objective-C desc:
NSError *err = nil;
- id<MTLComputePipelineState> newState =
- [metalDevice newComputePipelineStateWithFunction:computeFunction error:&err];
+ auto newState =
+ adoptObjCObj([metalDevice newComputePipelineStateWithFunction:computeFunction error:&err]);
if (err)
{
context->handleError(err, __FILE__, ANGLE_FUNCTION, __LINE__);
@@ -1241,7 +1241,7 @@
return nil;
}
- return [newState ANGLE_MTL_AUTORELEASE];
+ return newState;
}
}
