commit 416d374522757fc042282b3ae61c53f5510421fc
author Jamie Madill <jmadill@chromium.org> Thu Feb 27 11:34:09 2020 -0500
committer Nicolas Capens <nicolascapens@google.com> Fri Feb 28 15:17:19 2020 +0000
tree 04caa26e5a3a28c5bc721dfccbd5ae83e7ddd599
parent 1b90087896c5174b6bdeb5c5d34d7005718f87e8

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/+/41728
Tested-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>

src/Vulkan/VkPipelineCache.cpp

diff --git a/src/Vulkan/VkPipelineCache.cpp b/src/Vulkan/VkPipelineCache.cpp
index b9fb0d3..f54ac90 100644
--- a/src/Vulkan/VkPipelineCache.cpp
+++ b/src/Vulkan/VkPipelineCache.cpp
@@ -62,34 +62,43 @@
 
 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 > 0)
-		{
-			int cmp = memcmp(info->pMapEntries, specializationInfo.info->pMapEntries, info->mapEntryCount * sizeof(VkSpecializationMapEntry));
-			if(cmp != 0)
-			{
-				return cmp < 0;
-			}
-		}
+	if(info->mapEntryCount != specializationInfo.info->mapEntryCount)
+	{
+		return info->mapEntryCount < specializationInfo.info->mapEntryCount;
+	}
 
-		if(info->dataSize > 0)
+	if(info->dataSize != specializationInfo.info->dataSize)
+	{
+		return info->dataSize < specializationInfo.info->dataSize;
+	}
+
+	if(info->mapEntryCount > 0)
+	{
+		int cmp = memcmp(info->pMapEntries, specializationInfo.info->pMapEntries, info->mapEntryCount * sizeof(VkSpecializationMapEntry));
+		if(cmp != 0)
 		{
-			int cmp = memcmp(info->pData, specializationInfo.info->pData, info->dataSize);
-			if(cmp != 0)
-			{
-				return cmp < 0;
-			}
+			return cmp < 0;
+		}
+	}
+
+	if(info->dataSize > 0)
+	{
+		int cmp = memcmp(info->pData, specializationInfo.info->pData, info->dataSize);
+		if(cmp != 0)
+		{
+			return cmp < 0;
 		}
 	}