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

[Alex L via cfe-commits]

On 20 June 2017 at 16:53, Duncan P. N. Exon Smith <dexonsmith at apple.com>
wrote:

>
> 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'}}
>
>
Thanks, addressed in r305803!


>
> +
> + 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/48832f55/attachment-0001.html>