Bug 69543 - [10/11/12/13 Regression] _Pragma does not apply within macro
Summary: [10/11/12/13 Regression] _Pragma does not apply within macro
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: preprocessor (show other bugs)
Version: 6.0
: P3 normal
Target Milestone: 11.4
Assignee: David Malcolm
URL:
Keywords: diagnostic
Depends on:
Blocks:
 
Reported: 2016-01-28 19:57 UTC by David Malcolm
Modified: 2022-09-30 18:12 UTC (History)
5 users (show)

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


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description David Malcolm 2016-01-28 19:57:29 UTC
https://gcc.gnu.org/ml/gcc-patches/2016-01/msg02243.html
reported a regression introduced by r232893 (for PR preprocessor/69126)

Minimal reproducer seems to be:
$ cat /tmp/test.c
# define YY_IGNORE_MAYBE_UNINITIALIZED_BEGIN \
    _Pragma ("GCC diagnostic push") \
    _Pragma ("GCC diagnostic ignored \"-Wuninitialized\"")\
    _Pragma ("GCC diagnostic ignored \"-Wmaybe-uninitialized\"")
# define YY_IGNORE_MAYBE_UNINITIALIZED_END \
    _Pragma ("GCC diagnostic pop")

void test (char yylval)
{
  char *yyvsp;
  YY_IGNORE_MAYBE_UNINITIALIZED_BEGIN
  *++yyvsp = yylval;
  YY_IGNORE_MAYBE_UNINITIALIZED_END
}

$ ./xgcc -B. -c /tmp/test.c -Wall
/tmp/test.c: In function 'test':
/tmp/test.c:12:12: warning: 'yyvsp' is used uninitialized in this
function [-Wuninitialized]
   *++yyvsp = yylval;
   ~~~~~~~~~^~~~~~~~

$ ./xgcc -B. -c /tmp/test.c -save-temps
$ ./xgcc -B. -c test.i -Wall
(compiles with no warnings)
Comment 1 David Malcolm 2016-01-28 22:32:29 UTC
Breakpoint 4, linemap_compare_locations (set=0x7ffff7ff6000, pre=2147483641, post=post@entry=2147483656) at ../../src/libcpp/line-map.c:1326

(gdb) call inform (2147483641, "pre=2147483641")
../../src/gcc/testsuite/c-c++-common/pr69453.c: In function ‘test’:
../../src/gcc/testsuite/c-c++-common/pr69453.c:6:5: note: pre=2147483641
     _Pragma ("GCC diagnostic ignored \"-Wmaybe-uninitialized\"")
     ^
../../src/gcc/testsuite/c-c++-common/pr69453.c:13:3: note: in expansion of macro ‘YY_IGNORE_MAYBE_UNINITIALIZED_BEGIN’
   YY_IGNORE_MAYBE_UNINITIALIZED_BEGIN
   ^
(gdb) p /x 2147483641
$15 = 0x7ffffff9

(gdb) call inform (2147483656, "post=2147483656")
../../src/gcc/testsuite/c-c++-common/pr69453.c:14:12: note: post=2147483656
   *++yyvsp = yylval;
   ~~~~~~~~~^~~~~~~~
(gdb) p /x 2147483656
$8 = 0x80000008

(gdb) p l0
$16 = 2147483641
(gdb) p l1
$17 = 312096
(gdb) p /x l0
$18 = 0x7ffffff9

Then, after the call to:
1338	  if ((pre_virtual_p = linemap_location_from_macro_expansion_p (set, l0)))
1339	    l0 = linemap_resolve_location (set, l0,
1340					   LRK_MACRO_EXPANSION_POINT,
1341					   NULL);
1342	
we have:
(gdb) p /x l0
$21 = 0x80000004

so we have a comparison of locations, one (l0) from a macro expansion, the other not (l1).

linemap_compare_locations concludes by executing this:
1368	  return l1 - l0;
giving this rather nonsensical result:
Value returned is $25 = -2147171556

If I'm reading this right, it thus regards the "ignored" as happening *after* the source line of interest, hence the latter erroneously isn't affected by the _Pragma.
Comment 2 David Malcolm 2016-01-29 22:06:52 UTC
(In reply to David Malcolm from comment #1)
linemap_compare_locations does the right thing if passed a pair of ordinary locations, or if both locations are within the same macro expansion.

It generates nonsensical results if one of the locations is in a macro map, which is what's happening here (the location of the _Pragma, after r232893)
Comment 3 Jakub Jelinek 2016-02-01 22:36:38 UTC
Author: jakub
Date: Mon Feb  1 22:36:07 2016
New Revision: 233058

URL: https://gcc.gnu.org/viewcvs?rev=233058&root=gcc&view=rev
Log:
	PR preprocessor/69543
	PR c/69558
	* c-pragma.c (handle_pragma_diagnostic): Pass input_location
	instead of loc to control_warning_option.

	* gcc.dg/pr69543.c: New test.
	* gcc.dg/pr69558.c: New test.

Added:
    trunk/gcc/testsuite/gcc.dg/pr69543.c
    trunk/gcc/testsuite/gcc.dg/pr69558.c
Modified:
    trunk/gcc/c-family/ChangeLog
    trunk/gcc/c-family/c-pragma.c
    trunk/gcc/testsuite/ChangeLog
Comment 4 Jakub Jelinek 2016-02-01 22:43:23 UTC
Workaround applied, keeping this open to fix it for real for GCC 7.
Comment 5 David Malcolm 2016-02-23 17:39:48 UTC
Author: dmalcolm
Date: Tue Feb 23 17:39:16 2016
New Revision: 233637

URL: https://gcc.gnu.org/viewcvs?rev=233637&root=gcc&view=rev
Log:
Add test coverage for _Pragma (PR preprocessor 69126, 69543, 69558)

We had some regressions in the ability for _Pragma to disable a warning
(PR preprocessor/69126, PR preprocessor/69543, PR preprocessor/69558).

This patch attempts to add more test coverage for this, for the
various combinations of:
  - various warnings:
    -Wunused-variable
    -Wuninitialized
    -Wdeprecated-declarations
  - various combinations of location of _Pragma relative to location of
    the warning:
     - _Pragma is in a macro, warning isn't a macro
     - neither is in a macro
     - _Pragma isnt't in a macro, warning is in a macro
     - in different macros
     - both in the same macro
  - C vs C++ frontend.

It adds some XFAILs:
  - pr69543-1.c for C++ (fixed in the followup patch)
  - pr69543-3.c for both C and C++
  - pr69543-4.c for both C and C++
  - pr69558.c for C++ (moving it from gcc.dg to c-c++-common,
    marking it as xfail for C++ for now)

gcc/testsuite/ChangeLog:
	PR preprocessor/69126
	PR preprocessor/69543
	PR preprocessor/69558
	* c-c++-common/pr69126.c (MACRO_1, test_1): New.
	(f): Rename to...
	(test_2): ...this, and add leading comment.
	(MACRO_3, test_3): New.
	(MACRO_4A, MACRO_4B, test_4): New.
	(MACRO): Rename to...
	(MACRO_5): ...this.
	(g): Rename to...
	(test_5): ...this, updating for renaming of MACRO, and
	add leading comment.
	* c-c++-common/pr69543-1.c: New.
	* c-c++-common/pr69543-2.c: New.
	* c-c++-common/pr69543-3.c: New.
	* c-c++-common/pr69543-4.c: New.
	* c-c++-common/pr69558-1.c: New.
	* c-c++-common/pr69558-2.c: New.
	* c-c++-common/pr69558-3.c: New.
	* c-c++-common/pr69558-4.c: New.
	* gcc.dg/pr69558.c: Move to...
	* c-c++-common/pr69558.c: ...here.  Add dg-bogus directives, with
	xfail for c++.


Added:
    trunk/gcc/testsuite/c-c++-common/pr69543-1.c
    trunk/gcc/testsuite/c-c++-common/pr69543-2.c
    trunk/gcc/testsuite/c-c++-common/pr69543-3.c
    trunk/gcc/testsuite/c-c++-common/pr69543-4.c
    trunk/gcc/testsuite/c-c++-common/pr69558-1.c
    trunk/gcc/testsuite/c-c++-common/pr69558-2.c
    trunk/gcc/testsuite/c-c++-common/pr69558-3.c
    trunk/gcc/testsuite/c-c++-common/pr69558-4.c
    trunk/gcc/testsuite/c-c++-common/pr69558.c
      - copied, changed from r233636, trunk/gcc/testsuite/gcc.dg/pr69558.c
Removed:
    trunk/gcc/testsuite/gcc.dg/pr69558.c
Modified:
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/c-c++-common/pr69126.c
Comment 6 David Malcolm 2016-02-23 17:45:00 UTC
Author: dmalcolm
Date: Tue Feb 23 17:44:28 2016
New Revision: 233638

URL: https://gcc.gnu.org/viewcvs?rev=233638&root=gcc&view=rev
Log:
PR preprocessor/69126: avoid comparing ad-hoc and non-ad-hoc locations

gcc/testsuite/ChangeLog:
	PR preprocessor/69126
	PR preprocessor/69543
	* c-c++-common/pr69126-2-long.c: New test.
	* c-c++-common/pr69126-2-short.c: New test.
	* c-c++-common/pr69543-1.c: Remove xfail.

libcpp/ChangeLog:
	PR preprocessor/69126
	PR preprocessor/69543
	* line-map.c (linemap_compare_locations): At the function top,
	replace inlined bodies of get_location_from_adhoc_loc with calls
	to get_location_from_adhoc_loc.  Add a pair of calls to
	get_location_from_adhoc_loc at the bottom of the function, to
	avoid meaningless comparisons of ad-hoc and non-ad-hoc locations.


Added:
    trunk/gcc/testsuite/c-c++-common/pr69126-2-long.c
    trunk/gcc/testsuite/c-c++-common/pr69126-2-short.c
Modified:
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/c-c++-common/pr69543-1.c
    trunk/libcpp/ChangeLog
    trunk/libcpp/line-map.c
Comment 7 Aldy Hernandez 2016-12-07 18:40:23 UTC
Is still an issue, or has it been properly fixed with dmalcolm's latest commits to the PR?
Comment 8 David Malcolm 2017-01-19 21:26:26 UTC
The following testcases still have xfails:
  c-c++-common/pr69543-3.c
  c-c++-common/pr69543-4.c
so this isn't quite fixed yet.
Comment 9 David Malcolm 2017-01-19 21:30:45 UTC
(In reply to David Malcolm from comment #8)
> The following testcases still have xfails:
>   c-c++-common/pr69543-3.c
>   c-c++-common/pr69543-4.c
> so this isn't quite fixed yet.

These XFAILs are fixed (for both C and C++) by deleting these lines within tree-ssa-uninit.c:warn_uninit:

175	  location = linemap_resolve_location (line_table, location,
176					       LRK_SPELLING_LOCATION, NULL);

so that the location used for the warning retains any virtual location, rather than flattening it to the physical location.  Am not yet sure of the impact of such a change.
Comment 10 David Malcolm 2017-01-19 21:44:40 UTC
The lines in comment #9 came from 18f0e0e551a995687e1822aabb9b7d7ee8f11492
aka r186971 (affecting gcc.dg/cpp/pragma-diagnostic-2.c)
Comment 11 David Malcolm 2017-01-19 21:58:14 UTC
(In reply to David Malcolm from comment #10)
> The lines in comment #9 came from 18f0e0e551a995687e1822aabb9b7d7ee8f11492
> aka r186971 (affecting gcc.dg/cpp/pragma-diagnostic-2.c)

This was:
  "[PATCH 07/11] Fix -Wuninitialized for -ftrack-macro-expansion"
  * "https://gcc.gnu.org/ml/gcc-patches/2012-04/msg00574.html'

as part of this kit:
  "Patches to enable -ftrack-macro-expansion by default"
    * https://gcc.gnu.org/ml/gcc-patches/2012-04/msg00532.html
Comment 12 Jakub Jelinek 2018-05-02 10:04:37 UTC
GCC 8.1 has been released.
Comment 13 Jakub Jelinek 2018-07-26 11:00:01 UTC
GCC 8.2 has been released.
Comment 14 Martin Liška 2018-11-19 14:10:12 UTC
David: Can the bug be marked as resolved?
Comment 15 Jakub Jelinek 2019-02-22 15:20:05 UTC
GCC 8.3 has been released.
Comment 16 Jakub Jelinek 2020-03-04 09:35:58 UTC
GCC 8.4.0 has been released, adjusting target milestone.
Comment 17 Jakub Jelinek 2021-04-27 11:37:49 UTC
GCC 11.1 has been released, retargeting bugs to GCC 11.2.
Comment 18 Richard Biener 2021-07-28 07:04:24 UTC
GCC 11.2 is being released, retargeting bugs to GCC 11.3
Comment 19 Richard Biener 2022-04-21 07:47:38 UTC
GCC 11.3 is being released, retargeting bugs to GCC 11.4.
Comment 20 Lewis Hyatt 2022-07-07 14:05:30 UTC
See PR97498 which is closely related to this. It seems that the use of input_location while handling diagnostic pragmas is problematic in other cases besides the one discussed here (i.e. also when processing a diagnostic pop.)

Here is a test case from that PR, which breaks because of the input_location workaround discussed here though:

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

The above errors for C but not C++, because in C, input_location points to the start of the line and so it takes effect too early.
Comment 21 GCC Commits 2022-09-30 18:10:28 UTC
The master branch has been updated by Lewis Hyatt <lhyatt@gcc.gnu.org>:

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

commit r13-2994-gb52b99b62df8fc9b3a3010cb0a8cf49bc35037f0
Author: Lewis Hyatt <lhyatt@gmail.com>
Date:   Fri Sep 30 14:10:00 2022 -0400

    diagnostics: Fix virtual location for -Wuninitialized [PR69543]
    
    Warnings issued for -Wuninitialized have been using the spelling location of
    the problematic usage, discarding any information on the location of the macro
    expansion point if such usage was in a macro. This makes the warnings
    impossible to control reliably with #pragma GCC diagnostic, and also discards
    useful context in the diagnostic output. There seems to be no need to discard
    the virtual location information, so this patch fixes that.
    
    PR69543 was mostly about _Pragma issues which have been fixed for many years
    now. The PR remains open because two of the testcases added in response to it
    still have xfails, but those xfails have nothing to do with _Pragma and rather
    just with the issue fixed by this patch, so the PR can be closed now as well.
    
    The other testcase modified here, pragma-diagnostic-2.c, was explicitly
    testing for the undesirable behavior that was xfailed in pr69543-3.c. I have
    adjusted that and also added a new testcase verifying all 3 types of warning
    that come from tree-ssa-uninit.cc get the proper location information now.
    
    gcc/ChangeLog:
    
            PR preprocessor/69543
            * tree-ssa-uninit.cc (warn_uninit): Stop stripping macro tracking
            information away from the diagnostic location.
            (maybe_warn_read_write_only): Likewise.
            (maybe_warn_operand): Likewise.
    
    gcc/testsuite/ChangeLog:
    
            PR preprocessor/69543
            * c-c++-common/pr69543-3.c: Remove xfail.
            * c-c++-common/pr69543-4.c: Likewise.
            * gcc.dg/cpp/pragma-diagnostic-2.c: Adjust test for new behavior.
            * c-c++-common/pragma-diag-16.c: New test.