[Bug preprocessor/83173] C preprocessor generates incorrect linemarkers
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83173 David Malcolm changed: What|Removed |Added Status|UNCONFIRMED |RESOLVED Resolution|--- |FIXED --- Comment #17 from David Malcolm --- Fixed on trunk by r266516.
[Bug preprocessor/83173] C preprocessor generates incorrect linemarkers
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83173 --- Comment #16 from Mike Gulick --- (In reply to Eric Gallager from comment #15) > So can this be closed as FIXED now? Yes, fixed by r266516.
[Bug preprocessor/83173] C preprocessor generates incorrect linemarkers
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83173 --- Comment #15 from Eric Gallager --- (In reply to David Malcolm from comment #14) > Author: dmalcolm > Date: Tue Nov 27 16:04:31 2018 > New Revision: 266520 > > URL: https://gcc.gnu.org/viewcvs?rev=266520=gcc=rev > Log: > PR preprocessor/83173: Enhance -fdump-internal-locations output > > gcc/ChangeLog: > 2018-11-27 Mike Gulick > > PR preprocessor/83173 > * input.c (dump_location_info): Dump reason and included_from > fields from line_map_ordinary struct. Fix indentation when > location > 5 digits. > * diagnostic-show-locus.c (num_digits, num_digits): Move to > diagnostic.c to allow it to be utilized by input.c. > * diagnostic.c (num_digits, selftest::test_num_digits): Moved > here. > (selftest::diagnostic_c_tests): Run selftest::test_num_digits. > * diagnostic.h (num_digits): Add extern definition. > > libcpp/ChangeLog: > 2018-11-27 Mike Gulick > > PR preprocessor/83173 > * location-example.txt: Update example -fdump-internal-locations > output. > > > Modified: > trunk/gcc/ChangeLog > trunk/gcc/diagnostic-show-locus.c > trunk/gcc/diagnostic.c > trunk/gcc/diagnostic.h > trunk/gcc/input.c > trunk/libcpp/ChangeLog > trunk/libcpp/location-example.txt So can this be closed as FIXED now?
[Bug preprocessor/83173] C preprocessor generates incorrect linemarkers
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83173 --- Comment #14 from David Malcolm --- Author: dmalcolm Date: Tue Nov 27 16:04:31 2018 New Revision: 266520 URL: https://gcc.gnu.org/viewcvs?rev=266520=gcc=rev Log: PR preprocessor/83173: Enhance -fdump-internal-locations output gcc/ChangeLog: 2018-11-27 Mike Gulick PR preprocessor/83173 * input.c (dump_location_info): Dump reason and included_from fields from line_map_ordinary struct. Fix indentation when location > 5 digits. * diagnostic-show-locus.c (num_digits, num_digits): Move to diagnostic.c to allow it to be utilized by input.c. * diagnostic.c (num_digits, selftest::test_num_digits): Moved here. (selftest::diagnostic_c_tests): Run selftest::test_num_digits. * diagnostic.h (num_digits): Add extern definition. libcpp/ChangeLog: 2018-11-27 Mike Gulick PR preprocessor/83173 * location-example.txt: Update example -fdump-internal-locations output. Modified: trunk/gcc/ChangeLog trunk/gcc/diagnostic-show-locus.c trunk/gcc/diagnostic.c trunk/gcc/diagnostic.h trunk/gcc/input.c trunk/libcpp/ChangeLog trunk/libcpp/location-example.txt
[Bug preprocessor/83173] C preprocessor generates incorrect linemarkers
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83173 --- Comment #13 from David Malcolm --- Author: dmalcolm Date: Tue Nov 27 15:53:51 2018 New Revision: 266518 URL: https://gcc.gnu.org/viewcvs?rev=266518=gcc=rev Log: PR preprocessor/83173: New test 2018-11-27 Mike Gulick PR preprocessor/83173 * gcc.dg/plugin/location-overflow-test-pr83173.c: New test. * gcc.dg/plugin/location-overflow-test-pr83173.h: Header for pr83173.c. * gcc.dg/plugin/location-overflow-test-pr83173-1.h: Header for pr83173.c. * gcc.dg/plugin/location-overflow-test-pr83173-2.h: Header for pr83173.c. * gcc.dg/plugin/location_overflow_plugin.c: Use PLUGIN_PRAGMAS instead of PLUGIN_START_UNIT. * gcc.dg/plugin/plugin.exp: Enable new test. Added: trunk/gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173-1.h trunk/gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173-2.h trunk/gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173.c trunk/gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173.h Modified: trunk/gcc/testsuite/ChangeLog trunk/gcc/testsuite/gcc.dg/plugin/location_overflow_plugin.c trunk/gcc/testsuite/gcc.dg/plugin/plugin.exp
[Bug preprocessor/83173] C preprocessor generates incorrect linemarkers
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83173 --- Comment #12 from David Malcolm --- Author: dmalcolm Date: Tue Nov 27 15:49:43 2018 New Revision: 266516 URL: https://gcc.gnu.org/viewcvs?rev=266516=gcc=rev Log: PR preprocessor/83173: Additional check before decrementing highest_location 2018-11-27 Mike Gulick PR preprocessor/83173 * files.c (_cpp_stack_include): Check if line_table->highest_location is past current line before decrementing. Modified: trunk/libcpp/ChangeLog trunk/libcpp/files.c
[Bug preprocessor/83173] C preprocessor generates incorrect linemarkers
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83173 --- Comment #11 from Mike Gulick --- (In reply to Mike Gulick from comment #10) > In hopes of seeing some progress on this bug, I will rebase the patches on > the latest gcc master branch and re-test. I will also refactor the main > patch to separate out the functional fix from the diagnostics change, which > will hopefully make reviewing easier. I'll reply here with a link to the > gcc-patches archive once I have posted them. https://gcc.gnu.org/ml/gcc-patches/2018-11/msg00025.html
[Bug preprocessor/83173] C preprocessor generates incorrect linemarkers
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83173 --- Comment #10 from Mike Gulick --- In hopes of seeing some progress on this bug, I will rebase the patches on the latest gcc master branch and re-test. I will also refactor the main patch to separate out the functional fix from the diagnostics change, which will hopefully make reviewing easier. I'll reply here with a link to the gcc-patches archive once I have posted them.
[Bug preprocessor/83173] C preprocessor generates incorrect linemarkers
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83173 --- Comment #9 from Pádraig Brady --- Facebook
[Bug preprocessor/83173] C preprocessor generates incorrect linemarkers
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83173 --- Comment #8 from Eric Gallager --- (In reply to Pádraig Brady from comment #7) > Have been running with these patches on an extremely large code base for the > last few months, without issue Can you say which code base?
[Bug preprocessor/83173] C preprocessor generates incorrect linemarkers
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83173 --- Comment #7 from Pádraig Brady --- Have been running with these patches on an extremely large code base for the last few months, without issue
[Bug preprocessor/83173] C preprocessor generates incorrect linemarkers
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83173 --- Comment #6 from Mike Gulick --- (In reply to Pádraig Brady from comment #5) > Seeing this also with GCC 7.3. Will try proposed patches Latest version of patch and test: Patch: https://gcc.gnu.org/ml/gcc-patches/2018-02/msg00557.html Test: https://gcc.gnu.org/ml/gcc-patches/2018-01/msg01993.html
[Bug preprocessor/83173] C preprocessor generates incorrect linemarkers
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83173 Pádraig Brady changed: What|Removed |Added CC||P at draigBrady dot com --- Comment #5 from Pádraig Brady --- Seeing this also with GCC 7.3. Will try proposed patches
[Bug preprocessor/83173] C preprocessor generates incorrect linemarkers
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83173 David Malcolm changed: What|Removed |Added CC||dmalcolm at gcc dot gnu.org --- Comment #4 from David Malcolm --- (In reply to Jeffrey A. Law from comment #3) > Adding David who knows the linemap bits. > > David -- Mike has also posted a patch. See gcc-patches archives Dec 1. https://gcc.gnu.org/ml/gcc-patches/2017-12/msg00073.html
[Bug preprocessor/83173] C preprocessor generates incorrect linemarkers
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83173 Jeffrey A. Law changed: What|Removed |Added CC||dmalcolm at redhat dot com, ||law at redhat dot com --- Comment #3 from Jeffrey A. Law --- Adding David who knows the linemap bits. David -- Mike has also posted a patch. See gcc-patches archives Dec 1.
[Bug preprocessor/83173] C preprocessor generates incorrect linemarkers
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83173
--- Comment #2 from Mike Gulick ---
I have made some progress in determining the cause of this bug. This issue
occurs when the current source_location is > LINE_MAP_MAX_LOCATION_WITH_COLS
and when a #include is the last line in the file (with a terminating newline).
The corruption occurs when _cpp_stack_include decrements
ptable->line_table->highest_location. It does this so that highest_location
refers to the *current* line in the file, not the next line. For the case
where a #include is *not* the last line in the file, this works correctly.
However when the the source location is > LINE_MAP_MAX_LOCATION_WITH_COLS and
the current #include line being processed is the last line in the file, the
highest_location value already refers to the current line in the file, as there
is no next line. Thus this decrement sets highest_location to the previous
line in the file, which causes the corruption.
Consider an include file with two #includes:
#include "foo.h"
#include "bar.h"
EOF
Consider when do_include_common() processes the final '#include "bar.h"'. This
initially calls parse_include(). This calls check_eol(), which eventually
calls _cpp_lex_direct() via the following call stack:
0 _cpp_lex_direct
1 _cpp_lex_token
2 cpp_get_token_1
3 cpp_get_token
4 check_eol_1
5 check_eol
6 parse_include
7 do_include_common
8 do_include
9 _cpp_handle_directive
10 _cpp_lex_token
11 cpp_get_token_1
12 cpp_get_token_with_location
13 scan_translation_unit
14 preprocess_file
_cpp_lex_direct parses the current buffer one character at a time. In the case
of the line "#include bar.h", the buffer looks like:
#include "bar.h"\n\n
Note that the second '\n' is added to the buffer when the file is initially
read in. It doesn't exist in the file.
After parsing the '#include "bar.h", the buffer is sitting at the first '\n'.
#include "bar.h"\n\n
^ ^
buffer.cur---/ |
|
buffer.rlimit--/
buffer.rlimit is a pointer to the end of the buffer. It points to the final
newline that was added to the end of the buffer when the file was read.
_cpp_lex_direct() reads the buffer one character at a time, e.g.
c = *buffer->cur++
...
switch (c)
{
...
case '\n':
if (buffer->cur < buffer->rlimit)
CPP_INCREMENT_LINE (pfile, 0)
buffer->need_line = true;
goto fresh_line;
...
Under normal circumstances (i.e. if the #include is *not* the last line in the
file), when the '\n' is detected, CPP_INCREMENT_LINE increments the
line_maps->highest_line. However for this last #include, buffer->cur ==
buffer->rlimit, so CPP_INCREMENT_LINE is not called.
Thus if the #include token has source_location 1610612807, highest_location in
the line_maps structure also has 1610612807. Remember that we are past
LINE_MAP_MAX_LOCATION_WITH_COLS, so column numbers are not tracked, thus each
increment of a source_location value refers to a new line number and
potentially a new source file.
Continue stepping through do_include_common to _cpp_stack_include. This
function has the following comment:
/* Compensate for the increment in linemap_add that occurs if
_cpp_stack_file actually stacks the file. In the case of a
normal #include, we're currently at the start of the line
*following* the #include. A separate source_location for this
location makes no sense (until we do the LC_LEAVE), and
complicates LAST_SOURCE_LINE_LOCATION. This does not apply if we
found a PCH file (in which case linemap_add is not called) or we
were included from the command-line. */
Under normal circumstances, the comment stating "we're currently at the start
of the line *following* the include is correct. However in this case, this is
not true because we did not increment highest_line, thus highest_location still
refers to the current line. Thus when we decrement highest_line, this makes
highest_line actually refer to the *previous* line map location, not the
current. _cpp_stack_file then ultimately calls linemap_add, which sets
start_location to highest_location + 1. This is assumed to be a new, unused
location, but in this case it actually already refers to an existing line map.
Note that the linemap_assert in linemap_add will not catch this even if linemap
assertions are enabled. This is because it only asserts if the new
start_location is less than the source_location of last line in the line map,
however in this case it is equal to the source_location of the last line.
We fix this by no longer decrementing pfile->line_table->highest_location if it
is less than or equal to the source_location of the current include header.
The purpose of this decrement is to ensure that
pfile->line_table->highest_location still refers to the current line, so if it
already refers to the current line, there is no need to decrement it (and doing
so would be
[Bug preprocessor/83173] C preprocessor generates incorrect linemarkers
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83173 --- Comment #1 from Mike Gulick --- Just a minor update. I re-tested the reproducer on gcc 5.4 as well as 4.9.2, and the issue is present in both of those. I had originally thought the bug was not present in gcc 5.4 or earlier, however this is likely because the source code I was testing it on was not pushing the line map source location > 0x6000 in these versions of the compiler.
