Patchwork PR preprocessor/48532 (Wrong location in pragma involving macros)

login
register
mail settings
Submitter Dodji Seketeli
Date April 10, 2011, 7:46 a.m.
Message ID <m3y63i1rcp.fsf@redhat.com>
Download mbox | patch
Permalink /patch/90497/
State New
Headers show

Comments

Dodji Seketeli - April 10, 2011, 7:46 a.m.
Hello,

While looking at something else, I noticed that preprocessing this
code snippet:

cat -n test.c
~=~
void foo (void)
{
  int i1, j1, k1;
#define p parallel
#define P(x) private (x##1)
#define S(x) shared (x##1)
#define F(x) firstprivate (x##1)
#pragma omp p P(i) \
  S(j) \
  F(k)
  ;
}
~=~

yields:

cc1 -E -fopenmp test.c
~=~
# 1 "test.c"
# 1 "<interne>"
# 1 "<command-line>"
# 1 "test.c"
void foo (void)
{
  int i1, j1, k1;




       
# 33554432 "test.c"
                                                                                                                             
# 8 "test.c"
#pragma omp parallel private (i1) shared (j1) firstprivate (k1)


  ;
}
~=~


Note that the bogus line ...

    # 33554432 "test.c".

... should not be present in the preprocessed output

In scan_translation_unit,the call to cpp_get_token_with_location
yields a location equals to zero.

From there, bad things happen; basically maybe_print_line is passed a
zero location.  It looks up a location map for it, and that lookup
yields the map that was created for location 1 (for builtins).  The
file path of that location is "test.c" (hence the "test.c" file on the
wrong line) and the source line number is garbage, as that location
map was never used to map location 0.

I think cpp_get_token_with_location should not have returned a zero
location to begin with.

I tracked it down to the way we handle the pragma in do_pragma
(indirectly called by cpp_get_token_with_location).  While parsing the
name of the pragma (which is the macro 'p' here, as 'omp' is just the
namespace of the pragma) with cpp_get_token, we forget to record the
invocation location of the 'p' macro.  So when
cpp_get_token_with_location pokes at said invocation location, it gets
a value of zero.

This (morally one-liner) patch just sets invocation location there.

Bootstrapped and tested on x86_64-unknown-linux-gnu for
"c,ada,c++,fortran,java,lto,objc" and checking enabled, against trunk.

libcpp/

	* directives.c (do_pragma): Don't forget the invocation location
	when parsing the pragma name of a namespaced pragma directive.

gcc/testsuite/

	* gcc.dg/cpp/pragma-3.c: New test case.
---
 gcc/testsuite/gcc.dg/cpp/pragma-3.c |   39 +++++++++++++++++++++++++++++++++++
 libcpp/directives.c                 |   31 ++++++++++++++++++++++++++-
 2 files changed, 69 insertions(+), 1 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/cpp/pragma-3.c

Patch

diff --git a/gcc/testsuite/gcc.dg/cpp/pragma-3.c b/gcc/testsuite/gcc.dg/cpp/pragma-3.c
new file mode 100644
index 0000000..1d5fe9c
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/cpp/pragma-3.c
@@ -0,0 +1,39 @@ 
+/* 
+   { dg-options "-fopenmp" }
+   { dg-do preprocess }
+ */
+
+void foo (void)
+{
+  int i1, j1, k1;
+#define p parallel
+#define P(x) private (x##1)
+#define S(x) shared (x##1)
+#define F(x) firstprivate (x##1)
+#pragma omp \
+  p \
+  P(i) \
+  S(j) \
+  F(k)
+  ;
+}
+
+/* 
+   The bug here was that we had a line like:
+       # 33554432 "../../gcc/testsuite/gcc.dg/cpp/pragma-3.c"
+   
+   Before line:
+
+       #pragma omp parallel private (i1) shared (j1) firstprivate (k1)
+
+   Note the very big integer there.  Normally we should just have
+   this:
+   
+       # 13 "../../gcc/testsuite/gcc.dg/cpp/pragma-3.c"
+       #pragma omp parallel private (i1) shared (j1) firstprivate (k1)
+
+   So let's chech that we have no line with a number of 3 or more
+   digit after #:
+
+   { dg-final { scan-file-not pragma-3.i "# \[0-9\]{3} \[^\n\r\]*pragma-3.c" } }
+*/
diff --git a/libcpp/directives.c b/libcpp/directives.c
index f244ae5..27164ff 100644
--- a/libcpp/directives.c
+++ b/libcpp/directives.c
@@ -1360,7 +1360,36 @@  do_pragma (cpp_reader *pfile)
 	{
 	  bool allow_name_expansion = p->allow_expansion;
 	  if (allow_name_expansion)
-	    pfile->state.prevent_expansion--;
+	    {
+	      pfile->state.prevent_expansion--;
+	      /*
+		Kludge ahead.
+
+		Consider this code snippet:
+
+		#define P parallel
+		#pragma omp P for
+		... a for loop ...
+
+		Once we parsed the 'omp' namespace of the #pragma
+		directive, we then parse the 'P' token that represents the
+		pragma name.  P being a macro, it is expanded into the
+		resulting 'parallel' token.
+
+		At this point the 'p' variable contains the 'parallel'
+		pragma name.  And pfile->context->macro is non-null
+		because we are still right at the end of the macro
+		context of 'P'.  The problem is, if we are beeing
+		(indirectly) called by cpp_get_token_with_location,
+		that function might test pfile->context->macro to see
+		if we are in the context of a macro expansion, (and we
+		are) and then use pfile->invocation_location as the
+		location of the macro invocation.  So we must instruct
+		cpp_get_token below to set
+		pfile->invocation_location.  */
+	      pfile->set_invocation_location = true;
+	    }
+
 	  token = cpp_get_token (pfile);
 	  if (token->type == CPP_NAME)
 	    p = lookup_pragma_entry (p->u.space, token->val.node.node);