[clang] 1f3c1cb - [Parser][ObjC] Fix crash on nested top-level block with better recovery path
via cfe-commits
cfe-commits at lists.llvm.org
Wed Aug 2 21:58:58 PDT 2023
Author: dingfei
Date: 2023-08-03T12:58:48+08:00
New Revision: 1f3c1cba49a1ace7d2813b3be69279df322c2db0
URL: https://github.com/llvm/llvm-project/commit/1f3c1cba49a1ace7d2813b3be69279df322c2db0
DIFF: https://github.com/llvm/llvm-project/commit/1f3c1cba49a1ace7d2813b3be69279df322c2db0.diff
LOG: [Parser][ObjC] Fix crash on nested top-level block with better recovery 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 https://github.com/llvm/llvm-project/issues/64065.
Reviewed By: rjmccall
Differential Revision: https://reviews.llvm.org/D156277.
Added:
clang/test/Parser/missing-end-1-gh64065-nocrash.m
Modified:
clang/docs/ReleaseNotes.rst
clang/lib/Parse/ParseObjc.cpp
clang/test/Parser/missing-end-2.m
clang/test/Parser/missing-end-3.m
clang/test/Parser/missing-end-4.m
Removed:
################################################################################
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 3af39b4f409199..627a1541cfddb9 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/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
-----------------------
diff --git a/clang/lib/Parse/ParseObjc.cpp b/clang/lib/Parse/ParseObjc.cpp
index b30f0380621a17..d0af98b9106c3e 100644
--- a/clang/lib/Parse/ParseObjc.cpp
+++ b/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)
diff --git a/clang/test/Parser/missing-end-1-gh64065-nocrash.m b/clang/test/Parser/missing-end-1-gh64065-nocrash.m
new file mode 100644
index 00000000000000..22149f852969ac
--- /dev/null
+++ b/clang/test/Parser/missing-end-1-gh64065-nocrash.m
@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+ at interface Roo // expected-note {{class started here}}
+// expected-error at +1 {{missing '@end'}}
+ at interface // expected-error {{expected identifier}}
diff --git a/clang/test/Parser/missing-end-2.m b/clang/test/Parser/missing-end-2.m
index e89f28eb247b21..885556c7c9b55e 100644
--- a/clang/test/Parser/missing-end-2.m
+++ b/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 at +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
- at interface C // expected-note 2 {{class started here}}
+ at interface C // expected-note 1 {{class started here}}
@property int P;
+// expected-note at +1 {{implementation started here}}
@implementation C // expected-error 2 {{missing '@end'}}
diff --git a/clang/test/Parser/missing-end-3.m b/clang/test/Parser/missing-end-3.m
index 4875ecdd625b79..125b419d68f963 100644
--- a/clang/test/Parser/missing-end-3.m
+++ b/clang/test/Parser/missing-end-3.m
@@ -1,10 +1,12 @@
// RUN: %clang_cc1 -fsyntax-only -verify %s
// rdar://8283484
+// expected-note at +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 at +1 {{duplicate interface definition for class 'blah'}}
@interface blah // expected-error {{missing '@end'}}
- at end // and Unknown type name 'end' here
+ at end
diff --git a/clang/test/Parser/missing-end-4.m b/clang/test/Parser/missing-end-4.m
index 8a96e64421c133..84c3967d997372 100644
--- a/clang/test/Parser/missing-end-4.m
+++ b/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
- at interface I2 {}
- at protocol P2; // expected-error {{illegal interface qualifier}}
- at class C2; // expected-error {{illegal interface qualifier}}
- at end
+ at interface I2 {} // expected-note {{class started here}}
+ at protocol P2; // expected-error {{missing '@end'}}
+ at class C2;
+ at end // expected-error {{'@end' must appear in an Objective-C context}}
@interface I3
@end
More information about the cfe-commits
mailing list