diff options
author | Charles Giessen <charles@lunarg.com> | 2023-09-05 16:36:57 -0600 |
---|---|---|
committer | Charles Giessen <46324611+charles-lunarg@users.noreply.github.com> | 2023-09-08 15:56:37 -0600 |
commit | 40cebdf5763a0f8181da2b6c2a7ed25738a03e54 (patch) | |
tree | 440326749a6c7cefbc29f388c5d91201015c8375 | |
parent | a145cbe94611404fac9adb7367950c425b0f67fe (diff) | |
download | Vulkan-Loader-40cebdf5763a0f8181da2b6c2a7ed25738a03e54.tar.gz Vulkan-Loader-40cebdf5763a0f8181da2b6c2a7ed25738a03e54.tar.bz2 Vulkan-Loader-40cebdf5763a0f8181da2b6c2a7ed25738a03e54.zip |
Fix incorrect handling of Debug Marker functions
The implementation for VK_KHR_maintenance_5 exposed a bug in how the function
for debug_marker functions were gotten. The bug was to only check if a
driver supports a physical device when querying for device function pointers,
which ignores the possibility of a layer supporting a device extension. This
is an issue for VK_EXT_debug_marker, which requires trampoline functions to
properly unwrap data passed in, resulting in the various debug marker functions
returning NULL instead of a valid function pointer.
-rw-r--r-- | loader/generated/vk_loader_extensions.c | 64 | ||||
-rw-r--r-- | loader/loader.c | 46 | ||||
-rw-r--r-- | loader/loader_common.h | 10 | ||||
-rw-r--r-- | scripts/loader_extension_generator.py | 12 |
4 files changed, 85 insertions, 47 deletions
diff --git a/loader/generated/vk_loader_extensions.c b/loader/generated/vk_loader_extensions.c index b22d9993..97df84de 100644 --- a/loader/generated/vk_loader_extensions.c +++ b/loader/generated/vk_loader_extensions.c @@ -1429,38 +1429,38 @@ void init_extension_device_proc_terminator_dispatch(struct loader_device *dev) { struct loader_device_terminator_dispatch* dispatch = &dev->loader_dispatch.extension_terminator_dispatch; PFN_vkGetDeviceProcAddr gpda = (PFN_vkGetDeviceProcAddr)dev->phys_dev_term->this_icd_term->dispatch.GetDeviceProcAddr; // ---- VK_KHR_swapchain extension commands - if (dev->extensions.khr_swapchain_enabled) + if (dev->driver_extensions.khr_swapchain_enabled) dispatch->CreateSwapchainKHR = (PFN_vkCreateSwapchainKHR)gpda(dev->icd_device, "vkCreateSwapchainKHR"); - if (dev->extensions.khr_swapchain_enabled) + if (dev->driver_extensions.khr_swapchain_enabled) dispatch->GetDeviceGroupSurfacePresentModesKHR = (PFN_vkGetDeviceGroupSurfacePresentModesKHR)gpda(dev->icd_device, "vkGetDeviceGroupSurfacePresentModesKHR"); // ---- VK_KHR_display_swapchain extension commands - if (dev->extensions.khr_display_swapchain_enabled) + if (dev->driver_extensions.khr_display_swapchain_enabled) dispatch->CreateSharedSwapchainsKHR = (PFN_vkCreateSharedSwapchainsKHR)gpda(dev->icd_device, "vkCreateSharedSwapchainsKHR"); // ---- VK_EXT_debug_marker extension commands - if (dev->extensions.ext_debug_marker_enabled) + if (dev->driver_extensions.ext_debug_marker_enabled) dispatch->DebugMarkerSetObjectTagEXT = (PFN_vkDebugMarkerSetObjectTagEXT)gpda(dev->icd_device, "vkDebugMarkerSetObjectTagEXT"); - if (dev->extensions.ext_debug_marker_enabled) + if (dev->driver_extensions.ext_debug_marker_enabled) dispatch->DebugMarkerSetObjectNameEXT = (PFN_vkDebugMarkerSetObjectNameEXT)gpda(dev->icd_device, "vkDebugMarkerSetObjectNameEXT"); // ---- VK_EXT_debug_utils extension commands - if (dev->extensions.ext_debug_utils_enabled) + if (dev->driver_extensions.ext_debug_utils_enabled) dispatch->SetDebugUtilsObjectNameEXT = (PFN_vkSetDebugUtilsObjectNameEXT)gpda(dev->icd_device, "vkSetDebugUtilsObjectNameEXT"); - if (dev->extensions.ext_debug_utils_enabled) + if (dev->driver_extensions.ext_debug_utils_enabled) dispatch->SetDebugUtilsObjectTagEXT = (PFN_vkSetDebugUtilsObjectTagEXT)gpda(dev->icd_device, "vkSetDebugUtilsObjectTagEXT"); - if (dev->extensions.ext_debug_utils_enabled) + if (dev->driver_extensions.ext_debug_utils_enabled) dispatch->QueueBeginDebugUtilsLabelEXT = (PFN_vkQueueBeginDebugUtilsLabelEXT)gpda(dev->icd_device, "vkQueueBeginDebugUtilsLabelEXT"); - if (dev->extensions.ext_debug_utils_enabled) + if (dev->driver_extensions.ext_debug_utils_enabled) dispatch->QueueEndDebugUtilsLabelEXT = (PFN_vkQueueEndDebugUtilsLabelEXT)gpda(dev->icd_device, "vkQueueEndDebugUtilsLabelEXT"); - if (dev->extensions.ext_debug_utils_enabled) + if (dev->driver_extensions.ext_debug_utils_enabled) dispatch->QueueInsertDebugUtilsLabelEXT = (PFN_vkQueueInsertDebugUtilsLabelEXT)gpda(dev->icd_device, "vkQueueInsertDebugUtilsLabelEXT"); - if (dev->extensions.ext_debug_utils_enabled) + if (dev->driver_extensions.ext_debug_utils_enabled) dispatch->CmdBeginDebugUtilsLabelEXT = (PFN_vkCmdBeginDebugUtilsLabelEXT)gpda(dev->icd_device, "vkCmdBeginDebugUtilsLabelEXT"); - if (dev->extensions.ext_debug_utils_enabled) + if (dev->driver_extensions.ext_debug_utils_enabled) dispatch->CmdEndDebugUtilsLabelEXT = (PFN_vkCmdEndDebugUtilsLabelEXT)gpda(dev->icd_device, "vkCmdEndDebugUtilsLabelEXT"); - if (dev->extensions.ext_debug_utils_enabled) + if (dev->driver_extensions.ext_debug_utils_enabled) dispatch->CmdInsertDebugUtilsLabelEXT = (PFN_vkCmdInsertDebugUtilsLabelEXT)gpda(dev->icd_device, "vkCmdInsertDebugUtilsLabelEXT"); #if defined(VK_USE_PLATFORM_WIN32_KHR) // ---- VK_EXT_full_screen_exclusive extension commands - if (dev->extensions.ext_full_screen_exclusive_enabled && dev->extensions.khr_device_group_enabled) + if (dev->driver_extensions.ext_full_screen_exclusive_enabled && dev->driver_extensions.khr_device_group_enabled) dispatch->GetDeviceGroupSurfacePresentModes2EXT = (PFN_vkGetDeviceGroupSurfacePresentModes2EXT)gpda(dev->icd_device, "vkGetDeviceGroupSurfacePresentModes2EXT"); #endif // VK_USE_PLATFORM_WIN32_KHR } @@ -2439,8 +2439,8 @@ VKAPI_ATTR void* VKAPI_CALL loader_lookup_device_dispatch_table(const VkLayerDis if (!strcmp(name, "GetImageSubresourceLayout2KHR")) return (void *)table->GetImageSubresourceLayout2KHR; // ---- VK_EXT_debug_marker extension commands - if (!strcmp(name, "DebugMarkerSetObjectTagEXT")) return dev->extensions.ext_debug_marker_enabled ? (void *)DebugMarkerSetObjectTagEXT : NULL; - if (!strcmp(name, "DebugMarkerSetObjectNameEXT")) return dev->extensions.ext_debug_marker_enabled ? (void *)DebugMarkerSetObjectNameEXT : NULL; + if (!strcmp(name, "DebugMarkerSetObjectTagEXT")) return dev->layer_extensions.ext_debug_marker_enabled ? (void *)DebugMarkerSetObjectTagEXT : NULL; + if (!strcmp(name, "DebugMarkerSetObjectNameEXT")) return dev->layer_extensions.ext_debug_marker_enabled ? (void *)DebugMarkerSetObjectNameEXT : NULL; if (!strcmp(name, "CmdDebugMarkerBeginEXT")) return (void *)table->CmdDebugMarkerBeginEXT; if (!strcmp(name, "CmdDebugMarkerEndEXT")) return (void *)table->CmdDebugMarkerEndEXT; if (!strcmp(name, "CmdDebugMarkerInsertEXT")) return (void *)table->CmdDebugMarkerInsertEXT; @@ -2502,8 +2502,8 @@ VKAPI_ATTR void* VKAPI_CALL loader_lookup_device_dispatch_table(const VkLayerDis if (!strcmp(name, "SetHdrMetadataEXT")) return (void *)table->SetHdrMetadataEXT; // ---- VK_EXT_debug_utils extension commands - if (!strcmp(name, "SetDebugUtilsObjectNameEXT")) return dev->extensions.ext_debug_utils_enabled ? (void *)SetDebugUtilsObjectNameEXT : NULL; - if (!strcmp(name, "SetDebugUtilsObjectTagEXT")) return dev->extensions.ext_debug_utils_enabled ? (void *)SetDebugUtilsObjectTagEXT : NULL; + if (!strcmp(name, "SetDebugUtilsObjectNameEXT")) return dev->layer_extensions.ext_debug_utils_enabled ? (void *)SetDebugUtilsObjectNameEXT : NULL; + if (!strcmp(name, "SetDebugUtilsObjectTagEXT")) return dev->layer_extensions.ext_debug_utils_enabled ? (void *)SetDebugUtilsObjectTagEXT : NULL; if (!strcmp(name, "QueueBeginDebugUtilsLabelEXT")) return (void *)table->QueueBeginDebugUtilsLabelEXT; if (!strcmp(name, "QueueEndDebugUtilsLabelEXT")) return (void *)table->QueueEndDebugUtilsLabelEXT; if (!strcmp(name, "QueueInsertDebugUtilsLabelEXT")) return (void *)table->QueueInsertDebugUtilsLabelEXT; @@ -11385,77 +11385,77 @@ PFN_vkVoidFunction get_extension_device_proc_terminator(struct loader_device *de // ---- VK_KHR_swapchain extension commands if (!strcmp(name, "CreateSwapchainKHR")) { *found_name = true; - return dev->extensions.khr_swapchain_enabled ? + return dev->driver_extensions.khr_swapchain_enabled ? (PFN_vkVoidFunction)terminator_CreateSwapchainKHR : NULL; } if (!strcmp(name, "GetDeviceGroupSurfacePresentModesKHR")) { *found_name = true; - return dev->extensions.khr_swapchain_enabled ? + return dev->driver_extensions.khr_swapchain_enabled ? (PFN_vkVoidFunction)terminator_GetDeviceGroupSurfacePresentModesKHR : NULL; } // ---- VK_KHR_display_swapchain extension commands if (!strcmp(name, "CreateSharedSwapchainsKHR")) { *found_name = true; - return dev->extensions.khr_display_swapchain_enabled ? + return dev->driver_extensions.khr_display_swapchain_enabled ? (PFN_vkVoidFunction)terminator_CreateSharedSwapchainsKHR : NULL; } // ---- VK_EXT_debug_marker extension commands if (!strcmp(name, "DebugMarkerSetObjectTagEXT")) { *found_name = true; - return dev->extensions.ext_debug_marker_enabled ? + return dev->driver_extensions.ext_debug_marker_enabled ? (PFN_vkVoidFunction)terminator_DebugMarkerSetObjectTagEXT : NULL; } if (!strcmp(name, "DebugMarkerSetObjectNameEXT")) { *found_name = true; - return dev->extensions.ext_debug_marker_enabled ? + return dev->driver_extensions.ext_debug_marker_enabled ? (PFN_vkVoidFunction)terminator_DebugMarkerSetObjectNameEXT : NULL; } // ---- VK_EXT_debug_utils extension commands if (!strcmp(name, "SetDebugUtilsObjectNameEXT")) { *found_name = true; - return dev->extensions.ext_debug_utils_enabled ? + return dev->driver_extensions.ext_debug_utils_enabled ? (PFN_vkVoidFunction)terminator_SetDebugUtilsObjectNameEXT : NULL; } if (!strcmp(name, "SetDebugUtilsObjectTagEXT")) { *found_name = true; - return dev->extensions.ext_debug_utils_enabled ? + return dev->driver_extensions.ext_debug_utils_enabled ? (PFN_vkVoidFunction)terminator_SetDebugUtilsObjectTagEXT : NULL; } if (!strcmp(name, "QueueBeginDebugUtilsLabelEXT")) { *found_name = true; - return dev->extensions.ext_debug_utils_enabled ? + return dev->driver_extensions.ext_debug_utils_enabled ? (PFN_vkVoidFunction)terminator_QueueBeginDebugUtilsLabelEXT : NULL; } if (!strcmp(name, "QueueEndDebugUtilsLabelEXT")) { *found_name = true; - return dev->extensions.ext_debug_utils_enabled ? + return dev->driver_extensions.ext_debug_utils_enabled ? (PFN_vkVoidFunction)terminator_QueueEndDebugUtilsLabelEXT : NULL; } if (!strcmp(name, "QueueInsertDebugUtilsLabelEXT")) { *found_name = true; - return dev->extensions.ext_debug_utils_enabled ? + return dev->driver_extensions.ext_debug_utils_enabled ? (PFN_vkVoidFunction)terminator_QueueInsertDebugUtilsLabelEXT : NULL; } if (!strcmp(name, "CmdBeginDebugUtilsLabelEXT")) { *found_name = true; - return dev->extensions.ext_debug_utils_enabled ? + return dev->driver_extensions.ext_debug_utils_enabled ? (PFN_vkVoidFunction)terminator_CmdBeginDebugUtilsLabelEXT : NULL; } if (!strcmp(name, "CmdEndDebugUtilsLabelEXT")) { *found_name = true; - return dev->extensions.ext_debug_utils_enabled ? + return dev->driver_extensions.ext_debug_utils_enabled ? (PFN_vkVoidFunction)terminator_CmdEndDebugUtilsLabelEXT : NULL; } if (!strcmp(name, "CmdInsertDebugUtilsLabelEXT")) { *found_name = true; - return dev->extensions.ext_debug_utils_enabled ? + return dev->driver_extensions.ext_debug_utils_enabled ? (PFN_vkVoidFunction)terminator_CmdInsertDebugUtilsLabelEXT : NULL; } #if defined(VK_USE_PLATFORM_WIN32_KHR) // ---- VK_EXT_full_screen_exclusive extension commands if (!strcmp(name, "GetDeviceGroupSurfacePresentModes2EXT")) { *found_name = true; - return dev->extensions.ext_full_screen_exclusive_enabled && dev->extensions.khr_device_group_enabled ? + return dev->driver_extensions.ext_full_screen_exclusive_enabled && dev->driver_extensions.khr_device_group_enabled ? (PFN_vkVoidFunction)terminator_GetDeviceGroupSurfacePresentModes2EXT : NULL; } #endif // VK_USE_PLATFORM_WIN32_KHR diff --git a/loader/loader.c b/loader/loader.c index fdecd938..52ff44c1 100644 --- a/loader/loader.c +++ b/loader/loader.c @@ -4235,6 +4235,33 @@ bool loader_get_layer_interface_version(PFN_vkNegotiateLoaderLayerInterfaceVersi return true; } +// Every extension that has a loader-defined trampoline needs to be marked as enabled or disabled so that we know whether or +// not to return that trampoline when vkGetDeviceProcAddr is called +void setup_logical_device_enabled_layer_extensions(const struct loader_instance *inst, struct loader_device *dev, + const struct loader_extension_list *icd_exts, + const VkDeviceCreateInfo *pCreateInfo) { + // Can only setup debug marker as debug utils is an instance extensions. + for (uint32_t i = 0; i < pCreateInfo->enabledExtensionCount; ++i) { + if (!strcmp(pCreateInfo->ppEnabledExtensionNames[i], VK_EXT_DEBUG_MARKER_EXTENSION_NAME)) { + // Check if its supported by the driver + for (uint32_t j = 0; j < icd_exts->count; ++j) { + if (!strcmp(icd_exts->list[j].extensionName, VK_EXT_DEBUG_MARKER_EXTENSION_NAME)) { + dev->layer_extensions.ext_debug_marker_enabled = true; + } + } + // also check if any layers support it. + for (uint32_t j = 0; j < inst->app_activated_layer_list.count; j++) { + struct loader_layer_properties *layer = inst->app_activated_layer_list.list[j]; + for (uint32_t k = 0; k < layer->device_extension_list.count; k++) { + if (!strcmp(layer->device_extension_list.list[k].props.extensionName, VK_EXT_DEBUG_MARKER_EXTENSION_NAME)) { + dev->layer_extensions.ext_debug_marker_enabled = true; + } + } + } + } + } +} + VKAPI_ATTR VkResult VKAPI_CALL loader_layer_create_device(VkInstance instance, VkPhysicalDevice physicalDevice, const VkDeviceCreateInfo *pCreateInfo, const VkAllocationCallbacks *pAllocator, VkDevice *pDevice, @@ -4288,6 +4315,8 @@ VKAPI_ATTR VkResult VKAPI_CALL loader_layer_create_device(VkInstance instance, V goto out; } + setup_logical_device_enabled_layer_extensions(inst, dev, &icd_exts, pCreateInfo); + res = loader_create_device_chain(internal_device, pCreateInfo, pAllocator, inst, dev, layerGIPA, nextGDPA); if (res != VK_SUCCESS) { loader_log(inst, VULKAN_LOADER_ERROR_BIT, 0, "vkCreateDevice: Failed to create device chain."); @@ -5766,27 +5795,28 @@ VKAPI_ATTR VkResult VKAPI_CALL terminator_CreateDevice(VkPhysicalDevice physical // not to return that terminator when vkGetDeviceProcAddr is called for (uint32_t i = 0; i < localCreateInfo.enabledExtensionCount; ++i) { if (!strcmp(localCreateInfo.ppEnabledExtensionNames[i], VK_KHR_SWAPCHAIN_EXTENSION_NAME)) { - dev->extensions.khr_swapchain_enabled = true; + dev->driver_extensions.khr_swapchain_enabled = true; } else if (!strcmp(localCreateInfo.ppEnabledExtensionNames[i], VK_KHR_DISPLAY_SWAPCHAIN_EXTENSION_NAME)) { - dev->extensions.khr_display_swapchain_enabled = true; + dev->driver_extensions.khr_display_swapchain_enabled = true; } else if (!strcmp(localCreateInfo.ppEnabledExtensionNames[i], VK_KHR_DEVICE_GROUP_EXTENSION_NAME)) { - dev->extensions.khr_device_group_enabled = true; + dev->driver_extensions.khr_device_group_enabled = true; } else if (!strcmp(localCreateInfo.ppEnabledExtensionNames[i], VK_EXT_DEBUG_MARKER_EXTENSION_NAME)) { - dev->extensions.ext_debug_marker_enabled = true; + dev->driver_extensions.ext_debug_marker_enabled = true; } else if (!strcmp(localCreateInfo.ppEnabledExtensionNames[i], "VK_EXT_full_screen_exclusive")) { - dev->extensions.ext_full_screen_exclusive_enabled = true; + dev->driver_extensions.ext_full_screen_exclusive_enabled = true; } else if (!strcmp(localCreateInfo.ppEnabledExtensionNames[i], VK_KHR_MAINTENANCE_5_EXTENSION_NAME) && maintenance5_feature_enabled) { dev->should_ignore_device_commands_from_newer_version = true; } } - dev->extensions.ext_debug_utils_enabled = icd_term->this_instance->enabled_known_extensions.ext_debug_utils; + dev->layer_extensions.ext_debug_utils_enabled = icd_term->this_instance->enabled_known_extensions.ext_debug_utils; + dev->driver_extensions.ext_debug_utils_enabled = icd_term->this_instance->enabled_known_extensions.ext_debug_utils; VkPhysicalDeviceProperties properties; icd_term->dispatch.GetPhysicalDeviceProperties(phys_dev_term->phys_dev, &properties); - if (!dev->extensions.khr_device_group_enabled) { + if (!dev->driver_extensions.khr_device_group_enabled) { if (properties.apiVersion >= VK_API_VERSION_1_1) { - dev->extensions.khr_device_group_enabled = true; + dev->driver_extensions.khr_device_group_enabled = true; } } diff --git a/loader/loader_common.h b/loader/loader_common.h index 782e0d4c..2ff67f86 100644 --- a/loader/loader_common.h +++ b/loader/loader_common.h @@ -189,6 +189,14 @@ struct loader_device { VkAllocationCallbacks alloc_callbacks; + // List of activated device extensions that layers support (but not necessarily the driver which have functions that require + // trampolines to work correctly. EX - vkDebugMarkerSetObjectNameEXT can name wrapped handles like instance, physical device, + // or surface + struct { + bool ext_debug_marker_enabled; + bool ext_debug_utils_enabled; + } layer_extensions; + // List of activated device extensions that have terminators implemented in the loader struct { bool khr_swapchain_enabled; @@ -197,7 +205,7 @@ struct loader_device { bool ext_debug_marker_enabled; bool ext_debug_utils_enabled; bool ext_full_screen_exclusive_enabled; - } extensions; + } driver_extensions; struct loader_device *next; diff --git a/scripts/loader_extension_generator.py b/scripts/loader_extension_generator.py index a9c43aa2..c1c70e79 100644 --- a/scripts/loader_extension_generator.py +++ b/scripts/loader_extension_generator.py @@ -936,9 +936,9 @@ class LoaderExtensionOutputGenerator(OutputGenerator): tables += f' if (!strcmp(name, "{base_name}")) ' if cur_cmd.name in DEVICE_CMDS_MUST_USE_TRAMP: if version_check != '': - tables += f'{{\n{version_check} return dev->extensions.{cur_cmd.ext_name[3:].lower()}_enabled ? (void *){base_name} : NULL;\n }}\n' + tables += f'{{\n{version_check} return dev->layer_extensions.{cur_cmd.ext_name[3:].lower()}_enabled ? (void *){base_name} : NULL;\n }}\n' else: - tables += f'return dev->extensions.{cur_cmd.ext_name[3:].lower()}_enabled ? (void *){base_name} : NULL;\n' + tables += f'return dev->layer_extensions.{cur_cmd.ext_name[3:].lower()}_enabled ? (void *){base_name} : NULL;\n' else: if version_check != '': @@ -1501,9 +1501,9 @@ class LoaderExtensionOutputGenerator(OutputGenerator): term_func += f' if (!strcmp(name, "{ext_cmd.name[2:]}")) {{\n' term_func += f' *found_name = true;\n' if ext_cmd.require: - term_func += f' return dev->extensions.{ext_cmd.ext_name[3:].lower()}_enabled && dev->extensions.{ext_cmd.require[3:].lower()}_enabled ?\n' + term_func += f' return dev->driver_extensions.{ext_cmd.ext_name[3:].lower()}_enabled && dev->driver_extensions.{ext_cmd.require[3:].lower()}_enabled ?\n' else: - term_func += f' return dev->extensions.{ext_cmd.ext_name[3:].lower()}_enabled ?\n' + term_func += f' return dev->driver_extensions.{ext_cmd.ext_name[3:].lower()}_enabled ?\n' term_func += f' (PFN_vkVoidFunction)terminator_{(ext_cmd.name[2:])} : NULL;\n' term_func += f' }}\n' @@ -1598,10 +1598,10 @@ class LoaderExtensionOutputGenerator(OutputGenerator): if ext_cmd.require: - term_func += f' if (dev->extensions.{ext_cmd.ext_name[3:].lower()}_enabled && dev->extensions.{ext_cmd.require[3:].lower()}_enabled)\n' + term_func += f' if (dev->driver_extensions.{ext_cmd.ext_name[3:].lower()}_enabled && dev->driver_extensions.{ext_cmd.require[3:].lower()}_enabled)\n' term_func += f' dispatch->{ext_cmd.name[2:]} = (PFN_{(ext_cmd.name)})gpda(dev->icd_device, "{(ext_cmd.name)}");\n' else: - term_func += f' if (dev->extensions.{ext_cmd.ext_name[3:].lower()}_enabled)\n' + term_func += f' if (dev->driver_extensions.{ext_cmd.ext_name[3:].lower()}_enabled)\n' term_func += f' dispatch->{ext_cmd.name[2:]} = (PFN_{(ext_cmd.name)})gpda(dev->icd_device, "{(ext_cmd.name)}");\n' if last_protect is not None: |