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 (file)
--- a/src/Pipeline/SpirvShader.cpp
+++ b/src/Pipeline/SpirvShader.cpp
@@ -335,22 +335,22 @@ namespace sw
        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 @@ namespace sw
                                                   [&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 @@ namespace sw
                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 @@ namespace sw
                // 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 (file)
--- 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 @@ namespace sw
        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 @@ namespace sw
 
                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 @@ namespace sw
                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 @@ namespace sw
                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;
                }
        };