Bug 69543 - [7/8/9/10 Regression] _Pragma does not apply within macro
Summary: [7/8/9/10 Regression] _Pragma does not apply within macro
Status: WAITING
Alias: None
Product: gcc
Classification: Unclassified
Component: preprocessor (show other bugs)
Version: 6.0
: P3 normal
Target Milestone: 8.4
Assignee: David Malcolm
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-01-28 19:57 UTC by David Malcolm
Modified: 2019-02-22 15:20 UTC (History)
4 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.