[PATCH] Fix -E preprocessing of pragmas (PR preprocessor/57580)

[PATCH] Fix -E preprocessing of pragmas (PR preprocessor/57580)

[Jakub Jelinek]

Hi!

As the testcases show, we can happily emit e.g.
  { #pragma omp single
in the -E preprocessed output, which is not valid C/C++,
the preprocessing directives have to be at the beginning of line
or there can be optional whitespace before them.  But for -fpreprocessed
input there is even a stronger requirement - they have to be at the
beginning of line.

c-ppoutput.c already had a print.printed flag for it, but it has not
been updated in all the spots where it should be.  The following patch
ensures that print.printed is true if the last printed character is not a
newline (except for short chunks of code that obviously don't care about the
value of the flag).

I've compared preprocessed dwarf2out.c before/after the patch (both -E and
-E -P), and the patch makes no difference for that case.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2015-12-02  Jakub Jelinek  <jakub@redhat.com>

	PR preprocessor/57580
	* c-ppoutput.c (print): Change printed field to bool.
	Move src_file last for smaller padding.
	(init_pp_output): Set print.printed to false instead of 0.
	(scan_translation_unit): Fix up formatting.  Set print.printed
	to true after printing something other than newline.
	(scan_translation_unit_trad): Set print.printed to true instead of 1.
	(maybe_print_line_1): Set print.printed to false instead of 0.
	(print_line_1): Likewise.
	(do_line_change): Set print.printed to true instead of 1.
	(cb_define, dump_queued_macros, cb_include, cb_def_pragma,
	dump_macro): Set print.printed to false after printing newline.

	* c-c++-common/cpp/pr57580.c: New test.
	* c-c++-common/gomp/pr57580.c: New test.

--- gcc/c-family/c-ppoutput.c.jj	2015-11-14 19:35:34.000000000 +0100
+++ gcc/c-family/c-ppoutput.c	2015-12-02 16:50:54.345754775 +0100
@@ -31,11 +31,11 @@ static struct
   const cpp_token *prev;	/* Previous token.  */
   const cpp_token *source;	/* Source token for spacing.  */
   int src_line;			/* Line number currently being written.  */
-  unsigned char printed;	/* Nonzero if something output at line.  */
+  bool printed;			/* True if something output at line.  */
   bool first_time;		/* pp_file_change hasn't been called yet.  */
-  const char *src_file;		/* Current source file.  */
   bool prev_was_system_token;	/* True if the previous token was a
 				   system token.*/
+  const char *src_file;		/* Current source file.  */
 } print;
 
 /* Defined and undefined macros being queued for output with -dU at
@@ -153,7 +153,7 @@ init_pp_output (FILE *out_stream)
 
   /* Initialize the print structure.  */
   print.src_line = 1;
-  print.printed = 0;
+  print.printed = false;
   print.prev = 0;
   print.outf = out_stream;
   print.first_time = 1;
@@ -206,12 +206,16 @@ scan_translation_unit (cpp_reader *pfile
 	    {
 	      line_marker_emitted = do_line_change (pfile, token, loc, false);
 	      putc (' ', print.outf);
+	      print.printed = true;
 	    }
 	  else if (print.source->flags & PREV_WHITE
 		   || (print.prev
 		       && cpp_avoid_paste (pfile, print.prev, token))
 		   || (print.prev == NULL && token->type == CPP_HASH))
-	    putc (' ', print.outf);
+	    {
+	      putc (' ', print.outf);
+	      print.printed = true;
+	    }
 	}
       else if (token->flags & PREV_WHITE)
 	{
@@ -222,6 +226,7 @@ scan_translation_unit (cpp_reader *pfile
 	      && !in_pragma)
 	    line_marker_emitted = do_line_change (pfile, token, loc, false);
 	  putc (' ', print.outf);
+	  print.printed = true;
 	}
 
       avoid_paste = false;
@@ -239,7 +244,7 @@ scan_translation_unit (cpp_reader *pfile
 	    fprintf (print.outf, "%s %s", space, name);
 	  else
 	    fprintf (print.outf, "%s", name);
-	  print.printed = 1;
+	  print.printed = true;
 	  in_pragma = true;
 	}
       else if (token->type == CPP_PRAGMA_EOL)
@@ -250,23 +255,23 @@ scan_translation_unit (cpp_reader *pfile
       else
 	{
 	  if (cpp_get_options (parse_in)->debug)
-	      linemap_dump_location (line_table, token->src_loc,
-				     print.outf);
+	    linemap_dump_location (line_table, token->src_loc, print.outf);
 
 	  if (do_line_adjustments
 	      && !in_pragma
 	      && !line_marker_emitted
-	      && print.prev_was_system_token != !!in_system_header_at(loc)
+	      && print.prev_was_system_token != !!in_system_header_at (loc)
 	      && !is_location_from_builtin_token (loc))
 	    /* The system-ness of this token is different from the one
 	       of the previous token.  Let's emit a line change to
 	       mark the new system-ness before we emit the token.  */
 	    {
 	      do_line_change (pfile, token, loc, false);
-	      print.prev_was_system_token = !!in_system_header_at(loc);
+	      print.prev_was_system_token = !!in_system_header_at (loc);
 	    }
 	  cpp_output_token (token, print.outf);
 	  line_marker_emitted = false;
+	  print.printed = true;
 	}
 
       /* CPP_COMMENT tokens and raw-string literal tokens can
@@ -316,7 +321,7 @@ scan_translation_unit_trad (cpp_reader *
       size_t len = pfile->out.cur - pfile->out.base;
       maybe_print_line (pfile->out.first_line);
       fwrite (pfile->out.base, 1, len, print.outf);
-      print.printed = 1;
+      print.printed = true;
       if (!CPP_OPTION (pfile, discard_comments))
 	account_for_newlines (pfile->out.base, len);
     }
@@ -339,7 +344,7 @@ maybe_print_line_1 (source_location src_
     {
       putc ('\n', stream);
       print.src_line++;
-      print.printed = 0;
+      print.printed = false;
     }
 
   if (!flag_no_line_commands
@@ -385,7 +390,7 @@ print_line_1 (source_location src_loc, c
   /* End any previous line of text.  */
   if (print.printed)
     putc ('\n', stream);
-  print.printed = 0;
+  print.printed = false;
 
   if (!flag_no_line_commands)
     {
@@ -460,7 +465,7 @@ do_line_change (cpp_reader *pfile, const
   if (!CPP_OPTION (pfile, traditional))
     {
       int spaces = LOCATION_COLUMN (src_loc) - 2;
-      print.printed = 1;
+      print.printed = true;
 
       while (-- spaces >= 0)
 	putc (' ', print.outf);
@@ -503,6 +508,7 @@ cb_define (cpp_reader *pfile, source_loc
     fputs ((const char *) NODE_NAME (node), print.outf);
 
   putc ('\n', print.outf);
+  print.printed = false;
   linemap_resolve_location (line_table, line,
 			    LRK_MACRO_DEFINITION_LOCATION,
 			    &map);
@@ -554,7 +560,7 @@ dump_queued_macros (cpp_reader *pfile AT
     {
       putc ('\n', print.outf);
       print.src_line++;
-      print.printed = 0;
+      print.printed = false;
     }
 
   for (q = define_queue; q;)
@@ -563,6 +569,7 @@ dump_queued_macros (cpp_reader *pfile AT
       fputs ("#define ", print.outf);
       fputs (q->macro, print.outf);
       putc ('\n', print.outf);
+      print.printed = false;
       print.src_line++;
       oq = q;
       q = q->next;
@@ -606,6 +613,7 @@ cb_include (cpp_reader *pfile ATTRIBUTE_
     }
 
   putc ('\n', print.outf);
+  print.printed = false;
   print.src_line++;
 }
 
@@ -671,6 +679,7 @@ cb_def_pragma (cpp_reader *pfile, source
   maybe_print_line (line);
   fputs ("#pragma ", print.outf);
   cpp_output_line (pfile, print.outf);
+  print.printed = false;
   print.src_line++;
 }
 
@@ -684,6 +693,7 @@ dump_macro (cpp_reader *pfile, cpp_hashn
       fputs ((const char *) cpp_macro_definition (pfile, node),
 	     print.outf);
       putc ('\n', print.outf);
+      print.printed = false;
       print.src_line++;
     }
 
--- gcc/testsuite/c-c++-common/cpp/pr57580.c.jj	2015-12-02 16:25:06.646799972 +0100
+++ gcc/testsuite/c-c++-common/cpp/pr57580.c	2015-12-02 16:24:50.000000000 +0100
@@ -0,0 +1,9 @@
+/* PR preprocessor/57580 */
+/* { dg-do compile } */
+/* { dg-options "-save-temps" } */
+
+#define MSG 	\
+  _Pragma("message(\"message0\")")	\
+  _Pragma("message(\"message1\")")
+MSG	/* { dg-message "message0" } */
+/* { dg-message "message1" "" { target *-*-* } 8 } */
--- gcc/testsuite/c-c++-common/gomp/pr57580.c.jj	2015-12-02 16:19:01.538015106 +0100
+++ gcc/testsuite/c-c++-common/gomp/pr57580.c	2015-12-02 16:18:46.000000000 +0100
@@ -0,0 +1,36 @@
+/* PR preprocessor/57580 */
+/* { dg-do compile } */
+/* { dg-options "-fopenmp -save-temps -fdump-tree-gimple" } */
+
+#define PS \
+  _Pragma("omp parallel num_threads(2)") \
+  { \
+    _Pragma("omp single") \
+    { \
+      ret = 0; \
+    } \
+  }
+
+int
+main ()
+{
+  int ret;
+  _Pragma("omp parallel num_threads(3)")
+  {
+    _Pragma("omp single")
+    {
+      ret = 0;
+    }
+  }
+  _Pragma("omp parallel num_threads(4)") { _Pragma("omp single") { ret = 0; } }
+  { _Pragma("omp parallel num_threads(5)") { _Pragma("omp single") { ret = 0; } } }
+  PS
+  PS
+  return ret;
+}
+
+/* { dg-final { scan-tree-dump-times "#pragma omp parallel\[^\n\r]*num_threads\\(2\\)" 2 "gimple" } } */
+/* { dg-final { scan-tree-dump-times "#pragma omp parallel\[^\n\r]*num_threads\\(3\\)" 1 "gimple" } } */
+/* { dg-final { scan-tree-dump-times "#pragma omp parallel\[^\n\r]*num_threads\\(4\\)" 1 "gimple" } } */
+/* { dg-final { scan-tree-dump-times "#pragma omp parallel\[^\n\r]*num_threads\\(5\\)" 1 "gimple" } } */
+/* { dg-final { scan-tree-dump-times "#pragma omp single" 5 "gimple" } } */

	Jakub

Re: [PATCH] Fix -E preprocessing of pragmas (PR preprocessor/57580)

[Joseph Myers]

On Wed, 2 Dec 2015, Jakub Jelinek wrote:

> Hi!
> 
> As the testcases show, we can happily emit e.g.
>   { #pragma omp single
> in the -E preprocessed output, which is not valid C/C++,
> the preprocessing directives have to be at the beginning of line
> or there can be optional whitespace before them.  But for -fpreprocessed
> input there is even a stronger requirement - they have to be at the
> beginning of line.
> 
> c-ppoutput.c already had a print.printed flag for it, but it has not
> been updated in all the spots where it should be.  The following patch
> ensures that print.printed is true if the last printed character is not a
> newline (except for short chunks of code that obviously don't care about the
> value of the flag).
> 
> I've compared preprocessed dwarf2out.c before/after the patch (both -E and
> -E -P), and the patch makes no difference for that case.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

-- 
Joseph S. Myers
joseph@codesourcery.com