public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] PR preprocessor/48532 (Wrong location in pragma involving macros)
@ 2011-04-10  7:46 Dodji Seketeli
  0 siblings, 0 replies; only message in thread
From: Dodji Seketeli @ 2011-04-10  7:46 UTC (permalink / raw)
  To: Tom Tromey; +Cc: GCC Patches

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

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);
-- 
		Dodji

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2011-04-10  7:46 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-04-10  7:46 [PATCH] PR preprocessor/48532 (Wrong location in pragma involving macros) Dodji Seketeli

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).