r305719 - [Parser][ObjC] Use an artificial EOF token while parsing lexed ObjC methods

[Duncan P. N. Exon Smith via cfe-commits]

> On Jun 19, 2017, at 10:53, Alex Lorenz via cfe-commits <cfe-commits at lists.llvm.org> wrote:
> 
> Author: arphaman
> Date: Mon Jun 19 12:53:21 2017
> New Revision: 305719
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=305719&view=rev
> Log:
> [Parser][ObjC] Use an artificial EOF token while parsing lexed ObjC methods
> 
> This change avoid a crash that occurred when skipping to EOF while parsing an
> ObjC interface/implementation.
> 
> rdar://31963299
> 
> Differential Revision: https://reviews.llvm.org/D34185
> 
> Added:
>    cfe/trunk/test/Parser/objc-at-implementation-eof-crash.m
>    cfe/trunk/test/Parser/objc-at-interface-eof-crash.m
> Modified:
>    cfe/trunk/lib/Parse/ParseObjc.cpp
> 
> Modified: cfe/trunk/lib/Parse/ParseObjc.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseObjc.cpp?rev=305719&r1=305718&r2=305719&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Parse/ParseObjc.cpp (original)
> +++ cfe/trunk/lib/Parse/ParseObjc.cpp Mon Jun 19 12:53:21 2017
> @@ -3627,6 +3627,14 @@ void Parser::ParseLexedObjCMethodDefs(Le
>   SourceLocation OrigLoc = Tok.getLocation();
> 
>   assert(!LM.Toks.empty() && "ParseLexedObjCMethodDef - Empty body!");
> +  // Store an artificial EOF token to ensure that we don't run off the end of
> +  // the method's body when we come to parse it.
> +  Token Eof;
> +  Eof.startToken();
> +  Eof.setKind(tok::eof);
> +  Eof.setEofData(MCDecl);
> +  Eof.setLocation(OrigLoc);
> +  LM.Toks.push_back(Eof);
>   // Append the current token at the end of the new token stream so that it
>   // doesn't get lost.
>   LM.Toks.push_back(Tok);
> @@ -3658,7 +3666,7 @@ void Parser::ParseLexedObjCMethodDefs(Le
>       Actions.ActOnDefaultCtorInitializers(MCDecl);
>     ParseFunctionStatementBody(MCDecl, BodyScope);
>   }
> -  
> +
>   if (Tok.getLocation() != OrigLoc) {
>     // Due to parsing error, we either went over the cached tokens or
>     // there are still cached tokens left. If it's the latter case skip the
> @@ -3670,4 +3678,6 @@ void Parser::ParseLexedObjCMethodDefs(Le
>       while (Tok.getLocation() != OrigLoc && Tok.isNot(tok::eof))
>         ConsumeAnyToken();
>   }
> +  // Clean up the remaining EOF token.
> +  ConsumeAnyToken();
> }
> 
> Added: cfe/trunk/test/Parser/objc-at-implementation-eof-crash.m
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/objc-at-implementation-eof-crash.m?rev=305719&view=auto
> ==============================================================================
> --- cfe/trunk/test/Parser/objc-at-implementation-eof-crash.m (added)
> +++ cfe/trunk/test/Parser/objc-at-implementation-eof-crash.m Mon Jun 19 12:53:21 2017
> @@ -0,0 +1,21 @@
> +// RUN: %clang_cc1 -verify -Wno-objc-root-class %s
> +
> + at interface ClassA
> +
> +- (void)fileExistsAtPath:(int)x;
> +
> + at end
> +
> + at interface ClassB
> +
> + at end
> +
> + at implementation ClassB // expected-note {{implementation started here}}
> +
> +- (void) method:(ClassA *)mgr { // expected-note {{to match this '{'}}
> +  mgr fileExistsAtPath:0
> +} // expected-error {{expected ']'}}
> +
> + at implementation ClassC // expected-error {{missing '@end'}} // expected-error {{expected '}'}} // expected-warning {{cannot find interface declaration for 'ClassC'}}

I believe you can split these expectations over multiple lines.  Something like this seems more readable:

@implementation ClassC                 \
  // expected-error {{missing '@end'}} \
  // expected-error {{expected '}'}}   \
  // expected-warning {{cannot find interface declaration for 'ClassC'}}

> +
> + at end
> 
> Added: cfe/trunk/test/Parser/objc-at-interface-eof-crash.m
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/objc-at-interface-eof-crash.m?rev=305719&view=auto
> ==============================================================================
> --- cfe/trunk/test/Parser/objc-at-interface-eof-crash.m (added)
> +++ cfe/trunk/test/Parser/objc-at-interface-eof-crash.m Mon Jun 19 12:53:21 2017
> @@ -0,0 +1,21 @@
> +// RUN: %clang_cc1 -verify -Wno-objc-root-class %s
> +
> + at interface ClassA
> +
> +- (void)fileExistsAtPath:(int)x;
> +
> + at end
> +
> + at interface ClassB
> +
> + at end
> +
> + at implementation ClassB // expected-note {{implementation started here}}
> +
> +- (void) method:(ClassA *)mgr { // expected-note {{to match this '{'}}
> +  mgr fileExistsAtPath:0
> +} // expected-error {{expected ']'}}
> +
> + at interface ClassC // expected-error {{missing '@end'}} // expected-error {{expected '}'}}

Same recommendation as above.

> +
> + at end
> 
> 
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170620/2cfbbced/attachment.html>