PipelineCache: Handle nullptr mismatch of specialization info.

It's valid in Vulkan for a shader to have specialization constants and the pipeline to be lacking specialization constant data. This could lead to a key match between the two even though there should be a different shader payload. From the spec: > It is legal for a SPIR-V module with specializations to be > compiled into a pipeline where no specialization info was provided. > SPIR-V specialization constants contain default values such that > if a specialization is not provided. Fix this by adding in the required checks. Should not affect perf. Bug: b/150449637 Bug: angleproject:4426 Change-Id: I5f384420bdc1f03a36852db0b20393ae56452b62 Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/41728Tested-by: Nicolas Capens <nicolascapens@google.com> Kokoro-Presubmit: Jamie Madill <jmadill@chromium.org> Kokoro-Presubmit: kokoro <noreply+kokoro@google.com> Reviewed-by: Nicolas Capens <nicolascapens@google.com> Reviewed-by: Alexis Hétu <sugoi@google.com>

PipelineCache: Handle nullptr mismatch of specialization info.
416d3745 · Jamie Madill · Nicolas Capens · 1b900878 · 416d3745
Commit 416d3745 authored Feb 27, 2020 by Jamie Madill Committed by Nicolas Capens Feb 28, 2020
Hide whitespace changes
Inline Side-by-side

Showing with 30 additions and 21 deletions

VkPipelineCache.cpp src/Vulkan/VkPipelineCache.cpp +30 -21

No files found.
--- a/src/Vulkan/VkPipelineCache.cpp
+++ b/src/Vulkan/VkPipelineCache.cpp
@@ -62,34 +62,43 @@ void PipelineCache::SpirvShaderKey::SpecializationInfo::Deleter::operator()(VkSp

 bool PipelineCache::SpirvShaderKey::SpecializationInfo::operator<(const SpecializationInfo &specializationInfo) const
 {
-	if(info && specializationInfo.info)
+	// Check that either both or neither keys have specialization info.
+	if((info.get() == nullptr) != (specializationInfo.info.get() == nullptr))
 	{
-		if(info->mapEntryCount != specializationInfo.info->mapEntryCount)
-		{
-			return info->mapEntryCount < specializationInfo.info->mapEntryCount;
-		}
+		return info.get() == nullptr;
+	}

-		if(info->dataSize != specializationInfo.info->dataSize)
-		{
-			return info->dataSize < specializationInfo.info->dataSize;
-		}
+	if(!info)
+	{
+		ASSERT(!specializationInfo.info);
+		return false;
+	}
+
+	if(info->mapEntryCount != specializationInfo.info->mapEntryCount)
+	{
+		return info->mapEntryCount < specializationInfo.info->mapEntryCount;
+	}
+
+	if(info->dataSize != specializationInfo.info->dataSize)
+	{
+		return info->dataSize < specializationInfo.info->dataSize;
+	}

-		if(info->mapEntryCount > 0)
+	if(info->mapEntryCount > 0)
+	{
+		int cmp = memcmp(info->pMapEntries, specializationInfo.info->pMapEntries, info->mapEntryCount * sizeof(VkSpecializationMapEntry));
+		if(cmp != 0)
 		{
-			int cmp = memcmp(info->pMapEntries, specializationInfo.info->pMapEntries, info->mapEntryCount * sizeof(VkSpecializationMapEntry));
-			if(cmp != 0)
-			{
-				return cmp < 0;
-			}
+			return cmp < 0;
 		}
+	}

-		if(info->dataSize > 0)
+	if(info->dataSize > 0)
+	{
+		int cmp = memcmp(info->pData, specializationInfo.info->pData, info->dataSize);
+		if(cmp != 0)
 		{
-			int cmp = memcmp(info->pData, specializationInfo.info->pData, info->dataSize);
-			if(cmp != 0)
-			{
-				return cmp < 0;
-			}
+			return cmp < 0;
 		}
 	}