From ce7f8404d4f284f373f05b8ef0be111acddf0e40 Mon Sep 17 00:00:00 2001 From: okuznetsov Date: Mon, 12 Oct 2026 20:20:12 +0100 Subject: [PATCH] layersvt: Key DebugMarker object names by device 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 | 32 ++++++++++++++++++-------- 3 files changed, 30 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..62e11549cd 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,30 @@ 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"); + + const uint64_t device1 = 0x1000; + const uint64_t device2 = 0x2000; + 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")); } TEST_F(DebugMarkerTests, TemplateMethodDispatchTest) { @@ -253,7 +265,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 {