Bug 97498 - #pragma GCC diagnostic ignored "-Wunused-function" inconsistent
Summary: #pragma GCC diagnostic ignored "-Wunused-function" inconsistent
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: preprocessor (show other bugs)
Version: 9.3.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: diagnostic
: 95239 106267 (view as bug list)
Depends on:
Blocks:
 
Reported: 2020-10-19 23:49 UTC by Matthew Parker
Modified: 2022-10-19 20:51 UTC (History)
4 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2022-07-07 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Matthew Parker 2020-10-19 23:49:57 UTC
Ubuntu Linux 20.04


#define STR(x) #x
#define WIGNORE(x, instrs) \
	_Pragma("GCC diagnostic push") \
	_Pragma(STR(GCC diagnostic ignored #x)) \
	instrs \
	_Pragma("GCC diagnostic pop")

// REPLACE

int main(void) {return 1;}


When REPLACE is replaced with:

WIGNORE(-Wunused-function,
static int f(int a) {return a + 1;}
)

The command:

gcc main.c -Wall -Werror -Wextra

fails (-Werror=unused-function). However, breaking up the command into separate preprocessing and compilation stages:

gcc -E main.c -Wall -Werror -Wextra | gcc -x c -

passes.

When the macro is unwound by REPLACE being replaced with:

_Pragma("GCC diagnostic push");
_Pragma("GCC diagnostic ignored \"-Wunused-function\"");
static int f(int p) {return p + 1;}
_Pragma("GCC diagnostic pop");

The first command then passes. By adding a single backslash (\) right after the implementation of f, as is required to keep the WIGNORE macro all as one, the command fails. But how else can the macro be constructed?
Comment 1 Lewis Hyatt 2022-07-07 14:00:07 UTC
This is closely related to PR69543, but it's not quite the same. That PR is about the use of input_location when processing a "#pragma GCC diagnostic ignored"; this one is about the use of input_location when processing a pop rather.

In c-pragma.cc line ~943 we call

      diagnostic_pop_diagnostics (global_dc, input_location);

When processing the line:

static int f(int p) {return p + 1;} _Pragma("GCC diagnostic pop");

In C++ mode, input_location points to the _Pragma and so it works fine. In preprocess mode (gcc -E or gcc with -save-temps), it's fine also because the _Pragma turns into a #pragma on the next line  before being actually processed.

In C mode though, input_location points to the start of the line and hence the pop takes effect too early which leads to the issue.

It feels to me like we shouldn't use input_location anywhere in this function, rather the location of the relevant tokens, but I am not sure all the details of that, will see if anyone on PR69543 has ideas as well.

Here is a one-line test case that reveals the same problem with using input_location while processing the GCC diagnostic pragma, i.e. exactly PR69543's case:

======
static void f() {} _Pragma("GCC diagnostic error \"-Wunused-function\"")
======

Compiled without any other arguments, it works with gcc -x c++ and with gcc -save-temps, but it fails with gcc -x c.
Comment 2 Lewis Hyatt 2022-07-09 20:53:37 UTC
Patch submitted for review here: https://gcc.gnu.org/pipermail/gcc-patches/2022-July/598116.html
Comment 3 CVS Commits 2022-07-10 20:50:29 UTC
The master branch has been updated by Lewis Hyatt <lhyatt@gcc.gnu.org>:

https://gcc.gnu.org/g:0587cef3d7962a8b0f44779589ba2920dd3d71e5

commit r13-1596-g0587cef3d7962a8b0f44779589ba2920dd3d71e5
Author: Lewis Hyatt <lhyatt@gmail.com>
Date:   Sat Jul 9 16:12:21 2022 -0400

    c: Fix location for _Pragma tokens [PR97498]
    
    The handling of #pragma GCC diagnostic uses input_location, which is not always
    as precise as needed; in particular the relative location of some tokens and a
    _Pragma directive will crucially determine whether a given diagnostic is enabled
    or suppressed in the desired way. PR97498 shows how the C frontend ends up with
    input_location pointing to the beginning of the line containing a _Pragma()
    directive, resulting in the wrong behavior if the diagnostic to be modified
    pertains to some tokens found earlier on the same line. This patch fixes that by
    addressing two issues:
    
        a) libcpp was not assigning a valid location to the CPP_PRAGMA token
        generated by the _Pragma directive.
        b) C frontend was not setting input_location to something reasonable.
    
    With this change, the C frontend is able to change input_location to point to
    the _Pragma token as needed.
    
    This is just a two-line fix (one for each of a) and b)), the testsuite changes
    were needed only because the location on the tested warnings has been somewhat
    improved, so the tests need to look for the new locations.
    
    gcc/c/ChangeLog:
    
            PR preprocessor/97498
            * c-parser.cc (c_parser_pragma): Set input_location to the
            location of the pragma, rather than the start of the line.
    
    libcpp/ChangeLog:
    
            PR preprocessor/97498
            * directives.cc (destringize_and_run): Override the location of
            the CPP_PRAGMA token from a _Pragma directive to the location of
            the expansion point, as is done for the tokens lexed from it.
    
    gcc/testsuite/ChangeLog:
    
            PR preprocessor/97498
            * c-c++-common/pr97498.c: New test.
            * c-c++-common/gomp/pragma-3.c: Adapt for improved warning locations.
            * c-c++-common/gomp/pragma-5.c: Likewise.
            * gcc.dg/pragma-message.c: Likewise.
    
    libgomp/ChangeLog:
    
            * testsuite/libgomp.oacc-c-c++-common/reduction-5.c: Adapt for
            improved warning locations.
            * testsuite/libgomp.oacc-c-c++-common/vred2d-128.c: Likewise.
Comment 4 Lewis Hyatt 2022-07-10 20:51:42 UTC
Fixed for GCC 13.
Comment 5 Martin Liška 2022-07-12 09:47:15 UTC
*** Bug 106267 has been marked as a duplicate of this bug. ***
Comment 6 Martin Liška 2022-07-12 09:48:16 UTC
Am I correct that it's not something for backport to GCC 12 or any older version?
Comment 7 Martin Liška 2022-07-12 09:55:01 UTC
(In reply to Martin Liška from comment #6)
> Am I correct that it's not something for backport to GCC 12 or any older
> version?

We actually speak about 2 modified lines, so can we backport it, please?
Comment 8 Lewis Hyatt 2022-07-17 15:12:23 UTC
(In reply to Martin Liška from comment #7)
 
> We actually speak about 2 modified lines, so can we backport it, please?

It will work fine for any of 10, 11, 12. I'll ask for approval for that on gcc-patches then.
Comment 9 CVS Commits 2022-08-01 23:15:01 UTC
The releases/gcc-12 branch has been updated by Lewis Hyatt <lhyatt@gcc.gnu.org>:

https://gcc.gnu.org/g:98e2676558f6f50cfb90610e5a160642a24d1596

commit r12-8648-g98e2676558f6f50cfb90610e5a160642a24d1596
Author: Lewis Hyatt <lhyatt@gmail.com>
Date:   Sat Jul 9 16:12:21 2022 -0400

    c: Fix location for _Pragma tokens [PR97498]
    
    The handling of #pragma GCC diagnostic uses input_location, which is not always
    as precise as needed; in particular the relative location of some tokens and a
    _Pragma directive will crucially determine whether a given diagnostic is enabled
    or suppressed in the desired way. PR97498 shows how the C frontend ends up with
    input_location pointing to the beginning of the line containing a _Pragma()
    directive, resulting in the wrong behavior if the diagnostic to be modified
    pertains to some tokens found earlier on the same line. This patch fixes that by
    addressing two issues:
    
        a) libcpp was not assigning a valid location to the CPP_PRAGMA token
        generated by the _Pragma directive.
        b) C frontend was not setting input_location to something reasonable.
    
    With this change, the C frontend is able to change input_location to point to
    the _Pragma token as needed.
    
    This is just a two-line fix (one for each of a) and b)), the testsuite changes
    were needed only because the location on the tested warnings has been somewhat
    improved, so the tests need to look for the new locations.
    
    gcc/c/ChangeLog:
    
            PR preprocessor/97498
            * c-parser.cc (c_parser_pragma): Set input_location to the
            location of the pragma, rather than the start of the line.
    
    libcpp/ChangeLog:
    
            PR preprocessor/97498
            * directives.cc (destringize_and_run): Override the location of
            the CPP_PRAGMA token from a _Pragma directive to the location of
            the expansion point, as is done for the tokens lexed from it.
    
    gcc/testsuite/ChangeLog:
    
            PR preprocessor/97498
            * c-c++-common/pr97498.c: New test.
            * c-c++-common/gomp/pragma-3.c: Adapt for improved warning locations.
            * c-c++-common/gomp/pragma-5.c: Likewise.
            * gcc.dg/pragma-message.c: Likewise.
    
    libgomp/ChangeLog:
    
            * testsuite/libgomp.oacc-c-c++-common/reduction-5.c: Adapt for
            improved warning locations.
            * testsuite/libgomp.oacc-c-c++-common/vred2d-128.c: Likewise.
    
    (cherry picked from commit 0587cef3d7962a8b0f44779589ba2920dd3d71e5)
Comment 10 CVS Commits 2022-08-02 22:25:03 UTC
The releases/gcc-10 branch has been updated by Lewis Hyatt <lhyatt@gcc.gnu.org>:

https://gcc.gnu.org/g:8d783f0f6955e6aa7af218490b068004669b09e0

commit r10-10926-g8d783f0f6955e6aa7af218490b068004669b09e0
Author: Lewis Hyatt <lhyatt@gmail.com>
Date:   Sat Jul 9 16:12:21 2022 -0400

    c: Fix location for _Pragma tokens [PR97498]
    
    The handling of #pragma GCC diagnostic uses input_location, which is not always
    as precise as needed; in particular the relative location of some tokens and a
    _Pragma directive will crucially determine whether a given diagnostic is enabled
    or suppressed in the desired way. PR97498 shows how the C frontend ends up with
    input_location pointing to the beginning of the line containing a _Pragma()
    directive, resulting in the wrong behavior if the diagnostic to be modified
    pertains to some tokens found earlier on the same line. This patch fixes that by
    addressing two issues:
    
        a) libcpp was not assigning a valid location to the CPP_PRAGMA token
        generated by the _Pragma directive.
        b) C frontend was not setting input_location to something reasonable.
    
    With this change, the C frontend is able to change input_location to point to
    the _Pragma token as needed.
    
    This is just a two-line fix (one for each of a) and b)), the testsuite changes
    were needed only because the location on the tested warnings has been somewhat
    improved, so the tests need to look for the new locations.
    
    gcc/c/ChangeLog:
    
            PR preprocessor/97498
            * c-parser.c (c_parser_pragma): Set input_location to the
            location of the pragma, rather than the start of the line.
    
    libcpp/ChangeLog:
    
            PR preprocessor/97498
            * directives.c (destringize_and_run): Override the location of
            the CPP_PRAGMA token from a _Pragma directive to the location of
            the expansion point, as is done for the tokens lexed from it.
    
    gcc/testsuite/ChangeLog:
    
            PR preprocessor/97498
            * c-c++-common/pr97498.c: New test.
            * gcc.dg/pragma-message.c: Adapt for improved warning locations.
    
    (cherry picked from commit 0587cef3d7962a8b0f44779589ba2920dd3d71e5)
Comment 11 CVS Commits 2022-08-02 22:26:14 UTC
The releases/gcc-11 branch has been updated by Lewis Hyatt <lhyatt@gcc.gnu.org>:

https://gcc.gnu.org/g:d431d48c4e503c3ff17ccd51e504877f3fc9ce0e

commit r11-10188-gd431d48c4e503c3ff17ccd51e504877f3fc9ce0e
Author: Lewis Hyatt <lhyatt@gmail.com>
Date:   Sat Jul 9 16:12:21 2022 -0400

    c: Fix location for _Pragma tokens [PR97498]
    
    The handling of #pragma GCC diagnostic uses input_location, which is not always
    as precise as needed; in particular the relative location of some tokens and a
    _Pragma directive will crucially determine whether a given diagnostic is enabled
    or suppressed in the desired way. PR97498 shows how the C frontend ends up with
    input_location pointing to the beginning of the line containing a _Pragma()
    directive, resulting in the wrong behavior if the diagnostic to be modified
    pertains to some tokens found earlier on the same line. This patch fixes that by
    addressing two issues:
    
        a) libcpp was not assigning a valid location to the CPP_PRAGMA token
        generated by the _Pragma directive.
        b) C frontend was not setting input_location to something reasonable.
    
    With this change, the C frontend is able to change input_location to point to
    the _Pragma token as needed.
    
    This is just a two-line fix (one for each of a) and b)), the testsuite changes
    were needed only because the location on the tested warnings has been somewhat
    improved, so the tests need to look for the new locations.
    
    gcc/c/ChangeLog:
    
            PR preprocessor/97498
            * c-parser.c (c_parser_pragma): Set input_location to the
            location of the pragma, rather than the start of the line.
    
    libcpp/ChangeLog:
    
            PR preprocessor/97498
            * directives.c (destringize_and_run): Override the location of
            the CPP_PRAGMA token from a _Pragma directive to the location of
            the expansion point, as is done for the tokens lexed from it.
    
    gcc/testsuite/ChangeLog:
    
            PR preprocessor/97498
            * c-c++-common/pr97498.c: New test.
            * gcc.dg/pragma-message.c: Adapt for improved warning locations.
    
    (cherry picked from commit 0587cef3d7962a8b0f44779589ba2920dd3d71e5)
Comment 12 Lewis Hyatt 2022-10-19 20:51:57 UTC
*** Bug 95239 has been marked as a duplicate of this bug. ***