Skip to content

Commit 2695c24

Browse files
Fix exponential blowup parsing pathological files
1 parent c2a1ed2 commit 2695c24

File tree

5 files changed

+1388
-8
lines changed

5 files changed

+1388
-8
lines changed

src/Compilers/CSharp/Portable/Parser/LanguageParser.cs

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -906,10 +906,25 @@ private bool IsPossibleAttributeDeclaration()
906906

907907
private SyntaxList<AttributeListSyntax> ParseAttributeDeclarations(bool inExpressionContext)
908908
{
909-
var attributes = _pool.Allocate<AttributeListSyntax>();
910909
var saveTerm = _termState;
911910
_termState |= TerminatorState.IsAttributeDeclarationTerminator;
912911

912+
// An attribute can never appear *inside* an attribute argument (since a lambda expression cannot be used as
913+
// a constant argument value). However, during parsing we can end up in a state where we're trying to
914+
// exactly that, through a path of Attribute->Argument->Expression->Attribute (since attributes can not be
915+
// on lambda expressions).
916+
//
917+
// Worse, when we are in a deeply ambiguous (or gibberish) scenario, where we see lots of code with `... [
918+
// ... [ ... ] ... ] ...`, we can get into exponential speculative parsing where we try `[ ... ]` both as an
919+
// attribute *and* a collection expression.
920+
//
921+
// Since we cannot ever legally have an attribute within an attribute, we bail out here immediately
922+
// syntactically. This does mean we won't parse something like: `[X([Y]() => {})]` without errors, but that
923+
// is not semantically legal anyway.
924+
if (saveTerm == _termState)
925+
return default;
926+
927+
var attributes = _pool.Allocate<AttributeListSyntax>();
913928
while (this.IsPossibleAttributeDeclaration())
914929
{
915930
var attributeDeclaration = this.TryParseAttributeDeclaration(inExpressionContext);
@@ -11868,7 +11883,18 @@ private bool ScanExplicitlyTypedLambda(Precedence precedence)
1186811883
// If we have an `=` then parse out a default value. Note: this is not legal, but this allows us to
1186911884
// to be resilient to the user writing this so we don't go completely off the rails.
1187011885
if (equalsToken != null)
11886+
{
11887+
// Note: we don't do this if we have `=[`. Realistically, this is never going to be a lambda
11888+
// expression as a `[` can only start an attribute declaration or collection expression, neither of
11889+
// which can be a default arg. Checking for this helps us from going off the rails in pathological
11890+
// cases with lots of nested tokens that look like the could be anything.
11891+
if (this.CurrentToken.Kind == SyntaxKind.OpenBracketToken)
11892+
{
11893+
return false;
11894+
}
11895+
1187111896
this.ParseExpressionCore();
11897+
}
1187211898

1187311899
switch (this.CurrentToken.Kind)
1187411900
{

src/Compilers/CSharp/Test/Emit2/Semantics/OutVarTests.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36336,9 +36336,9 @@ public MyAttribute(string name1) { }
3633636336

3633736337
var comp = CreateCompilation(source);
3633836338
comp.VerifyDiagnostics(
36339-
// (6,16): error CS1660: Cannot convert lambda expression to type 'string' because it is not a delegate type
36339+
// (6,41): error CS1002: ; expected
3634036340
// [My(() => { [My(M2(out var x))] static string M2(out int x) => throw null; })]
36341-
Diagnostic(ErrorCode.ERR_AnonMethToNonDel, "=>").WithArguments("lambda expression", "string").WithLocation(6, 16),
36341+
Diagnostic(ErrorCode.ERR_SemicolonExpected, "static").WithLocation(6, 41),
3634236342
// (7,14): warning CS8321: The local function 'local' is declared but never used
3634336343
// void local(int parameter) { }
3634436344
Diagnostic(ErrorCode.WRN_UnreferencedLocalFunction, "local").WithArguments("local").WithLocation(7, 14));
@@ -36350,7 +36350,7 @@ public MyAttribute(string name1) { }
3635036350
var method = tree2.GetRoot().DescendantNodes().OfType<MethodDeclarationSyntax>().First();
3635136351
Assert.True(model.TryGetSpeculativeSemanticModelForMethodBody(method.Body.SpanStart, method, out var speculativeModel));
3635236352

36353-
var invocation = tree2.GetRoot().DescendantNodes().OfType<InvocationExpressionSyntax>().Single();
36353+
var invocation = tree2.GetRoot().DescendantNodes().OfType<InvocationExpressionSyntax>().Skip(1).First();
3635436354
Assert.Equal("M2(out var x)", invocation.ToString());
3635536355
var symbolInfo = speculativeModel.GetSymbolInfo(invocation);
3635636356
Assert.Equal("System.String M2(out System.Int32 x)", symbolInfo.Symbol.ToTestDisplayString());

src/Compilers/CSharp/Test/Semantic/Semantics/LambdaTests.cs

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3809,9 +3809,21 @@ class BAttribute : Attribute
38093809
}";
38103810
var comp = CreateCompilation(source, parseOptions: TestOptions.RegularPreview);
38113811
comp.VerifyDiagnostics(
3812-
// (6,2): error CS0181: Attribute constructor parameter 'a' has type 'Action', which is not a valid attribute parameter type
3812+
// (6,11): error CS1003: Syntax error, ',' expected
38133813
// [A([B] () => { })]
3814-
Diagnostic(ErrorCode.ERR_BadAttributeParamType, "A").WithArguments("a", "System.Action").WithLocation(6, 2));
3814+
Diagnostic(ErrorCode.ERR_SyntaxError, "=>").WithArguments(",").WithLocation(6, 11),
3815+
// (6,16): error CS1026: ) expected
3816+
// [A([B] () => { })]
3817+
Diagnostic(ErrorCode.ERR_CloseParenExpected, "}").WithLocation(6, 16),
3818+
// (6,16): error CS1003: Syntax error, ']' expected
3819+
// [A([B] () => { })]
3820+
Diagnostic(ErrorCode.ERR_SyntaxError, "}").WithArguments("]").WithLocation(6, 16),
3821+
// (6,16): error CS1022: Type or namespace definition, or end-of-file expected
3822+
// [A([B] () => { })]
3823+
Diagnostic(ErrorCode.ERR_EOFExpected, "}").WithLocation(6, 16),
3824+
// (6,17): error CS1022: Type or namespace definition, or end-of-file expected
3825+
// [A([B] () => { })]
3826+
Diagnostic(ErrorCode.ERR_EOFExpected, ")").WithLocation(6, 17));
38153827
}
38163828

38173829
[Fact]

0 commit comments

Comments
 (0)