From 51ec81006b89513d62836ac8c5ac7ae83133d54d Mon Sep 17 00:00:00 2001 From: olehkuznetsov <44271635+olehkuznetsov@users.noreply.github.com> Date: Mon, 26 Oct 2026 17:15:35 +0000 Subject: [PATCH 1/4] layersvt: Key DebugMarker object names by device (#46) Non-dispatchable handles are only unique within their VkDevice, so two devices in one process can return the same handle value. Naming an object on one device overwrote the name of an unrelated object on another, and the lost name was missing from later session replays. Key stored names by device as well as object type and handle. Bug: Test: new tests - DebugMarkerTests#SameHandleOnDifferentDevicesTest Change-Id: I8c0edd4d958451aebf1fbd17df1cb0cb6a6a6964 --- layersvt/debug_marker/debug_marker.cpp | 6 +-- layersvt/debug_marker/debug_marker.h | 9 +++-- layersvt/test/test_debugmarker.cpp | 54 +++++++++++++++++++++----- 3 files changed, 52 insertions(+), 17 deletions(-) diff --git a/layersvt/debug_marker/debug_marker.cpp b/layersvt/debug_marker/debug_marker.cpp index 6c6942df4b..2857455d9c 100644 --- a/layersvt/debug_marker/debug_marker.cpp +++ b/layersvt/debug_marker/debug_marker.cpp @@ -97,7 +97,7 @@ void DebugMarker::SetDebugObjectName(uint64_t device, int32_t type, uint64_t han std::lock_guard lock(mutex_); std::string name_str = name ? name : "NULL"; - debug_object_names_[std::make_pair(type, handle)] = DebugObjectName(device, type, handle, name_str); + debug_object_names_[std::make_tuple(device, type, handle)] = DebugObjectName(device, type, handle, name_str); EmitDebugObjectNamePacket(device, type, handle, name_str); } @@ -109,9 +109,9 @@ void DebugMarker::EmitAllDebugMarkers() { } } -bool DebugMarker::HasDebugObjectName(int32_t type, uint64_t handle, const std::string& name) { +bool DebugMarker::HasDebugObjectName(uint64_t device, int32_t type, uint64_t handle, const std::string& name) { std::lock_guard lock(mutex_); - auto it = debug_object_names_.find(std::make_pair(type, handle)); + auto it = debug_object_names_.find(std::make_tuple(device, type, handle)); if (it == debug_object_names_.end()) return false; return it->second.name == name; } diff --git a/layersvt/debug_marker/debug_marker.h b/layersvt/debug_marker/debug_marker.h index 05df9ce890..fc811bed88 100644 --- a/layersvt/debug_marker/debug_marker.h +++ b/layersvt/debug_marker/debug_marker.h @@ -21,6 +21,7 @@ #include #include +#include #include #include "common/layer_base.h" @@ -79,7 +80,7 @@ class DebugMarker : public layersvt::LayerBase { * @brief Checks if a debug name is stored for a given object. * @note This function is for testing only. */ - bool HasDebugObjectName(int32_t type, uint64_t handle, const std::string& name); + bool HasDebugObjectName(uint64_t device, int32_t type, uint64_t handle, const std::string& name); /** * @brief Emulated VK_EXT_debug_utils messengers, used for instances whose downstream lacks the @@ -128,10 +129,10 @@ class DebugMarker : public layersvt::LayerBase { std::mutex mutex_; /** - * @brief Maps a pair of (object_type, object_handle) to its debug name information. - * We use a pair as the key because handles are not guaranteed to be unique across different object types. + * @brief Maps (vk_device, object_type, object_handle) to its debug name information. + * Non-dispatchable handles are not guaranteed to be unique across object types or devices. */ - std::map, DebugObjectName> debug_object_names_; + std::map, DebugObjectName> debug_object_names_; struct EmulatedMessenger { VkInstance instance; diff --git a/layersvt/test/test_debugmarker.cpp b/layersvt/test/test_debugmarker.cpp index 825f84f605..63df65986f 100644 --- a/layersvt/test/test_debugmarker.cpp +++ b/layersvt/test/test_debugmarker.cpp @@ -60,16 +60,16 @@ TEST_F(DebugMarkerTests, CombinedTest) { // 1. Set instance name DebugMarker::Get().SetDebugObjectName(0, VK_OBJECT_TYPE_INSTANCE, (uint64_t)instance, "MyInstance"); - EXPECT_TRUE(DebugMarker::Get().HasDebugObjectName(VK_OBJECT_TYPE_INSTANCE, (uint64_t)instance, "MyInstance")); + EXPECT_TRUE(DebugMarker::Get().HasDebugObjectName(0, VK_OBJECT_TYPE_INSTANCE, (uint64_t)instance, "MyInstance")); // 2. Override new name DebugMarker::Get().SetDebugObjectName(0, VK_OBJECT_TYPE_INSTANCE, (uint64_t)instance, "MyInstanceRenamed"); - EXPECT_TRUE(DebugMarker::Get().HasDebugObjectName(VK_OBJECT_TYPE_INSTANCE, (uint64_t)instance, "MyInstanceRenamed")); - EXPECT_FALSE(DebugMarker::Get().HasDebugObjectName(VK_OBJECT_TYPE_INSTANCE, (uint64_t)instance, "MyInstance")); + EXPECT_TRUE(DebugMarker::Get().HasDebugObjectName(0, VK_OBJECT_TYPE_INSTANCE, (uint64_t)instance, "MyInstanceRenamed")); + EXPECT_FALSE(DebugMarker::Get().HasDebugObjectName(0, VK_OBJECT_TYPE_INSTANCE, (uint64_t)instance, "MyInstance")); // 3. Reset layer_test::ResetLayer(); - EXPECT_FALSE(DebugMarker::Get().HasDebugObjectName(VK_OBJECT_TYPE_INSTANCE, (uint64_t)instance, "MyInstanceRenamed")); + EXPECT_FALSE(DebugMarker::Get().HasDebugObjectName(0, VK_OBJECT_TYPE_INSTANCE, (uint64_t)instance, "MyInstanceRenamed")); } TEST_F(DebugMarkerTests, ManifestTest) { @@ -160,18 +160,52 @@ TEST_F(DebugMarkerTests, PreDestroyDeviceCleanupTest) { DebugMarker::Get().SetDebugObjectName((uint64_t)dev1, VK_OBJECT_TYPE_BUFFER, 0x1111, "Buffer1"); DebugMarker::Get().SetDebugObjectName((uint64_t)dev2, VK_OBJECT_TYPE_BUFFER, 0x2222, "Buffer2"); - EXPECT_TRUE(DebugMarker::Get().HasDebugObjectName(VK_OBJECT_TYPE_BUFFER, 0x1111, "Buffer1")); - EXPECT_TRUE(DebugMarker::Get().HasDebugObjectName(VK_OBJECT_TYPE_BUFFER, 0x2222, "Buffer2")); + EXPECT_TRUE(DebugMarker::Get().HasDebugObjectName((uint64_t)dev1, VK_OBJECT_TYPE_BUFFER, 0x1111, "Buffer1")); + EXPECT_TRUE(DebugMarker::Get().HasDebugObjectName((uint64_t)dev2, VK_OBJECT_TYPE_BUFFER, 0x2222, "Buffer2")); // Destroy dev1 - should remove Buffer1 but keep Buffer2 layersvt::LayerBaseTestPeer::DestroyDevice(dev1, nullptr); - EXPECT_FALSE(DebugMarker::Get().HasDebugObjectName(VK_OBJECT_TYPE_BUFFER, 0x1111, "Buffer1")); - EXPECT_TRUE(DebugMarker::Get().HasDebugObjectName(VK_OBJECT_TYPE_BUFFER, 0x2222, "Buffer2")); + EXPECT_FALSE(DebugMarker::Get().HasDebugObjectName((uint64_t)dev1, VK_OBJECT_TYPE_BUFFER, 0x1111, "Buffer1")); + EXPECT_TRUE(DebugMarker::Get().HasDebugObjectName((uint64_t)dev2, VK_OBJECT_TYPE_BUFFER, 0x2222, "Buffer2")); // Destroy dev2 - should remove Buffer2 layersvt::LayerBaseTestPeer::DestroyDevice(dev2, nullptr); - EXPECT_FALSE(DebugMarker::Get().HasDebugObjectName(VK_OBJECT_TYPE_BUFFER, 0x2222, "Buffer2")); + EXPECT_FALSE(DebugMarker::Get().HasDebugObjectName((uint64_t)dev2, VK_OBJECT_TYPE_BUFFER, 0x2222, "Buffer2")); +} + +TEST_F(DebugMarkerTests, SameHandleOnDifferentDevicesTest) { + TEST_DESCRIPTION("Verify identical handles owned by different devices keep separate names"); + + void* mock_dev1_vtable = reinterpret_cast(0x1000); + VkDevice dev1 = reinterpret_cast(&mock_dev1_vtable); + void* mock_dev2_vtable = reinterpret_cast(0x2000); + VkDevice dev2 = reinterpret_cast(&mock_dev2_vtable); + + layersvt::LayerBaseTestPeer::GetDispatchTableManager(DebugMarker::Get()) + .InitDeviceTable(dev1, [](VkDevice, const char*) -> PFN_vkVoidFunction { + return reinterpret_cast(+[](VkDevice, const VkAllocationCallbacks*) {}); + }); + layersvt::LayerBaseTestPeer::GetDispatchTableManager(DebugMarker::Get()) + .InitDeviceTable(dev2, [](VkDevice, const char*) -> PFN_vkVoidFunction { + return reinterpret_cast(+[](VkDevice, const VkAllocationCallbacks*) {}); + }); + + const uint64_t device1 = (uint64_t)dev1; + const uint64_t device2 = (uint64_t)dev2; + DebugMarker::Get().SetDebugObjectName(device1, VK_OBJECT_TYPE_BUFFER, 0x1, "Device1Buffer"); + DebugMarker::Get().SetDebugObjectName(device2, VK_OBJECT_TYPE_BUFFER, 0x1, "Device2Buffer"); + + EXPECT_TRUE(DebugMarker::Get().HasDebugObjectName(device1, VK_OBJECT_TYPE_BUFFER, 0x1, "Device1Buffer")); + EXPECT_TRUE(DebugMarker::Get().HasDebugObjectName(device2, VK_OBJECT_TYPE_BUFFER, 0x1, "Device2Buffer")); + + // Destroying device1 must keep the name that device2 stores for the same handle. + layersvt::LayerBaseTestPeer::DestroyDevice(dev1, nullptr); + EXPECT_FALSE(DebugMarker::Get().HasDebugObjectName(device1, VK_OBJECT_TYPE_BUFFER, 0x1, "Device1Buffer")); + EXPECT_TRUE(DebugMarker::Get().HasDebugObjectName(device2, VK_OBJECT_TYPE_BUFFER, 0x1, "Device2Buffer")); + + layersvt::LayerBaseTestPeer::DestroyDevice(dev2, nullptr); + EXPECT_FALSE(DebugMarker::Get().HasDebugObjectName(device2, VK_OBJECT_TYPE_BUFFER, 0x1, "Device2Buffer")); } TEST_F(DebugMarkerTests, TemplateMethodDispatchTest) { @@ -253,7 +287,7 @@ TEST_F(DebugMarkerTests, MissingDownstreamExtensionTest) { name_info.objectHandle = 0x4444; name_info.pObjectName = "Buffer"; EXPECT_EQ(set_object_name(mock_device, &name_info), VK_SUCCESS); - EXPECT_TRUE(DebugMarker::Get().HasDebugObjectName(VK_OBJECT_TYPE_BUFFER, 0x4444, "Buffer")); + EXPECT_TRUE(DebugMarker::Get().HasDebugObjectName((uint64_t)mock_device, VK_OBJECT_TYPE_BUFFER, 0x4444, "Buffer")); } namespace { From 820e6352007eae64029e52483c7339359a3e66a2 Mon Sep 17 00:00:00 2001 From: olehkuznetsov <44271635+olehkuznetsov@users.noreply.github.com> Date: Mon, 26 Oct 2026 17:17:57 +0000 Subject: [PATCH 2/4] layersvt: Remove DebugMarker object names on null or empty name (#47) VK_EXT_debug_utils defines a null or empty pObjectName as removing the object's name. DebugMarker stored and emitted the literal "NULL" instead, so cleared objects appeared as "NULL" in traces and were replayed under that name on every later session start. Erase the stored name instead and emit an empty name, so trace consumers treat the object as unnamed from that point on. The removal is a separate method so object destruction can reuse it. Bug: Test: new tests - DebugMarkerTests#NullOrEmptyNameRemovesNameTest Change-Id: Ia6d7df2386b911a0b50911ebc7c5e9176a6a6964 --- layersvt/debug_marker/debug_marker.cpp | 22 ++++++-- layersvt/debug_marker/debug_marker.h | 23 ++++---- layersvt/test/test_debugmarker.cpp | 72 +++++++++++++++++++------- 3 files changed, 83 insertions(+), 34 deletions(-) diff --git a/layersvt/debug_marker/debug_marker.cpp b/layersvt/debug_marker/debug_marker.cpp index 2857455d9c..45fa52af62 100644 --- a/layersvt/debug_marker/debug_marker.cpp +++ b/layersvt/debug_marker/debug_marker.cpp @@ -24,6 +24,8 @@ namespace { // The packet shares the track event sequence, whose defaults declare an incremental clock, so the // absolute timestamp must carry an explicit clock id to not be read as a delta. +// An empty object_name, emitted by RemoveDebugObjectName, means the object is unnamed from this +// packet's timestamp on. void EmitDebugObjectNamePacket(uint64_t device, int32_t type, uint64_t handle, const std::string& name) { perfetto::TrackEvent::Trace([device, type, handle, &name](perfetto::TrackEvent::TraceContext ctx) { auto packet = ctx.NewTracePacket(); @@ -94,13 +96,25 @@ void DebugMarker::PreDestroyDevice(VkDevice device, const VkAllocationCallbacks* } void DebugMarker::SetDebugObjectName(uint64_t device, int32_t type, uint64_t handle, const char* name) { + if (name == nullptr || name[0] == '\0') { + RemoveDebugObjectName(device, type, handle); + return; + } + std::lock_guard lock(mutex_); - std::string name_str = name ? name : "NULL"; + std::string name_str = name; debug_object_names_[std::make_tuple(device, type, handle)] = DebugObjectName(device, type, handle, name_str); EmitDebugObjectNamePacket(device, type, handle, name_str); } +void DebugMarker::RemoveDebugObjectName(uint64_t device, int32_t type, uint64_t handle) { + std::lock_guard lock(mutex_); + if (debug_object_names_.erase(std::make_tuple(device, type, handle)) == 0) return; + // An empty name tells trace consumers the object is unnamed from here on. + EmitDebugObjectNamePacket(device, type, handle, std::string()); +} + void DebugMarker::EmitAllDebugMarkers() { std::lock_guard lock(mutex_); for (const auto& entry : debug_object_names_) { @@ -109,11 +123,11 @@ void DebugMarker::EmitAllDebugMarkers() { } } -bool DebugMarker::HasDebugObjectName(uint64_t device, int32_t type, uint64_t handle, const std::string& name) { +std::optional DebugMarker::GetDebugObjectName(uint64_t device, int32_t type, uint64_t handle) const { std::lock_guard lock(mutex_); auto it = debug_object_names_.find(std::make_tuple(device, type, handle)); - if (it == debug_object_names_.end()) return false; - return it->second.name == name; + if (it == debug_object_names_.end()) return std::nullopt; + return it->second.name; } VkResult DebugMarker::CreateEmulatedMessenger(VkInstance instance, const VkDebugUtilsMessengerCreateInfoEXT* pCreateInfo, diff --git a/layersvt/debug_marker/debug_marker.h b/layersvt/debug_marker/debug_marker.h index fc811bed88..10e26dc07c 100644 --- a/layersvt/debug_marker/debug_marker.h +++ b/layersvt/debug_marker/debug_marker.h @@ -20,6 +20,7 @@ #include #include +#include #include #include #include @@ -63,24 +64,26 @@ class DebugMarker : public layersvt::LayerBase { } /** - * @brief Sets or updates the name associated with a Vulkan object. - * @param device The handle of the Vulkan device that owns the object. - * @param type The type of the Vulkan object (represented as int32_t). - * @param handle The handle of the Vulkan object. - * @param name The name to associate with the object. + * Sets or updates the name associated with a Vulkan object. A null or empty name + * removes the object's name. */ void SetDebugObjectName(uint64_t device, int32_t type, uint64_t handle, const char* name); /** - * @brief Emits all stored debug markers to the tracing system. + * Removes the name associated with a Vulkan object (no-op if unnamed). + */ + void RemoveDebugObjectName(uint64_t device, int32_t type, uint64_t handle); + + /** + * Emits all stored debug markers to the tracing system. */ void EmitAllDebugMarkers(); /** - * @brief Checks if a debug name is stored for a given object. - * @note This function is for testing only. + * Returns the name stored for a Vulkan object, or std::nullopt if the object has no name. + * For testing only. */ - bool HasDebugObjectName(uint64_t device, int32_t type, uint64_t handle, const std::string& name); + std::optional GetDebugObjectName(uint64_t device, int32_t type, uint64_t handle) const; /** * @brief Emulated VK_EXT_debug_utils messengers, used for instances whose downstream lacks the @@ -127,7 +130,7 @@ class DebugMarker : public layersvt::LayerBase { : vk_device(dev), object_type(type), handle(h), name(n) {} }; - std::mutex mutex_; + mutable std::mutex mutex_; /** * @brief Maps (vk_device, object_type, object_handle) to its debug name information. * Non-dispatchable handles are not guaranteed to be unique across object types or devices. diff --git a/layersvt/test/test_debugmarker.cpp b/layersvt/test/test_debugmarker.cpp index 63df65986f..b35465fec0 100644 --- a/layersvt/test/test_debugmarker.cpp +++ b/layersvt/test/test_debugmarker.cpp @@ -60,16 +60,15 @@ TEST_F(DebugMarkerTests, CombinedTest) { // 1. Set instance name DebugMarker::Get().SetDebugObjectName(0, VK_OBJECT_TYPE_INSTANCE, (uint64_t)instance, "MyInstance"); - EXPECT_TRUE(DebugMarker::Get().HasDebugObjectName(0, VK_OBJECT_TYPE_INSTANCE, (uint64_t)instance, "MyInstance")); + EXPECT_EQ(DebugMarker::Get().GetDebugObjectName(0, VK_OBJECT_TYPE_INSTANCE, (uint64_t)instance), "MyInstance"); // 2. Override new name DebugMarker::Get().SetDebugObjectName(0, VK_OBJECT_TYPE_INSTANCE, (uint64_t)instance, "MyInstanceRenamed"); - EXPECT_TRUE(DebugMarker::Get().HasDebugObjectName(0, VK_OBJECT_TYPE_INSTANCE, (uint64_t)instance, "MyInstanceRenamed")); - EXPECT_FALSE(DebugMarker::Get().HasDebugObjectName(0, VK_OBJECT_TYPE_INSTANCE, (uint64_t)instance, "MyInstance")); + EXPECT_EQ(DebugMarker::Get().GetDebugObjectName(0, VK_OBJECT_TYPE_INSTANCE, (uint64_t)instance), "MyInstanceRenamed"); // 3. Reset layer_test::ResetLayer(); - EXPECT_FALSE(DebugMarker::Get().HasDebugObjectName(0, VK_OBJECT_TYPE_INSTANCE, (uint64_t)instance, "MyInstanceRenamed")); + EXPECT_EQ(DebugMarker::Get().GetDebugObjectName(0, VK_OBJECT_TYPE_INSTANCE, (uint64_t)instance), std::nullopt); } TEST_F(DebugMarkerTests, ManifestTest) { @@ -160,18 +159,39 @@ TEST_F(DebugMarkerTests, PreDestroyDeviceCleanupTest) { DebugMarker::Get().SetDebugObjectName((uint64_t)dev1, VK_OBJECT_TYPE_BUFFER, 0x1111, "Buffer1"); DebugMarker::Get().SetDebugObjectName((uint64_t)dev2, VK_OBJECT_TYPE_BUFFER, 0x2222, "Buffer2"); - EXPECT_TRUE(DebugMarker::Get().HasDebugObjectName((uint64_t)dev1, VK_OBJECT_TYPE_BUFFER, 0x1111, "Buffer1")); - EXPECT_TRUE(DebugMarker::Get().HasDebugObjectName((uint64_t)dev2, VK_OBJECT_TYPE_BUFFER, 0x2222, "Buffer2")); + EXPECT_EQ(DebugMarker::Get().GetDebugObjectName((uint64_t)dev1, VK_OBJECT_TYPE_BUFFER, 0x1111), "Buffer1"); + EXPECT_EQ(DebugMarker::Get().GetDebugObjectName((uint64_t)dev2, VK_OBJECT_TYPE_BUFFER, 0x2222), "Buffer2"); // Destroy dev1 - should remove Buffer1 but keep Buffer2 layersvt::LayerBaseTestPeer::DestroyDevice(dev1, nullptr); - EXPECT_FALSE(DebugMarker::Get().HasDebugObjectName((uint64_t)dev1, VK_OBJECT_TYPE_BUFFER, 0x1111, "Buffer1")); - EXPECT_TRUE(DebugMarker::Get().HasDebugObjectName((uint64_t)dev2, VK_OBJECT_TYPE_BUFFER, 0x2222, "Buffer2")); + EXPECT_EQ(DebugMarker::Get().GetDebugObjectName((uint64_t)dev1, VK_OBJECT_TYPE_BUFFER, 0x1111), std::nullopt); + EXPECT_EQ(DebugMarker::Get().GetDebugObjectName((uint64_t)dev2, VK_OBJECT_TYPE_BUFFER, 0x2222), "Buffer2"); // Destroy dev2 - should remove Buffer2 layersvt::LayerBaseTestPeer::DestroyDevice(dev2, nullptr); - EXPECT_FALSE(DebugMarker::Get().HasDebugObjectName((uint64_t)dev2, VK_OBJECT_TYPE_BUFFER, 0x2222, "Buffer2")); + EXPECT_EQ(DebugMarker::Get().GetDebugObjectName((uint64_t)dev2, VK_OBJECT_TYPE_BUFFER, 0x2222), std::nullopt); +} + +TEST_F(DebugMarkerTests, NullOrEmptyNameRemovesNameTest) { + TEST_DESCRIPTION("Verify a null or empty object name removes the stored name instead of storing it"); + + const uint64_t device = 0x1000; + DebugMarker::Get().SetDebugObjectName(device, VK_OBJECT_TYPE_BUFFER, 0x1, "Buffer"); + EXPECT_EQ(DebugMarker::Get().GetDebugObjectName(device, VK_OBJECT_TYPE_BUFFER, 0x1), "Buffer"); + DebugMarker::Get().SetDebugObjectName(device, VK_OBJECT_TYPE_BUFFER, 0x1, nullptr); + EXPECT_EQ(DebugMarker::Get().GetDebugObjectName(device, VK_OBJECT_TYPE_BUFFER, 0x1), std::nullopt); + + DebugMarker::Get().SetDebugObjectName(device, VK_OBJECT_TYPE_IMAGE, 0x2, "Image"); + EXPECT_EQ(DebugMarker::Get().GetDebugObjectName(device, VK_OBJECT_TYPE_IMAGE, 0x2), "Image"); + DebugMarker::Get().SetDebugObjectName(device, VK_OBJECT_TYPE_IMAGE, 0x2, ""); + EXPECT_EQ(DebugMarker::Get().GetDebugObjectName(device, VK_OBJECT_TYPE_IMAGE, 0x2), std::nullopt); + + // Removing the name of an object that was never named leaves other names untouched. + DebugMarker::Get().SetDebugObjectName(device, VK_OBJECT_TYPE_BUFFER, 0x3, "Other"); + DebugMarker::Get().RemoveDebugObjectName(device, VK_OBJECT_TYPE_BUFFER, 0x4); + EXPECT_EQ(DebugMarker::Get().GetDebugObjectName(device, VK_OBJECT_TYPE_BUFFER, 0x3), "Other"); + EXPECT_EQ(DebugMarker::Get().GetDebugObjectName(device, VK_OBJECT_TYPE_BUFFER, 0x4), std::nullopt); } TEST_F(DebugMarkerTests, SameHandleOnDifferentDevicesTest) { @@ -196,16 +216,16 @@ TEST_F(DebugMarkerTests, SameHandleOnDifferentDevicesTest) { DebugMarker::Get().SetDebugObjectName(device1, VK_OBJECT_TYPE_BUFFER, 0x1, "Device1Buffer"); DebugMarker::Get().SetDebugObjectName(device2, VK_OBJECT_TYPE_BUFFER, 0x1, "Device2Buffer"); - EXPECT_TRUE(DebugMarker::Get().HasDebugObjectName(device1, VK_OBJECT_TYPE_BUFFER, 0x1, "Device1Buffer")); - EXPECT_TRUE(DebugMarker::Get().HasDebugObjectName(device2, VK_OBJECT_TYPE_BUFFER, 0x1, "Device2Buffer")); + EXPECT_EQ(DebugMarker::Get().GetDebugObjectName(device1, VK_OBJECT_TYPE_BUFFER, 0x1), "Device1Buffer"); + EXPECT_EQ(DebugMarker::Get().GetDebugObjectName(device2, VK_OBJECT_TYPE_BUFFER, 0x1), "Device2Buffer"); // Destroying device1 must keep the name that device2 stores for the same handle. layersvt::LayerBaseTestPeer::DestroyDevice(dev1, nullptr); - EXPECT_FALSE(DebugMarker::Get().HasDebugObjectName(device1, VK_OBJECT_TYPE_BUFFER, 0x1, "Device1Buffer")); - EXPECT_TRUE(DebugMarker::Get().HasDebugObjectName(device2, VK_OBJECT_TYPE_BUFFER, 0x1, "Device2Buffer")); + EXPECT_EQ(DebugMarker::Get().GetDebugObjectName(device1, VK_OBJECT_TYPE_BUFFER, 0x1), std::nullopt); + EXPECT_EQ(DebugMarker::Get().GetDebugObjectName(device2, VK_OBJECT_TYPE_BUFFER, 0x1), "Device2Buffer"); layersvt::LayerBaseTestPeer::DestroyDevice(dev2, nullptr); - EXPECT_FALSE(DebugMarker::Get().HasDebugObjectName(device2, VK_OBJECT_TYPE_BUFFER, 0x1, "Device2Buffer")); + EXPECT_EQ(DebugMarker::Get().GetDebugObjectName(device2, VK_OBJECT_TYPE_BUFFER, 0x1), std::nullopt); } TEST_F(DebugMarkerTests, TemplateMethodDispatchTest) { @@ -287,7 +307,7 @@ TEST_F(DebugMarkerTests, MissingDownstreamExtensionTest) { name_info.objectHandle = 0x4444; name_info.pObjectName = "Buffer"; EXPECT_EQ(set_object_name(mock_device, &name_info), VK_SUCCESS); - EXPECT_TRUE(DebugMarker::Get().HasDebugObjectName((uint64_t)mock_device, VK_OBJECT_TYPE_BUFFER, 0x4444, "Buffer")); + EXPECT_EQ(DebugMarker::Get().GetDebugObjectName((uint64_t)mock_device, VK_OBJECT_TYPE_BUFFER, 0x4444), "Buffer"); } namespace { @@ -467,7 +487,9 @@ TEST_F(DebugMarkerTests, ForwardedDebugUtilsMessengerTest) { } TEST_F(DebugMarkerTests, ObjectNamePacketFieldsTest) { - TEST_DESCRIPTION("Verify emitted VkDebugUtilsObjectName TracePacket has timestamp_clock_id, pid, and object name fields"); + TEST_DESCRIPTION( + "Verify emitted VkDebugUtilsObjectName TracePackets have timestamp_clock_id, pid, and object name fields, and that " + "removing a name emits an empty name only for named objects"); perfetto::TracingInitArgs args; args.backends = perfetto::kInProcessBackend; @@ -488,8 +510,13 @@ TEST_F(DebugMarkerTests, ObjectNamePacketFieldsTest) { constexpr int32_t kObjectType = VK_OBJECT_TYPE_BUFFER; constexpr uint64_t kObjectHandle = 0x5678u; constexpr const char* kObjectName = "TestBufferName"; + constexpr uint64_t kUnnamedObjectHandle = 0x9abcu; DebugMarker::Get().SetDebugObjectName(kDevice, kObjectType, kObjectHandle, kObjectName); + // Clearing the name emits a second packet with an empty object name. + DebugMarker::Get().SetDebugObjectName(kDevice, kObjectType, kObjectHandle, nullptr); + // Removing the name of an object that was never named emits no packet. + DebugMarker::Get().RemoveDebugObjectName(kDevice, kObjectType, kUnnamedObjectHandle); perfetto::TrackEvent::Flush(); tracing_session->StopBlocking(); @@ -497,7 +524,8 @@ TEST_F(DebugMarkerTests, ObjectNamePacketFieldsTest) { ASSERT_FALSE(raw_trace.empty()); perfetto::protos::pbzero::Trace::Decoder trace(reinterpret_cast(raw_trace.data()), raw_trace.size()); - bool found = false; + std::vector object_names; + bool found_unnamed_object = false; for (auto it = trace.packet(); it; ++it) { perfetto::protos::pbzero::TracePacket::Decoder packet(*it); if (!packet.has_vulkan_api_event()) { @@ -509,10 +537,13 @@ TEST_F(DebugMarkerTests, ObjectNamePacketFieldsTest) { } perfetto::protos::pbzero::VulkanApiEvent_VkDebugUtilsObjectName::Decoder event( vk_event.vk_debug_utils_object_name()); + if (event.object() == kUnnamedObjectHandle) { + found_unnamed_object = true; + continue; + } if (event.object() != kObjectHandle) { continue; } - found = true; EXPECT_TRUE(packet.has_timestamp()); EXPECT_TRUE(packet.has_timestamp_clock_id()); EXPECT_EQ(packet.timestamp_clock_id(), static_cast(perfetto::TrackEvent::GetTraceClockId())); @@ -521,7 +552,8 @@ TEST_F(DebugMarkerTests, ObjectNamePacketFieldsTest) { EXPECT_EQ(event.vk_device(), kDevice); EXPECT_EQ(event.object_type(), kObjectType); EXPECT_EQ(event.object(), kObjectHandle); - EXPECT_EQ(event.object_name().ToStdString(), kObjectName); + object_names.push_back(event.object_name().ToStdString()); } - EXPECT_TRUE(found); + EXPECT_EQ(object_names, (std::vector{kObjectName, ""})); + EXPECT_FALSE(found_unnamed_object); } From bb075be11701ea67e5a627e8ef618e0d6a9d090f Mon Sep 17 00:00:00 2001 From: olehkuznetsov <44271635+olehkuznetsov@users.noreply.github.com> Date: Mon, 26 Oct 2026 17:18:17 +0000 Subject: [PATCH 3/4] layersvt: Remove DebugMarker object names on destruction (#48) DebugMarker kept object names until the device was destroyed. Drivers can reuse handle values for new objects, so a new resource inherited the stale name and trace consumers misattributed it. Erase an object's name when its destroy or free command is called, before dispatching downstream, so a handle reused by another thread cannot pick up the old name. Extension destroy commands are intercepted only when the device exposes them. Bug: Test: new tests - DebugMarkerTests#DestroyedObjectNamesRemovedTest, DebugMarkerTests#DestroyedObjectNamesOfOtherTypesRemovedTest, DebugMarkerTests#ExtensionDestroyCommandRequiresDownstreamTest Change-Id: Iad9147fca27b76c329450f0b9f01802a6a6a6964 --- layersvt/debug_marker/debug_marker.cpp | 77 ++++++++++++++- layersvt/test/test_debugmarker.cpp | 131 +++++++++++++++++++++++++ 2 files changed, 207 insertions(+), 1 deletion(-) diff --git a/layersvt/debug_marker/debug_marker.cpp b/layersvt/debug_marker/debug_marker.cpp index 45fa52af62..c0dd7e8608 100644 --- a/layersvt/debug_marker/debug_marker.cpp +++ b/layersvt/debug_marker/debug_marker.cpp @@ -40,6 +40,72 @@ 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 in practice. 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. The spec +// guarantees unique non-dispatchable handles only when the privateData feature is enabled, so this split +// is a practical heuristic, not a hard rule. +// +// Command buffers and descriptor sets are not covered. vkFreeCommandBuffers and vkFreeDescriptorSets free +// them one at a time, but take an array of handles, which DestroyNamedObject does not support. +// vkDestroyCommandPool, vkResetDescriptorPool and vkDestroyDescriptorPool free them implicitly, which needs +// pool-to-child tracking. Their names stay until the device is destroyed. +// +// The list is deliberately partial. VkShaderEXT, VkDescriptorUpdateTemplate, VkVideoSessionKHR, +// VkVideoSessionParametersKHR and VkMicromapEXT, among others, are not covered yet. Lookup is by command +// name, so a promoted command also needs an entry for each alias (for example +// vkDestroyDescriptorUpdateTemplateKHR). +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 +246,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 +272,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 b35465fec0..7abb600e27 100644 --- a/layersvt/test/test_debugmarker.cpp +++ b/layersvt/test/test_debugmarker.cpp @@ -194,6 +194,137 @@ TEST_F(DebugMarkerTests, NullOrEmptyNameRemovesNameTest) { EXPECT_EQ(DebugMarker::Get().GetDebugObjectName(device, VK_OBJECT_TYPE_BUFFER, 0x4), std::nullopt); } +namespace { + +uint32_t downstream_destroy_calls = 0; +bool buffer_name_removed_before_downstream_destroy = false; + +} // 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; + buffer_name_removed_before_downstream_destroy = false; + 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 device, VkBuffer buffer, const VkAllocationCallbacks*) { + ++downstream_destroy_calls; + buffer_name_removed_before_downstream_destroy = + DebugMarker::Get().GetDebugObjectName((uint64_t)device, VK_OBJECT_TYPE_BUFFER, (uint64_t)buffer) == + std::nullopt; + }); + } + 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); + // The name must already be gone when the downstream destroy runs. + EXPECT_TRUE(buffer_name_removed_before_downstream_destroy); + EXPECT_EQ(DebugMarker::Get().GetDebugObjectName(device, VK_OBJECT_TYPE_DEVICE_MEMORY, 0x1), std::nullopt); + EXPECT_EQ(DebugMarker::Get().GetDebugObjectName(device, VK_OBJECT_TYPE_BUFFER, 0x2), std::nullopt); + EXPECT_EQ(DebugMarker::Get().GetDebugObjectName(device, VK_OBJECT_TYPE_IMAGE, 0x3), std::nullopt); + EXPECT_EQ(DebugMarker::Get().GetDebugObjectName(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_EQ(DebugMarker::Get().GetDebugObjectName(device, VK_OBJECT_TYPE_IMAGE_VIEW, 0x1), std::nullopt); + EXPECT_EQ(DebugMarker::Get().GetDebugObjectName(device, VK_OBJECT_TYPE_PIPELINE, 0x2), std::nullopt); +} + +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_EQ(DebugMarker::Get().GetDebugObjectName((uint64_t)mock_device_with_swapchain, VK_OBJECT_TYPE_SWAPCHAIN_KHR, 0x1), + std::nullopt); +} + TEST_F(DebugMarkerTests, SameHandleOnDifferentDevicesTest) { TEST_DESCRIPTION("Verify identical handles owned by different devices keep separate names"); From 96be52ba6d1c04fbefea62e408fa1da642e4407b Mon Sep 17 00:00:00 2001 From: okuznetsov Date: Wed, 28 Oct 2026 14:02:01 +0000 Subject: [PATCH 4/4] layersvt: Emit empty DebugMarker names on device destruction Destroying a device erased the names of its objects without telling trace consumers. Object names are now removed one by one with an empty name packet, so after device destruction the trace processor still showed the old names. Its lookup has no device key, so an object with the same handle value on a later device could resolve a stale name. Emit an empty name packet for each object name that the device owned, using the same packet as per-object removal. The packets are emitted after the lock is released, because the tracing session start observer takes the same lock to replay all names. Bug: Test: new tests - DebugMarkerTests#DestroyDeviceEmitsEmptyNamePacketsTest Change-Id: I5676ac7a0fe723f32b22c19a05cfe3ef6a6a6964 --- layersvt/debug_marker/debug_marker.cpp | 22 +++++--- layersvt/test/test_debugmarker.cpp | 70 ++++++++++++++++++++++++++ 2 files changed, 86 insertions(+), 6 deletions(-) diff --git a/layersvt/debug_marker/debug_marker.cpp b/layersvt/debug_marker/debug_marker.cpp index c0dd7e8608..a08e5aa084 100644 --- a/layersvt/debug_marker/debug_marker.cpp +++ b/layersvt/debug_marker/debug_marker.cpp @@ -150,15 +150,25 @@ void DebugMarker::PreDestroyInstance(VkInstance instance, const VkAllocationCall void DebugMarker::PreDestroyDevice(VkDevice device, const VkAllocationCallbacks* pAllocator) { (void)pAllocator; - std::lock_guard lock(mutex_); uint64_t dev_handle = (uint64_t)device; - for (auto it = debug_object_names_.begin(); it != debug_object_names_.end();) { - if (it->second.vk_device == dev_handle) { - it = debug_object_names_.erase(it); - } else { - ++it; + std::vector removed_names; + { + std::lock_guard lock(mutex_); + for (auto it = debug_object_names_.begin(); it != debug_object_names_.end();) { + if (it->second.vk_device == dev_handle) { + removed_names.push_back(std::move(it->second)); + it = debug_object_names_.erase(it); + } else { + ++it; + } } } + // Tell trace consumers that the objects of this device are unnamed from here on. The packets are + // emitted after mutex_ is released, because the session start observer takes mutex_ from the tracing + // thread (EmitAllDebugMarkers). The device is still alive here, so its handles cannot be reused yet. + for (const auto& removed : removed_names) { + EmitDebugObjectNamePacket(removed.vk_device, removed.object_type, removed.handle, std::string()); + } } void DebugMarker::SetDebugObjectName(uint64_t device, int32_t type, uint64_t handle, const char* name) { diff --git a/layersvt/test/test_debugmarker.cpp b/layersvt/test/test_debugmarker.cpp index 7abb600e27..23a8f413a9 100644 --- a/layersvt/test/test_debugmarker.cpp +++ b/layersvt/test/test_debugmarker.cpp @@ -20,6 +20,8 @@ #include #include #include +#include +#include #include "test/common/layer_base_test_peer.h" @@ -688,3 +690,71 @@ TEST_F(DebugMarkerTests, ObjectNamePacketFieldsTest) { EXPECT_EQ(object_names, (std::vector{kObjectName, ""})); EXPECT_FALSE(found_unnamed_object); } + +TEST_F(DebugMarkerTests, DestroyDeviceEmitsEmptyNamePacketsTest) { + TEST_DESCRIPTION("Verify destroying a device emits an empty-name packet for each named object of that device only"); + + void* mock_dev1_vtable = reinterpret_cast(0x1000); + VkDevice dev1 = reinterpret_cast(&mock_dev1_vtable); + void* mock_dev2_vtable = reinterpret_cast(0x2000); + VkDevice dev2 = reinterpret_cast(&mock_dev2_vtable); + for (VkDevice dev : {dev1, dev2}) { + layersvt::LayerBaseTestPeer::GetDispatchTableManager(DebugMarker::Get()) + .InitDeviceTable(dev, [](VkDevice, const char*) -> PFN_vkVoidFunction { + return reinterpret_cast(+[](VkDevice, const VkAllocationCallbacks*) {}); + }); + } + + perfetto::TracingInitArgs args; + args.backends = perfetto::kInProcessBackend; + perfetto::Tracing::Initialize(args); + perfetto::TrackEvent::Register(); + + perfetto::TraceConfig cfg; + cfg.add_buffers()->set_size_kb(1024); + cfg.add_data_sources()->mutable_config()->set_name("track_event"); + + auto tracing_session = perfetto::Tracing::NewTrace(); + ASSERT_NE(tracing_session, nullptr); + tracing_session->Setup(cfg); + tracing_session->StartBlocking(); + + const uint64_t device1 = (uint64_t)dev1; + const uint64_t device2 = (uint64_t)dev2; + DebugMarker::Get().SetDebugObjectName(device1, VK_OBJECT_TYPE_BUFFER, 0x1, "Device1Buffer"); + DebugMarker::Get().SetDebugObjectName(device1, VK_OBJECT_TYPE_IMAGE, 0x2, "Device1Image"); + // device2 uses the same buffer handle value as device1. + DebugMarker::Get().SetDebugObjectName(device2, VK_OBJECT_TYPE_BUFFER, 0x1, "Device2Buffer"); + + layersvt::LayerBaseTestPeer::DestroyDevice(dev1, nullptr); + + perfetto::TrackEvent::Flush(); + tracing_session->StopBlocking(); + std::vector raw_trace = tracing_session->ReadTraceBlocking(); + ASSERT_FALSE(raw_trace.empty()); + + std::set> cleared_objects; + perfetto::protos::pbzero::Trace::Decoder trace(reinterpret_cast(raw_trace.data()), raw_trace.size()); + for (auto it = trace.packet(); it; ++it) { + perfetto::protos::pbzero::TracePacket::Decoder packet(*it); + if (!packet.has_vulkan_api_event()) { + continue; + } + perfetto::protos::pbzero::VulkanApiEvent::Decoder vk_event(packet.vulkan_api_event()); + if (!vk_event.has_vk_debug_utils_object_name()) { + continue; + } + perfetto::protos::pbzero::VulkanApiEvent_VkDebugUtilsObjectName::Decoder event(vk_event.vk_debug_utils_object_name()); + if (event.object_name().size == 0) { + cleared_objects.emplace(event.vk_device(), event.object_type(), event.object()); + } + } + const std::set> expected_cleared_objects = { + {device1, VK_OBJECT_TYPE_BUFFER, 0x1}, + {device1, VK_OBJECT_TYPE_IMAGE, 0x2}, + }; + EXPECT_EQ(cleared_objects, expected_cleared_objects); + EXPECT_EQ(DebugMarker::Get().GetDebugObjectName(device2, VK_OBJECT_TYPE_BUFFER, 0x1), "Device2Buffer"); + + layersvt::LayerBaseTestPeer::DestroyDevice(dev2, nullptr); +}