Merge pull request #2911 from ahoppen/fix-recovery-stack-overflow

Fix a stack overflow when using `canRecoveryTo` with too many open delimiters

Author: ahoppen

Sources/SwiftParser/Recovery.swift

@@ -61,17 +61,19 @@ struct RecoveryConsumptionHandle {
 extension Parser.Lookahead {
   /// See `canRecoverTo` that takes 3 specs.
   mutating func canRecoverTo(
-    _ spec: TokenSpec
+    _ spec: TokenSpec,
+    recursionDepth: Int = 1
   ) -> RecoveryConsumptionHandle? {
-    return canRecoverTo(spec, spec, spec)
+    return canRecoverTo(spec, spec, spec, recursionDepth: recursionDepth)
   }
 
   /// See `canRecoverTo` that takes 3 specs.
   mutating func canRecoverTo(
     _ spec1: TokenSpec,
-    _ spec2: TokenSpec
+    _ spec2: TokenSpec,
+    recursionDepth: Int = 1
   ) -> RecoveryConsumptionHandle? {
-    return canRecoverTo(spec1, spec2, spec1)
+    return canRecoverTo(spec1, spec2, spec1, recursionDepth: recursionDepth)
   }
 
   /// Tries eating tokens until it finds a token that matches `spec1`, `spec2` or `spec3`
@@ -84,8 +86,18 @@ extension Parser.Lookahead {
   mutating func canRecoverTo(
     _ spec1: TokenSpec,
     _ spec2: TokenSpec,
-    _ spec3: TokenSpec
+    _ spec3: TokenSpec,
+    recursionDepth: Int = 1
   ) -> RecoveryConsumptionHandle? {
+    if recursionDepth > 10 {
+      // `canRecoverTo` calls itself recursively if it finds a nested opening token, eg. when calling `canRecoverTo` on
+      // `{{{`. To avoid stack overflowing, limit the number of nested `canRecoverTo` calls we make. Since returning a
+      // recovery handle from this function only improves error recovery but is not necessary for correctness, bailing
+      // from recovery is safe.
+      // The value 10 was chosen fairly arbitrarily. It seems unlikely that we get useful recovery if we find more than
+      // 10 nested open and closing delimiters.
+      return nil
+    }
     #if SWIFTPARSER_ENABLE_ALTERNATE_TOKEN_INTROSPECTION
     if shouldRecordAlternativeTokenChoices {
       recordAlternativeTokenChoice(for: self.currentToken, choices: [spec1, spec2, spec3])
@@ -144,7 +156,7 @@ extension Parser.Lookahead {
           continue
         }
         self.consumeAnyToken()
-        guard self.canRecoverTo(closingDelimiterSpec) != nil else {
+        guard self.canRecoverTo(closingDelimiterSpec, recursionDepth: recursionDepth + 1) != nil else {
           continue
         }
         self.eat(closingDelimiterSpec)

Tests/SwiftParserTest/ParserTests.swift

@@ -125,7 +125,6 @@ class ParserTests: ParserTestCase {
   /// Swift compiler. This requires the Swift compiler to have been checked
   /// out into the "swift" directory alongside swift-syntax.
   func testSwiftValidationTestsuite() throws {
-    try XCTSkipIf(true, "Crashing with signal 10 in CI")
     try XCTSkipIf(longTestsDisabled)
     let testDir =
       packageDir