Bug 1136410 - Forbid attrib aliasing. - r=mtseng'

MozReview-Commit-ID: 6shjIyJQQ6V

dom/canvas/WebGLActiveInfo.cpp

@@ -9,7 +9,7 @@
 
 namespace mozilla {
 
-static uint8_t
+uint8_t
 ElemSizeFromType(GLenum elemType)
 {
     switch (elemType) {

dom/canvas/WebGLActiveInfo.h

@@ -88,6 +88,10 @@ private:
     ~WebGLActiveInfo() { }
 };
 
+//////////
+
+uint8_t ElemSizeFromType(GLenum elemType);
+
 } // namespace mozilla
 
 #endif // WEBGL_ACTIVE_INFO_H_

dom/canvas/WebGLContext.h

@@ -1045,6 +1045,7 @@ private:
     bool mBufferFetchingHasPerVertex;
     uint32_t mMaxFetchedVertices;
     uint32_t mMaxFetchedInstances;
+    bool mBufferFetch_IsAttrib0Active;
 
     bool DrawArrays_check(GLint first, GLsizei count, GLsizei primcount,
                           const char* info);

dom/canvas/WebGLContextDraw.cpp

@@ -583,24 +583,39 @@ WebGLContext::ValidateBufferFetching(const char* info)
     bool hasPerVertex = false;
     uint32_t maxVertices = UINT32_MAX;
     uint32_t maxInstances = UINT32_MAX;
-    uint32_t attribs = mBoundVertexArray->mAttribs.Length();
-
-    for (uint32_t i = 0; i < attribs; ++i) {
-        const WebGLVertexAttribData& vd = mBoundVertexArray->mAttribs[i];
+    const uint32_t attribCount = mBoundVertexArray->mAttribs.Length();
 
+    uint32_t i = 0;
+    for (const auto& vd : mBoundVertexArray->mAttribs) {
         // If the attrib array isn't enabled, there's nothing to check;
         // it's a static value.
         if (!vd.enabled)
             continue;
 
         if (vd.buf == nullptr) {
-            ErrorInvalidOperation("%s: no VBO bound to enabled vertex attrib index %d!", info, i);
+            ErrorInvalidOperation("%s: no VBO bound to enabled vertex attrib index %du!",
+                                  info, i);
             return false;
         }
 
-        // If the attrib is not in use, then we don't have to validate
-        // it, just need to make sure that the binding is non-null.
-        if (!mActiveProgramLinkInfo->HasActiveAttrib(i))
+        ++i;
+    }
+
+    mBufferFetch_IsAttrib0Active = false;
+
+    for (const auto& pair : mActiveProgramLinkInfo->activeAttribLocs) {
+        const auto attrib = pair.first;
+        const uint32_t attribLoc = pair.second;
+
+        if (attribLoc >= attribCount)
+            continue;
+
+        if (attribLoc == 0) {
+            mBufferFetch_IsAttrib0Active = true;
+        }
+
+        const auto& vd = mBoundVertexArray->mAttribs[attribLoc];
+        if (!vd.enabled)
             continue;
 
         // the base offset
@@ -610,7 +625,9 @@ WebGLContext::ValidateBufferFetching(const char* info)
         if (!checked_byteLength.isValid() ||
             !checked_sizeOfLastElement.isValid())
         {
-            ErrorInvalidOperation("%s: integer overflow occured while checking vertex attrib %d", info, i);
+            ErrorInvalidOperation("%s: Integer overflow occured while checking vertex"
+                                  " attrib %u.",
+                                  info, attribLoc);
             return false;
         }
 
@@ -623,7 +640,9 @@ WebGLContext::ValidateBufferFetching(const char* info)
         CheckedUint32 checked_maxAllowedCount = ((checked_byteLength - checked_sizeOfLastElement) / vd.actualStride()) + 1;
 
         if (!checked_maxAllowedCount.isValid()) {
-            ErrorInvalidOperation("%s: integer overflow occured while checking vertex attrib %d", info, i);
+            ErrorInvalidOperation("%s: Integer overflow occured while checking vertex"
+                                  " attrib %u.",
+                                  info, attribLoc);
             return false;
         }
 
@@ -663,7 +682,7 @@ WebGLContext::WhatDoesVertexAttrib0Need()
 #ifdef XP_MACOSX
     if (gl->WorkAroundDriverBugs() &&
         mBoundVertexArray->IsAttribArrayEnabled(0) &&
-        !mActiveProgramLinkInfo->HasActiveAttrib(0))
+        !mBufferFetch_IsAttrib0Active)
     {
         return WebGLVertexAttrib0Status::EmulatedUninitializedArray;
     }
@@ -675,7 +694,7 @@ WebGLContext::WhatDoesVertexAttrib0Need()
         return WebGLVertexAttrib0Status::Default;
     }
 
-    return mActiveProgramLinkInfo->HasActiveAttrib(0)
+    return mBufferFetch_IsAttrib0Active
            ? WebGLVertexAttrib0Status::EmulatedInitializedArray
            : WebGLVertexAttrib0Status::EmulatedUninitializedArray;
 }

dom/canvas/WebGLProgram.cpp

@@ -69,7 +69,7 @@ ParseName(const nsCString& name, nsCString* const out_baseName,
     return true;
 }
 
-static void
+static WebGLActiveInfo*
 AddActiveInfo(WebGLContext* webgl, GLint elemCount, GLenum elemType, bool isArray,
               const nsACString& baseUserName, const nsACString& baseMappedName,
               std::vector<RefPtr<WebGLActiveInfo>>* activeInfoList,
@@ -81,6 +81,7 @@ AddActiveInfo(WebGLContext* webgl, GLint elemCount, GLenum elemType, bool isArra
     activeInfoList->push_back(info);
 
     infoLocMap->insert(std::make_pair(info->mBaseUserName, info.get()));
+    return info.get();
 }
 
 static void
@@ -168,8 +169,9 @@ QueryProgramInfo(WebGLProgram* prog, gl::GLContext* gl)
 #endif
 
         const bool isArray = false;
-        AddActiveInfo(prog->mContext, elemCount, elemType, isArray, userName, mappedName,
-                      &info->activeAttribs, &info->attribMap);
+        const auto attrib = AddActiveInfo(prog->mContext, elemCount, elemType, isArray,
+                                          userName, mappedName, &info->activeAttribs,
+                                          &info->attribMap);
 
         // Collect active locations:
         GLint loc = gl->fGetAttribLocation(prog->mGLName, mappedName.BeginReading());
@@ -177,7 +179,7 @@ QueryProgramInfo(WebGLProgram* prog, gl::GLContext* gl)
             if (mappedName != "gl_InstanceID")
                 MOZ_CRASH("GFX: Active attrib has no location.");
         } else {
-            info->activeAttribLocs.insert(loc);
+            info->activeAttribLocs.insert({attrib, (GLuint)loc});
         }
     }
 
@@ -963,21 +965,16 @@ WebGLProgram::LinkProgram()
     }
 
     LinkAndUpdate();
-    if (IsLinked()) {
-        // Check if the attrib name conflicting to uniform name
-        for (const auto& uniform : mMostRecentLinkInfo->uniformMap) {
-            if (mMostRecentLinkInfo->attribMap.find(uniform.first) != mMostRecentLinkInfo->attribMap.end()) {
-                mLinkLog = nsPrintfCString("The uniform name (%s) conflicts with attribute name.",
-                                           uniform.first.get());
-                mMostRecentLinkInfo = nullptr;
-                break;
-            }
-        }
-    }
 
-    if (mMostRecentLinkInfo)
+    if (mMostRecentLinkInfo) {
+        nsCString postLinkLog;
+        if (ValidateAfterTentativeLink(&postLinkLog))
             return;
 
+        mMostRecentLinkInfo = nullptr;
+        mLinkLog = postLinkLog;
+    }
+
     // Failed link.
     if (mContext->ShouldGenerateWarnings()) {
         // report shader/program infoLogs as warnings.
@@ -993,6 +990,57 @@ WebGLProgram::LinkProgram()
     }
 }
 
+bool
+WebGLProgram::ValidateAfterTentativeLink(nsCString* const out_linkLog) const
+{
+    const auto& linkInfo = mMostRecentLinkInfo;
+
+    // Check if the attrib name conflicting to uniform name
+    for (const auto& uniform : linkInfo->uniformMap) {
+        if (linkInfo->attribMap.find(uniform.first) != linkInfo->attribMap.end()) {
+            *out_linkLog = nsPrintfCString("The uniform name (%s) conflicts with"
+                                           " attribute name.",
+                                           uniform.first.get());
+            return false;
+        }
+    }
+
+    std::map<GLuint, const WebGLActiveInfo*> attribsByLoc;
+    for (const auto& pair : linkInfo->activeAttribLocs) {
+        const auto dupe = attribsByLoc.find(pair.second);
+        if (dupe != attribsByLoc.end()) {
+            *out_linkLog = nsPrintfCString("Aliased location between active attribs"
+                                           " \"%s\" and \"%s\".",
+                                           dupe->second->mBaseUserName.BeginReading(),
+                                           pair.first->mBaseUserName.BeginReading());
+            return false;
+        }
+    }
+
+    for (const auto& pair : attribsByLoc) {
+        const GLuint attribLoc = pair.first;
+        const auto attrib = pair.second;
+
+        const auto elemSize = ElemSizeFromType(attrib->mElemType);
+        const GLuint locationsUsed = (elemSize + 3) / 4;
+        for (GLuint i = 1; i < locationsUsed; i++) {
+            const GLuint usedLoc = attribLoc + i;
+
+            const auto dupe = attribsByLoc.find(usedLoc);
+            if (dupe != attribsByLoc.end()) {
+                *out_linkLog = nsPrintfCString("Attrib \"%s\" of type \"0x%04x\" aliases"
+                                               " \"%s\" by overhanging its location.",
+                                               attrib->mBaseUserName.BeginReading(),
+                                               attrib->mElemType,
+                                               dupe->second->mBaseUserName.BeginReading());
+                return false;
+            }
+        }
+    }
+
+    return true;
+}
+
 bool
 WebGLProgram::UseProgram() const
 {
@@ -1061,7 +1109,7 @@ WebGLProgram::LinkAndUpdate()
         return;
 
     mMostRecentLinkInfo = QueryProgramInfo(this, gl);
-    MOZ_RELEASE_ASSERT(mMostRecentLinkInfo, "GFX: most rent link info not set.");
+    MOZ_RELEASE_ASSERT(mMostRecentLinkInfo, "GFX: most recent link info not set.");
 }
 
 bool

dom/canvas/WebGLProgram.h

@@ -71,7 +71,9 @@ struct LinkedProgramInfo final
     std::vector<RefPtr<UniformBlockInfo>> uniformBlocks;
 
     // Needed for draw call validation.
-    std::set<GLuint> activeAttribLocs;
+    std::map<const WebGLActiveInfo*, GLuint> activeAttribLocs;
+
+    //////
 
     explicit LinkedProgramInfo(WebGLProgram* prog);
 
@@ -110,11 +112,6 @@ struct LinkedProgramInfo final
 
         return false;
     }
-
-    bool HasActiveAttrib(GLuint loc) const {
-        auto itr = activeAttribLocs.find(loc);
-        return itr != activeAttribLocs.end();
-    }
 };
 
 } // namespace webgl
@@ -196,6 +193,7 @@ private:
     ~WebGLProgram();
 
     void LinkAndUpdate();
+    bool ValidateAfterTentativeLink(nsCString* const out_linkLog) const;
 
 public:
     const GLuint mGLName;