diff mbox

PATCH: PR bootstrap/61914: [4.10 Regression] wide-int change breaks bootstrap

Message ID 20140729001453.GA14203@intel.com
State New
Headers show

Commit Message

H.J. Lu July 29, 2014, 12:14 a.m. UTC 
wide-int change introduced C++ templates in header file, like

generic_wide_int<fixed_wide_int_storage<int_traits<T1>::precision>>

create_user_defined_type in gengtype.c calls

1. strtok (arg, ",>").
2. resolve_typedef (field_name, pos), which calls create_user_defined_type,
which calls strtok (arg, ",>") again.
3. strtok (0, ",>"), which uses the wrong saved pointer.

On Linux/x32, I got

build/gengtype  \
                    -S /export/gnu/import/git/gcc/gcc -I gtyp-input.list
-w tmp-gtype.state
/bin/sh /export/gnu/import/git/gcc/gcc/../move-if-change tmp-gtype.state
gtype.state
build/gengtype  \
                    -r gtype.state
gengtype: gtype.state:22372: Invalid state file; Lexical error...
make: *** [s-gtype] Error 1

[hjl@gnu-mic-2 gcc]$ grep xfff gtype.state
 (!type undefined 1481 nil  gc_unused "qQ\x02\xffffff98\x01"
   (!pair  "qQ\x02\xffffff98\x01"
 (!type undefined 1486 nil  gc_unused "qQ\x02\xffffff87\x01"
   (!pair  "qQ\x02\xffffff87\x01"
 (!pair  "qQ\x02\xffffff98\x01"
 (!pair  "qQ\x02\xffffff87\x01"
[hjl@gnu-mic-2 gcc]$ 

It is a pure luck that it doesn't fail on other platforms.  This patch
replaces strtok with strtoken.  OK for trunk after bootstrap +
testsuite?


H.J.
---
2014-07-28  H.J. Lu  <hongjiu.lu@intel.com>

	PR bootstrap/61914
	* gengtype.c (strtoken): New function.
	(create_user_defined_type): Replace strtok with strtoken.

Comments

Mike Stump July 29, 2014, 3:31 a.m. UTC | #1 
On Jul 28, 2014, at 5:14 PM, H.J. Lu <hongjiu.lu@intel.com> wrote:
> wide-int change introduced C++ templates in header file, like
> 
> generic_wide_int<fixed_wide_int_storage<int_traits<T1>::precision>>
> 
> create_user_defined_type in gengtype.c calls

Hum…  In the original wide-int, the templates where less beefy; the code worked because the types were simple enough.  gengtype isn’t meant to do all of C++, and certainly this is the type of code that shows why parsing C++ on the fly when you heart isn’t into it can lead to problems.  Then someone wanted to make more use of the power of C++, not realizing the existing landmine present.  Ah… one for the books.

  google("strtok considered harmful”)

So, I never use strtok (usually), and now I know exactly why; it just isn’t composable.

So the replacement looks fine, though, if a strspn style person wants to comment on it further, that’d be nice.  It was new supposedly in BSD 4.3…  kinda new, but, likely that’s reasonably old enough now to not matter now.  I use newlib to test my cross with a native build of gcc under simulation and newlib has it, so I’m happy.

Ok.
diff mbox

Patch

diff --git a/gcc/gengtype.c b/gcc/gengtype.c
index ffe3f94..e66941b 100644
--- a/gcc/gengtype.c
+++ b/gcc/gengtype.c
@@ -569,6 +569,40 @@  do_scalar_typedef (const char *s, struct fileloc *pos)
   do_typedef (s, &scalar_nonchar, pos);
 }
 
+/* Similar to strtok_r.  */
+
+static char *
+strtoken (char *str, const char *delim, char **next)
+{
+  char *p;
+
+  if (str == NULL)
+    str = *next;
+
+  /* Skip the leading delimiters.  */
+  str += strspn (str, delim);
+  if (*str == '\0')
+    /* This is an empty token.  */
+    return NULL;
+
+  /* The current token.  */
+  p = str;
+
+  /* Find the next delimiter.  */
+  str += strcspn (str, delim);
+  if (*str == '\0')
+    /* This is the last token.  */
+    *next = str;
+  else
+    {
+      /* Terminate the current token.  */
+      *str = '\0';
+      /* Advance to the next token.  */
+      *next = str + 1;
+    }
+
+  return p;
+}
 
 /* Define TYPE_NAME to be a user defined type at location POS.  */
 
@@ -599,7 +633,8 @@  create_user_defined_type (const char *type_name, struct fileloc *pos)
 	 comma-separated list of strings, implicitly assumed to
 	 be type names, potentially with "*" characters.  */
       char *arg = open_bracket + 1;
-      char *type_id = strtok (arg, ",>");
+      char *next;
+      char *type_id = strtoken (arg, ",>", &next);
       pair_p fields = 0;
       while (type_id)
 	{
@@ -628,7 +663,7 @@  create_user_defined_type (const char *type_name, struct fileloc *pos)
 	    arg_type = resolve_typedef (field_name, pos);
 
 	  fields = create_field_at (fields, arg_type, field_name, 0, pos);
-	  type_id = strtok (0, ",>");
+	  type_id = strtoken (0, ",>", &next);
 	}
 
       /* Associate the field list to TY.  */