Fix memory corruption SpirvShaderKey::SpecializationInfo
SpecializationInfo was default copy-constructed / assigned, and held a raw pointer to the copy of the VkSpecializationInfo. This struct was deleted in the destructor, trashing any other copies.
Fixes undefined behavior and crashes of a few Vulkan samples.
Bug b/123588002
Change-Id: I7adcec2d51bc357ef5bcee1ec6bdafe9ecd208a7
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/34454
Kokoro-Presubmit: kokoro <noreply+kokoro@google.com>
Reviewed-by: Alexis Hétu <sugoi@google.com>
Tested-by: Ben Clayton <bclayton@google.com>
diff --git a/src/Vulkan/VkPipelineCache.cpp b/src/Vulkan/VkPipelineCache.cpp
index 1013f2a..f9fe9cf 100644
--- a/src/Vulkan/VkPipelineCache.cpp
+++ b/src/Vulkan/VkPipelineCache.cpp
@@ -22,9 +22,11 @@
{
if(specializationInfo)
{
- info = reinterpret_cast<VkSpecializationInfo*>(
+ auto ptr = reinterpret_cast<VkSpecializationInfo*>(
allocate(sizeof(VkSpecializationInfo), REQUIRED_MEMORY_ALIGNMENT, DEVICE_MEMORY));
+ info = std::shared_ptr<VkSpecializationInfo>(ptr, Deleter());
+
info->mapEntryCount = specializationInfo->mapEntryCount;
if(specializationInfo->mapEntryCount > 0)
{
@@ -42,10 +44,14 @@
memcpy(data, specializationInfo->pData, specializationInfo->dataSize);
info->pData = data;
}
+ else
+ {
+ info->pData = nullptr;
+ }
}
}
-PipelineCache::SpirvShaderKey::SpecializationInfo::~SpecializationInfo()
+void PipelineCache::SpirvShaderKey::SpecializationInfo::Deleter::operator() (VkSpecializationInfo* info) const
{
if(info)
{
@@ -57,34 +63,34 @@
bool PipelineCache::SpirvShaderKey::SpecializationInfo::operator<(const SpecializationInfo& specializationInfo) const
{
- if(info && specializationInfo.info)
- {
- 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)
- {
- int cmp = memcmp(info->pMapEntries, specializationInfo.info->pMapEntries, info->mapEntryCount * sizeof(VkSpecializationMapEntry));
- if(cmp != 0)
- {
- return cmp < 0;
- }
- }
-
+ if(info && specializationInfo.info)
+ {
+ 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)
+ {
+ int cmp = memcmp(info->pMapEntries, specializationInfo.info->pMapEntries, info->mapEntryCount * sizeof(VkSpecializationMapEntry));
+ if(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;
- }
+ if(cmp != 0)
+ {
+ return cmp < 0;
+ }
}
}
@@ -106,46 +112,46 @@
{
}
-bool PipelineCache::SpirvShaderKey::operator<(const SpirvShaderKey &other) const
-{
- if(pipelineStage != other.pipelineStage)
- {
- return pipelineStage < other.pipelineStage;
- }
-
- if(renderPass != other.renderPass)
- {
- return renderPass < other.renderPass;
- }
-
- if(subpassIndex != other.subpassIndex)
- {
- return subpassIndex < other.subpassIndex;
- }
-
- if(insns.size() != other.insns.size())
- {
- return insns.size() < other.insns.size();
- }
-
- if(entryPointName.size() != other.entryPointName.size())
- {
- return entryPointName.size() < other.entryPointName.size();
- }
-
- int cmp = memcmp(entryPointName.c_str(), other.entryPointName.c_str(), entryPointName.size());
- if(cmp != 0)
- {
- return cmp < 0;
- }
-
- cmp = memcmp(insns.data(), other.insns.data(), insns.size() * sizeof(uint32_t));
- if(cmp != 0)
- {
- return cmp < 0;
- }
-
- return (specializationInfo < other.specializationInfo);
+bool PipelineCache::SpirvShaderKey::operator<(const SpirvShaderKey &other) const
+{
+ if(pipelineStage != other.pipelineStage)
+ {
+ return pipelineStage < other.pipelineStage;
+ }
+
+ if(renderPass != other.renderPass)
+ {
+ return renderPass < other.renderPass;
+ }
+
+ if(subpassIndex != other.subpassIndex)
+ {
+ return subpassIndex < other.subpassIndex;
+ }
+
+ if(insns.size() != other.insns.size())
+ {
+ return insns.size() < other.insns.size();
+ }
+
+ if(entryPointName.size() != other.entryPointName.size())
+ {
+ return entryPointName.size() < other.entryPointName.size();
+ }
+
+ int cmp = memcmp(entryPointName.c_str(), other.entryPointName.c_str(), entryPointName.size());
+ if(cmp != 0)
+ {
+ return cmp < 0;
+ }
+
+ cmp = memcmp(insns.data(), other.insns.data(), insns.size() * sizeof(uint32_t));
+ if(cmp != 0)
+ {
+ return cmp < 0;
+ }
+
+ return (specializationInfo < other.specializationInfo);
}
PipelineCache::PipelineCache(const VkPipelineCacheCreateInfo* pCreateInfo, void* mem) :
diff --git a/src/Vulkan/VkPipelineCache.hpp b/src/Vulkan/VkPipelineCache.hpp
index ffdfc2c..5195dbe 100644
--- a/src/Vulkan/VkPipelineCache.hpp
+++ b/src/Vulkan/VkPipelineCache.hpp
@@ -52,14 +52,18 @@
struct SpecializationInfo
{
SpecializationInfo(const VkSpecializationInfo* specializationInfo);
- ~SpecializationInfo();
bool operator<(const SpecializationInfo& specializationInfo) const;
- VkSpecializationInfo* get() const { return info; }
+ const VkSpecializationInfo* get() const { return info.get(); }
private:
- VkSpecializationInfo* info = nullptr;
+ struct Deleter
+ {
+ void operator()(VkSpecializationInfo*) const;
+ };
+
+ std::shared_ptr<VkSpecializationInfo> info;
};
SpirvShaderKey(const VkShaderStageFlagBits pipelineStage,
@@ -97,9 +101,9 @@
shader(shader), layout(layout)
{}
- bool operator<(const ComputeProgramKey &other) const
- {
- return (shader < other.shader) || (layout < other.layout);
+ bool operator<(const ComputeProgramKey &other) const
+ {
+ return (shader < other.shader) || (layout < other.layout);
}
const sw::SpirvShader* getShader() const { return shader; }
