public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc r13-8257] c-family: Fix ICE with large column number after restoring a PCH [PR105608]
@ 2024-01-27 21:51 Lewis Hyatt
  0 siblings, 0 replies; only message in thread
From: Lewis Hyatt @ 2024-01-27 21:51 UTC (permalink / raw)
  To: gcc-cvs

https://gcc.gnu.org/g:52029ef151cc9b1c90fa620079fc17f3960c467c

commit r13-8257-g52029ef151cc9b1c90fa620079fc17f3960c467c
Author: Lewis Hyatt <lhyatt@gmail.com>
Date:   Tue Dec 5 11:33:39 2023 -0500

    c-family: Fix ICE with large column number after restoring a PCH [PR105608]
    
    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.

Diff:
---
 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(-)

diff --git a/gcc/c-family/c-pch.cc b/gcc/c-family/c-pch.cc
index 2f014fca210b..9ee6f1790023 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 000000000000..9d1ac6d1683a
--- /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 000000000000..3b6178bfae0f
--- /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 000000000000..0be035781c83
--- /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 000000000000..3b6178bfae0f
--- /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 000000000000..3390d7adba21
--- /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 000000000000..3b6178bfae0f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/pch/line-map-3.Hs
@@ -0,0 +1 @@
+/* This space intentionally left blank.  */

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2024-01-27 21:51 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-27 21:51 [gcc r13-8257] c-family: Fix ICE with large column number after restoring a PCH [PR105608] Lewis Hyatt

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).