Skip to content

Commit

Permalink
Create use-after-free detector for Metal textures (#7250)
Browse files Browse the repository at this point in the history
  • Loading branch information
bejado committed Oct 20, 2023
1 parent 1b7187f commit 4dd98e6
Show file tree
Hide file tree
Showing 24 changed files with 366 additions and 44 deletions.
7 changes: 7 additions & 0 deletions filament/backend/include/backend/Platform.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,13 @@ class UTILS_PUBLIC Platform {
* Driver clamps to valid values.
*/
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;
};

Platform() noexcept;
Expand Down
1 change: 1 addition & 0 deletions filament/backend/include/private/backend/Driver.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include <backend/PipelineState.h>
#include <backend/TargetBufferInfo.h>

#include <utils/CString.h>
#include <utils/compiler.h>

#include <functional>
Expand Down
2 changes: 1 addition & 1 deletion filament/backend/include/private/backend/DriverAPI.inc
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ DECL_DRIVER_API_R_N(backend::TextureHandle, importTexture,
backend::TextureUsage, usage)

DECL_DRIVER_API_R_N(backend::SamplerGroupHandle, createSamplerGroup,
uint32_t, size)
uint32_t, size, utils::FixedSizeString<32>, debugName)

DECL_DRIVER_API_R_N(backend::RenderPrimitiveHandle, createRenderPrimitive,
backend::VertexBufferHandle, vbh,
Expand Down
13 changes: 13 additions & 0 deletions filament/backend/src/metal/MetalContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
#include <Metal/Metal.h>
#include <QuartzCore/QuartzCore.h>

#include <utils/FixedCircularBuffer.h>

#include <array>
#include <atomic>
#include <stack>
Expand Down Expand Up @@ -53,6 +55,9 @@ struct MetalVertexBuffer;
constexpr static uint8_t MAX_SAMPLE_COUNT = 8; // Metal devices support at most 8 MSAA samples

struct MetalContext {
explicit MetalContext(size_t metalFreedTextureListSize)
: texturesToDestroy(metalFreedTextureListSize) {}

MetalDriver* driver;
id<MTLDevice> device = nullptr;
id<MTLCommandQueue> commandQueue = nullptr;
Expand Down Expand Up @@ -111,6 +116,14 @@ struct MetalContext {
tsl::robin_set<MetalSamplerGroup*> samplerGroups;
tsl::robin_set<MetalTexture*> 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<Handle<HwTexture>> texturesToDestroy;

MetalBufferPool* bufferPool;

MetalSwapChain* currentDrawSwapChain = nil;
Expand Down
52 changes: 39 additions & 13 deletions filament/backend/src/metal/MetalDriver.mm
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@
Driver* MetalDriver::create(MetalPlatform* const platform, const Platform::DriverConfig& driverConfig) {
assert_invariant(platform);
size_t defaultSize = FILAMENT_METAL_HANDLE_ARENA_SIZE_IN_MB * 1024U * 1024U;
Platform::DriverConfig validConfig { .handleArenaSize = std::max(driverConfig.handleArenaSize, defaultSize) };
Platform::DriverConfig validConfig {driverConfig};
validConfig.handleArenaSize = std::max(driverConfig.handleArenaSize, defaultSize);
return new MetalDriver(platform, validConfig);
}

Expand All @@ -60,7 +61,7 @@

MetalDriver::MetalDriver(MetalPlatform* platform, const Platform::DriverConfig& driverConfig) noexcept
: mPlatform(*platform),
mContext(new MetalContext),
mContext(new MetalContext(driverConfig.textureUseAfterFreePoolSize)),
mHandleAllocator("Handles", driverConfig.handleArenaSize) {
mContext->driver = this;

Expand Down Expand Up @@ -303,8 +304,9 @@
target, levels, format, samples, width, height, depth, usage, metalTexture));
}

void MetalDriver::createSamplerGroupR(Handle<HwSamplerGroup> sbh, uint32_t size) {
mContext->samplerGroups.insert(construct_handle<MetalSamplerGroup>(sbh, size));
void MetalDriver::createSamplerGroupR(
Handle<HwSamplerGroup> sbh, uint32_t size, utils::FixedSizeString<32> debugName) {
mContext->samplerGroups.insert(construct_handle<MetalSamplerGroup>(sbh, size, debugName));
}

void MetalDriver::createRenderPrimitiveR(Handle<HwRenderPrimitive> rph,
Expand Down Expand Up @@ -530,8 +532,18 @@
return;
}

mContext->textures.erase(handle_cast<MetalTexture>(th));
destruct_handle<MetalTexture>(th);
auto* metalTexture = handle_cast<MetalTexture>(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<MetalTexture>(handleToFree.value());
}
}

void MetalDriver::destroyRenderTarget(Handle<HwRenderTarget> rth) {
Expand All @@ -557,6 +569,12 @@
}

void MetalDriver::terminate() {
// Terminate any outstanding MetalTextures.
while (!mContext->texturesToDestroy.empty()) {
Handle<HwTexture> toDestroy = mContext->texturesToDestroy.pop();
destruct_handle<MetalTexture>(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();
Expand Down Expand Up @@ -854,22 +872,26 @@
assert_invariant(sb->size == data.size / sizeof(SamplerDescriptor));
auto const* const samplers = (SamplerDescriptor const*) data.buffer;

#ifndef NDEBUG
// In debug builds, verify that all the textures in the sampler group are still alive.
// Verify that all the textures in the sampler group are still alive.
// These bugs lead to memory corruption and can be difficult to track down.
for (size_t s = 0; s < data.size / sizeof(SamplerDescriptor); s++) {
if (!samplers[s].t) {
continue;
}
// The difference between this check and the one below is that in release, we do this for
// only a set number of recently freed textures, while the debug check is exhaustive.
auto* metalTexture = handle_cast<MetalTexture>(samplers[s].t);
metalTexture->checkUseAfterFree(sb->debugName.c_str(), s);
#ifndef NDEBUG
auto iter = mContext->textures.find(handle_cast<MetalTexture>(samplers[s].t));
if (iter == mContext->textures.end()) {
utils::slog.e << "updateSamplerGroup: texture #"
<< (int) s << " is dead, texture handle = "
<< samplers[s].t << utils::io::endl;
}
assert_invariant(iter != mContext->textures.end());
}
#endif
}

// Create a MTLArgumentEncoder for these textures.
// Ideally, we would create this encoder at createSamplerGroup time, but we need to know the
Expand Down Expand Up @@ -1357,23 +1379,27 @@

id<MTLCommandBuffer> cmdBuffer = getPendingCommandBuffer(mContext);

#ifndef NDEBUG
// In debug builds, verify that all the textures in the sampler group are still alive.
// Verify that all the textures in the sampler group are still alive.
// These bugs lead to memory corruption and can be difficult to track down.
const auto& handles = samplerGroup->getTextureHandles();
for (size_t s = 0; s < handles.size(); s++) {
if (!handles[s]) {
continue;
}
auto iter = mContext->textures.find(handle_cast<MetalTexture>(handles[s]));
// The difference between this check and the one below is that in release, we do this for
// only a set number of recently freed textures, while the debug check is exhaustive.
auto* metalTexture = handle_cast<MetalTexture>(handles[s]);
metalTexture->checkUseAfterFree(samplerGroup->debugName.c_str(), s);
#ifndef NDEBUG
auto iter = mContext->textures.find(metalTexture);
if (iter == mContext->textures.end()) {
utils::slog.e << "finalizeSamplerGroup: texture #"
<< (int) s << " is dead, texture handle = "
<< handles[s] << utils::io::endl;
}
assert_invariant(iter != mContext->textures.end());
}
#endif
}

utils::FixedCapacityVector<id<MTLTexture>> newTextures(samplerGroup->size, nil);
for (size_t binding = 0; binding < samplerGroup->size; binding++) {
Expand Down
37 changes: 30 additions & 7 deletions filament/backend/src/metal/MetalHandles.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
#include "private/backend/SamplerGroup.h"

#include <utils/bitset.h>
#include <utils/CString.h>
#include <utils/FixedCapacityVector.h>
#include <utils/Panic.h>

Expand Down Expand Up @@ -228,8 +229,8 @@ class MetalTexture : public HwTexture {
// - using the texture as a render target attachment
// - calling setMinMaxLevels
// A texture's available mips are consistent throughout a render pass.
void setLodRange(uint32_t minLevel, uint32_t maxLevel);
void extendLodRangeTo(uint32_t level);
void setLodRange(uint16_t minLevel, uint16_t maxLevel);
void extendLodRangeTo(uint16_t level);

static MTLPixelFormat decidePixelFormat(MetalContext* context, TextureFormat format);

Expand All @@ -243,6 +244,26 @@ 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* samplerGroupDebugName, size_t textureIndex) const {
if (UTILS_LIKELY(!isTerminated())) {
return;
}
NSString* reason =
[NSString stringWithFormat:
@"Filament Metal texture use after free, sampler group = "
@"%s, texture index = %zu",
samplerGroupDebugName, textureIndex];
NSException* useAfterFreeException =
[NSException exceptionWithName:@"MetalTextureUseAfterFree"
reason:reason
userInfo:nil];
[useAfterFreeException raise];
}

private:
void loadSlice(uint32_t level, MTLRegion region, uint32_t byteOffset, uint32_t slice,
PixelBufferDescriptor const& data) noexcept;
Expand All @@ -259,14 +280,17 @@ class MetalTexture : public HwTexture {
id<MTLTexture> swizzledTextureView = nil;
id<MTLTexture> lodTextureView = nil;

uint32_t minLod = UINT_MAX;
uint32_t maxLod = 0;
uint16_t minLod = std::numeric_limits<uint16_t>::max();
uint16_t maxLod = 0;

bool terminated = false;
};

class MetalSamplerGroup : public HwSamplerGroup {
public:
explicit MetalSamplerGroup(size_t size) noexcept
explicit MetalSamplerGroup(size_t size, utils::FixedSizeString<32> name) noexcept
: size(size),
debugName(name),
textureHandles(size, Handle<HwTexture>()),
textures(size, nil),
samplers(size, nil) {}
Expand All @@ -276,12 +300,10 @@ class MetalSamplerGroup : public HwSamplerGroup {
textureHandles[index] = th;
}

#ifndef NDEBUG
// This method is only used for debugging, to ensure all texture handles are alive.
const auto& getTextureHandles() const {
return textureHandles;
}
#endif

// Encode a MTLTexture into this SamplerGroup at the given index.
inline void setFinalizedTexture(size_t index, id<MTLTexture> t) {
Expand Down Expand Up @@ -327,6 +349,7 @@ class MetalSamplerGroup : public HwSamplerGroup {
void useResources(id<MTLRenderCommandEncoder> renderPassEncoder);

size_t size;
utils::FixedSizeString<32> debugName;

public:

Expand Down
13 changes: 11 additions & 2 deletions filament/backend/src/metal/MetalHandles.mm
Original file line number Diff line number Diff line change
Expand Up @@ -535,6 +535,15 @@ void presentDrawable(bool presentFrame, void* user) {
setLodRange(0, levels - 1);
}

void MetalTexture::terminate() noexcept {
texture = nil;
swizzledTextureView = nil;
lodTextureView = nil;
msaaSidecar = nil;
externalImage.set(nullptr);
terminated = true;
}

MetalTexture::~MetalTexture() {
externalImage.set(nullptr);
}
Expand Down Expand Up @@ -807,14 +816,14 @@ void presentDrawable(bool presentFrame, void* user) {
context.blitter->blit(getPendingCommandBuffer(&context), args, "Texture upload blit");
}

void MetalTexture::extendLodRangeTo(uint32_t level) {
void MetalTexture::extendLodRangeTo(uint16_t level) {
assert_invariant(!isInRenderPass(&context));
minLod = std::min(minLod, level);
maxLod = std::max(maxLod, level);
lodTextureView = nil;
}

void MetalTexture::setLodRange(uint32_t min, uint32_t max) {
void MetalTexture::setLodRange(uint16_t min, uint16_t max) {
assert_invariant(!isInRenderPass(&context));
assert_invariant(min <= max);
minLod = min;
Expand Down
7 changes: 4 additions & 3 deletions filament/backend/src/opengl/OpenGLDriver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -147,8 +147,8 @@ Driver* OpenGLDriver::create(OpenGLPlatform* const platform,
#endif

size_t const defaultSize = FILAMENT_OPENGL_HANDLE_ARENA_SIZE_IN_MB * 1024U * 1024U;
Platform::DriverConfig const validConfig {
.handleArenaSize = std::max(driverConfig.handleArenaSize, defaultSize) };
Platform::DriverConfig validConfig {driverConfig};
validConfig.handleArenaSize = std::max(driverConfig.handleArenaSize, defaultSize);
OpenGLDriver* const driver = new OpenGLDriver(ec, validConfig);
return driver;
}
Expand Down Expand Up @@ -555,7 +555,8 @@ void OpenGLDriver::createProgramR(Handle<HwProgram> ph, Program&& program) {
CHECK_GL_ERROR(utils::slog.e)
}

void OpenGLDriver::createSamplerGroupR(Handle<HwSamplerGroup> sbh, uint32_t size) {
void OpenGLDriver::createSamplerGroupR(Handle<HwSamplerGroup> sbh, uint32_t size,
utils::FixedSizeString<32> debugName) {
DEBUG_MARKER()

construct<GLSamplerGroup>(sbh, size);
Expand Down
7 changes: 4 additions & 3 deletions filament/backend/src/vulkan/VulkanDriver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -214,8 +214,8 @@ Driver* VulkanDriver::create(VulkanPlatform* platform, VulkanContext const& cont
Platform::DriverConfig const& driverConfig) noexcept {
assert_invariant(platform);
size_t defaultSize = FVK_HANDLE_ARENA_SIZE_IN_MB * 1024U * 1024U;
Platform::DriverConfig validConfig{
.handleArenaSize = std::max(driverConfig.handleArenaSize, defaultSize)};
Platform::DriverConfig validConfig {driverConfig};
validConfig.handleArenaSize = std::max(driverConfig.handleArenaSize, defaultSize);
return new VulkanDriver(platform, context, validConfig);
}

Expand Down Expand Up @@ -315,7 +315,8 @@ void VulkanDriver::finish(int dummy) {
FVK_SYSTRACE_END();
}

void VulkanDriver::createSamplerGroupR(Handle<HwSamplerGroup> sbh, uint32_t count) {
void VulkanDriver::createSamplerGroupR(Handle<HwSamplerGroup> sbh, uint32_t count,
utils::FixedSizeString<32> debugName) {
auto sg = mResourceAllocator.construct<VulkanSamplerGroup>(sbh, count);
mResourceManager.acquire(sg);
}
Expand Down
3 changes: 2 additions & 1 deletion filament/backend/test/test_FeedbackLoops.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,8 @@ TEST_F(BackendTest, FeedbackLoops) {
sparams.filterMag = SamplerMagFilter::LINEAR;
sparams.filterMin = SamplerMinFilter::LINEAR_MIPMAP_NEAREST;
samplers.setSampler(0, { texture, sparams });
auto sgroup = api.createSamplerGroup(samplers.getSize());
auto sgroup =
api.createSamplerGroup(samplers.getSize(), utils::FixedSizeString<32>("Test"));
api.updateSamplerGroup(sgroup, samplers.toBufferDescriptor(api));
auto ubuffer = api.createBufferObject(sizeof(MaterialParams),
BufferObjectBinding::UNIFORM, BufferUsage::STATIC);
Expand Down
Loading

0 comments on commit 4dd98e6

Please sign in to comment.