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

2017-06-20 Thread Alex L via cfe-commits 
On 20 June 2017 at 16:53, Duncan P. N. Exon Smith 
wrote:

>
> On Jun 19, 2017, at 10:53, Alex Lorenz via cfe-commits <
> cfe-commits@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=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=305718=305719=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=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
> +
> +@interface ClassA
> +
> +- (void)fileExistsAtPath:(int)x;
> +
> +@end
> +
> +@interface ClassB
> +
> +@end
> +
> +@implementation ClassB // expected-note {{implementation started here}}
> +
> +- (void) method:(ClassA *)mgr { // expected-note {{to match this '{'}}
> +  mgr fileExistsAtPath:0
> +} // expected-error {{expected ']'}}
> +
> +@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!


>
> +
> +@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=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
> +
> +@interface ClassA
> +
> +- (void)fileExistsAtPath:(int)x;
> +
> +@end
> +
> +@interface ClassB
> +
> +@end
> +
> +@implementation ClassB // expected-note {{implementation started here}}
> +
> +- (void) method:(ClassA *)mgr { // expected-note {{to match this '{'}}
> +  mgr fileExistsAtPath:0
> +} // expected-error {{expected ']'}}
> +
> +@interface ClassC // expected-error {{missing '@end'}} // expected-error
> {{expected '}'}}
>
>
> Same recommendation as above.
>
> +
> +@end
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
>
___
cfe-commits mailing list

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

2017-06-20 Thread Duncan P. N. Exon Smith via cfe-commits 

> On Jun 19, 2017, at 10:53, Alex Lorenz via cfe-commits 
>  wrote:
> 
> Author: arphaman
> Date: Mon Jun 19 12:53:21 2017
> New Revision: 305719
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=305719=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=305718=305719=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=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
> +
> +@interface ClassA
> +
> +- (void)fileExistsAtPath:(int)x;
> +
> +@end
> +
> +@interface ClassB
> +
> +@end
> +
> +@implementation ClassB // expected-note {{implementation started here}}
> +
> +- (void) method:(ClassA *)mgr { // expected-note {{to match this '{'}}
> +  mgr fileExistsAtPath:0
> +} // expected-error {{expected ']'}}
> +
> +@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'}}

> +
> +@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=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
> +
> +@interface ClassA
> +
> +- (void)fileExistsAtPath:(int)x;
> +
> +@end
> +
> +@interface ClassB
> +
> +@end
> +
> +@implementation ClassB // expected-note {{implementation started here}}
> +
> +- (void) method:(ClassA *)mgr { // expected-note {{to match this '{'}}
> +  mgr fileExistsAtPath:0
> +} // expected-error {{expected ']'}}
> +
> +@interface ClassC // expected-error {{missing '@end'}} // expected-error 
> {{expected '}'}}

Same recommendation as above.

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

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits