Bug 1136410 - Forbid attrib aliasing. - r=mtseng' draft
authorJeff Gilbert <jgilbert@mozilla.com>
Thu, 07 Jul 2016 09:12:10 -0700
changeset 385279 e556ce3f60488c81b43c67647903d68fd77b98b4
parent 384060 dbb31bcad5a1f60a35b5600ea1578d9b9fa55237
child 524901 34b07e6ef963cf27fe11f3b7d68fc1407ebbd0c1
push id22474
push userbmo:jgilbert@mozilla.com
push dateFri, 08 Jul 2016 02:46:27 +0000
reviewersmtseng
bugs1136410
milestone50.0a1
Bug 1136410 - Forbid attrib aliasing. - r=mtseng' MozReview-Commit-ID: 6shjIyJQQ6V
dom/canvas/WebGLActiveInfo.cpp
dom/canvas/WebGLActiveInfo.h
dom/canvas/WebGLContext.h
dom/canvas/WebGLContextDraw.cpp
dom/canvas/WebGLProgram.cpp
dom/canvas/WebGLProgram.h
--- a/dom/canvas/WebGLActiveInfo.cpp
+++ b/dom/canvas/WebGLActiveInfo.cpp
@@ -4,17 +4,17 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "WebGLActiveInfo.h"
 
 #include "mozilla/dom/WebGLRenderingContextBinding.h"
 
 namespace mozilla {
 
-static uint8_t
+uint8_t
 ElemSizeFromType(GLenum elemType)
 {
     switch (elemType) {
     case LOCAL_GL_BOOL:
     case LOCAL_GL_FLOAT:
     case LOCAL_GL_INT:
     case LOCAL_GL_UNSIGNED_INT:
     case LOCAL_GL_SAMPLER_2D:
--- a/dom/canvas/WebGLActiveInfo.h
+++ b/dom/canvas/WebGLActiveInfo.h
@@ -83,11 +83,15 @@ private:
         , mElemSize(0)
         , mBaseMappedName("")
     { }
 
     // Private destructor, to discourage deletion outside of Release():
     ~WebGLActiveInfo() { }
 };
 
+//////////
+
+uint8_t ElemSizeFromType(GLenum elemType);
+
 } // namespace mozilla
 
 #endif // WEBGL_ACTIVE_INFO_H_
--- a/dom/canvas/WebGLContext.h
+++ b/dom/canvas/WebGLContext.h
@@ -1026,16 +1026,17 @@ public:
 
 private:
     // Cache the max number of vertices and instances that can be read from
     // bound VBOs (result of ValidateBuffers).
     bool mBufferFetchingIsVerified;
     bool mBufferFetchingHasPerVertex;
     uint32_t mMaxFetchedVertices;
     uint32_t mMaxFetchedInstances;
+    bool mBufferFetch_IsAttrib0Active;
 
     bool DrawArrays_check(GLint first, GLsizei count, GLsizei primcount,
                           const char* info);
     bool DrawElements_check(GLsizei count, GLenum type, WebGLintptr byteOffset,
                             GLsizei primcount, const char* info,
                             GLuint* out_upperBound);
     bool DrawInstanced_check(const char* info);
     void Draw_cleanup(const char* funcName);
--- a/dom/canvas/WebGLContextDraw.cpp
+++ b/dom/canvas/WebGLContextDraw.cpp
@@ -578,57 +578,76 @@ WebGLContext::ValidateBufferFetching(con
 #endif
 
     if (mBufferFetchingIsVerified)
         return true;
 
     bool hasPerVertex = false;
     uint32_t maxVertices = UINT32_MAX;
     uint32_t maxInstances = UINT32_MAX;
-    uint32_t attribs = mBoundVertexArray->mAttribs.Length();
+    const uint32_t attribCount = mBoundVertexArray->mAttribs.Length();
 
-    for (uint32_t i = 0; i < attribs; ++i) {
-        const WebGLVertexAttribData& vd = mBoundVertexArray->mAttribs[i];
-
+    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
         CheckedUint32 checked_byteLength = CheckedUint32(vd.buf->ByteLength()) - vd.byteOffset;
         CheckedUint32 checked_sizeOfLastElement = CheckedUint32(vd.componentSize()) * vd.size;
 
         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;
         }
 
         if (checked_byteLength.value() < checked_sizeOfLastElement.value()) {
             maxVertices = 0;
             maxInstances = 0;
             break;
         }
 
         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;
         }
 
         if (vd.divisor == 0) {
             maxVertices = std::min(maxVertices, checked_maxAllowedCount.value());
             hasPerVertex = true;
         } else {
             CheckedUint32 checked_curMaxInstances = checked_maxAllowedCount * vd.divisor;
@@ -658,29 +677,29 @@ WebGLContext::WhatDoesVertexAttrib0Need(
 {
     MOZ_ASSERT(mCurrentProgram);
     MOZ_ASSERT(mActiveProgramLinkInfo);
 
     // work around Mac OSX crash, see bug 631420
 #ifdef XP_MACOSX
     if (gl->WorkAroundDriverBugs() &&
         mBoundVertexArray->IsAttribArrayEnabled(0) &&
-        !mActiveProgramLinkInfo->HasActiveAttrib(0))
+        !mBufferFetch_IsAttrib0Active)
     {
         return WebGLVertexAttrib0Status::EmulatedUninitializedArray;
     }
 #endif
 
     if (MOZ_LIKELY(gl->IsGLES() ||
                    mBoundVertexArray->IsAttribArrayEnabled(0)))
     {
         return WebGLVertexAttrib0Status::Default;
     }
 
-    return mActiveProgramLinkInfo->HasActiveAttrib(0)
+    return mBufferFetch_IsAttrib0Active
            ? WebGLVertexAttrib0Status::EmulatedInitializedArray
            : WebGLVertexAttrib0Status::EmulatedUninitializedArray;
 }
 
 bool
 WebGLContext::DoFakeVertexAttrib0(GLuint vertexCount)
 {
     WebGLVertexAttrib0Status whatDoesAttrib0Need = WhatDoesVertexAttrib0Need();
--- a/dom/canvas/WebGLProgram.cpp
+++ b/dom/canvas/WebGLProgram.cpp
@@ -64,28 +64,29 @@ ParseName(const nsCString& name, nsCStri
         return false;
 
     *out_baseName = StringHead(name, indexOpenBracket);
     *out_isArray = true;
     *out_arrayIndex = indexNum;
     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,
               std::map<nsCString, const WebGLActiveInfo*>* infoLocMap)
 {
     RefPtr<WebGLActiveInfo> info = new WebGLActiveInfo(webgl, elemCount, elemType,
                                                        isArray, baseUserName,
                                                        baseMappedName);
     activeInfoList->push_back(info);
 
     infoLocMap->insert(std::make_pair(info->mBaseUserName, info.get()));
+    return info.get();
 }
 
 static void
 AddActiveBlockInfo(const nsACString& baseUserName,
                    const nsACString& baseMappedName,
                    std::vector<RefPtr<webgl::UniformBlockInfo>>* activeInfoList)
 {
     RefPtr<webgl::UniformBlockInfo> info = new webgl::UniformBlockInfo(baseUserName, baseMappedName);
@@ -163,26 +164,27 @@ QueryProgramInfo(WebGLProgram* prog, gl:
 
 #ifdef DUMP_SHADERVAR_MAPPINGS
         printf_stderr("[attrib %i] %s/%s\n", i, mappedName.BeginReading(),
                       userName.BeginReading());
         printf_stderr("    lengthWithoutNull: %d\n", lengthWithoutNull);
 #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());
         if (loc == -1) {
             if (mappedName != "gl_InstanceID")
                 MOZ_CRASH("GFX: Active attrib has no location.");
         } else {
-            info->activeAttribLocs.insert(loc);
+            info->activeAttribLocs.insert({attrib, (GLuint)loc});
         }
     }
 
     // Uniforms
 
     const bool needsCheckForArrays = gl->WorkAroundDriverBugs();
 
     GLuint numActiveUniforms = 0;
@@ -958,47 +960,93 @@ WebGLProgram::LinkProgram()
         // This can't be done trivially, because we have to deal with mapped names too.
         mVertShader->ApplyTransformFeedbackVaryings(mGLName,
                                                     mTransformFeedbackVaryings,
                                                     mTransformFeedbackBufferMode,
                                                     &mTempMappedVaryings);
     }
 
     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) {
+        nsCString postLinkLog;
+        if (ValidateAfterTentativeLink(&postLinkLog))
+            return;
+
+        mMostRecentLinkInfo = nullptr;
+        mLinkLog = postLinkLog;
     }
 
-    if (mMostRecentLinkInfo)
-        return;
-
     // Failed link.
     if (mContext->ShouldGenerateWarnings()) {
         // report shader/program infoLogs as warnings.
         // note that shader compilation errors can be deferred to linkProgram,
         // which is why we can't do anything in compileShader. In practice we could
         // report in compileShader the translation errors generated by ANGLE,
         // but it seems saner to keep a single way of obtaining shader infologs.
         if (!mLinkLog.IsEmpty()) {
             mContext->GenerateWarning("linkProgram: Failed to link, leaving the following"
                                       " log:\n%s\n",
                                       mLinkLog.BeginReading());
         }
     }
 }
 
 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
 {
     if (!mMostRecentLinkInfo) {
         mContext->ErrorInvalidOperation("useProgram: Program has not been successfully"
                                         " linked.");
         return false;
     }
 
@@ -1056,17 +1104,17 @@ WebGLProgram::LinkAndUpdate()
     empty.swap(mTempMappedVaryings);
 
     GLint ok = 0;
     gl->fGetProgramiv(mGLName, LOCAL_GL_LINK_STATUS, &ok);
     if (!ok)
         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
 WebGLProgram::FindActiveOutputMappedNameByUserName(const nsACString& userName,
                                                    nsCString* const out_mappedName) const
 {
     if (mFragShader->FindActiveOutputMappedNameByUserName(userName, out_mappedName)) {
         return true;
--- a/dom/canvas/WebGLProgram.h
+++ b/dom/canvas/WebGLProgram.h
@@ -66,17 +66,19 @@ struct LinkedProgramInfo final
     // user-facing `GLActiveInfo::name`s, without any final "[0]".
     std::map<nsCString, const WebGLActiveInfo*> attribMap;
     std::map<nsCString, const WebGLActiveInfo*> uniformMap;
     std::map<nsCString, const WebGLActiveInfo*> transformFeedbackVaryingsMap;
 
     std::vector<RefPtr<UniformBlockInfo>> uniformBlocks;
 
     // Needed for draw call validation.
-    std::set<GLuint> activeAttribLocs;
+    std::map<const WebGLActiveInfo*, GLuint> activeAttribLocs;
+
+    //////
 
     explicit LinkedProgramInfo(WebGLProgram* prog);
 
     bool FindAttrib(const nsCString& baseUserName,
                     const WebGLActiveInfo** const out_activeInfo) const
     {
         auto itr = attribMap.find(baseUserName);
         if (itr == attribMap.end())
@@ -105,21 +107,16 @@ struct LinkedProgramInfo final
             if (baseUserName == uniformBlocks[i]->mBaseUserName) {
                 *out_info = uniformBlocks[i].get();
                 return true;
             }
         }
 
         return false;
     }
-
-    bool HasActiveAttrib(GLuint loc) const {
-        auto itr = activeAttribLocs.find(loc);
-        return itr != activeAttribLocs.end();
-    }
 };
 
 } // namespace webgl
 
 class WebGLProgram final
     : public nsWrapperCache
     , public WebGLRefCountedObject<WebGLProgram>
     , public LinkedListElement<WebGLProgram>
@@ -191,16 +188,17 @@ public:
     }
 
     virtual JSObject* WrapObject(JSContext* js, JS::Handle<JSObject*> givenProto) override;
 
 private:
     ~WebGLProgram();
 
     void LinkAndUpdate();
+    bool ValidateAfterTentativeLink(nsCString* const out_linkLog) const;
 
 public:
     const GLuint mGLName;
 
 private:
     WebGLRefPtr<WebGLShader> mVertShader;
     WebGLRefPtr<WebGLShader> mFragShader;
     std::map<nsCString, GLuint> mBoundAttribLocs;