[Fix] (ShaderTranspiler): enforce the layout(binding) range rule for samplers, images and uniform/atomic-counter blocks, not only for SSBOs

MobileGL/MG_Util/ShaderTranspiler/ShaderCompiler.cpp

@@ -548,12 +548,28 @@ namespace MobileGL {
                                                        attrib.explicitFragmentOutIndices,
                                                        attrib.explicitOpaqueUniformBindings,
                                                        attrib.storageBlocksWithoutBinding,
-                                                       attrib.uniformBlocksWithoutBinding);
+                                                       attrib.uniformBlocksWithoutBinding,
+                                                       &attrib.resourceBindingLimits,
+                                                       attrib.resourceBindingViolation);
                     break;
                 }
                 auto ioMapper = UniquePtr<glslang::TIoMapper>(glslang::GetGlslIoMapper());
 
-                if (!program->mapIO(resolver.get(), ioMapper.get())) {
+                const bool mapped = program->mapIO(resolver.get(), ioMapper.get());
+
+                // The binding-range verdict is read BEFORE mapIO's own outcome, and unconditionally:
+                // the resolver fills it during the collect phase, which runs whether or not doMap()
+                // later succeeds, and a shader that names an out-of-range binding is rejected for
+                // THAT reason no matter what else the mapper made of it. Reporting the mapper's
+                // generic failure instead would hand the application an info log that says nothing
+                // about the declaration it has to fix.
+                if (attrib.resourceBindingViolation != nullptr && !attrib.resourceBindingViolation->empty()) {
+                    ResultInfo r;
+                    r.log = *attrib.resourceBindingViolation;
+                    r.errc = -5;
+                    return std::unexpected(r);
+                }
+                if (!mapped) {
                     ResultInfo r;
                     r.log = "Error: [glslang] Cannot mapIO:\n" + std::string(program->getInfoLog());
                     r.errc = -4;

MobileGL/MG_Util/ShaderTranspiler/Types.h

@@ -145,6 +145,33 @@ namespace MobileGL {
                 const CompileEnv* env = nullptr;
             };
 
+            // The per-device ceilings a shader-declared `layout(binding = N)` is measured
+            // against - one per resource kind, because GL gives each kind its own limit and they
+            // differ by an order of magnitude on real hardware (a Mali-G925 reports 96 combined
+            // texture image units and 21 image units).
+            //
+            // These exist because glslang cannot enforce them for MobileGL. It owns ceilings for
+            // samplers/images and for atomic counters, and both are switched OFF by the parse
+            // configuration MobileGL uses everywhere - `spvVersion.vulkan == 0` gates the first
+            // and `!spvVersion.vulkanRelaxed` the second (ParseHelper.cpp layoutTypeCheck), and
+            // MobileGL always parses with setEnvClient(EShClientVulkan) +
+            // setEnvInputVulkanRulesRelaxed(). For uniform and storage BLOCKS glslang quotes the
+            // spec sentence and then checks nothing at all. Flipping to the OpenGL client to wake
+            // those checks is not an option (it would change the parse the whole relaxed
+            // lowering pipeline is built on) and would not even be correct: glslang measures
+            // IMAGE bindings against the SAMPLER limit and hardcodes that limit at 80, so it
+            // would reject legal bindings 80..95 and keep under-rejecting images.
+            //
+            // Zero or negative means "no ceiling to enforce for this kind" - a backendless
+            // environment, which every unit test and the pre-init preload path run in.
+            struct ResourceBindingLimits {
+                Int MaxSamplerBindings = 0;              // GL_MAX_COMBINED_TEXTURE_IMAGE_UNITS
+                Int MaxImageBindings = 0;                // GL_MAX_IMAGE_UNITS
+                Int MaxUniformBufferBindings = 0;        // GL_MAX_UNIFORM_BUFFER_BINDINGS
+                Int MaxShaderStorageBufferBindings = 0;  // GL_MAX_SHADER_STORAGE_BUFFER_BINDINGS
+                Int MaxAtomicCounterBufferBindings = 0;  // GL_MAX_ATOMIC_COUNTER_BUFFER_BINDINGS
+            };
+
             struct ProgramAttrib {
                 Vector<SharedPtr<glslang::TShader>> shaders;
                 UnorderedMap<String, Uint> explicitVertexInLocations;
@@ -160,6 +187,11 @@ namespace MobileGL {
                 UnorderedMap<String, Uint>* explicitOpaqueUniformBindings = nullptr;
                 std::set<String>* storageBlocksWithoutBinding = nullptr;
                 std::set<String>* uniformBlocksWithoutBinding = nullptr;
+                // IN: the ceilings above. OUT: the first violation the resolver found, in the
+                // same capture window and for the same reason - past mapIO's doMap() every
+                // resource carries an ASSIGNED binding and the question can no longer be asked.
+                ResourceBindingLimits resourceBindingLimits{};
+                String* resourceBindingViolation = nullptr;
             };
 
             struct ProgramBinaryAttrib {

MobileGL/MG_Util/ShaderTranspiler/glslang/TMglGlslIoResolver.cpp

@@ -13,6 +13,8 @@
 #include "TMglGlslIoResolver.h"
 
 #include <cstring>
+#include <cstdlib>
+#include <string>
 
 #include <MG_Util/ShaderTranspiler/Types.h>
 
@@ -165,6 +167,88 @@ namespace MobileGL {
     // before the preprocessor's macros were expanded and therefore could not read
     // `binding = SOME_MACRO` - the spelling Flywheel's indirect engine uses for every one of
     // its storage blocks. Asking the AST instead makes the macro case ordinary.
+    // GLSL 4.30 4.4.5 and ES 3.1 4.4.4: `layout(binding = N)` on any opaque uniform, uniform
+    // block, storage block or atomic counter is a COMPILE-TIME error when N is not less than that
+    // resource kind's implementation limit - and, for an ARRAY of them, when base + count - 1 is
+    // not. MobileGL enforces it here rather than at compile because here is the last point where
+    // `qualifier.hasBinding()` still means "the SHADER said so" (see the comment on the caller),
+    // and because the per-device ceilings are deliberately not part of the compile pipeline's
+    // memo keys. The conformance suite accepts a link-time rejection: its predicate is
+    // compiledAndLinked(), which is the AND of the two.
+    //
+    // ONE enforcement point for all five kinds, on purpose. Before this, exactly one kind -
+    // shader-storage blocks - was checked, by a bespoke lexical scan of the shader source, which
+    // is why the storage sub-family was the one that passed while sampler, image, uniform-block
+    // and atomic-counter bindings sailed past every ceiling. That scanner is retired; a second
+    // enforcement point is a second thing to drift.
+    void TMglGlslIoResolver::CheckDeclaredBindingRange(const glslang::TType& type, const glslang::TString& name) {
+        if (m_bindingLimits == nullptr || m_bindingViolation == nullptr) return;
+        if (!m_bindingViolation->empty()) return;   // first violation wins; the link is already lost
+
+        const glslang::TQualifier& qualifier = type.getQualifier();
+        const char* kind = nullptr;
+        const char* limitName = nullptr;
+        Int limit = 0;
+        long long binding = -1;
+
+        if (type.getBasicType() == glslang::EbtSampler && qualifier.hasBinding()) {
+            const bool isImage = type.getSampler().isImage();
+            kind = isImage ? "image" : "sampler";
+            limitName = isImage ? "GL_MAX_IMAGE_UNITS" : "GL_MAX_COMBINED_TEXTURE_IMAGE_UNITS";
+            limit = isImage ? m_bindingLimits->MaxImageBindings : m_bindingLimits->MaxSamplerBindings;
+            binding = qualifier.layoutBinding;
+        } else if (type.getBasicType() == glslang::EbtBlock) {
+            // An atomic counter never reaches here as a counter: the relaxed parse has already
+            // folded it into a synthesized "gl_AtomicCounterBlock_<binding>" storage block whose
+            // TRAILING NUMBER is the GL binding the shader asked for (ParseContextBase::
+            // growAtomicCounterBlock names it from bufferBinding). That name is the only surviving
+            // record of the declaration, so it is what the counter ceiling is read off.
+            const Int counterBinding = MG_Util::ShaderTranspiler::AtomicCounterBlockGlBinding(
+                StringView(name.c_str(), name.size()));
+            if (counterBinding >= 0) {
+                kind = "atomic_uint";
+                limitName = "GL_MAX_ATOMIC_COUNTER_BUFFER_BINDINGS";
+                limit = m_bindingLimits->MaxAtomicCounterBufferBindings;
+                binding = counterBinding;
+            } else if (qualifier.hasBinding() && qualifier.storage == glslang::EvqUniform &&
+                       name.compare(MG_Util::ShaderTranspiler::GLOBAL_UBO_NAME) != 0) {
+                kind = "uniform block";
+                limitName = "GL_MAX_UNIFORM_BUFFER_BINDINGS";
+                limit = m_bindingLimits->MaxUniformBufferBindings;
+                binding = qualifier.layoutBinding;
+            } else if (qualifier.hasBinding() && qualifier.storage == glslang::EvqBuffer) {
+                kind = "buffer block";
+                limitName = "GL_MAX_SHADER_STORAGE_BUFFER_BINDINGS";
+                limit = m_bindingLimits->MaxShaderStorageBufferBindings;
+                binding = qualifier.layoutBinding;
+            }
+        }
+
+        if (kind == nullptr || limit <= 0 || binding < 0) return;
+
+        // The ARRAYED-INSTANCE rule: an array of N takes bindings base .. base + N - 1, and every
+        // one of them has to fit. getCumulativeArraySize() folds a multi-dimensional array into
+        // the count of leaf elements, which is exactly how many consecutive bindings GL hands out.
+        // An unsized or implicitly-sized array reports 0; treat it as one binding rather than
+        // guess, since it cannot be the shape the rule is about.
+        long long elementCount = 1;
+        if (type.isArray()) {
+            const int cumulative = static_cast<int>(type.getCumulativeArraySize());
+            if (cumulative > 1) elementCount = cumulative;
+        }
+        const long long lastBinding = binding + elementCount - 1;
+        if (lastBinding < static_cast<long long>(limit)) return;
+
+        String message = "Error: layout(binding = " + std::to_string(binding) + ") on " + kind + " '" +
+                         String(name.c_str()) + "'";
+        if (elementCount > 1) {
+            message += " (an array of " + std::to_string(elementCount) + ", occupying bindings " +
+                       std::to_string(binding) + ".." + std::to_string(lastBinding) + ")";
+        }
+        message += " is not less than " + String(limitName) + " (" + std::to_string(limit) + ").";
+        *m_bindingViolation = Move(message);
+    }
+
     void TMglGlslIoResolver::reserverResourceSlot(glslang::TVarEntryInfo& ent, TInfoSink& infoSink) {
         const glslang::TType& type = ent.symbol->getType();
         const glslang::TQualifier& qualifier = type.getQualifier();
@@ -207,6 +291,8 @@ namespace MobileGL {
             m_uniformBlocksWithoutBinding->insert(name.c_str());
         }
 
+        CheckDeclaredBindingRange(type, name);
+
         TDefaultGlslIoResolver::reserverResourceSlot(ent, infoSink);
     }
 

MobileGL/MG_Util/ShaderTranspiler/glslang/TMglGlslIoResolver.h

@@ -21,27 +21,35 @@
 #include <glslang/MachineIndependent/iomapper.h>
 #include "TVarEntryInfo.h"
 #include "MG_Util/Types.h"
+#include "MG_Util/ShaderTranspiler/Types.h"
 
 namespace MobileGL {
     class TMglGlslIoResolver : public glslang::TDefaultGlslIoResolver {
     public:
         using ExplicitVarSlotMap = UnorderedMap<String, Uint>;
+        using ResourceBindingLimits = MG_Util::ShaderTranspiler::ResourceBindingLimits;
         TMglGlslIoResolver(const glslang::TIntermediate& intermediate, const ExplicitVarSlotMap& vertexIns,
                            const ExplicitVarSlotMap& fragOuts, const ExplicitVarSlotMap& fragOutIndices,
                            ExplicitVarSlotMap* opaqueUniformBindings,
                            std::set<String>* storageBlocksWithoutBinding = nullptr,
-                           std::set<String>* uniformBlocksWithoutBinding = nullptr)
+                           std::set<String>* uniformBlocksWithoutBinding = nullptr,
+                           const ResourceBindingLimits* bindingLimits = nullptr,
+                           String* bindingViolation = nullptr)
             : TDefaultGlslIoResolver(intermediate), m_explicitVertexIns(vertexIns), m_explicitFragOuts(fragOuts),
               m_explicitFragOutIndices(fragOutIndices), m_explicitOpaqueUniformBindings(opaqueUniformBindings),
               m_storageBlocksWithoutBinding(storageBlocksWithoutBinding),
-              m_uniformBlocksWithoutBinding(uniformBlocksWithoutBinding) {}
+              m_uniformBlocksWithoutBinding(uniformBlocksWithoutBinding), m_bindingLimits(bindingLimits),
+              m_bindingViolation(bindingViolation) {}
         TMglGlslIoResolver(const glslang::TProgram& program, const EShLanguage stage,
                            const ExplicitVarSlotMap& vertexIns, const ExplicitVarSlotMap& fragOuts,
                            const ExplicitVarSlotMap& fragOutIndices, ExplicitVarSlotMap* opaqueUniformBindings,
                            std::set<String>* storageBlocksWithoutBinding = nullptr,
-                           std::set<String>* uniformBlocksWithoutBinding = nullptr)
+                           std::set<String>* uniformBlocksWithoutBinding = nullptr,
+                           const ResourceBindingLimits* bindingLimits = nullptr,
+                           String* bindingViolation = nullptr)
             : TMglGlslIoResolver(*program.getIntermediate(stage), vertexIns, fragOuts, fragOutIndices,
-                                 opaqueUniformBindings, storageBlocksWithoutBinding, uniformBlocksWithoutBinding) {}
+                                 opaqueUniformBindings, storageBlocksWithoutBinding, uniformBlocksWithoutBinding,
+                                 bindingLimits, bindingViolation) {}
         void reserverStorageSlot(glslang::TVarEntryInfo& ent, TInfoSink& infoSink) override;
         void reserverResourceSlot(glslang::TVarEntryInfo& ent, TInfoSink& infoSink) override;
         int resolveInOutLocation(EShLanguage stage, glslang::TVarEntryInfo& ent) override;
@@ -72,6 +80,10 @@ namespace MobileGL {
         // resource kind on set 0), so an unbound block declared after an unbound image lands on
         // 1. See ProgramLinkTask's UBO reflection loop for what is done with them.
         std::set<String>* m_uniformBlocksWithoutBinding = nullptr;
+        // The binding-range rule, IN and OUT. See RecordBindingRangeViolation.
+        const ResourceBindingLimits* m_bindingLimits = nullptr;
+        String* m_bindingViolation = nullptr;
+        void CheckDeclaredBindingRange(const glslang::TType& type, const glslang::TString& name);
         std::map<glslang::TString, int> m_plainUniformLocationSizeByName;
         std::map<glslang::TString, int> m_plainUniformLocationByName;
         bool m_plainUniformLocationsAssigned = false;