diff --git a/filament/backend/include/backend/Platform.h b/filament/backend/include/backend/Platform.h index b8befb4cee6..af18b1488a4 100644 --- a/filament/backend/include/backend/Platform.h +++ b/filament/backend/include/backend/Platform.h @@ -68,13 +68,6 @@ class UTILS_PUBLIC Platform { */ size_t handleArenaSize = 0; - /** - * This number of most-recently destroyed textures will be tracked for use-after-free. - * Throws an exception when a texture is freed but still bound to a SamplerGroup and used in - * a draw call. 0 disables completely. Currently only respected by the Metal backend. - */ - size_t textureUseAfterFreePoolSize = 0; - size_t metalUploadBufferSizeBytes = 512 * 1024; /** diff --git a/filament/backend/src/metal/MetalContext.h b/filament/backend/src/metal/MetalContext.h index 3f6cbaa1a95..ef1175fa688 100644 --- a/filament/backend/src/metal/MetalContext.h +++ b/filament/backend/src/metal/MetalContext.h @@ -118,9 +118,6 @@ class MetalDynamicOffsets { }; struct MetalContext { - explicit MetalContext(size_t metalFreedTextureListSize) - : texturesToDestroy(metalFreedTextureListSize) {} - MetalDriver* driver; id device = nullptr; id commandQueue = nullptr; @@ -187,14 +184,6 @@ struct MetalContext { // Keeps track of all alive textures. tsl::robin_set textures; - // This circular buffer implements delayed destruction for Metal texture handles. It keeps a - // handle to a fixed number of the most recently destroyed texture handles. When we're asked to - // destroy a texture handle, we free its texture memory, but keep the MetalTexture object alive, - // marking it as "terminated". If we later are asked to use that texture, we can check its - // terminated status and throw an Objective-C error instead of crashing, which is helpful for - // debugging use-after-free issues in release builds. - utils::FixedCircularBuffer> texturesToDestroy; - MetalBufferPool* bufferPool; MetalBumpAllocator* bumpAllocator; diff --git a/filament/backend/src/metal/MetalDriver.mm b/filament/backend/src/metal/MetalDriver.mm index e73e597c56f..84c980e92a8 100644 --- a/filament/backend/src/metal/MetalDriver.mm +++ b/filament/backend/src/metal/MetalDriver.mm @@ -105,13 +105,13 @@ return ConcreteDispatcher::make(); } -MetalDriver::MetalDriver(MetalPlatform* platform, const Platform::DriverConfig& driverConfig) noexcept - : mPlatform(*platform), - mContext(new MetalContext(driverConfig.textureUseAfterFreePoolSize)), - mHandleAllocator("Handles", - driverConfig.handleArenaSize, - driverConfig.disableHandleUseAfterFreeCheck), - mStereoscopicType(driverConfig.stereoscopicType) { +MetalDriver::MetalDriver( + MetalPlatform* platform, const Platform::DriverConfig& driverConfig) noexcept + : mPlatform(*platform), + mContext(new MetalContext), + mHandleAllocator( + "Handles", driverConfig.handleArenaSize, driverConfig.disableHandleUseAfterFreeCheck), + mStereoscopicType(driverConfig.stereoscopicType) { mContext->driver = this; TrackedMetalBuffer::setPlatform(platform); @@ -347,9 +347,6 @@ auto* descriptorSet = handle_cast(dsh); auto* texture = handle_cast(th); - texture->checkUseAfterFree( - "updateDescriptorSetTexture", - [&]() { return mHandleAllocator.getHandleTag(th.getId()).c_str_safe(); }, binding); id mtlTexture = texture->getMtlTextureForRead(); if (texture->target == SamplerType::SAMPLER_EXTERNAL) { @@ -460,8 +457,6 @@ void MetalDriver::createTextureViewR( Handle th, Handle srch, uint8_t baseLevel, uint8_t levelCount) { MetalTexture const* src = handle_cast(srch); - src->checkUseAfterFree("createTextureView", - [&]() { return mHandleAllocator.getHandleTag(th.getId()).c_str_safe(); }); mContext->textures.insert( construct_handle(th, *mContext, src, baseLevel, levelCount)); } @@ -470,8 +465,6 @@ backend::TextureSwizzle r, backend::TextureSwizzle g, backend::TextureSwizzle b, backend::TextureSwizzle a) { MetalTexture const* src = handle_cast(srch); - src->checkUseAfterFree("createTextureViewSwizzle", - [&]() { return mHandleAllocator.getHandleTag(th.getId()).c_str_safe(); }); mContext->textures.insert(construct_handle(th, *mContext, src, r, g, b, a)); } @@ -555,9 +548,6 @@ FILAMENT_CHECK_PRECONDITION(buffer.handle) << "The COLOR" << i << " flag was specified, but invalid color handle provided."; auto colorTexture = handle_cast(buffer.handle); - colorTexture->checkUseAfterFree("createRenderTarget.color", [&]() { - return mHandleAllocator.getHandleTag(buffer.handle.getId()).c_str_safe(); - }); FILAMENT_CHECK_PRECONDITION(colorTexture->getMtlTextureForWrite()) << "Color texture passed to render target has no texture allocation"; colorAttachments[i] = { colorTexture, color[i].level, color[i].layer }; @@ -568,9 +558,6 @@ FILAMENT_CHECK_PRECONDITION(depth.handle) << "The DEPTH flag was specified, but invalid depth handle provided."; auto depthTexture = handle_cast(depth.handle); - depthTexture->checkUseAfterFree("createRenderTarget.depth", [&]() { - return mHandleAllocator.getHandleTag(depth.handle.getId()).c_str_safe(); - }); FILAMENT_CHECK_PRECONDITION(depthTexture->getMtlTextureForWrite()) << "Depth texture passed to render target has no texture allocation."; depthAttachment = { depthTexture, depth.level, depth.layer }; @@ -581,9 +568,6 @@ FILAMENT_CHECK_PRECONDITION(stencil.handle) << "The STENCIL flag was specified, but invalid stencil handle provided."; auto stencilTexture = handle_cast(stencil.handle); - stencilTexture->checkUseAfterFree("createRenderTarget.stencil", [&]() { - return mHandleAllocator.getHandleTag(stencil.handle.getId()).c_str_safe(); - }); FILAMENT_CHECK_PRECONDITION(stencilTexture->getMtlTextureForWrite()) << "Stencil texture passed to render target has no texture allocation."; stencilAttachment = { stencilTexture, stencil.level, stencil.layer }; @@ -816,15 +800,7 @@ auto* metalTexture = handle_cast(th); mContext->textures.erase(metalTexture); - // Free memory from the texture and mark it as freed. - metalTexture->terminate(); - - // Add this texture handle to our texturesToDestroy queue to be destroyed later. - if (auto handleToFree = mContext->texturesToDestroy.push(th)) { - // If texturesToDestroy is full, then .push evicts the oldest texture handle in the - // queue (or simply th, if use-after-free detection is disabled). - destruct_handle(handleToFree.value()); - } + destruct_handle(th); } void MetalDriver::destroyRenderTarget(Handle rth) { @@ -874,12 +850,6 @@ } void MetalDriver::terminate() { - // Terminate any outstanding MetalTextures. - while (!mContext->texturesToDestroy.empty()) { - Handle toDestroy = mContext->texturesToDestroy.pop(); - destruct_handle(toDestroy); - } - // finish() will flush the pending command buffer and will ensure all GPU work has finished. // This must be done before calling bufferPool->reset() to ensure no buffers are in flight. finish(); @@ -1148,8 +1118,6 @@ FILAMENT_CHECK_PRECONDITION(!isInRenderPass(mContext)) << "update3DImage must be called outside of a render pass."; auto tex = handle_cast(th); - tex->checkUseAfterFree("update3DImage", - [&]() { return mHandleAllocator.getHandleTag(th.getId()).c_str_safe(); }); tex->loadImage(level, MTLRegionMake3D(xoffset, yoffset, zoffset, width, height, depth), data); scheduleDestroy(std::move(data)); @@ -1186,8 +1154,6 @@ FILAMENT_CHECK_PRECONDITION(!isInRenderPass(mContext)) << "generateMipmaps must be called outside of a render pass."; auto tex = handle_cast(th); - tex->checkUseAfterFree("generateMipmaps", - [&]() { return mHandleAllocator.getHandleTag(th.getId()).c_str_safe(); }); tex->generateMipmaps(); DEBUG_LOG("generateMipmaps(th = %d)\n", th.getId()); diff --git a/filament/backend/src/metal/MetalHandles.h b/filament/backend/src/metal/MetalHandles.h index 602aa5a6d17..79263d1015f 100644 --- a/filament/backend/src/metal/MetalHandles.h +++ b/filament/backend/src/metal/MetalHandles.h @@ -265,38 +265,7 @@ class MetalTexture : public HwTexture { MTLPixelFormat devicePixelFormat; - // Frees memory associated with this texture and marks it as "terminated". - // Used to track "use after free" scenario. - void terminate() noexcept; - bool isTerminated() const noexcept { return terminated; } - inline void checkUseAfterFree(const char* operation, - utils::Invocable getTextureTag, size_t textureIndex) const { - if (UTILS_LIKELY(!isTerminated())) { - return; - } - throwUseAfterFreeException([NSString - stringWithFormat:@"Filament Metal texture use after free, operation = %s, tag = " - @"%s, binding = %zu", - operation, getTextureTag(), textureIndex]); - } - inline void checkUseAfterFree( - const char* operation, utils::Invocable getTextureTag) const { - if (UTILS_LIKELY(!isTerminated())) { - return; - } - throwUseAfterFreeException([NSString - stringWithFormat:@"Filament Metal texture use after free, operation = %s, tag = %s", - operation, getTextureTag()]); - } - private: - inline void throwUseAfterFreeException(NSString* reason) const { - NSException* useAfterFreeException = - [NSException exceptionWithName:@"MetalTextureUseAfterFree" - reason:reason - userInfo:nil]; - [useAfterFreeException raise]; - } void loadSlice(uint32_t level, MTLRegion region, uint32_t byteOffset, uint32_t slice, PixelBufferDescriptor const& data) noexcept; void loadWithCopyBuffer(uint32_t level, uint32_t slice, MTLRegion region, PixelBufferDescriptor const& data, @@ -312,8 +281,6 @@ class MetalTexture : public HwTexture { // Filament swizzling only affects texture reads, so this should not be used when the texture is // bound as a render target attachment. id swizzledTextureView = nil; - - bool terminated = false; }; class MetalRenderTarget : public HwRenderTarget { diff --git a/filament/backend/src/metal/MetalHandles.mm b/filament/backend/src/metal/MetalHandles.mm index e28a66fe1eb..cc2642936d2 100644 --- a/filament/backend/src/metal/MetalHandles.mm +++ b/filament/backend/src/metal/MetalHandles.mm @@ -634,14 +634,6 @@ static void func(void* user) { texture = externalImage->getMtlTexture(); } -void MetalTexture::terminate() noexcept { - texture = nil; - swizzledTextureView = nil; - msaaSidecar = nil; - externalImage = nullptr; - terminated = true; -} - id MetalTexture::getMtlTextureForRead() const noexcept { return swizzledTextureView ? swizzledTextureView : texture; } diff --git a/filament/include/filament/Engine.h b/filament/include/filament/Engine.h index 641c9846eda..2686f25ea1c 100644 --- a/filament/include/filament/Engine.h +++ b/filament/include/filament/Engine.h @@ -288,16 +288,6 @@ class UTILS_PUBLIC Engine { */ uint32_t jobSystemThreadCount = 0; - /* - * Number of most-recently destroyed textures to track for use-after-free. - * - * This will cause the backend to throw an exception when a texture is freed but still bound - * to a SamplerGroup and used in a draw call. 0 disables completely. - * - * Currently only respected by the Metal backend. - */ - size_t textureUseAfterFreePoolSize = 0; - /** * When uploading vertex or index data, the Filament Metal backend copies data * into a shared staging area before transferring it to the GPU. This setting controls diff --git a/filament/src/details/Engine.cpp b/filament/src/details/Engine.cpp index c6598409538..b1aaa5c9f5f 100644 --- a/filament/src/details/Engine.cpp +++ b/filament/src/details/Engine.cpp @@ -103,7 +103,6 @@ Engine* FEngine::create(Engine::Builder const& builder) { } DriverConfig const driverConfig{ .handleArenaSize = instance->getRequestedDriverHandleArenaSize(), - .textureUseAfterFreePoolSize = instance->getConfig().textureUseAfterFreePoolSize, .metalUploadBufferSizeBytes = instance->getConfig().metalUploadBufferSizeBytes, .disableParallelShaderCompile = instance->getConfig().disableParallelShaderCompile, .disableHandleUseAfterFreeCheck = instance->getConfig().disableHandleUseAfterFreeCheck, @@ -701,7 +700,6 @@ int FEngine::loop() { DriverConfig const driverConfig { .handleArenaSize = getRequestedDriverHandleArenaSize(), - .textureUseAfterFreePoolSize = mConfig.textureUseAfterFreePoolSize, .metalUploadBufferSizeBytes = mConfig.metalUploadBufferSizeBytes, .disableParallelShaderCompile = mConfig.disableParallelShaderCompile, .disableHandleUseAfterFreeCheck = mConfig.disableHandleUseAfterFreeCheck,