From bf73bb58bc179f143190f52762d908f6a4851eb7 Mon Sep 17 00:00:00 2001 From: Nathan Ridge Date: Tue, 26 Sep 2017 22:27:27 -0400 Subject: [PATCH] Bug 512297 - Impose a limit on the nesting depth of template arguments This avoid stack overflows when processing code that has very deeply nested template arguments. Change-Id: I748e2d827fd1e7842737ec0652cf3733ae9962b1 --- .../parser/tests/ast2/AST2TemplateTests.java | 51 +++++++++++++++ .../eclipse/cdt/core/dom/ast/IASTProblem.java | 13 ++++ .../org/eclipse/cdt/core/parser/IProblem.java | 8 ++- .../internal/core/dom/parser/ASTProblem.java | 28 +++++++- .../parser/AbstractGNUSourceCodeParser.java | 11 +++- .../core/dom/parser/BacktrackException.java | 6 ++ .../dom/parser/cpp/GNUCPPSourceParser.java | 64 ++++++++++++------- .../core/parser/ParserMessages.properties | 1 + 8 files changed, 154 insertions(+), 28 deletions(-) diff --git a/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/ast2/AST2TemplateTests.java b/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/ast2/AST2TemplateTests.java index f93deae62e2..f0a13482154 100644 --- a/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/ast2/AST2TemplateTests.java +++ b/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/ast2/AST2TemplateTests.java @@ -28,6 +28,7 @@ import org.eclipse.cdt.core.dom.IName; import org.eclipse.cdt.core.dom.ast.ASTTypeUtil; import org.eclipse.cdt.core.dom.ast.IASTBinaryExpression; import org.eclipse.cdt.core.dom.ast.IASTCompoundStatement; +import org.eclipse.cdt.core.dom.ast.IASTDeclaration; import org.eclipse.cdt.core.dom.ast.IASTExpressionStatement; import org.eclipse.cdt.core.dom.ast.IASTFunctionCallExpression; import org.eclipse.cdt.core.dom.ast.IASTFunctionDefinition; @@ -37,6 +38,8 @@ import org.eclipse.cdt.core.dom.ast.IASTImplicitNameOwner; import org.eclipse.cdt.core.dom.ast.IASTName; import org.eclipse.cdt.core.dom.ast.IASTNode; import org.eclipse.cdt.core.dom.ast.IASTNodeSelector; +import org.eclipse.cdt.core.dom.ast.IASTProblem; +import org.eclipse.cdt.core.dom.ast.IASTProblemDeclaration; import org.eclipse.cdt.core.dom.ast.IASTProblemStatement; import org.eclipse.cdt.core.dom.ast.IASTTranslationUnit; import org.eclipse.cdt.core.dom.ast.IASTTypeId; @@ -92,6 +95,7 @@ import org.eclipse.cdt.core.dom.ast.cpp.ICPPTemplateTemplateParameter; import org.eclipse.cdt.core.dom.ast.cpp.ICPPTemplateTypeParameter; import org.eclipse.cdt.core.dom.ast.cpp.ICPPUsingDeclaration; import org.eclipse.cdt.core.dom.ast.cpp.ICPPVariable; +import org.eclipse.cdt.core.parser.IProblem; import org.eclipse.cdt.internal.core.dom.parser.IntegralValue; import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTNameBase; import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPBasicType; @@ -10383,4 +10387,51 @@ public class AST2TemplateTests extends AST2CPPTestBase { public void testMemberOfUnknownMemberClass_519819() throws Exception { parseAndCheckBindings(); } + + // template class any {}; + // typedef any>>>>>>>>>>>>>>>>>>>> + // >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> + // >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> + // >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> + // >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> + // >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> + // >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> + // >>>>>>>>>>>>> parser_killer_type; + public void testTemplateArgumentNestingDepthLimit_512297() throws Exception { + BindingAssertionHelper helper = getAssertionHelper(); + IASTTranslationUnit tu = helper.getTranslationUnit(); + IASTDeclaration[] declarations = tu.getDeclarations(); + assertEquals(2, declarations.length); + assertInstance(declarations[1], IASTProblemDeclaration.class); + IASTProblemDeclaration problemDecl = (IASTProblemDeclaration) declarations[1]; + IASTProblem problem = problemDecl.getProblem().getOriginalProblem(); + assertNotNull(problem); + assertEquals(IProblem.TEMPLATE_ARGUMENT_NESTING_DEPTH_LIMIT_EXCEEDED, problem.getID()); + } } diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/core/dom/ast/IASTProblem.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/core/dom/ast/IASTProblem.java index c10d71cfbdb..296a817f9dd 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/core/dom/ast/IASTProblem.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/core/dom/ast/IASTProblem.java @@ -30,4 +30,17 @@ public interface IASTProblem extends IProblem, IASTNode { */ @Override public IASTProblem copy(CopyStyle style); + + /** + * If this problem was triggered by another problem, returns that problem, + * otherwise returns null. + * @since 6.4 + */ + public IASTProblem getOriginalProblem(); + + /** + * Record another problem as being the original cause of this one. + * @since 6.4 + */ + public void setOriginalProblem(IASTProblem original); } diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/core/parser/IProblem.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/core/parser/IProblem.java index 17c35141843..4e3c8148db0 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/core/parser/IProblem.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/core/parser/IProblem.java @@ -111,7 +111,7 @@ public interface IProblem { public final static int SYNTAX_RELATED = 0x04000000; /** - * IProblem relates to an implementation of design limitation + * IProblem relates to an implementation or design limitation */ public final static int INTERNAL_RELATED = 0x10000000; @@ -356,6 +356,12 @@ public interface IProblem { */ public final static int MISSING_SEMICOLON = SYNTAX_RELATED | 0x002; + /** + * The parser's template argument nesting depth limit was exceeded. + * @since 6.4 + */ + public final static int TEMPLATE_ARGUMENT_NESTING_DEPTH_LIMIT_EXCEEDED = INTERNAL_RELATED | 0x01; + /** * @deprecated Not used. * @noreference This field is not intended to be referenced by clients. diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/ASTProblem.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/ASTProblem.java index d6ba7ce8f01..a9ef2b81ee7 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/ASTProblem.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/ASTProblem.java @@ -98,11 +98,14 @@ public class ASTProblem extends ASTNode implements IASTProblem { ParserMessages.getString("ParserProblemFactory.error.syntax.syntaxError")); //$NON-NLS-1$ errorMessages.put(Integer.valueOf(MISSING_SEMICOLON), ParserMessages.getString("ParserProblemFactory.error.syntax.missingSemicolon")); //$NON-NLS-1$ + errorMessages.put(Integer.valueOf(TEMPLATE_ARGUMENT_NESTING_DEPTH_LIMIT_EXCEEDED), + ParserMessages.getString("ParserProblemFactory.error.syntax.templateArgumentNestingDepthLimitExceeded")); //$NON-NLS-1$ } private final int id; private final char[] arg; private boolean isError; + private IASTProblem originalProblem = null; public ASTProblem(IASTNode parent, ASTNodeProperty property, int id, char[] arg, boolean isError, int startNumber, int endNumber) { @@ -158,20 +161,27 @@ public class ASTProblem extends ASTNode implements IASTProblem { return ParserMessages.getFormattedString("BaseProblemFactory.problemPattern", args); //$NON-NLS-1$ } - public static String getMessage(int id, String arg) { + private static String getMessage(int id, String arg, IASTProblem originalProblem) { String msg = errorMessages.get(Integer.valueOf(id)); if (msg == null) msg = ""; //$NON-NLS-1$ if (arg != null) { - return MessageFormat.format(msg, new Object[] {arg}); + msg = MessageFormat.format(msg, new Object[] {arg}); + } + if (originalProblem != null) { + msg = MessageFormat.format("{0}: {1}", msg, originalProblem.getMessage()); //$NON-NLS-1$ } return msg; } + + public static String getMessage(int id, String arg) { + return getMessage(id, arg, null); + } @Override public String getMessage() { - return getMessage(id, arg == null ? null : new String(arg)); + return getMessage(id, arg == null ? null : new String(arg), originalProblem); } @Override @@ -219,4 +229,16 @@ public class ASTProblem extends ASTNode implements IASTProblem { } return INT_VALUE_NOT_PROVIDED; } + + + + @Override + public IASTProblem getOriginalProblem() { + return originalProblem; + } + + @Override + public void setOriginalProblem(IASTProblem original) { + originalProblem = original; + } } diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/AbstractGNUSourceCodeParser.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/AbstractGNUSourceCodeParser.java index 4c117a22481..4302ee90c2e 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/AbstractGNUSourceCodeParser.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/AbstractGNUSourceCodeParser.java @@ -739,11 +739,18 @@ public abstract class AbstractGNUSourceCodeParser implements ISourceCodeParser { return result; } - protected IASTProblemDeclaration skipProblemDeclaration(int offset) { + protected IASTProblemDeclaration skipProblemDeclaration(int offset) { + return skipProblemDeclaration(offset, null); + } + + protected IASTProblemDeclaration skipProblemDeclaration(int offset, IASTProblem origProblem) { failParse(); declarationMark= null; int endOffset = skipToSemiOrClosingBrace(offset, false); IASTProblem problem= createProblem(IProblem.SYNTAX_ERROR, offset, endOffset-offset); + if (origProblem != null && origProblem.getID() != IProblem.SYNTAX_ERROR) { + problem.setOriginalProblem(origProblem); + } return buildProblemDeclaration(problem); } @@ -1771,7 +1778,7 @@ public abstract class AbstractGNUSourceCodeParser implements ISourceCodeParser { } } - return new IASTDeclaration[] {skipProblemDeclaration(offset)}; + return new IASTDeclaration[] {skipProblemDeclaration(offset, origProblem)}; } protected IASTDeclaration asmDeclaration() throws EndOfFileException, BacktrackException { diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/BacktrackException.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/BacktrackException.java index 5c28988da0b..32f2b4772ef 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/BacktrackException.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/BacktrackException.java @@ -14,6 +14,7 @@ package org.eclipse.cdt.internal.core.dom.parser; import org.eclipse.cdt.core.dom.ast.IASTNode; import org.eclipse.cdt.core.dom.ast.IASTProblem; +import org.eclipse.cdt.core.parser.IProblem; /** * @author jcamelon @@ -95,4 +96,9 @@ public class BacktrackException extends Exception { public StackTraceElement[] getStackTrace() { return EMPTY_STACK; } + + // Don't try alternative parses if this returns true. + public boolean isFatal() { + return problem != null && problem.getID() == IProblem.TEMPLATE_ARGUMENT_NESTING_DEPTH_LIMIT_EXCEEDED; + } } diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/GNUCPPSourceParser.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/GNUCPPSourceParser.java index 280fe8f63a7..d4e6f8a9a1b 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/GNUCPPSourceParser.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/GNUCPPSourceParser.java @@ -185,6 +185,7 @@ import org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.CPPVisitor; public class GNUCPPSourceParser extends AbstractGNUSourceCodeParser { private static final int DEFAULT_PARM_LIST_SIZE = 4; private static final int DEFAULT_CATCH_HANDLER_LIST_SIZE= 4; + private static final int TEMPLATE_ARGUMENT_NESTING_DEPTH_LIMIT = 256; // This is a parameter to the protected function {@link #declarator(DtorStrategy, DeclarationOptions)} // so it needs to be protected too. @@ -200,6 +201,7 @@ public class GNUCPPSourceParser extends AbstractGNUSourceCodeParser { protected ICPPASTTranslationUnit translationUnit; private int functionBodyCount; + private int templateArgumentNestingDepth = 0; private char[] currentClassName; private char[] additionalNumericalSuffixes; @@ -281,6 +283,9 @@ public class GNUCPPSourceParser extends AbstractGNUSourceCodeParser { try { return nameSpecifier(ctx, strat); } catch (BacktrackException e) { + if (e.isFatal()) { + throw e; + } if (strat.setNextAlternative(true /* previous alternative failed to parse */)) { backup(m); } else { @@ -705,33 +710,42 @@ public class GNUCPPSourceParser extends AbstractGNUSourceCodeParser { } private List templateArgumentList(ITemplateIdStrategy strat) throws EndOfFileException, BacktrackException { - int startingOffset = LA(1).getOffset(); - int endOffset = 0; - List list= null; - - boolean needComma= false; - int lt1= LT(1); - while (lt1 != IToken.tGT && lt1 != IToken.tGT_in_SHIFTR && lt1 != IToken.tEOC) { - if (needComma) { - if (lt1 != IToken.tCOMMA) { - throwBacktrack(startingOffset, endOffset - startingOffset); + if (templateArgumentNestingDepth >= TEMPLATE_ARGUMENT_NESTING_DEPTH_LIMIT) { + throwBacktrack(createProblem(IProblem.TEMPLATE_ARGUMENT_NESTING_DEPTH_LIMIT_EXCEEDED, + LA(1).getOffset(), 1)); + } + ++templateArgumentNestingDepth; + try { + int startingOffset = LA(1).getOffset(); + int endOffset = 0; + List list= null; + + boolean needComma= false; + int lt1= LT(1); + while (lt1 != IToken.tGT && lt1 != IToken.tGT_in_SHIFTR && lt1 != IToken.tEOC) { + if (needComma) { + if (lt1 != IToken.tCOMMA) { + throwBacktrack(startingOffset, endOffset - startingOffset); + } + consume(); + } else { + needComma= true; } - consume(); - } else { - needComma= true; + + IASTNode node= templateArgument(strat); + if (list == null) { + list= new ArrayList<>(); + } + list.add(node); + lt1= LT(1); } - - IASTNode node= templateArgument(strat); if (list == null) { - list= new ArrayList<>(); + return Collections.emptyList(); } - list.add(node); - lt1= LT(1); + return list; + } finally { + --templateArgumentNestingDepth; } - if (list == null) { - return Collections.emptyList(); - } - return list; } private IASTNode templateArgument(ITemplateIdStrategy strat) throws EndOfFileException, BacktrackException { @@ -742,6 +756,9 @@ public class GNUCPPSourceParser extends AbstractGNUSourceCodeParser { typeId= typeId(DeclarationOptions.TYPEID); lt1 = LT(1); } catch (BacktrackException e) { + if (e.isFatal()) { + throw e; + } } if (typeId != null @@ -800,6 +817,9 @@ public class GNUCPPSourceParser extends AbstractGNUSourceCodeParser { } // The type-id is longer, use it. } catch (BacktrackException e) { + if (e.isFatal()) { + throw e; + } // Failed to parse an expression, use the type id. } diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/parser/ParserMessages.properties b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/parser/ParserMessages.properties index ed4a546ed4b..6791d5f39a1 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/parser/ParserMessages.properties +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/parser/ParserMessages.properties @@ -48,6 +48,7 @@ ScannerProblemFactory.error.scanner.floatWithBadPrefix=Floating constant "{0}" h ParserProblemFactory.error.syntax.syntaxError=Syntax error ParserProblemFactory.error.syntax.missingSemicolon=Missing ';' +ParserProblemFactory.error.syntax.templateArgumentNestingDepthLimitExceeded=Template argument nesting depth limit exceeded BaseProblemFactory.problemPattern={0} in file: {1}:{2, number, integer}