From patchwork Sat Oct 8 21:18:04 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Lewis Hyatt X-Patchwork-Id: 1835 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a5d:4ac7:0:0:0:0:0 with SMTP id y7csp897937wrs; Sat, 8 Oct 2022 14:19:15 -0700 (PDT) X-Google-Smtp-Source: AMsMyM4CJHd6IeEIC0cJjpWYNY5ykLhwQUvj9l8WEaUzzbu/2l5sOcd1DV9D2LtCFvfmAEH/fbvH X-Received: by 2002:a17:907:75f2:b0:78d:49a6:5052 with SMTP id jz18-20020a17090775f200b0078d49a65052mr8672878ejc.680.1665263955500; Sat, 08 Oct 2022 14:19:15 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1665263955; cv=none; d=google.com; s=arc-20160816; b=bv68+Z+N2H6AtYBlTtba9YdlnqVbfWQYOmCJ1SXgIwhdcdjku5b1nOKs92MilMAHeW 3XsTbMCf16XWOAvx0OO2J5j9PI0aFyQSpSdrLKb2bKQdWTdxp/C1yqYVDdzbwGgRV4oC GIjUczeMErnfYzkr0KNoCxOydJ4+gOGpsXXpTXUBxaLHzoSKScC042cdZ5SAndLrLL04 5cxZYj/ORr2/Bjt7KJVCJIZjLIrk7yyhRg0P+w25puyV17FDGgOf6Cp2lLMsnApoUyxA wn8zvv1SdtYriUIrMiut2awloQLGOVPfNNurZ/YXYwAbjvm+apZ7BVbHh08lmT1nt9G3 XZSw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:reply-to:from:list-subscribe:list-help:list-post :list-archive:list-unsubscribe:list-id:precedence :content-transfer-encoding:mime-version:message-id:date:subject:to :dmarc-filter:delivered-to:dkim-signature:dkim-filter; bh=ltSuPlOLFDe7DKY4Navdj3U+eJ7Y9sw/YEskw+fAJOM=; b=Hg8hTbZ6mluz3qBDhKhXEqiq0TOHwBQ9h468FlnMt28c0ueVC3W1j1ZNpKZ4vH+XiF QrT3VvUP9kcJHRgsDYCgoIsdhucEd67v/LOeltnx028Hgy/jD16UVWuMLf0cVk7f3hMz EVmSZWXiKM26Qc35dRGtHXKLeCRJ1bo1B1M+AhLwWVTkCdKgz123++oxA5x3SujbXzAi rDUqlAq5pnvu/TPVDb7vgmzB0fIKoq+6u1/fuLa0lOb4H+sihtqLVZdbId/r+EavFhkt 1cDd9RxIrfI7tGHl+vFYOAtQbLgQ/hlhyGdTW7omO7U2OMsEy8tfXauZkSD8qqURdFWf rrrg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gcc.gnu.org header.s=default header.b=R1Fwq38V; spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 8.43.85.97 as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=gnu.org Received: from sourceware.org (ip-8-43-85-97.sourceware.org. [8.43.85.97]) by mx.google.com with ESMTPS id qw26-20020a1709066a1a00b0078262e3839esi6019193ejc.27.2022.10.08.14.19.14 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 08 Oct 2022 14:19:15 -0700 (PDT) Received-SPF: pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 8.43.85.97 as permitted sender) client-ip=8.43.85.97; Authentication-Results: mx.google.com; dkim=pass header.i=@gcc.gnu.org header.s=default header.b=R1Fwq38V; spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 8.43.85.97 as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=gnu.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 261C13858023 for ; Sat, 8 Oct 2022 21:19:14 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 261C13858023 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1665263954; bh=ltSuPlOLFDe7DKY4Navdj3U+eJ7Y9sw/YEskw+fAJOM=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=R1Fwq38V3e0uJb9U5MRs9Y89VF6jTUGJyvBD+yo0dy2224JySbLTeadCN3RJ9bL3u vHh0aqt9seSXV3VJXuti4Fz2SKwf1Ip2M8tEty/7qGhrMZqeUvAooQt2D3Dvat1fws 2fL2+aIxZmRNWpoTlhMcFFY+LJJUT1hfK92ooHDg= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mail-qv1-xf32.google.com (mail-qv1-xf32.google.com [IPv6:2607:f8b0:4864:20::f32]) by sourceware.org (Postfix) with ESMTPS id 8A90B3858CDB for ; Sat, 8 Oct 2022 21:18:31 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 8A90B3858CDB Received: by mail-qv1-xf32.google.com with SMTP id de14so5139322qvb.5 for ; Sat, 08 Oct 2022 14:18:31 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=ltSuPlOLFDe7DKY4Navdj3U+eJ7Y9sw/YEskw+fAJOM=; b=XWx/EFnc8Vo3fxeeFlG6VE7ja9E39wBZGE91EEq6P33GCobZr2t3zq5PVTEBG4jkGY cNOwTkBGvmjFdnsDOk/DSTXsb/ipA+GfGxgjn+UB+6yplp+PY2UK46PVLE/DKLNG4zTQ hrjZtoWl/JpDvse3rTgVeU10E0rmkfu4gAv+JEtNVf5UMtVpindMDF/wohNQoslDgd1t Rzb49kzrWat7ytjtWcya3SaKgwisVFNuNLZ26+SOcenozZWLSwxP/hFr//4hBO1mCcn9 3MQyqZ5fIrPSuOoX8egNwWLlfzIahTfROF/k2sHHGVv5NB1ytgc4nKyFnNKLQHwQ0RP9 CL/g== X-Gm-Message-State: ACrzQf3/loPnZiUsZ5dg83NL44sLpxB5eZC4JWKcIvAQKPWTeSe4xXWK 9zwxzx7j1nVCugD5pS9YsYUNVOlsRkE= X-Received: by 2002:a05:6214:20aa:b0:4b3:e0de:cbc2 with SMTP id 10-20020a05621420aa00b004b3e0decbc2mr2937761qvd.91.1665263910749; Sat, 08 Oct 2022 14:18:30 -0700 (PDT) Received: from localhost.localdomain (96-67-140-173-static.hfc.comcastbusiness.net. [96.67.140.173]) by smtp.gmail.com with ESMTPSA id bq18-20020a05620a469200b006bb0f9b89cfsm5672380qkb.87.2022.10.08.14.18.29 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 08 Oct 2022 14:18:30 -0700 (PDT) To: gcc-patches@gcc.gnu.org Subject: [PATCH] preprocessor: Fix tracking of system header state [PR60014, PR60723] Date: Sat, 8 Oct 2022 17:18:04 -0400 Message-Id: <5dce970b21e788deaa3d08f21995d8cb3cdb3752.1665263871.git.lhyatt@gmail.com> X-Mailer: git-send-email 2.25.1 MIME-Version: 1.0 X-Spam-Status: No, score=-3039.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Lewis Hyatt via Gcc-patches From: Lewis Hyatt Reply-To: Lewis Hyatt Errors-To: gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org Sender: "Gcc-patches" X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1746155817824225323?= X-GMAIL-MSGID: =?utf-8?q?1746155817824225323?= The token_streamer class (which implements gcc mode -E and -save-temps/-no-integrated-cpp) needs to keep track whether the last tokens output were in a system header, so that it can generate line marker annotations as necessary for a downstream consumer to reconstruct the state. The logic for tracking it, which was added by r5-1863 to resolve PR60723, has some edge case issues as revealed by the three new test cases. The first, coming from the original PR60014, was incidentally fixed by r9-1926 for unrelated reasons. The other two were still failing on master prior to this commit. Such code paths were not realizable prior to r13-1544, which made it possible for the token streamer to see CPP_PRAGMA tokens in more contexts. The two main issues being corrected here are: 1) print.prev_was_system_token needs to indicate whether the previous token output was in a system location. However, it was not being set on every token, only on those that triggered the main code path; specifically it was not triggered on a CPP_PRAGMA token. Testcase 2 covers this case. 2) The token_streamer uses a variable "line_marker_emitted" to remember whether a line marker has been emitted while processing a given token, so that it wouldn't be done more than once in case multiple conditions requiring a line marker are true. There was no reason for this to be a member variable that retains its value from token to token, since it is just needed for tracking the state locally while processing a single given token. The fact that it could retain its value for a subsequent token is rather difficult to observe, but testcase 3 demonstrates incorrect behavior resulting from that. Moving this to a local variable also simplifies understanding the control flow going forward. gcc/c-family/ChangeLog: PR preprocessor/60014 PR preprocessor/60723 * c-ppoutput.cc (class token_streamer): Remove member line_marker_emitted to... (token_streamer::stream): ...a local variable here. Set print.prev_was_system_token on all code paths. gcc/testsuite/ChangeLog: PR preprocessor/60014 PR preprocessor/60723 * gcc.dg/cpp/pr60014-1.c: New test. * gcc.dg/cpp/pr60014-1.h: New test. * gcc.dg/cpp/pr60014-2.c: New test. * gcc.dg/cpp/pr60014-2.h: New test. * gcc.dg/cpp/pr60014-3.c: New test. * gcc.dg/cpp/pr60014-3.h: New test. --- Notes: Hello- I tried to describe it all in the commit message, please see also https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60014#c8 for more details. bootstrap+regtest all languages looks good on x86-64 Linux. Please let me know if it looks OK? Thanks! -Lewis gcc/c-family/c-ppoutput.cc | 17 ++++++++++------- gcc/testsuite/gcc.dg/cpp/pr60014-1.c | 9 +++++++++ gcc/testsuite/gcc.dg/cpp/pr60014-1.h | 5 +++++ gcc/testsuite/gcc.dg/cpp/pr60014-2.c | 5 +++++ gcc/testsuite/gcc.dg/cpp/pr60014-2.h | 5 +++++ gcc/testsuite/gcc.dg/cpp/pr60014-3.c | 16 ++++++++++++++++ gcc/testsuite/gcc.dg/cpp/pr60014-3.h | 2 ++ 7 files changed, 52 insertions(+), 7 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/cpp/pr60014-1.c create mode 100644 gcc/testsuite/gcc.dg/cpp/pr60014-1.h create mode 100644 gcc/testsuite/gcc.dg/cpp/pr60014-2.c create mode 100644 gcc/testsuite/gcc.dg/cpp/pr60014-2.h create mode 100644 gcc/testsuite/gcc.dg/cpp/pr60014-3.c create mode 100644 gcc/testsuite/gcc.dg/cpp/pr60014-3.h diff --git a/gcc/c-family/c-ppoutput.cc b/gcc/c-family/c-ppoutput.cc index 98081ccfbb0..a99d9e9c5ca 100644 --- a/gcc/c-family/c-ppoutput.cc +++ b/gcc/c-family/c-ppoutput.cc @@ -184,15 +184,13 @@ class token_streamer bool avoid_paste; bool do_line_adjustments; bool in_pragma; - bool line_marker_emitted; public: token_streamer (cpp_reader *pfile) :avoid_paste (false), do_line_adjustments (cpp_get_options (pfile)->lang != CLK_ASM && !flag_no_line_commands), - in_pragma (false), - line_marker_emitted (false) + in_pragma (false) { gcc_assert (!print.streamer); print.streamer = this; @@ -227,7 +225,14 @@ token_streamer::stream (cpp_reader *pfile, const cpp_token *token, if (token->type == CPP_EOF) return; + /* Keep track when we move into and out of system locations. */ + const bool is_system_token = in_system_header_at (loc); + const bool system_state_changed + = (is_system_token != print.prev_was_system_token); + print.prev_was_system_token = is_system_token; + /* Subtle logic to output a space if and only if necessary. */ + bool line_marker_emitted = false; if (avoid_paste) { unsigned src_line = LOCATION_LINE (loc); @@ -301,19 +306,17 @@ token_streamer::stream (cpp_reader *pfile, const cpp_token *token, if (do_line_adjustments && !in_pragma && !line_marker_emitted - && print.prev_was_system_token != !!in_system_header_at (loc) + && system_state_changed && !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); + line_marker_emitted = do_line_change (pfile, token, loc, false); } if (!in_pragma || should_output_pragmas ()) { cpp_output_token (token, print.outf); - line_marker_emitted = false; print.printed = true; } } diff --git a/gcc/testsuite/gcc.dg/cpp/pr60014-1.c b/gcc/testsuite/gcc.dg/cpp/pr60014-1.c new file mode 100644 index 00000000000..de52b30c161 --- /dev/null +++ b/gcc/testsuite/gcc.dg/cpp/pr60014-1.c @@ -0,0 +1,9 @@ +/* { dg-do compile } */ +/* { dg-options "-save-temps -Wint-conversion" } */ +#include "pr60014-1.h" +int main () +{ + X(a, + b); + char *should_warn = 1; /* { dg-warning {-Wint-conversion} } */ +} diff --git a/gcc/testsuite/gcc.dg/cpp/pr60014-1.h b/gcc/testsuite/gcc.dg/cpp/pr60014-1.h new file mode 100644 index 00000000000..50c159c44ee --- /dev/null +++ b/gcc/testsuite/gcc.dg/cpp/pr60014-1.h @@ -0,0 +1,5 @@ +#pragma GCC system_header + +/* N.B. the semicolon in the macro definition is important, since it produces a + second token from this system header on the same line as the __LINE__ token. */ +#define X(a, b) __LINE__; diff --git a/gcc/testsuite/gcc.dg/cpp/pr60014-2.c b/gcc/testsuite/gcc.dg/cpp/pr60014-2.c new file mode 100644 index 00000000000..115c9858ec7 --- /dev/null +++ b/gcc/testsuite/gcc.dg/cpp/pr60014-2.c @@ -0,0 +1,5 @@ +/* { dg-do compile } */ +/* { dg-options "-save-temps -Wint-conversion" } */ +#include "pr60014-2.h" +X +char *should_warn = 1; /* { dg-warning {-Wint-conversion} } */ diff --git a/gcc/testsuite/gcc.dg/cpp/pr60014-2.h b/gcc/testsuite/gcc.dg/cpp/pr60014-2.h new file mode 100644 index 00000000000..455f1ed2e5b --- /dev/null +++ b/gcc/testsuite/gcc.dg/cpp/pr60014-2.h @@ -0,0 +1,5 @@ +#pragma GCC system_header + +/* N.B. the semicolon in the macro definition is important, since it produces a + second token from this system header on the same line as the _Pragma. */ +#define X _Pragma("GCC diagnostic push"); diff --git a/gcc/testsuite/gcc.dg/cpp/pr60014-3.c b/gcc/testsuite/gcc.dg/cpp/pr60014-3.c new file mode 100644 index 00000000000..c4306035f05 --- /dev/null +++ b/gcc/testsuite/gcc.dg/cpp/pr60014-3.c @@ -0,0 +1,16 @@ +/* { dg-do compile } */ +/* { dg-options "-save-temps -Wint-conversion" } */ +#include "pr60014-3.h" + +/* The line continuation on the next line is what triggers the problem here, + because it synchronizes the output line between the input source and the + preprocessed output (whereas without the line continuation, the + preprocessed output would be off by one line from having output a #pragma + on a line by itself). Therefore, the token streamer doesn't have a reason + to generate a line marker purely based on the line number. That gives it + the chance to consider whether instead it needs to generate a line marker + based on a change of the "in-system-header" state, allowing us to test that + it comes to the right conclusion, which it did not, prior to this commit to + resolve PR60014. */ +P(GCC diagnostic) \ +const char *should_warn = 1; /* { dg-warning {-Wint-conversion} } */ diff --git a/gcc/testsuite/gcc.dg/cpp/pr60014-3.h b/gcc/testsuite/gcc.dg/cpp/pr60014-3.h new file mode 100644 index 00000000000..aedf038d95f --- /dev/null +++ b/gcc/testsuite/gcc.dg/cpp/pr60014-3.h @@ -0,0 +1,2 @@ +#pragma GCC system_header +#define P(x) _Pragma(#x)