diff --git a/layersvt/debug_marker/debug_marker.cpp b/layersvt/debug_marker/debug_marker.cpp index 45fa52af62..9974bb3c92 100644 --- a/layersvt/debug_marker/debug_marker.cpp +++ b/layersvt/debug_marker/debug_marker.cpp @@ -40,6 +40,61 @@ void EmitDebugObjectNamePacket(uint64_t device, int32_t type, uint64_t handle, c }); } +// Names are removed before the downstream call: once destruction returns, the driver may hand out +// the same handle value to a new object created on another thread. +template +VKAPI_ATTR void VKAPI_CALL DestroyNamedObject(VkDevice device, Handle handle, const VkAllocationCallbacks* pAllocator) { + DebugMarker::Get().RemoveDebugObjectName((uint64_t)device, ObjectType, (uint64_t)handle); + layersvt::DispatchDownstream(device, handle, pAllocator); +} + +template +PFN_vkVoidFunction DestroyHookFunction() { + return reinterpret_cast(&DestroyNamedObject); +} + +struct DestroyHook { + const char* command_name; + PFN_vkVoidFunction function; + bool is_extension = false; +}; + +// Covers object types whose live handles are unique. Samplers, sampler YCbCr conversions, pipeline +// layouts and descriptor set layouts are excluded: drivers may return the same handle for identical +// create infos, so destroying one copy must not remove the name of another live copy. Pool-allocated +// command buffers and descriptor sets are excluded because pools release them without a per-object call. +const DestroyHook kDestroyHooks[] = { + {"vkFreeMemory", DestroyHookFunction<&VkuDeviceDispatchTable::FreeMemory, VK_OBJECT_TYPE_DEVICE_MEMORY, VkDeviceMemory>()}, + {"vkDestroyBuffer", DestroyHookFunction<&VkuDeviceDispatchTable::DestroyBuffer, VK_OBJECT_TYPE_BUFFER, VkBuffer>()}, + {"vkDestroyImage", DestroyHookFunction<&VkuDeviceDispatchTable::DestroyImage, VK_OBJECT_TYPE_IMAGE, VkImage>()}, + {"vkDestroyImageView", DestroyHookFunction<&VkuDeviceDispatchTable::DestroyImageView, VK_OBJECT_TYPE_IMAGE_VIEW, VkImageView>()}, + {"vkDestroyBufferView", + DestroyHookFunction<&VkuDeviceDispatchTable::DestroyBufferView, VK_OBJECT_TYPE_BUFFER_VIEW, VkBufferView>()}, + {"vkDestroyFramebuffer", + DestroyHookFunction<&VkuDeviceDispatchTable::DestroyFramebuffer, VK_OBJECT_TYPE_FRAMEBUFFER, VkFramebuffer>()}, + {"vkDestroyRenderPass", + DestroyHookFunction<&VkuDeviceDispatchTable::DestroyRenderPass, VK_OBJECT_TYPE_RENDER_PASS, VkRenderPass>()}, + {"vkDestroyPipeline", DestroyHookFunction<&VkuDeviceDispatchTable::DestroyPipeline, VK_OBJECT_TYPE_PIPELINE, VkPipeline>()}, + {"vkDestroyShaderModule", + DestroyHookFunction<&VkuDeviceDispatchTable::DestroyShaderModule, VK_OBJECT_TYPE_SHADER_MODULE, VkShaderModule>()}, + {"vkDestroyPipelineCache", + DestroyHookFunction<&VkuDeviceDispatchTable::DestroyPipelineCache, VK_OBJECT_TYPE_PIPELINE_CACHE, VkPipelineCache>()}, + {"vkDestroyQueryPool", DestroyHookFunction<&VkuDeviceDispatchTable::DestroyQueryPool, VK_OBJECT_TYPE_QUERY_POOL, VkQueryPool>()}, + {"vkDestroyFence", DestroyHookFunction<&VkuDeviceDispatchTable::DestroyFence, VK_OBJECT_TYPE_FENCE, VkFence>()}, + {"vkDestroySemaphore", DestroyHookFunction<&VkuDeviceDispatchTable::DestroySemaphore, VK_OBJECT_TYPE_SEMAPHORE, VkSemaphore>()}, + {"vkDestroyEvent", DestroyHookFunction<&VkuDeviceDispatchTable::DestroyEvent, VK_OBJECT_TYPE_EVENT, VkEvent>()}, + {"vkDestroyCommandPool", + DestroyHookFunction<&VkuDeviceDispatchTable::DestroyCommandPool, VK_OBJECT_TYPE_COMMAND_POOL, VkCommandPool>()}, + {"vkDestroyDescriptorPool", + DestroyHookFunction<&VkuDeviceDispatchTable::DestroyDescriptorPool, VK_OBJECT_TYPE_DESCRIPTOR_POOL, VkDescriptorPool>()}, + {"vkDestroySwapchainKHR", + DestroyHookFunction<&VkuDeviceDispatchTable::DestroySwapchainKHR, VK_OBJECT_TYPE_SWAPCHAIN_KHR, VkSwapchainKHR>(), true}, + {"vkDestroyAccelerationStructureKHR", + DestroyHookFunction<&VkuDeviceDispatchTable::DestroyAccelerationStructureKHR, VK_OBJECT_TYPE_ACCELERATION_STRUCTURE_KHR, + VkAccelerationStructureKHR>(), + true}, +}; + } // namespace DebugMarker::DebugMarker() = default; @@ -180,7 +235,6 @@ PFN_vkVoidFunction DebugMarker::GetLayerInstanceCommand(VkInstance instance, con } PFN_vkVoidFunction DebugMarker::GetLayerDeviceCommand(VkDevice device, const char* name) { - (void)device; // VK_EXT_debug_marker if (strcmp(name, "vkCmdDebugMarkerBeginEXT") == 0) return reinterpret_cast(vkCmdDebugMarkerBeginEXT); @@ -207,5 +261,15 @@ PFN_vkVoidFunction DebugMarker::GetLayerDeviceCommand(VkDevice device, const cha if (strcmp(name, "vkQueueInsertDebugUtilsLabelEXT") == 0) return reinterpret_cast(vkQueueInsertDebugUtilsLabelEXT); + for (const DestroyHook& hook : kDestroyHooks) { + if (strcmp(name, hook.command_name) != 0) continue; + // Extension commands are only intercepted when the downstream device provides them. + if (hook.is_extension && device != VK_NULL_HANDLE && + layersvt::DispatchDownstreamOr<&VkuDeviceDispatchTable::GetDeviceProcAddr>(nullptr, device, name) == nullptr) { + return nullptr; + } + return hook.function; + } + return nullptr; } diff --git a/layersvt/test/test_debugmarker.cpp b/layersvt/test/test_debugmarker.cpp index 3605bf4c1b..b9cd980b35 100644 --- a/layersvt/test/test_debugmarker.cpp +++ b/layersvt/test/test_debugmarker.cpp @@ -194,6 +194,129 @@ TEST_F(DebugMarkerTests, NullOrEmptyNameRemovesNameTest) { EXPECT_EQ(DebugMarker::Get().GetDebugObjectName(device, VK_OBJECT_TYPE_BUFFER, 0x4), std::nullopt); } +namespace { + +uint32_t downstream_destroy_calls = 0; + +} // namespace + +TEST_F(DebugMarkerTests, DestroyedObjectNamesRemovedTest) { + TEST_DESCRIPTION("Verify freeing memory and destroying buffers and images removes their names and forwards downstream"); + + downstream_destroy_calls = 0; + void* mock_device_object = reinterpret_cast(0x7000); + VkDevice mock_device = reinterpret_cast(&mock_device_object); + layersvt::LayerBaseTestPeer::GetDispatchTableManager(DebugMarker::Get()) + .InitDeviceTable(mock_device, [](VkDevice, const char* name) -> PFN_vkVoidFunction { + if (strcmp(name, "vkFreeMemory") == 0) { + return reinterpret_cast( + +[](VkDevice, VkDeviceMemory, const VkAllocationCallbacks*) { ++downstream_destroy_calls; }); + } + if (strcmp(name, "vkDestroyBuffer") == 0) { + return reinterpret_cast( + +[](VkDevice, VkBuffer, const VkAllocationCallbacks*) { ++downstream_destroy_calls; }); + } + if (strcmp(name, "vkDestroyImage") == 0) { + return reinterpret_cast( + +[](VkDevice, VkImage, const VkAllocationCallbacks*) { ++downstream_destroy_calls; }); + } + return nullptr; + }); + + auto free_memory = reinterpret_cast(layersvt::LayerBaseTestPeer::GetKnownDeviceCommand("vkFreeMemory")); + auto destroy_buffer = + reinterpret_cast(layersvt::LayerBaseTestPeer::GetKnownDeviceCommand("vkDestroyBuffer")); + auto destroy_image = reinterpret_cast(layersvt::LayerBaseTestPeer::GetKnownDeviceCommand("vkDestroyImage")); + ASSERT_NE(free_memory, nullptr); + ASSERT_NE(destroy_buffer, nullptr); + ASSERT_NE(destroy_image, nullptr); + + const uint64_t device = (uint64_t)mock_device; + DebugMarker::Get().SetDebugObjectName(device, VK_OBJECT_TYPE_DEVICE_MEMORY, 0x1, "Memory"); + DebugMarker::Get().SetDebugObjectName(device, VK_OBJECT_TYPE_BUFFER, 0x2, "Buffer"); + DebugMarker::Get().SetDebugObjectName(device, VK_OBJECT_TYPE_IMAGE, 0x3, "Image"); + // A sampler sharing the buffer's handle value must keep its name. + DebugMarker::Get().SetDebugObjectName(device, VK_OBJECT_TYPE_SAMPLER, 0x2, "Sampler"); + + free_memory(mock_device, (VkDeviceMemory)0x1, nullptr); + destroy_buffer(mock_device, (VkBuffer)0x2, nullptr); + destroy_image(mock_device, (VkImage)0x3, nullptr); + + EXPECT_EQ(downstream_destroy_calls, 3u); + EXPECT_FALSE(DebugMarker::Get().HasDebugObjectName(device, VK_OBJECT_TYPE_DEVICE_MEMORY, 0x1, "Memory")); + EXPECT_FALSE(DebugMarker::Get().HasDebugObjectName(device, VK_OBJECT_TYPE_BUFFER, 0x2, "Buffer")); + EXPECT_FALSE(DebugMarker::Get().HasDebugObjectName(device, VK_OBJECT_TYPE_IMAGE, 0x3, "Image")); + EXPECT_TRUE(DebugMarker::Get().HasDebugObjectName(device, VK_OBJECT_TYPE_SAMPLER, 0x2, "Sampler")); +} + +TEST_F(DebugMarkerTests, DestroyedObjectNamesOfOtherTypesRemovedTest) { + TEST_DESCRIPTION("Verify destroying other uniquely-handled objects removes names, while samplers are not intercepted"); + + downstream_destroy_calls = 0; + void* mock_device_object = reinterpret_cast(0x8000); + VkDevice mock_device = reinterpret_cast(&mock_device_object); + layersvt::LayerBaseTestPeer::GetDispatchTableManager(DebugMarker::Get()) + .InitDeviceTable(mock_device, [](VkDevice, const char* name) -> PFN_vkVoidFunction { + if (strcmp(name, "vkDestroyImageView") == 0) { + return reinterpret_cast( + +[](VkDevice, VkImageView, const VkAllocationCallbacks*) { ++downstream_destroy_calls; }); + } + if (strcmp(name, "vkDestroyPipeline") == 0) { + return reinterpret_cast( + +[](VkDevice, VkPipeline, const VkAllocationCallbacks*) { ++downstream_destroy_calls; }); + } + return nullptr; + }); + + auto destroy_image_view = + reinterpret_cast(layersvt::LayerBaseTestPeer::GetKnownDeviceCommand("vkDestroyImageView")); + auto destroy_pipeline = + reinterpret_cast(layersvt::LayerBaseTestPeer::GetKnownDeviceCommand("vkDestroyPipeline")); + ASSERT_NE(destroy_image_view, nullptr); + ASSERT_NE(destroy_pipeline, nullptr); + EXPECT_EQ(layersvt::LayerBaseTestPeer::GetKnownDeviceCommand("vkDestroySampler"), nullptr); + + const uint64_t device = (uint64_t)mock_device; + DebugMarker::Get().SetDebugObjectName(device, VK_OBJECT_TYPE_IMAGE_VIEW, 0x1, "ImageView"); + DebugMarker::Get().SetDebugObjectName(device, VK_OBJECT_TYPE_PIPELINE, 0x2, "Pipeline"); + + destroy_image_view(mock_device, (VkImageView)0x1, nullptr); + destroy_pipeline(mock_device, (VkPipeline)0x2, nullptr); + + EXPECT_EQ(downstream_destroy_calls, 2u); + EXPECT_FALSE(DebugMarker::Get().HasDebugObjectName(device, VK_OBJECT_TYPE_IMAGE_VIEW, 0x1, "ImageView")); + EXPECT_FALSE(DebugMarker::Get().HasDebugObjectName(device, VK_OBJECT_TYPE_PIPELINE, 0x2, "Pipeline")); +} + +TEST_F(DebugMarkerTests, ExtensionDestroyCommandRequiresDownstreamTest) { + TEST_DESCRIPTION("Verify extension destroy commands are intercepted only when the downstream device provides them"); + + void* mock_device_with_swapchain_object = reinterpret_cast(0x9000); + VkDevice mock_device_with_swapchain = reinterpret_cast(&mock_device_with_swapchain_object); + void* mock_device_without_swapchain_object = reinterpret_cast(0xA000); + VkDevice mock_device_without_swapchain = reinterpret_cast(&mock_device_without_swapchain_object); + + static const PFN_vkVoidFunction downstream_destroy_swapchain = + reinterpret_cast(+[](VkDevice, VkSwapchainKHR, const VkAllocationCallbacks*) {}); + layersvt::LayerBaseTestPeer::GetDispatchTableManager(DebugMarker::Get()) + .InitDeviceTable(mock_device_with_swapchain, [](VkDevice, const char* name) -> PFN_vkVoidFunction { + return strcmp(name, "vkDestroySwapchainKHR") == 0 ? downstream_destroy_swapchain : nullptr; + }); + layersvt::LayerBaseTestPeer::GetDispatchTableManager(DebugMarker::Get()) + .InitDeviceTable(mock_device_without_swapchain, [](VkDevice, const char*) -> PFN_vkVoidFunction { return nullptr; }); + + PFN_vkVoidFunction intercepted = + layersvt::LayerBaseTestPeer::GetDeviceProcAddr(mock_device_with_swapchain, "vkDestroySwapchainKHR"); + EXPECT_NE(intercepted, nullptr); + EXPECT_NE(intercepted, downstream_destroy_swapchain); + EXPECT_EQ(layersvt::LayerBaseTestPeer::GetDeviceProcAddr(mock_device_without_swapchain, "vkDestroySwapchainKHR"), nullptr); + + DebugMarker::Get().SetDebugObjectName((uint64_t)mock_device_with_swapchain, VK_OBJECT_TYPE_SWAPCHAIN_KHR, 0x1, "Swapchain"); + reinterpret_cast(intercepted)(mock_device_with_swapchain, (VkSwapchainKHR)0x1, nullptr); + EXPECT_FALSE( + DebugMarker::Get().HasDebugObjectName((uint64_t)mock_device_with_swapchain, VK_OBJECT_TYPE_SWAPCHAIN_KHR, 0x1, "Swapchain")); +} + TEST_F(DebugMarkerTests, SameHandleOnDifferentDevicesTest) { TEST_DESCRIPTION("Verify identical handles owned by different devices keep separate names");