Fix edge cases of various enumeration functions
- It is acceptable to pass a larger array to these functions than you
actually need. The functions are specified to overwrite the `count`
parameter with the number of elements actually written. Previously we
would assert in debug, or in release we would leave the input count
unmodified, which would lead an app which uses this pattern to
consider uninitialized junk to be valid elements.
- It is acceptable to provide a pointer to a result buffer *and* a
count of zero. This should return VK_INCOMPLETE if there are any
elements. We mishandled this in physical device and physical device
group queries.
Bug: b/117974925
Test: dEQP-VK.api.info.*
Change-Id: I2764831726bb4911ba4cab847fa4b404817508c5
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/32749
Reviewed-by: Nicolas Capens <nicolascapens@google.com>
Reviewed-by: Alexis Hétu <sugoi@google.com>
Kokoro-Presubmit: kokoro <noreply+kokoro@google.com>
Tested-by: Chris Forbes <chrisforbes@google.com>
diff --git a/src/Vulkan/VkInstance.cpp b/src/Vulkan/VkInstance.cpp
index e4079b8..f93dc7f 100644
--- a/src/Vulkan/VkInstance.cpp
+++ b/src/Vulkan/VkInstance.cpp
@@ -19,8 +19,7 @@
{
Instance::Instance(const VkInstanceCreateInfo* pCreateInfo, void* mem, VkPhysicalDevice physicalDevice)
- : physicalDevice(physicalDevice),
- physicalDeviceCount(physicalDevice ? 1 : 0)
+ : physicalDevice(physicalDevice)
{
}
@@ -29,31 +28,45 @@
vk::destroy(physicalDevice, pAllocator);
}
-uint32_t Instance::getPhysicalDeviceCount() const
+VkResult Instance::getPhysicalDevices(uint32_t *pPhysicalDeviceCount, VkPhysicalDevice* pPhysicalDevices) const
{
- return physicalDeviceCount;
+ if (!pPhysicalDevices)
+ {
+ *pPhysicalDeviceCount = 1;
+ return VK_SUCCESS;
+ }
+
+ if (*pPhysicalDeviceCount < 1)
+ {
+ return VK_INCOMPLETE;
+ }
+
+ pPhysicalDevices[0] = physicalDevice;
+ *pPhysicalDeviceCount = 1;
+
+ return VK_SUCCESS;
}
-void Instance::getPhysicalDevices(uint32_t pPhysicalDeviceCount, VkPhysicalDevice* pPhysicalDevices) const
+VkResult Instance::getPhysicalDeviceGroups(uint32_t *pPhysicalDeviceGroupCount,
+ VkPhysicalDeviceGroupProperties* pPhysicalDeviceGroupProperties) const
{
- ASSERT(pPhysicalDeviceCount == 1);
+ if (!pPhysicalDeviceGroupProperties)
+ {
+ *pPhysicalDeviceGroupCount = 1;
+ return VK_SUCCESS;
+ }
- *pPhysicalDevices = physicalDevice;
-}
+ if (*pPhysicalDeviceGroupCount < 1)
+ {
+ return VK_INCOMPLETE;
+ }
-uint32_t Instance::getPhysicalDeviceGroupCount() const
-{
- return physicalDeviceGroupCount;
-}
+ pPhysicalDeviceGroupProperties[0].physicalDeviceCount = 1;
+ pPhysicalDeviceGroupProperties[0].physicalDevices[0] = physicalDevice;
+ pPhysicalDeviceGroupProperties[0].subsetAllocation = VK_FALSE;
+ *pPhysicalDeviceGroupCount = 1;
-void Instance::getPhysicalDeviceGroups(uint32_t pPhysicalDeviceGroupCount,
- VkPhysicalDeviceGroupProperties* pPhysicalDeviceGroupProperties) const
-{
- ASSERT(pPhysicalDeviceGroupCount == 1);
-
- pPhysicalDeviceGroupProperties->physicalDeviceCount = physicalDeviceCount;
- pPhysicalDeviceGroupProperties->physicalDevices[0] = physicalDevice;
- pPhysicalDeviceGroupProperties->subsetAllocation = VK_FALSE;
+ return VK_SUCCESS;
}
} // namespace vk
diff --git a/src/Vulkan/VkInstance.hpp b/src/Vulkan/VkInstance.hpp
index f63dc3b..10eac6d 100644
--- a/src/Vulkan/VkInstance.hpp
+++ b/src/Vulkan/VkInstance.hpp
@@ -30,16 +30,12 @@
static size_t ComputeRequiredAllocationSize(const VkInstanceCreateInfo*) { return 0; }
- uint32_t getPhysicalDeviceCount() const;
- void getPhysicalDevices(uint32_t pPhysicalDeviceCount, VkPhysicalDevice* pPhysicalDevices) const;
- uint32_t getPhysicalDeviceGroupCount() const;
- void getPhysicalDeviceGroups(uint32_t pPhysicalDeviceGroupCount,
+ VkResult getPhysicalDevices(uint32_t *pPhysicalDeviceCount, VkPhysicalDevice* pPhysicalDevices) const;
+ VkResult getPhysicalDeviceGroups(uint32_t *pPhysicalDeviceGroupCount,
VkPhysicalDeviceGroupProperties* pPhysicalDeviceGroupProperties) const;
private:
VkPhysicalDevice physicalDevice = VK_NULL_HANDLE;
- const uint32_t physicalDeviceCount = 0;
- const uint32_t physicalDeviceGroupCount = 1;
};
using DispatchableInstance = DispatchableObject<Instance, VkInstance>;
diff --git a/src/Vulkan/libVulkan.cpp b/src/Vulkan/libVulkan.cpp
index 3a68c40..bc39d38 100644
--- a/src/Vulkan/libVulkan.cpp
+++ b/src/Vulkan/libVulkan.cpp
@@ -200,16 +200,7 @@
TRACE("(VkInstance instance = %p, uint32_t* pPhysicalDeviceCount = %p, VkPhysicalDevice* pPhysicalDevices = %p)",
instance, pPhysicalDeviceCount, pPhysicalDevices);
- if(!pPhysicalDevices)
- {
- *pPhysicalDeviceCount = vk::Cast(instance)->getPhysicalDeviceCount();
- }
- else
- {
- vk::Cast(instance)->getPhysicalDevices(*pPhysicalDeviceCount, pPhysicalDevices);
- }
-
- return VK_SUCCESS;
+ return vk::Cast(instance)->getPhysicalDevices(pPhysicalDeviceCount, pPhysicalDevices);
}
VKAPI_ATTR void VKAPI_CALL vkGetPhysicalDeviceFeatures(VkPhysicalDevice physicalDevice, VkPhysicalDeviceFeatures* pFeatures)
@@ -527,12 +518,14 @@
return VK_SUCCESS;
}
- for(uint32_t i = 0; i < std::min(*pPropertyCount, extensionPropertiesCount); i++)
+ auto toCopy = std::min(*pPropertyCount, extensionPropertiesCount);
+ for(uint32_t i = 0; i < toCopy; i++)
{
pProperties[i] = instanceExtensionProperties[i];
}
- return (*pPropertyCount < extensionPropertiesCount) ? VK_INCOMPLETE : VK_SUCCESS;
+ *pPropertyCount = toCopy;
+ return (toCopy < extensionPropertiesCount) ? VK_INCOMPLETE : VK_SUCCESS;
}
VKAPI_ATTR VkResult VKAPI_CALL vkEnumerateDeviceExtensionProperties(VkPhysicalDevice physicalDevice, const char* pLayerName, uint32_t* pPropertyCount, VkExtensionProperties* pProperties)
@@ -547,12 +540,14 @@
return VK_SUCCESS;
}
- for(uint32_t i = 0; i < std::min(*pPropertyCount, extensionPropertiesCount); i++)
+ auto toCopy = std::min(*pPropertyCount, extensionPropertiesCount);
+ for(uint32_t i = 0; i < toCopy; i++)
{
pProperties[i] = deviceExtensionProperties[i];
}
- return (*pPropertyCount < extensionPropertiesCount) ? VK_INCOMPLETE : VK_SUCCESS;
+ *pPropertyCount = toCopy;
+ return (toCopy < extensionPropertiesCount) ? VK_INCOMPLETE : VK_SUCCESS;
}
VKAPI_ATTR VkResult VKAPI_CALL vkEnumerateInstanceLayerProperties(uint32_t* pPropertyCount, VkLayerProperties* pProperties)
@@ -2082,16 +2077,7 @@
TRACE("VkInstance instance = %p, uint32_t* pPhysicalDeviceGroupCount = %p, VkPhysicalDeviceGroupProperties* pPhysicalDeviceGroupProperties = %p",
instance, pPhysicalDeviceGroupCount, pPhysicalDeviceGroupProperties);
- if(!pPhysicalDeviceGroupProperties)
- {
- *pPhysicalDeviceGroupCount = vk::Cast(instance)->getPhysicalDeviceGroupCount();
- }
- else
- {
- vk::Cast(instance)->getPhysicalDeviceGroups(*pPhysicalDeviceGroupCount, pPhysicalDeviceGroupProperties);
- }
-
- return VK_SUCCESS;
+ return vk::Cast(instance)->getPhysicalDeviceGroups(pPhysicalDeviceGroupCount, pPhysicalDeviceGroupProperties);
}
VKAPI_ATTR void VKAPI_CALL vkGetImageMemoryRequirements2(VkDevice device, const VkImageMemoryRequirementsInfo2* pInfo, VkMemoryRequirements2* pMemoryRequirements)
