From 8800c6b068eb940a66c1849626d95d204500ca8d Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Mon, 12 Oct 2026 03:52:34 -0400 Subject: [PATCH] [Fix] (Shader): keep subroutine type declarations as prototypes so reserved type names are rejected --- MobileGL/MG_Test/Program/ProgramUtilTest.cpp | 27 +++++++++++++++++++ .../ShaderSourceProcessor.cpp | 8 ++++-- 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/MobileGL/MG_Test/Program/ProgramUtilTest.cpp b/MobileGL/MG_Test/Program/ProgramUtilTest.cpp index af038dc1..5a3e3704 100644 --- a/MobileGL/MG_Test/Program/ProgramUtilTest.cpp +++ b/MobileGL/MG_Test/Program/ProgramUtilTest.cpp @@ -4550,6 +4550,33 @@ subroutine(FuncType) void Func0(int coord) { } } +// The type declaration only sheds its keyword, so the type NAME still reaches glslang as a function +// prototype: a reserved word there has to fail the compile (KHR-GL40.CommonBugs.CommonBug_ReservedNames) +// instead of vanishing with a blanked declaration. +TEST_F(ProgramUtilTest, PreprocessSubroutineKeepsTheTypeNameVisibleToTheCompiler) { + using namespace MG_Util::ShaderTranspiler; + + const auto lowerAndCompile = [](const String& typeName) { + String source = "#version 400 core\n" + "out vec4 fragColor;\n" + "subroutine void " + typeName + "(void);\n" + "subroutine(" + typeName + ") void Impl(void) { fragColor = vec4(1.0); }\n" + "subroutine uniform " + typeName + " g_routine;\n" + "void main() { g_routine(); }\n"; + PreprocessShaderSource(ShaderStage::Fragment, source); + EXPECT_EQ(source.find("subroutine"), String::npos) << source; + EXPECT_NE(source.find("void " + typeName + "(void);"), String::npos) + << "the type declaration must survive as a prototype:\n" << source; + ShaderAttrib attrib{.shaderType = GL_FRAGMENT_SHADER, .sourceStr = source}; + return Bool(ShaderCompiler::CompileShader(attrib)); + }; + + EXPECT_TRUE(lowerAndCompile("Routine")); + for (const char* reserved : {"const", "vec4", "asm", "sampler2D"}) { + EXPECT_FALSE(lowerAndCompile(reserved)) << "a subroutine type named '" << reserved << "' must not compile"; + } +} + // The forwarding function is rebuilt from the subroutine TYPE declaration, so it has to carry the // parameter qualifiers and array shapes across (an parameter that arrives by value writes // nothing back) and has to return the forwarded value for a non-void subroutine. diff --git a/MobileGL/MG_Util/ShaderTranspiler/ShaderSourceProcessor.cpp b/MobileGL/MG_Util/ShaderTranspiler/ShaderSourceProcessor.cpp index ef72053a..d7ed95b4 100644 --- a/MobileGL/MG_Util/ShaderTranspiler/ShaderSourceProcessor.cpp +++ b/MobileGL/MG_Util/ShaderTranspiler/ShaderSourceProcessor.cpp @@ -1275,7 +1275,7 @@ namespace { // it is left to fail at compile time exactly as it does today rather than silently pinned to // one of the alternatives. // - // subroutine void FuncType(int coord); -> (blanked) + // subroutine void FuncType(int coord); -> void FuncType(int coord); // subroutine uniform FuncType g_func; -> void g_func(int mgl_sr_arg0); // subroutine(FuncType) void Func0(int c) { } -> void Func0(int c) { } // ...plus, appended at end of source, @@ -1440,7 +1440,11 @@ namespace { type.returnText = JoinTokenText(tokens, i + 1, paren - 1); if (type.returnText.empty()) return; if (!ParseSubroutineParameters(tokens, paren + 1, listEnd - 1, type.parameters)) return; - blanks.push_back({token.begin, tokens[listEnd].end}); + // Only the keyword goes. What is left is an ordinary prototype that is never defined + // or called - legal - and it keeps the type NAME in front of glslang, which is what + // still rejects a reserved word there (`subroutine void const();`) instead of the + // name vanishing with the declaration. + blanks.push_back({token.begin, token.end}); i = listEnd; }