From fdc0cd560f51109d49a9612e757e76545f2fa9ed Mon Sep 17 00:00:00 2001 From: Kelsey Gilbert Date: Wed, 26 Jun 2024 21:26:12 +0000 Subject: [PATCH] Bug 1888340 - [angle] Cherry-pick to "Add GLSL variable byte size limits to ShBuiltInResources." a=dmeehan Original Revision: https://phabricator.services.mozilla.com/D212854 Differential Revision: https://phabricator.services.mozilla.com/D214745 --- .../checkout/include/GLSLANG/ShaderLang.h | 4 ++ .../checkout/out/gen/angle/angle_commit.h | 6 +-- .../src/compiler/translator/Compiler.cpp | 2 +- .../src/compiler/translator/ShaderLang.cpp | 8 +++ .../ValidateTypeSizeLimitations.cpp | 52 ++++++++++++------- .../translator/ValidateTypeSizeLimitations.h | 3 +- gfx/angle/cherry_picks.txt | 20 +++++++ 7 files changed, 72 insertions(+), 23 deletions(-) diff --git a/gfx/angle/checkout/include/GLSLANG/ShaderLang.h b/gfx/angle/checkout/include/GLSLANG/ShaderLang.h index d55e7ebce364..472c9545feec 100644 --- a/gfx/angle/checkout/include/GLSLANG/ShaderLang.h +++ b/gfx/angle/checkout/include/GLSLANG/ShaderLang.h @@ -666,6 +666,10 @@ struct ShBuiltInResources int MaxPixelLocalStoragePlanes; int MaxColorAttachmentsWithActivePixelLocalStorage; int MaxCombinedDrawBuffersAndPixelLocalStoragePlanes; + + // Variable size limits for webgl-mode validation. + size_t MaxVariableSizeInBytes; + size_t MaxPrivateVariableSizeInBytes; }; // diff --git a/gfx/angle/checkout/out/gen/angle/angle_commit.h b/gfx/angle/checkout/out/gen/angle/angle_commit.h index 7001cc33097a..0988b577d0e3 100644 --- a/gfx/angle/checkout/out/gen/angle/angle_commit.h +++ b/gfx/angle/checkout/out/gen/angle/angle_commit.h @@ -1,5 +1,5 @@ -#define ANGLE_COMMIT_HASH "ddaf44ac75d5" +#define ANGLE_COMMIT_HASH "791816843657" #define ANGLE_COMMIT_HASH_SIZE 12 -#define ANGLE_COMMIT_DATE "2024-01-02 15:01:34 -0800" -#define ANGLE_COMMIT_POSITION 19736 +#define ANGLE_COMMIT_DATE "2024-06-06 10:35:11 -0700" +#define ANGLE_COMMIT_POSITION 19738 #define ANGLE_HAS_BINARY_LOADING diff --git a/gfx/angle/checkout/src/compiler/translator/Compiler.cpp b/gfx/angle/checkout/src/compiler/translator/Compiler.cpp index 891d1ddf61f1..f70e275787a2 100644 --- a/gfx/angle/checkout/src/compiler/translator/Compiler.cpp +++ b/gfx/angle/checkout/src/compiler/translator/Compiler.cpp @@ -721,7 +721,7 @@ bool TCompiler::checkAndSimplifyAST(TIntermBlock *root, return false; } - if (shouldLimitTypeSizes() && !ValidateTypeSizeLimitations(root, &mSymbolTable, &mDiagnostics)) + if (shouldLimitTypeSizes() && !ValidateTypeSizeLimitations(mResources, root, &mSymbolTable, &mDiagnostics)) { return false; } diff --git a/gfx/angle/checkout/src/compiler/translator/ShaderLang.cpp b/gfx/angle/checkout/src/compiler/translator/ShaderLang.cpp index abc680656a5c..73bf948821dd 100644 --- a/gfx/angle/checkout/src/compiler/translator/ShaderLang.cpp +++ b/gfx/angle/checkout/src/compiler/translator/ShaderLang.cpp @@ -342,6 +342,14 @@ void InitBuiltInResources(ShBuiltInResources *resources) resources->SubPixelBits = 8; resources->MaxSamples = 4; + + // 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 + // the GPU registers anyway. + resources->MaxVariableSizeInBytes = static_cast(2) * 1024 * 1024 * 1024; + resources->MaxPrivateVariableSizeInBytes = static_cast(1) * 1024 * 1024; } // diff --git a/gfx/angle/checkout/src/compiler/translator/ValidateTypeSizeLimitations.cpp b/gfx/angle/checkout/src/compiler/translator/ValidateTypeSizeLimitations.cpp index 6097b6d236b5..5ad497a9429d 100644 --- a/gfx/angle/checkout/src/compiler/translator/ValidateTypeSizeLimitations.cpp +++ b/gfx/angle/checkout/src/compiler/translator/ValidateTypeSizeLimitations.cpp @@ -20,22 +20,17 @@ namespace sh 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 -// the GPU registers anyway. -constexpr size_t kMaxVariableSizeInBytes = static_cast(2) * 1024 * 1024 * 1024; -constexpr size_t kMaxPrivateVariableSizeInBytes = static_cast(1) * 1024 * 1024; - // Traverses intermediate tree to ensure that the shader does not // exceed certain implementation-defined limits on the sizes of types. // Some code was copied from the CollectVariables pass. class ValidateTypeSizeLimitationsTraverser : public TIntermTraverser { public: - ValidateTypeSizeLimitationsTraverser(TSymbolTable *symbolTable, TDiagnostics *diagnostics) - : TIntermTraverser(true, false, false, symbolTable), mDiagnostics(diagnostics) + ValidateTypeSizeLimitationsTraverser(const ShBuiltInResources& limits, TSymbolTable *symbolTable, TDiagnostics *diagnostics) + : TIntermTraverser(true, false, false, symbolTable), + mLimits(limits), + mDiagnostics(diagnostics), + mTotalPrivateVariablesSize(0) { ASSERT(diagnostics); } @@ -82,7 +77,8 @@ class ValidateTypeSizeLimitationsTraverser : public TIntermTraverser // whether the row-major layout is correctly determined. bool isRowMajorLayout = false; TraverseShaderVariable(shaderVar, isRowMajorLayout, &visitor); - if (layoutEncoder.getCurrentOffset() > kMaxVariableSizeInBytes) + if (mLimits.MaxVariableSizeInBytes && + layoutEncoder.getCurrentOffset() > mLimits.MaxVariableSizeInBytes) { error(asSymbol->getLine(), "Size of declared variable exceeds implementation-defined limit", @@ -93,18 +89,33 @@ class ValidateTypeSizeLimitationsTraverser : public TIntermTraverser const bool isPrivate = variableType.getQualifier() == EvqTemporary || variableType.getQualifier() == EvqGlobal || variableType.getQualifier() == EvqConst; - if (layoutEncoder.getCurrentOffset() > kMaxPrivateVariableSizeInBytes && isPrivate) + if (isPrivate) { - error(asSymbol->getLine(), - "Size of declared private variable exceeds implementation-defined limit", - asSymbol->getName()); - return false; + if (mLimits.MaxPrivateVariableSizeInBytes && layoutEncoder.getCurrentOffset() > mLimits.MaxPrivateVariableSizeInBytes) + { + error(asSymbol->getLine(), + "Size of declared private variable exceeds implementation-defined limit", + asSymbol->getName()); + return false; + } + mTotalPrivateVariablesSize += layoutEncoder.getCurrentOffset(); } } return true; } + void validateTotalPrivateVariableSize() + { + if (mTotalPrivateVariablesSize > mLimits.MaxPrivateVariableSizeInBytes) + { + mDiagnostics->error( + TSourceLoc{}, + "Total size of declared private variables exceeds implementation-defined limit", + ""); + } + } + private: void error(TSourceLoc loc, const char *reason, const ImmutableString &token) { @@ -211,18 +222,23 @@ class ValidateTypeSizeLimitationsTraverser : public TIntermTraverser } } + const ShBuiltInResources& mLimits; TDiagnostics *mDiagnostics; std::vector mLoopSymbolIds; + + size_t mTotalPrivateVariablesSize; }; } // namespace -bool ValidateTypeSizeLimitations(TIntermNode *root, +bool ValidateTypeSizeLimitations(const ShBuiltInResources& limits, + TIntermNode *root, TSymbolTable *symbolTable, TDiagnostics *diagnostics) { - ValidateTypeSizeLimitationsTraverser validate(symbolTable, diagnostics); + ValidateTypeSizeLimitationsTraverser validate(limits, symbolTable, diagnostics); root->traverse(&validate); + validate.validateTotalPrivateVariableSize(); return diagnostics->numErrors() == 0; } diff --git a/gfx/angle/checkout/src/compiler/translator/ValidateTypeSizeLimitations.h b/gfx/angle/checkout/src/compiler/translator/ValidateTypeSizeLimitations.h index defa39876db0..41925186aa2d 100644 --- a/gfx/angle/checkout/src/compiler/translator/ValidateTypeSizeLimitations.h +++ b/gfx/angle/checkout/src/compiler/translator/ValidateTypeSizeLimitations.h @@ -16,7 +16,8 @@ class TDiagnostics; // Returns true if the given shader does not violate certain // implementation-defined limits on the size of variables' types. -bool ValidateTypeSizeLimitations(TIntermNode *root, +bool ValidateTypeSizeLimitations(const ShBuiltInResources&, + TIntermNode *root, TSymbolTable *symbolTable, TDiagnostics *diagnostics); diff --git a/gfx/angle/cherry_picks.txt b/gfx/angle/cherry_picks.txt index 7669804f324f..59a3f544384d 100644 --- a/gfx/angle/cherry_picks.txt +++ b/gfx/angle/cherry_picks.txt @@ -1,3 +1,23 @@ +commit 7918168436578718b234bfd56da152e34a85af1d +Author: Kelsey Gilbert +Date: Tue Jun 4 15:37:29 2024 -0700 + + Add GLSL variable byte size limits to ShBuiltInResources. + +commit 31c0a5bff1330706aff3c4594c8166745814a45b +Author: Shahbaz Youssefi +Date: Wed May 3 13:41:36 2023 -0400 + + WebGL: Limit total size of private data + + ... not just individual arrays. + + Bug: chromium:1431761 + Change-Id: I721e29aeceeaf12c3f6a67b668abffb8dfbc89b0 + Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/4503753 + Reviewed-by: Kenneth Russell + Commit-Queue: Shahbaz Youssefi + commit ddaf44ac75d5d0390873c2af193e02159ecbe672 Author: Geoff Lang Date: Fri Dec 8 13:20:36 2023 -0500