Patchwork =?UTF-8?Q?Fix=20for=20PR=20objc/47078=20("[4.6=20Regression]=20ICE=20on?= =?UTF-8?Q?=20invalid=20type")?=

login
register
mail settings
Submitter Nicola Pero
Date Jan. 8, 2011, 3:17 p.m.
Message ID <1294499879.464817710@192.168.4.58>
Download mbox | patch
Permalink /patch/77954/
State New
Headers show

Comments

Nicola Pero - Jan. 8, 2011, 3:17 p.m.
This patch fixes objc/47078.  The (invalid) ObjC code that crashes the 4.6 
compiler (but not older compilers) is

@implementation MyClass
- (x) method
{
   return 0;
}
@end

The problem is the 'x', the invalid type name.  The new code in 4.6 which detects 
invalid typenames [and produces nice error messages :-)] was passing 
error_mark_node to the ObjC frontend, which wasn't prepared to deal with it and 
it all crashed later.

The fix simply changes the parsing of ObjC methods to detect when error_mark_node is produced due to an invalid type name, and replace it with NULL_TREE, which is 
what is produced when no type is specified.  The ObjC frontend then uses the 
default (id) for unspecified types, which is also the best guess when trying to 
do error recovery.

As I added the testcase for ObjC, I noticed the testcase also crashed ObjC++ in 
exactly the same way, and I added the same testcase and fix for ObjC++.  I'm not 
sure the ObjC++ one is a regression, but it's a trivial fix and matches exactly the ObjC fix (which *is* a regression) and I'm trying to keep ObjC and ObjC++
in sync so, unless Mike disagrees, I think the ObjC++ fix should go in too in the 
same patch.

Bootstrapped and tested on i686-pc-linux-gnu (apologies for the last patch before holidays, which failed bootstrap, this one doesn't).

Ok to commit ?

Thanks
Mike Stump - Jan. 8, 2011, 6:38 p.m.
On Jan 8, 2011, at 7:17 AM, "Nicola Pero" <nicola.pero@meta-innovation.com> wrote:
> This patch fixes objc/47078.  The (invalid) ObjC code that crashes the 4.6 
> compiler (but not older compilers)

> Ok to commit ?

Ok.

Patch

Index: ChangeLog
===================================================================
--- ChangeLog   (revision 168596)
+++ ChangeLog   (working copy)
@@ -1,3 +1,10 @@ 
+2011-01-08  Nicola Pero  <nicola.pero@meta-innovation.com>
+
+       PR objc/47078
+       * c-parser.c (c_parser_objc_type_name): If the type is unknown,
+       for error recovery purposes behave as if it was not specified so
+       that the default type is usd.
+
 2011-01-07  Joseph Myers  <joseph@codesourcery.com>
 
        * opts.c (finish_options): Set opts->x_flag_opts_finished to true,
Index: testsuite/ChangeLog
===================================================================
--- testsuite/ChangeLog (revision 168596)
+++ testsuite/ChangeLog (working copy)
@@ -1,3 +1,9 @@ 
+2011-01-08  Nicola Pero  <nicola.pero@meta-innovation.com>
+
+       PR objc/47078
+       * objc.dg/invalid-method-2.m: New.
+       * obj-c++.dg/invalid-method-2.mm: New.
+
 2011-01-08  Thomas Koenig  <tkoenig@gcc.gnu.org>
 
        PR fortran/45777
Index: testsuite/objc.dg/invalid-method-2.m
===================================================================
--- testsuite/objc.dg/invalid-method-2.m        (revision 0)
+++ testsuite/objc.dg/invalid-method-2.m        (revision 0)
@@ -0,0 +1,18 @@ 
+/* { dg-do compile } */
+
+/* Test that using an invalid type in a method declaration produces a
+   friendly error without a compiler crash.  */
+
+@interface MyClass
+@end
+
+@implementation MyClass
+- (x) method /* { dg-error "unknown type name" } */
+{
+  return 0;
+}
+- (id) method2: (x)argument /* { dg-error "unknown type name" } */
+{
+  return 0;
+}
+@end
Index: testsuite/obj-c++.dg/invalid-method-2.mm
===================================================================
--- testsuite/obj-c++.dg/invalid-method-2.mm    (revision 0)
+++ testsuite/obj-c++.dg/invalid-method-2.mm    (revision 0)
@@ -0,0 +1,18 @@ 
+/* { dg-do compile } */
+
+/* Test that using an invalid type in a method declaration produces a
+   friendly error without a compiler crash.  */
+
+@interface MyClass
+@end
+
+@implementation MyClass
+- (x) method /* { dg-error "expected" } */
+{
+  return 0;
+}
+- (id) method2: (x)argument /* { dg-error "expected" } */
+{
+  return 0;
+}
+@end
Index: cp/ChangeLog
===================================================================
--- cp/ChangeLog        (revision 168596)
+++ cp/ChangeLog        (working copy)
@@ -1,3 +1,10 @@ 
+2011-01-08  Nicola Pero  <nicola.pero@meta-innovation.com>
+
+       PR objc/47078
+       * parser.c (cp_parser_objc_typename): If the type is unknown, for
+       error recovery purposes behave as if it was not specified so that
+       the default type is used.
+
 2011-01-07  Jakub Jelinek  <jakub@redhat.com>
 
        PR c++/47022
Index: cp/parser.c
===================================================================
--- cp/parser.c (revision 168596)
+++ cp/parser.c (working copy)
@@ -21915,7 +21915,25 @@  cp_parser_objc_typename (cp_parser* pars
       /* An ObjC type name may consist of just protocol qualifiers, in which
         case the type shall default to 'id'.  */
       if (cp_lexer_next_token_is_not (parser->lexer, CPP_CLOSE_PAREN))
-       cp_type = cp_parser_type_id (parser);
+       {
+         cp_type = cp_parser_type_id (parser);
+         
+         /* If the type could not be parsed, an error has already
+            been produced.  For error recovery, behave as if it had
+            not been specified, which will use the default type
+            'id'.  */
+         if (cp_type == error_mark_node)
+           {
+             cp_type = NULL_TREE;
+             /* We need to skip to the closing parenthesis as
+                cp_parser_type_id() does not seem to do it for
+                us.  */
+             cp_parser_skip_to_closing_parenthesis (parser,
+                                                    /*recovering=*/true,
+                                                    /*or_comma=*/false,
+                                                    /*consume_paren=*/false);
+           }
+       }
 
       cp_parser_require (parser, CPP_CLOSE_PAREN, RT_CLOSE_PAREN);
       type_name = build_tree_list (proto_quals, cp_type);
Index: c-parser.c
===================================================================
--- c-parser.c  (revision 168596)
+++ c-parser.c  (working copy)
@@ -7482,6 +7482,14 @@  c_parser_objc_type_name (c_parser *parse
     type_name = c_parser_type_name (parser);
   if (type_name)
     type = groktypename (type_name, NULL, NULL);
+
+  /* If the type is unknown, and error has already been produced and
+     we need to recover from the error.  In that case, use NULL_TREE
+     for the type, as if no type had been specified; this will use the
+     default type ('id') which is good for error recovery.  */
+  if (type == error_mark_node)
+    type = NULL_TREE;
+
   return build_tree_list (quals, type);
 }