18 Nov
2020
18 Nov
'20
10:42 p.m.
I was testing Serious Sam 4 crash on start on Nvidia and found that it also happens due to debug function implementation in winevulkan. More precisely, in case of Serious Sam, the crash happens in host vkSetDebugUtilsObjectNameEXT() when called with objectType VK_OBJECT_TYPE_SURFACE_KHR. wine_vk_unwrap_handle() returns an unwrapped handler for this object type, while the native handle is actually wrapped in struct wine_vk_surfaceby wine driver (winex11.drv). I tested that correctly unwrapping it fixes the crash.
I am a bit unsure how to properly fix it. The straightforward way is to pull all the debug functions to wine[x11|mac].drv, but that means pulling rather lot. Besides, debugging functions have to be queried from device which winex11 currently doesn't do so we would probably need to also pull vkCreateDevice() to initialize native function pointers. The other much shorter way would be to export the unwrapping function from the wine driver which will be unwrapping handles wrapped in there. It would probably be unfortunate to introduce a special export for that, maybe we could put it in the generated 'struct vulkan_funcs' somehow?
Any suggestions?
On 10/6/20 06:35, Thomas Crider wrote:
Just a heads up, I was testing these patches and found they cause Horizon Zero Dawn to crash on startup on nvidia. works fine on AMD/radv
On Tue, Sep 29, 2020 at 5:21 PM Liam Middlebrook <lmiddlebrook@nvidia.com mailto:lmiddlebrook@nvidia.com> wrote:
This looks good to me, but given the requested changes to dependent patches 1/3 and 2/3, I'll wait to sign-off on this one. Thanks, Liam Middlebrook On 9/25/20 8:16 AM, Georg Lehmann wrote: > Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=49813 <https://bugs.winehq.org/show_bug.cgi?id=49813> > > Signed-off-by: Georg Lehmann <dadschoorse@gmail.com <mailto:dadschoorse@gmail.com>> > --- > dlls/winevulkan/make_vulkan | 8 +- > dlls/winevulkan/vulkan.c | 183 +++++++++++++++++++++++++++++++ > dlls/winevulkan/vulkan_private.h | 31 ++++++ > 3 files changed, 221 insertions(+), 1 deletion(-) > > diff --git a/dlls/winevulkan/make_vulkan b/dlls/winevulkan/make_vulkan > index afea087a92..623d1b9cde 100755 > --- a/dlls/winevulkan/make_vulkan > +++ b/dlls/winevulkan/make_vulkan > @@ -92,7 +92,6 @@ UNSUPPORTED_EXTENSIONS = [ > # plumbing down to the native layer, we will get each message twice as we > # use 2 loaders (win32+native), but we may get output from the driver. > # In any case callback conversion is required. > - "VK_EXT_debug_utils", > "VK_EXT_validation_features", > "VK_EXT_validation_flags", > "VK_KHR_display", # Needs WSI work. > @@ -225,6 +224,11 @@ FUNCTION_OVERRIDES = { > # VK_EXT_calibrated_timestamps > "vkGetPhysicalDeviceCalibrateableTimeDomainsEXT" : {"dispatch" : True, "driver" : False, "thunk" : False}, > "vkGetCalibratedTimestampsEXT" : {"dispatch" : True, "driver" : False, "thunk" : False}, > + > + # VK_EXT_debug_utils > + "vkCreateDebugUtilsMessengerEXT" : {"dispatch": True, "driver" : False, "thunk" : False}, > + "vkDestroyDebugUtilsMessengerEXT" : {"dispatch": True, "driver" : False, "thunk" : False}, > + "vkSubmitDebugUtilsMessageEXT" : {"dispatch": True, "driver" : False, "thunk" : True, "private_thunk" : True}, > } > > STRUCT_CHAIN_CONVERSIONS = [ > @@ -941,6 +945,8 @@ class VkHandle(object): > > if self.name <http://self.name> == "VkCommandPool": > return "wine_cmd_pool_from_handle({0})->command_pool".format(name) > + if self.name <http://self.name> == "VkDebugUtilsMessengerEXT": > + return "wine_debug_utils_messenger_from_handle({0})->debug_messenger".format(name) > > native_handle_name = None > > diff --git a/dlls/winevulkan/vulkan.c b/dlls/winevulkan/vulkan.c > index 54b910c5e5..f831d486d0 100644 > --- a/dlls/winevulkan/vulkan.c > +++ b/dlls/winevulkan/vulkan.c > @@ -57,6 +57,21 @@ static void *wine_vk_find_struct_(void *s, VkStructureType t) > return NULL; > } > > +#define wine_vk_count_struct(s, t) wine_vk_count_struct_((void *)s, VK_STRUCTURE_TYPE_##t) > +static uint32_t wine_vk_count_struct_(void *s, VkStructureType t) > +{ > + const VkBaseInStructure *header; > + uint32_t result = 0; > + > + for (header = s; header; header = header->pNext) > + { > + if (header->sType == t) > + result++; > + } > + > + return result; > +} > + > static void *wine_vk_get_global_proc_addr(const char *name); > > static HINSTANCE hinstance; > @@ -113,6 +128,68 @@ static uint64_t wine_vk_get_wrapper(struct VkInstance_T *instance, uint64_t nati > return result; > } > > +static VkBool32 debug_utils_callback_conversion(VkDebugUtilsMessageSeverityFlagBitsEXT severity, > + VkDebugUtilsMessageTypeFlagsEXT message_types, > +#if defined(USE_STRUCT_CONVERSION) > + const VkDebugUtilsMessengerCallbackDataEXT_host *callback_data, > +#else > + const VkDebugUtilsMessengerCallbackDataEXT *callback_data, > +#endif > + void *user_data) > +{ > + struct VkDebugUtilsMessengerCallbackDataEXT wine_callback_data; > + VkDebugUtilsObjectNameInfoEXT *object_name_infos; > + struct wine_debug_utils_messenger *object; > + VkBool32 result; > + int i; > + > + TRACE("%i, %u, %p, %p\n", severity, message_types, callback_data, user_data); > + > + object = user_data; > + > + if (!object->instance->instance) > + { > + /* instance wasn't yet created, this is a message from the native loader */ > + return VK_FALSE; > + } > + > + wine_callback_data = *((VkDebugUtilsMessengerCallbackDataEXT *) callback_data); > + > + object_name_infos = heap_calloc(wine_callback_data.objectCount, sizeof(*object_name_infos)); > + > + for (i = 0; i < wine_callback_data.objectCount; i++) > + { > + object_name_infos[i].sType = callback_data->pObjects[i].sType; > + object_name_infos[i].pNext = callback_data->pObjects[i].pNext; > + object_name_infos[i].objectType = callback_data->pObjects[i].objectType; > + object_name_infos[i].pObjectName = callback_data->pObjects[i].pObjectName; > + > + if (wine_vk_is_type_wrapped(callback_data->pObjects[i].objectType)) > + { > + object_name_infos[i].objectHandle = wine_vk_get_wrapper(object->instance, callback_data->pObjects[i].objectHandle); > + if (!object_name_infos[i].objectHandle) > + { > + WARN("handle conversion failed 0x%s\n", wine_dbgstr_longlong(callback_data->pObjects[i].objectHandle)); > + heap_free(object_name_infos); > + return VK_FALSE; > + } > + } > + else > + { > + object_name_infos[i].objectHandle = callback_data->pObjects[i].objectHandle; > + } > + } > + > + wine_callback_data.pObjects = object_name_infos; > + > + /* applications should always return VK_FALSE */ > + result = object->user_callback(severity, message_types, &wine_callback_data, object->user_data); > + > + heap_free(object_name_infos); > + > + return result; > +} > + > static void wine_vk_physical_device_free(struct VkPhysicalDevice_T *phys_dev) > { > if (!phys_dev) > @@ -393,6 +470,8 @@ static void wine_vk_init_once(void) > static VkResult wine_vk_instance_convert_create_info(const VkInstanceCreateInfo *src, > VkInstanceCreateInfo *dst, struct VkInstance_T *object) > { > + VkDebugUtilsMessengerCreateInfoEXT *debug_utils_messenger; > + VkBaseInStructure *header; > unsigned int i; > VkResult res; > > @@ -404,6 +483,26 @@ static VkResult wine_vk_instance_convert_create_info(const VkInstanceCreateInfo > return res; > } > > + object->utils_messenger_count = wine_vk_count_struct(dst, DEBUG_UTILS_MESSENGER_CREATE_INFO_EXT); > + object->utils_messengers = heap_calloc(object->utils_messenger_count, sizeof(*object->utils_messengers)); > + header = (VkBaseInStructure *) dst; > + for (i = 0; i < object->utils_messenger_count; i++) > + { > + header = wine_vk_find_struct(header->pNext, DEBUG_UTILS_MESSENGER_CREATE_INFO_EXT); > + debug_utils_messenger = (VkDebugUtilsMessengerCreateInfoEXT *) header; > + > + object->utils_messengers[i].instance = object; > + object->utils_messengers[i].debug_messenger = VK_NULL_HANDLE; > + object->utils_messengers[i].user_callback = debug_utils_messenger->pfnUserCallback; > + object->utils_messengers[i].user_data = debug_utils_messenger->pUserData; > + > + /* convert_VkInstanceCreateInfo_struct_chain already copied the chain, > + * so we can modify it in-place. > + */ > + debug_utils_messenger->pfnUserCallback = (void *) &debug_utils_callback_conversion; > + debug_utils_messenger->pUserData = &object->utils_messengers[i]; > + } > + > /* ICDs don't support any layers, so nothing to copy. Modern versions of the loader > * filter this data out as well. > */ > @@ -427,6 +526,10 @@ static VkResult wine_vk_instance_convert_create_info(const VkInstanceCreateInfo > free_VkInstanceCreateInfo_struct_chain(dst); > return VK_ERROR_EXTENSION_NOT_PRESENT; > } > + if (!strcmp(extension_name, "VK_EXT_debug_utils")) > + { > + object->enable_wrapper_list = VK_TRUE; > + } > } > > return VK_SUCCESS; > @@ -527,6 +630,8 @@ static void wine_vk_instance_free(struct VkInstance_T *instance) > WINE_VK_REMOVE_HANDLE_MAPPING(instance, instance); > } > > + heap_free(instance->utils_messengers); > + > heap_free(instance); > } > > @@ -1673,6 +1778,84 @@ VkResult WINAPI wine_vkGetPhysicalDeviceSurfaceCapabilities2KHR(VkPhysicalDevice > return res; > } > > +VkResult WINAPI wine_vkCreateDebugUtilsMessengerEXT(VkInstance instance, const VkDebugUtilsMessengerCreateInfoEXT *create_info, > + const VkAllocationCallbacks *allocator, VkDebugUtilsMessengerEXT *messenger) > +{ > + VkDebugUtilsMessengerCreateInfoEXT wine_create_info; > + struct wine_debug_utils_messenger *object; > + VkResult res; > + > + TRACE("%p, %p, %p, %p\n", instance, create_info, allocator, messenger); > + > + if (allocator) > + FIXME("Support for allocation callbacks not implemented yet\n"); > + > + if (!(object = heap_alloc_zero(sizeof(*object)))) > + return VK_ERROR_OUT_OF_HOST_MEMORY; > + > + object->instance = instance; > + object->user_callback = create_info->pfnUserCallback; > + object->user_data = create_info->pUserData; > + > + wine_create_info = *create_info; > + > + wine_create_info.pfnUserCallback = (void *) &debug_utils_callback_conversion; > + wine_create_info.pUserData = object; > + > + res = instance->funcs.p_vkCreateDebugUtilsMessengerEXT(instance->instance, &wine_create_info, NULL, &object->debug_messenger); > + > + if (res != VK_SUCCESS) > + { > + heap_free(object); > + return res; > + } > + > + WINE_VK_ADD_NON_DISPATCHABLE_MAPPING(instance, object, object->debug_messenger); > + *messenger = wine_debug_utils_messenger_to_handle(object); > + > + return VK_SUCCESS; > +} > + > +void WINAPI wine_vkDestroyDebugUtilsMessengerEXT( > + VkInstance instance, VkDebugUtilsMessengerEXT messenger, const VkAllocationCallbacks *allocator) > +{ > + struct wine_debug_utils_messenger *object; > + > + TRACE("%p, 0x%s, %p\n", instance, wine_dbgstr_longlong(messenger), allocator); > + > + object = wine_debug_utils_messenger_from_handle(messenger); > + > + instance->funcs.p_vkDestroyDebugUtilsMessengerEXT(instance->instance, object->debug_messenger, NULL); > + WINE_VK_REMOVE_HANDLE_MAPPING(instance, object); > + > + heap_free(object); > +} > + > +void WINAPI wine_vkSubmitDebugUtilsMessageEXT(VkInstance instance, VkDebugUtilsMessageSeverityFlagBitsEXT severity, > + VkDebugUtilsMessageTypeFlagsEXT types, const VkDebugUtilsMessengerCallbackDataEXT *callback_data) > +{ > + VkDebugUtilsMessengerCallbackDataEXT native_callback_data; > + VkDebugUtilsObjectNameInfoEXT *object_names; > + int i; > + > + TRACE("%p, %#x, %#x, %p\n", instance, severity, types, callback_data); > + > + native_callback_data = *callback_data; > + object_names = heap_calloc(callback_data->objectCount, sizeof(*object_names)); > + memcpy(object_names, callback_data->pObjects, callback_data->objectCount * sizeof(*object_names)); > + native_callback_data.pObjects = object_names; > + > + for (i = 0; i < callback_data->objectCount; i++) > + { > + object_names[i].objectHandle = > + wine_vk_unwrap_handle(callback_data->pObjects[i].objectType, callback_data->pObjects[i].objectHandle); > + } > + > + thunk_vkSubmitDebugUtilsMessageEXT(instance->instance, severity, types, &native_callback_data); > + > + heap_free(object_names); > +} > + > BOOL WINAPI DllMain(HINSTANCE hinst, DWORD reason, void *reserved) > { > TRACE("%p, %u, %p\n", hinst, reason, reserved); > diff --git a/dlls/winevulkan/vulkan_private.h b/dlls/winevulkan/vulkan_private.h > index 7000da7133..8379fafecf 100644 > --- a/dlls/winevulkan/vulkan_private.h > +++ b/dlls/winevulkan/vulkan_private.h > @@ -95,6 +95,8 @@ struct VkDevice_T > struct wine_vk_mapping mapping; > }; > > +struct wine_debug_utils_messenger; > + > struct VkInstance_T > { > struct wine_vk_base base; > @@ -111,6 +113,9 @@ struct VkInstance_T > struct list wrappers; > SRWLOCK wrapper_lock; > > + struct wine_debug_utils_messenger *utils_messengers; > + uint32_t utils_messenger_count; > + > unsigned int quirks; > > struct wine_vk_mapping mapping; > @@ -158,6 +163,32 @@ static inline VkCommandPool wine_cmd_pool_to_handle(struct wine_cmd_pool *cmd_po > return (VkCommandPool)(uintptr_t)cmd_pool; > } > > +struct wine_debug_utils_messenger > +{ > + struct VkInstance_T *instance; /* parent */ > + VkDebugUtilsMessengerEXT debug_messenger; /* native messenger */ > + > + /* application callback + data */ > + PFN_vkDebugUtilsMessengerCallbackEXT user_callback; > + void *user_data; > + > + struct wine_vk_mapping mapping; > +}; > + > +static inline struct wine_debug_utils_messenger *wine_debug_utils_messenger_from_handle( > + VkDebugUtilsMessengerEXT handle) > +{ > + return (struct wine_debug_utils_messenger *)(uintptr_t)handle; > +} > + > +static inline VkDebugUtilsMessengerEXT wine_debug_utils_messenger_to_handle( > + struct wine_debug_utils_messenger *debug_messenger) > +{ > + return (VkDebugUtilsMessengerEXT)(uintptr_t)debug_messenger; > +} > + > + > + > void *wine_vk_get_device_proc_addr(const char *name) DECLSPEC_HIDDEN; > void *wine_vk_get_instance_proc_addr(const char *name) DECLSPEC_HIDDEN; > >