diff --git a/.github/workflows/build_test.yaml b/.github/workflows/build_test.yaml index 3d8a852..0785228 100644 --- a/.github/workflows/build_test.yaml +++ b/.github/workflows/build_test.yaml @@ -80,6 +80,15 @@ jobs: cmake -G "Unix Makefiles" -DCMAKE_BUILD_TYPE=Release .. make -j4 + - name: Build layer_gpu_profile + run: | + export CC=clang + export CXX=clang++ + mkdir layer_gpu_profile/build_rel + cd layer_gpu_profile/build_rel + cmake -G "Unix Makefiles" -DCMAKE_BUILD_TYPE=Release .. + make -j4 + - name: Build and run unit tests run: | export CC=clang @@ -126,6 +135,15 @@ jobs: cmake -G "Unix Makefiles" -DCMAKE_BUILD_TYPE=Release .. make -j4 + - name: Build layer_gpu_profile + run: | + export CC=gcc + export CXX=g++ + mkdir layer_gpu_profile/build_rel + cd layer_gpu_profile/build_rel + cmake -G "Unix Makefiles" -DCMAKE_BUILD_TYPE=Release .. + make -j4 + build-android: name: Android runs-on: ubuntu-22.04 @@ -150,6 +168,11 @@ jobs: cd layer_gpu_timeline bash ./android_build.sh Release + - name: Build layer_gpu_profile + run: | + cd layer_gpu_profile + bash ./android_build.sh Release + build-ubuntu-x64-clang-new-common: name: Ubuntu x64 generate common runs-on: ubuntu-22.04 diff --git a/generator/vk_layer/source/device.cpp b/generator/vk_layer/source/device.cpp index be5de52..f1ea68d 100644 --- a/generator/vk_layer/source/device.cpp +++ b/generator/vk_layer/source/device.cpp @@ -79,10 +79,15 @@ Device* Device::retrieve( } /* See header for documentation. */ -void Device::destroy( - Device* device +std::unique_ptr Device::destroy( + VkDevice handle ) { - g_devices.erase(getDispatchKey(device)); + void* key = getDispatchKey(handle); + assert(isInMap(key, g_devices)); + + auto device = std::move(g_devices.at(key)); + g_devices.erase(key); + return device; } /* See header for documentation. */ diff --git a/generator/vk_layer/source/device.hpp b/generator/vk_layer/source/device.hpp index d1ee02a..607a18d 100644 --- a/generator/vk_layer/source/device.hpp +++ b/generator/vk_layer/source/device.hpp @@ -115,12 +115,17 @@ class Device VkCommandBuffer handle); /** - * @brief Drop a device from the global store of dispatchable devices. + * \brief Drop a device from the global store of dispatchable devices. * - * @param device The device to drop. + * This must be called before the driver VkDevice has been destroyed, as + * we deference the native device handle to get the dispatch key. + * + * \param handle The dispatchable device handle to use as an indirect lookup. + * + * \return Returns the ownership of the Device object to the caller. */ - static void destroy( - Device* device); + static std::unique_ptr destroy( + VkDevice handle); /** * @brief Create a new layer device object. diff --git a/generator/vk_layer/source/instance.cpp b/generator/vk_layer/source/instance.cpp index 8c1adb4..164ffb4 100644 --- a/generator/vk_layer/source/instance.cpp +++ b/generator/vk_layer/source/instance.cpp @@ -70,10 +70,15 @@ Instance* Instance::retrieve( } /* See header for documentation. */ -void Instance::destroy( - Instance* instance +std::unique_ptr Instance::destroy( + VkInstance handle ) { - g_instances.erase(getDispatchKey(instance->instance)); + void* key = getDispatchKey(handle); + assert(isInMap(key, g_instances)); + + auto instance = std::move(g_instances.at(key)); + g_instances.erase(key); + return instance; } /* See header for documentation. */ diff --git a/generator/vk_layer/source/instance.hpp b/generator/vk_layer/source/instance.hpp index b6bdea1..f9b20a5 100644 --- a/generator/vk_layer/source/instance.hpp +++ b/generator/vk_layer/source/instance.hpp @@ -99,12 +99,17 @@ class Instance VkPhysicalDevice handle); /** - * @brief Drop an instance from the global store of dispatchable instances. + * \brief Drop an instance from the global store of dispatchable instances. * - * @param instance The instance to drop. + * This must be called before the driver VkInstance has been destroyed, as + * we deference the native instance handle to get the dispatch key. + * + * \param handle The dispatchable instance handle to use as an indirect lookup. + * + * \return Returns the ownership of the Instance object to the caller. */ - static void destroy( - Instance* instance); + static std::unique_ptr destroy( + VkInstance handle); /** * @brief Create a new layer instance object. diff --git a/layer_example/source/device.cpp b/layer_example/source/device.cpp index fcf2532..9dfa233 100644 --- a/layer_example/source/device.cpp +++ b/layer_example/source/device.cpp @@ -75,9 +75,15 @@ Device* Device::retrieve(VkCommandBuffer handle) } /* See header for documentation. */ -void Device::destroy(Device* device) -{ - g_devices.erase(getDispatchKey(device)); +std::unique_ptr Device::destroy( + VkDevice handle +) { + void* key = getDispatchKey(handle); + assert(isInMap(key, g_devices)); + + auto device = std::move(g_devices.at(key)); + g_devices.erase(key); + return device; } /* See header for documentation. */ diff --git a/layer_example/source/device.hpp b/layer_example/source/device.hpp index 0ce2e92..3036d3e 100644 --- a/layer_example/source/device.hpp +++ b/layer_example/source/device.hpp @@ -110,11 +110,17 @@ class Device static Device* retrieve(VkCommandBuffer handle); /** - * @brief Drop a device from the global store of dispatchable devices. + * \brief Drop a device from the global store of dispatchable devices. * - * @param device The device to drop. + * This must be called before the driver VkDevice has been destroyed, as + * we deference the native device handle to get the dispatch key. + * + * \param handle The dispatchable device handle to use as an indirect lookup. + * + * \return Returns the ownership of the Device object to the caller. */ - static void destroy(Device* device); + static std::unique_ptr destroy( + VkDevice handle); /** * @brief Create a new layer device object. diff --git a/layer_example/source/instance.cpp b/layer_example/source/instance.cpp index 42db622..78a7941 100644 --- a/layer_example/source/instance.cpp +++ b/layer_example/source/instance.cpp @@ -64,9 +64,15 @@ Instance* Instance::retrieve(VkPhysicalDevice handle) } /* See header for documentation. */ -void Instance::destroy(Instance* instance) -{ - g_instances.erase(getDispatchKey(instance->instance)); +std::unique_ptr Instance::destroy( + VkInstance handle +) { + void* key = getDispatchKey(handle); + assert(isInMap(key, g_instances)); + + auto instance = std::move(g_instances.at(key)); + g_instances.erase(key); + return instance; } /* See header for documentation. */ diff --git a/layer_example/source/instance.hpp b/layer_example/source/instance.hpp index f3ffc7a..16ac5e5 100644 --- a/layer_example/source/instance.hpp +++ b/layer_example/source/instance.hpp @@ -95,11 +95,17 @@ class Instance static Instance* retrieve(VkPhysicalDevice handle); /** - * @brief Drop an instance from the global store of dispatchable instances. + * \brief Drop an instance from the global store of dispatchable instances. * - * @param instance The instance to drop. + * This must be called before the driver VkInstance has been destroyed, as + * we deference the native instance handle to get the dispatch key. + * + * \param handle The dispatchable instance handle to use as an indirect lookup. + * + * \return Returns the ownership of the Instance object to the caller. */ - static void destroy(Instance* instance); + static std::unique_ptr destroy( + VkInstance handle); /** * @brief Create a new layer instance object. diff --git a/layer_gpu_profile/source/device.cpp b/layer_gpu_profile/source/device.cpp index b040669..27e5af6 100644 --- a/layer_gpu_profile/source/device.cpp +++ b/layer_gpu_profile/source/device.cpp @@ -82,9 +82,15 @@ Device* Device::retrieve(VkCommandBuffer handle) } /* See header for documentation. */ -void Device::destroy(Device* device) -{ - g_devices.erase(getDispatchKey(device)); +std::unique_ptr Device::destroy( + VkDevice handle +) { + void* key = getDispatchKey(handle); + assert(isInMap(key, g_devices)); + + auto device = std::move(g_devices.at(key)); + g_devices.erase(key); + return device; } /* See header for documentation. */ diff --git a/layer_gpu_profile/source/device.hpp b/layer_gpu_profile/source/device.hpp index b52a47a..a4f6619 100644 --- a/layer_gpu_profile/source/device.hpp +++ b/layer_gpu_profile/source/device.hpp @@ -118,11 +118,17 @@ class Device static Device* retrieve(VkCommandBuffer handle); /** - * @brief Drop a device from the global store of dispatchable devices. + * \brief Drop a device from the global store of dispatchable devices. * - * @param device The device to drop. + * This must be called before the driver VkDevice has been destroyed, as + * we deference the native device handle to get the dispatch key. + * + * \param handle The dispatchable device handle to use as an indirect lookup. + * + * \return Returns the ownership of the Device object to the caller. */ - static void destroy(Device* device); + static std::unique_ptr destroy( + VkDevice handle); /** * @brief Create a new layer device object. diff --git a/layer_gpu_profile/source/instance.cpp b/layer_gpu_profile/source/instance.cpp index d567bbb..71bd4c1 100644 --- a/layer_gpu_profile/source/instance.cpp +++ b/layer_gpu_profile/source/instance.cpp @@ -66,9 +66,15 @@ Instance* Instance::retrieve(VkPhysicalDevice handle) } /* See header for documentation. */ -void Instance::destroy(Instance* instance) -{ - g_instances.erase(getDispatchKey(instance->instance)); +std::unique_ptr Instance::destroy( + VkInstance handle +) { + void* key = getDispatchKey(handle); + assert(isInMap(key, g_instances)); + + auto instance = std::move(g_instances.at(key)); + g_instances.erase(key); + return instance; } /* See header for documentation. */ diff --git a/layer_gpu_profile/source/instance.hpp b/layer_gpu_profile/source/instance.hpp index 878c84c..50b109b 100644 --- a/layer_gpu_profile/source/instance.hpp +++ b/layer_gpu_profile/source/instance.hpp @@ -96,11 +96,17 @@ class Instance static Instance* retrieve(VkPhysicalDevice handle); /** - * @brief Drop an instance from the global store of dispatchable instances. + * \brief Drop an instance from the global store of dispatchable instances. * - * @param instance The instance to drop. + * This must be called before the driver VkInstance has been destroyed, as + * we deference the native instance handle to get the dispatch key. + * + * \param handle The dispatchable instance handle to use as an indirect lookup. + * + * \return Returns the ownership of the Instance object to the caller. */ - static void destroy(Instance* instance); + static std::unique_ptr destroy( + VkInstance handle); /** * @brief Create a new layer instance object. diff --git a/layer_gpu_support/source/device.cpp b/layer_gpu_support/source/device.cpp index 0280e50..f3cdeab 100644 --- a/layer_gpu_support/source/device.cpp +++ b/layer_gpu_support/source/device.cpp @@ -80,9 +80,15 @@ Device* Device::retrieve(VkCommandBuffer handle) } /* See header for documentation. */ -void Device::destroy(Device* device) -{ - g_devices.erase(getDispatchKey(device)); +std::unique_ptr Device::destroy( + VkDevice handle +) { + void* key = getDispatchKey(handle); + assert(isInMap(key, g_devices)); + + auto device = std::move(g_devices.at(key)); + g_devices.erase(key); + return device; } /* See header for documentation. */ diff --git a/layer_gpu_support/source/device.hpp b/layer_gpu_support/source/device.hpp index 5128780..d9bbcbc 100644 --- a/layer_gpu_support/source/device.hpp +++ b/layer_gpu_support/source/device.hpp @@ -111,11 +111,17 @@ class Device static Device* retrieve(VkCommandBuffer handle); /** - * @brief Drop a device from the global store of dispatchable devices. + * \brief Drop a device from the global store of dispatchable devices. * - * @param device The device to drop. + * This must be called before the driver VkDevice has been destroyed, as + * we deference the native device handle to get the dispatch key. + * + * \param handle The dispatchable device handle to use as an indirect lookup. + * + * \return Returns the ownership of the Device object to the caller. */ - static void destroy(Device* device); + static std::unique_ptr destroy( + VkDevice handle); /** * @brief Create a new layer device object. diff --git a/layer_gpu_support/source/instance.cpp b/layer_gpu_support/source/instance.cpp index d567bbb..71bd4c1 100644 --- a/layer_gpu_support/source/instance.cpp +++ b/layer_gpu_support/source/instance.cpp @@ -66,9 +66,15 @@ Instance* Instance::retrieve(VkPhysicalDevice handle) } /* See header for documentation. */ -void Instance::destroy(Instance* instance) -{ - g_instances.erase(getDispatchKey(instance->instance)); +std::unique_ptr Instance::destroy( + VkInstance handle +) { + void* key = getDispatchKey(handle); + assert(isInMap(key, g_instances)); + + auto instance = std::move(g_instances.at(key)); + g_instances.erase(key); + return instance; } /* See header for documentation. */ diff --git a/layer_gpu_support/source/instance.hpp b/layer_gpu_support/source/instance.hpp index b38f2bd..6cbbfa2 100644 --- a/layer_gpu_support/source/instance.hpp +++ b/layer_gpu_support/source/instance.hpp @@ -95,11 +95,17 @@ class Instance static Instance* retrieve(VkPhysicalDevice handle); /** - * @brief Drop an instance from the global store of dispatchable instances. + * \brief Drop an instance from the global store of dispatchable instances. * - * @param instance The instance to drop. + * This must be called before the driver VkInstance has been destroyed, as + * we deference the native instance handle to get the dispatch key. + * + * \param handle The dispatchable instance handle to use as an indirect lookup. + * + * \return Returns the ownership of the Instance object to the caller. */ - static void destroy(Instance* instance); + static std::unique_ptr destroy( + VkInstance handle); /** * @brief Create a new layer instance object. diff --git a/layer_gpu_timeline/source/device.cpp b/layer_gpu_timeline/source/device.cpp index ab5e913..3c928c3 100644 --- a/layer_gpu_timeline/source/device.cpp +++ b/layer_gpu_timeline/source/device.cpp @@ -78,9 +78,15 @@ Device* Device::retrieve(VkCommandBuffer handle) } /* See header for documentation. */ -void Device::destroy(Device* device) -{ - g_devices.erase(getDispatchKey(device)); +std::unique_ptr Device::destroy( + VkDevice handle +) { + void* key = getDispatchKey(handle); + assert(isInMap(key, g_devices)); + + auto device = std::move(g_devices.at(key)); + g_devices.erase(key); + return device; } /* See header for documentation. */ diff --git a/layer_gpu_timeline/source/device.hpp b/layer_gpu_timeline/source/device.hpp index 77a0163..4d9245b 100644 --- a/layer_gpu_timeline/source/device.hpp +++ b/layer_gpu_timeline/source/device.hpp @@ -113,11 +113,17 @@ class Device static Device* retrieve(VkCommandBuffer handle); /** - * @brief Drop a device from the global store of dispatchable devices. + * \brief Drop a device from the global store of dispatchable devices. * - * @param device The device to drop. + * This must be called before the driver VkDevice has been destroyed, as + * we deference the native device handle to get the dispatch key. + * + * \param handle The dispatchable device handle to use as an indirect lookup. + * + * \return Returns the ownership of the Device object to the caller. */ - static void destroy(Device* device); + static std::unique_ptr destroy( + VkDevice handle); /** * @brief Create a new layer device object. diff --git a/layer_gpu_timeline/source/instance.cpp b/layer_gpu_timeline/source/instance.cpp index d567bbb..71bd4c1 100644 --- a/layer_gpu_timeline/source/instance.cpp +++ b/layer_gpu_timeline/source/instance.cpp @@ -66,9 +66,15 @@ Instance* Instance::retrieve(VkPhysicalDevice handle) } /* See header for documentation. */ -void Instance::destroy(Instance* instance) -{ - g_instances.erase(getDispatchKey(instance->instance)); +std::unique_ptr Instance::destroy( + VkInstance handle +) { + void* key = getDispatchKey(handle); + assert(isInMap(key, g_instances)); + + auto instance = std::move(g_instances.at(key)); + g_instances.erase(key); + return instance; } /* See header for documentation. */ diff --git a/layer_gpu_timeline/source/instance.hpp b/layer_gpu_timeline/source/instance.hpp index f3ffc7a..16ac5e5 100644 --- a/layer_gpu_timeline/source/instance.hpp +++ b/layer_gpu_timeline/source/instance.hpp @@ -95,11 +95,17 @@ class Instance static Instance* retrieve(VkPhysicalDevice handle); /** - * @brief Drop an instance from the global store of dispatchable instances. + * \brief Drop an instance from the global store of dispatchable instances. * - * @param instance The instance to drop. + * This must be called before the driver VkInstance has been destroyed, as + * we deference the native instance handle to get the dispatch key. + * + * \param handle The dispatchable instance handle to use as an indirect lookup. + * + * \return Returns the ownership of the Instance object to the caller. */ - static void destroy(Instance* instance); + static std::unique_ptr destroy( + VkInstance handle); /** * @brief Create a new layer instance object. diff --git a/source_common/framework/manual_functions.cpp b/source_common/framework/manual_functions.cpp index f651ab9..9428612 100644 --- a/source_common/framework/manual_functions.cpp +++ b/source_common/framework/manual_functions.cpp @@ -749,17 +749,13 @@ VKAPI_ATTR void VKAPI_CALL layer_vkDestroyInstance_default(VkInstance instance, // Hold the lock to access layer-wide global store std::unique_lock lock {g_vulkanLock}; - auto* layer = Instance::retrieve(instance); - // Save the driver function to avoid a use-after free when proxy is destroyed - auto destroyInstance = layer->driver.vkDestroyInstance; - - // Layer proxy must be destroyed before the driver object as we use its dispatchable handle - Instance::destroy(layer); + // Take local ownership of the Instance before calling the driver + auto layer = Instance::destroy(instance); // Release the lock to call into the driver lock.unlock(); - destroyInstance(instance, pAllocator); + layer->driver.vkDestroyInstance(instance, pAllocator); } /* See Vulkan API for documentation. */ @@ -839,17 +835,13 @@ VKAPI_ATTR void VKAPI_CALL layer_vkDestroyDevice_default(VkDevice device, const // Hold the lock to access layer-wide global store std::unique_lock lock {g_vulkanLock}; - auto* layer = Device::retrieve(device); - - // Save the driver function to avoid a use-after free when proxy is destroyed - auto destroyDevice = layer->driver.vkDestroyDevice; - // Layer proxy must be destroyed before the driver object as we use its dispatchable handle - Device::destroy(layer); + // Take local ownership of the Device before calling the driver + auto layer = Device::destroy(device); // Release the lock to call into the driver lock.unlock(); - destroyDevice(device, pAllocator); + layer->driver.vkDestroyDevice(device, pAllocator); } /* See Vulkan API for documentation. */