On Fri, Mar 2, 2018 at 3:57 AM, Józef Kucia joseph.kucia@gmail.com wrote:
On Fri, Mar 2, 2018 at 7:13 AM, Roderick Colenbrander thunderbird2k@gmail.com wrote:
Signed-off-by: Roderick Colenbrander thunderbird2k@gmail.com
dlls/winevulkan/make_vulkan | 1 + dlls/winevulkan/vulkan.c | 140 +++++++++++++++++++++++++++++++++++++++ dlls/winevulkan/vulkan_private.h | 14 ++++ dlls/winevulkan/vulkan_thunks.c | 6 -- dlls/winevulkan/vulkan_thunks.h | 1 + 5 files changed, 156 insertions(+), 6 deletions(-)
diff --git a/dlls/winevulkan/make_vulkan b/dlls/winevulkan/make_vulkan index 5657b70c4c..3b045db8a9 100755 --- a/dlls/winevulkan/make_vulkan +++ b/dlls/winevulkan/make_vulkan @@ -90,6 +90,7 @@ FUNCTION_OVERRIDES = {
# Instance functions "vkDestroyInstance" : {"dispatch" : True, "driver" : True, "thunk" : False },
- "vkEnumeratePhysicalDevices" : {"dispatch" : True, "driver" : False, "thunk" : False},
}
diff --git a/dlls/winevulkan/vulkan.c b/dlls/winevulkan/vulkan.c index 0f48d971e2..234be589ae 100644 --- a/dlls/winevulkan/vulkan.c +++ b/dlls/winevulkan/vulkan.c @@ -58,6 +58,85 @@ static BOOL wine_vk_init(void) return TRUE; }
+/* Helper function which stores wrapped physical devices in the instance object. */ +static VkResult wine_vk_instance_load_physical_devices(struct VkInstance_T *instance) +{
- VkResult res;
- struct VkPhysicalDevice_T **tmp_phys_devs = NULL;
- uint32_t num_phys_devs = 0;
- unsigned int i;
- res = instance->funcs.p_vkEnumeratePhysicalDevices(instance->instance, &num_phys_devs, NULL);
- if (res != VK_SUCCESS)
- {
ERR("Failed to enumerate physical devices, res=%d\n", res);return res;- }
- /* Don't bother with any of the rest if the system just lacks devices. */
- if (num_phys_devs == 0)
- {
instance->num_phys_devs = 0;instance->phys_devs isn't initialized.
return VK_SUCCESS;- }
- tmp_phys_devs = heap_alloc(num_phys_devs * sizeof(*tmp_phys_devs));
- if (!tmp_phys_devs)
return VK_ERROR_OUT_OF_HOST_MEMORY;This leaves a lot of fields uninitialized in "instance". We don't want to crash in wine_vk_instance_free().
- res = instance->funcs.p_vkEnumeratePhysicalDevices(instance->instance, &num_phys_devs, tmp_phys_devs);
- if (res != VK_SUCCESS)
goto err;- instance->phys_devs = heap_alloc(num_phys_devs * sizeof(*instance->phys_devs));
heap_calloc() may be more suitable.
- if (!instance->phys_devs)
- {
res = VK_ERROR_OUT_OF_HOST_MEMORY;goto err;- }
- /* Wrap each native physical device handle into a dispatchable object for the ICD loader. */
- for (i = 0; i < num_phys_devs; i++)
- {
VkPhysicalDevice phys_dev = heap_alloc(sizeof(*phys_dev));struct VkPhysicalDevice_T *
if (!phys_dev){ERR("Unable to allocate memory for physical device!\n");res = VK_ERROR_OUT_OF_HOST_MEMORY;goto err;}phys_dev->base.loader_magic = VULKAN_ICD_MAGIC_VALUE;phys_dev->instance = instance;phys_dev->phys_dev = tmp_phys_devs[i];instance->phys_devs[i] = phys_dev;instance->num_phys_devs = i;- }
- instance->num_phys_devs = num_phys_devs;
- heap_free(tmp_phys_devs);
- return VK_SUCCESS;
+err:
- if (tmp_phys_devs)
heap_free(tmp_phys_devs);It's fine to call heap_free() with NULL.
I believe it results in undefined behaviour, that's what MSDN told me and our heap_alloc doesn't have guards for NULL.
From MSDN:
"A pointer to the memory block to be freed. This pointer is returned by the HeapAlloc or HeapReAlloc function. If this pointer is NULL, the behavior is undefined."
- if (instance->phys_devs)
- {
for (i = 0; i < instance->num_phys_devs; i++){heap_free(instance->phys_devs[i]);instance->phys_devs[i] = NULL;}heap_free(instance->phys_devs);instance->num_phys_devs = 0;instance->phys_devs = NULL;- }
- return res;
+}
/* Helper function used for freeing an instance structure. This function supports full
- and partial object cleanups and can thus be used for vkCreateInstance failures.
*/ @@ -66,6 +145,17 @@ static void wine_vk_instance_free(struct VkInstance_T *instance) if (!instance) return;
- if (instance->phys_devs)
- {
unsigned int i;for (i = 0; i < instance->num_phys_devs; i++){heap_free(&instance->phys_devs[i]);}heap_free(instance->phys_devs);- }
It might crash because "phys_devs" is uninitialized. We probably want to use heap_alloc_zero() to allocate "instance".
You are right. I used to have a wine_vk_alloc or some call before heap_alloc got introduced, which did the zero'ing. Forgot to change this spot to heap_alloc_zero.
- if (instance->instance) vk_funcs->p_vkDestroyInstance(instance->instance, NULL /* allocator */);
@@ -108,6 +198,18 @@ static VkResult WINAPI wine_vkCreateInstance(const VkInstanceCreateInfo *create_ ALL_VK_INSTANCE_FUNCS() #undef USE_VK_FUNC
- /* Cache physical devices for vkEnumeratePhysicalDevices within the instance as
* each vkPhysicalDevice is a dispatchable object, which means we need to wrap* the native physical device and present those the application.* Cleanup happens as part of wine_vkDestroyInstance.*/- res = wine_vk_instance_load_physical_devices(object);
- if (res != VK_SUCCESS)
- {
ERR("Failed to cache physical devices, res=%d\n", res);goto err;- }
- *instance = object; TRACE("Done, instance=%p native_instance=%p\n", object, object->instance); return VK_SUCCESS;
@@ -134,6 +236,44 @@ static VkResult WINAPI wine_vkEnumerateInstanceExtensionProperties(const char *l return vk_funcs->p_vkEnumerateInstanceExtensionProperties(layer_name, count, properties); }
+VkResult WINAPI wine_vkEnumeratePhysicalDevices(VkInstance instance, uint32_t *device_count,
VkPhysicalDevice *devices)+{
- VkResult res;
- unsigned int i, num_copies;
- TRACE("%p %p %p\n", instance, device_count, devices);
- if (!devices)
- {
*device_count = instance->num_phys_devs;return VK_SUCCESS;- }
- if (*device_count < instance->num_phys_devs)
- {
/* Incomplete is a type of success used to signal the application* that not all devices got copied.*/num_copies = *device_count;res = VK_INCOMPLETE;- }
- else
- {
num_copies = instance->num_phys_devs;res = VK_SUCCESS;- }
- for (i = 0; i < num_copies; i++)
- {
devices[i] = instance->phys_devs[i];- }
- *device_count = num_copies;
- TRACE("Returning %d devices\n", *device_count);
- return res;
+}
static PFN_vkVoidFunction WINAPI wine_vkGetInstanceProcAddr(VkInstance instance, const char *name) { void *func; diff --git a/dlls/winevulkan/vulkan_private.h b/dlls/winevulkan/vulkan_private.h index 16aa0ce55c..8ef4b924a2 100644 --- a/dlls/winevulkan/vulkan_private.h +++ b/dlls/winevulkan/vulkan_private.h @@ -52,7 +52,21 @@ struct VkInstance_T { struct wine_vk_base base; struct vulkan_instance_funcs funcs;
- /* We cache devices as we need to wrap them as they are
* dispatchable objects.*/- uint32_t num_phys_devs;
- struct VkPhysicalDevice_T **phys_devs;
- VkInstance instance; /* native instance */
};
+struct VkPhysicalDevice_T +{
- struct wine_vk_base base;
- struct VkInstance_T *instance; /* parent */
- VkPhysicalDevice phys_dev; /* native physical device */
+};
#endif /* __WINE_VULKAN_PRIVATE_H */ diff --git a/dlls/winevulkan/vulkan_thunks.c b/dlls/winevulkan/vulkan_thunks.c index c1c0b079a5..ae4f773ef0 100644 --- a/dlls/winevulkan/vulkan_thunks.c +++ b/dlls/winevulkan/vulkan_thunks.c @@ -28,12 +28,6 @@ static VkResult WINAPI wine_vkEnumerateDeviceLayerProperties(VkPhysicalDevice ph return VK_ERROR_OUT_OF_HOST_MEMORY; }
-static VkResult WINAPI wine_vkEnumeratePhysicalDevices(VkInstance instance, uint32_t *pPhysicalDeviceCount, VkPhysicalDevice *pPhysicalDevices) -{
- FIXME("stub: %p, %p, %p\n", instance, pPhysicalDeviceCount, pPhysicalDevices);
- return VK_ERROR_OUT_OF_HOST_MEMORY;
-}
static void WINAPI wine_vkGetPhysicalDeviceFeatures(VkPhysicalDevice physicalDevice, VkPhysicalDeviceFeatures *pFeatures) { FIXME("stub: %p, %p\n", physicalDevice, pFeatures); diff --git a/dlls/winevulkan/vulkan_thunks.h b/dlls/winevulkan/vulkan_thunks.h index 087fa17d0e..26e114ab74 100644 --- a/dlls/winevulkan/vulkan_thunks.h +++ b/dlls/winevulkan/vulkan_thunks.h @@ -8,6 +8,7 @@ void *wine_vk_get_instance_proc_addr(const char *name) DECLSPEC_HIDDEN;
/* Functions for which we have custom implementations outside of the thunks. */ void WINAPI wine_vkDestroyInstance(VkInstance instance, const VkAllocationCallbacks *pAllocator) DECLSPEC_HIDDEN; +VkResult WINAPI wine_vkEnumeratePhysicalDevices(VkInstance instance, uint32_t *pPhysicalDeviceCount, VkPhysicalDevice *pPhysicalDevices) DECLSPEC_HIDDEN;
/* For use by vkInstance and children */ struct vulkan_instance_funcs -- 2.14.3