From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-qv1-xf32.google.com with SMTP id de14so5139322qvb.5 for ; Sat, 08 Oct 2022 14:18:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=ltSuPlOLFDe7DKY4Navdj3U+eJ7Y9sw/YEskw+fAJOM=; b=jUoffAUAJpHouvQX/dMw7hjW50T8IQP/sHV47ky+2QMMX5j9DLXaD+i62XseyzHcRg CXCvi/Y90tzpUDTHyfwpGks5JL5jEbdBzcCKuVWlNNAWoAZK8ky1288k59DouUZ8tQrq SxG77eCAVJvyWiDmQJIgwkTVJDhAf/Re1DfpClc38kXosxNg2vWDl3jFCFuekWC3ZuTx mi67dxA8fuzZTOYqe1far4G7ewC+xzpCNsze4DGLvZNmhfvImbYOM2VpDiiDqAKpSJlB PrD1GOIL97rVRAKCTEtoZtSZKk6nDjk3hvcN8PlVhpWmSqqzgHHDm/LMARWKyWlL7hjC aCyw== 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-Google-Smtp-Source: AMsMyM770q1G0HENMBNx8GNN2Lnj30SPaV4k6H16EIexpfHqFNlcDnav+4XJUzXK5RnJTE+RBNv8rw== 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) From: Lewis Hyatt To: gcc-patches@gcc.gnu.org Cc: Lewis Hyatt 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 Content-Transfer-Encoding: 8bit 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 List-Id: 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)