public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
From: "lhyatt at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug preprocessor/60014] Bad warning suppression caused by track-macro-expansion when not using integrated cpp
Date: Thu, 06 Oct 2022 21:51:52 +0000	[thread overview]
Message-ID: <bug-60014-4-LCt4Y2Chpx@http.gcc.gnu.org/bugzilla/> (raw)
In-Reply-To: <bug-60014-4@http.gcc.gnu.org/bugzilla/>

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60014

Lewis Hyatt <lhyatt at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |lhyatt at gcc dot gnu.org

--- Comment #8 from Lewis Hyatt <lhyatt at gcc dot gnu.org> ---
The testcase for this PR was one of many that got fixed by r9-1926 (for
PR69558). The location of the token resulting from expanding the builtin macro
__LINE__ was, prior to r9-1926, pointing to the closing paren of the macro
invocation, i.e. not in the system header. After r9-1926, the location of the
token is a virtual location encoding that the token resulted from expansion of
a macro defined in a system header, and so the "system"-ness of the token no
longer gets lost.

Fredrik's original testcase is a nice one. Every element in it is essential to
reveal the issue, including the extra semicolon in the FOO macro and the
newline in the invocation. Although that testcase now works correctly, Manuel's
point still stands, c-ppoutput.cc should not have behaved this way, even absent
r9-1926. The problem is that here:

======
      if (do_line_adjustments
          && !in_pragma
          && !line_marker_emitted
          && print.prev_was_system_token != !!in_system_header_at (loc)
          && !is_location_from_builtin_token (loc))
        /* The system-ness of this token is different from the one of
           the previous token.  Let's emit a line change to mark the
           new system-ness before we emit the token.  */
        {
          do_line_change (pfile, token, loc, false);
          print.prev_was_system_token = !!in_system_header_at (loc);
        }
=======

print.prev_was_system_token should be set always, not only when the if
statement is reached and evaluates to true. In this PR's testcase prior to
r9-1926, the check evaluated to false when streaming the semicolon from the
macro expansion, because a line marker had been printed due to the fact that
the __LINE__ token and the semicolon were assigned locations on different
lines.

So the logic in c-ppoutput.cc assumes that you can never get two tokens on
different lines without a line change callback, which is not a crazy assumption
but was violated due to the issue fixed by r9-1926.

However, there are other code paths besides the line change logic that can
trigger the same issue still now. One way is to stream a deferred CPP_PRAGMA
token, since that code path doesn't even execute the above if statement. As of
r13-1544, we do see such tokens while preprocessing, so here is a modified
testcase that fails on master:

======
$ cat t2.h
#pragma GCC system_header
#define X _Pragma("GCC diagnostic push");

$ cat t2.c
#include "./t2.h"
X
const char* should_warn = 1;

$ gcc -Wint-conversion -c t2.c
t2.c:3:27: warning: initialization of ‘const char *’ from ‘int’ makes pointer
from integer without a cast [-Wint-conversion]
    3 | const char* should_warn = 1;
      |                           ^
$ gcc -Wint-conversion -c t2.c -save-temps
$
======

I can test the fix and prepare a patch for that.

  parent reply	other threads:[~2022-10-06 21:51 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-01-31 21:27 [Bug c/60014] New: Bad warning suppression megahallon at gmail dot com
2014-06-10  6:30 ` [Bug preprocessor/60014] Bad warning suppression caused by track-macro-expansion when not using integrated cpp peff at peff dot net
2014-06-10 10:24 ` manu at gcc dot gnu.org
2015-04-28 19:47 ` megahallon at gmail dot com
2022-10-06 21:51 ` lhyatt at gcc dot gnu.org [this message]
2022-10-12 22:16 ` cvs-commit at gcc dot gnu.org
2022-10-12 22:17 ` lhyatt at gcc dot gnu.org

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=bug-60014-4-LCt4Y2Chpx@http.gcc.gnu.org/bugzilla/ \
    --to=gcc-bugzilla@gcc.gnu.org \
    --cc=gcc-bugs@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).