SpirvShader: Add debug checks on Intermediate.
Memset the scalars to 0, and check their pointers before use. Catches cases where the intermediate was declared but not set.
Also switch from using assert() to ASSERT() as the latter still fires with DCHECK_ALWAYS_ON.
Change-Id: I81abb50aea41568f95c22a340b90b7c56839d3ce
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/25869
Reviewed-by: Nicolas Capens <nicolascapens@google.com>
Reviewed-by: Chris Forbes <chrisforbes@google.com>
Kokoro-Presubmit: kokoro <noreply+kokoro@google.com>
Tested-by: Ben Clayton <bclayton@google.com>
diff --git a/src/Pipeline/SpirvShader.cpp b/src/Pipeline/SpirvShader.cpp
index 1c02bd7..4789108 100644
--- a/src/Pipeline/SpirvShader.cpp
+++ b/src/Pipeline/SpirvShader.cpp
@@ -335,22 +335,22 @@
void SpirvShader::ProcessInterfaceVariable(Object &object)
{
auto &objectTy = getType(object.type);
- assert(objectTy.storageClass == spv::StorageClassInput || objectTy.storageClass == spv::StorageClassOutput);
+ ASSERT(objectTy.storageClass == spv::StorageClassInput || objectTy.storageClass == spv::StorageClassOutput);
- assert(objectTy.definition.opcode() == spv::OpTypePointer);
+ ASSERT(objectTy.definition.opcode() == spv::OpTypePointer);
auto pointeeTy = getType(objectTy.element);
auto &builtinInterface = (objectTy.storageClass == spv::StorageClassInput) ? inputBuiltins : outputBuiltins;
auto &userDefinedInterface = (objectTy.storageClass == spv::StorageClassInput) ? inputs : outputs;
- assert(object.definition.opcode() == spv::OpVariable);
+ ASSERT(object.definition.opcode() == spv::OpVariable);
ObjectID resultId = object.definition.word(2);
if (objectTy.isBuiltInBlock)
{
// walk the builtin block, registering each of its members separately.
auto m = memberDecorations.find(objectTy.element);
- assert(m != memberDecorations.end()); // otherwise we wouldn't have marked the type chain
+ ASSERT(m != memberDecorations.end()); // otherwise we wouldn't have marked the type chain
auto &structType = pointeeTy.definition;
auto offset = 0u;
auto word = 2u;
@@ -381,7 +381,7 @@
[&userDefinedInterface](Decorations const &d, AttribType type) {
// Populate a single scalar slot in the interface from a collection of decorations and the intended component type.
auto scalarSlot = (d.Location << 2) | d.Component;
- assert(scalarSlot >= 0 &&
+ ASSERT(scalarSlot >= 0 &&
scalarSlot < static_cast<int32_t>(userDefinedInterface.size()));
auto &slot = userDefinedInterface[scalarSlot];
@@ -564,7 +564,7 @@
ApplyDecorationsForId(&d, id);
auto def = getObject(id).definition;
- assert(def.opcode() == spv::OpVariable);
+ ASSERT(def.opcode() == spv::OpVariable);
VisitInterfaceInner<F>(def.word(1), d, f);
}
@@ -774,8 +774,8 @@
// TODO: not encountered yet since we only use this for array sizes etc,
// but is possible to construct integer constant 0 via OpConstantNull.
auto insn = getObject(id).definition;
- assert(insn.opcode() == spv::OpConstant);
- assert(getType(insn.word(1)).definition.opcode() == spv::OpTypeInt);
+ ASSERT(insn.opcode() == spv::OpConstant);
+ ASSERT(getType(insn.word(1)).definition.opcode() == spv::OpTypeInt);
return insn.word(3);
}
diff --git a/src/Pipeline/SpirvShader.hpp b/src/Pipeline/SpirvShader.hpp
index a90f5fb..e7b9966 100644
--- a/src/Pipeline/SpirvShader.hpp
+++ b/src/Pipeline/SpirvShader.hpp
@@ -20,6 +20,7 @@
#include "ShaderCore.hpp"
#include "SpirvID.hpp"
+#include <cstring>
#include <string>
#include <vector>
#include <unordered_map>
@@ -58,7 +59,11 @@
public:
using Scalar = RValue<SIMD::Float>;
- Intermediate(uint32_t size) : contents(new ContentsType[size]), size(size) {}
+ Intermediate(uint32_t size) : contents(new ContentsType[size]), size(size) {
+#if !defined(NDEBUG) || defined(DCHECK_ALWAYS_ON)
+ memset(contents, 0, sizeof(ContentsType[size]));
+#endif
+ }
~Intermediate()
{
@@ -69,20 +74,24 @@
void emplace(uint32_t n, Scalar&& value)
{
- assert(n < size);
+ ASSERT(n < size);
+ ASSERT(reinterpret_cast<Scalar const *>(&contents[n])->value == nullptr);
new (&contents[n]) Scalar(value);
}
void emplace(uint32_t n, const Scalar& value)
{
- assert(n < size);
+ ASSERT(n < size);
+ ASSERT(reinterpret_cast<Scalar const *>(&contents[n])->value == nullptr);
new (&contents[n]) Scalar(value);
}
Scalar const & operator[](uint32_t n) const
{
- assert(n < size);
- return *reinterpret_cast<Scalar const *>(&contents[n]);
+ ASSERT(n < size);
+ auto scalar = reinterpret_cast<Scalar const *>(&contents[n]);
+ ASSERT(scalar->value != nullptr);
+ return *scalar;
}
// No copy/move construction or assignment
@@ -340,14 +349,14 @@
Type const &getType(TypeID id) const
{
auto it = types.find(id);
- assert(it != types.end());
+ ASSERT(it != types.end());
return it->second;
}
Object const &getObject(ObjectID id) const
{
auto it = defs.find(id);
- assert(it != defs.end());
+ ASSERT(it != defs.end());
return it->second;
}
@@ -427,14 +436,14 @@
Value& getValue(SpirvShader::ObjectID id)
{
auto it = lvalues.find(id);
- assert(it != lvalues.end());
+ ASSERT(it != lvalues.end());
return it->second;
}
Intermediate const& getIntermediate(SpirvShader::ObjectID id) const
{
auto it = intermediates.find(id);
- assert(it != intermediates.end());
+ ASSERT(it != intermediates.end());
return it->second;
}
};
