[Clang] Increase the default expression nesting limit #132021
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: cor3ntin (cor3ntin) ChangesThis iterates on #104717 (which we had to revert) In a bid to increase our chances of success, we try to avoid blowing up the stack by
On a 64 bits system, the following stack size reductions are observed Fixes #94728 Full diff: https://github.com/llvm/llvm-project/pull/132021.diff 10 Files Affected:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 0adbc19f40096..af72e80cc1a4b 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -169,6 +169,8 @@ Modified Compiler Flags
the behavior of ``-mtp`` in gcc. This changes the default behavior for ARM targets that provide the ``TPIDRURO`` register as this will be used instead of a call to the ``__aeabi_read_tp``.
Programs that use ``__aeabi_read_tp`` but do not use the ``TPIDRURO`` register must use ``-mtp=soft``. Fixes #123864
+- The compiler flag `-fbracket-depth` default value is increased from 256 to 2048. (#GH94728)
+
Removed Compiler Flags
-------------------------
@@ -264,7 +266,7 @@ Improvements to Clang's diagnostics
as function arguments or return value respectively. Note that
:doc:`ThreadSafetyAnalysis` still does not perform alias analysis. The
feature will be default-enabled with ``-Wthread-safety`` in a future release.
-- The ``-Wsign-compare`` warning now treats expressions with bitwise not(~) and minus(-) as signed integers
+- The ``-Wsign-compare`` warning now treats expressions with bitwise not(~) and minus(-) as signed integers
except for the case where the operand is an unsigned integer
and throws warning if they are compared with unsigned integers (##18878).
- The ``-Wunnecessary-virtual-specifier`` warning has been added to warn about
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 66ae8f1c7f064..d41c7f20feba9 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -8161,7 +8161,7 @@ def fapply_global_visibility_to_externs : Flag<["-"], "fapply-global-visibility-
MarshallingInfoFlag<LangOpts<"SetVisibilityForExternDecls">>;
def fbracket_depth : Separate<["-"], "fbracket-depth">,
HelpText<"Maximum nesting level for parentheses, brackets, and braces">,
- MarshallingInfoInt<LangOpts<"BracketDepth">, "256">;
+ MarshallingInfoInt<LangOpts<"BracketDepth">, "2048">;
defm const_strings : BoolOption<"f", "const-strings",
LangOpts<"ConstStrings">, DefaultFalse,
PosFlag<SetTrue, [], [ClangOption, CC1Option], "Use">,
diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h
index 1c8caf55a546d..fbe2865b1b7c1 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -91,6 +91,8 @@ class Parser : public CodeCompletionHandler {
DiagnosticsEngine &Diags;
+ StackExhaustionHandler StackHandler;
+
/// ScopeCache - Cache scopes to reduce malloc traffic.
enum { ScopeCacheSize = 16 };
unsigned NumCachedScopes;
@@ -518,7 +520,7 @@ class Parser : public CodeCompletionHandler {
typedef Sema::FullExprArg FullExprArg;
/// A SmallVector of statements.
- typedef SmallVector<Stmt *, 32> StmtVector;
+ typedef SmallVector<Stmt *, 24> StmtVector;
// Parsing methods.
@@ -3842,6 +3844,8 @@ class Parser : public CodeCompletionHandler {
DeclGroupPtrTy ParseTemplateDeclarationOrSpecialization(
DeclaratorContext Context, SourceLocation &DeclEnd,
ParsedAttributes &AccessAttrs, AccessSpecifier AS);
+ clang::Parser::DeclGroupPtrTy ParseTemplateDeclarationOrSpecialization(
+ DeclaratorContext Context, SourceLocation &DeclEnd, AccessSpecifier AS);
DeclGroupPtrTy ParseDeclarationAfterTemplate(
DeclaratorContext Context, ParsedTemplateInfo &TemplateInfo,
ParsingDeclRAIIObject &DiagsFromParams, SourceLocation &DeclEnd,
diff --git a/clang/include/clang/Sema/ParsedAttr.h b/clang/include/clang/Sema/ParsedAttr.h
index e1faab205f647..b88b871dc8821 100644
--- a/clang/include/clang/Sema/ParsedAttr.h
+++ b/clang/include/clang/Sema/ParsedAttr.h
@@ -970,6 +970,14 @@ class ParsedAttributes : public ParsedAttributesView {
pool.takeAllFrom(Other.pool);
}
+ void takeAllAtEndFrom(ParsedAttributes &Other) {
+ assert(&Other != this &&
+ "ParsedAttributes can't take attributes from itself");
+ addAllAtEnd(Other.begin(), Other.end());
+ Other.clearListOnly();
+ pool.takeAllFrom(Other.pool);
+ }
+
void takeOneFrom(ParsedAttributes &Other, ParsedAttr *PA) {
assert(&Other != this &&
"ParsedAttributes can't take attribute from itself");
@@ -1067,10 +1075,11 @@ class ParsedAttributes : public ParsedAttributesView {
mutable AttributePool pool;
};
-/// Consumes the attributes from `First` and `Second` and concatenates them into
-/// `Result`. Sets `Result.Range` to the combined range of `First` and `Second`.
-void takeAndConcatenateAttrs(ParsedAttributes &First, ParsedAttributes &Second,
- ParsedAttributes &Result);
+/// Consumes the attributes from `Second` and concatenates them
+/// at the end of `First`. Sets `First.Range`
+/// to the combined range of `First` and `Second`.
+void takeAndConcatenateAttrs(ParsedAttributes &First,
+ ParsedAttributes &&Second);
/// These constants match the enumerated choices of
/// err_attribute_argument_n_type and err_attribute_argument_type.
diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index 82b394d5b4ca6..9ca3e2b5756ca 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -2076,10 +2076,9 @@ Parser::DeclGroupPtrTy Parser::ParseDeclaration(DeclaratorContext Context,
ProhibitAttributes(DeclSpecAttrs);
return ParseNamespace(Context, DeclEnd);
case tok::kw_using: {
- ParsedAttributes Attrs(AttrFactory);
- takeAndConcatenateAttrs(DeclAttrs, DeclSpecAttrs, Attrs);
+ takeAndConcatenateAttrs(DeclAttrs, std::move(DeclSpecAttrs));
return ParseUsingDirectiveOrDeclaration(Context, ParsedTemplateInfo(),
- DeclEnd, Attrs);
+ DeclEnd, DeclAttrs);
}
case tok::kw_static_assert:
case tok::kw__Static_assert:
diff --git a/clang/lib/Parse/ParseStmt.cpp b/clang/lib/Parse/ParseStmt.cpp
index 623b4c799327c..150b2879fc94f 100644
--- a/clang/lib/Parse/ParseStmt.cpp
+++ b/clang/lib/Parse/ParseStmt.cpp
@@ -126,18 +126,15 @@ Parser::ParseStatementOrDeclaration(StmtVector &Stmts,
Stmts, StmtCtx, TrailingElseLoc, CXX11Attrs, GNUOrMSAttrs);
MaybeDestroyTemplateIds();
- // Attributes that are left should all go on the statement, so concatenate the
- // two lists.
- ParsedAttributes Attrs(AttrFactory);
- takeAndConcatenateAttrs(CXX11Attrs, GNUOrMSAttrs, Attrs);
+ takeAndConcatenateAttrs(CXX11Attrs, std::move(GNUOrMSAttrs));
- assert((Attrs.empty() || Res.isInvalid() || Res.isUsable()) &&
+ assert((CXX11Attrs.empty() || Res.isInvalid() || Res.isUsable()) &&
"attributes on empty statement");
- if (Attrs.empty() || Res.isInvalid())
+ if (CXX11Attrs.empty() || Res.isInvalid())
return Res;
- return Actions.ActOnAttributedStmt(Attrs, Res.get());
+ return Actions.ActOnAttributedStmt(CXX11Attrs, Res.get());
}
namespace {
@@ -207,11 +204,10 @@ StmtResult Parser::ParseStatementOrDeclarationAfterAttributes(
// Both C++11 and GNU attributes preceding the label appertain to the
// label, so put them in a single list to pass on to
// ParseLabeledStatement().
- ParsedAttributes Attrs(AttrFactory);
- takeAndConcatenateAttrs(CXX11Attrs, GNUAttrs, Attrs);
+ takeAndConcatenateAttrs(CXX11Attrs, std::move(GNUAttrs));
// identifier ':' statement
- return ParseLabeledStatement(Attrs, StmtCtx);
+ return ParseLabeledStatement(CXX11Attrs, StmtCtx);
}
// Look up the identifier, and typo-correct it to a keyword if it's not
@@ -302,9 +298,7 @@ StmtResult Parser::ParseStatementOrDeclarationAfterAttributes(
case tok::kw_template: {
SourceLocation DeclEnd;
- ParsedAttributes Attrs(AttrFactory);
ParseTemplateDeclarationOrSpecialization(DeclaratorContext::Block, DeclEnd,
- Attrs,
getAccessSpecifierIfPresent());
return StmtError();
}
@@ -1057,7 +1051,11 @@ StmtResult Parser::ParseCompoundStatement(bool isStmtExpr,
ParseScope CompoundScope(this, ScopeFlags);
// Parse the statements in the body.
- return ParseCompoundStatementBody(isStmtExpr);
+ StmtResult R;
+ StackHandler.runWithSufficientStackSpace(Tok.getLocation(), [&, this]() {
+ R = ParseCompoundStatementBody(isStmtExpr);
+ });
+ return R;
}
/// Parse any pragmas at the start of the compound expression. We handle these
@@ -1222,7 +1220,7 @@ StmtResult Parser::ParseCompoundStatementBody(bool isStmtExpr) {
while (Tok.is(tok::kw___label__)) {
SourceLocation LabelLoc = ConsumeToken();
- SmallVector<Decl *, 8> DeclsInGroup;
+ SmallVector<Decl *, 4> DeclsInGroup;
while (true) {
if (Tok.isNot(tok::identifier)) {
Diag(Tok, diag::err_expected) << tok::identifier;
diff --git a/clang/lib/Parse/ParseTemplate.cpp b/clang/lib/Parse/ParseTemplate.cpp
index b2cb3d71e09f9..e8b4aee8c2686 100644
--- a/clang/lib/Parse/ParseTemplate.cpp
+++ b/clang/lib/Parse/ParseTemplate.cpp
@@ -179,6 +179,13 @@ Parser::DeclGroupPtrTy Parser::ParseTemplateDeclarationOrSpecialization(
Context, TemplateInfo, ParsingTemplateParams, DeclEnd, AccessAttrs, AS);
}
+Parser::DeclGroupPtrTy Parser::ParseTemplateDeclarationOrSpecialization(
+ DeclaratorContext Context, SourceLocation &DeclEnd, AccessSpecifier AS) {
+ ParsedAttributes AccessAttrs(AttrFactory);
+ return ParseTemplateDeclarationOrSpecialization(Context, DeclEnd, AccessAttrs,
+ AS);
+}
+
/// Parse a single declaration that declares a template,
/// template specialization, or explicit instantiation of a template.
///
diff --git a/clang/lib/Parse/Parser.cpp b/clang/lib/Parse/Parser.cpp
index 83dd7b17c73b8..6128bc58d69c0 100644
--- a/clang/lib/Parse/Parser.cpp
+++ b/clang/lib/Parse/Parser.cpp
@@ -17,6 +17,7 @@
#include "clang/AST/DeclTemplate.h"
#include "clang/Basic/DiagnosticParse.h"
#include "clang/Basic/FileManager.h"
+#include "clang/Basic/StackExhaustionHandler.h"
#include "clang/Parse/RAIIObjectsForParser.h"
#include "clang/Sema/DeclSpec.h"
#include "clang/Sema/EnterExpressionEvaluationContext.h"
@@ -54,9 +55,10 @@ IdentifierInfo *Parser::getSEHExceptKeyword() {
Parser::Parser(Preprocessor &pp, Sema &actions, bool skipFunctionBodies)
: PP(pp), PreferredType(pp.isCodeCompletionEnabled()), Actions(actions),
- Diags(PP.getDiagnostics()), GreaterThanIsOperator(true),
- ColonIsSacred(false), InMessageExpression(false),
- TemplateParameterDepth(0), ParsingInObjCContainer(false) {
+ Diags(PP.getDiagnostics()), StackHandler(Diags),
+ GreaterThanIsOperator(true), ColonIsSacred(false),
+ InMessageExpression(false), TemplateParameterDepth(0),
+ ParsingInObjCContainer(false) {
SkipFunctionBodies = pp.isCodeCompletionEnabled() || skipFunctionBodies;
Tok.startToken();
Tok.setKind(tok::eof);
diff --git a/clang/lib/Sema/ParsedAttr.cpp b/clang/lib/Sema/ParsedAttr.cpp
index eadc6ed6e6f58..b19a02b8c1a09 100644
--- a/clang/lib/Sema/ParsedAttr.cpp
+++ b/clang/lib/Sema/ParsedAttr.cpp
@@ -310,18 +310,13 @@ bool ParsedAttr::checkAtMostNumArgs(Sema &S, unsigned Num) const {
}
void clang::takeAndConcatenateAttrs(ParsedAttributes &First,
- ParsedAttributes &Second,
- ParsedAttributes &Result) {
- // Note that takeAllFrom() puts the attributes at the beginning of the list,
- // so to obtain the correct ordering, we add `Second`, then `First`.
- Result.takeAllFrom(Second);
- Result.takeAllFrom(First);
- if (First.Range.getBegin().isValid())
- Result.Range.setBegin(First.Range.getBegin());
- else
- Result.Range.setBegin(Second.Range.getBegin());
+ ParsedAttributes &&Second) {
+
+ First.takeAllAtEndFrom(Second);
+
+ if (!First.Range.getBegin().isValid())
+ First.Range.setBegin(Second.Range.getBegin());
+
if (Second.Range.getEnd().isValid())
- Result.Range.setEnd(Second.Range.getEnd());
- else
- Result.Range.setEnd(First.Range.getEnd());
+ First.Range.setEnd(Second.Range.getEnd());
}
diff --git a/clang/test/Parser/parser_overflow.c b/clang/test/Parser/parser_overflow.c
index 9514e808550a4..53c79bc06d993 100644
--- a/clang/test/Parser/parser_overflow.c
+++ b/clang/test/Parser/parser_overflow.c
@@ -1,5 +1,5 @@
// RUN: not %clang_cc1 %s -fsyntax-only -DHUGE 2>&1 | FileCheck %s
-// RUN: not %clang_cc1 %s -fsyntax-only 2>&1 | FileCheck %s
+// RUN: %clang_cc1 %s -fsyntax-only
// RUN: not %clang_cc1 %s -fsyntax-only -fbracket-depth 299 2>&1 | FileCheck %s
// RUN: %clang_cc1 %s -fsyntax-only -fbracket-depth 300
// RUN: not %clang %s -fsyntax-only -fbracket-depth=299 2>&1 | FileCheck %s
@@ -15,5 +15,5 @@ void foo(void) {
#endif
}
-// CHECK: fatal error: bracket nesting level exceeded maximum of {{256|299}}
+// CHECK: fatal error: bracket nesting level exceeded maximum of {{2048|299}}
// CHECK: note: use -fbracket-depth=N to increase maximum nesting level
|
|
Note this doesn't appear to impact performance negatively https://llvm-compile-time-tracker.com/compare.php?from=33e5d013b7f7a6ae136a058f842b30c87623ecfb&to=9e96b0f696f2478cdfeccec0c349b973e96f1432&stat=instructions%3Au |
erichkeane
reviewed
Mar 19, 2025
| @@ -169,6 +169,8 @@ Modified Compiler Flags | |||
| the behavior of ``-mtp`` in gcc. This changes the default behavior for ARM targets that provide the ``TPIDRURO`` register as this will be used instead of a call to the ``__aeabi_read_tp``. | |||
| Programs that use ``__aeabi_read_tp`` but do not use the ``TPIDRURO`` register must use ``-mtp=soft``. Fixes #123864 | |||
|
|
|||
| - The compiler flag `-fbracket-depth` default value is increased from 256 to 2048. (#GH94728) | |||
There was a problem hiding this comment.
Wow, this is an aggressive change :)
erichkeane
approved these changes
Mar 19, 2025
|
Thanks @erichkeane. Let see if the bots scream |
shafik
reviewed
Mar 19, 2025
There was a problem hiding this comment.
I think you need to update this too, unless I missed something: https://clang.llvm.org/docs/UsersManual.html#controlling-implementation-limits
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This iterates on #104717 (which we had to revert)
In a bid to increase our chances of success, we try to avoid blowing up the stack by
runWithSufficientStackSpacein ParseCompoundStatementStmtVectora bitDeclsInGroupa bitParsedAttributesfrom the stacks in places where they are not strictly necessary.ParsedAttributesis a huge objectOn a 64 bits system, the following stack size reductions are observed
Fixes #94728