[Bug preprocessor/105608] [11/12/13/14 Regression] ICE: in linemap_add with a really long defined macro on the command line r11-338-g2a0225e47868fbfc
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105608 --- Comment #16 from GCC Commits --- The releases/gcc-13 branch has been updated by Lewis Hyatt : https://gcc.gnu.org/g:0e438772e788244236045d75cd2be895e2ab4e08 commit r13-8353-g0e438772e788244236045d75cd2be895e2ab4e08 Author: Lewis Hyatt Date: Thu Feb 22 07:50:10 2024 -0500 testsuite: Remove test that should not have been backported [PR105608] This test (backported as part of r13-8257, from r14-8465) was not meant to be backported, since it fails on some platforms without other GCC 14 patches that will not be backported. Remove it from the 13 branch. gcc/testsuite/ChangeLog: PR preprocessor/105608 * g++.dg/pch/line-map-3.C: Removed. * g++.dg/pch/line-map-3.Hs: Removed.
[Bug preprocessor/105608] [11/12/13/14 Regression] ICE: in linemap_add with a really long defined macro on the command line r11-338-g2a0225e47868fbfc
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105608 --- Comment #15 from GCC Commits --- The master branch has been updated by Lewis Hyatt : https://gcc.gnu.org/g:019dc63819befb2b82077fb2d76b5dd670946f36 commit r14-8698-g019dc63819befb2b82077fb2d76b5dd670946f36 Author: Lewis Hyatt Date: Wed Jan 31 15:50:11 2024 -0500 libcpp: Stabilize the location for macros restored after PCH load [PR105608] libcpp currently lacks the infrastructure to assign correct locations to macros that were defined prior to loading a PCH and then restored afterwards. While I plan to address that fully for GCC 15, this patch improves things by using at least a valid location, even if it's not the best one. Without this change, libcpp uses pfile->directive_line as the location for the restored macros, but this location_t applies to the old line map, not the one that was just restored from the PCH, so the resulting location is unpredictable and depends on what was stored in the line maps before. With this change, all restored macros get assigned locations at the line of the #include that triggered the PCH restore. A future patch will store the actual file name and line number of each definition and then synthesize locations in the new line map pointing to the right place. gcc/c-family/ChangeLog: PR preprocessor/105608 * c-pch.cc (c_common_read_pch): Adjust line map so that libcpp assigns a location to restored macros which is the same location that triggered the PCH include. libcpp/ChangeLog: PR preprocessor/105608 * pch.cc (cpp_read_state): Set a valid location for restored macros.
[Bug preprocessor/105608] [11/12/13/14 Regression] ICE: in linemap_add with a really long defined macro on the command line r11-338-g2a0225e47868fbfc
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105608 Lewis Hyatt changed: What|Removed |Added CC||danglin at gcc dot gnu.org --- Comment #14 from Lewis Hyatt --- *** Bug 113672 has been marked as a duplicate of this bug. ***
[Bug preprocessor/105608] [11/12/13/14 Regression] ICE: in linemap_add with a really long defined macro on the command line r11-338-g2a0225e47868fbfc
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105608
--- Comment #13 from Lewis Hyatt ---
I think I understand why it is platform dependent. The location being assigned
to the restored macros is pfile->directive_line, which was stored relative to
the original line map instance, before replacing it with the one from the PCH.
So looking that location up in the new line_maps instance produces some value
that is dependent on exactly what was stored in the old and new line maps. In
particular it will depend on stuff that was included internally such as
/usr/include/stdc-predef.h. The output with -fdump-internal-locations would
confirm it, but I am not sure it's too important to understand all the ways
that this location is wrong, since it needs to just get fixed regardless...
it's not even trying to be correct right now.
This patch in libcpp would stabilize the output to be the same on all
platforms:
=
diff --git a/libcpp/pch.cc b/libcpp/pch.cc
index e156fe257b3..08df0f8a86d 100644
--- a/libcpp/pch.cc
+++ b/libcpp/pch.cc
@@ -838,7 +838,7 @@ cpp_read_state (cpp_reader *r, const char *name, FILE *f,
!= NULL)
{
_cpp_clean_line (r);
- if (!_cpp_create_definition (r, h, 0))
+ if (!_cpp_create_definition (r, h, r->line_table->highest_line))
abort ();
_cpp_pop_buffer (r);
}
=
It would change the location to be always line 3 in the source file, matching
the "wrong" result you get now on i386-pc-solaris2.11 . The test case would
need adjustment to match. I am going to ask if it can be approved for GCC 14, I
think it is worth it to not use an unpredictable location here.
[Bug preprocessor/105608] [11/12/13/14 Regression] ICE: in linemap_add with a really long defined macro on the command line r11-338-g2a0225e47868fbfc
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105608 --- Comment #12 from ro at CeBiTec dot Uni-Bielefeld.DE --- > --- Comment #11 from Lewis Hyatt --- > Oh interesting. So the purpose of this test was just to record that GCC > outputs > incorrect locations for this case, I wanted to xfail it and then fix it > properly for GCC 15. I did not consider that it might output different wrong > locations for different platforms, but I could buy that it may happen, for a > similar reason why this switched from being silently broken to ICEing since > r11-338 which was seemingly unrelated. It seems like in one case the wrong > location is inside the header file and in the other case, the wrong location > is > the line just following the include. It may have to do with line endings or > some other issue with the treatment of EOF? If this test is causing problems > we It's still weird given that it's exactly the same version of Solaris on both SPARC and x86. > could just skip it on some architectures maybe? Once the underlying issue is I guess that's best for now. I'll check if the test behaves differently for a 64-bit-default (amd64-pc-solaris2.11) compiler. > fixed, the location (line 2 of the .C file) will be correct everywhere. I am > curious why it gets a different wrong output though, if there is a compile > farm > machine with this architecture I could look into it. There's no Solaris/x86 system in the cfarm right now, unfortunately. The only one runs Solaris 11.3/SPARC, where the test works just like everywhere else. That said, I've accquired systems to add to the cfarm that will both be running current Solaris 11.4 (SPARC and x86). I'm working on installing and integrating them as we speak, but I don't have an ETA yet.
[Bug preprocessor/105608] [11/12/13/14 Regression] ICE: in linemap_add with a really long defined macro on the command line r11-338-g2a0225e47868fbfc
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105608 --- Comment #11 from Lewis Hyatt --- Oh interesting. So the purpose of this test was just to record that GCC outputs incorrect locations for this case, I wanted to xfail it and then fix it properly for GCC 15. I did not consider that it might output different wrong locations for different platforms, but I could buy that it may happen, for a similar reason why this switched from being silently broken to ICEing since r11-338 which was seemingly unrelated. It seems like in one case the wrong location is inside the header file and in the other case, the wrong location is the line just following the include. It may have to do with line endings or some other issue with the treatment of EOF? If this test is causing problems we could just skip it on some architectures maybe? Once the underlying issue is fixed, the location (line 2 of the .C file) will be correct everywhere. I am curious why it gets a different wrong output though, if there is a compile farm machine with this architecture I could look into it.
[Bug preprocessor/105608] [11/12/13/14 Regression] ICE: in linemap_add with a really long defined macro on the command line r11-338-g2a0225e47868fbfc
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105608 Rainer Orth changed: What|Removed |Added CC||ro at gcc dot gnu.org --- Comment #10 from Rainer Orth --- Something very weird is going on here: the new g++.dg/pch/line-map-3.C test FAILs on i386-pc-solaris2.11 only: XPASS: g++.dg/pch/line-map-3.C -O2 -I. -Dwith_PCH (test for bogus messages, line 2) +XPASS: g++.dg/pch/line-map-3.C -O2 -I. -Dwith_PCH at line 17 (test for bogus messages, line 2) +FAIL: g++.dg/pch/line-map-3.C -O2 -I. -Dwith_PCH (test for excess errors) +XPASS: g++.dg/pch/line-map-3.C -O2 -g -I. -Dwith_PCH (test for bogus messages, line 2) +XPASS: g++.dg/pch/line-map-3.C -O2 -g -I. -Dwith_PCH at line 17 (test for bogus messages, line 2) +FAIL: g++.dg/pch/line-map-3.C -O2 -g -I. -Dwith_PCH (test for excess errors) +XPASS: g++.dg/pch/line-map-3.C -g -I. -Dwith_PCH (test for bogus messages, line 2) +XPASS: g++.dg/pch/line-map-3.C -g -I. -Dwith_PCH at line 17 (test for bogus messages, line 2) +FAIL: g++.dg/pch/line-map-3.C -g -I. -Dwith_PCH (test for excess errors) both 32 and 64-bit. The excess error is Excess errors: /vol/gcc/src/hg/master/local/gcc/testsuite/g++.dg/pch/line-map-3.C:3:9: error: macro "UNUSED_MACRO" is not used [-Werror=unused-macros] /vol/gcc/src/hg/master/local/gcc/testsuite/g++.dg/pch/line-map-3.C:3:9: error: macro "with_PCH" is not used [-Werror=unused-macros] When checking sparc-sun-solaris2.11 for comparison, the output is different: ./line-map-3.H:2: error: macro "UNUSED_MACRO" is not used [-Werror=unused-macros] ./line-map-3.H:2: error: macro "with_PCH" is not used [-Werror=unused-macros] This is totally strange since the setup of both systems is identical; they're even building from a shared source tree.
[Bug preprocessor/105608] [11/12/13/14 Regression] ICE: in linemap_add with a really long defined macro on the command line r11-338-g2a0225e47868fbfc
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105608 Lewis Hyatt changed: What|Removed |Added URL|https://gcc.gnu.org/piperma | |il/gcc-patches/2023-Decembe | |r/639467.html | Keywords|ice-on-valid-code, patch| --- Comment #9 from Lewis Hyatt --- The ICE regression is fixed for GCC 11,12,13,14. Leaving it open to track the wrong location issue.
[Bug preprocessor/105608] [11/12/13/14 Regression] ICE: in linemap_add with a really long defined macro on the command line r11-338-g2a0225e47868fbfc
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105608 --- Comment #8 from GCC Commits --- The releases/gcc-13 branch has been updated by Lewis Hyatt : https://gcc.gnu.org/g:52029ef151cc9b1c90fa620079fc17f3960c467c commit r13-8257-g52029ef151cc9b1c90fa620079fc17f3960c467c Author: Lewis Hyatt 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.
[Bug preprocessor/105608] [11/12/13/14 Regression] ICE: in linemap_add with a really long defined macro on the command line r11-338-g2a0225e47868fbfc
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105608 --- Comment #7 from GCC Commits --- The releases/gcc-12 branch has been updated by Lewis Hyatt : https://gcc.gnu.org/g:e8e584a81817713f98f16b2c81426905748237e3 commit r12-10118-ge8e584a81817713f98f16b2c81426905748237e3 Author: Lewis Hyatt 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). 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.
[Bug preprocessor/105608] [11/12/13/14 Regression] ICE: in linemap_add with a really long defined macro on the command line r11-338-g2a0225e47868fbfc
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105608 --- Comment #6 from GCC Commits --- The releases/gcc-11 branch has been updated by Lewis Hyatt : https://gcc.gnu.org/g:7a525d23aad8bf2f4db37f384c331af1abf7f103 commit r11-11213-g7a525d23aad8bf2f4db37f384c331af1abf7f103 Author: Lewis Hyatt 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). 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.c (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.
[Bug preprocessor/105608] [11/12/13/14 Regression] ICE: in linemap_add with a really long defined macro on the command line r11-338-g2a0225e47868fbfc
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105608 --- Comment #5 from GCC Commits --- The master branch has been updated by Lewis Hyatt : https://gcc.gnu.org/g:5200ef26ac1771a75596394c20c5f1a348694d5e commit r14-8465-g5200ef26ac1771a75596394c20c5f1a348694d5e Author: Lewis Hyatt 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.
[Bug preprocessor/105608] [11/12/13/14 Regression] ICE: in linemap_add with a really long defined macro on the command line r11-338-g2a0225e47868fbfc
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105608 Lewis Hyatt changed: What|Removed |Added Keywords||patch URL||https://gcc.gnu.org/piperma ||il/gcc-patches/2023-Decembe ||r/639467.html --- Comment #4 from Lewis Hyatt --- I submitted the simple patch to resolve the ICE here: https://gcc.gnu.org/pipermail/gcc-patches/2023-December/639467.html
[Bug preprocessor/105608] [11/12/13/14 Regression] ICE: in linemap_add with a really long defined macro on the command line r11-338-g2a0225e47868fbfc
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105608 Jakub Jelinek changed: What|Removed |Added Target Milestone|11.4|11.5 --- Comment #3 from Jakub Jelinek --- GCC 11.4 is being released, retargeting bugs to GCC 11.5.
[Bug preprocessor/105608] [11/12/13/14 Regression] ICE: in linemap_add with a really long defined macro on the command line r11-338-g2a0225e47868fbfc
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105608 --- Comment #2 from Lewis Hyatt --- When a PCH file is restored, the global line_maps instance is replaced with the one that was stored in the PCH file. Hence any locations that were generated prior to restoring the PCH file need to be added back to it. Currently, the only such locations that can arise will be from preprocessor directives, such as #define. The save/restore process is handled by libcpp/pch.cc and, looking over this now, it doesn't seem it makes any effort to assign valid locations when re-adding the saved macros. If the just-restored line map is at depth=0, as it is after processing the empty pch, then it produces the ICE noted here in case the first line map added is of type LC_RENAME, which happens if an ad-hoc location is needed due to the long line. However you can see the invalid locations without an ICE as well, for instance in this simpler example: $ echo -n > h.h $ cat > a.cpp #define HELLO #include "h.h" $ gcc -x c++-header -c h.h $ gcc -x c++ -c a.cpp -Wunused-macros h.h:2: warning: macro "HELLO" is not used [-Wunused-macros] Note that the warning is emitted with the wrong filename and the wrong line number. If the PCH is absent, it will be correct, but when the definition of HELLO is processed the second time after PCH restore, it comes out wrong. Fixing this issue will also fix the ICE reported here; I think it will be needed to add some more infrastructure in libcpp/pch.cc so that it can add to the line map with proper locations. I can look into it sometime. This was always incorrect AFAIK, and did not regress with r11-338-g2a0225e47868fbfc specifically; that commit did make the line number change from 1 to 2, because it handles EOF differently, and I think that caused Sergei's test case to ICE rather than be silently wrong. BTW, the issue can also be papered over by moving these 2 lines: 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) = With that change, you still get wrong locations for the restored macros, but the line_map state is sufficiently improved that the ICE is avoided. (The locations get assigned, as if all the macros were defined on the line following the PCH include.)
