From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-yw1-x112d.google.com (mail-yw1-x112d.google.com [IPv6:2607:f8b0:4864:20::112d]) by sourceware.org (Postfix) with ESMTPS id 767F53858C98 for ; Thu, 25 Jan 2024 22:38:16 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 767F53858C98 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 767F53858C98 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::112d ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1706222304; cv=none; b=kwob2zf0B0cEykHMMv3Ign/ZcWk6RgnP8mXGCTdWy5vUsrPuTxiYcN9PP7ILhqi47dqA1ac/iXQ3IM8tc2qt1KSXPMmomIR6NwMdYx5RhCMKDn8+8E1muHJe9RYHyfcRYhNS0UuKlogT2Ro5Ny/Og1lBajx+/HLu3z/7Nqy/U0g= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1706222304; c=relaxed/simple; bh=CZ9qqQu43+Y689yC9i1eFcQCgEEfGFtA2Kf8S60JItU=; h=DKIM-Signature:MIME-Version:From:Date:Message-ID:Subject:To; b=WkC5uNf2MxNC7SJTsJyYpzHuxsSYTaO2RpUmjn6RiwWSQlzSjZHqqsmReYEM6kxk9rEL3jTavUQXh3D/7uvMxKZAUKIDTU2EGlf7PfXsINaoNTSKC6S5qHkAZU6hK9kkt5UqKMWm1yktPRwpahx9mtwUhUQu7nnnMIC/hQ14kjY= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-yw1-x112d.google.com with SMTP id 00721157ae682-5ff7dc53ce0so56022557b3.1 for ; Thu, 25 Jan 2024 14:38:16 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1706222296; x=1706827096; darn=gcc.gnu.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=WjY1s7uFOAa32/iDq8h51adskehqojPDnykv9cOJziM=; b=beuYHSgyrz8HI2KAWXYT2dauosUSNem+2m6RMgpAXjNsshJfcr7V9CVieBzuvAnS1t W+RxnXvqdtpTL8AX+T9K9j9eeEIdsHjKL7m51nE2hl07X9qvEap5Or6K+fQP59lmhRNa 3SrVNHjoJwsFktuFloaZ+NnRxElkIEX8xoQbG/Y1PYm6fBCCvA6n0tpKV6RJirwLcgmC YFGlPtsj2zeso5ce+WI8JbvQdUMF4vPf48D3s9tlpsgYCdzSG7Lc3IT/C9UzkZUmZsO6 SopidbekJ+AH6rMMJqFMFUJfX5TiM4gFPM3t4hoJzx2DG3SJPt36q4WqvlE1cYDzzQCU vC3g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1706222296; x=1706827096; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=WjY1s7uFOAa32/iDq8h51adskehqojPDnykv9cOJziM=; b=bpKGXjeRlRXp8AFXgOokyCpASaCS54qseNxde/X1PQ8qHUqry39ldVeg1rj9YlcmFX CMJMF6JRyX4hIQBXpX03kYfqUpemtHdK4Jb/IoaNTRTpNP23OFskJpwIc4Hmxa4jH8Wl gpb/0aRQA4Wd2+th7+4h/nuuZg+t6fNDwXkMXaq1IJPYDmStd2Qk5qdwEwMrD6LNk/KH gHn5JLUJZ2/T1TaqeSCzuKGzIQ/aWS0Px7aV7+rFFwRFXzdA3IZuHdChMJUqOAQ7eyg8 2RHD5e4U5TN3jhDLxDre9CYSlTTIC282xu+cgoagfOe5tVBy8tISFEY/XqxxgIbFTApL 01Rg== X-Gm-Message-State: AOJu0YxRyY0ewmkTg0/wtc8rue/XpaIcUnRHLj9d7vHn5IFn9V5gW+Uz KFF2vDRIV3OjiAAjVkL8ki0JG7Jxap3qniyr30/hgB7Pwa5g4/R0kLbtI79VN7ozp4AyNOjDhLx rjncjpBPSHOKP25Cc3NezGhTej148Oq3hY68= X-Google-Smtp-Source: AGHT+IHZiLmhTwtUuwbCDSggdK6vtA4ZggxJXt9YMJhh4lTfReF8Tc4QENbgb/D9dNxBvM0PDXzrt22FN+RuQR6Ov1w= X-Received: by 2002:a05:690c:a16:b0:5ff:4d76:e2e3 with SMTP id cg22-20020a05690c0a1600b005ff4d76e2e3mr478391ywb.63.1706222295583; Thu, 25 Jan 2024 14:38:15 -0800 (PST) MIME-Version: 1.0 References: <20231206015211.682650-1-lhyatt@gmail.com> In-Reply-To: From: Lewis Hyatt Date: Thu, 25 Jan 2024 17:38:04 -0500 Message-ID: Subject: ping: [PATCH] c-family: Fix ICE with large column number after restoring a PCH [PR105608] To: gcc-patches List Cc: Jason Merrill Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-3035.5 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,T_SCC_BODY_TEXT_LINE 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: Hello- May I please ping this small patch? Thanks.... https://gcc.gnu.org/pipermail/gcc-patches/2023-December/639467.html -Lewis On Wed, Dec 20, 2023 at 8:02=E2=80=AFPM Lewis Hyatt wrot= e: > > Hello- > > May I please ping this PCH patch? Thanks! > https://gcc.gnu.org/pipermail/gcc-patches/2023-December/639467.html > > -Lewis > > On Tue, Dec 5, 2023 at 8:52=E2=80=AFPM Lewis Hyatt wro= te: > > > > Hello- > > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=3D105608 > > > > There are two related issues here really, a regression since GCC 11 whe= re we > > can ICE after restoring a PCH, and a deeper issue with bogus locations > > assigned to macros that were defined prior to restoring a PCH. This pa= tch > > fixes the ICE regression with a simple change, and I think it's appropr= iate > > for GCC 14 as well as backport to 11, 12, 13. The bad locations (wrong,= but > > not generally causing an ICE, and mostly affecting only the output of > > -Wunused-macros) are not as problematic, and will be harder to fix. I c= ould > > take a stab at that for GCC 15. In the meantime the patch adds XFAILed > > tests for the wrong locations (as well as passing tests for the regress= ion > > fix). Does it look OK please? Bootstrap + regtest all languages on x86-= 64 > > Linux. Thanks! > > > > -Lewis > > > > -- >8 -- > > > > Users are allowed to define macros prior to restoring a precompiled hea= der > > file, as long as those macros are not defined (or are defined identical= ly) > > in the PCH. However, the PCH restoration process destroys all the macr= o > > definitions, so libcpp has to record them before restoring the PCH and = then > > redefine them afterward. > > > > This process does not currently assign great locations to the macros af= ter > > redefining them. Some work is needed to also remember the original loca= tions > > and get the line_maps instance in the right state (since, like all othe= r > > data structures, the line_maps instance is also reset after restoring a= PCH). > > The new testcase line-map-3.C contains XFAILed examples where the locat= ions > > are wrong. > > > > This patch addresses a more pressing issue, which is that we ICE in som= e > > cases since GCC 11, hitting an assert in line-maps.cc. It happens if th= e > > first line encountered after the PCH restore requires an LC_RENAME map,= such > > as will happen if the line is sufficiently long. This is much easier t= o > > fix, since we just need to call linemap_line_start before asking libcpp= to > > redefine the stored macros, instead of afterward, to avoid the unexpect= ed > > need for an LC_RENAME before an LC_ENTER has been seen. > > > > gcc/c-family/ChangeLog: > > > > PR preprocessor/105608 > > * c-pch.cc (c_common_read_pch): Start a new line map before ask= ing > > libcpp to restore macros defined prior to reading the PCH, inst= ead > > of afterward. > > > > gcc/testsuite/ChangeLog: > > > > PR preprocessor/105608 > > * g++.dg/pch/line-map-1.C: New test. > > * g++.dg/pch/line-map-1.Hs: New test. > > * g++.dg/pch/line-map-2.C: New test. > > * g++.dg/pch/line-map-2.Hs: New test. > > * g++.dg/pch/line-map-3.C: New test. > > * g++.dg/pch/line-map-3.Hs: New test. > > --- > > gcc/c-family/c-pch.cc | 5 ++--- > > gcc/testsuite/g++.dg/pch/line-map-1.C | 4 ++++ > > gcc/testsuite/g++.dg/pch/line-map-1.Hs | 1 + > > gcc/testsuite/g++.dg/pch/line-map-2.C | 6 ++++++ > > gcc/testsuite/g++.dg/pch/line-map-2.Hs | 1 + > > gcc/testsuite/g++.dg/pch/line-map-3.C | 23 +++++++++++++++++++++++ > > gcc/testsuite/g++.dg/pch/line-map-3.Hs | 1 + > > 7 files changed, 38 insertions(+), 3 deletions(-) > > create mode 100644 gcc/testsuite/g++.dg/pch/line-map-1.C > > create mode 100644 gcc/testsuite/g++.dg/pch/line-map-1.Hs > > create mode 100644 gcc/testsuite/g++.dg/pch/line-map-2.C > > create mode 100644 gcc/testsuite/g++.dg/pch/line-map-2.Hs > > create mode 100644 gcc/testsuite/g++.dg/pch/line-map-3.C > > create mode 100644 gcc/testsuite/g++.dg/pch/line-map-3.Hs > > > > diff --git a/gcc/c-family/c-pch.cc b/gcc/c-family/c-pch.cc > > index 2f014fca210..9ee6f179002 100644 > > --- a/gcc/c-family/c-pch.cc > > +++ b/gcc/c-family/c-pch.cc > > @@ -342,6 +342,8 @@ c_common_read_pch (cpp_reader *pfile, const char *n= ame, > > gt_pch_restore (f); > > cpp_set_line_map (pfile, line_table); > > rebuild_location_adhoc_htab (line_table); > > + line_table->trace_includes =3D saved_trace_includes; > > + linemap_add (line_table, LC_ENTER, 0, saved_loc.file, saved_loc.line= ); > > > > timevar_push (TV_PCH_CPP_RESTORE); > > if (cpp_read_state (pfile, name, f, smd) !=3D 0) > > @@ -355,9 +357,6 @@ c_common_read_pch (cpp_reader *pfile, const char *n= ame, > > > > fclose (f); > > > > - line_table->trace_includes =3D saved_trace_includes; > > - linemap_add (line_table, LC_ENTER, 0, saved_loc.file, saved_loc.line= ); > > - > > /* Give the front end a chance to take action after a PCH file has > > been loaded. */ > > if (lang_post_pch_load) > > diff --git a/gcc/testsuite/g++.dg/pch/line-map-1.C b/gcc/testsuite/g++.= dg/pch/line-map-1.C > > new file mode 100644 > > index 00000000000..9d1ac6d1683 > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/pch/line-map-1.C > > @@ -0,0 +1,4 @@ > > +/* PR preprocessor/105608 */ > > +/* { dg-do compile } */ > > +#define MACRO_ON_A_LONG_LINE "this line is long enough that it forces = the line table to create an LC_RENAME map, which formerly triggered an ICE = after PCH restore" > > +#include "line-map-1.H" > > diff --git a/gcc/testsuite/g++.dg/pch/line-map-1.Hs b/gcc/testsuite/g++= .dg/pch/line-map-1.Hs > > new file mode 100644 > > index 00000000000..3b6178bfae0 > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/pch/line-map-1.Hs > > @@ -0,0 +1 @@ > > +/* This space intentionally left blank. */ > > diff --git a/gcc/testsuite/g++.dg/pch/line-map-2.C b/gcc/testsuite/g++.= dg/pch/line-map-2.C > > new file mode 100644 > > index 00000000000..0be035781c8 > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/pch/line-map-2.C > > @@ -0,0 +1,6 @@ > > +/* PR preprocessor/105608 */ > > +/* { dg-do compile } */ > > +/* { dg-additional-options "-save-temps" } */ > > +#define MACRO_ON_A_LONG_LINE "this line is long enough that it forces = the line table to create an LC_RENAME map, which formerly triggered an ICE = after PCH restore" > > +#include "line-map-2.H" > > +#error "suppress PCH assembly comparison, which does not work with -sa= ve-temps" /* { dg-error "." } */ > > diff --git a/gcc/testsuite/g++.dg/pch/line-map-2.Hs b/gcc/testsuite/g++= .dg/pch/line-map-2.Hs > > new file mode 100644 > > index 00000000000..3b6178bfae0 > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/pch/line-map-2.Hs > > @@ -0,0 +1 @@ > > +/* This space intentionally left blank. */ > > diff --git a/gcc/testsuite/g++.dg/pch/line-map-3.C b/gcc/testsuite/g++.= dg/pch/line-map-3.C > > new file mode 100644 > > index 00000000000..3390d7adba2 > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/pch/line-map-3.C > > @@ -0,0 +1,23 @@ > > +#define UNUSED_MACRO /* { dg-error "UNUSED_MACRO" "" { xfail *-*-* } }= */ > > +#include "line-map-3.H" /* { dg-bogus "-:UNUSED_MACRO" "" { xfail *-*-= * } } */ > > + > > +/* { dg-do compile } */ > > +/* { dg-additional-options "-Werror=3Dunused-macros" } */ > > + > > +/* PR preprocessor/105608 */ > > +/* This test case is currently xfailed and requires work in libcpp/pch= .cc to > > + resolve. Currently, the macro location is incorrectly assigned to = line 2 > > + of the header file when read via PCH, because libcpp doesn't try to > > + assign locations relative to the newly loaded line map after restor= ing > > + the PCH. */ > > + > > +/* In PCH mode we also complain incorrectly about the command line mac= ro -Dwith_PCH > > + added by dejagnu; that warning would get suppressed if the macro lo= cation were > > + correctly restored by libcpp to reflect that it was a command line = macro. */ > > +/* { dg-bogus "-:with_PCH" "" { xfail *-*-* } 2 } */ > > + > > +/* The reason we used -Werror was to prevent pch.exp from rerunning wi= thout PCH; > > + in that case we would get unnecessary XPASS outputs since the test = does work > > + fine without PCH. Once the bug is fixed, remove the -Werror and sw= itch to > > + dg-warning. */ > > +/* { dg-regexp {[^[:space:]]*: some warnings being treated as errors} = } */ > > diff --git a/gcc/testsuite/g++.dg/pch/line-map-3.Hs b/gcc/testsuite/g++= .dg/pch/line-map-3.Hs > > new file mode 100644 > > index 00000000000..3b6178bfae0 > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/pch/line-map-3.Hs > > @@ -0,0 +1 @@ > > +/* This space intentionally left blank. */