[Parser][ObjC] Fix crash on nested top-level block with better recove…

…ry path

Delay consuming tokens until we are certain that the next token is not top
level block. Otherwise we bail out as if we saw an @EnD for better diagnostic
and recovery.

Fixes #64065.

Reviewed By: rjmccall

Differential Revision: https://reviews.llvm.org/D156277.

clang/docs/ReleaseNotes.rst

@@ -146,6 +146,10 @@ Miscellaneous Bug Fixes
 
 Miscellaneous Clang Crashes Fixed
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+- Fixed a crash when parsing top-level ObjC blocks that aren't properly
+  terminated. Clang should now also recover better when an @end is missing
+  between blocks.
+  `Issue 64065 <https://github.com/llvm/llvm-project/issues/64065>`_
 
 Target Specific Changes
 -----------------------

clang/lib/Parse/ParseObjc.cpp

@@ -613,6 +613,19 @@ ObjCTypeParamList *Parser::parseObjCTypeParamList() {
                                               /*mayBeProtocolList=*/false);
 }
 
+static bool isTopLevelObjCKeyword(tok::ObjCKeywordKind DirectiveKind) {
+  switch (DirectiveKind) {
+  case tok::objc_class:
+  case tok::objc_compatibility_alias:
+  case tok::objc_interface:
+  case tok::objc_implementation:
+  case tok::objc_protocol:
+    return true;
+  default:
+    return false;
+  }
+}
+
 ///   objc-interface-decl-list:
 ///     empty
 ///     objc-interface-decl-list objc-property-decl [OBJC2]
@@ -705,27 +718,34 @@ void Parser::ParseObjCInterfaceDeclList(tok::ObjCKeywordKind contextKey,
       continue;
     }
 
-    // Otherwise, we have an @ directive, eat the @.
-    SourceLocation AtLoc = ConsumeToken(); // the "@"
-    if (Tok.is(tok::code_completion)) {
+    // Otherwise, we have an @ directive, peak at the next token
+    SourceLocation AtLoc = Tok.getLocation();
+    const auto &NextTok = NextToken();
+    if (NextTok.is(tok::code_completion)) {
       cutOffParsing();
       Actions.CodeCompleteObjCAtDirective(getCurScope());
       return;
     }
 
-    tok::ObjCKeywordKind DirectiveKind = Tok.getObjCKeywordID();
-
+    tok::ObjCKeywordKind DirectiveKind = NextTok.getObjCKeywordID();
     if (DirectiveKind == tok::objc_end) { // @end -> terminate list
+      ConsumeToken();                     // the "@"
       AtEnd.setBegin(AtLoc);
       AtEnd.setEnd(Tok.getLocation());
       break;
     } else if (DirectiveKind == tok::objc_not_keyword) {
-      Diag(Tok, diag::err_objc_unknown_at);
+      Diag(NextTok, diag::err_objc_unknown_at);
       SkipUntil(tok::semi);
       continue;
     }
 
-    // Eat the identifier.
+    // If we see something like '@interface' that's only allowed at the top
+    // level, bail out as if we saw an '@end'. We'll diagnose this below.
+    if (isTopLevelObjCKeyword(DirectiveKind))
+      break;
+
+    // Otherwise parse it as part of the current declaration. Eat "@identifier".
+    ConsumeToken();
     ConsumeToken();
 
     switch (DirectiveKind) {
@@ -739,15 +759,6 @@ void Parser::ParseObjCInterfaceDeclList(tok::ObjCKeywordKind contextKey,
       SkipUntil(tok::r_brace, tok::at, StopAtSemi);
       break;
 
-    case tok::objc_implementation:
-    case tok::objc_interface:
-      Diag(AtLoc, diag::err_objc_missing_end)
-          << FixItHint::CreateInsertion(AtLoc, "@end\n");
-      Diag(CDecl->getBeginLoc(), diag::note_objc_container_start)
-          << (int)Actions.getObjCContainerKind();
-      ConsumeToken();
-      break;
-
     case tok::objc_required:
     case tok::objc_optional:
       // This is only valid on protocols.
@@ -816,13 +827,10 @@ void Parser::ParseObjCInterfaceDeclList(tok::ObjCKeywordKind contextKey,
     }
   }
 
-  // We break out of the big loop in two cases: when we see @end or when we see
-  // EOF.  In the former case, eat the @end.  In the later case, emit an error.
-  if (Tok.is(tok::code_completion)) {
-    cutOffParsing();
-    Actions.CodeCompleteObjCAtDirective(getCurScope());
-    return;
-  } else if (Tok.isObjCAtKeyword(tok::objc_end)) {
+  // We break out of the big loop in 3 cases: when we see @end or when we see
+  // top-level ObjC keyword or EOF. In the former case, eat the @end. In the
+  // later cases, emit an error.
+  if (Tok.isObjCAtKeyword(tok::objc_end)) {
     ConsumeToken(); // the "end" identifier
   } else {
     Diag(Tok, diag::err_objc_missing_end)

clang/test/Parser/missing-end-1-gh64065-nocrash.m

@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+@interface Roo // expected-note {{class started here}}
+// expected-error@+1 {{missing '@end'}}
+@interface // expected-error {{expected identifier}}

clang/test/Parser/missing-end-2.m

@@ -1,9 +1,10 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wno-objc-root-class -verify %s
 // rdar: //7824372
 
 @interface A // expected-note {{class started here}}
--(void) im0;
+-(void) im0; // expected-note {{method 'im0' declared here}}
 
+// expected-warning@+1 {{method definition for 'im0' not found}}
 @implementation A // expected-error {{missing '@end'}}
 @end
 
@@ -13,7 +14,8 @@ @interface B { // expected-note {{class started here}}
 @implementation B // expected-error {{missing '@end'}}
 @end
 
-@interface C // expected-note 2 {{class started here}}
+@interface C // expected-note 1 {{class started here}}
 @property int P;
 
+// expected-note@+1 {{implementation started here}}
 @implementation C // expected-error 2 {{missing '@end'}}

clang/test/Parser/missing-end-3.m

@@ -1,10 +1,12 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
 // rdar://8283484
+// expected-note@+1 {{previous definition is here}}
 @interface blah { // expected-note {{class started here}}
     @private
 }
 // since I forgot the @end here it should say something
 
+// expected-error@+1 {{duplicate interface definition for class 'blah'}}
 @interface blah  // expected-error {{missing '@end'}}
-@end // and Unknown type name 'end' here
+@end
 

clang/test/Parser/missing-end-4.m

@@ -37,10 +37,10 @@ @protocol P; // forward declarations of protocols in @implementations is allowed
 - (C<P>*) MyMeth {}
 @end
 
-@interface I2 {}
-@protocol P2; // expected-error {{illegal interface qualifier}}
-@class C2; // expected-error {{illegal interface qualifier}}
-@end
+@interface I2 {} // expected-note {{class started here}}
+@protocol P2; // expected-error {{missing '@end'}}
+@class C2;
+@end // expected-error {{'@end' must appear in an Objective-C context}}
 
 @interface I3
 @end