From 7d67c6cbc210ab547de46facd5b829478fe59d23 Mon Sep 17 00:00:00 2001 From: John Kessenich Date: Mon, 16 Oct 2017 15:29:07 -0600 Subject: [PATCH] PP: Fix #1104: Missing check for #if overflow. Also, rationalized this to generally make it safer and more readable. It could use a more modern approach, at some point... --- Test/baseResults/cppDeepNest.frag.out | 20 +++ Test/cppDeepNest.frag | 117 ++++++++++++++++++ .../MachineIndependent/preprocessor/Pp.cpp | 41 +++--- .../preprocessor/PpContext.h | 2 +- gtests/AST.FromFile.cpp | 1 + 5 files changed, 164 insertions(+), 17 deletions(-) create mode 100755 Test/baseResults/cppDeepNest.frag.out create mode 100644 Test/cppDeepNest.frag diff --git a/Test/baseResults/cppDeepNest.frag.out b/Test/baseResults/cppDeepNest.frag.out new file mode 100755 index 00000000..969a256d --- /dev/null +++ b/Test/baseResults/cppDeepNest.frag.out @@ -0,0 +1,20 @@ +cppDeepNest.frag +ERROR: 0:66: '#if/#ifdef/#ifndef' : maximum nesting depth exceeded +ERROR: 0:66: '' : missing #endif +ERROR: 0:66: '' : syntax error, unexpected $end +ERROR: 3 compilation errors. No code generated. + + +Shader version: 100 +ERROR: node is still EOpNull! +0:? Linker Objects + + +Linked fragment stage: + +ERROR: Linking fragment stage: Missing entry point: Each stage requires one entry point + +Shader version: 100 +ERROR: node is still EOpNull! +0:? Linker Objects + diff --git a/Test/cppDeepNest.frag b/Test/cppDeepNest.frag new file mode 100644 index 00000000..51c436e4 --- /dev/null +++ b/Test/cppDeepNest.frag @@ -0,0 +1,117 @@ +#ifdef O +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#if +#endif diff --git a/glslang/MachineIndependent/preprocessor/Pp.cpp b/glslang/MachineIndependent/preprocessor/Pp.cpp index ecda34cd..e9930a97 100644 --- a/glslang/MachineIndependent/preprocessor/Pp.cpp +++ b/glslang/MachineIndependent/preprocessor/Pp.cpp @@ -241,15 +241,20 @@ int TPpContext::CPPelse(int matchelse, TPpToken* ppToken) int nextAtom = atomStrings.getAtom(ppToken->name); if (nextAtom == PpAtomIf || nextAtom == PpAtomIfdef || nextAtom == PpAtomIfndef) { depth++; - ifdepth++; - elsetracker++; + if (ifdepth >= maxIfNesting || elsetracker >= maxIfNesting) { + parseContext.ppError(ppToken->loc, "maximum nesting depth exceeded", "#if/#ifdef/#ifndef", ""); + return EndOfInput; + } else { + ifdepth++; + elsetracker++; + } } else if (nextAtom == PpAtomEndif) { token = extraTokenCheck(nextAtom, ppToken, scanToken(ppToken)); elseSeen[elsetracker] = false; --elsetracker; if (depth == 0) { // found the #endif we are looking for - if (ifdepth) + if (ifdepth > 0) --ifdepth; break; } @@ -266,7 +271,7 @@ int TPpContext::CPPelse(int matchelse, TPpToken* ppToken) parseContext.ppError(ppToken->loc, "#elif after #else", "#elif", ""); /* we decrement ifdepth here, because CPPif will increment * it and we really want to leave it alone */ - if (ifdepth) { + if (ifdepth > 0) { --ifdepth; elseSeen[elsetracker] = false; --elsetracker; @@ -536,11 +541,12 @@ int TPpContext::evalToToken(int token, bool shortCircuit, int& res, bool& err, T int TPpContext::CPPif(TPpToken* ppToken) { int token = scanToken(ppToken); - elsetracker++; - ifdepth++; - if (ifdepth > maxIfNesting) { + if (ifdepth >= maxIfNesting || elsetracker >= maxIfNesting) { parseContext.ppError(ppToken->loc, "maximum nesting depth exceeded", "#if", ""); - return 0; + return EndOfInput; + } else { + elsetracker++; + ifdepth++; } int res = 0; bool err = false; @@ -556,11 +562,14 @@ int TPpContext::CPPif(TPpToken* ppToken) int TPpContext::CPPifdef(int defined, TPpToken* ppToken) { int token = scanToken(ppToken); - if (++ifdepth > maxIfNesting) { + if (ifdepth > maxIfNesting || elsetracker > maxIfNesting) { parseContext.ppError(ppToken->loc, "maximum nesting depth exceeded", "#ifdef", ""); - return 0; + return EndOfInput; + } else { + elsetracker++; + ifdepth++; } - elsetracker++; + if (token != PpAtomIdentifier) { if (defined) parseContext.ppError(ppToken->loc, "must be followed by macro name", "#ifdef", ""); @@ -886,16 +895,16 @@ int TPpContext::readCPPline(TPpToken* ppToken) token = CPPdefine(ppToken); break; case PpAtomElse: - if (elsetracker[elseSeen]) + if (elseSeen[elsetracker]) parseContext.ppError(ppToken->loc, "#else after #else", "#else", ""); - elsetracker[elseSeen] = true; - if (! ifdepth) + elseSeen[elsetracker] = true; + if (ifdepth == 0) parseContext.ppError(ppToken->loc, "mismatched statements", "#else", ""); token = extraTokenCheck(PpAtomElse, ppToken, scanToken(ppToken)); token = CPPelse(0, ppToken); break; case PpAtomElif: - if (! ifdepth) + if (ifdepth == 0) parseContext.ppError(ppToken->loc, "mismatched statements", "#elif", ""); if (elseSeen[elsetracker]) parseContext.ppError(ppToken->loc, "#elif after #else", "#elif", ""); @@ -906,7 +915,7 @@ int TPpContext::readCPPline(TPpToken* ppToken) token = CPPelse(0, ppToken); break; case PpAtomEndif: - if (! ifdepth) + if (ifdepth == 0) parseContext.ppError(ppToken->loc, "mismatched statements", "#endif", ""); else { elseSeen[elsetracker] = false; diff --git a/glslang/MachineIndependent/preprocessor/PpContext.h b/glslang/MachineIndependent/preprocessor/PpContext.h index de48e279..854bbbad 100644 --- a/glslang/MachineIndependent/preprocessor/PpContext.h +++ b/glslang/MachineIndependent/preprocessor/PpContext.h @@ -309,7 +309,7 @@ protected: bool endOfReplacementList() { return inputStack.empty() || inputStack.back()->endOfReplacementList(); } bool isMacroInput() { return inputStack.size() > 0 && inputStack.back()->isMacroInput(); } - static const int maxIfNesting = 64; + static const int maxIfNesting = 65; int ifdepth; // current #if-#else-#endif nesting in the cpp.c file (pre-processor) bool elseSeen[maxIfNesting]; // Keep a track of whether an else has been seen at a particular depth diff --git a/gtests/AST.FromFile.cpp b/gtests/AST.FromFile.cpp index fbbf81b5..b9633377 100644 --- a/gtests/AST.FromFile.cpp +++ b/gtests/AST.FromFile.cpp @@ -82,6 +82,7 @@ INSTANTIATE_TEST_CASE_P( "cppBad.vert", "cppBad2.vert", "cppComplexExpr.vert", + "cppDeepNest.frag", "badChars.frag", "pointCoord.frag", "array.frag",