cpp token_buffer not being NULL terminated causes fixproto bugs

Zack Weinberg zack@wolery.cumb.org
Sun Apr 30 10:30:00 GMT 2000


On Sun, Apr 30, 2000 at 10:04:51AM -0400, Kaveh R. Ghazi wrote:
>  > From: Zack Weinberg <zack@wolery.cumb.org>
>  > 
>  > I couldn't find any places where you missed an assumed NUL-
>  > termination.  Would you care to give this patch a whirl on sunos4?  It
>  > works for me on your errno.h example.  As a fringe bonus, we no longer
>  > mess up glibc's getopt.h.
>  > zw
> 
> Your patch works for me on SunOS4.  Thanks.
> 
>  > 	* fixproto: Don't redefine __STDC__ to 0.
> 
> FYI, the above __STDC__ change does seem to have an effect.  Mainly on
> X11 files WRT which clause of #if __STDC__ conditionals gets diddled
> with.  The header diffs look harmless though, but I thought you should
> know.  I'm not sure whether we want to keep __STDC__=0 for platforms
> which define STDC_0_IN_SYSTEM_HEADERS.  Something to think about.

SunOS 4 doesn't define STDC_0_IN_SYSTEM_HEADERS.  Maybe it should.
(Only Solaris 2.x uses that define at the moment, but several other
platforms could benefit - see my other message last Friday.)

Anyway, I took out that change and the other changes for glibc's
getopt.h.  Here's the patch I did commit:

	* cpplex.c (cpp_idcmp): New function.
	* cpplib.h: Prototype it.
	* scan_decls.c (scan_decls): Use it to inspect token names.
	* fix-header.c (read_scan_file): Likewise.  Set system_header_p on
	the file being run through the preprocessor.
	(check_macro_names): Provide length of token to cpp_defined.

	* Makefile.in: Remove stale warning message.

===================================================================
Index: cpplex.c
--- cpplex.c	2000/04/28 18:17:54	1.30
+++ cpplex.c	2000/04/30 17:28:28
@@ -2048,6 +2048,37 @@ _cpp_init_input_buffer (pfile)
   pfile->input_buffer_len = 8192;
 }
 
+/* Utility routine:
+   Compares, in the manner of strcmp(3), the token beginning at TOKEN
+   and extending for LEN characters to the NUL-terminated string
+   STRING.  Typical usage:
+
+   if (! cpp_idcmp (pfile->token_buffer + here, CPP_WRITTEN (pfile) - here,
+                 "inline"))
+     { ... }
+ */
+
+int
+cpp_idcmp (token, len, string)
+     const U_CHAR *token;
+     size_t len;
+     const char *string;
+{
+  size_t len2 = strlen (string);
+  int r;
+
+  if ((r = memcmp (token, string, MIN (len, len2))))
+    return r;
+
+  /* The longer of the two strings sorts after the shorter.  */
+  if (len == len2)
+    return 0;
+  else if (len < len2)
+    return -1;
+  else
+    return 1;
+}
+
 #if 0
 
 /* Lexing algorithm.
===================================================================
Index: cpplib.h
--- cpplib.h	2000/04/27 05:49:33	1.86
+++ cpplib.h	2000/04/30 17:28:28
@@ -655,8 +655,8 @@ extern cpp_buffer *cpp_push_buffer	PARAM
 extern cpp_buffer *cpp_pop_buffer	PARAMS ((cpp_reader *));
 extern void cpp_scan_buffer		PARAMS ((cpp_reader *, cpp_printer *));
 extern void cpp_scan_buffer_nooutput	PARAMS ((cpp_reader *));
-
-
+extern int cpp_idcmp			PARAMS ((const unsigned char *,
+						 size_t, const char *));
 
 /* In cpphash.c */
 extern int cpp_defined			PARAMS ((cpp_reader *,
===================================================================
Index: fix-header.c
--- fix-header.c	2000/04/14 23:29:45	1.41
+++ fix-header.c	2000/04/30 17:28:28
@@ -602,11 +602,13 @@ check_macro_names (pfile, names)
      cpp_reader *pfile;
      namelist names;
 {
+  size_t len;
   while (*names)
     {
-      if (cpp_defined (pfile, names, -1))
+      len = strlen (names);
+      if (cpp_defined (pfile, names, len))
 	recognized_macro (names);
-      names += strlen (names) + 1;
+      names += len + 1;
     }
 }
 
@@ -637,6 +639,9 @@ read_scan_file (in_fname, argc, argv)
   if (! cpp_start_read (&scan_in, 0, in_fname))
     exit (FATAL_EXIT_CODE);
 
+  /* We are scanning a system header, so mark it as such.  */
+  CPP_BUFFER (&scan_in)->system_header_p = 1;
+
   scan_decls (&scan_in, argc, argv);
   for (cur_symbols = &symbol_table[0]; cur_symbols->names; cur_symbols++)
     check_macro_names (&scan_in, cur_symbols->names);
@@ -663,6 +668,8 @@ read_scan_file (in_fname, argc, argv)
 	{
 	  enum cpp_ttype token = cpp_get_token (&scan_in);
 	  int length = CPP_WRITTEN (&scan_in) - old_written;
+	  unsigned char *id = scan_in.token_buffer + old_written;
+	  
 	  CPP_SET_WRITTEN (&scan_in, old_written);
 	  if (token == CPP_EOF) /* Should not happen ...  */
 	    break;
@@ -671,8 +678,7 @@ read_scan_file (in_fname, argc, argv)
 	      cpp_pop_buffer (&scan_in);
 	      break;
 	    }
-	  if (token == CPP_NAME && length == 7
-	      && strcmp ("_filbuf", scan_in.token_buffer + old_written) == 0)
+	  if (token == CPP_NAME && cpp_idcmp (id, length, "_filbuf") == 0)
 	    seen_filbuf++;
 	}
       if (seen_filbuf)
@@ -690,7 +696,7 @@ read_scan_file (in_fname, argc, argv)
 		SET_REQUIRED (fn);
 	      if (need_flsbuf)
 		SET_REQUIRED (flsbuf_fn);
-	      if (need_flsbuf + need_filbuf == 2)
+	      if (need_flsbuf && need_filbuf)
 		new_list = "_filbuf\0_flsbuf\0";
 	      else if (need_flsbuf)
 		new_list = "_flsbuf\0";
===================================================================
Index: scan-decls.c
--- scan-decls.c	2000/04/20 19:33:11	1.16
+++ scan-decls.c	2000/04/30 17:28:28
@@ -191,9 +191,7 @@ scan_decls (pfile, argc, argv)
 	maybe_handle_comma:
 	  if (token != CPP_COMMA)
 	    goto new_statement;
-#if 0
-	handle_comma:
-#endif
+
 	  /* Handle multiple declarators in a single declaration,
 	     as in:  extern char *strcpy (), *strcat (), ... ; */
 	  if (declarator_start == 0)
@@ -202,12 +200,14 @@ scan_decls (pfile, argc, argv)
 	  break;
 	case CPP_NAME:
 	  /* "inline" and "extern" are recognized but skipped */
-	  if (strcmp (pfile->token_buffer, "inline") == 0)
+	  if (!cpp_idcmp (pfile->token_buffer,
+			  CPP_WRITTEN (pfile), "inline"))
 	    {
 	      saw_inline = 1;
 	      CPP_SET_WRITTEN (pfile, start_written);
 	    }
-	  if (strcmp (pfile->token_buffer, "extern") == 0)
+	  else if (!cpp_idcmp (pfile->token_buffer,
+			       CPP_WRITTEN (pfile), "extern"))
 	    {
 	      saw_extern = 1;
 	      CPP_SET_WRITTEN (pfile, start_written);
===================================================================
Index: Makefile.in
--- Makefile.in	2000/04/30 11:01:52	1.430
+++ Makefile.in	2000/04/30 17:28:29
@@ -2297,7 +2297,6 @@ fixhdr.ready: fix-header
 # The if statement is so that we don't run fixproto a second time
 # if it has already been run on the files in `include'.
 stmp-fixproto: fixhdr.ready fixproto stmp-int-hdrs
-	@echo "Various warnings and error messages from fixproto are normal"
 	if [ -f include/fixed ] ; then true; \
 	else \
 	  : This line works around a 'make' bug in BSDI 1.1.; \


More information about the Gcc-patches mailing list