KhronosGroup / Vulkan-LoaderAndValidationLayers

**Deprecated repository** for Vulkan loader and validation layers
Apache License 2.0
414 stars 172 forks source link

Validation layers cause crash when using vulkan.hpp and the dynamic dispatcher #2562

Closed jherico closed 6 years ago

jherico commented 6 years ago

Hardware: nVidia 860m Driver: 378.92 Vulkan SDK: 1.1.70.1

I'm working on some code that uses push descriptors. As such I'm trying to use the vkGetPhysicalDeviceProperties2 function so that I can use populate a VkPhysicalDevicePushDescriptorPropertiesKHR structure. However, while the driver exposes vkGetPhysicalDeviceProperties2KHR, it does not expose vkGetPhysicalDeviceProperties2 (go figure).

I'm using the C++ interface from vulkan.hpp and the vk::DispatchLoaderDynamic to fetch the functions I need to call. I'm using the following code block after the dispatcher is initialized....

        // Get device push descriptor properties (to display them)
        if (dispatcher.vkGetPhysicalDeviceProperties2) {
            pushDescriptorProps = context.physicalDevice.getProperties2<vk::PhysicalDeviceProperties2, vk::PhysicalDevicePushDescriptorPropertiesKHR>(dispatcher).get<vk::PhysicalDevicePushDescriptorPropertiesKHR>();
        } else if (dispatcher.vkGetPhysicalDeviceProperties2KHR) {
            pushDescriptorProps = context.physicalDevice.getProperties2KHR<vk::PhysicalDeviceProperties2KHR, vk::PhysicalDevicePushDescriptorPropertiesKHR>(dispatcher).get<vk::PhysicalDevicePushDescriptorPropertiesKHR>();
        } else {
            throw std::runtime_error("Could not get a valid function pointer for vkGetPhysicalDeviceProperties2KHR");
        }

If I have validation layers turned off, then this code works fine... the first if fails, and so it makes the second call using the KHR version of the function. However, if I have validation layers turned on, the dispatcher.vkGetPhysicalDeviceProperties2 value is not null, and the code crashes because the validation layers eventually try to forward the call to a null pointer.

How can I both enable validation and avoid the crash in this situation? The only path I can currently see would be to create an instance and device without validation enabled, query for the functions, store that information, destroy the instance and device, then create a new instance and device with validation turned on and run my code. That's a crazy amount of work to test whether an underlying function actually really exists or not when validation is enabled.

mark-lunarg commented 6 years ago

@jherico, I think you're in a murky area of the loader-layer interface here, but there is some weirdness with your particular circumstances that @lenny-lunarg is looking into.

lenny-lunarg commented 6 years ago

The obvious problem here is that using a null check to determine if you can call into vkGetPhysicalDeviceProperties2 is completely invalid. The Vulkan spec states

Physical-device-level functionality or behavior added by a new core version of the API must not be used unless it is supported by the physical device as determined by vkGetPhysicalDeviceProperties.

So you have to use vkGetPhysicalDeviceProperties to check the device's API version to see if using vkGetPhysicalDeviceProperties2 is legal.

The reason you can't use a null check for this function is that the function cannot be retrieved from vkGetDeviceProcAddr (because it is most useful before a device is created), but whether or not calling it is legal depends on what Vulkan version the device supports. So the loader has to return a valid pointer from vkGetInstanceProcAddr if it's possible that this instance could support a 1.1 device. And since the loader is capable of picking up new devices at runtime, the loader must return a valid function pointer for any instance.

The real bug that I'm trying to find is why you don't get a valid pointer when validation layers are off. You should still get a valid pointer in that situation.

jherico commented 6 years ago

Actually I was just thinking about this myself and came to a similar conclusion: using a null check to determine if functionality was available was an invalid way of testing for a feature, as opposed to enumerating available extensions, or as you say, checking the device properties.

I've modified my code to do the check thusly:

if (context.deviceProperties.apiVersion >= VK_MAKE_VERSION(1, 1, 0)) {
    pushDescriptorProps = context.physicalDevice.getProperties2<vk::PhysicalDeviceProperties2, vk::PhysicalDevicePushDescriptorPropertiesKHR>(dispatcher).get<vk::PhysicalDevicePushDescriptorPropertiesKHR>();
} else {
    pushDescriptorProps = context.physicalDevice.getProperties2KHR<vk::PhysicalDeviceProperties2KHR, vk::PhysicalDevicePushDescriptorPropertiesKHR>(dispatcher).get<vk::PhysicalDevicePushDescriptorPropertiesKHR>();
}

... where context.deviceProperties is populated using the non-extension method. The third condition shouldn't be required because my instance and device initializing code will throw an exception during setup if the required extension(s) are missing.

The real bug that I'm trying to find is why you don't get a valid pointer when validation layers are off. You should still get a valid pointer in that situation.

The driver is reporting API version 1.0.37, so not surprising to me that the 1.1 core version of the function isn't there. Sorry, I reported the Vulkan SDK version in the original report, but not the version I was getting back from the driver.