From d0f3dc4cc0ba86006ec32b9429e0cbdfc59df6f8 Mon Sep 17 00:00:00 2001 From: xurui Date: Thu, 1 Feb 2024 22:00:34 +0800 Subject: [PATCH] =?UTF-8?q?fixed=20cc0425c=20from=20https://gitee.com/clea?= =?UTF-8?q?r=5Faddr/chromium=5Fthird=5Fparty=5Fangle/pulls/16=20=E4=BF=AE?= =?UTF-8?q?=E5=A4=8DCVE-2024-0223?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: xurui --- src/compiler/translator/Compiler.cpp | 12 +- .../ValidateTypeSizeLimitations.cpp | 131 +++++++++++++---- .../angle_end2end_tests_expectations.txt | 8 ++ .../RecordConstantPrecision_test.cpp | 6 +- src/tests/gl_tests/GLSLTest.cpp | 133 ++++++++++++++++++ 5 files changed, 251 insertions(+), 39 deletions(-) diff --git a/src/compiler/translator/Compiler.cpp b/src/compiler/translator/Compiler.cpp index 2b4f99d..d5a1a93 100644 --- a/src/compiler/translator/Compiler.cpp +++ b/src/compiler/translator/Compiler.cpp @@ -673,11 +673,6 @@ bool TCompiler::checkAndSimplifyAST(TIntermBlock *root, return false; } - if (shouldLimitTypeSizes() && !ValidateTypeSizeLimitations(root, &mSymbolTable, &mDiagnostics)) - { - return false; - } - if (!ValidateFragColorAndFragData(mShaderType, mShaderVersion, mSymbolTable, &mDiagnostics)) { return false; @@ -938,6 +933,13 @@ bool TCompiler::checkAndSimplifyAST(TIntermBlock *root, return false; } + // Run after RemoveUnreferencedVariables, validate that the shader does not have excessively + // large variables. + if (shouldLimitTypeSizes() && !ValidateTypeSizeLimitations(root, &mSymbolTable, &mDiagnostics)) + { + return false; + } + // Built-in function emulation needs to happen after validateLimitations pass. GetGlobalPoolAllocator()->lock(); initBuiltInFunctionEmulator(&mBuiltInFunctionEmulator, compileOptions); diff --git a/src/compiler/translator/ValidateTypeSizeLimitations.cpp b/src/compiler/translator/ValidateTypeSizeLimitations.cpp index 2a033ad..8b6be48 100644 --- a/src/compiler/translator/ValidateTypeSizeLimitations.cpp +++ b/src/compiler/translator/ValidateTypeSizeLimitations.cpp @@ -23,10 +23,11 @@ namespace // Arbitrarily enforce that all types declared with a size in bytes of over 2 GB will cause // compilation failure. // -// For local and global variables, the limit is much lower (1MB) as that much memory won't fit in +// For local and global variables, the limit is much lower (64KB) as that much memory won't fit in // the GPU registers anyway. -constexpr size_t kMaxVariableSizeInBytes = static_cast(2) * 1024 * 1024 * 1024; -constexpr size_t kMaxPrivateVariableSizeInBytes = static_cast(1) * 1024 * 1024; +constexpr size_t kMaxVariableSizeInBytes = static_cast(2) * 1024 * 1024 * 1024; +constexpr size_t kMaxPrivateVariableSizeInBytes = static_cast(64) * 1024; +constexpr size_t kMaxTotalPrivateVariableSizeInBytes = static_cast(16) * 1024 * 1024; // Traverses intermediate tree to ensure that the shader does not // exceed certain implementation-defined limits on the sizes of types. @@ -69,43 +70,111 @@ class ValidateTypeSizeLimitationsTraverser : public TIntermTraverser continue; } - const TType &variableType = asSymbol->getType(); - - // Create a ShaderVariable from which to compute - // (conservative) sizing information. - ShaderVariable shaderVar; - setCommonVariableProperties(variableType, variable, &shaderVar); - - // Compute the std140 layout of this variable, assuming - // it's a member of a block (which it might not be). - Std140BlockEncoder layoutEncoder; - BlockEncoderVisitor visitor("", "", &layoutEncoder); - // Since the size limit's arbitrary, it doesn't matter - // whether the row-major layout is correctly determined. - bool isRowMajorLayout = false; - TraverseShaderVariable(shaderVar, isRowMajorLayout, &visitor); - if (layoutEncoder.getCurrentOffset() > kMaxVariableSizeInBytes) + if (!validateVariableSize(variable, asSymbol->getLine())) { - error(asSymbol->getLine(), - "Size of declared variable exceeds implementation-defined limit", - asSymbol->getName()); return false; } + } + + return true; + } + + void visitFunctionPrototype(TIntermFunctionPrototype *node) override + { + const TFunction *function = node->getFunction(); + const size_t paramCount = function->getParamCount(); + + for (size_t paramIndex = 0; paramIndex < paramCount; ++paramIndex) + { + validateVariableSize(*function->getParam(paramIndex), node->getLine()); + } + } + + bool validateVariableSize(const TVariable &variable, const TSourceLoc &location) + { + const TType &variableType = variable.getType(); + + // Create a ShaderVariable from which to compute + // (conservative) sizing information. + ShaderVariable shaderVar; + setCommonVariableProperties(variableType, variable, &shaderVar); + + // Compute the std140 layout of this variable, assuming + // it's a member of a block (which it might not be). + Std140BlockEncoder layoutEncoder; + BlockEncoderVisitor visitor("", "", &layoutEncoder); + // Since the size limit's arbitrary, it doesn't matter + // whether the row-major layout is correctly determined. + bool isRowMajorLayout = false; + TraverseShaderVariable(shaderVar, isRowMajorLayout, &visitor); + if (layoutEncoder.getCurrentOffset() > kMaxVariableSizeInBytes) + { + error(location, "Size of declared variable exceeds implementation-defined limit", + variable.name()); + return false; + } + + // Skip over struct declarations. As long as they are not used (or if they are used later + // in a less-restricted context (such as a UBO or SSBO)), they can be larger than + // kMaxPrivateVariableSizeInBytes. + if (variable.symbolType() == SymbolType::Empty && variableType.isStructSpecifier()) + { + return true; + } + + switch (variableType.getQualifier()) + { + // List of all types that need to be limited (for example because they cause overflows + // in drivers, or create trouble for the SPIR-V gen as the number of an instruction's + // arguments cannot be more than 64KB (see OutputSPIRVTraverser::cast)). + + // Local/global variables + case EvqTemporary: + case EvqGlobal: + case EvqConst: + + // Function arguments + case EvqParamIn: + case EvqParamOut: + case EvqParamInOut: + case EvqParamConst: + + // Varyings + case EvqVaryingIn: + case EvqVaryingOut: + case EvqSmoothOut: + case EvqFlatOut: + case EvqNoPerspectiveOut: + case EvqCentroidOut: + case EvqSampleOut: + case EvqSmoothIn: + case EvqFlatIn: + case EvqNoPerspectiveIn: + case EvqCentroidIn: + case EvqVertexOut: + case EvqFragmentIn: + case EvqGeometryIn: + case EvqGeometryOut: + case EvqPerVertexIn: + case EvqPerVertexOut: + case EvqPatchIn: + case EvqPatchOut: + case EvqTessControlIn: + case EvqTessControlOut: + case EvqTessEvaluationIn: + case EvqTessEvaluationOut: - const bool isPrivate = variableType.getQualifier() == EvqTemporary || - variableType.getQualifier() == EvqGlobal || - variableType.getQualifier() == EvqConst; - if (isPrivate) - { if (layoutEncoder.getCurrentOffset() > kMaxPrivateVariableSizeInBytes) { - error(asSymbol->getLine(), + error(location, "Size of declared private variable exceeds implementation-defined limit", - asSymbol->getName()); + variable.name()); return false; } mTotalPrivateVariablesSize += layoutEncoder.getCurrentOffset(); - } + break; + default: + break; } return true; @@ -113,7 +182,7 @@ class ValidateTypeSizeLimitationsTraverser : public TIntermTraverser void validateTotalPrivateVariableSize() { - if (mTotalPrivateVariablesSize > kMaxPrivateVariableSizeInBytes) + if (mTotalPrivateVariablesSize > kMaxTotalPrivateVariableSizeInBytes) { mDiagnostics->error( TSourceLoc{}, diff --git a/src/tests/angle_end2end_tests_expectations.txt b/src/tests/angle_end2end_tests_expectations.txt index 7e38e04..d10d6ee 100644 --- a/src/tests/angle_end2end_tests_expectations.txt +++ b/src/tests/angle_end2end_tests_expectations.txt @@ -58,6 +58,8 @@ 1229184 WIN NVIDIA VULKAN : SimpleStateChangeTest.RedefineFramebufferTexture/* = SKIP 6173 WIN INTEL OPENGL : GLSLTest_ES31.BoolInInterfaceBlocks/* = SKIP 6217 WIN INTEL OPENGL : GLSLTest_ES31.StorageBufferBoolVectorPassedToFunctions/* = SKIP +8441 WIN INTEL OPENGL : GLSLTest_ES3.LargeInterfaceBlockArray/* = SKIP +8441 WIN INTEL OPENGL : GLSLTest_ES3.LargeInterfaceBlockNestedArray/* = SKIP 6826 WIN NVIDIA OPENGL : GLSLTest.VectorAndMatrixScalarizationDoesNotAffectRendering/* = SKIP 6826 WIN NVIDIA GLES : GLSLTest.VectorAndMatrixScalarizationDoesNotAffectRendering/* = SKIP @@ -89,6 +91,10 @@ // Generates invalid errors when compiling the shader in the GL backend 6172 NVIDIA OpenGL : GLSLTest_ES31.TypesUsedInDifferentBlockStorages/* = SKIP 6172 NVIDIA GLES : GLSLTest_ES31.TypesUsedInDifferentBlockStorages/* = SKIP +8441 NVIDIA OPENGL : GLSLTest_ES3.LargeInterfaceBlockArray/* = SKIP +8441 NVIDIA OPENGL : GLSLTest_ES3.LargeInterfaceBlockNestedArray/* = SKIP +8441 NVIDIA GLES : GLSLTest_ES3.LargeInterfaceBlockArray/* = SKIP +8441 NVIDIA GLES : GLSLTest_ES3.LargeInterfaceBlockNestedArray/* = SKIP // Intel Vulkan @@ -140,6 +146,8 @@ 6570 MAC INTEL OPENGL : WebGLCompatibilityTest.RG32FTextures/* = SKIP 6570 MAC INTEL OPENGL : WebGLCompatibilityTest.RGB32FTextures/* = SKIP 6603 MAC OPENGL : ProgramBinaryES3Test.SaveAndLoadDetachedShaders/* = SKIP +8437 MAC OPENGL : GLSLTest_ES3.LargeInterfaceBlockArray/* = SKIP +8437 MAC OPENGL : GLSLTest_ES3.LargeInterfaceBlockNestedArray/* = SKIP 6643 MAC AMD OPENGL : TransformFeedbackTest.TransformFeedbackQueryPausedDrawThenResume/* = SKIP 6643 MAC AMD OPENGL : TransformFeedbackTest.TransformFeedbackPausedDrawThenResume/* = SKIP 6738 MAC AMD OPENGL : Texture3DTestES3.PixelUnpackStateTex* = SKIP diff --git a/src/tests/compiler_tests/RecordConstantPrecision_test.cpp b/src/tests/compiler_tests/RecordConstantPrecision_test.cpp index 4e7dcce..f13e70c 100644 --- a/src/tests/compiler_tests/RecordConstantPrecision_test.cpp +++ b/src/tests/compiler_tests/RecordConstantPrecision_test.cpp @@ -141,11 +141,11 @@ TEST_F(RecordConstantPrecisionTest, HigherPrecisionConstantInIndex) uniform mediump float u; void main() { - const highp int a = 33000; - mediump float b[34000]; + const highp int a = 330; + mediump float b[340]; gl_FragColor = vec4(b[a]); })"; compile(shaderString); ASSERT_FALSE(foundInCode("const highp int s")); - ASSERT_TRUE(foundInCode("b[33000]")); + ASSERT_TRUE(foundInCode("b[330]")); } diff --git a/src/tests/gl_tests/GLSLTest.cpp b/src/tests/gl_tests/GLSLTest.cpp index cfe5bb2..20f8e45 100644 --- a/src/tests/gl_tests/GLSLTest.cpp +++ b/src/tests/gl_tests/GLSLTest.cpp @@ -14673,6 +14673,139 @@ void main() glGetShaderiv(shader, GL_COMPILE_STATUS, &compileResult); EXPECT_NE(compileResult, 0); } + +// Test that passing large arrays to functions are compiled correctly. Regression test for the +// SPIR-V generator that made a copy of the array to pass to the function, by decomposing and +// reconstructing it (in the absence of OpCopyLogical), but the reconstruction instruction has a +// length higher than can fit in SPIR-V. +TEST_P(GLSLTest_ES3, LargeInterfaceBlockArrayPassedToFunction) +{ + constexpr char kFS[] = R"(#version 300 es +precision highp float; +uniform Large { float a[65536]; }; +float f(float b[65536]) +{ + b[0] = 1.0; + return b[0] + b[1]; +} +out vec4 color; +void main() { + color = vec4(f(a), 0.0, 0.0, 1.0); +})"; + + GLuint shader = CompileShader(GL_FRAGMENT_SHADER, kFS); + EXPECT_EQ(0u, shader); +} + +// Make sure the shader in LargeInterfaceBlockArrayPassedToFunction works if the large local is +// avoided. +TEST_P(GLSLTest_ES3, LargeInterfaceBlockArray) +{ + int maxUniformBlockSize = 0; + glGetIntegerv(GL_MAX_UNIFORM_BLOCK_SIZE, &maxUniformBlockSize); + ANGLE_SKIP_TEST_IF(maxUniformBlockSize < 16384 * 4); + + constexpr char kFS[] = R"(#version 300 es +precision highp float; +uniform Large { float a[16384]; }; +out vec4 color; +void main() { + color = vec4(a[0], 0.0, 0.0, 1.0); +})"; + + ANGLE_GL_PROGRAM(program, essl3_shaders::vs::Simple(), kFS); +} + +// Similar to LargeInterfaceBlockArrayPassedToFunction, but the array is nested in a struct. +TEST_P(GLSLTest_ES3, LargeInterfaceBlockNestedArrayPassedToFunction) +{ + constexpr char kFS[] = R"(#version 300 es +precision highp float; +struct S { float a[65536]; }; +uniform Large { S s; }; +float f(float b[65536]) +{ + b[0] = 1.0; + return b[0] + b[1]; +} +out vec4 color; +void main() { + color = vec4(f(s.a), 0.0, 0.0, 1.0); +})"; + + GLuint shader = CompileShader(GL_FRAGMENT_SHADER, kFS); + EXPECT_EQ(0u, shader); +} + +// Make sure the shader in LargeInterfaceBlockNestedArrayPassedToFunction works if the large local +// is avoided. +TEST_P(GLSLTest_ES3, LargeInterfaceBlockNestedArray) +{ + int maxUniformBlockSize = 0; + glGetIntegerv(GL_MAX_UNIFORM_BLOCK_SIZE, &maxUniformBlockSize); + ANGLE_SKIP_TEST_IF(maxUniformBlockSize < 16384 * 4); + + constexpr char kFS[] = R"(#version 300 es +precision highp float; +struct S { float a[16384]; }; +uniform Large { S s; }; +out vec4 color; +void main() { + color = vec4(s.a[0], 0.0, 0.0, 1.0); +})"; + + ANGLE_GL_PROGRAM(program, essl3_shaders::vs::Simple(), kFS); +} + +// Similar to LargeInterfaceBlockArrayPassedToFunction, but the large array is copied to a local +// variable instead. +TEST_P(GLSLTest_ES3, LargeInterfaceBlockArrayCopiedToLocal) +{ + constexpr char kFS[] = R"(#version 300 es +precision highp float; +uniform Large { float a[65536]; }; +out vec4 color; +void main() { + float b[65536] = a; + color = vec4(b[0], 0.0, 0.0, 1.0); +})"; + + GLuint shader = CompileShader(GL_FRAGMENT_SHADER, kFS); + EXPECT_EQ(0u, shader); +} + +// Similar to LargeInterfaceBlockArrayCopiedToLocal, but the array is nested in a struct +TEST_P(GLSLTest_ES3, LargeInterfaceBlockNestedArrayCopiedToLocal) +{ + constexpr char kFS[] = R"(#version 300 es +precision highp float; +struct S { float a[65536]; }; +uniform Large { S s; }; +out vec4 color; +void main() { + S s2 = s; + color = vec4(s2.a[0], 0.0, 0.0, 1.0); +})"; + + GLuint shader = CompileShader(GL_FRAGMENT_SHADER, kFS); + EXPECT_EQ(0u, shader); +} + +// Test that too large varyings are rejected. +TEST_P(GLSLTest_ES3, LargeArrayVarying) +{ + constexpr char kFS[] = R"(#version 300 es +precision highp float; +in float a[65536]; +out vec4 color; +void main() { + color = vec4(a[0], 0.0, 0.0, 1.0); +})"; + + GLuint shader = CompileShader(GL_FRAGMENT_SHADER, kFS); + EXPECT_EQ(0u, shader); +} + } // anonymous namespace ANGLE_INSTANTIATE_TEST_ES2_AND_ES3_AND(GLSLTest, WithGlslang(ES2_VULKAN())); -- Gitee