public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* PATCH: PR bootstrap/61914: [4.10 Regression] wide-int change breaks bootstrap
@ 2014-07-29  2:33 H.J. Lu
  2014-07-29  5:30 ` Mike Stump
  0 siblings, 1 reply; 2+ messages in thread
From: H.J. Lu @ 2014-07-29  2:33 UTC (permalink / raw)
  To: gcc-patches

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.

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.  */

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: PATCH: PR bootstrap/61914: [4.10 Regression] wide-int change breaks bootstrap
  2014-07-29  2:33 PATCH: PR bootstrap/61914: [4.10 Regression] wide-int change breaks bootstrap H.J. Lu
@ 2014-07-29  5:30 ` Mike Stump
  0 siblings, 0 replies; 2+ messages in thread
From: Mike Stump @ 2014-07-29  5:30 UTC (permalink / raw)
  To: H.J. Lu; +Cc: gcc-patches

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.

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2014-07-29  3:31 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-29  2:33 PATCH: PR bootstrap/61914: [4.10 Regression] wide-int change breaks bootstrap H.J. Lu
2014-07-29  5:30 ` Mike Stump

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).