fix stack overflow on deeply nested parens [SR-4866] (swiftlang#19631)

Make sure StructureMarkerRAII checks structure nesting level on all paths.  Previously swift crashed with  no diagnostic on deeply nested '('. Now we print an error when more than 256 parens deep, just as we always have for '['.

fixes SR-4866

Author: modelorganism

include/swift/Parse/Parser.h

@@ -306,30 +306,34 @@ class Parser {
 
   /// An RAII object that notes when we have seen a structure marker.
   class StructureMarkerRAII {
-    Parser &P;
+    Parser *const P;
 
     /// Max nesting level
     // TODO: customizable.
     enum { MaxDepth = 256 };
 
-    void diagnoseOverflow();
+    StructureMarkerRAII(Parser *parser) : P(parser) {}
+
+    /// Have the parser start the new Structure or fail if already too deep.
+    bool pushStructureMarker(Parser &parser, SourceLoc loc,
+                             StructureMarkerKind kind);
 
   public:
-    StructureMarkerRAII(Parser &parser, SourceLoc loc,
-                               StructureMarkerKind kind)
-      : P(parser) {
-      P.StructureMarkers.push_back({loc, kind, None});
-
-      if (P.StructureMarkers.size() >= MaxDepth) {
-        diagnoseOverflow();
-        P.cutOffParsing();
-      }
-    }
+    StructureMarkerRAII(Parser &parser, SourceLoc loc, StructureMarkerKind kind)
+        : StructureMarkerRAII(
+              pushStructureMarker(parser, loc, kind) ? &parser : nullptr) {}
 
     StructureMarkerRAII(Parser &parser, const Token &tok);
 
+    /// Did we fail to push the new structure?
+    bool isFailed() {
+      return P == nullptr;
+    }
+
     ~StructureMarkerRAII() {
-      P.StructureMarkers.pop_back();
+      if (P != nullptr) {
+        P->StructureMarkers.pop_back();
+      }
     }
   };
   friend class StructureMarkerRAII;

lib/Parse/ParseExpr.cpp

@@ -2888,6 +2888,10 @@ ParserStatus Parser::parseExprList(tok leftTok, tok rightTok,
   trailingClosure = nullptr;
 
   StructureMarkerRAII ParsingExprList(*this, Tok);
+  
+  if (ParsingExprList.isFailed()) {
+    return makeParserError();
+  }
 
   leftLoc = consumeToken(leftTok);
   ParserStatus status = parseList(rightTok, leftLoc, rightLoc,

lib/Parse/ParseGeneric.cpp

@@ -55,6 +55,10 @@ Parser::parseGenericParametersBeforeWhere(SourceLoc LAngleLoc,
     // Note that we're parsing a declaration.
     StructureMarkerRAII ParsingDecl(*this, Tok.getLoc(),
                                     StructureMarkerKind::Declaration);
+    
+    if (ParsingDecl.isFailed()) {
+      return makeParserError();
+    }
 
     // Parse attributes.
     DeclAttributes attributes;

lib/Parse/ParsePattern.cpp

@@ -1038,6 +1038,9 @@ ParserResult<Pattern> Parser::parsePatternTuple() {
   SyntaxParsingContext TuplePatternCtxt(SyntaxContext,
                                         SyntaxKind::TuplePattern);
   StructureMarkerRAII ParsingPatternTuple(*this, Tok);
+  if (ParsingPatternTuple.isFailed()) {
+    return makeParserError();
+  }
   SourceLoc LPLoc = consumeToken(tok::l_paren);
   SourceLoc RPLoc;
 

lib/Parse/ParseStmt.cpp

@@ -1250,6 +1250,11 @@ ParserResult<PoundAvailableInfo> Parser::parseStmtConditionPoundAvailable() {
   }
 
   StructureMarkerRAII ParsingAvailabilitySpecList(*this, Tok);
+
+  if (ParsingAvailabilitySpecList.isFailed()) {
+    return makeParserError();
+  }
+    
   SourceLoc LParenLoc = consumeToken(tok::l_paren);
 
   SmallVector<AvailabilitySpec *, 5> Specs;

lib/Parse/ParseType.cpp

@@ -852,6 +852,11 @@ ParserResult<TypeRepr> Parser::parseOldStyleProtocolComposition() {
 ParserResult<TupleTypeRepr> Parser::parseTypeTupleBody() {
   SyntaxParsingContext TypeContext(SyntaxContext, SyntaxKind::TupleType);
   Parser::StructureMarkerRAII ParsingTypeTuple(*this, Tok);
+   
+  if (ParsingTypeTuple.isFailed()) {
+    return makeParserError();
+  }
+
   SourceLoc RPLoc, LPLoc = consumeToken(tok::l_paren);
   SourceLoc EllipsisLoc;
   unsigned EllipsisIdx;

lib/Parse/Parser.cpp

@@ -757,39 +757,42 @@ bool Parser::parseEndIfDirective(SourceLoc &Loc) {
   return false;
 }
 
-Parser::StructureMarkerRAII::StructureMarkerRAII(Parser &parser,
-                                                 const Token &tok)
-  : P(parser)
-{
+static Parser::StructureMarkerKind
+getStructureMarkerKindForToken(const Token &tok) {
   switch (tok.getKind()) {
   case tok::l_brace:
-    P.StructureMarkers.push_back({tok.getLoc(),
-                                  StructureMarkerKind::OpenBrace,
-                                  None});
-    break;
-
+    return Parser::StructureMarkerKind::OpenBrace;
   case tok::l_paren:
-    P.StructureMarkers.push_back({tok.getLoc(),
-                                  StructureMarkerKind::OpenParen,
-                                  None});
-    break;
-
+    return Parser::StructureMarkerKind::OpenParen;
   case tok::l_square:
-    P.StructureMarkers.push_back({tok.getLoc(),
-                                  StructureMarkerKind::OpenSquare,
-                                  None});
-    break;
-
+    return Parser::StructureMarkerKind::OpenSquare;
   default:
     llvm_unreachable("Not a matched token");
   }
 }
 
-void Parser::StructureMarkerRAII::diagnoseOverflow() {
-  auto Loc = P.StructureMarkers.back().Loc;
-  P.diagnose(Loc, diag::structure_overflow, MaxDepth);
-}
+Parser::StructureMarkerRAII::StructureMarkerRAII(Parser &parser,
+                                                 const Token &tok)
+    : StructureMarkerRAII(parser, tok.getLoc(),
+                          getStructureMarkerKindForToken(tok)) {}
 
+bool Parser::StructureMarkerRAII::pushStructureMarker(
+                                      Parser &parser, SourceLoc loc,    
+                                      StructureMarkerKind kind) {
+  
+  if (parser.StructureMarkers.size() < MaxDepth) {
+    parser.StructureMarkers.push_back({loc, kind, None});
+    return true;
+  } else {
+    parser.diagnose(loc, diag::structure_overflow, MaxDepth);
+    // We need to cut off parsing or we will stack-overflow.
+    // But `cutOffParsing` changes the current token to eof, and we may be in
+    // a place where `consumeToken()` will be expecting e.g. '[',
+    // since we need that to get to the callsite, so this can cause an assert.
+    parser.cutOffParsing();
+    return false;
+  }
+}
 //===----------------------------------------------------------------------===//
 // Primitive Parsing
 //===----------------------------------------------------------------------===//

test/Parse/structure_no_overflow.swift

@@ -0,0 +1,37 @@
+// RUN: %target-swift-frontend -parse %s 2>&1 | %FileCheck -allow-empty %s
+
+// Test that we can have the allowed # of parens.
+// CHECK-NOT: error
+let a = (((((((((((((((((((((((((((((((( ((((((((((((((((((((((((((((((((
+        (((((((((((((((((((((((((((((((( ((((((((((((((((((((((((((((((((
+        (((((((((((((((((((((((((((((((( ((((((((((((((((((((((((((((((((
+        (((((((((((((((((((((((((((((((( ((((((((((((((((((((((((((((((
+        1
+          )))))))))))))))))))))))))))))) ))))))))))))))))))))))))))))))))
+        )))))))))))))))))))))))))))))))) ))))))))))))))))))))))))))))))))
+        )))))))))))))))))))))))))))))))) ))))))))))))))))))))))))))))))))
+        )))))))))))))))))))))))))))))))) ))))))))))))))))))))))))))))))))
+
+// Test that we can have the allowed # of curly braces.
+/// CHECK-NOT: error
+let b = {{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{ {{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{
+        {{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{ {{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{
+        {{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{ {{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{
+        {{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{ {{{{{{{{{{{{{{{{{{{{{{{{{{{{{{
+        1
+          }}}}}}}}}}}}}}}}}}}}}}}}}}}}}} }}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}
+        }}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}} }}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}
+        }}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}} }}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}
+        }}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}} }}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}
+
+// Test that we can have the allowed # of mixed brackets.
+// CHECK-NOT: error
+let c = ({({({({ ({({({([ ({({({({ ({({({([  ({({({({ ({({({([ ({({({({ ({({({({
+        ({({({({ ({({({([ ({({({({ ({({({([  ({({({({ ({({({([ ({({({({ ({({({({
+        ({({({({ ({({({([ ({({({({ ({({({([  ({({({({ ({({({([ ({({({({ ({({({({
+        ({({({({ ({({({([ ({({({({ ({({({([  ({({({({ ({({({([ ({({({({ ({({({
+        1
+          })})}) })})})}) ])})})}) })})})})  ])})})}) })})})}) ])})})}) })})})}) 
+        })})})}) })})})}) ])})})}) })})})})  ])})})}) })})})}) ])})})}) })})})}) 
+        })})})}) })})})}) ])})})}) })})})})  ])})})}) })})})}) ])})})}) })})})}) 
+        })})})}) })})})}) ])})})}) })})})})  ])})})}) })})})}) ])})})}) })})})})