From a86836ede213899dcb56f8220d5a8d40d342dfe8 Mon Sep 17 00:00:00 2001 From: John Kessenich Date: Sat, 9 Jul 2016 14:50:57 -0600 Subject: [PATCH] Front-end: Fix known crashes by early exit on error (issue #29, issue #34, issue #35). Added -C option to request cascading errors. By default, will exit early, to avoid all error-recovery-based crashes. This works by simulating end-of-file in input on first error, so no need for exception handling, or stack unwinding, or any complex error checking/handling to get out of the stack. --- StandAlone/StandAlone.cpp | 45 +++++++++++++++++------------- Test/runtests | 6 ++-- glslang/MachineIndependent/ParseHelper.cpp | 6 ++++ glslang/MachineIndependent/Scan.h | 16 ++++++++--- glslang/MachineIndependent/ShaderLang.cpp | 4 +-- glslang/Public/ShaderLang.h | 1 + gtests/Config.FromFile.cpp | 4 +-- gtests/Initializer.h | 2 +- gtests/Link.FromFile.cpp | 3 +- gtests/TestFixture.cpp | 2 +- gtests/TestFixture.h | 5 ++-- 11 files changed, 57 insertions(+), 37 deletions(-) diff --git a/StandAlone/StandAlone.cpp b/StandAlone/StandAlone.cpp index 2d01b87..63e9878 100644 --- a/StandAlone/StandAlone.cpp +++ b/StandAlone/StandAlone.cpp @@ -58,25 +58,26 @@ extern "C" { // Command-line options enum TOptions { - EOptionNone = 0x0000, - EOptionIntermediate = 0x0001, - EOptionSuppressInfolog = 0x0002, - EOptionMemoryLeakMode = 0x0004, - EOptionRelaxedErrors = 0x0008, - EOptionGiveWarnings = 0x0010, - EOptionLinkProgram = 0x0020, - EOptionMultiThreaded = 0x0040, - EOptionDumpConfig = 0x0080, - EOptionDumpReflection = 0x0100, - EOptionSuppressWarnings = 0x0200, - EOptionDumpVersions = 0x0400, - EOptionSpv = 0x0800, - EOptionHumanReadableSpv = 0x1000, - EOptionVulkanRules = 0x2000, - EOptionDefaultDesktop = 0x4000, - EOptionOutputPreprocessed = 0x8000, - EOptionOutputHexadecimal = 0x10000, - EOptionReadHlsl = 0x20000, + EOptionNone = 0, + EOptionIntermediate = (1 << 0), + EOptionSuppressInfolog = (1 << 1), + EOptionMemoryLeakMode = (1 << 2), + EOptionRelaxedErrors = (1 << 3), + EOptionGiveWarnings = (1 << 4), + EOptionLinkProgram = (1 << 5), + EOptionMultiThreaded = (1 << 6), + EOptionDumpConfig = (1 << 7), + EOptionDumpReflection = (1 << 8), + EOptionSuppressWarnings = (1 << 9), + EOptionDumpVersions = (1 << 10), + EOptionSpv = (1 << 11), + EOptionHumanReadableSpv = (1 << 12), + EOptionVulkanRules = (1 << 13), + EOptionDefaultDesktop = (1 << 14), + EOptionOutputPreprocessed = (1 << 15), + EOptionOutputHexadecimal = (1 << 16), + EOptionReadHlsl = (1 << 17), + EOptionCascadingErrors = (1 << 18), }; // @@ -247,6 +248,9 @@ void ProcessArguments(int argc, char* argv[]) case 'c': Options |= EOptionDumpConfig; break; + case 'C': + Options |= EOptionCascadingErrors; + break; case 'd': Options |= EOptionDefaultDesktop; break; @@ -347,6 +351,8 @@ void SetMessageOptions(EShMessages& messages) messages = (EShMessages)(messages | EShMsgOnlyPreprocessor); if (Options & EOptionReadHlsl) messages = (EShMessages)(messages | EShMsgReadHlsl); + if (Options & EOptionCascadingErrors) + messages = (EShMessages)(messages | EShMsgCascadingErrors); } // @@ -772,6 +778,7 @@ void usage() " errors will appear on stderr.\n" " -c configuration dump;\n" " creates the default configuration file (redirect to a .conf file)\n" + " -C cascading errors; risks crashes from accumulation of error recoveries\n" " -d default to desktop (#version 110) when there is no shader #version\n" " (default is ES version 100)\n" " -D input is HLSL\n" diff --git a/Test/runtests b/Test/runtests index 3bf03fb..ae7ed45 100755 --- a/Test/runtests +++ b/Test/runtests @@ -22,15 +22,15 @@ rm -f comp.spv frag.spv geom.spv tesc.spv tese.spv vert.spv # reflection tests # echo Running reflection... -$EXE -l -q reflection.vert > $TARGETDIR/reflection.vert.out +$EXE -l -q -C reflection.vert > $TARGETDIR/reflection.vert.out diff -b $BASEDIR/reflection.vert.out $TARGETDIR/reflection.vert.out || HASERROR=1 # # multi-threaded test # echo Comparing single thread to multithread for all tests in current directory... -$EXE -i *.vert *.geom *.frag *.tes* *.comp > singleThread.out -$EXE -i *.vert *.geom *.frag *.tes* *.comp -t > multiThread.out +$EXE -i -C *.vert *.geom *.frag *.tes* *.comp > singleThread.out +$EXE -i -C *.vert *.geom *.frag *.tes* *.comp -t > multiThread.out diff singleThread.out multiThread.out || HASERROR=1 if [ $HASERROR -eq 0 ] diff --git a/glslang/MachineIndependent/ParseHelper.cpp b/glslang/MachineIndependent/ParseHelper.cpp index 5dacc0e..bd4b3c9 100644 --- a/glslang/MachineIndependent/ParseHelper.cpp +++ b/glslang/MachineIndependent/ParseHelper.cpp @@ -377,6 +377,9 @@ void C_DECL TParseContext::error(const TSourceLoc& loc, const char* szReason, co va_start(args, szExtraInfoFormat); outputMessage(loc, szReason, szToken, szExtraInfoFormat, EPrefixError, args); va_end(args); + + if ((messages & EShMsgCascadingErrors) == 0) + currentScanner->setEndOfInput(); } void C_DECL TParseContext::warn(const TSourceLoc& loc, const char* szReason, const char* szToken, @@ -397,6 +400,9 @@ void C_DECL TParseContext::ppError(const TSourceLoc& loc, const char* szReason, va_start(args, szExtraInfoFormat); outputMessage(loc, szReason, szToken, szExtraInfoFormat, EPrefixError, args); va_end(args); + + if ((messages & EShMsgCascadingErrors) == 0) + currentScanner->setEndOfInput(); } void C_DECL TParseContext::ppWarn(const TSourceLoc& loc, const char* szReason, const char* szToken, diff --git a/glslang/MachineIndependent/Scan.h b/glslang/MachineIndependent/Scan.h index 15e3c93..4282cd5 100644 --- a/glslang/MachineIndependent/Scan.h +++ b/glslang/MachineIndependent/Scan.h @@ -40,7 +40,7 @@ namespace glslang { -// Use a global end-of-input character, so no tranlation is needed across +// Use a global end-of-input character, so no translation is needed across // layers of encapsulation. Characters are all 8 bit, and positive, so there is // no aliasing of character 255 onto -1, for example. const int EndOfInput = -1; @@ -82,7 +82,8 @@ public: int get() { int ret = peek(); - if (ret == EndOfInput) return ret; + if (ret == EndOfInput) + return ret; ++loc[currentSource].column; ++logicalSourceLoc.column; if (ret == '\n') { @@ -123,7 +124,8 @@ public: void unget() { // Do not roll back once we've reached the end of the file. - if (endOfFileReached) return; + if (endOfFileReached) + return; if (currentChar > 0) { --currentChar; @@ -196,6 +198,12 @@ public: loc[getLastValidSourceIndex()].column = col; } + void setEndOfInput() + { + endOfFileReached = true; + currentSource = numSources; + } + const TSourceLoc& getSourceLoc() const { if (singleLogical) { @@ -255,7 +263,7 @@ protected: bool singleLogical; // treats the strings as a single logical string. // locations will be reported from the first string. - // set to true once peak() returns EndOfFile, so that we won't roll back + // Set to true once peek() returns EndOfFile, so that we won't roll back // once we've reached EndOfFile. bool endOfFileReached; }; diff --git a/glslang/MachineIndependent/ShaderLang.cpp b/glslang/MachineIndependent/ShaderLang.cpp index 049f18b..dccc1f0 100644 --- a/glslang/MachineIndependent/ShaderLang.cpp +++ b/glslang/MachineIndependent/ShaderLang.cpp @@ -809,8 +809,8 @@ struct DoPreprocessing { explicit DoPreprocessing(std::string* string): outputString(string) {} bool operator()(TParseContextBase& parseContext, TPpContext& ppContext, TInputScanner& input, bool versionWillBeError, - TSymbolTable& , TIntermediate& , - EShOptimizationLevel , EShMessages ) + TSymbolTable&, TIntermediate&, + EShOptimizationLevel, EShMessages) { // This is a list of tokens that do not require a space before or after. static const std::string unNeededSpaceTokens = ";()[]"; diff --git a/glslang/Public/ShaderLang.h b/glslang/Public/ShaderLang.h index c01d67e..4b4d0fc 100644 --- a/glslang/Public/ShaderLang.h +++ b/glslang/Public/ShaderLang.h @@ -141,6 +141,7 @@ enum EShMessages { EShMsgVulkanRules = (1 << 4), // issue messages for Vulkan-requirements of GLSL for SPIR-V EShMsgOnlyPreprocessor = (1 << 5), // only print out errors produced by the preprocessor EShMsgReadHlsl = (1 << 6), // use HLSL parsing rules and semantics + EShMsgCascadingErrors = (1 << 7), // get cascading errors; risks error-recovery issues, instead of an early exit }; // diff --git a/gtests/Config.FromFile.cpp b/gtests/Config.FromFile.cpp index 20c98be..7bc6a70 100644 --- a/gtests/Config.FromFile.cpp +++ b/gtests/Config.FromFile.cpp @@ -97,8 +97,8 @@ TEST_P(ConfigTest, FromFile) INSTANTIATE_TEST_CASE_P( Glsl, ConfigTest, ::testing::ValuesIn(std::vector({ - {"specExamples.vert", "baseResults/test.conf", "specExamples.vert.out", EShMsgAST}, - {"100Limits.vert", "100.conf", "100LimitsConf.vert.out", EShMsgDefault}, + {"specExamples.vert", "baseResults/test.conf", "specExamples.vert.out", (EShMessages)(EShMsgAST | EShMsgCascadingErrors)}, + {"100Limits.vert", "100.conf", "100LimitsConf.vert.out", EShMsgCascadingErrors}, })), ); // clang-format on diff --git a/gtests/Initializer.h b/gtests/Initializer.h index 3cafb52..34af9ad 100644 --- a/gtests/Initializer.h +++ b/gtests/Initializer.h @@ -51,7 +51,7 @@ namespace glslangtest { // gets fixed. class GlslangInitializer { public: - GlslangInitializer() : lastMessages(EShMsgDefault) + GlslangInitializer() : lastMessages(EShMsgCascadingErrors) { glslang::InitializeProcess(); } diff --git a/gtests/Link.FromFile.cpp b/gtests/Link.FromFile.cpp index 331c5b2..b324b9a 100644 --- a/gtests/Link.FromFile.cpp +++ b/gtests/Link.FromFile.cpp @@ -48,8 +48,7 @@ TEST_P(LinkTest, FromFile) { const auto& fileNames = GetParam(); const size_t fileCount = fileNames.size(); - const EShMessages controls = DeriveOptions( - Source::GLSL, Semantics::OpenGL, Target::AST); + const EShMessages controls = DeriveOptions(Source::GLSL, Semantics::OpenGL, Target::AST); GlslangResult result; // Compile each input shader file. diff --git a/gtests/TestFixture.cpp b/gtests/TestFixture.cpp index dad973c..f170f36 100644 --- a/gtests/TestFixture.cpp +++ b/gtests/TestFixture.cpp @@ -68,7 +68,7 @@ EShLanguage GetShaderStage(const std::string& stage) EShMessages DeriveOptions(Source source, Semantics semantics, Target target) { - EShMessages result = EShMsgDefault; + EShMessages result = EShMsgCascadingErrors; switch (source) { case Source::GLSL: diff --git a/gtests/TestFixture.h b/gtests/TestFixture.h index ea74d41..8dffb80 100644 --- a/gtests/TestFixture.h +++ b/gtests/TestFixture.h @@ -264,8 +264,7 @@ public: tryLoadFile(expectedOutputFname, "expected output", &expectedOutput); const EShMessages controls = DeriveOptions(source, semantics, target); - GlslangResult result = - compileAndLink(testName, input, entryPointName, controls); + GlslangResult result = compileAndLink(testName, input, entryPointName, controls); // Generate the hybrid output in the way of glslangValidator. std::ostringstream stream; @@ -290,7 +289,7 @@ public: glslang::TShader::ForbidInclude includer; const bool success = shader.preprocess( &glslang::DefaultTBuiltInResource, defaultVersion, defaultProfile, - forceVersionProfile, isForwardCompatible, EShMsgOnlyPreprocessor, + forceVersionProfile, isForwardCompatible, (EShMessages)(EShMsgOnlyPreprocessor | EShMsgCascadingErrors), &ppShader, includer); std::string log = shader.getInfoLog(); -- 2.7.4