From patchwork Fri Jun 3 17:46:26 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dodji Seketeli X-Patchwork-Id: 98615 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id 43CA5B6FB0 for ; Sat, 4 Jun 2011 03:46:48 +1000 (EST) Received: (qmail 10515 invoked by alias); 3 Jun 2011 17:46:47 -0000 Received: (qmail 10504 invoked by uid 22791); 3 Jun 2011 17:46:46 -0000 X-SWARE-Spam-Status: No, hits=-1.9 required=5.0 tests=AWL, BAYES_00, T_MIME_NO_TEXT, T_TVD_MIME_NO_HEADERS X-Spam-Check-By: sourceware.org Received: from seketeli.net (HELO ms.seketeli.net) (91.121.166.71) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 03 Jun 2011 17:46:29 +0000 Received: from localhost (torimasen.com [82.237.12.13]) by ms.seketeli.net (Postfix) with ESMTP id 3BAD1EA040; Fri, 3 Jun 2011 19:45:07 +0200 (CEST) Received: by localhost (Postfix, from userid 500) id CCBBE8E604B; Fri, 3 Jun 2011 19:46:26 +0200 (CEST) From: Dodji Seketeli To: Tom Tromey Cc: GCC Patches Subject: [PING] [PATCH] PR preprocessor/48532 (Wrong location in pragma involving macros) X-URL: http://www.seketeli.net/~dodji Mail-Followup-To: Tom Tromey , GCC Patches Date: Fri, 03 Jun 2011 19:46:26 +0200 Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.2 (gnu/linux) MIME-Version: 1.0 X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org I am pinging this patch first posted to http://gcc.gnu.org/ml/gcc-patches/2011-04/msg00697.html. 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 "" # 1 "" # 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);