Unchecked usage of optional extension VK_EXT_debug_utils

[RedNicStone]

Context

VK_EXT_debug_utils is a Vulkan extension that allows the developer to gather more feedback about his usage of the API. For more details see the Vulkan registry entry. This extension has not yet been promoted to core, so its availability is optional to the driver.

Issue:

Daxa requests the extension in daxa::ImplInstace::ImplInstance(daxa::InstanceInfo) at Daxa/src/impl_instance.cpp:61: extension_names.push_back(VK_EXT_DEBUG_UTILS_EXTENSION_NAME);

It the tries to acquire a pointer to the vkSetDebugUtilsObjectNameEXT in daxa::ImplDevice::ImplDevice(daxa::DeviceInfo, daxa::ManagedWeakPtr, VkPhysicalDevice) at Daxa/src/impl_device.cpp:714: this->vkSetDebugUtilsObjectNameEXT = reinterpret_cast<PFN_vkSetDebugUtilsObjectNameEXT>(vkGetDeviceProcAddr(this->vk_device, "vkSetDebugUtilsObjectNameEXT"));

This is done without checking the extensions availability using vkEnumerateInstanceExtensionProperties. The same also applies for vkCmdBeginDebugUtilsLabelEXT and vkCmdEndDebugUtilsLabelEXT.

Effect:

Data exits with SIGSEGV (Segmentation fault) when calling daxa::Instance::create_device on platforms that do not support VK_EXT_debug_utils.

Proposed solution:

The existence of VK_EXT_debug_utils should be checked before tying to acquire a function pointer to one of its methods.

Additionally I propose the following changes:

• VK_EXT_debug_utils should only be enabled when compiled under a certain condition, for example only in DEBUG mode. Otherwise this may needlessly consume resources.
• The usage of vkGetDeviceProcAddr in this case is also wrong. Since VK_EXT_debug_utils is an instance level extension the pointer may only be acquired using an instance handle using vkGetInstanceProcAddr. Other use may led to undefined behaviour as stated in the Vulkan registry. The pointer should be acquired using and instance handle in daxa::InstanceImpl::InstanceImpl(daxa::InstanceInfo).

[MatejSakmary]

Thanks for the well written issue! I'll take a look at this when I'm home later today.

[MatejSakmary]

Alternatively - if you want - you can also submit a PR fixing this issue and we will make sure to merge it asap :)

[RedNicStone]

I might do that today if I find some time. I already have a custom branch with some of my own changes, so I should be able to apply a fix and cherry-pick it.

[RedNicStone]

Committed some code to resolve this issue. Having issues with the linker though :/

/usr/bin/ld: module/backend/daxa/lib/Daxa/libdaxa.a(unity_0_cxx.cxx.o): in function `daxa::ImplDevice::ImplDevice(daxa::DeviceInfo, daxa::ManagedWeakPtr, VkPhysicalDevice_T*)':
Embers/module/backend/daxa/lib/Daxa/src/impl_device.cpp:1090: undefined reference to `vkSetDebugUtilsObjectNameEXT'

Seems like the linker cant find the corresponding Vulkan function (although Vulkan is included). Has anyone experienced a similar linker error before?

[MatejSakmary]

Does the issue occur when you try to build in release? nevermind

[Ipotrick]

Added an instance info boolean to en/disable debug utils as a whole https://github.com/Ipotrick/Daxa/commit/775f6a2523d50cc5035ead8e05a2a2ccd86702a7.

[Ipotrick]

idk why your merged changes were lost (probably gabe doing sus things on the git repo again :ogre:).