| Summary: | [12/13/14/15 Regression] ICE: in linemap_add with a really long defined macro on the command line r11-338-g2a0225e47868fbfc | ||
|---|---|---|---|
| Product: | gcc | Reporter: | Sergei Trofimovich <slyfox> |
| Component: | preprocessor | Assignee: | Not yet assigned to anyone <unassigned> |
| Status: | NEW --- | ||
| Severity: | normal | CC: | danglin, egallager, lhyatt, marxin, nathan, ro, sjames |
| Priority: | P2 | Keywords: | patch |
| Version: | 13.0 | ||
| Target Milestone: | 12.5 | ||
| URL: | https://gcc.gnu.org/pipermail/gcc-patches/2023-December/639467.html | ||
| See Also: | https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103012 | ||
| Host: | Target: | ||
| Build: | Known to work: | ||
| Known to fail: | Last reconfirmed: | 2022-05-16 00:00:00 | |
With checking compiler, it started with r11-338-g2a0225e47868fbfc. 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.) GCC 11.4 is being released, retargeting bugs to GCC 11.5. I submitted the simple patch to resolve the ICE here: https://gcc.gnu.org/pipermail/gcc-patches/2023-December/639467.html The master branch has been updated by Lewis Hyatt <lhyatt@gcc.gnu.org>: https://gcc.gnu.org/g:5200ef26ac1771a75596394c20c5f1a348694d5e commit r14-8465-g5200ef26ac1771a75596394c20c5f1a348694d5e 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. The releases/gcc-11 branch has been updated by Lewis Hyatt <lhyatt@gcc.gnu.org>: https://gcc.gnu.org/g:7a525d23aad8bf2f4db37f384c331af1abf7f103 commit r11-11213-g7a525d23aad8bf2f4db37f384c331af1abf7f103 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). 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. The releases/gcc-12 branch has been updated by Lewis Hyatt <lhyatt@gcc.gnu.org>: https://gcc.gnu.org/g:e8e584a81817713f98f16b2c81426905748237e3 commit r12-10118-ge8e584a81817713f98f16b2c81426905748237e3 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). 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. The releases/gcc-13 branch has been updated by Lewis Hyatt <lhyatt@gcc.gnu.org>: 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. The ICE regression is fixed for GCC 11,12,13,14. Leaving it open to track the wrong location issue. 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. 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. > --- Comment #11 from Lewis Hyatt <lhyatt at gcc dot gnu.org> --- > 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. 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 113672 has been marked as a duplicate of this bug. *** The master branch has been updated by Lewis Hyatt <lhyatt@gcc.gnu.org>: https://gcc.gnu.org/g:019dc63819befb2b82077fb2d76b5dd670946f36 commit r14-8698-g019dc63819befb2b82077fb2d76b5dd670946f36 Author: Lewis Hyatt <lhyatt@gmail.com> 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. The releases/gcc-13 branch has been updated by Lewis Hyatt <lhyatt@gcc.gnu.org>: https://gcc.gnu.org/g:0e438772e788244236045d75cd2be895e2ab4e08 commit r13-8353-g0e438772e788244236045d75cd2be895e2ab4e08 Author: Lewis Hyatt <lhyatt@gmail.com> 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. GCC 11 branch is being closed. |
ICE initially observed on ovito-3.7.1 source code where -DCOPYRIGHT="\"${long-string}\"" is used along with precompiled headers in standard cmake setup. Here is a 0-file reproducer: # create empty files and dirs: $ mkdir -p ph $ touch ph/h.hxx.cxx $ touch a.cpp # trigger the bug: $ /tmp/gb/gcc/xg++ -B/tmp/gb/gcc -Winvalid-pch -x c++-header -o ph/h.hxx.gch -c ph/h.hxx.cxx $ /tmp/gb/gcc/xg++ -B/tmp/gb/gcc -DBUG=aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa -Winvalid-pch -include ph/h.hxx -o a.o -c a.cpp ph/h.hxx.cxx:1:116: internal compiler error: in linemap_add, at libcpp/line-map.cc:502 0x220adce linemap_add(line_maps*, lc_reason, unsigned int, char const*, unsigned int) <<GCC>>/libcpp/line-map.cc:502 0x220b129 linemap_line_start(line_maps*, unsigned int, unsigned int) <<GCC>>/libcpp/line-map.cc:827 0x220b3f3 linemap_position_for_column(line_maps*, unsigned int) <<GCC>>/libcpp/line-map.cc:898 0x220802b _cpp_lex_direct <<GCC>>/libcpp/lex.cc:3596 0x22119dd lex_expansion_token <<GCC>>/libcpp/macro.cc:3552 0x22119dd create_iso_definition <<GCC>>/libcpp/macro.cc:3665 0x22119dd _cpp_create_definition <<GCC>>/libcpp/macro.cc:3829 0x2214574 cpp_read_state(cpp_reader*, char const*, _IO_FILE*, save_macro_data*) <<GCC>>/libcpp/pch.cc:841 0xad8b8d c_common_read_pch(cpp_reader*, char const*, int, char const*) <<GCC>>/gcc/c-family/c-pch.cc:347 0x2201213 is_known_idempotent_file <<GCC>>/libcpp/files.cc:817 0x2201213 _cpp_stack_file <<GCC>>/libcpp/files.cc:906 0x220182e _cpp_stack_include <<GCC>>/libcpp/files.cc:1113 0x2201e26 cpp_push_include(cpp_reader*, char const*) <<GCC>>/libcpp/files.cc:1621 0xad5d51 push_command_line_include <<GCC>>/gcc/c-family/c-opts.cc:1565 0xad5e79 cb_file_change <<GCC>>/gcc/c-family/c-opts.cc:1619 0x21f95cc _cpp_do_file_change <<GCC>>/libcpp/directives.cc:1181 0x21fa073 _cpp_pop_buffer <<GCC>>/libcpp/directives.cc:2773 0x22045b0 _cpp_get_fresh_line <<GCC>>/libcpp/lex.cc:3490 0x2207eb8 _cpp_lex_direct <<GCC>>/libcpp/lex.cc:3552 0x2209867 _cpp_lex_token <<GCC>>/libcpp/lex.cc:3398 gcc-12.1.0 does not seem to be affected. Only gcc-13 fails for me including current master. $ /tmp/gb/gcc/xg++ -B/tmp/gb/gcc -v Reading specs from /tmp/gb/gcc/specs COLLECT_GCC=/tmp/gb/gcc/xg++ COLLECT_LTO_WRAPPER=/tmp/gb/gcc/lto-wrapper Target: x86_64-pc-linux-gnu Configured with: <<GCC>>/configure --disable-multilib --disable-bootstrap --with-native-system-header-dir=/<<NIX>>/glibc-2.34-115-dev/include --prefix=/tmp/gb/__td__ CFLAGS='-O1 -ggdb3' CXXFLAGS='-O1 -ggdb3' LDFLAGS='-O1 -ggdb3' Thread model: posix Supported LTO compression algorithms: zlib gcc version 13.0.0 20220515 (experimental) (GCC)