Hello. During reduction of another ICE, I observed the following one: $ cat Unified_cpp_js_src35.ii namespace js {} # 9 "" 2 namespace js { int a; } $ c++ -flto Unified_cpp_js_src35.ii line-map.c: file "<stdin>" left but not entered lto1: internal compiler error: in linemap_line_start, at libcpp/line-map.c:803 0x17b4ebc linemap_line_start(line_maps*, unsigned int, unsigned int) ../../libcpp/line-map.c:803 0xbadeb3 lto_location_cache::apply_location_cache() ../../gcc/lto-streamer-in.c:193 0x7a3399 lto_read_decls ../../gcc/lto/lto.c:1788 0x7a3c38 lto_file_finalize ../../gcc/lto/lto.c:2043 0x7a3c8c lto_create_files_from_ids ../../gcc/lto/lto.c:2053 0x7a3db8 lto_file_read ../../gcc/lto/lto.c:2094 0x7a7632 read_cgraph_and_symbols ../../gcc/lto/lto.c:2804 0x7a86bf lto_main() ../../gcc/lto/lto.c:3304 Martin
It's failing this assertion: 803 linemap_assert (SOURCE_LINE (map, r) == to_line); here: (gdb) bt #2 0x0000000001257bba in linemap_line_start (set=0x7ffff7ffb000, to_line=to_line@entry=1048577, max_column_hint=128) at ../../src/libcpp/line-map.c:803 #3 0x000000000092e0da in lto_location_cache::apply_location_cache (this=this@entry=0x1dc6e08) at ../../src/gcc/lto-streamer-in.c:193 with a bogus-looking line number: (gdb) p to_line $1 = 1048577 The bogus line number is in the LTO data itself.
Looking at the LTO data creation, by putting a breakpoint in cc1plus on lto_output_location to see the values that are written, I see that the bogus-looking location is coming from this ordinary map. It has an insane-looking value for "to_line": (gdb) p *map $13 = {<line_map> = {start_location = 8224, reason = LC_LEAVE}, to_file = 0x2154630 "Unified_cpp_js_src35.ii", to_line = 1048576, included_from = -1, sysp = 0 '\000', m_column_and_range_bits = 12, m_range_bits = 5} This ordinary map was created within cc1plus in response to the "#" line directive: (gdb) bt #0 linemap_add (set=0x7ffff7ffb000, reason=LC_RENAME, sysp=0, to_file=0x2154630 "Unified_cpp_js_src35.ii", to_line=1048576) at ../../src/libcpp/line-map.c:560 #1 0x0000000001541f13 in _cpp_do_file_change (pfile=0x213d5d0, reason=<optimized out>, to_file=<optimized out>, file_line=<optimized out>, sysp=<optimized out>) at ../../src/libcpp/directives.c:1071 #2 0x00000000015420bc in do_linemarker (pfile=0x213d5d0) at ../../src/libcpp/directives.c:1056 #3 0x0000000001541cb0 in _cpp_handle_directive (pfile=pfile@entry=0x213d5d0, indented=<optimized out>) at ../../src/libcpp/directives.c:510 Within linemap_add, it transitions from this map: (gdb) p map[-1] $47 = {<line_map> = {start_location = 32, reason = LC_ENTER}, to_file = 0x2161630 "Unified_cpp_js_src35.ii", to_line = 1, included_from = -1, sysp = 0 '\000', m_column_and_range_bits = 12, m_range_bits = 5} to this map: (gdb) p *map $46 = {<line_map> = {start_location = 0, reason = LC_LEAVE}, to_file = 0x0, to_line = 0, included_from = 0, sysp = 0 '\000', m_column_and_range_bits = 0, m_range_bits = 0} and this conditional fires: if (MAIN_FILE_P (map - 1)) and we hit this error-handling: /* A TO_FILE of NULL is special - we use the natural values. */ if (error || to_file == NULL) { to_file = ORDINARY_MAP_FILE_NAME (from); to_line = SOURCE_LINE (from, from[1].start_location); sysp = ORDINARY_MAP_IN_SYSTEM_HEADER_P (from); } and so we have: (gdb) p to_file $48 = 0x2161630 "Unified_cpp_js_src35.ii" (gdb) p to_line $49 = 1048576 (gdb) p /x to_line $50 = 0x100000 giving us the bogus to_line value. Where is this "to_line" value coming from? 551 to_line = SOURCE_LINE (from, from[1].start_location); Breakpoint 9, SOURCE_LINE (ord_map=0x7ffff7fed000, loc=0) at ../../src/libcpp/include/line-map.h:1092 (gdb) p *ord_map $54 = {<line_map> = {start_location = 32, reason = LC_ENTER}, to_file = 0x2161630 "Unified_cpp_js_src35.ii", to_line = 1, included_from = -1, sysp = 0 '\000', m_column_and_range_bits = 12, m_range_bits = 5} 1088 /* Converts a map and a source_location to source line. */ 1089 inline linenum_type 1090 SOURCE_LINE (const line_map_ordinary *ord_map, source_location loc) 1091 { 1092 return ((loc - ord_map->start_location) 1093 >> ord_map->m_column_and_range_bits) + ord_map->to_line; 1094 } 1095 so we have this calculation: ((0 - 32) >> 12) + 1 (gdb) p /x ((unsigned int)(0 - 32)) >> 12 $59 = 0xfffff (gdb) p /x (((unsigned int)(0 - 32)) >> 12) + 1 $60 = 0x100000 which explains where the bogus value is coming from.
A more minimal reproducer for these insane line numbers: $ cat test.C # 9 "" 2 not_a_type a; # With recent trunk: $ gcc -c test.C line-map.c: file "<stdin>" left but not entered test.C:1048537:1: error: ‘not_a_type’ does not name a type # With gcc 4.8.3: $ /usr/bin/gcc -c test.C line-map.c: file "<stdin>" left but not entered test.C:33554393:1: error: ‘not_a_type’ does not name a type
(FWIW, the test case seems to run to completion if I remove the assert in comment #1)
(In reply to David Malcolm from comment #2) > Within linemap_add, it transitions from this map: > (gdb) p map[-1] > $47 = {<line_map> = {start_location = 32, reason = LC_ENTER}, to_file = > 0x2161630 "Unified_cpp_js_src35.ii", to_line = 1, > included_from = -1, sysp = 0 '\000', m_column_and_range_bits = 12, > m_range_bits = 5} > > to this map: > (gdb) p *map > $46 = {<line_map> = {start_location = 0, reason = LC_LEAVE}, to_file = 0x0, > to_line = 0, included_from = 0, sysp = 0 '\000', > m_column_and_range_bits = 0, m_range_bits = 0} start_location cannot decrease. What creates this map? Something seems to be handling wrong the "left but not entered" problem. It should not create a LC_LEAVE map.
Note that it is ok to completely ignore such an invalid line directive and: line-map.c: file "<stdin>" left but not entered should actually be an error or an ICE. As the code says: /* Depending upon whether we are handling preprocessed input or not, this can be a user error or an ICE. */ if (error) fprintf (stderr, "line-map.c: file \"%s\" left but not entered\n", to_file); The problem is that nobody so far has done the work of adding diagnostics/ICE reporting support to line-maps.c, so that the above can be a proper ICE or diagnostic (perhaps the error should be reported by the caller, who presumably knows whether this comes from preprocessed input?). There is also: /* Depending upon whether we are handling preprocessed input or not, this can be a user error or an ICE. */ for (map = LINEMAPS_LAST_ORDINARY_MAP (set); ! MAIN_FILE_P (map); map = INCLUDED_FROM (set, map)) fprintf (stderr, "line-map.c: file \"%s\" entered but not left\n", ORDINARY_MAP_FILE_NAME (map));
Confirmed.
Created attachment 37889 [details] Candidate patch Testing this.
Hmm, seems to break Ada of all things...
When looking at DECL_SOURCE_LOCATION of 'a' during compilation we get #1 0x0000000001359e29 in streamer_write_tree_bitfields (ob=0x28ab160, expr= <var_decl 0x7ffff7fefb40 a>) at /space/rguenther/src/svn/trunk3/gcc/tree-streamer-out.c:434 434 stream_output_location (ob, &bp, DECL_SOURCE_LOCATION (expr)); (gdb) down #0 lto_output_location (ob=0x28ab160, bp=0x7fffffffd750, loc=12480) at /space/rguenther/src/svn/trunk3/gcc/lto-streamer-out.c:176 176 bp_pack_value (bp, ob->current_file != xloc.file, 1); (gdb) p xloc $2 = {file = 0x28f5680 "t.ii", line = 1048577, column = 5, data = 0x0, sysp = false} so the bogus info is there right from the start. Initialized through Old value = 0 New value = 12480 make_node_stat (code=VAR_DECL) at /space/rguenther/src/svn/trunk3/gcc/tree.c:1030 1029 DECL_SOURCE_LOCATION (t) = input_location; (gdb) p expand_location (input_location) $10 = {file = 0x28f5680 "t.ii", line = 1048577, column = 5, data = 0x0, sysp = false} there are other similarly bogus locations like New value = 8224 fe_file_change (new_map=0x7ffff7fe1020) at /space/rguenther/src/svn/trunk3/gcc/c-family/c-lex.c:228 228 (*debug_hooks->end_source_file) (LINEMAP_LINE (new_map)); (gdb) p expand_location (8224) $13 = {file = 0x28f5680 "t.ii", line = 1048576, column = 0, data = 0x0, sysp = false} that's when c-lex.c does 226 input_location = new_map->start_location; maybe this location isn't supposed to be expanded? Or nothing is ever supposed to get this location? For example changing the testcase to (invalid) namespace js {} # 9 "" 2 namespace js { int a } results in > ./cc1plus -quiet t.ii line-map.c: file "<stdin>" left but not entered t.ii:1048539:1: error: expected initializer before ‘}’ token exposing those same bogus locations to the user. Marking as libcpp issue. Note the bogus line info on the diagnostic (and thus the underlying issue) is not a regression it seems (checked up to GCC 4.1). Not sure who's familiar with the C/C++ FE interfacing to libcpp and if those "bogus" locations should simply never be assigned to input_location...
(In reply to Richard Biener from comment #10) > maybe this location isn't supposed to be expanded? > > Or nothing is ever supposed to get this location? > > For example changing the testcase to (invalid) > > namespace js {} > # 9 "" 2 > namespace js { > int a > } This is an invalid linemarker. libcpp should ignore it completely and behave as if it was not present. It seems it is not doing that for some reason and that is messing up the line-table.
On Wed, 23 Mar 2016, manu at gcc dot gnu.org wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69650 > > --- Comment #11 from Manuel López-Ibáñez <manu at gcc dot gnu.org> --- > (In reply to Richard Biener from comment #10) > > maybe this location isn't supposed to be expanded? > > > > Or nothing is ever supposed to get this location? > > > > For example changing the testcase to (invalid) > > > > namespace js {} > > # 9 "" 2 > > namespace js { > > int a > > } > > This is an invalid linemarker. libcpp should ignore it completely and behave as > if it was not present. It seems it is not doing that for some reason and that > is messing up the line-table. Sure, but I don't see that 1048539 line number anywhere so it feels like sth is used uninitialized?
(In reply to Richard Biener from comment #10) > Not sure who's familiar with the C/C++ FE interfacing to libcpp and if > those "bogus" locations should simply never be assigned to input_location... As long as we keep mucking up input_location to hack passing around the proper locations, we will keep having surprises like this caused by unrelated changes elsewhere.
(In reply to rguenther@suse.de from comment #12) > > This is an invalid linemarker. libcpp should ignore it completely and behave as > > if it was not present. It seems it is not doing that for some reason and that > > is messing up the line-table. > > Sure, but I don't see that 1048539 line number anywhere so it feels like > sth is used uninitialized? This was thoroughly analyzed by David above. The invalid linemarker causes an invalid LC_LEAVE which then triggers an invalid line-number calculation. Somehow, the code for handling the invalid linemarker is doing the wrong thing. Perhaps this: to_line = SOURCE_LINE (from, from[1].start_location); should be to_line = SOURCE_LINE (from, from->start_location); But I don't understand what the following comment means (love those cryptic comments in GCC!) /* A TO_FILE of NULL is special - we use the natural values. */ if (error || to_file == NULL) { to_file = ORDINARY_MAP_FILE_NAME (from); to_line = SOURCE_LINE (from, from[1].start_location); sysp = ORDINARY_MAP_IN_SYSTEM_HEADER_P (from); } so it may be trying to do something else besides error-handling.
(In reply to Manuel López-Ibáñez from comment #14) > (In reply to rguenther@suse.de from comment #12) > > > This is an invalid linemarker. libcpp should ignore it completely and behave as > > > if it was not present. It seems it is not doing that for some reason and that > > > is messing up the line-table. > > > > Sure, but I don't see that 1048539 line number anywhere so it feels like > > sth is used uninitialized? > > This was thoroughly analyzed by David above. The invalid linemarker causes > an invalid LC_LEAVE which then triggers an invalid line-number calculation. > Somehow, the code for handling the invalid linemarker is doing the wrong > thing. In any case, the current error-handling creates: {<line_map> = {start_location = 8224, reason = LC_LEAVE}, to_file = 0x2154630 "Unified_cpp_js_src35.ii", to_line = 1048576, included_from = -1, sysp = 0 '\000', m_column_and_range_bits = 12, m_range_bits = 5} in response to the invalid line-marker. Even if the to_line was correct, the linemarker was invalid, so this map does not even need to exist. The line-table at the end should be the same as if the original file had '//' in front of #.
On Wed, 23 Mar 2016, manu at gcc dot gnu.org wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69650 > > --- Comment #15 from Manuel López-Ibáñez <manu at gcc dot gnu.org> --- > (In reply to Manuel López-Ibáñez from comment #14) > > (In reply to rguenther@suse.de from comment #12) > > > > This is an invalid linemarker. libcpp should ignore it completely and behave as > > > > if it was not present. It seems it is not doing that for some reason and that > > > > is messing up the line-table. > > > > > > Sure, but I don't see that 1048539 line number anywhere so it feels like > > > sth is used uninitialized? > > > > This was thoroughly analyzed by David above. The invalid linemarker causes > > an invalid LC_LEAVE which then triggers an invalid line-number calculation. > > Somehow, the code for handling the invalid linemarker is doing the wrong > > thing. > > In any case, the current error-handling creates: > > {<line_map> = {start_location = 8224, reason = LC_LEAVE}, to_file = 0x2154630 > "Unified_cpp_js_src35.ii", to_line = 1048576, included_from = -1, sysp = 0 > '\000', m_column_and_range_bits = 12, m_range_bits = 5} > > in response to the invalid line-marker. Even if the to_line was correct, the > linemarker was invalid, so this map does not even need to exist. The line-table > at the end should be the same as if the original file had '//' in front of #. Index: libcpp/line-map.c =================================================================== --- libcpp/line-map.c (revision 234415) +++ libcpp/line-map.c (working copy) @@ -543,7 +543,7 @@ linemap_add (struct line_maps *set, enum to_file); /* A TO_FILE of NULL is special - we use the natural values. */ - if (error || to_file == NULL) + if (to_file == NULL) { to_file = ORDINARY_MAP_FILE_NAME (from); to_line = SOURCE_LINE (from, from[1].start_location); "fixes" both issues, the diagnostic and the LTO ICE.
"caused" by r44789 which introduced this path (initially with error || to_file == NULL) with a description of just (add_line_map): Return pointer to const. When passed NULL to_file with LC_LEAVE, use the obvious values for the return point so the caller doesn't have to figure them out. I will test the change.
FWIW Bernd posted a different patch for this, here: https://gcc.gnu.org/ml/gcc-patches/2016-03/msg00598.html
(In reply to Richard Biener from comment #17) > "caused" by r44789 which introduced this path (initially with error || > to_file == NULL) with a description of just > > (add_line_map): Return pointer to const. When passed NULL to_file > with LC_LEAVE, use the obvious values for the return point so the > caller doesn't have to figure them out. > > I will test the change. I'm not sure what this change ends up creating in the line-table. You may end up creating just a different kind of invalid line-table. If it creates a LC_LEAVE or a LC_RENAME with to_file==NULL or to_line==0, that seems broken in just a different way. A LC_RENAME with to_file=="Unified_cpp_js_src35.ii" and to_line==2 would be ok (just useless, because we are basically creating a new map to say: we are at the same file at the same line as before). Also, it may work for this case, since we are returning to the main file and the main file info was ok. I'm not sure what will happen when there are several included files (that is, the other branch that can set error == true).
(In reply to David Malcolm from comment #18) > FWIW Bernd posted a different patch for this, here: > https://gcc.gnu.org/ml/gcc-patches/2016-03/msg00598.html FWIW, if that works for all testcases here, it seems a better solution. Even if it misses some corner-case, it seems a better design to catch this invalid input at the caller and give an error than try to guess in line-map.c what the caller was trying to do and recover somehow.
On Wed, 23 Mar 2016, manu at gcc dot gnu.org wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69650 > > --- Comment #19 from Manuel López-Ibáñez <manu at gcc dot gnu.org> --- > (In reply to Richard Biener from comment #17) > > "caused" by r44789 which introduced this path (initially with error || > > to_file == NULL) with a description of just > > > > (add_line_map): Return pointer to const. When passed NULL to_file > > with LC_LEAVE, use the obvious values for the return point so the > > caller doesn't have to figure them out. > > > > I will test the change. > > I'm not sure what this change ends up creating in the line-table. You may end > up creating just a different kind of invalid line-table. If it creates a > LC_LEAVE or a LC_RENAME with to_file==NULL or to_line==0, that seems broken in > just a different way. A LC_RENAME with to_file=="Unified_cpp_js_src35.ii" and > to_line==2 would be ok (just useless, because we are basically creating a new > map to say: we are at the same file at the same line as before). It depends on what the line directive was parsed with. > Also, it may work for this case, since we are returning to the main file and > the main file info was ok. I'm not sure what will happen when there are several > included files (that is, the other branch that can set error == true). We parse the thing as LC_RENAME, so I think we're fine and any further bug would be in the callers. That said, I'd like to see motivating testcases for the error || as it seemingly was introduced as an unrelated change that may seemed to make sense (but certainly "obvious values" as the ChangeLog claims is only valid for the NULL to_file case, not for the error case as seen here).
On Wed, 23 Mar 2016, manu at gcc dot gnu.org wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69650 > > --- Comment #20 from Manuel López-Ibáñez <manu at gcc dot gnu.org> --- > (In reply to David Malcolm from comment #18) > > FWIW Bernd posted a different patch for this, here: > > https://gcc.gnu.org/ml/gcc-patches/2016-03/msg00598.html > > FWIW, if that works for all testcases here, it seems a better solution. Even if > it misses some corner-case, it seems a better design to catch this invalid > input at the caller and give an error than try to guess in line-map.c what the > caller was trying to do and recover somehow. Yes, but not sure if issuing an error is ok at this point - I know there are some code generators out there that emit line directives we currently warn about. Other compilers may preprocess into such as well. Maybe we can put the error under some new flag though.
(In reply to rguenther@suse.de from comment #22) > Maybe we can put the error under some new flag though. Does Bernd's patch still work if we just warn instead of error? I still think doing this in the caller is the sane thing to do.
Created attachment 38071 [details] alternate patch Good question - I attached mine and will check.
(In reply to Manuel López-Ibáñez from comment #23) > (In reply to rguenther@suse.de from comment #22) > > Maybe we can put the error under some new flag though. > > Does Bernd's patch still work if we just warn instead of error? I still > think doing this in the caller is the sane thing to do. Yes, Bernd's patch still works then. I'd prefer this at this stage. There doesn't seem to be any CPP_W_* that fits though.
Also, let's keep in mind the issue David found - "left but not entered" seems like a misleading message, something like "unexpectedly reentered" seems to fit better.
On Wed, 23 Mar 2016, bernds at gcc dot gnu.org wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69650 > > --- Comment #26 from Bernd Schmidt <bernds at gcc dot gnu.org> --- > Also, let's keep in mind the issue David found - "left but not entered" seems > like a misleading message, something like "unexpectedly reentered" seems to fit > better. Sure, though that part is not a regression (only the LTO ICE is for some reason)
(In reply to Richard Biener from comment #25) > Yes, Bernd's patch still works then. I'd prefer this at this stage. > There doesn't seem to be any CPP_W_* that fits though. Do you need one? Perhaps -Wcpp? Or -Wpreprocessed? I think it should be a warning enabled by default.
(In reply to Bernd Schmidt from comment #26) > Also, let's keep in mind the issue David found - "left but not entered" > seems like a misleading message, something like "unexpectedly reentered" > seems to fit better. But the linemaker is an invalid "leaving file" marker. So the error is that we are leaving a file that we have not entered. Perhaps I'm misunderstanding what you mean?
I'm just using CPP_W_NONE in the patch I'm testing and that seems to work. Regarding the error message, here's what we'll get: 69650.c:4:55: warning: file "fish" left but not entered # 9 "fish" 2 /* { dg-error "left but not entered" } */ The problem is that the file we'd be leaving is 69650.c, as shown by the warning's location. "fish" AFAICT is the file we'd be switching to.
(In reply to Bernd Schmidt from comment #30) > I'm just using CPP_W_NONE in the patch I'm testing and that seems to work. > > Regarding the error message, here's what we'll get: > > 69650.c:4:55: warning: file "fish" left but not entered > # 9 "fish" 2 /* { dg-error "left but not entered" } */ > > The problem is that the file we'd be leaving is 69650.c, as shown by the > warning's location. "fish" AFAICT is the file we'd be switching to. I may have been wrong in my earlier question on the mailing list; doesn't the flag value of 2 mean "LC_LEAVE"? (is the filename meant to refer to the file being left, or the file being entered? am new to this part of libcpp)
(In reply to David Malcolm from comment #31) > I may have been wrong in my earlier question on the mailing list; doesn't > the flag value of 2 mean "LC_LEAVE"? (is the filename meant to refer to the > file being left, or the file being entered? am new to this part of libcpp) This is my understanding as well: https://gcc.gnu.org/onlinedocs/cpp/Preprocessor-Output.html That is, it means "leaving fish returning to 69650.c". If there is not previous enter "fish" directive, there is a problem. An empty file means, leaving whatever file we last entered. If there is no previous enter file directive at all (# N "whatever" 1), then there is also a problem. One cannot leave the main file. It should be easy to play with the line directives to trigger both types of error.
It does mean LC_LEAVE, but AFAICT the filename is the file being returned to. Including a file called "t.h" from "v.c" gives this after -E: # 1 "t.h" 1 int t; # 2 "v.c" 2
(In reply to Bernd Schmidt from comment #33) > It does mean LC_LEAVE, but AFAICT the filename is the file being returned to. > > Including a file called "t.h" from "v.c" gives this after -E: > > # 1 "t.h" 1 > int t; > # 2 "v.c" 2 Then, I agree the error doesn't make any sense. My assumption of how the filename in the directive was being used was completely wrong. But then in case of error, one cannot really tell which file was going to be left. However, one could detect which file we are suppose to enter, like in: # 1 "test.c" # 1 "a.h" 1 # 1 "b.h" 1 int a; # 1 "c.h" 2 # 2 "test.c" 2
I don't think guesswork will be very helpful in practice with a corrupted #line structure, and errors of this nature shouldn't really occur anyway outside of artificial testcases. I'm leaning towards either something like 69650.c:4:55: warning: file "fish" unexpectedly reentered or simply 69650.c:4:55: warning: line directive ignored due to incorrect nesting
(In reply to Bernd Schmidt from comment #35) > I don't think guesswork will be very helpful in practice with a corrupted > #line structure, and errors of this nature shouldn't really occur anyway > outside of artificial testcases. I'm leaning towards either something like > > 69650.c:4:55: warning: file "fish" unexpectedly reentered > > or simply > > 69650.c:4:55: warning: line directive ignored due to incorrect nesting According to the manual, these are linemarkers (line directives are less expressive): https://gcc.gnu.org/onlinedocs/cpp/Preprocessor-Output.html https://gcc.gnu.org/onlinedocs/cpp/Line-Control.html warning: linemarker ignored because file unexpectedly reentered ? I know, bike-shedding... :)
Author: law Date: Fri Mar 25 16:15:39 2016 New Revision: 234481 URL: https://gcc.gnu.org/viewcvs?rev=234481&root=gcc&view=rev Log: PR lto/69650 * directives.c (do_linemarker): Test for file left but not entered here. * line-map.c (linemap_add): Not here. PR lto/69650 * gcc.dg/pr69650.c: New test. Added: trunk/gcc/testsuite/gcc.dg/pr69650.c Modified: trunk/gcc/testsuite/ChangeLog trunk/libcpp/ChangeLog trunk/libcpp/directives.c trunk/libcpp/line-map.c
Fixed by Bernd's patch on the trunk.
The resolution of this issue causes a problem for me as it results in compilation errors in our build. We are compiling with distcc which AIUI distributes the pre-processed intermediate output. With revision 234480 we get no warnings logged and clean compilation. With revision 234481 we get a number of warnings logged - but this interacts unfortunately with -Werror (which we also set) and so quite a number of our compilations fail. (I cannot as yet detect a pattern with which compilations are affected, but the set is consistent on a rebuild.) I can reproduce the distcc problem by using -E to generate pre-processed source and compiling this. Note: the pre-processed intermediate output does not change between 234480 and 234481, it's the compilation of this pre-processed output that fails. Unfortunately the failing code is proprietary so I am not at liberty to upload the pre-processed output, I have attempted to produce some smaller examples but so far with no success. Could the generation of the warning be made dependent upon a more specific warning flag?
The message previously was a plain printf, not a warning or error so it would not have interrupted compilation. Did you get the message with earlier compilers, or did it start to appear recently?
I have seen the message before: for example from a build with revision line-map.c: file "/usr/include/asm/sockios.h" left but not entered I've only noticed it with builds from gcc-trunk, which I tested with back in Dec/Jan. I saw this message from some compilation but it seemed benign... I have not seen this message with builds from gcc 5.3.0 and before, but it is possible I've simply not noticed as it does gets a bit buried in the build logs.
(In reply to Roger Orr from comment #41) > I have seen the message before: for example from a build with revision > > line-map.c: file "/usr/include/asm/sockios.h" left but not entered You can also simply filter every '^# [0-9]' line from the preprocessed output before compiling. There must be some broken linemarker, perhaps as a result of copy-pasting from another preprocessed file. I wonder, don't we give a precise location now for the broken linemarker? Perhaps we should control this with -Wlinemarker, such that people can use -Wno-linemarker. We may have underestimated how much broken code might be out there.
I suspect the message Roger quoted was from the earlier version with the printf; that one didn't give a location. I advocate no action until we have a reproducer so we can know what's going on.
Created attachment 38218 [details] Intermediate file I am as yet unable to upload the code for an example. However, I've attached a file containing the generated line directives using grep "#" rsMessaging.cxx.ii > lineDirectives from one of the troublesome pre-processsed output files (rsMessaging.cxx.ii). As far as I can tell the directives are correct. The .ii file generates the following diagnostic when compiled: $ /var/tmp/gcc-trunk-234481/install/bin/g++ -std=c++14 -O3 -Werror -c rsMessaging.cxx.ii In file included from /opt/reactor-buildkit/B2BH-BK2GIT44-1/poco/include/Poco/Net/IPAddress.h:379:0, from /home/rorr/linuxdev109ws_9119/rorr_linuxdev109ws_9119/Reactor5/project/prj-lcc-device-longer-name/reactor/src/base/messaging/messaging/rsHintServerBase.hxx:18, from /home/rorr/linuxdev109ws_9119/rorr_linuxdev109ws_9119/Reactor5/project/prj-lcc-device-longer-name/reactor/src/base/messaging/src/rsMessaging.cxx:15: /home/rorr/linuxdev109ws_9119/rorr_linuxdev109ws_9119/Reactor5/project/prj-lcc-device-longer-name/reactor/src/base/hxx/rsEnumMnemonic.hxx:117:165: error: file "/home/rorr/linuxdev109ws_9119/rorr_linuxdev109ws_9119/Reactor5/project/prj-lcc-device-longer-name/reactor/src/base/messaging/messaging/rsHintServerBase.hxx" linemarker ignored due to incorrect nesting [-Werror] ^ /home/rorr/linuxdev109ws_9119/rorr_linuxdev109ws_9119/Reactor5/project/prj-lcc-device-longer-name/reactor/src/base/hxx/rsEnumMnemonic.hxx:212:154: error: file "/home/rorr/linuxdev109ws_9119/rorr_linuxdev109ws_9119/Reactor5/project/prj-lcc-device-longer-name/reactor/src/base/messaging/src/rsMessaging.cxx" linemarker ignored due to incorrect nesting [-Werror] cc1plus: all warnings being treated as errors
While investigating the problem I am experiencing I have found it is stable across g++ builds - I have tried a build using revision 234741 which has the identical diagnostic output from the compilation. However, small changes to the source code -- such as changing the length of the directory paths involved -- can change whether or not the diagnostic is produced. In the example I posted, if I change path "/home/rorr/linuxdev109ws_9119/..." to "/home/rorr/..." in the line directives of the ii file the compilation succeeds without problem. Hence it is not clear to me whether this diagnostic is the root problem or merely a symptom of a different problem - possibly a memory overwrite or use of an uninitialised variable whose value depends on the history of the memory location.
I tried compiling this file, with C or C++, with or without -fpreprocessed, all sorts of options including -std=c++14 -O3, and did not get a compilation failure. Can anyone else reproduce this?
Sorry, I wasn't clear enough. I'm unfortunately not at present able to post the whole source file. The file I posted *only* contains the line directives, so people can confirm whether there were any problems with them. (That file compiles fine for me too) There seems to be some interaction with some other part of the compilation, I'm trying to get a example I can post but this is proving troublesome.
Try running cc1plus with valgrind to see whether that detects anything untoward.
Alas, valgrind finds nothing untoward. make MAKEINFO=true STAGE1_CXXFLAGS="-g -O0" clean-stage1 make MAKEINFO=true STAGE1_CXXFLAGS="-g -O0" all-stage1 valgrind /var/tmp/gcc-trunk-234481/build/gcc/cc1plus -fpreprocessed direct.ii -quiet -dumpbase direct.ii -mtune=generic -march=x86-64 -auxbase direct -O3 -Werror -std=c++14 -fmax-errors=1 -o /tmp/ccGReQOF.s I've also tried a build with -fsanitize=address,undefined but this too reports no problems. Running under gdb and breaking at libcpp/directives.c:1057 I can see the following: (gdb) p MAIN_FILE_P(map) $17 = true # This is the reason why it is trying to print the message if (reason == LC_LEAVE) { const line_map_ordinary *from; if (MAIN_FILE_P (map) || (/*...*/)) { cpp_warning ( // ... (gdb) p map $18 = (const line_map_ordinary *) 0x7fffec6ddfe0 (gdb) p *map $19 = {<line_map> = {start_location = 2779096485, reason = 165}, to_file = 0xa5a5a5a5a5a5a5a5 <error: Cannot access memory at address 0xa5a5a5a5a5a5a5a5>, to_line = 2779096485, included_from = -1515870811, sysp = 165 '▒', m_column_and_range_bits = 165, m_range_bits = 165} # These values look concerning ... what do they mean and where do they come from?! 'map' comes from line 979: const line_map_ordinary *map = LINEMAPS_LAST_ORDINARY_MAP (line_table); (gdb) p line_table $20 = (line_maps *) 0x7fffecf8b000 (gdb) p *line_table $21 = {info_ordinary = {maps = 0x7fffec9f4000, allocated = 16384, used = 4097, cache = 4096}, info_macro = {maps = 0x0, allocated = 0, used = 0, cache = 0}, depth = 3, trace_includes = false, highest_location = 485446816, highest_line = 485446816, max_column_hint = 256, reallocator = 0x25409c0 <realloc_for_line_map(void*, size_t)>, round_alloc_size = 0x11bf0fa <ggc_round_alloc_size(unsigned long)>, location_adhoc_data_map = {htab = 0x60b00000ad80, curr_loc = 4140, allocated = 8192, data = 0x7fffeb80c000}, builtin_location = 1, seen_line_directive = true, default_range_bits = 5, num_optimized_ranges = 486644, num_unoptimized_ranges = 4140} This code is unknown to me, so I am not sure where to look next ...
The 0xa5 bytes probably come from ggc-page, which tries to poison freed memory. This does suggest that there's a use-after-free issue here. At the step after "p map", try "watch -l map->to_file" followed by "run", which should get you to the point where the memory is overwritten.
Thanks, I hope to try that on Monday. The function gets called multiple times, so I need to identify the last time. The memory must have changed *during* the call to do_linemarker as the new_file variable is set from map on line 981: const char *new_file = ORDINARY_MAP_FILE_NAME (map); but the same call executed now fails: (gdb) p ORDINARY_MAP_FILE_NAME (map) $3 = 0xa5a5a5a5a5a5a5a5 <error: Cannot access memory at address 0xa5a5a5a5a5a5a5a5>
Ah - added print of *pfile->line_table at line 978 and compared with the value when we error. 978 struct line_maps *line_table = pfile->line_table; $3702 = {info_ordinary = {maps = 0x7fffec65d000, allocated = 4096, used = 4096, cache = 4095}, info_macro = {maps = 0x0, allocated = 0, used = 0, cache = 0}, depth = 8, trace_includes = false, highest_location = 511461984, highest_line = 511461856, max_column_hint = 128, reallocator = 0x25409c0 <realloc_for_line_map(void*, size_t)>, round_alloc_size = 0x11bf0fa <ggc_round_alloc_size(unsigned long)>, location_adhoc_data_map = {htab = 0x60b00000ad80, curr_loc = 4884, allocated = 8192, data = 0x7fffeb7ab000}, builtin_location = 1, seen_line_directive = true, default_range_bits = 5, num_optimized_ranges = 500030, num_unoptimized_ranges = 4884} (gdb) p *pfile->line_table $3703 = {info_ordinary = {maps = 0x7fffec993000, allocated = 16384, used = 4097, cache = 4096}, info_macro = {maps = 0x0, allocated = 0, used = 0, cache = 0}, depth = 8, trace_includes = false, highest_location = 511470272, highest_line = 511470272, max_column_hint = 256, reallocator = 0x25409c0 <realloc_for_line_map(void*, size_t)>, round_alloc_size = 0x11bf0fa <ggc_round_alloc_size(unsigned long)>, location_adhoc_data_map = {htab = 0x60b00000ad80, curr_loc = 4885, allocated = 8192, data = 0x7fffeb7ab000}, builtin_location = 1, seen_line_directive = true, default_range_bits = 5, num_optimized_ranges = 500031, num_unoptimized_ranges = 4885} The 'mapos' address has changed and the allocated size has changed from 4096 to 16384. The 'map' variable points 0x1ffe0 bytes beyond the *old* value of maps.
Hmm. Something got reallocated perhaps? It's not entirely obvious to me where that would happen. Does the following help at all? Index: directives.c =================================================================== --- directives.c (revision 234831) +++ directives.c (working copy) @@ -977,6 +977,8 @@ do_linemarker (cpp_reader *pfile) { struct line_maps *line_table = pfile->line_table; const line_map_ordinary *map = LINEMAPS_LAST_ORDINARY_MAP (line_table); + bool main_file_p = MAIN_FILE_P (map); + const line_map_ordinary *from = INCLUDED_FROM (pfile->line_table, map); const cpp_token *token; const char *new_file = ORDINARY_MAP_FILE_NAME (map); linenum_type new_lineno; @@ -1048,10 +1050,9 @@ do_linemarker (cpp_reader *pfile) if (reason == LC_LEAVE) { - const line_map_ordinary *from; - if (MAIN_FILE_P (map) + if (main_file_p || (new_file - && (from = INCLUDED_FROM (pfile->line_table, map)) != NULL + && from != NULL && filename_cmp (ORDINARY_MAP_FILE_NAME (from), new_file) != 0)) { cpp_warning (pfile, CPP_W_NONE,
Unfortunately the patch does not help: the cached 'from' pointer is a pointer into the old maps entry -- the one which has now been deallocated. The first test, main_file_p, now (correctly) fails. The second test, (new_file && from != NULL && filename_cmp (ORDINARY_MAP_FILE_NAME (from), new_file) != 0) now SEGVs as the memory area 'from' refers has been freed, so ORDINARY_MAP_FILE_NAME returns a pointer value read from this freed memory, which now contains 0x5a. The call stack of the re-allocation is: #0 __memset_x86_64 () at ../sysdeps/x86_64/memset.S:1229 #1 0x0000000000b1d595 in ggc_free (p=0x7ffff18b2000) at ../../gcc/ggc-page.c:1611 #2 0x0000000000d5019d in ggc_realloc (x=0x7ffff18b2000, size=524288) at ../../gcc/ggc-common.c:162 #3 0x00000000010ce764 in realloc_for_line_map (ptr=0x7ffff18b2000, len=524288) at ../../gcc/toplev.c:939 #4 0x0000000001b223aa in new_linemap (set=0x7ffff7ffc000, reason=LC_RENAME) at ../../libcpp/line-map.c:427 #5 0x0000000001b2264b in linemap_add (set=0x7ffff7ffc000, reason=LC_RENAME, sysp=2, to_file=0x2c9ba00 "/opt/reactor-buildkit/B2BH-BK2GIT44-1/poco/include/Poco/Net/IPAddress.h", to_line=344) at ../../libcpp/line-map.c:500 #6 0x0000000001b22d2d in linemap_line_start (set=0x7ffff7ffc000, to_line=344, max_column_hint=256) at ../../libcpp/line-map.c:748 #7 0x0000000001b22f1a in linemap_position_for_column (set=0x7ffff7ffc000, to_column=162) at ../../libcpp/line-map.c:809 #8 0x0000000001b2066f in _cpp_lex_direct (pfile=0x2b20310) at ../../libcpp/lex.c:2730 #9 0x0000000001b1f0f2 in _cpp_lex_token (pfile=0x2b20310) at ../../libcpp/lex.c:2202 #10 0x0000000001b29956 in cpp_get_token_1 (pfile=0x2b20310, location=0x0) at ../../libcpp/macro.c:2439 #11 0x0000000001b29d9b in cpp_get_token (pfile=0x2b20310) at ../../libcpp/macro.c:2581 #12 0x0000000001b0cec6 in do_linemarker (pfile=0x2b20310) at ../../libcpp/directives.c:1009 #13 0x0000000001b0bf65 in _cpp_handle_directive (pfile=0x2b20310, indented=0) at ../../libcpp/directives.c:510 #14 0x0000000001b1f142 in _cpp_lex_token (pfile=0x2b20310) at ../../libcpp/lex.c:2214 #15 0x0000000001b29956 in cpp_get_token_1 (pfile=0x2b20310, location=0x7fffffffd144) at ../../libcpp/macro.c:2439 #16 0x0000000001b29dc0 in cpp_get_token_with_location (pfile=0x2b20310, loc=0x7fffffffd144) at ../../libcpp/macro.c:2625 #17 0x0000000000ae8d90 in c_lex_with_flags (value=0x7fffffffd148, loc=0x7fffffffd144, cpp_flags=0x7fffffffd142 "@▒▒S▒\034", lex_flags=2) at ../../gcc/c-family/c-lex.c:391 #18 0x0000000000900009 in cp_lexer_get_preprocessor_token (lexer=0x7ffff1f2b480, token=0x7fffffffd140) at ../../gcc/cp/parser.c:792 #19 0x00000000008ffcb4 in cp_lexer_new_main () at ../../gcc/cp/parser.c:656 #20 0x000000000090349b in cp_parser_new () at ../../gcc/cp/parser.c:3689 #21 0x0000000000952055 in c_parse_file () at ../../gcc/cp/parser.c:37405 #22 0x0000000000af45f8 in c_common_parse_file () at ../../gcc/c-family/c-opts.c:1064 #23 0x00000000010cdb41 in compile_file () at ../../gcc/toplev.c:465 #24 0x00000000010d00e9 in do_compile () at ../../gcc/toplev.c:1988 #25 0x00000000010d036e in toplev::main (this=0x7fffffffd260, argc=16, argv=0x7fffffffd368) at ../../gcc/toplev.c:2096 #26 0x0000000001ae22ce in main (argc=16, argv=0x7fffffffd368) at ../../gcc/main.c:39 and the arguments to memset() are: void *s = 0x7ffff18b2000 int c = 0xa5a size_t n = 131782 In frame 12, from = 0x7ffff18d0c20, which is within this range and so the contents are set to 0x5a. I've tried out this patch, re-reading the potentially changed maps value from pfile->line_table, and it seems to work for me: *** ../../gcc-trunk-234481-original/libcpp/directives.c 2016-04-07 12:46:40.000000000 +0100 --- directives.c 2016-04-11 11:54:18.000000000 +0100 *************** do_linemarker (cpp_reader *pfile) *** 1048,1053 **** --- 1048,1056 ---- if (reason == LC_LEAVE) { + // reload map in case re-allocation has occurred + const line_map_ordinary *map = LINEMAPS_LAST_ORDINARY_MAP (pfile->line_table); + const line_map_ordinary *from; if (MAIN_FILE_P (map) || (new_file
Note - I should have added that I am not at all sure the fix above is *correct*, simply that it prevents accessing the freed entry. I don't know enough about how the code works to know whether the value obtained by re-loading via: maps = LINEMAPS_LAST_ORDINARY_MAP (pfile->line_table); is reading the the value that _actually_ needs to be verified in the subsequent code: the logical entry referred to by "LINEMAPS_LAST_ORDINARY_MAP" might have changed from the one obtained at the start of the function.
Ok, thanks for the effort in identifying the problem. I should reopen this so it gets tracked as a regression. I'll put your patch through a test cycle.
Created attachment 38232 [details] Stripped down intermediate file I've managed to reproduce the problem without including any proprietary code - code replaced with blank lines between the "#" lines (and a slight change in the number of includes) now provokes the diagnostic. /var/tmp/gcc-trunk-234481/install/libexec/gcc/x86_64-pc-linux-gnu/6.0.0/cc1plus -fpreprocessed pr69650.ii -quiet -dumpbase pr69650.ii -mtune=generic -march=x86-64 -auxbase-strip /var/tmp/pr69650.o -O3 -Werror -std=c++14 -o /tmp/ccNlrUZ8.s In file included from /opt/reactor-buildkit/B2BH-BK2GIT44-1/poco/include/Poco/Net/IPAddress.h:346:0, from /home/rorr/linuxdev109ws_9119/rorr_linuxdev109ws_9119/Reactor5/project/prj-lcc-device-longer-name/reactor/src/base/messaging/messaging/rsHintServerBase.hxx:19, from /home/rorr/linuxdev109ws_9119/rorr_linuxdev109ws_9119/Reactor5/project/prj-lcc-device-longer-name/reactor/src/base/messaging/src/rsMessaging.cxx:16: /home/rorr/linuxdev109ws_9119/rorr_linuxdev109ws_9119/Reactor5/project/prj-lcc-device-longer-name/reactor/src/base/hxx/rsEnumMnemonic.hxx:10:165: error: file "/home/rorr/linuxdev109ws_9119/rorr_linuxdev109ws_9119/Reactor5/project/prj-lcc-device-longer-name/reactor/src/base/messaging/messaging/rsHintServerBase.hxx" linemarker ignored due to incorrect nesting [-Werror] /home/rorr/linuxdev109ws_9119/rorr_linuxdev109ws_9119/Reactor5/project/prj-lcc-device-longer-name/reactor/src/base/hxx/rsEnumMnemonic.hxx:12:154: error: file "/home/rorr/linuxdev109ws_9119/rorr_linuxdev109ws_9119/Reactor5/project/prj-lcc-device-longer-name/reactor/src/base/messaging/src/rsMessaging.cxx" linemarker ignored due to incorrect nesting [-Werror] cc1plus: all warnings being treated as errors (234480 compiles cleanly)
Author: bernds Date: Wed Apr 13 11:40:37 2016 New Revision: 234932 URL: https://gcc.gnu.org/viewcvs?rev=234932&root=gcc&view=rev Log: Patch from Roger Orr <rogero@howzatt.demon.co.uk> PR preprocessor/69650 * directives.c (do_linemarker): Reread map after calling cpp_get_token. Modified: trunk/libcpp/ChangeLog trunk/libcpp/directives.c
Should be fixed for good now.
Thanks; I can now confirm that a full build of our application with distcc works without problems.