Skip to content

S
swiftshader
Commit df84b946 by Chris Forbes
Options

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/+/32749Reviewed-by: 's avatarNicolas Capens <nicolascapens@google.com> Reviewed-by: 's avatarAlexis Hétu <sugoi@google.com> Kokoro-Presubmit: kokoro <noreply+kokoro@google.com> Tested-by: 's avatarChris Forbes <chrisforbes@google.com>
parent df81c830
Showing with 44 additions and 49 deletions
src/Vulkan/VkInstance.cpp
...@@ -19,8 +19,7 @@ namespace vk ...@@ -19,8 +19,7 @@ namespace vk
{ {
Instance::Instance(const VkInstanceCreateInfo* pCreateInfo, void* mem, VkPhysicalDevice physicalDevice) Instance::Instance(const VkInstanceCreateInfo* pCreateInfo, void* mem, VkPhysicalDevice physicalDevice)
: physicalDevice(physicalDevice), : physicalDevice(physicalDevice)
physicalDeviceCount(physicalDevice ? 1 : 0)
{ {
} }
...@@ -29,31 +28,45 @@ void Instance::destroy(const VkAllocationCallbacks* pAllocator) ...@@ -29,31 +28,45 @@ void Instance::destroy(const VkAllocationCallbacks* pAllocator)
vk::destroy(physicalDevice, pAllocator); 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;
}
void Instance::getPhysicalDevices(uint32_t pPhysicalDeviceCount, VkPhysicalDevice* pPhysicalDevices) const if (*pPhysicalDeviceCount < 1)
{ {
ASSERT(pPhysicalDeviceCount == 1); return VK_INCOMPLETE;
}
*pPhysicalDevices = physicalDevice; pPhysicalDevices[0] = physicalDevice;
} *pPhysicalDeviceCount = 1;
uint32_t Instance::getPhysicalDeviceGroupCount() const return VK_SUCCESS;
{
return physicalDeviceGroupCount;
} }
void Instance::getPhysicalDeviceGroups(uint32_t pPhysicalDeviceGroupCount, VkResult Instance::getPhysicalDeviceGroups(uint32_t *pPhysicalDeviceGroupCount,
VkPhysicalDeviceGroupProperties* pPhysicalDeviceGroupProperties) const VkPhysicalDeviceGroupProperties* pPhysicalDeviceGroupProperties) const
{ {
ASSERT(pPhysicalDeviceGroupCount == 1); if (!pPhysicalDeviceGroupProperties)
{
*pPhysicalDeviceGroupCount = 1;
return VK_SUCCESS;
}
if (*pPhysicalDeviceGroupCount < 1)
{
return VK_INCOMPLETE;
}
pPhysicalDeviceGroupProperties[0].physicalDeviceCount = 1;
pPhysicalDeviceGroupProperties[0].physicalDevices[0] = physicalDevice;
pPhysicalDeviceGroupProperties[0].subsetAllocation = VK_FALSE;
*pPhysicalDeviceGroupCount = 1;
pPhysicalDeviceGroupProperties->physicalDeviceCount = physicalDeviceCount; return VK_SUCCESS;
pPhysicalDeviceGroupProperties->physicalDevices[0] = physicalDevice;
pPhysicalDeviceGroupProperties->subsetAllocation = VK_FALSE;
} }
} // namespace vk } // namespace vk
src/Vulkan/VkInstance.hpp
...@@ -30,16 +30,12 @@ public: ...@@ -30,16 +30,12 @@ public:
static size_t ComputeRequiredAllocationSize(const VkInstanceCreateInfo*) { return 0; } static size_t ComputeRequiredAllocationSize(const VkInstanceCreateInfo*) { return 0; }
uint32_t getPhysicalDeviceCount() const; VkResult getPhysicalDevices(uint32_t *pPhysicalDeviceCount, VkPhysicalDevice* pPhysicalDevices) const;
void getPhysicalDevices(uint32_t pPhysicalDeviceCount, VkPhysicalDevice* pPhysicalDevices) const; VkResult getPhysicalDeviceGroups(uint32_t *pPhysicalDeviceGroupCount,
uint32_t getPhysicalDeviceGroupCount() const;
void getPhysicalDeviceGroups(uint32_t pPhysicalDeviceGroupCount,
VkPhysicalDeviceGroupProperties* pPhysicalDeviceGroupProperties) const; VkPhysicalDeviceGroupProperties* pPhysicalDeviceGroupProperties) const;
private: private:
VkPhysicalDevice physicalDevice = VK_NULL_HANDLE; VkPhysicalDevice physicalDevice = VK_NULL_HANDLE;
const uint32_t physicalDeviceCount = 0;
const uint32_t physicalDeviceGroupCount = 1;
}; };
using DispatchableInstance = DispatchableObject<Instance, VkInstance>; using DispatchableInstance = DispatchableObject<Instance, VkInstance>;
......
src/Vulkan/libVulkan.cpp
...@@ -200,16 +200,7 @@ VKAPI_ATTR VkResult VKAPI_CALL vkEnumeratePhysicalDevices(VkInstance instance, u ...@@ -200,16 +200,7 @@ VKAPI_ATTR VkResult VKAPI_CALL vkEnumeratePhysicalDevices(VkInstance instance, u
TRACE("(VkInstance instance = %p, uint32_t* pPhysicalDeviceCount = %p, VkPhysicalDevice* pPhysicalDevices = %p)", TRACE("(VkInstance instance = %p, uint32_t* pPhysicalDeviceCount = %p, VkPhysicalDevice* pPhysicalDevices = %p)",
instance, pPhysicalDeviceCount, pPhysicalDevices); instance, pPhysicalDeviceCount, pPhysicalDevices);
if(!pPhysicalDevices) return vk::Cast(instance)->getPhysicalDevices(pPhysicalDeviceCount, pPhysicalDevices);
{
*pPhysicalDeviceCount = vk::Cast(instance)->getPhysicalDeviceCount();
}
else
{
vk::Cast(instance)->getPhysicalDevices(*pPhysicalDeviceCount, pPhysicalDevices);
}
return VK_SUCCESS;
} }
VKAPI_ATTR void VKAPI_CALL vkGetPhysicalDeviceFeatures(VkPhysicalDevice physicalDevice, VkPhysicalDeviceFeatures* pFeatures) VKAPI_ATTR void VKAPI_CALL vkGetPhysicalDeviceFeatures(VkPhysicalDevice physicalDevice, VkPhysicalDeviceFeatures* pFeatures)
...@@ -527,12 +518,14 @@ VKAPI_ATTR VkResult VKAPI_CALL vkEnumerateInstanceExtensionProperties(const char ...@@ -527,12 +518,14 @@ VKAPI_ATTR VkResult VKAPI_CALL vkEnumerateInstanceExtensionProperties(const char
return VK_SUCCESS; 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]; 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) VKAPI_ATTR VkResult VKAPI_CALL vkEnumerateDeviceExtensionProperties(VkPhysicalDevice physicalDevice, const char* pLayerName, uint32_t* pPropertyCount, VkExtensionProperties* pProperties)
...@@ -547,12 +540,14 @@ VKAPI_ATTR VkResult VKAPI_CALL vkEnumerateDeviceExtensionProperties(VkPhysicalDe ...@@ -547,12 +540,14 @@ VKAPI_ATTR VkResult VKAPI_CALL vkEnumerateDeviceExtensionProperties(VkPhysicalDe
return VK_SUCCESS; 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]; 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) VKAPI_ATTR VkResult VKAPI_CALL vkEnumerateInstanceLayerProperties(uint32_t* pPropertyCount, VkLayerProperties* pProperties)
...@@ -2082,16 +2077,7 @@ VKAPI_ATTR VkResult VKAPI_CALL vkEnumeratePhysicalDeviceGroups(VkInstance instan ...@@ -2082,16 +2077,7 @@ VKAPI_ATTR VkResult VKAPI_CALL vkEnumeratePhysicalDeviceGroups(VkInstance instan
TRACE("VkInstance instance = %p, uint32_t* pPhysicalDeviceGroupCount = %p, VkPhysicalDeviceGroupProperties* pPhysicalDeviceGroupProperties = %p", TRACE("VkInstance instance = %p, uint32_t* pPhysicalDeviceGroupCount = %p, VkPhysicalDeviceGroupProperties* pPhysicalDeviceGroupProperties = %p",
instance, pPhysicalDeviceGroupCount, pPhysicalDeviceGroupProperties); instance, pPhysicalDeviceGroupCount, pPhysicalDeviceGroupProperties);
if(!pPhysicalDeviceGroupProperties) return vk::Cast(instance)->getPhysicalDeviceGroups(pPhysicalDeviceGroupCount, pPhysicalDeviceGroupProperties);
{
*pPhysicalDeviceGroupCount = vk::Cast(instance)->getPhysicalDeviceGroupCount();
}
else
{
vk::Cast(instance)->getPhysicalDeviceGroups(*pPhysicalDeviceGroupCount, pPhysicalDeviceGroupProperties);
}
return VK_SUCCESS;
} }
VKAPI_ATTR void VKAPI_CALL vkGetImageMemoryRequirements2(VkDevice device, const VkImageMemoryRequirementsInfo2* pInfo, VkMemoryRequirements2* pMemoryRequirements) VKAPI_ATTR void VKAPI_CALL vkGetImageMemoryRequirements2(VkDevice device, const VkImageMemoryRequirementsInfo2* pInfo, VkMemoryRequirements2* pMemoryRequirements)
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment