Bug 1136410 - Forbid attrib aliasing. - r=mtseng'
authorJeff Gilbert <jgilbert@mozilla.com>
Thu, 07 Jul 2016 09:12:10 -0700
changeset 304566 fd20afbbe00dfffaec1bdf196dd3eed0454e4fcf
parent 304565 b957e4b64c2bbb0de38c66cb89f91578d95d204c
child 304567 a39f9b0037aee910b833655a62f7177d522b5353
push id30432
push usercbook@mozilla.com
push dateTue, 12 Jul 2016 08:58:43 +0000
treeherdermozilla-central@aac8ff1024c5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmtseng
bugs1136410
milestone50.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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
@@ -1040,16 +1040,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;