Skip to content

S
swiftshader
Commit e6092f3b by Ben Clayton
Options

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: 's avatarAlexis Hétu <sugoi@google.com> Tested-by: 's avatarBen Clayton <bclayton@google.com>
parent 03c2aea0
Showing with 83 additions and 73 deletions
src/Vulkan/VkPipelineCache.cpp
......@@ -22,9 +22,11 @@ PipelineCache::SpirvShaderKey::SpecializationInfo::SpecializationInfo(const VkSp
{
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 @@ PipelineCache::SpirvShaderKey::SpecializationInfo::SpecializationInfo(const VkSp
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 @@ PipelineCache::SpirvShaderKey::SpecializationInfo::~SpecializationInfo()
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 @@ PipelineCache::SpirvShaderKey::SpirvShaderKey(const VkShaderStageFlagBits pipeli
{
}
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) :
......
src/Vulkan/VkPipelineCache.hpp
......@@ -52,14 +52,18 @@ public:
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 @@ public:
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; }
......
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