[Fix, Test] (ShaderTranspiler, ProgramState): reject an out-of-range atomic-counter offset at compile

MobileGL/MG_State/GLState/ProgramState/ShaderCompileTask.cpp

@@ -194,6 +194,13 @@ namespace {
             return result;
         }
 
+        if (const std::optional<String> counterOffsetError =
+                FindAtomicCounterOffsetViolation(result.preprocessedSource)) {
+            result.outcome = ShaderPreprocessOutcome::AtomicCounterOffsetRejected;
+            result.infoLog = *counterOffsetError;
+            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

@@ -29,6 +29,9 @@ namespace MobileGL::MG_State::GLState {
         // FindShaderStorageBindingViolation rejected it: a storage block declared a binding at or
         // past GL_MAX_SHADER_STORAGE_BUFFER_BINDINGS.
         ResourceBindingRejected,
+        // FindAtomicCounterOffsetViolation rejected it: an atomic counter declared a
+        // layout(offset =) that is misaligned or reaches past GL_MAX_ATOMIC_COUNTER_BUFFER_SIZE.
+        AtomicCounterOffsetRejected,
         // 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

@@ -4064,6 +4064,49 @@ TEST_F(ProgramUtilTest, StorageBlockBindingCeilingIsCheckedAtItsExactBoundary) {
     EXPECT_FALSE(FindShaderStorageBindingViolation("layout(binding = 36) buffer B { int x; };\n", 0).has_value());
 }
 
+// KHR-GL43.shader_atomic_counters.negative-offset-1: an atomic counter whose layout(offset = N)
+// puts its last byte past GL_MAX_ATOMIC_COUNTER_BUFFER_SIZE is a COMPILE-time error, and the CTS
+// never links the shader at all. MobileGL only had the rule at link, because the Vulkan-relaxed
+// parse never reaches glslang's fixOffset().
+TEST_F(ProgramUtilTest, AtomicCounterOffsetCeilingIsCheckedAtCompile) {
+    using namespace MG_Util::ShaderTranspiler;
+
+    const auto violation = [](const String& body) {
+        return FindAtomicCounterOffsetViolation("#version 430 core\n" + body + "void main() {}\n");
+    };
+    const String maxSize = std::to_string(MAX_ATOMIC_COUNTER_BUFFER_SIZE);
+    const String lastLegal = std::to_string(MAX_ATOMIC_COUNTER_BUFFER_SIZE - 4);
+
+    // The boundary itself: the last counter that still fits, and the first that does not.
+    EXPECT_FALSE(violation("layout(binding = 0, offset = " + lastLegal + ") uniform atomic_uint c;\n").has_value());
+    EXPECT_TRUE(violation("layout(binding = 0, offset = " + maxSize + ") uniform atomic_uint c;\n").has_value());
+
+    // An array occupies one word per element, so what has to fit is the LAST one.
+    EXPECT_FALSE(violation("layout(offset = " + std::to_string(MAX_ATOMIC_COUNTER_BUFFER_SIZE - 16) +
+                           ") uniform atomic_uint c[4];\n")
+                     .has_value());
+    EXPECT_TRUE(violation("layout(offset = " + std::to_string(MAX_ATOMIC_COUNTER_BUFFER_SIZE - 8) +
+                          ") uniform atomic_uint c[4];\n")
+                    .has_value());
+
+    // An offset that is not a multiple of 4 (GL 4.6 core 7.7), and one that is.
+    EXPECT_TRUE(violation("layout(offset = 2) uniform atomic_uint c;\n").has_value());
+    EXPECT_FALSE(violation("layout(offset = 8) uniform atomic_uint c;\n").has_value());
+
+    // Things the scanner must NOT judge: a counter with no explicit offset, an `offset` that is
+    // an ordinary identifier rather than a layout qualifier, an array sized by an expression,
+    // and an offset qualifier that belongs to a different declaration.
+    EXPECT_FALSE(violation("uniform atomic_uint c;\nconst int offset = 99999;\n").has_value());
+    EXPECT_FALSE(violation("const int kCount = 4;\nlayout(offset = " + maxSize +
+                           ") uniform atomic_uint c[kCount];\n")
+                     .has_value());
+    EXPECT_FALSE(violation("layout(offset = " + maxSize + ") uniform Block { int x; };\n"
+                           "uniform atomic_uint c;\n")
+                     .has_value());
+    // A source with no counter at all never pays for the scan and never reports one.
+    EXPECT_FALSE(FindAtomicCounterOffsetViolation("#version 430 core\nvoid main() {}\n").has_value());
+}
+
 // KHR-GL43.explicit_uniform_location.uniform-loc-nondecimal: GLSL integer literals are C-style, so
 // layout(location = 0xA) is 10 and layout(location = 010) is OCTAL 8. The extractor used to accept
 // a base-10 digit run and nothing else: the hex spelling failed the test entirely and the

MobileGL/MG_Util/ShaderTranspiler/ShaderSourceProcessor.cpp

@@ -18,6 +18,7 @@
 #include <Config.h>
 #include <MG_Backend/BackendObjects.h>
 #include <MG_Util/ShaderTranspiler/CompileEnv.h>
+#include <MG_Util/ShaderTranspiler/Types.h>
 
 #include "EsslBuiltinFunctionNames.h"
 
@@ -1603,6 +1604,92 @@ namespace MobileGL {
                 return std::nullopt;
             }
 
+            std::optional<String> FindAtomicCounterOffsetViolation(const String& source) {
+                // Fast path: both keywords are required for a violation to exist, and the pair is
+                // absent from every shader-pack source.
+                if (source.find("atomic_uint") == String::npos || source.find("offset") == String::npos) {
+                    return std::nullopt;
+                }
+
+                constexpr long long kAtomicCounterSize = 4; // one 32-bit word per counter
+                const long long maxBufferSize = static_cast<long long>(MAX_ATOMIC_COUNTER_BUFFER_SIZE);
+                const Vector<CodeToken> tokens = TokenizeCode(source);
+                const SizeT count = tokens.size();
+                // The offset the qualifier run currently being scanned declared, -1 for none.
+                // Same accumulate-then-consume shape as the storage-binding scan above.
+                long long offset = -1;
+                long long literal = 0;
+                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 == "offset" && j + 2 < count &&
+                                       tokens[j + 1].text == "=" &&
+                                       ParseGlslIntegerLiteral(tokens[j + 2].text, literal)) {
+                                offset = literal;
+                                j += 2;
+                            }
+                            ++j;
+                        }
+                        pos = j - 1;
+                        continue;
+                    }
+                    if (text == "atomic_uint") {
+                        // How far the declaration reaches: `atomic_uint c[N]` occupies N words
+                        // from the offset. An unparsable or absent declarator (an expression-sized
+                        // array, or the "layout(...) uniform atomic_uint;" default-qualifier form,
+                        // which declares no counter at all) is left alone rather than guessed at -
+                        // over-rejection here would be a compile failure the application cannot
+                        // work around.
+                        long long elements = 1;
+                        SizeT k = pos + 1;
+                        if (k < count && IsIdentifierToken(tokens[k])) {
+                            ++k;
+                            if (k < count && tokens[k].text == "[") {
+                                elements = (k + 2 < count && tokens[k + 2].text == "]" &&
+                                            ParseGlslIntegerLiteral(tokens[k + 1].text, literal))
+                                               ? std::max<long long>(1, literal)
+                                               : -1;
+                            }
+                        } else {
+                            elements = -1;
+                        }
+                        // Clamped so the byte arithmetic below cannot overflow on an absurd
+                        // literal; any element count at or past the ceiling already fails.
+                        elements = std::min(elements, maxBufferSize);
+
+                        if (offset >= 0 && elements > 0) {
+                            if (offset % kAtomicCounterSize != 0) {
+                                return "ERROR: invalid value " + std::to_string(offset) +
+                                       " for layout specifier 'offset': an atomic counter offset must be a "
+                                       "multiple of 4.";
+                            }
+                            if (offset > maxBufferSize - elements * kAtomicCounterSize) {
+                                return "ERROR: invalid value " + std::to_string(offset) +
+                                       " for layout specifier 'offset': an atomic counter ending at byte " +
+                                       std::to_string(offset + elements * kAtomicCounterSize) +
+                                       " passes GL_MAX_ATOMIC_COUNTER_BUFFER_SIZE (" +
+                                       std::to_string(maxBufferSize) + ").";
+                            }
+                        }
+                        offset = -1;
+                        continue;
+                    }
+                    // `uniform` and the precision/auxiliary qualifiers may sit between the layout
+                    // list and the type keyword; anything else ends the run, so an offset never
+                    // leaks onto an unrelated declaration.
+                    if (text != "uniform" && !IsNonLayoutQualifierKeyword(text)) offset = -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

@@ -75,6 +75,18 @@ namespace MobileGL {
             // `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);
+
+            // GL 4.6 core 7.7 / ARB_shader_atomic_counters makes it a COMPILE-time error to
+            // declare an atomic counter at an offset that is not a multiple of 4, or whose last
+            // byte passes GL_MAX_ATOMIC_COUNTER_BUFFER_SIZE. glslang enforces both in fixOffset(),
+            // which the Vulkan-relaxed parse never reaches (vkRelaxedRemapUniformVariable folds
+            // the atomic_uint into a synthesized storage block and returns from declareVariable()
+            // first), so MobileGL only caught them at LINK - and
+            // KHR-GL43.shader_atomic_counters.negative-offset-1 never links at all. The
+            // cross-stage rule (two counters sharing a binding must not overlap) stays at link:
+            // a single-stage source cannot see it. Returns the compile-error text for the first
+            // violation, or nullopt for a clean source.
+            std::optional<String> FindAtomicCounterOffsetViolation(const String& source);
         } // namespace ShaderTranspiler
     } // namespace MG_Util
 } // namespace MobileGL