swung0x48/MobileGL
1
0
Fork 0
mirror of https://github.com/MobileGL-Dev/MobileGL synced 2026-09-09 04:38:30 +09:00
Code Issues Packages Projects Releases Wiki Activity

[Fix] (MG_Util/ShaderTranspiler): reject malformed #version directives instead of legalizing them - an unrecognized version number (329/331), a bad profile keyword, a float or trailing token used to be rewritten to "#version 330 core" (or rescued to 460 by the retry); now InspectShaderLanguage marks such directives invalid so NormalizeVersionDirective and RetargetLegacyVersionDirectiveTo460 leave them for glslang to reject, while every valid version still normalizes as before

Browse Source
This commit is contained in:
swung0x48
2026-07-20 22:03:36 -04:00
parent 48ba622387
commit b6a7807a3a
2 changed files with 114 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files
MobileGL/MG_Test/Program/ProgramUtilTest.cpp
+66
View File
@@ -426,6 +426,62 @@ void main() {
verifyVersion("#version 460 core");
}
// KHR-GL33.shaders.preprocessor.directive.version_* (also re-run verbatim under GL40-GL44): the
// compiler must REJECT a malformed #version line. MobileGL used to rewrite the whole line to
// "#version 330 core" whenever it could scrape a leading integer - or treat an unknown profile token
// as core - which silently legalized every form below. CTS compiles the shader's own #version
// verbatim, so the rejection has to survive preprocessing (and the 460 retry).
TEST_F(ProgramUtilTest, PreprocessRejectsMalformedVersionDirectives) {
using namespace MG_Util::ShaderTranspiler;
const char* body = "\nout vec4 fragColor;\nvoid main() { fragColor = vec4(1.0); }\n";
const auto rejects = [](const String& fullSource) {
String src = fullSource;
PreprocessShaderSource(ShaderStage::Fragment, src);
ShaderAttrib attrib{.shaderType = GL_FRAGMENT_SHADER, .sourceStr = src};
auto res = ShaderCompiler::CompileShader(attrib);
return res ? false : true; // "rejects" == compile failed
};
// Silently legalized today - the five this fix must flip to rejection:
EXPECT_TRUE(rejects(String("#version 329") + body)) << "329 is not a real version";
EXPECT_TRUE(rejects(String("#version 331") + body)) << "331 is not a real version";
EXPECT_TRUE(rejects(String("#version 330 foo") + body)) << "unknown profile keyword";
EXPECT_TRUE(rejects(String("#version 330.0") + body)) << "float literal, not an int token";
EXPECT_TRUE(rejects(String("#version 330 foobar") + body)) << "trailing tokens after a valid decl";
// Already rejected (no leading integer, or #version is not the first token) - pinned so a future
// change to the normalizer cannot start legalizing them either:
EXPECT_TRUE(rejects(String("#version") + body)) << "missing version number";
EXPECT_TRUE(rejects(String("#version foobar") + body)) << "identifier where the int belongs";
EXPECT_TRUE(rejects(String("#version AAA") + body)) << "identifier where the int belongs";
EXPECT_TRUE(rejects(String("precision mediump float;\n#version 330") + body))
<< "#version must be the first statement";
EXPECT_TRUE(rejects(String("#define FOO BAR\n#version 330") + body))
<< "#version must precede a #define";
}
// The PASS half of the same CTS group: a valid decl, and #version preceded only by whitespace or a
// comment, must still compile. Guards the fix above from over-rejecting.
TEST_F(ProgramUtilTest, PreprocessKeepsValidVersionDirectivesCompiling) {
using namespace MG_Util::ShaderTranspiler;
const char* body = "\nout vec4 fragColor;\nvoid main() { fragColor = vec4(1.0); }\n";
const auto compiles = [](const String& fullSource) {
String src = fullSource;
PreprocessShaderSource(ShaderStage::Fragment, src);
ShaderAttrib attrib{.shaderType = GL_FRAGMENT_SHADER, .sourceStr = src};
auto res = ShaderCompiler::CompileShader(attrib);
return res ? true : false;
};
EXPECT_TRUE(compiles(String("#version 330 core") + body));
EXPECT_TRUE(compiles(String("\n#version 330 core") + body))
<< "leading whitespace is legal before #version";
EXPECT_TRUE(compiles(String("// test\n#version 330 core") + body))
<< "a leading comment is legal before #version";
}
TEST_F(ProgramUtilTest, PreprocessUsesRealSpacedVersionDirectiveForInjectedOutput) {
using namespace MG_Util::ShaderTranspiler;
@@ -847,6 +903,16 @@ TEST_F(ProgramUtilTest, RetargetLegacyVersionDirectiveOnlyTouchesNormalizedDeskt
String commented = "// #version 330 core\nvoid main() {}\n";
EXPECT_FALSE(RetargetLegacyVersionDirectiveTo460(commented));
EXPECT_EQ(commented.find("#version 460"), String::npos);
// A malformed directive must NOT be rescued to 460 - that is what silently legalized the CTS
// directive.version_* rejection cases. The bad version stays put so glslang keeps rejecting it.
String badNumber = "#version 331\nvoid main() {}\n";
EXPECT_FALSE(RetargetLegacyVersionDirectiveTo460(badNumber));
EXPECT_EQ(badNumber.find("#version 460"), String::npos);
String badProfile = "#version 330 foo\nvoid main() {}\n";
EXPECT_FALSE(RetargetLegacyVersionDirectiveTo460(badProfile));
EXPECT_EQ(badProfile.find("#version 460"), String::npos);
}
const char* fs = R"(#version 150
MobileGL/MG_Util/ShaderTranspiler/ShaderSourceProcessor.cpp
+48 -1
View File
@@ -573,6 +573,23 @@ namespace {
static_cast<unsigned char>(source[1]) == 0xbb && static_cast<unsigned char>(source[2]) == 0xbf;
}
// The GLSL versions MobileGL is willing to normalize. Anything else in a #version line - a number
// that is not a real language version (329, 331), a bad profile keyword, a float/identifier where
// the integer belongs, or trailing tokens - is left untouched so glslang rejects it, matching
// KHR-GL33.shaders.preprocessor.directive.version_*. The set is deliberately generous (every real
// desktop and ES version) so the normalizer never starts rejecting a form it used to accept.
bool IsRecognizedGlslVersion(unsigned version) {
switch (version) {
case 100: case 110: case 120: case 130: case 140: case 150:
case 300: case 310: case 320:
case 330: case 400: case 410: case 420: case 430:
case 440: case 450: case 460:
return true;
default:
return false;
}
}
struct ShaderLanguageInfo {
unsigned version = 110;
MobileGL::ShaderProfile profile = MobileGL::ShaderProfile::Core;
@@ -580,6 +597,9 @@ namespace {
SizeT versionDirectiveEnd = MobileGL::String::npos;
bool hasUtf8Bom = false;
bool enablesGpuShader5 = false;
// Whether the parsed #version directive is a well-formed one MobileGL should rewrite. A
// malformed directive (see IsRecognizedGlslVersion) is left alone for glslang to reject.
bool hasValidVersionDirective = false;
bool HasVersionDirective() const { return versionDirectiveStart != MobileGL::String::npos; }
};
@@ -623,13 +643,25 @@ namespace {
info.versionDirectiveEnd = lineEnd + (hasLineBreak ? 1 : 0);
SkipDirectiveWhitespace(code, probe, lineEnd);
const MobileGL::String profile = ReadDirectiveIdentifier(code, probe, lineEnd);
if (profile == "es" || profile == "ES") {
bool profileTokenValid = true;
if (profile.empty() || profile == "core") {
info.profile = MobileGL::ShaderProfile::Core;
} else if (profile == "es" || profile == "ES") {
info.profile = MobileGL::ShaderProfile::ES;
} else if (profile == "compatibility") {
info.profile = MobileGL::ShaderProfile::Compatibility;
} else {
// "#version 330 foo": an unrecognized profile keyword. Keep Core for any
// downstream routing, but mark the directive malformed.
info.profile = MobileGL::ShaderProfile::Core;
profileTokenValid = false;
}
// Comments are already masked to spaces, so anything non-blank left on the
// line is real trailing garbage: "#version 330 foobar" / "#version 330.0".
SkipDirectiveWhitespace(code, probe, lineEnd);
const bool hasTrailingTokens = probe < lineEnd;
info.hasValidVersionDirective =
IsRecognizedGlslVersion(info.version) && profileTokenValid && !hasTrailingTokens;
}
} else if (directive == "extension") {
SkipDirectiveWhitespace(code, probe, lineEnd);
@@ -676,6 +708,17 @@ namespace {
}
void NormalizeVersionDirective(MobileGL::String& source, const ShaderLanguageInfo& info) {
// A malformed #version (329, 331, bad profile, float/trailing tokens) is left exactly as the
// application wrote it so glslang rejects it - rewriting it to "#version 330 core" would
// silently legalize the CTS directive.version_* rejection cases. Still drop a leading BOM so
// the reported error is the bad version rather than a stray byte-order mark.
if (info.HasVersionDirective() && !info.hasValidVersionDirective) {
if (info.hasUtf8Bom) {
source.erase(0, 3);
}
return;
}
const MobileGL::String replacement = GetNormalizedVersionDirective(info);
if (info.HasVersionDirective()) {
source.replace(info.versionDirectiveStart, info.versionDirectiveEnd - info.versionDirectiveStart,
@@ -1240,6 +1283,10 @@ namespace MobileGL {
// must not be mistaken for the real one.
const ShaderLanguageInfo info = InspectShaderLanguage(source);
if (!info.HasVersionDirective()) return false;
// Never rescue a malformed directive to 460: that is precisely what re-legalized the
// CTS directive.version_* rejection cases after the first compile failed. The shader-
// pack retry this exists for only ever sees a valid low version (a real "#version 330").
if (!info.hasValidVersionDirective) return false;
// Only the set NormalizeVersionDirective downgraded: desktop core below 400. ES and
// compatibility shaders keep whatever they declared.
if (info.profile != ShaderProfile::Core || info.version >= 400) return false;