Skip to content

Commit

Permalink
Fix exponential blowup parsing pathological files
Browse files Browse the repository at this point in the history
  • Loading branch information
CyrusNajmabadi committed Jun 3, 2024
1 parent c2a1ed2 commit 2695c24
Show file tree
Hide file tree
Showing 5 changed files with 1,388 additions and 8 deletions.
28 changes: 27 additions & 1 deletion src/Compilers/CSharp/Portable/Parser/LanguageParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -906,10 +906,25 @@ private bool IsPossibleAttributeDeclaration()

private SyntaxList<AttributeListSyntax> ParseAttributeDeclarations(bool inExpressionContext)
{
var attributes = _pool.Allocate<AttributeListSyntax>();
var saveTerm = _termState;
_termState |= TerminatorState.IsAttributeDeclarationTerminator;

// An attribute can never appear *inside* an attribute argument (since a lambda expression cannot be used as
// a constant argument value). However, during parsing we can end up in a state where we're trying to
// exactly that, through a path of Attribute->Argument->Expression->Attribute (since attributes can not be
// on lambda expressions).
//
// Worse, when we are in a deeply ambiguous (or gibberish) scenario, where we see lots of code with `... [
// ... [ ... ] ... ] ...`, we can get into exponential speculative parsing where we try `[ ... ]` both as an
// attribute *and* a collection expression.
//
// Since we cannot ever legally have an attribute within an attribute, we bail out here immediately
// syntactically. This does mean we won't parse something like: `[X([Y]() => {})]` without errors, but that
// is not semantically legal anyway.
if (saveTerm == _termState)
return default;

var attributes = _pool.Allocate<AttributeListSyntax>();
while (this.IsPossibleAttributeDeclaration())
{
var attributeDeclaration = this.TryParseAttributeDeclaration(inExpressionContext);
Expand Down Expand Up @@ -11868,7 +11883,18 @@ private bool ScanExplicitlyTypedLambda(Precedence precedence)
// If we have an `=` then parse out a default value. Note: this is not legal, but this allows us to
// to be resilient to the user writing this so we don't go completely off the rails.
if (equalsToken != null)
{
// Note: we don't do this if we have `=[`. Realistically, this is never going to be a lambda
// expression as a `[` can only start an attribute declaration or collection expression, neither of
// which can be a default arg. Checking for this helps us from going off the rails in pathological
// cases with lots of nested tokens that look like the could be anything.
if (this.CurrentToken.Kind == SyntaxKind.OpenBracketToken)
{
return false;
}

this.ParseExpressionCore();
}

switch (this.CurrentToken.Kind)
{
Expand Down
6 changes: 3 additions & 3 deletions src/Compilers/CSharp/Test/Emit2/Semantics/OutVarTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -36336,9 +36336,9 @@ public MyAttribute(string name1) { }

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

var invocation = tree2.GetRoot().DescendantNodes().OfType<InvocationExpressionSyntax>().Single();
var invocation = tree2.GetRoot().DescendantNodes().OfType<InvocationExpressionSyntax>().Skip(1).First();
Assert.Equal("M2(out var x)", invocation.ToString());
var symbolInfo = speculativeModel.GetSymbolInfo(invocation);
Assert.Equal("System.String M2(out System.Int32 x)", symbolInfo.Symbol.ToTestDisplayString());
Expand Down
16 changes: 14 additions & 2 deletions src/Compilers/CSharp/Test/Semantic/Semantics/LambdaTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3809,9 +3809,21 @@ class BAttribute : Attribute
}";
var comp = CreateCompilation(source, parseOptions: TestOptions.RegularPreview);
comp.VerifyDiagnostics(
// (6,2): error CS0181: Attribute constructor parameter 'a' has type 'Action', which is not a valid attribute parameter type
// (6,11): error CS1003: Syntax error, ',' expected
// [A([B] () => { })]
Diagnostic(ErrorCode.ERR_BadAttributeParamType, "A").WithArguments("a", "System.Action").WithLocation(6, 2));
Diagnostic(ErrorCode.ERR_SyntaxError, "=>").WithArguments(",").WithLocation(6, 11),
// (6,16): error CS1026: ) expected
// [A([B] () => { })]
Diagnostic(ErrorCode.ERR_CloseParenExpected, "}").WithLocation(6, 16),
// (6,16): error CS1003: Syntax error, ']' expected
// [A([B] () => { })]
Diagnostic(ErrorCode.ERR_SyntaxError, "}").WithArguments("]").WithLocation(6, 16),
// (6,16): error CS1022: Type or namespace definition, or end-of-file expected
// [A([B] () => { })]
Diagnostic(ErrorCode.ERR_EOFExpected, "}").WithLocation(6, 16),
// (6,17): error CS1022: Type or namespace definition, or end-of-file expected
// [A([B] () => { })]
Diagnostic(ErrorCode.ERR_EOFExpected, ")").WithLocation(6, 17));
}

[Fact]
Expand Down
Loading

0 comments on commit 2695c24

Please sign in to comment.