Reject shaders using attribute aliasing.

src/libANGLE/Program.cpp

@@ -322,7 +322,7 @@ Error Program::link(const Data &data)
     }
     ASSERT(mVertexShader->getType() == GL_VERTEX_SHADER);
 
-    if (!linkAttributes(mInfoLog, mAttributeBindings, mVertexShader))
+    if (!linkAttributes(data, mInfoLog, mAttributeBindings, mVertexShader))
     {
         return Error(GL_NO_ERROR);
     }
@@ -446,15 +446,25 @@ Error Program::loadBinary(GLenum binaryFormat, const void *binary, GLsizei lengt
         return Error(GL_NO_ERROR);
     }
 
+    // TODO(jmadill): replace MAX_VERTEX_ATTRIBS
     for (int i = 0; i < MAX_VERTEX_ATTRIBS; ++i)
     {
         stream.readInt(&mLinkedAttribute[i].type);
         stream.readString(&mLinkedAttribute[i].name);
-        stream.readInt(&mProgram->getShaderAttributes()[i].type);
-        stream.readString(&mProgram->getShaderAttributes()[i].name);
         stream.readInt(&mProgram->getSemanticIndexes()[i]);
     }
 
+    unsigned int attribCount = stream.readInt<unsigned int>();
+    for (unsigned int attribIndex = 0; attribIndex < attribCount; ++attribIndex)
+    {
+        GLenum type = stream.readInt<GLenum>();
+        GLenum precision = stream.readInt<GLenum>();
+        std::string name = stream.readString();
+        GLint arraySize = stream.readInt<GLint>();
+        int location = stream.readInt<int>();
+        mProgram->setShaderAttribute(attribIndex, type, precision, name, arraySize, location);
+    }
+
     rx::LinkResult result = mProgram->load(mInfoLog, &stream);
     if (result.error.isError() || !result.linkSuccess)
     {
@@ -480,15 +490,25 @@ Error Program::saveBinary(GLenum *binaryFormat, void *binary, GLsizei bufSize, G
     stream.writeInt(ANGLE_MINOR_VERSION);
     stream.writeBytes(reinterpret_cast<const unsigned char*>(ANGLE_COMMIT_HASH), ANGLE_COMMIT_HASH_SIZE);
 
+    // TODO(jmadill): replace MAX_VERTEX_ATTRIBS
     for (unsigned int i = 0; i < MAX_VERTEX_ATTRIBS; ++i)
     {
         stream.writeInt(mLinkedAttribute[i].type);
         stream.writeString(mLinkedAttribute[i].name);
-        stream.writeInt(mProgram->getShaderAttributes()[i].type);
-        stream.writeString(mProgram->getShaderAttributes()[i].name);
         stream.writeInt(mProgram->getSemanticIndexes()[i]);
     }
 
+    const auto &shaderAttributes = mProgram->getShaderAttributes();
+    stream.writeInt(shaderAttributes.size());
+    for (const auto &attrib : shaderAttributes)
+    {
+        stream.writeInt(attrib.type);
+        stream.writeInt(attrib.precision);
+        stream.writeString(attrib.name);
+        stream.writeInt(attrib.arraySize);
+        stream.writeInt(attrib.location);
+    }
+
     gl::Error error = mProgram->save(&stream);
     if (error.isError())
     {
@@ -1306,10 +1326,21 @@ bool Program::linkValidateInterfaceBlockFields(InfoLog &infoLog, const std::stri
 }
 
 // Determines the mapping between GL attributes and Direct3D 9 vertex stream usage indices
-bool Program::linkAttributes(InfoLog &infoLog, const AttributeBindings &attributeBindings, const Shader *vertexShader)
+bool Program::linkAttributes(const Data &data,
+                             InfoLog &infoLog,
+                             const AttributeBindings &attributeBindings,
+                             const Shader *vertexShader)
 {
     unsigned int usedLocations = 0;
     const std::vector<sh::Attribute> &shaderAttributes = vertexShader->getActiveAttributes();
+    GLuint maxAttribs = data.caps->maxVertexAttributes;
+
+    // TODO(jmadill): handle aliasing robustly
+    if (shaderAttributes.size() >= maxAttribs)
+    {
+        infoLog.append("Too many vertex attributes.");
+        return false;
+    }
 
     // Link attributes that have a binding location
     for (unsigned int attributeIndex = 0; attributeIndex < shaderAttributes.size(); attributeIndex++)
@@ -1320,13 +1351,13 @@ bool Program::linkAttributes(InfoLog &infoLog, const AttributeBindings &attribut
 
         const int location = attribute.location == -1 ? attributeBindings.getAttributeBinding(attribute.name) : attribute.location;
 
-        mProgram->getShaderAttributes()[attributeIndex] = attribute;
+        mProgram->setShaderAttribute(attributeIndex, attribute);
 
         if (location != -1)   // Set by glBindAttribLocation or by location layout qualifier
         {
             const int rows = VariableRegisterCount(attribute.type);
 
-            if (rows + location > MAX_VERTEX_ATTRIBS)
+            if (static_cast<GLuint>(rows + location) > maxAttribs)
             {
                 infoLog.append("Active attribute (%s) at location %d is too big to fit", attribute.name.c_str(), location);
 
@@ -1339,8 +1370,9 @@ bool Program::linkAttributes(InfoLog &infoLog, const AttributeBindings &attribut
                 sh::ShaderVariable &linkedAttribute = mLinkedAttribute[rowLocation];
 
                 // In GLSL 3.00, attribute aliasing produces a link error
-                // In GLSL 1.00, attribute aliasing is allowed
-                if (mProgram->getShaderVersion() >= 300)
+                // In GLSL 1.00, attribute aliasing is allowed, but ANGLE currently has a bug
+                // TODO(jmadill): fix aliasing on ES2
+                // if (mProgram->getShaderVersion() >= 300)
                 {
                     if (!linkedAttribute.name.empty())
                     {
@@ -1367,9 +1399,9 @@ bool Program::linkAttributes(InfoLog &infoLog, const AttributeBindings &attribut
         if (location == -1)   // Not set by glBindAttribLocation or by location layout qualifier
         {
             int rows = VariableRegisterCount(attribute.type);
-            int availableIndex = AllocateFirstFreeBits(&usedLocations, rows, MAX_VERTEX_ATTRIBS);
+            int availableIndex = AllocateFirstFreeBits(&usedLocations, rows, maxAttribs);
 
-            if (availableIndex == -1 || availableIndex + rows > MAX_VERTEX_ATTRIBS)
+            if (availableIndex == -1 || static_cast<GLuint>(availableIndex + rows) > maxAttribs)
             {
                 infoLog.append("Too many active attributes (%s)", attribute.name.c_str());
 
@@ -1380,7 +1412,7 @@ bool Program::linkAttributes(InfoLog &infoLog, const AttributeBindings &attribut
         }
     }
 
-    for (int attributeIndex = 0; attributeIndex < MAX_VERTEX_ATTRIBS; )
+    for (GLuint attributeIndex = 0; attributeIndex < maxAttribs;)
     {
         int index = vertexShader->getSemanticIndex(mLinkedAttribute[attributeIndex].name);
         int rows = VariableRegisterCount(mLinkedAttribute[attributeIndex].type);

src/libANGLE/Program.h

@@ -224,7 +224,10 @@ class Program : angle::NonCopyable
     void unlink(bool destroy = false);
     void resetUniformBlockBindings();
 
-    bool linkAttributes(InfoLog &infoLog, const AttributeBindings &attributeBindings, const Shader *vertexShader);
+    bool linkAttributes(const Data &data,
+                        InfoLog &infoLog,
+                        const AttributeBindings &attributeBindings,
+                        const Shader *vertexShader);
     bool linkUniformBlocks(InfoLog &infoLog, const Shader &vertexShader, const Shader &fragmentShader, const Caps &caps);
     bool areMatchingInterfaceBlocks(gl::InfoLog &infoLog, const sh::InterfaceBlock &vertexInterfaceBlock,
                                     const sh::InterfaceBlock &fragmentInterfaceBlock);

src/libANGLE/renderer/ProgramImpl.cpp

@@ -131,4 +131,26 @@ void ProgramImpl::reset()
     mTransformFeedbackLinkedVaryings.clear();
 }
 
+void ProgramImpl::setShaderAttribute(size_t index, const sh::Attribute &attrib)
+{
+    if (mShaderAttributes.size() <= index)
+    {
+        mShaderAttributes.resize(index + 1);
+    }
+    mShaderAttributes[index] = attrib;
+}
+
+void ProgramImpl::setShaderAttribute(size_t index, GLenum type, GLenum precision, const std::string &name, GLint size, int location)
+{
+    if (mShaderAttributes.size() <= index)
+    {
+        mShaderAttributes.resize(index + 1);
+    }
+    mShaderAttributes[index].type = type;
+    mShaderAttributes[index].precision = precision;
+    mShaderAttributes[index].name = name;
+    mShaderAttributes[index].arraySize = size;
+    mShaderAttributes[index].location = location;
+}
+
 }

src/libANGLE/renderer/ProgramImpl.h

@@ -102,14 +102,13 @@ class ProgramImpl : angle::NonCopyable
     const std::vector<gl::VariableLocation> &getUniformIndices() const { return mUniformIndex; }
     const std::vector<gl::UniformBlock*> &getUniformBlocks() const { return mUniformBlocks; }
     const std::vector<gl::LinkedVarying> &getTransformFeedbackLinkedVaryings() const { return mTransformFeedbackLinkedVaryings; }
-    const sh::Attribute *getShaderAttributes() const { return mShaderAttributes; }
+    const std::vector<sh::Attribute> getShaderAttributes() { return mShaderAttributes; }
     const SemanticIndexArray &getSemanticIndexes() const { return mSemanticIndex; }
 
     std::vector<gl::LinkedUniform*> &getUniforms() { return mUniforms; }
     std::vector<gl::VariableLocation> &getUniformIndices() { return mUniformIndex; }
     std::vector<gl::UniformBlock*> &getUniformBlocks() { return mUniformBlocks; }
     std::vector<gl::LinkedVarying> &getTransformFeedbackLinkedVaryings() { return mTransformFeedbackLinkedVaryings; }
-    sh::Attribute *getShaderAttributes() { return mShaderAttributes; }
     SemanticIndexArray &getSemanticIndexes() { return mSemanticIndex; }
 
     gl::LinkedUniform *getUniformByLocation(GLint location) const;
@@ -120,6 +119,9 @@ class ProgramImpl : angle::NonCopyable
     GLuint getUniformIndex(const std::string &name) const;
     GLuint getUniformBlockIndex(const std::string &name) const;
 
+    void setShaderAttribute(size_t index, const sh::Attribute &attrib);
+    void setShaderAttribute(size_t index, GLenum type, GLenum precision, const std::string &name, GLint size, int location);
+
     virtual void reset();
 
   protected:
@@ -129,7 +131,9 @@ class ProgramImpl : angle::NonCopyable
     std::vector<gl::LinkedVarying> mTransformFeedbackLinkedVaryings;
 
     SemanticIndexArray mSemanticIndex;
-    sh::Attribute mShaderAttributes[gl::MAX_VERTEX_ATTRIBS];
+
+  private:
+    std::vector<sh::Attribute> mShaderAttributes;
 };
 
 }

src/libANGLE/renderer/d3d/DynamicHLSL.cpp

@@ -379,7 +379,7 @@ std::string DynamicHLSL::generateVaryingHLSL(const ShaderD3D *shader) const
 
 std::string DynamicHLSL::generateVertexShaderForInputLayout(const std::string &sourceShader,
                                                             const VertexFormat inputLayout[],
-                                                            const sh::Attribute shaderAttributes[]) const
+                                                            const std::vector<sh::Attribute> &shaderAttributes) const
 {
     std::string structHLSL, initHLSL;
 
@@ -406,7 +406,7 @@ std::string DynamicHLSL::generateVertexShaderForInputLayout(const std::string &s
         structHLSL += "    float2 spriteTexCoord : SPRITETEXCOORD0;\n";
     }
 
-    for (unsigned int attributeIndex = 0; attributeIndex < MAX_VERTEX_ATTRIBS; attributeIndex++)
+    for (size_t attributeIndex = 0; attributeIndex < shaderAttributes.size(); ++attributeIndex)
     {
         const sh::Attribute &shaderAttribute = shaderAttributes[attributeIndex];
         if (!shaderAttribute.name.empty())

src/libANGLE/renderer/d3d/DynamicHLSL.h

@@ -56,8 +56,9 @@ class DynamicHLSL : angle::NonCopyable
 
     int packVaryings(gl::InfoLog &infoLog, VaryingPacking packing, ShaderD3D *fragmentShader,
                      ShaderD3D *vertexShader, const std::vector<std::string>& transformFeedbackVaryings);
-    std::string generateVertexShaderForInputLayout(const std::string &sourceShader, const gl::VertexFormat inputLayout[],
-                                                   const sh::Attribute shaderAttributes[]) const;
+    std::string generateVertexShaderForInputLayout(const std::string &sourceShader,
+                                                   const gl::VertexFormat inputLayout[],
+                                                   const std::vector<sh::Attribute> &shaderAttributes) const;
     std::string generatePixelShaderForOutputSignature(const std::string &sourceShader, const std::vector<PixelShaderOutputVariable> &outputVariables,
                                                       bool usesFragDepth, const std::vector<GLenum> &outputLayout) const;
     bool generateShaderLinkHLSL(const gl::Data &data, gl::InfoLog &infoLog, int registers,

src/libANGLE/renderer/d3d/ProgramD3D.cpp

@@ -924,7 +924,7 @@ gl::Error ProgramD3D::getVertexExecutableForInputLayout(const gl::VertexFormat i
     }
 
     // Generate new dynamic layout with attribute conversions
-    std::string finalVertexHLSL = mDynamicHLSL->generateVertexShaderForInputLayout(mVertexHLSL, inputLayout, mShaderAttributes);
+    std::string finalVertexHLSL = mDynamicHLSL->generateVertexShaderForInputLayout(mVertexHLSL, inputLayout, getShaderAttributes());
 
     // Generate new vertex executable
     ShaderExecutableD3D *vertexExecutable = NULL;

src/libANGLE/renderer/gl/ProgramGL.cpp

@@ -183,12 +183,8 @@ LinkResult ProgramGL::link(const gl::Data &data, gl::InfoLog &infoLog,
 
         std::string attributeName(&attributeNameBuffer[0], attributeNameLength);
 
-        mShaderAttributes[i].type = attributeType;
         // TODO: determine attribute precision
-        mShaderAttributes[i].precision = GL_NONE;
-        mShaderAttributes[i].name = attributeName;
-        mShaderAttributes[i].arraySize = attributeSize;
-        mShaderAttributes[i].location = i;
+        setShaderAttribute(static_cast<size_t>(i), attributeType, GL_NONE, attributeName, attributeSize, i);
     }
 
     return LinkResult(true, gl::Error(GL_NO_ERROR));