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 PM Lewis Hyatt <lhy...@gmail.com> wrote: > > 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 PM Lewis Hyatt <lhy...@gmail.com> wrote: > > > > Hello- > > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105608 > > > > There are two related issues here really, a regression since GCC 11 where 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 patch > > fixes the ICE regression with a simple change, and I think it's appropriate > > 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 could > > 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 regression > > 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 header > > file, as long as those macros are not defined (or are defined identically) > > in the PCH. However, the PCH restoration process destroys all the macro > > 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 after > > redefining them. Some work is needed to also remember the original locations > > and get the line_maps instance in the right state (since, like all other > > 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 locations > > are wrong. > > > > This patch addresses a more pressing issue, which is that we ICE in some > > cases since GCC 11, hitting an assert in line-maps.cc. It happens if the > > 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 to > > fix, since we just need to call linemap_line_start before asking libcpp to > > redefine the stored macros, instead of afterward, to avoid the unexpected > > 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 asking > > libcpp to restore macros defined prior to reading the PCH, instead > > 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 *name, > > gt_pch_restore (f); > > cpp_set_line_map (pfile, line_table); > > rebuild_location_adhoc_htab (line_table); > > + line_table->trace_includes = 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) != 0) > > @@ -355,9 +357,6 @@ c_common_read_pch (cpp_reader *pfile, const char *name, > > > > fclose (f); > > > > - line_table->trace_includes = 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 > > -save-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=unused-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 restoring > > + the PCH. */ > > + > > +/* In PCH mode we also complain incorrectly about the command line macro > > -Dwith_PCH > > + added by dejagnu; that warning would get suppressed if the macro > > location 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 > > without 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 switch > > 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. */
