diff mbox

Fix for PR objc/34033 "compiler accepts invalid string concatenation"

Message ID 1290341810.99812627@192.168.2.229
State New
Headers show

Commit Message

Nicola Pero Nov. 21, 2010, 12:16 p.m. UTC 
This patch fixes PR objc/34033.

Summarizing, GCC was accepting any number of "@" signs, in any position, when doing string concatenation.  This patch adds the relevant checks to make sure there is a single '@' sign before each string.  Testcases for both Objective-C and Objective-C++ are included.

Ok to commit ?

Thanks

Comments

Mike Stump Nov. 21, 2010, 4:54 p.m. UTC | #1 
On Nov 21, 2010, at 4:16 AM, "Nicola Pero" <nicola.pero@meta-innovation.com> wrote:
> This patch fixes PR objc/34033.
> 
> Summarizing, GCC was accepting any number of "@" signs, in any position, when doing string concatenation.  This patch adds the relevant checks to make sure there is a single '@' sign before each string.

Does clang or Apple's gcc accept concat with c strings and objc string?

I kinda think it should be allowed.  Essentially the final type being that of the first string.  Thoughts?

> Ok to commit ?

Ok.
>
Paolo Bonzini Nov. 21, 2010, 9:32 p.m. UTC | #2 
On 11/21/2010 05:54 PM, Mike Stump wrote:
>>> Summarizing, GCC was accepting any number of "@" signs, in any
>>> position, when doing string concatenation.  This patch adds the
>>> relevant checks to make sure there is a single '@' sign before
>>> each string.
>
> Does clang or Apple's gcc accept concat with c strings and objc
> string?

Assuming Nicola followed their lead, no.  But I was thinking the same 
when looking at the code and testcase.

Actually, I would allow only concat-ing C to ObjC, like @"ab" "cd".  I 
don't think "cd" @"ab" should be allowed.

Paolo
diff mbox

Patch

Index: c-family/ChangeLog
===================================================================
--- c-family/ChangeLog  (revision 166987)
+++ c-family/ChangeLog  (working copy)
@@ -1,3 +1,9 @@ 
+2010-11-21  Nicola Pero  <nicola.pero@meta-innovation.com>
+
+       PR objc/34033
+       * c-lex.c (lex_string): Check that each Objective-C string in a
+       string concatenation sequence starts with exactly a single '@'.
+
 2010-11-20  Nathan Froyd  <froydnj@codesourcery.com>
 
        PR c++/16189
Index: c-family/c-lex.c
===================================================================
--- c-family/c-lex.c    (revision 166987)
+++ c-family/c-lex.c    (working copy)
@@ -891,7 +891,8 @@  interpret_fixed (const cpp_token *token,
    UTF8STRING tokens into a tree, performing string constant
    concatenation.  TOK is the first of these.  VALP is the location
    to write the string into. OBJC_STRING indicates whether an '@' token
-   preceded the incoming token.
+   preceded the incoming token (in that case, all the strings must
+   be preceded by a '@', but not other '@' should be present).
    Returns the CPP token type of the result (CPP_STRING, CPP_WSTRING,
    CPP_STRING32, CPP_STRING16, CPP_UTF8STRING, or CPP_OBJC_STRING).
 
@@ -918,6 +919,12 @@  lex_string (const cpp_token *tok, tree *
   cpp_string str = tok->val.str;
   cpp_string *strs = &str;
 
+  /* objc_at_sign_was_seen is only used when doing Objective-C string
+     concatenation.  It is 'true' if we have seen an '@' before the
+     current string, and 'false' if not.  We must see exactly one '@'
+     before each string.  */
+  bool objc_at_sign_was_seen = false;
+
  retry:
   tok = cpp_get_token (parse_in);
   switch (tok->type)
@@ -925,9 +932,12 @@  lex_string (const cpp_token *tok, tree *
     case CPP_PADDING:
       goto retry;
     case CPP_ATSIGN:
-      if (c_dialect_objc ())
+      if (objc_string)
        {
-         objc_string = true;
+         if (objc_at_sign_was_seen)
+           error ("repeated %<@%> before Objective-C string");
+
+         objc_at_sign_was_seen = true;
          goto retry;
        }
       /* FALLTHROUGH */
@@ -948,6 +958,12 @@  lex_string (const cpp_token *tok, tree *
        }
 
     case CPP_STRING:
+      if (objc_string)
+       {
+         if (!objc_at_sign_was_seen)
+           error ("missing %<@%> before Objective-C string");
+         objc_at_sign_was_seen = false;
+       }
       if (!concats)
        {
          gcc_obstack_init (&str_ob);
@@ -959,6 +975,10 @@  lex_string (const cpp_token *tok, tree *
       goto retry;
     }
 
+  /* It is an error if we saw a '@' with no following string.  */
+  if (objc_at_sign_was_seen)
+    error ("stray %<@%> in program");
+
   /* We have read one more token than we want.  */
   _cpp_backup_tokens (parse_in, 1);
   if (concats)
Index: testsuite/ChangeLog
===================================================================
--- testsuite/ChangeLog (revision 166987)
+++ testsuite/ChangeLog (working copy)
@@ -1,3 +1,11 @@ 
+2010-11-21  Nicola Pero  <nicola.pero@meta-innovation.com>
+
+       PR objc/34033
+       * objc.dg/strings-1.m: New.
+       * objc.dg/strings-2.m: New.
+       * obj-c++.dg/strings-1.mm: New.
+       * obj-c++.dg/strings-2.mm: New. 
+
 2010-11-20  Eric Botcazou  <ebotcazou@adacore.com>
 
        * gcc.target/sparc/sparc-align-1.c: New test.
Index: testsuite/objc.dg/strings-1.m
===================================================================
--- testsuite/objc.dg/strings-1.m       (revision 0)
+++ testsuite/objc.dg/strings-1.m       (revision 0)
@@ -0,0 +1,28 @@ 
+/* Contributed by Nicola Pero <nicola.pero@meta-innovation.com>, November 2010.  */
+/* { dg-do compile } */
+
+#include "../objc-obj-c++-shared/Object1.h"
+#include "../objc-obj-c++-shared/next-mapping.h"
+#ifndef __NEXT_RUNTIME__
+#include <objc/NXConstStr.h>
+#endif
+
+/* The following are correct.  */
+id test_valid1 = @"test";
+id test_valid2 = @"te" @"st";
+id test_valid3 = @"te" @"s" @"t";
+id test_valid4 = @ "t" @ "e" @ "s" @ "t";
+
+/* The following are not correct.  */
+id test_invalid1          = @@"test";            /* { dg-error "stray .@. in program" } */
+const char *test_invalid2 = "test"@;             /* { dg-error "stray .@. in program" } */
+const char *test_invalid3 = "test"@@;            /* { dg-error "stray .@. in program" } */
+id test_invalid4          = @"te" "st";          /* { dg-error "missing .@. before Objective.C string" } */
+const char *test_invalid5 = "te" @"st";          /* { dg-error "expected" } */
+id test_invalid6          = @"te" @@"st";        /* { dg-error "repeated .@. before Objective-C string" } */
+id test_invalid7          = @@"te" @"st";        /* { dg-error "stray .@. in program" } */
+id test_invalid8          = @"te" @"s" "t";      /* { dg-error "missing .@. before Objective.C string" } */
+id test_invalid9          = @"te" "s" @"t";      /* { dg-error "missing .@. before Objective.C string" } */
+id test_invalidA          = @"te" @"s" @@"t";    /* { dg-error "repeated .@. before Objective-C string" } */
+id test_invalidB          = @"te" @@"s" @"t";    /* { dg-error "repeated .@. before Objective-C string" } */
+id test_invalidC          = @"te" @"s" @"t" @;   /* { dg-error "stray .@. in program" } */
Index: testsuite/objc.dg/strings-2.m
===================================================================
--- testsuite/objc.dg/strings-2.m       (revision 0)
+++ testsuite/objc.dg/strings-2.m       (revision 0)
@@ -0,0 +1,47 @@ 
+/* Contributed by Nicola Pero <nicola.pero@meta-innovation.com>, November 2010.  */
+
+/* { dg-do run } */
+/* { dg-xfail-run-if "Needs OBJC2 ABI" { *-*-darwin* && { lp64 && { ! objc2 } } } { "-fnext-runtime" } { "" } } */
+/* { dg-options "-fconstant-string-class=MyTestString" } */
+/* { dg-options "-mno-constant-cfstrings -fconstant-string-class=MyTestString" { target *-*-darwin* } } */
+
+/* { dg-additional-sources "../objc-obj-c++-shared/Object1.m" } */
+
+#include "../objc-obj-c++-shared/Object1.h"
+#include "../objc-obj-c++-shared/next-mapping.h"
+
+#include <stdlib.h> /* For abort() */
+
+@interface MyTestString : Object
+{
+  char *string;
+  unsigned int len;
+}
+/* All strings should contain the C string 'test'.  Call -check to
+   test that this is true.  */
+- (void) check;
+@end
+
+@implementation MyTestString
+- (void) check
+{
+  if (len != 4 || string[0] != 't' || string[1] != 'e'
+      || string[2] != 's' || string[3] != 't' || string[4] != '\0')
+    abort ();
+}
+@end
+
+int main (void)
+{
+  MyTestString *test_valid1 = @"test";
+  MyTestString *test_valid2 = @"te" @"st";
+  MyTestString *test_valid3 = @"te" @"s" @"t";
+  MyTestString *test_valid4 = @ "t" @ "e" @ "s" @ "t";
+
+  [test_valid1 check];
+  [test_valid2 check];
+  [test_valid3 check];
+  [test_valid4 check];
+
+  return 0;
+}
Index: testsuite/obj-c++.dg/strings-1.mm
===================================================================
--- testsuite/obj-c++.dg/strings-1.mm   (revision 0)
+++ testsuite/obj-c++.dg/strings-1.mm   (revision 0)
@@ -0,0 +1,28 @@ 
+/* Contributed by Nicola Pero <nicola.pero@meta-innovation.com>, November 2010.  */
+/* { dg-do compile } */
+
+#include "../objc-obj-c++-shared/Object1.h"
+#include "../objc-obj-c++-shared/next-mapping.h"
+#ifndef __NEXT_RUNTIME__
+#include <objc/NXConstStr.h>
+#endif
+
+/* The following are correct.  */
+id test_valid1 = @"test";
+id test_valid2 = @"te" @"st";
+id test_valid3 = @"te" @"s" @"t";
+id test_valid4 = @ "t" @ "e" @ "s" @ "t";
+
+/* The following are not correct.  */
+id test_invalid1    = @@"test";            /* { dg-error "stray .@. in program" } */
+const char *test_invalid2 = "test"@;             /* { dg-error "stray .@. in program" } */
+const char *test_invalid3 = "test"@@;            /* { dg-error "stray .@. in program" } */
+id test_invalid4    = @"te" "st";          /* { dg-error "missing .@. before Objective.C string" } */
+const char *test_invalid5 = "te" @"st";          /* { dg-error "expected" } */
+id test_invalid6    = @"te" @@"st";        /* { dg-error "repeated .@. before Objective-C string" } */
+id test_invalid7    = @@"te" @"st";        /* { dg-error "stray .@. in program" } */
+id test_invalid8    = @"te" @"s" "t";      /* { dg-error "missing .@. before Objective.C string" } */
+id test_invalid9    = @"te" "s" @"t";      /* { dg-error "missing .@. before Objective.C string" } */
+id test_invalidA    = @"te" @"s" @@"t";    /* { dg-error "repeated .@. before Objective-C string" } */
+id test_invalidB    = @"te" @@"s" @"t";    /* { dg-error "repeated .@. before Objective-C string" } */
+id test_invalidC    = @"te" @"s" @"t" @;   /* { dg-error "stray .@. in program" } */
Index: testsuite/obj-c++.dg/strings-2.mm
===================================================================
--- testsuite/obj-c++.dg/strings-2.mm   (revision 0)
+++ testsuite/obj-c++.dg/strings-2.mm   (revision 0)
@@ -0,0 +1,47 @@ 
+/* Contributed by Nicola Pero <nicola.pero@meta-innovation.com>, November 2010.  */
+
+/* { dg-do run } */
+/* { dg-xfail-run-if "Needs OBJC2 ABI" { *-*-darwin* && { lp64 && { ! objc2 } } } { "-fnext-runtime" } { "" } } */
+/* { dg-options "-fconstant-string-class=MyTestString" } */
+/* { dg-options "-mno-constant-cfstrings -fconstant-string-class=MyTestString" { target *-*-darwin* } } */
+
+/* { dg-additional-sources "../objc-obj-c++-shared/Object1.mm" } */
+
+#include "../objc-obj-c++-shared/Object1.h"
+#include "../objc-obj-c++-shared/next-mapping.h"
+
+#include <stdlib.h> /* For abort() */
+
+@interface MyTestString : Object
+{
+  char *string;
+  unsigned int len;
+}
+/* All strings should contain the C string 'test'.  Call -check to
+   test that this is true.  */
+- (void) check;
+@end
+
+@implementation MyTestString
+- (void) check
+{
+  if (len != 4 || string[0] != 't' || string[1] != 'e'
+      || string[2] != 's' || string[3] != 't' || string[4] != '\0')
+    abort ();
+}
+@end
+
+int main (void)
+{
+  MyTestString *test_valid1 = @"test";
+  MyTestString *test_valid2 = @"te" @"st";
+  MyTestString *test_valid3 = @"te" @"s" @"t";
+  MyTestString *test_valid4 = @ "t" @ "e" @ "s" @ "t";
+
+  [test_valid1 check];
+  [test_valid2 check];
+  [test_valid3 check];
+  [test_valid4 check];
+
+  return 0;
+}