[Fix, Test] (GLState, ShaderTranspiler): reject a storage-block binding at or past GL_MAX_SHADER_STORAGE_BUFFER_BINDINGS

MobileGL/MG_State/GLState/ProgramState/ShaderCompileTask.cpp

@@ -8,6 +8,7 @@
 
 #include "ShaderCompileTask.h"
 
+#include <MG_State/GLState/BufferState/BufferState.h>
 #include <MG_Util/Converters/MGToGL/ProgramEnumConverter.h>
 #include <MG_Util/ShaderTranspiler/ShaderCompiler.h>
 #include <MG_Util/ShaderTranspiler/ShaderSourceProcessor.h>
@@ -15,6 +16,7 @@
 
 #include <glslang/Include/PoolAlloc.h>
 
+#include <algorithm>
 #include <charconv>
 
 namespace {
@@ -137,8 +139,21 @@ namespace {
         return std::nullopt;
     }
 
+    // What glGetIntegerv(GL_MAX_SHADER_STORAGE_BUFFER_BINDINGS) answers, recomputed rather than
+    // queried: the compile runs on a worker with no context, and the pname is not a plain backend
+    // parameter - the getter caps the backend's count by the state layer's fixed binding-point
+    // array (GL_Getter's GetIndexedBufferQueryPointCount). A shader must be judged against the
+    // number the application was told, not against either half of it.
+    static MobileGL::Int MaxShaderStorageBufferBindings(
+        const MobileGL::MG_Util::ShaderTranspiler::CompileEnv& env) {
+        const MobileGL::Int frontendPoints =
+            static_cast<MobileGL::Int>(MobileGL::MG_State::GLState::BufferBindingPointCount);
+        if (!env.HasBackend()) return frontendPoints;
+        return std::min(frontendPoints, std::max(env.params.MaxShaderStorageBufferBindings, 0));
+    }
+
     // The half of a compile that depends on nothing but the source text, the stage and the
-    // environment snapshot: preprocessing, the two lexical rejections, and the two lexical
+    // environment snapshot: preprocessing, the three lexical rejections, and the two lexical
     // side-channel extractions. Split out so P0b layer 2 can memoize exactly this and
     // nothing else - the glslang parse stays per-object because its TShader is consume-once.
     // Deliberately free of any per-object state so the memo is sound.
@@ -172,6 +187,13 @@ namespace {
             return result;
         }
 
+        if (const std::optional<String> bindingError = FindShaderStorageBindingViolation(
+                result.preprocessedSource, MaxShaderStorageBufferBindings(env))) {
+            result.outcome = ShaderPreprocessOutcome::ResourceBindingRejected;
+            result.infoLog = *bindingError;
+            return result;
+        }
+
         // The parse this feeds runs in the link-compatible configuration (Vulkan-client
         // env with relaxed rules): the TShader it produces is what glLinkProgram links and
         // what the backends' SPIR-V is generated from - there is no second, GL-client

MobileGL/MG_State/GLState/ProgramState/ShaderPreprocessCache.h

@@ -26,6 +26,9 @@ namespace MobileGL::MG_State::GLState {
         ComputeLocalSizeRejected,
         // FindReservedIdentifierViolation rejected it.
         ReservedIdentifierRejected,
+        // FindShaderStorageBindingViolation rejected it: a storage block declared a binding at or
+        // past GL_MAX_SHADER_STORAGE_BUFFER_BINDINGS.
+        ResourceBindingRejected,
         // The source-only half was clean but glslang rejected the preprocessed source.
         // Memoizing this saves the parse itself on every later object with that source.
         ParseFailed,

MobileGL/MG_Test/Program/ProgramUtilTest.cpp

@@ -3840,3 +3840,41 @@ TEST_F(ProgramUtilTest, EsslCoreImageFormatSetIsTheThirteenTheSpecLists) {
     EXPECT_FALSE(ShaderCompiler::GLInternalFormatIsCoreEsslImageFormat(0x8051 /*GL_RGB8*/));
     EXPECT_FALSE(ShaderCompiler::GLInternalFormatIsCoreEsslImageFormat(0 /*GL_NONE*/));
 }
+
+// KHR-GL43.shader_storage_buffer_object.negative-glsl-compileTime: a storage block declared at
+// GL_MAX_SHADER_STORAGE_BUFFER_BINDINGS must fail to compile, and so must an arrayed one whose
+// LAST element passes the ceiling. The relaxed Vulkan-rules parse enforces neither.
+TEST_F(ProgramUtilTest, StorageBlockBindingCeilingIsCheckedAtItsExactBoundary) {
+    using namespace MG_Util::ShaderTranspiler;
+
+    constexpr Int kMaxBindings = 36;
+    const auto violation = [](const String& body) {
+        return FindShaderStorageBindingViolation("#version 430 core\n" + body + "void main() {}\n", kMaxBindings);
+    };
+
+    // The boundary itself: max - 1 is the last legal point, max is one past it.
+    EXPECT_FALSE(violation("layout(binding = 35) buffer Buffer { int x; };\n").has_value());
+    EXPECT_TRUE(violation("layout(binding = 36) buffer Buffer { int x; };\n").has_value());
+
+    // An instance array takes CONSECUTIVE points, so what has to fit is base + count - 1.
+    EXPECT_FALSE(violation("layout(binding = 32) buffer Buffer { int x; } g_array[4];\n").has_value());
+    EXPECT_TRUE(violation("layout(binding = 34) buffer Buffer { int x; } g_array[4];\n").has_value());
+
+    // Qualifiers and a second layout list may sit between the binding and the keyword.
+    EXPECT_TRUE(violation("layout(std430) layout(binding = 36) coherent restrict buffer B { int x; };\n")
+                    .has_value());
+
+    // Things the scanner must NOT judge: a uniform block (a different ceiling), a storage block
+    // with no explicit binding, the bare default-qualifier form, and an instance array whose size
+    // is not a literal.
+    EXPECT_FALSE(violation("layout(binding = 40) uniform Block { int x; };\n"
+                           "layout(binding = 0) buffer Buffer { int y; };\n")
+                     .has_value());
+    EXPECT_FALSE(violation("buffer Buffer { int x; };\nconst int binding = 40;\n").has_value());
+    EXPECT_FALSE(violation("layout(binding = 1) buffer;\nbuffer Buffer { int x; };\n").has_value());
+    EXPECT_FALSE(violation("const int kCount = 4;\nlayout(binding = 34) buffer B { int x; } g[kCount];\n")
+                     .has_value());
+
+    // A backend that advertises no binding points has no ceiling to enforce.
+    EXPECT_FALSE(FindShaderStorageBindingViolation("layout(binding = 36) buffer B { int x; };\n", 0).has_value());
+}

MobileGL/MG_Util/ShaderTranspiler/ShaderSourceProcessor.cpp

@@ -1332,6 +1332,97 @@ namespace MobileGL {
                 return bindings;
             }
 
+            namespace {
+                // Binding points a storage-block declaration starting at `bufferPos` occupies.
+                // One for a scalar instance (and for the "layout(...) buffer;" default-qualifier
+                // form, which declares no block at all); the element count for an instance array,
+                // whose elements take base, base+1, ... (GLSL 4.30 4.4.5). -1 means "the grammar
+                // here is outside this scanner's narrow subset", i.e. do not judge this one.
+                long long StorageBlockBindingPointCount(const Vector<CodeToken>& tokens, SizeT bufferPos,
+                                                        SizeT count) {
+                    SizeT k = bufferPos + 1;
+                    if (k < count && IsIdentifierToken(tokens[k])) ++k; // block type name
+                    if (k >= count || tokens[k].text != "{") return 1;
+
+                    MobileGL::Int braceDepth = 0;
+                    while (k < count) {
+                        if (tokens[k].text == "{") {
+                            ++braceDepth;
+                        } else if (tokens[k].text == "}") {
+                            --braceDepth;
+                            if (braceDepth == 0) {
+                                ++k;
+                                break;
+                            }
+                        }
+                        ++k;
+                    }
+                    if (braceDepth != 0) return -1; // unterminated block: not this scanner's business
+
+                    if (k < count && IsIdentifierToken(tokens[k])) ++k; // instance name
+                    if (k >= count || tokens[k].text != "[") return 1;
+                    if (k + 2 < count && IsDecimalIntegerToken(tokens[k + 1].text) && tokens[k + 2].text == "]") {
+                        return std::max<long long>(1, std::strtoll(tokens[k + 1].text.c_str(), nullptr, 10));
+                    }
+                    return -1; // sized by an expression, or unsized
+                }
+            } // namespace
+
+            std::optional<String> FindShaderStorageBindingViolation(const String& source, Int maxBindings) {
+                // A backend that advertises nothing has no ceiling to enforce.
+                if (maxBindings <= 0) return std::nullopt;
+                // Fast path: no storage block, nothing to check. Both keywords are required for a
+                // violation to exist, and the pair is absent from almost every shader-pack source.
+                if (source.find("buffer") == String::npos || source.find("binding") == String::npos) {
+                    return std::nullopt;
+                }
+
+                const Vector<CodeToken> tokens = TokenizeCode(source);
+                const SizeT count = tokens.size();
+                // The binding the qualifier run currently being scanned declared, -1 for none.
+                // Several layout(...) lists may precede one declaration and the later one wins,
+                // which is the same accumulate-then-consume shape the extractors above use.
+                long long binding = -1;
+                for (SizeT pos = 0; pos < count; ++pos) {
+                    const String& text = tokens[pos].text;
+                    if (text == "layout" && pos + 1 < count && tokens[pos + 1].text == "(") {
+                        SizeT j = pos + 2;
+                        Int parenDepth = 1;
+                        while (j < count && parenDepth > 0) {
+                            const String& layoutToken = tokens[j].text;
+                            if (layoutToken == "(") {
+                                ++parenDepth;
+                            } else if (layoutToken == ")") {
+                                --parenDepth;
+                            } else if (parenDepth == 1 && layoutToken == "binding" && j + 2 < count &&
+                                       tokens[j + 1].text == "=" && IsDecimalIntegerToken(tokens[j + 2].text)) {
+                                binding = std::min(std::strtoll(tokens[j + 2].text.c_str(), nullptr, 10),
+                                                   static_cast<long long>(INT_MAX / 2));
+                                j += 2;
+                            }
+                            ++j;
+                        }
+                        pos = j - 1;
+                        continue;
+                    }
+                    if (text == "buffer") {
+                        const long long points = binding >= 0 ? StorageBlockBindingPointCount(tokens, pos, count) : -1;
+                        if (points > 0 && binding + points > static_cast<long long>(maxBindings)) {
+                            return "ERROR: invalid value " + std::to_string(binding) +
+                                   " for layout specifier 'binding': a shader storage block occupying " +
+                                   std::to_string(points) + " binding point(s) from there passes " +
+                                   "GL_MAX_SHADER_STORAGE_BUFFER_BINDINGS (" + std::to_string(maxBindings) + ").";
+                        }
+                        binding = -1;
+                        continue;
+                    }
+                    // Qualifiers may sit between the layout list and the `buffer` keyword; anything
+                    // else ends the run, so a binding never leaks onto an unrelated declaration.
+                    if (!IsNonLayoutQualifierKeyword(text)) binding = -1;
+                }
+                return std::nullopt;
+            }
+
             UnorderedMap<String, Int> ExtractExplicitUniformLocations(const String& source) {
                 UnorderedMap<String, Int> locations;
                 // Fast path: without the qualifier keyword there is nothing to extract.

MobileGL/MG_Util/ShaderTranspiler/ShaderSourceProcessor.h

@@ -64,6 +64,17 @@ namespace MobileGL {
             // mapIO can capture them, so they are recovered lexically (same narrow
             // grammar discipline as ExtractExplicitUniformLocations).
             UnorderedMap<String, Uint> ExtractExplicitOpaqueBindings(const String& source);
+
+            // A shader storage block whose layout(binding = N) reaches or passes
+            // GL_MAX_SHADER_STORAGE_BUFFER_BINDINGS is a compile-time error in GL 4.3 core 4.4.5,
+            // and an arrayed block instance takes CONSECUTIVE points, so the last element is what
+            // has to fit. glslang cannot raise it for MobileGL: every shader is parsed as a Vulkan
+            // client under relaxed rules, where the GL ceilings do not apply, and TBuiltInResource
+            // has no storage-buffer binding field to check against in the first place. Returns the
+            // compile-error text for the first violation, or nullopt for a clean source.
+            // `maxBindings` is what glGetIntegerv answers for that pname; a non-positive value
+            // means "nothing to check against" and every declaration passes.
+            std::optional<String> FindShaderStorageBindingViolation(const String& source, Int maxBindings);
         } // namespace ShaderTranspiler
     } // namespace MG_Util
 } // namespace MobileGL