* [RFC][PATCH] PR preprocessor/83173: Additional check before decrementing highest_location @ 2017-12-01 22:57 Mike Gulick 2018-01-12 23:33 ` David Malcolm 2018-11-01 15:58 ` [PATCH v3 0/3] PR preprocessor/83173: C preprocessor generates incorrect linemarkers Mike Gulick 0 siblings, 2 replies; 34+ messages in thread From: Mike Gulick @ 2017-12-01 22:57 UTC (permalink / raw) To: gcc-patches [-- Attachment #1: Type: text/plain, Size: 699 bytes --] I've come up with some patches that fix PR preprocessor/83173, which I reported a couple of weeks ago. The first patch is a test case. The second and third patches are two versions of the fix. The first version is simpler, but it may still leave in place some subtle incorrect behavior that happens when the current source location is less than LINE_MAP_MAX_COLUMN_NUMBER. The second version tries to handle that case as well, however I'm less comfortable with it as I don't know whether I'm computing the source_location of the *end* of the current line correctly in all cases. Both of these pass the gcc/g++ test suites with no regressions. Thanks in advance for the review/feedback! -Mike [-- Attachment #2: 0001-PR-preprocessor-83173-New-test.patch --] [-- Type: text/x-patch, Size: 5025 bytes --] From 6ff0068284c346c8db08c4b6b4d9a66d8464aeac Mon Sep 17 00:00:00 2001 From: Mike Gulick <mgulick@mathworks.com> Date: Thu, 30 Nov 2017 18:35:48 -0500 Subject: [PATCH 1/2] PR preprocessor/83173: New test 2017-12-01 Mike Gulick <mgulick@mathworks.com> PR preprocessor/83173 * gcc.dg/plugin/pr83173.c: New test. * gcc.dg/plugin/pr83173.h: Header for pr83173.c * gcc.dg/plugin/pr83173-1.h: Header for pr83173.c * gcc.dg/plugin/pr83173-2.h: Header for pr83173.c * gcc.dg/plugin/location_overflow_pp_plugin.c: New plugin to override line_table->highest_location for preprocessor. --- .../gcc.dg/plugin/location_overflow_pp_plugin.c | 44 ++++++++++++++++++++++ gcc/testsuite/gcc.dg/plugin/plugin.exp | 1 + gcc/testsuite/gcc.dg/plugin/pr83173-1.h | 2 + gcc/testsuite/gcc.dg/plugin/pr83173-2.h | 2 + gcc/testsuite/gcc.dg/plugin/pr83173.c | 21 +++++++++++ gcc/testsuite/gcc.dg/plugin/pr83173.h | 2 + 6 files changed, 72 insertions(+) create mode 100644 gcc/testsuite/gcc.dg/plugin/location_overflow_pp_plugin.c create mode 100644 gcc/testsuite/gcc.dg/plugin/pr83173-1.h create mode 100644 gcc/testsuite/gcc.dg/plugin/pr83173-2.h create mode 100644 gcc/testsuite/gcc.dg/plugin/pr83173.c create mode 100644 gcc/testsuite/gcc.dg/plugin/pr83173.h diff --git a/gcc/testsuite/gcc.dg/plugin/location_overflow_pp_plugin.c b/gcc/testsuite/gcc.dg/plugin/location_overflow_pp_plugin.c new file mode 100644 index 00000000000..ba5a795b937 --- /dev/null +++ b/gcc/testsuite/gcc.dg/plugin/location_overflow_pp_plugin.c @@ -0,0 +1,44 @@ +/* Plugin for testing how gracefully we degrade in the face of very + large source files. */ + +#include "config.h" +#include "gcc-plugin.h" +#include "system.h" +#include "coretypes.h" +#include "diagnostic.h" + +int plugin_is_GPL_compatible; + +static location_t base_location; + +/* Callback handler for the PLUGIN_PRAGMAS event. This is used to set the + initial line table offset for the preprocessor, to make it appear as if we + had parsed a very large file. PRAGMA_START_UNIT is not suitable here as is + not invoked during the preprocessor stage. */ + +static void +on_pragma_registration (void *gcc_data, void *user_data) +{ + line_table->highest_location = base_location; +} + +int +plugin_init (struct plugin_name_args *plugin_info, + struct plugin_gcc_version * /*version */ ) +{ + /* Read VALUE from -fplugin-arg-location_overflow_pp_plugin-value=<VALUE> + in hexadecimal form into base_location. */ + for (int i = 0; i < plugin_info->argc; i++) + { + if (0 == strcmp (plugin_info->argv[i].key, "value")) + base_location = strtol (plugin_info->argv[i].value, NULL, 16); + } + + if (!base_location) + error_at (UNKNOWN_LOCATION, "missing plugin argument"); + + register_callback (plugin_info->base_name, + PLUGIN_PRAGMAS, on_pragma_registration, NULL); + + return 0; +} diff --git a/gcc/testsuite/gcc.dg/plugin/plugin.exp b/gcc/testsuite/gcc.dg/plugin/plugin.exp index c7a3b4dbf2f..69d67caa846 100644 --- a/gcc/testsuite/gcc.dg/plugin/plugin.exp +++ b/gcc/testsuite/gcc.dg/plugin/plugin.exp @@ -79,6 +79,7 @@ set plugin_test_list [list \ { location_overflow_plugin.c \ location-overflow-test-1.c \ location-overflow-test-2.c } \ + { location_overflow_pp_plugin.c pr83173.c } \ { must_tail_call_plugin.c \ must-tail-call-1.c \ must-tail-call-2.c } \ diff --git a/gcc/testsuite/gcc.dg/plugin/pr83173-1.h b/gcc/testsuite/gcc.dg/plugin/pr83173-1.h new file mode 100644 index 00000000000..bf05d561976 --- /dev/null +++ b/gcc/testsuite/gcc.dg/plugin/pr83173-1.h @@ -0,0 +1,2 @@ +#pragma once +#define PR83173_1_H diff --git a/gcc/testsuite/gcc.dg/plugin/pr83173-2.h b/gcc/testsuite/gcc.dg/plugin/pr83173-2.h new file mode 100644 index 00000000000..dd0fc94bf53 --- /dev/null +++ b/gcc/testsuite/gcc.dg/plugin/pr83173-2.h @@ -0,0 +1,2 @@ +#pragma once +#define PR83173_2_H diff --git a/gcc/testsuite/gcc.dg/plugin/pr83173.c b/gcc/testsuite/gcc.dg/plugin/pr83173.c new file mode 100644 index 00000000000..ff1858a2b33 --- /dev/null +++ b/gcc/testsuite/gcc.dg/plugin/pr83173.c @@ -0,0 +1,21 @@ +/* + { dg-options "-fplugin-arg-location_overflow_pp_plugin-value=0x60000001" } + { dg-additional-files "pr83173.h" "pr83173-1.h" "pr83173-2.h" } + { dg-do preprocess } +*/ + +#include "pr83173.h" + +int +main () +{ + return 0; +} + +/* + The preprocessor output should contain '# 1 "path/to/pr83173-1.h" 1', but + should not contain any other references to pr83183-1.h. + + { dg-final { scan-file pr83173.i "# 1 \[^\r\n\]+pr83173-1\.h\" 1" } } + { dg-final { scan-file-not pr83173.i "# (?!1 \[^\r\n\]+pr83173-1\.h\" 1)\[0-9\]+ \[^\r\n\]+pr83173-1\.h\"" } } +*/ diff --git a/gcc/testsuite/gcc.dg/plugin/pr83173.h b/gcc/testsuite/gcc.dg/plugin/pr83173.h new file mode 100644 index 00000000000..a998a77a6b9 --- /dev/null +++ b/gcc/testsuite/gcc.dg/plugin/pr83173.h @@ -0,0 +1,2 @@ +#include "pr83173-1.h" +#include "pr83173-2.h" -- 2.11.0 [-- Attachment #3: 0002-PR-preprocessor-83173-Additional-check-before-decrem_v1.patch --] [-- Type: text/x-patch, Size: 1934 bytes --] From 96f8d7157994500f80e50039aed3fc4c77548c48 Mon Sep 17 00:00:00 2001 From: Mike Gulick <mgulick@mathworks.com> Date: Fri, 1 Dec 2017 09:43:22 -0500 Subject: [PATCH 2/2] PR preprocessor/83173: Additional check before decrementing highest_location 2017-12-01 Mike Gulick <mgulick@mathworks.com> PR preprocessor/83173 * files.c (_cpp_stack_include): Check if line_table->highest_location is past current line before decrementing. --- libcpp/files.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/libcpp/files.c b/libcpp/files.c index 969a531033f..7e2db25a407 100644 --- a/libcpp/files.c +++ b/libcpp/files.c @@ -1035,15 +1035,19 @@ _cpp_stack_include (cpp_reader *pfile, const char *fname, int angle_brackets, return false; /* Compensate for the increment in linemap_add that occurs if - _cpp_stack_file actually stacks the file. In the case of a + _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. */ + were included from the command-line. Additionally, if the + highest_location is not past the current source location, then we + are still at the current line, not the next line, so we should + not decrement highest_location. */ if (file->pchname == NULL && file->err_no == 0 - && type != IT_CMDLINE && type != IT_DEFAULT) + && type != IT_CMDLINE && type != IT_DEFAULT + && pfile->line_table->highest_location > loc) pfile->line_table->highest_location--; stacked = _cpp_stack_file (pfile, file, type == IT_IMPORT, loc); -- 2.11.0 [-- Attachment #4: 0002-PR-preprocessor-83173-Additional-check-before-decrem_v2.patch --] [-- Type: text/x-patch, Size: 2157 bytes --] From 1c3f4e067c8049b6bf4aedc47edb56db45d6edc8 Mon Sep 17 00:00:00 2001 From: Mike Gulick <mgulick@mathworks.com> Date: Fri, 1 Dec 2017 09:43:22 -0500 Subject: [PATCH 2/2] PR preprocessor/83173: Additional check before decrementing highest_location 2017-12-01 Mike Gulick <mgulick@mathworks.com> PR preprocessor/83173 * files.c (_cpp_stack_include): Check if line_table->highest_location is past current line before decrementing. --- libcpp/files.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/libcpp/files.c b/libcpp/files.c index 969a531033f..0972a9d8342 100644 --- a/libcpp/files.c +++ b/libcpp/files.c @@ -1035,16 +1035,26 @@ _cpp_stack_include (cpp_reader *pfile, const char *fname, int angle_brackets, return false; /* Compensate for the increment in linemap_add that occurs if - _cpp_stack_file actually stacks the file. In the case of a + _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. */ + were included from the command-line. Additionally, if the + highest_location is not past the current source location, then we + are still at the current line, not the next line, so we should + not decrement highest_location. */ if (file->pchname == NULL && file->err_no == 0 && type != IT_CMDLINE && type != IT_DEFAULT) - pfile->line_table->highest_location--; + { + line_map_ordinary *last_ord = + LINEMAPS_LAST_ORDINARY_MAP (pfile->line_table); + source_location last_map_end = + last_ord->start_location + (1 << last_ord->m_column_and_range_bits) - 1; + if (pfile->line_table->highest_location > last_map_end) + pfile->line_table->highest_location--; + } stacked = _cpp_stack_file (pfile, file, type == IT_IMPORT, loc); -- 2.11.0 ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [RFC][PATCH] PR preprocessor/83173: Additional check before decrementing highest_location 2017-12-01 22:57 [RFC][PATCH] PR preprocessor/83173: Additional check before decrementing highest_location Mike Gulick @ 2018-01-12 23:33 ` David Malcolm 2018-01-14 17:32 ` Mike Gulick 2018-11-01 15:58 ` [PATCH v3 0/3] PR preprocessor/83173: C preprocessor generates incorrect linemarkers Mike Gulick 1 sibling, 1 reply; 34+ messages in thread From: David Malcolm @ 2018-01-12 23:33 UTC (permalink / raw) To: Mike Gulick, gcc-patches On Fri, 2017-12-01 at 17:57 -0500, Mike Gulick wrote: > I've come up with some patches that fix PR preprocessor/83173, which > I reported > a couple of weeks ago. > > The first patch is a test case. The second and third patches are two > versions > of the fix. The first version is simpler, but it may still leave in > place some > subtle incorrect behavior that happens when the current source > location is less > than LINE_MAP_MAX_COLUMN_NUMBER. The second version tries to handle > that case > as well, however I'm less comfortable with it as I don't know whether > I'm > computing the source_location of the *end* of the current line > correctly in all > cases. Both of these pass the gcc/g++ test suites with no > regressions. > > Thanks in advance for the review/feedback! > > -Mike Hi Mike; sorry about the delay in reviewing this. Do you have the gcc contributor paperwork in place? > From 6ff0068284c346c8db08c4b6b4d9a66d8464aeac Mon Sep 17 00:00:00 2001 > From: Mike Gulick <mgulick@mathworks.com> > Date: Thu, 30 Nov 2017 18:35:48 -0500 > Subject: [PATCH 1/2] PR preprocessor/83173: New test > > 2017-12-01 Mike Gulick <mgulick@mathworks.com> > > PR preprocessor/83173 > * gcc.dg/plugin/pr83173.c: New test. > * gcc.dg/plugin/pr83173.h: Header for pr83173.c > * gcc.dg/plugin/pr83173-1.h: Header for pr83173.c > * gcc.dg/plugin/pr83173-2.h: Header for pr83173.c > * gcc.dg/plugin/location_overflow_pp_plugin.c: New plugin to > override line_table->highest_location for preprocessor. > --- > .../gcc.dg/plugin/location_overflow_pp_plugin.c | 44 ++++++++++++++++++++++ > gcc/testsuite/gcc.dg/plugin/plugin.exp | 1 + > gcc/testsuite/gcc.dg/plugin/pr83173-1.h | 2 + > gcc/testsuite/gcc.dg/plugin/pr83173-2.h | 2 + > gcc/testsuite/gcc.dg/plugin/pr83173.c | 21 +++++++++++ > gcc/testsuite/gcc.dg/plugin/pr83173.h | 2 + > 6 files changed, 72 insertions(+) > create mode 100644 gcc/testsuite/gcc.dg/plugin/location_overflow_pp_plugin.c > create mode 100644 gcc/testsuite/gcc.dg/plugin/pr83173-1.h > create mode 100644 gcc/testsuite/gcc.dg/plugin/pr83173-2.h > create mode 100644 gcc/testsuite/gcc.dg/plugin/pr83173.c > create mode 100644 gcc/testsuite/gcc.dg/plugin/pr83173.h > > diff --git a/gcc/testsuite/gcc.dg/plugin/location_overflow_pp_plugin.c b/gcc/testsuite/gcc.dg/plugin/location_overflow_pp_plugin.c > new file mode 100644 > index 00000000000..ba5a795b937 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/plugin/location_overflow_pp_plugin.c > @@ -0,0 +1,44 @@ > +/* Plugin for testing how gracefully we degrade in the face of very > + large source files. */ > + > +#include "config.h" > +#include "gcc-plugin.h" > +#include "system.h" > +#include "coretypes.h" > +#include "diagnostic.h" > + > +int plugin_is_GPL_compatible; > + > +static location_t base_location; > + > +/* Callback handler for the PLUGIN_PRAGMAS event. This is used to set the > + initial line table offset for the preprocessor, to make it appear as if we > + had parsed a very large file. PRAGMA_START_UNIT is not suitable here as is PLUGIN_START_UNIT, presumably? > + not invoked during the preprocessor stage. */ This new test plugin seems almost identical to the existing location_overflow_plugin.c. I tested changing the existing plugin to use PLUGIN_PRAGMAS rather than PLUGIN_START_UNIT, and it works fine for that event, so if that's the only difference, then maybe we don't need this new plugin? [...snip...] > diff --git a/gcc/testsuite/gcc.dg/plugin/pr83173.c b/gcc/testsuite/gcc.dg/plugin/pr83173.c > new file mode 100644 > index 00000000000..ff1858a2b33 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/plugin/pr83173.c > @@ -0,0 +1,21 @@ > +/* > + { dg-options "-fplugin-arg-location_overflow_pp_plugin-value=0x60000001" } This hardcodes a location_t overflow value. There are a pair of comments in libcpp/include/line-map.h that read: /* Do not pack ranges if locations get higher than this. If you change this, update: gcc.dg/plugin/location-overflow-test-*.c. */ I was going to suggest renaming your new test to e.g. location-overflow-test-pr83173.c so that it matches the glob in those comments, but given that you refer to the testname in dg-final directives, please update the line-map.h comments to refer to e.g. "all of testcases in gcc.dg/plugin that use location_overflow_plugin.c.", or somesuch wording. > +/* > + The preprocessor output should contain '# 1 "path/to/pr83173-1.h" 1', but > + should not contain any other references to pr83183-1.h. > + > + { dg-final { scan-file pr83173.i "# 1 \[^\r\n\]+pr83173-1\.h\" 1" } } > + { dg-final { scan-file-not pr83173.i "# (?!1 \[^\r\n\]+pr83173-1\.h\" 1)\[0-9\]+ \[^\r\n\]+pr83173-1\.h\"" } } [...snip...] If I'm reading your description in the PR right, this test case covers the loc > LINE_MAP_MAX_LOCATION_WITH_COLS case, but not the: loc <= LINE_MAP_MAX_LOCATION_WITH_COLS case. Can the latter be done by re-using the testcase, but with a different (or no) plugin argument? I'm still mulling over the two proposed fixes (it's been a while since I poked at the innards of the linemap impl); sorry. Dave ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [RFC][PATCH] PR preprocessor/83173: Additional check before decrementing highest_location 2018-01-12 23:33 ` David Malcolm @ 2018-01-14 17:32 ` Mike Gulick 2018-01-24 5:17 ` Mike Gulick 2018-02-09 22:55 ` [RFC][PATCH v2] " Mike Gulick 0 siblings, 2 replies; 34+ messages in thread From: Mike Gulick @ 2018-01-14 17:32 UTC (permalink / raw) To: David Malcolm, gcc-patches On 01/12/2018 06:16 PM, David Malcolm wrote: > On Fri, 2017-12-01 at 17:57 -0500, Mike Gulick wrote: >> I've come up with some patches that fix PR preprocessor/83173, which >> I reported >> a couple of weeks ago. >> >> The first patch is a test case. The second and third patches are two >> versions >> of the fix. The first version is simpler, but it may still leave in >> place some >> subtle incorrect behavior that happens when the current source >> location is less >> than LINE_MAP_MAX_COLUMN_NUMBER. The second version tries to handle >> that case >> as well, however I'm less comfortable with it as I don't know whether >> I'm >> computing the source_location of the *end* of the current line >> correctly in all >> cases. Both of these pass the gcc/g++ test suites with no >> regressions. >> >> Thanks in advance for the review/feedback! >> >> -Mike > > Hi Mike; sorry about the delay in reviewing this. > > Do you have the gcc contributor paperwork in place? Hi Dave. I don't have any gcc contributor paperwork in place, however I just finished that process for a gdb patch so I think it could be done pretty quickly. >> From 6ff0068284c346c8db08c4b6b4d9a66d8464aeac Mon Sep 17 00:00:00 2001 >> From: Mike Gulick <mgulick@mathworks.com> >> Date: Thu, 30 Nov 2017 18:35:48 -0500 >> Subject: [PATCH 1/2] PR preprocessor/83173: New test >> >> 2017-12-01 Mike Gulick <mgulick@mathworks.com> >> >> PR preprocessor/83173 >> * gcc.dg/plugin/pr83173.c: New test. >> * gcc.dg/plugin/pr83173.h: Header for pr83173.c >> * gcc.dg/plugin/pr83173-1.h: Header for pr83173.c >> * gcc.dg/plugin/pr83173-2.h: Header for pr83173.c >> * gcc.dg/plugin/location_overflow_pp_plugin.c: New plugin to >> override line_table->highest_location for preprocessor. >> --- >> .../gcc.dg/plugin/location_overflow_pp_plugin.c | 44 ++++++++++++++++++++++ >> gcc/testsuite/gcc.dg/plugin/plugin.exp | 1 + >> gcc/testsuite/gcc.dg/plugin/pr83173-1.h | 2 + >> gcc/testsuite/gcc.dg/plugin/pr83173-2.h | 2 + >> gcc/testsuite/gcc.dg/plugin/pr83173.c | 21 +++++++++++ >> gcc/testsuite/gcc.dg/plugin/pr83173.h | 2 + >> 6 files changed, 72 insertions(+) >> create mode 100644 gcc/testsuite/gcc.dg/plugin/location_overflow_pp_plugin.c >> create mode 100644 gcc/testsuite/gcc.dg/plugin/pr83173-1.h >> create mode 100644 gcc/testsuite/gcc.dg/plugin/pr83173-2.h >> create mode 100644 gcc/testsuite/gcc.dg/plugin/pr83173.c >> create mode 100644 gcc/testsuite/gcc.dg/plugin/pr83173.h >> >> diff --git a/gcc/testsuite/gcc.dg/plugin/location_overflow_pp_plugin.c b/gcc/testsuite/gcc.dg/plugin/location_overflow_pp_plugin.c >> new file mode 100644 >> index 00000000000..ba5a795b937 >> --- /dev/null >> +++ b/gcc/testsuite/gcc.dg/plugin/location_overflow_pp_plugin.c >> @@ -0,0 +1,44 @@ >> +/* Plugin for testing how gracefully we degrade in the face of very >> + large source files. */ >> + >> +#include "config.h" >> +#include "gcc-plugin.h" >> +#include "system.h" >> +#include "coretypes.h" >> +#include "diagnostic.h" >> + >> +int plugin_is_GPL_compatible; >> + >> +static location_t base_location; >> + >> +/* Callback handler for the PLUGIN_PRAGMAS event. This is used to set the >> + initial line table offset for the preprocessor, to make it appear as if we >> + had parsed a very large file. PRAGMA_START_UNIT is not suitable here as is > > PLUGIN_START_UNIT, presumably? > Sorry, yes that is right. >> + not invoked during the preprocessor stage. */ > > This new test plugin seems almost identical to the existing > location_overflow_plugin.c. Yes, I was originally hoping to use location_overflow_plugin.c until I realized that PLUGIN_START_UNIT event was not triggered during the preprocessor. Additionally, location_overflow_plugin.c has some restrictions that allow only a couple of specific initial offsets to be accepted, although I don't see any reason why that couldn't be changed. > > I tested changing the existing plugin to use PLUGIN_PRAGMAS rather than > PLUGIN_START_UNIT, and it works fine for that event, so if that's the > only difference, then maybe we don't need this new plugin? > I imagine it would work, although it does seem like PLUGIN_PRAGMAS is being used for something other than its intended purpose here, which I was trying to keep to a minimum. Its been a while since I wrote this patch, and I can't recall whether I noticed any other side-effects of using PLUGIN_PRAGMAS that could impact the existing location overflow tests. I'll take a look at it again and update the patch or report back any concerns. > [...snip...] > >> diff --git a/gcc/testsuite/gcc.dg/plugin/pr83173.c b/gcc/testsuite/gcc.dg/plugin/pr83173.c >> new file mode 100644 >> index 00000000000..ff1858a2b33 >> --- /dev/null >> +++ b/gcc/testsuite/gcc.dg/plugin/pr83173.c >> @@ -0,0 +1,21 @@ >> +/* >> + { dg-options "-fplugin-arg-location_overflow_pp_plugin-value=0x60000001" } > > This hardcodes a location_t overflow value. > There are a pair of comments in libcpp/include/line-map.h that read: > > /* Do not pack ranges if locations get higher than this. > If you change this, update: > gcc.dg/plugin/location-overflow-test-*.c. */ > > I was going to suggest renaming your new test to e.g. > location-overflow-test-pr83173.c > so that it matches the glob in those comments, but given that you refer > to the testname in dg-final directives, please update the line-map.h > comments to refer to e.g. "all of testcases in gcc.dg/plugin that use > location_overflow_plugin.c.", or somesuch wording. > If I'm going to modify location_overflow_plugin.c and reuse it for this PR, then it would make sense to rename the test and its accompanying helper files to your suggested naming as well. The dg-final regexes will likely continue to work. >> +/* >> + The preprocessor output should contain '# 1 "path/to/pr83173-1.h" 1', but >> + should not contain any other references to pr83183-1.h. >> + >> + { dg-final { scan-file pr83173.i "# 1 \[^\r\n\]+pr83173-1\.h\" 1" } } >> + { dg-final { scan-file-not pr83173.i "# (?!1 \[^\r\n\]+pr83173-1\.h\" 1)\[0-9\]+ \[^\r\n\]+pr83173-1\.h\"" } } > > [...snip...] > > If I'm reading your description in the PR right, this test case covers > the > loc > LINE_MAP_MAX_LOCATION_WITH_COLS > case, but not the: > loc <= LINE_MAP_MAX_LOCATION_WITH_COLS > case. > > Can the latter be done by re-using the testcase, but with a different > (or no) plugin argument? > I haven't really poked too hard to try to find if there is any corruption of the line map occurring when loc <= LINE_MAP_MAX_LOCATION_WITH_COLS. It is just a suspicion given the fact that this code is still decrementing line_table->highest_location in that case. I would imagine this may result in corruption of the column number or range rather than the file name and line number. > I'm still mulling over the two proposed fixes (it's been a while since > I poked at the innards of the linemap impl); sorry. > > Dave > Thanks. A number of developers at my company have been using gcc 6.3 internally with the first proposed patch backported to it, and haven't reported any issues. When I was testing the second version of the patch, I had also suspected some of the bounds in the assertions in linemap_add could be tightened, but I did see some assertion failures after doing that when running the test suite. This simply convinced me that I didn't really understand the nuances of the line map, so I really appreciate feedback from someone who is able to grok that code. Thanks, Mike ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [RFC][PATCH] PR preprocessor/83173: Additional check before decrementing highest_location 2018-01-14 17:32 ` Mike Gulick @ 2018-01-24 5:17 ` Mike Gulick 2018-02-09 22:55 ` [RFC][PATCH v2] " Mike Gulick 1 sibling, 0 replies; 34+ messages in thread From: Mike Gulick @ 2018-01-24 5:17 UTC (permalink / raw) To: David Malcolm, gcc-patches [-- Attachment #1: Type: text/plain, Size: 2818 bytes --] On 01/14/2018 12:16 PM, Mike Gulick wrote: > On 01/12/2018 06:16 PM, David Malcolm wrote: [snip] >> >> I was going to suggest renaming your new test to e.g. >> location-overflow-test-pr83173.c >> so that it matches the glob in those comments, but given that you refer >> to the testname in dg-final directives, please update the line-map.h >> comments to refer to e.g. "all of testcases in gcc.dg/plugin that use >> location_overflow_plugin.c.", or somesuch wording. >> > > If I'm going to modify location_overflow_plugin.c and reuse it for this PR, then > it would make sense to rename the test and its accompanying helper files to your > suggested naming as well. The dg-final regexes will likely continue to work. > I've attached an new version of the test patch that updates location_overflow_plugin.c to use PLUGIN_PRAGMAS, and updates the test filenames to be more consistent with the existing location-overflow-test-* tests. [snip] >> >> If I'm reading your description in the PR right, this test case covers >> the >> loc > LINE_MAP_MAX_LOCATION_WITH_COLS >> case, but not the: >> loc <= LINE_MAP_MAX_LOCATION_WITH_COLS >> case. >> >> Can the latter be done by re-using the testcase, but with a different >> (or no) plugin argument? >> > > I haven't really poked too hard to try to find if there is any corruption of the > line map occurring when loc <= LINE_MAP_MAX_LOCATION_WITH_COLS. It is just a > suspicion given the fact that this code is still decrementing > line_table->highest_location in that case. I would imagine this may result in > corruption of the column number or range rather than the file name and line > number. > I haven't been able to find any clear bugs in the gcc output when loc <= LINE_MAP_MAX_LOCATION_WITH_COLS, but I'm not quite sure how this behavior, if indeed incorrect, would manifest itself. The original bug is only visible (AFAIK) when running gcc with -E, as it results in incorrect file names and line numbers in the preprocessed output. If the file name and line number are correct in this output (as they would be when loc <= LINE_MAP_MAX_LOCATION_WITH_COLS), then everything will be fine when the .i file is read back in. I'm not sure if/how this bug can trigger any incorrect behavior when not using -E (or -no-integrated-cpp). Still, it does seem to me that even when loc <= LINE_MAP_MAX_LOCATION_WITH_COLS, _cpp_stack_include() is doing the wrong thing by decrementing pfile->line_table->highest_location when CPP_INCREMENT_LINE was not called from _cpp_lex_direct(). I will think about this a little more, and would value any insight you can offer. There is some more information about the details of what goes wrong in these functions in the original bug report. Thanks, Mike [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: v2-0001-PR-preprocessor-83173-New-test.patch --] [-- Type: text/x-patch; name=v2-0001-PR-preprocessor-83173-New-test.patch, Size: 5723 bytes --] From a0320fc6e4292a194a8680b26f4951a320fceaf2 Mon Sep 17 00:00:00 2001 From: Mike Gulick <mgulick@mathworks.com> Date: Thu, 30 Nov 2017 18:35:48 -0500 Subject: [PATCH v2] PR preprocessor/83173: New test 2017-12-01 Mike Gulick <mgulick@mathworks.com> 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. --- .../plugin/location-overflow-test-pr83173-1.h | 2 ++ .../plugin/location-overflow-test-pr83173-2.h | 2 ++ .../gcc.dg/plugin/location-overflow-test-pr83173.c | 21 +++++++++++++++++++++ .../gcc.dg/plugin/location-overflow-test-pr83173.h | 2 ++ .../gcc.dg/plugin/location_overflow_plugin.c | 13 ++++++++----- gcc/testsuite/gcc.dg/plugin/plugin.exp | 3 ++- 6 files changed, 37 insertions(+), 6 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173-1.h create mode 100644 gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173-2.h create mode 100644 gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173.c create mode 100644 gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173.h diff --git a/gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173-1.h b/gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173-1.h new file mode 100644 index 00000000000..f47dc3643c2 --- /dev/null +++ b/gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173-1.h @@ -0,0 +1,2 @@ +#pragma once +#define LOCATION_OVERFLOW_TEST_PR83173_1_H diff --git a/gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173-2.h b/gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173-2.h new file mode 100644 index 00000000000..bc23ed2a188 --- /dev/null +++ b/gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173-2.h @@ -0,0 +1,2 @@ +#pragma once +#define LOCATION_OVERFLOW_TEST_PR83173_2_H diff --git a/gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173.c b/gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173.c new file mode 100644 index 00000000000..2d581283474 --- /dev/null +++ b/gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173.c @@ -0,0 +1,21 @@ +/* + { dg-options "-fplugin-arg-location_overflow_plugin-value=0x60000001" } + { dg-do preprocess } +*/ + +#include "location-overflow-test-pr83173.h" + +int +main () +{ + return 0; +} + +/* + The preprocessor output should contain + '# 1 "path/to/location-overflow-test-pr83173-1.h" 1', but should not + contain any other references to location-overflow-test-pr83183-1.h. + + { dg-final { scan-file location-overflow-test-pr83173.i "# 1 \[^\r\n\]+location-overflow-test-pr83173-1\.h\" 1" } } + { dg-final { scan-file-not location-overflow-test-pr83173.i "# (?!1 \[^\r\n\]+location-overflow-test-pr83173-1\.h\" 1)\[0-9\]+ \[^\r\n\]+location-overflow-test-pr83173-1\.h\"" } } +*/ diff --git a/gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173.h b/gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173.h new file mode 100644 index 00000000000..49076f7ea3c --- /dev/null +++ b/gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173.h @@ -0,0 +1,2 @@ +#include "location-overflow-test-pr83173-1.h" +#include "location-overflow-test-pr83173-2.h" diff --git a/gcc/testsuite/gcc.dg/plugin/location_overflow_plugin.c b/gcc/testsuite/gcc.dg/plugin/location_overflow_plugin.c index 3644d9fd82e..317232bc19d 100644 --- a/gcc/testsuite/gcc.dg/plugin/location_overflow_plugin.c +++ b/gcc/testsuite/gcc.dg/plugin/location_overflow_plugin.c @@ -12,11 +12,14 @@ int plugin_is_GPL_compatible; static location_t base_location; -/* Callback handler for the PLUGIN_START_UNIT event; pretend - we parsed a very large include file. */ +/* Callback handler for the PLUGIN_PRAGMAS event; pretend we parsed a + very large include file. This is used to set the initial line table + offset for the preprocessor, to make it appear as if we had parsed a + very large file. PRAGMA_START_UNIT is not suitable here as is not + invoked during the preprocessor stage. */ static void -on_start_unit (void */*gcc_data*/, void */*user_data*/) +on_pragma_registration (void */*gcc_data*/, void */*user_data*/) { /* Act as if we've already parsed a large body of code; so that we can simulate various fallbacks in libcpp: @@ -79,8 +82,8 @@ plugin_init (struct plugin_name_args *plugin_info, error_at (UNKNOWN_LOCATION, "missing plugin argument"); register_callback (plugin_info->base_name, - PLUGIN_START_UNIT, - on_start_unit, + PLUGIN_PRAGMAS, + on_pragma_registration, NULL); /* void *user_data */ /* Hack in additional testing, based on the exact value supplied. */ diff --git a/gcc/testsuite/gcc.dg/plugin/plugin.exp b/gcc/testsuite/gcc.dg/plugin/plugin.exp index 5a19fc907e8..3c2a8e35e8a 100644 --- a/gcc/testsuite/gcc.dg/plugin/plugin.exp +++ b/gcc/testsuite/gcc.dg/plugin/plugin.exp @@ -90,7 +90,8 @@ set plugin_test_list [list \ diagnostic-test-inlining-4.c } \ { location_overflow_plugin.c \ location-overflow-test-1.c \ - location-overflow-test-2.c } \ + location-overflow-test-2.c \ + location-overflow-test-pr83173.c } \ { must_tail_call_plugin.c \ must-tail-call-1.c \ must-tail-call-2.c } \ -- 2.11.0 ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [RFC][PATCH v2] PR preprocessor/83173: Additional check before decrementing highest_location 2018-01-14 17:32 ` Mike Gulick 2018-01-24 5:17 ` Mike Gulick @ 2018-02-09 22:55 ` Mike Gulick 2018-03-04 19:27 ` Mike Gulick 1 sibling, 1 reply; 34+ messages in thread From: Mike Gulick @ 2018-02-09 22:55 UTC (permalink / raw) To: David Malcolm, gcc-patches [-- Attachment #1: Type: text/plain, Size: 4720 bytes --] Hi David, Here is a new version of the linemap patch (see my earlier emails for an updated version of the test code). I spent several days poring over the line map code, and I think I understand it a little better now. I also discovered -fdump-internal-locations, and it was a big help. If I use this switch with the code from the test I posted with an initial offset of 0x60000001, I get the following (I have removed some unnecessary fields for brevity): ORDINARY MAP: 21 source_location interval: 1610612805 <= loc < 1610612809 file: location-overflow-test-pr83173.c starting at line: 3 reason: 2 (LC_RENAME) location-overflow-test-pr83173.c: 3|loc:1610612805| { dg-do preprocess } location-overflow-test-pr83173.c: 4|loc:1610612806|*/ location-overflow-test-pr83173.c: 5|loc:1610612807| location-overflow-test-pr83173.c: 6|loc:1610612808|#include "location-overflow-test-pr83173.h" ORDINARY MAP: 22 source_location interval: 1610612809 <= loc < 1610612810 file: location-overflow-test-pr83173.h starting at line: 1 reason: 0 (LC_ENTER) location-overflow-test-pr83173.h: 1|loc:1610612809|#include "location-overflow-test-pr83173-1.h" ORDINARY MAP: 23 source_location interval: 1610612810 <= loc < 1610612812 file: location-overflow-test-pr83173-1.h starting at line: 1 reason: 0 (LC_ENTER) location-overflow-test-pr83173-1.h: 1|loc:1610612810|#pragma once location-overflow-test-pr83173-1.h: 2|loc:1610612811|#define LOCATION_OVERFLOW_TEST_PR83173_1_H ORDINARY MAP: 24 source_location interval: 1610612812 <= loc < 1610612812 file: location-overflow-test-pr83173.h starting at line: 2 reason: 1 (LC_LEAVE) ORDINARY MAP: 25 source_location interval: 1610612812 <= loc < 1610612814 file: location-overflow-test-pr83173-2.h starting at line: 1 reason: 0 (LC_ENTER) location-overflow-test-pr83173-2.h: 1|loc:1610612812|#pragma once location-overflow-test-pr83173-2.h: 2|loc:1610612813|#define LOCATION_OVERFLOW_TEST_PR83173_2_H ORDINARY MAP: 26 source_location interval: 1610612814 <= loc < 1610612815 file: location-overflow-test-pr83173.h starting at line: 2 reason: 1 (LC_LEAVE) location-overflow-test-pr83173.h: 2|loc:1610612814|#include "location-overflow-test-pr83173-2.h" ORDINARY MAP: 27 source_location interval: 1610612815 <= loc < 1610612823 file: location-overflow-test-pr83173.c starting at line: 7 reason: 1 (LC_LEAVE) ORDINARY MAP: 28 source_location interval: 1610612823 <= loc < 1610612825 file: location-overflow-test-pr83173.c starting at line: 15 reason: 2 (LC_RENAME) Notice that map 24 and 25 have the same start_location. This is because line_table->highest_location is being decremented when it should not be. This new patch uses a more robust method to check whether the source line that highest_location refers to is greater than loc (which is the source_location of the #include). Instead of comparing the source_location values directly, I use linemap_get_expansion_line to map the source location to a line number, and then compare the line numbers. This handles the case that loc is an adhoc location. I noticed that after updating the filenames in the test to be 'location-overflow-test-pr83173-1.h', the source locations passed into _cpp_stack_include were adhoc locations when I wasn't using the location offset plugin. This is because these filenames are now 34 characters long, which is longer than the 5 range bits that an ordinary source_location allows. I spent a lot of time comparing the -fdump-internal-locations output when this patch is in use to the unpatched version. The only differences occur when a #include is the last line in the file. For the case where the source location > LINE_MAP_MAX_SOURCE_LOCATION, the ordinary map 25 (see above) now starts at 1610612813 instead of 1610612812. For the case where the source location < LINE_MAP_MAX_LOCATION_WITH_COLS, the start location of that map has increased by 32 (which is effectively one column). For the case where the source location is between LINE_MAP_MAX_LOCATION_WITH_COLS and LINE_MAP_MAX_SOURCE_LOCATION (so that there are no range bits), the output was unchanged. I can post these location map dumps if you would like to see them. There are no regressions in the gcc test suite from this patch. The other addition in this version if the patch is a small improvement to the -fdump-internal-locations output to include the reason and included_from fields from the line_map_ordinary data structure. I aos fixed an issue with the output alignment. I found this useful when evaluating these changes. Thanks, Mike [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: v2-0001-PR-preprocessor-83173-Additional-check-before-dec.patch --] [-- Type: text/x-patch; name=v2-0001-PR-preprocessor-83173-Additional-check-before-dec.patch, Size: 17661 bytes --] From e3f23f66ad18598fc21a9223b7f29572a14d1588 Mon Sep 17 00:00:00 2001 From: Mike Gulick <mgulick@mathworks.com> Date: Fri, 1 Dec 2017 09:43:22 -0500 Subject: [PATCH v2] PR preprocessor/83173: Additional check before decrementing highest_location 2017-12-01 Mike Gulick <mgulick@mathworks.com> PR preprocessor/83173 * libcpp/files.c (_cpp_stack_include): Check if line_table->highest_location is past current line before decrementing. * gcc/input.c (dump_location_info): Dump reason and included_from fields from line_map_ordinary struct. Fix indentation when location > 5 digits. * libcpp/location-example.txt: Update example -fdump-internal-locations output. --- gcc/input.c | 43 +++++++- libcpp/files.c | 32 ++++-- libcpp/location-example.txt | 261 +++++++++++++++++++++++++------------------- 3 files changed, 214 insertions(+), 122 deletions(-) diff --git a/gcc/input.c b/gcc/input.c index 081e7856916..c7619703625 100644 --- a/gcc/input.c +++ b/gcc/input.c @@ -1077,6 +1077,17 @@ dump_labelled_location_range (FILE *stream, fprintf (stream, "\n"); } +#define NUM_DIGITS(x) ((x) >= 1000000000 ? 10 : \ + (x) >= 100000000 ? 9 : \ + (x) >= 10000000 ? 8 : \ + (x) >= 1000000 ? 7 : \ + (x) >= 100000 ? 6 : \ + (x) >= 10000 ? 5 : \ + (x) >= 1000 ? 4 : \ + (x) >= 100 ? 3 : \ + (x) >= 10 ? 2 : \ + 1) + /* Write a visualization of the locations in the line_table to STREAM. */ void @@ -1106,6 +1117,29 @@ dump_location_info (FILE *stream) map->m_column_and_range_bits - map->m_range_bits); fprintf (stream, " range bits: %i\n", map->m_range_bits); + const char * reason; + switch (map->reason) { + case LC_ENTER: + reason = "LC_ENTER"; + break; + case LC_LEAVE: + reason = "LC_LEAVE"; + break; + case LC_RENAME: + reason = "LC_RENAME"; + break; + case LC_RENAME_VERBATIM: + reason = "LC_RENAME_VERBATIM"; + break; + case LC_ENTER_MACRO: + reason = "LC_RENAME_MACRO"; + break; + default: + reason = "Unknown"; + } + fprintf (stream, " reason: %d (%s)\n", map->reason, reason); + fprintf (stream, " included from idx: %d\n", + ORDINARY_MAP_INCLUDER_FILE_INDEX (map)); /* Render the span of source lines that this "map" covers. */ for (source_location loc = MAP_START_LOCATION (map); @@ -1141,7 +1175,14 @@ dump_location_info (FILE *stream) if (max_col > line_size) max_col = line_size + 1; - int indent = 14 + strlen (exploc.file); + int len_lnum = NUM_DIGITS (exploc.line); + if (len_lnum < 3) + len_lnum = 3; + int len_loc = NUM_DIGITS (loc); + if (len_loc < 5) + len_loc = 5; + + int indent = 6 + strlen (exploc.file) + len_lnum + len_loc; /* Thousands. */ if (end_location > 999) diff --git a/libcpp/files.c b/libcpp/files.c index e8d21b28e62..543730b9267 100644 --- a/libcpp/files.c +++ b/libcpp/files.c @@ -1012,6 +1012,7 @@ _cpp_stack_include (cpp_reader *pfile, const char *fname, int angle_brackets, struct cpp_dir *dir; _cpp_file *file; bool stacked; + bool decremented = false; /* For -include command-line flags we have type == IT_CMDLINE. When the first -include file is processed we have the case, where @@ -1035,20 +1036,33 @@ _cpp_stack_include (cpp_reader *pfile, const char *fname, int angle_brackets, return false; /* 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. */ + _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. In the case that the #include is the last + line in the file, highest_location still points to the current + line, not the start of the next line, so we do not decrement in + this case. See plugin/location-overflow-test-pr83173.h for an + example. */ if (file->pchname == NULL && file->err_no == 0 && type != IT_CMDLINE && type != IT_DEFAULT) - pfile->line_table->highest_location--; + { + int highest_line = linemap_get_expansion_line (pfile->line_table, + pfile->line_table->highest_location); + int source_line = linemap_get_expansion_line (pfile->line_table, loc); + if (highest_line > source_line) + { + pfile->line_table->highest_location--; + decremented = true; + } + } stacked = _cpp_stack_file (pfile, file, type == IT_IMPORT, loc); - if (!stacked) + if (decremented && !stacked) /* _cpp_stack_file didn't stack the file, so let's rollback the compensation dance we performed above. */ pfile->line_table->highest_location++; diff --git a/libcpp/location-example.txt b/libcpp/location-example.txt index 14b5c2e284a..c6b8f7be881 100644 --- a/libcpp/location-example.txt +++ b/libcpp/location-example.txt @@ -33,8 +33,11 @@ ORDINARY MAP: 0 source_location interval: 32 <= loc < 64 file: test.c starting at line: 1 - column bits: 12 + column and range bits: 12 + column bits: 7 range bits: 5 + reason: 0 (LC_ENTER) + included from idx: -1 test.c: 1|loc: 32|#include "test.h" |69269258258148147 |46802468024680246 @@ -43,186 +46,220 @@ ORDINARY MAP: 1 source_location interval: 64 <= loc < 96 file: <built-in> starting at line: 0 + column and range bits: 0 column bits: 0 range bits: 0 + reason: 2 (LC_RENAME) + included from idx: -1 ORDINARY MAP: 2 source_location interval: 96 <= loc < 128 file: <command-line> starting at line: 0 + column and range bits: 0 column bits: 0 range bits: 0 + reason: 2 (LC_RENAME) + included from idx: -1 ORDINARY MAP: 3 - source_location interval: 128 <= loc < 160128 + source_location interval: 128 <= loc < 250240 file: /usr/include/stdc-predef.h starting at line: 1 - column bits: 12 + column and range bits: 12 + column bits: 7 range bits: 5 + reason: 0 (LC_ENTER) + included from idx: 2 (contents of /usr/include/stdc-predef.h snipped for brevity) ORDINARY MAP: 4 - source_location interval: 160128 <= loc < 160160 + source_location interval: 250240 <= loc < 250272 file: <command-line> starting at line: 32 - column bits: 12 + column and range bits: 12 + column bits: 7 range bits: 5 + reason: 1 (LC_LEAVE) + included from idx: -1 ORDINARY MAP: 5 - source_location interval: 160160 <= loc < 164256 + source_location interval: 250272 <= loc < 254368 file: test.c starting at line: 1 - column bits: 12 + column and range bits: 12 + column bits: 7 range bits: 5 -test.c: 1|loc:160160|#include "test.h" - |00000000000000000 - |12223334445556667 - |92582581481470470 - |24680246802468024 + reason: 2 (LC_RENAME) + included from idx: -1 +test.c: 1|loc:250272|#include "test.h" + |00000000000000000 + |33344445556667778 + |03603692692582581 + |46802468024680246 ORDINARY MAP: 6 - source_location interval: 164256 <= loc < 173280 + source_location interval: 254368 <= loc < 263392 file: test.h starting at line: 1 - column bits: 12 + column and range bits: 12 + column bits: 7 range bits: 5 -test.h: 1|loc:164256|extern int foo (); - |444444444444444444 - |233344455566677788 - |825814814704703603 - |802468024680246802 -test.h: 2|loc:168352| - | - | - | - | -test.h: 3|loc:172448|#define PLUS(A, B) A + B - |222222222222222223333333 - |455566677788889990001112 - |814704703603692692582581 - |024680246802468024680246 + reason: 0 (LC_ENTER) + included from idx: 5 +test.h: 1|loc:254368|extern int foo (); + |444444444444444444 + |444455566677788899 + |036926925825814814 + |024680246802468024 +test.h: 2|loc:258464| + | + | + | + | +test.h: 3|loc:262560|#define PLUS(A, B) A + B + |222222222222233333333333 + |566677788899900011122223 + |925825814814704703603692 + |246802468024680246802468 ORDINARY MAP: 7 - source_location interval: 173280 <= loc < 202016 + source_location interval: 263392 <= loc < 292128 file: test.c starting at line: 2 - column bits: 12 + column and range bits: 12 + column bits: 7 range bits: 5 -test.c: 2|loc:173280| - | - | - | - | -test.c: 3|loc:177376|int - |777 - |444 - |047 - |802 -test.c: 4|loc:181472|main (int argc, char **argv) - |1111111111111111222222222222 - |5556666777888999000111222333 - |0360369269258258148147047036 - |4680246802468024680246802468 -test.c: 5|loc:185568|{ - |5 - |6 - |0 - |0 -test.c: 6|loc:189664| int a = PLUS (1,2); - |999999999900000000000 - |677788899900011122233 - |926925825814814704703 - |680246802468024680246 -test.c: 7|loc:193760| int b = PLUS (3,4); - |333333344444444444444 - |788899900011122233344 - |925825814814704703603 - |246802468024680246802 -test.c: 8|loc:197856| return 0; - |77778888888 - |89990001112 - |82581481470 - |80246802468 -test.c: 9|loc:201952|} - |1 - |9 - |8 - |4 + reason: 1 (LC_LEAVE) + included from idx: -1 +test.c: 2|loc:263392| + | + | + | + | +test.c: 3|loc:267488|int + |777 + |555 + |258 + |024 +test.c: 4|loc:271584|main (int argc, char **argv) + |1111111111112222222222222222 + |6667778889990000111222333444 + |1481470470360369269258258148 + |6802468024680246802468024680 +test.c: 5|loc:275680|{ + |5 + |7 + |1 + |2 +test.c: 6|loc:279776| int a = PLUS (1,2); + |999999000000000000000 + |888999000011122233344 + |047036036926925825814 + |802468024680246802468 +test.c: 7|loc:283872| int b = PLUS (3,4); + |333444444444444444444 + |999000011122233344455 + |036036926925825814814 + |468024680246802468024 +test.c: 8|loc:287968| return 0; + |88888888888 + |00001112223 + |03692692582 + |02468024680 +test.c: 9|loc:292064|} + |2 + |0 + |9 + |6 UNALLOCATED LOCATIONS - source_location interval: 202016 <= loc < 2147483633 + source_location interval: 292128 <= loc < 2147483631 -MACRO 1: PLUS (7 tokens) - source_location interval: 2147483633 <= loc < 2147483640 -test.c:7:11: note: expansion point is location 194115 +MACRO 3: PLUS (7 tokens) + source_location interval: 2147483631 <= loc < 2147483638 +test.c:7:11: note: expansion point is location 284227 int b = PLUS (3,4); ^~~~ - - map->start_location: 2147483633 + map->start_location: 2147483631 macro_locations: - 0: 194304, 173088 -test.c:7:17: note: token 0 has x-location == 194304 + 0: 284416, 263200 +test.c:7:17: note: token 0 has x-location == 284416 int b = PLUS (3,4); ^ - -test.c:7:17: note: token 0 has y-location == 173088 - 1: 173152, 173152 -In file included from test.c:1:0: -test.h:3:22: note: token 1 has x-location == y-location == 173152 +test.c:7:17: note: token 0 has y-location == 263200 + 1: 263264, 263264 +In file included from test.c:1: +test.h:3:22: note: token 1 has x-location == y-location == 263264 #define PLUS(A, B) A + B ^ - - 2: 194368, 173216 -test.c:7:19: note: token 2 has x-location == 194368 + 2: 284480, 263328 +test.c:7:19: note: token 2 has x-location == 284480 int b = PLUS (3,4); ^ - -test.c:7:19: note: token 2 has y-location == 173216 +test.c:7:19: note: token 2 has y-location == 263328 3: 0, 2947526575 cc1: note: token 3 has x-location == 0 cc1: note: token 3 has y-location == 2947526575 4: 2947526575, 2947526575 -x-location == y-location == 2947526575 encodes token # 800042942 +x-location == y-location == 2947526575 encodes token # 800042944 5: 2947526575, 2947526575 -x-location == y-location == 2947526575 encodes token # 800042942 +x-location == y-location == 2947526575 encodes token # 800042944 6: 2947526575, 2947526575 -x-location == y-location == 2947526575 encodes token # 800042942 +x-location == y-location == 2947526575 encodes token # 800042944 -MACRO 0: PLUS (7 tokens) - source_location interval: 2147483640 <= loc < 2147483647 -test.c:6:11: note: expansion point is location 190019 +MACRO 2: PLUS (7 tokens) + source_location interval: 2147483638 <= loc < 2147483645 +test.c:6:11: note: expansion point is location 280131 int a = PLUS (1,2); ^~~~ - - map->start_location: 2147483640 + map->start_location: 2147483638 macro_locations: - 0: 190208, 173088 -test.c:6:17: note: token 0 has x-location == 190208 + 0: 280320, 263200 +test.c:6:17: note: token 0 has x-location == 280320 int a = PLUS (1,2); ^ - -test.c:6:17: note: token 0 has y-location == 173088 - 1: 173152, 173152 -In file included from test.c:1:0: -test.h:3:22: note: token 1 has x-location == y-location == 173152 +test.c:6:17: note: token 0 has y-location == 263200 + 1: 263264, 263264 +In file included from test.c:1: +test.h:3:22: note: token 1 has x-location == y-location == 263264 #define PLUS(A, B) A + B ^ - - 2: 190272, 173216 -test.c:6:19: note: token 2 has x-location == 190272 + 2: 280384, 263328 +test.c:6:19: note: token 2 has x-location == 280384 int a = PLUS (1,2); ^ - -test.c:6:19: note: token 2 has y-location == 173216 +test.c:6:19: note: token 2 has y-location == 263328 3: 0, 2947526575 cc1: note: token 3 has x-location == 0 cc1: note: token 3 has y-location == 2947526575 4: 2947526575, 2947526575 -x-location == y-location == 2947526575 encodes token # 800042935 +x-location == y-location == 2947526575 encodes token # 800042937 5: 2947526575, 2947526575 -x-location == y-location == 2947526575 encodes token # 800042935 +x-location == y-location == 2947526575 encodes token # 800042937 6: 2947526575, 2947526575 -x-location == y-location == 2947526575 encodes token # 800042935 +x-location == y-location == 2947526575 encodes token # 800042937 + +MACRO 1: __GCC_IEC_559_COMPLEX (1 tokens) + source_location interval: 2147483645 <= loc < 2147483646 +In file included from <command-line>:31: +/usr/include/stdc-predef.h:45:6: note: expansion point is location 180564 + # if __GCC_IEC_559_COMPLEX > 0 + ^~~~~~~~~~~~~~~~~~~~~ + map->start_location: 2147483645 + macro_locations: + 0: 1, 1 +<built-in>: note: token 0 has x-location == y-location == 1 + +MACRO 0: __GCC_IEC_559 (1 tokens) + source_location interval: 2147483646 <= loc < 2147483647 +/usr/include/stdc-predef.h:37:6: note: expansion point is location 147788 + # if __GCC_IEC_559 > 0 + ^~~~~~~~~~~~~ + map->start_location: 2147483646 + macro_locations: + 0: 1, 1 +<built-in>: note: token 0 has x-location == y-location == 1 MAX_SOURCE_LOCATION source_location interval: 2147483647 <= loc < 2147483648 -- 2.11.0 ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [RFC][PATCH v2] PR preprocessor/83173: Additional check before decrementing highest_location 2018-02-09 22:55 ` [RFC][PATCH v2] " Mike Gulick @ 2018-03-04 19:27 ` Mike Gulick 2018-05-29 15:31 ` Mike Gulick 0 siblings, 1 reply; 34+ messages in thread From: Mike Gulick @ 2018-03-04 19:27 UTC (permalink / raw) To: David Malcolm, gcc-patches On 02/09/2018 05:54 PM, Mike Gulick wrote: > Hi David, > > Here is a new version of the linemap patch (see my earlier emails for an updated > version of the test code). <snip> Hi David, Just wondering if you have had a chance to look at these updated patches yet. Let me know if you have any questions I can answer, or if there is anything you would like me to do that would make reviewing them easier (reposting, rebasing, refactoring the bug fix from the diagnostics change in the last patch). The most recent postings are: Bug fix patch: https://gcc.gnu.org/ml/gcc-patches/2018-02/msg00557.html Test case: https://gcc.gnu.org/ml/gcc-patches/2018-01/msg01993.html Thanks, Mike ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [RFC][PATCH v2] PR preprocessor/83173: Additional check before decrementing highest_location 2018-03-04 19:27 ` Mike Gulick @ 2018-05-29 15:31 ` Mike Gulick 2018-08-10 14:04 ` Mike Gulick 0 siblings, 1 reply; 34+ messages in thread From: Mike Gulick @ 2018-05-29 15:31 UTC (permalink / raw) To: David Malcolm, gcc-patches On 03/04/2018 02:27 PM, Mike Gulick wrote: > > > On 02/09/2018 05:54 PM, Mike Gulick wrote: >> Hi David, >> >> Here is a new version of the linemap patch (see my earlier emails for an updated >> version of the test code). > > <snip> > > Hi David, > > Just wondering if you have had a chance to look at these updated patches > yet. Let me know if you have any questions I can answer, or if there is > anything you would like me to do that would make reviewing them easier > (reposting, rebasing, refactoring the bug fix from the diagnostics > change in the last patch). > > The most recent postings are: > Bug fix patch: https://gcc.gnu.org/ml/gcc-patches/2018-02/msg00557.html > Test case: https://gcc.gnu.org/ml/gcc-patches/2018-01/msg01993.html > > Thanks, > Mike > Hi David, Now that gcc 8.1 is out the door, would you have any time to review these patches? I re-tested them after rebasing on the latest git master, and everything still behaved as expected. I can post the rebased patches if you would like, but it was a trivial merge with no conflicts. Thanks, Mike ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [RFC][PATCH v2] PR preprocessor/83173: Additional check before decrementing highest_location 2018-05-29 15:31 ` Mike Gulick @ 2018-08-10 14:04 ` Mike Gulick 0 siblings, 0 replies; 34+ messages in thread From: Mike Gulick @ 2018-08-10 14:04 UTC (permalink / raw) To: David Malcolm, gcc-patches On 05/29/2018 11:25 AM, Mike Gulick wrote: > On 03/04/2018 02:27 PM, Mike Gulick wrote: >> >> >> On 02/09/2018 05:54 PM, Mike Gulick wrote: >>> Hi David, >>> >>> Here is a new version of the linemap patch (see my earlier emails for an updated >>> version of the test code). >> >> <snip> >> >> Hi David, >> >> Just wondering if you have had a chance to look at these updated patches >> yet. Let me know if you have any questions I can answer, or if there is >> anything you would like me to do that would make reviewing them easier >> (reposting, rebasing, refactoring the bug fix from the diagnostics >> change in the last patch). >> >> The most recent postings are: >> Bug fix patch: https://gcc.gnu.org/ml/gcc-patches/2018-02/msg00557.html >> Test case: https://gcc.gnu.org/ml/gcc-patches/2018-01/msg01993.html >> >> Thanks, >> Mike >> > > Hi David, > > Now that gcc 8.1 is out the door, would you have any time to review these > patches? I re-tested them after rebasing on the latest git master, and > everything still behaved as expected. I can post the rebased patches if you > would like, but it was a trivial merge with no conflicts. > > Thanks, > Mike > I'd still like to get this bug fixed. I don't think I need the [RFC] tag any more on these patches. Should I post a new thread to this list to get the ball rolling again? ^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v3 0/3] PR preprocessor/83173: C preprocessor generates incorrect linemarkers 2017-12-01 22:57 [RFC][PATCH] PR preprocessor/83173: Additional check before decrementing highest_location Mike Gulick 2018-01-12 23:33 ` David Malcolm @ 2018-11-01 15:58 ` Mike Gulick 2018-11-01 15:58 ` [PATCH v3 3/3] PR preprocessor/83173: Enhance -fdump-internal-locations output Mike Gulick ` (2 more replies) 1 sibling, 3 replies; 34+ messages in thread From: Mike Gulick @ 2018-11-01 15:58 UTC (permalink / raw) To: gcc-patches; +Cc: David Malcolm, Mike Gulick Hi, Changes in v3: I haven't gotten a response in several months, so I'm reposting these patches. I split up the enhancement to the -fdump-internal-locations output from the actual bugfix. Hopefully this will make these easier to review. I rebased these patches on the gcc master and had to make one minor change to patch 3/3 to account for removal of the ORDINARY_MAP_INCLUDER_FILE_INDEX macro that was made back in August. We have been using this patch internally for the last 9 months without any issues, and the some other users have indicated the same on the bug report. Please let me know if you have any feedback, and how I should proceed. I do not have gcc contributor paperwork in place, as I believe that needs to be initiated by a maintainer, but I do have an employer disclaimer already approved, so the process should be simple. Thanks, Mike Mike Gulick (3): PR preprocessor/83173: Additional check before decrementing highest_location PR preprocessor/83173: New test PR preprocessor/83173: Enhance -fdump-internal-locations output gcc/input.c | 49 ++- .../plugin/location-overflow-test-pr83173-1.h | 2 + .../plugin/location-overflow-test-pr83173-2.h | 2 + .../plugin/location-overflow-test-pr83173.c | 21 ++ .../plugin/location-overflow-test-pr83173.h | 2 + .../gcc.dg/plugin/location_overflow_plugin.c | 13 +- gcc/testsuite/gcc.dg/plugin/plugin.exp | 3 +- libcpp/files.c | 32 +- libcpp/location-example.txt | 333 ++++++++++-------- 9 files changed, 301 insertions(+), 156 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173-1.h create mode 100644 gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173-2.h create mode 100644 gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173.c create mode 100644 gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173.h -- 2.19.1 ^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v3 3/3] PR preprocessor/83173: Enhance -fdump-internal-locations output 2018-11-01 15:58 ` [PATCH v3 0/3] PR preprocessor/83173: C preprocessor generates incorrect linemarkers Mike Gulick @ 2018-11-01 15:58 ` Mike Gulick 2018-11-02 21:04 ` David Malcolm 2018-11-01 15:58 ` [PATCH v3 1/3] PR preprocessor/83173: Additional check before decrementing highest_location Mike Gulick 2018-11-01 15:58 ` [PATCH v3 2/3] PR preprocessor/83173: New test Mike Gulick 2 siblings, 1 reply; 34+ messages in thread From: Mike Gulick @ 2018-11-01 15:58 UTC (permalink / raw) To: gcc-patches; +Cc: David Malcolm, Mike Gulick 2017-10-31 Mike Gulick <mgulick@mathworks.com> PR preprocessor/83173 * gcc/input.c (dump_location_info): Dump reason and included_from fields from line_map_ordinary struct. Fix indentation when location > 5 digits. * libcpp/location-example.txt: Update example -fdump-internal-locations output. --- gcc/input.c | 49 +++++- libcpp/location-example.txt | 333 +++++++++++++++++++++--------------- 2 files changed, 241 insertions(+), 141 deletions(-) diff --git a/gcc/input.c b/gcc/input.c index a94a010f353..f938a37f20e 100644 --- a/gcc/input.c +++ b/gcc/input.c @@ -1075,6 +1075,17 @@ dump_labelled_location_range (FILE *stream, fprintf (stream, "\n"); } +#define NUM_DIGITS(x) ((x) >= 1000000000 ? 10 : \ + (x) >= 100000000 ? 9 : \ + (x) >= 10000000 ? 8 : \ + (x) >= 1000000 ? 7 : \ + (x) >= 100000 ? 6 : \ + (x) >= 10000 ? 5 : \ + (x) >= 1000 ? 4 : \ + (x) >= 100 ? 3 : \ + (x) >= 10 ? 2 : \ + 1) + /* Write a visualization of the locations in the line_table to STREAM. */ void @@ -1104,6 +1115,35 @@ dump_location_info (FILE *stream) map->m_column_and_range_bits - map->m_range_bits); fprintf (stream, " range bits: %i\n", map->m_range_bits); + const char * reason; + switch (map->reason) { + case LC_ENTER: + reason = "LC_ENTER"; + break; + case LC_LEAVE: + reason = "LC_LEAVE"; + break; + case LC_RENAME: + reason = "LC_RENAME"; + break; + case LC_RENAME_VERBATIM: + reason = "LC_RENAME_VERBATIM"; + break; + case LC_ENTER_MACRO: + reason = "LC_RENAME_MACRO"; + break; + default: + reason = "Unknown"; + } + fprintf (stream, " reason: %d (%s)\n", map->reason, reason); + + const line_map_ordinary *includer_map + = linemap_included_from_linemap (line_table, map); + fprintf (stream, " included from map: %d\n", + includer_map ? int (includer_map - line_table->info_ordinary.maps) + : -1); + fprintf (stream, " included from location: %d\n", + linemap_included_from (map)); /* Render the span of source lines that this "map" covers. */ for (source_location loc = MAP_START_LOCATION (map); @@ -1137,7 +1177,14 @@ dump_location_info (FILE *stream) if (max_col > line_text.length ()) max_col = line_text.length () + 1; - int indent = 14 + strlen (exploc.file); + int len_lnum = NUM_DIGITS (exploc.line); + if (len_lnum < 3) + len_lnum = 3; + int len_loc = NUM_DIGITS (loc); + if (len_loc < 5) + len_loc = 5; + + int indent = 6 + strlen (exploc.file) + len_lnum + len_loc; /* Thousands. */ if (end_location > 999) diff --git a/libcpp/location-example.txt b/libcpp/location-example.txt index 14b5c2e284a..dc448b0493e 100644 --- a/libcpp/location-example.txt +++ b/libcpp/location-example.txt @@ -33,8 +33,12 @@ ORDINARY MAP: 0 source_location interval: 32 <= loc < 64 file: test.c starting at line: 1 - column bits: 12 + column and range bits: 12 + column bits: 7 range bits: 5 + reason: 0 (LC_ENTER) + included from map: -1 + included from location: 0 test.c: 1|loc: 32|#include "test.h" |69269258258148147 |46802468024680246 @@ -43,186 +47,235 @@ ORDINARY MAP: 1 source_location interval: 64 <= loc < 96 file: <built-in> starting at line: 0 + column and range bits: 0 column bits: 0 range bits: 0 + reason: 2 (LC_RENAME) + included from map: -1 + included from location: 0 ORDINARY MAP: 2 source_location interval: 96 <= loc < 128 file: <command-line> starting at line: 0 + column and range bits: 0 column bits: 0 range bits: 0 + reason: 2 (LC_RENAME) + included from map: -1 + included from location: 0 ORDINARY MAP: 3 - source_location interval: 128 <= loc < 160128 + source_location interval: 128 <= loc < 250240 file: /usr/include/stdc-predef.h starting at line: 1 - column bits: 12 + column and range bits: 12 + column bits: 7 range bits: 5 + reason: 0 (LC_ENTER) + included from map: 2 + included from location: 127 (contents of /usr/include/stdc-predef.h snipped for brevity) ORDINARY MAP: 4 - source_location interval: 160128 <= loc < 160160 + source_location interval: 250240 <= loc < 250272 file: <command-line> starting at line: 32 - column bits: 12 + column and range bits: 12 + column bits: 7 range bits: 5 + reason: 1 (LC_LEAVE) + included from map: -1 + included from location: 0 ORDINARY MAP: 5 - source_location interval: 160160 <= loc < 164256 + source_location interval: 250272 <= loc < 254368 file: test.c starting at line: 1 - column bits: 12 + column and range bits: 12 + column bits: 7 range bits: 5 -test.c: 1|loc:160160|#include "test.h" - |00000000000000000 - |12223334445556667 - |92582581481470470 - |24680246802468024 + reason: 2 (LC_RENAME) + included from map: -1 + included from location: 0 +test.c: 1|loc:250272|#include "test.h" + |00000000000000000 + |33344445556667778 + |03603692692582581 + |46802468024680246 ORDINARY MAP: 6 - source_location interval: 164256 <= loc < 173280 + source_location interval: 254368 <= loc < 266720 file: test.h starting at line: 1 - column bits: 12 + column and range bits: 12 + column bits: 7 range bits: 5 -test.h: 1|loc:164256|extern int foo (); - |444444444444444444 - |233344455566677788 - |825814814704703603 - |802468024680246802 -test.h: 2|loc:168352| - | - | - | - | -test.h: 3|loc:172448|#define PLUS(A, B) A + B - |222222222222222223333333 - |455566677788889990001112 - |814704703603692692582581 - |024680246802468024680246 + reason: 0 (LC_ENTER) + included from map: 5 + included from location: 250272 +test.h: 1|loc:254368|extern int foo (); + |444444444444444444 + |444455566677788899 + |036926925825814814 + |024680246802468024 +test.h: 2|loc:258464| + | + | + | + | +test.h: 3|loc:262560|#define PLUS(A, B) A + B + |222222222222233333333333 + |566677788899900011122223 + |925825814814704703603692 + |246802468024680246802468 +test.h: 4|loc:266656| + | + | + | + | ORDINARY MAP: 7 - source_location interval: 173280 <= loc < 202016 + source_location interval: 266720 <= loc < 299520 file: test.c starting at line: 2 - column bits: 12 + column and range bits: 12 + column bits: 7 range bits: 5 -test.c: 2|loc:173280| - | - | - | - | -test.c: 3|loc:177376|int - |777 - |444 - |047 - |802 -test.c: 4|loc:181472|main (int argc, char **argv) - |1111111111111111222222222222 - |5556666777888999000111222333 - |0360369269258258148147047036 - |4680246802468024680246802468 -test.c: 5|loc:185568|{ - |5 - |6 - |0 - |0 -test.c: 6|loc:189664| int a = PLUS (1,2); - |999999999900000000000 - |677788899900011122233 - |926925825814814704703 - |680246802468024680246 -test.c: 7|loc:193760| int b = PLUS (3,4); - |333333344444444444444 - |788899900011122233344 - |925825814814704703603 - |246802468024680246802 -test.c: 8|loc:197856| return 0; - |77778888888 - |89990001112 - |82581481470 - |80246802468 -test.c: 9|loc:201952|} - |1 - |9 - |8 - |4 + reason: 1 (LC_LEAVE) + included from map: -1 + included from location: 0 +test.c: 2|loc:266720| + | + | + | + | +test.c: 3|loc:270816|int + |000 + |889 + |481 + |802 +test.c: 4|loc:274912|main (int argc, char **argv) + |4455555555555555555555555555 + |9900011122223334445556667778 + |4704703603692692582581481470 + |4680246802468024680246802468 +test.c: 5|loc:279008|{ + |9 + |0 + |4 + |0 +test.c: 6|loc:283104| int a = PLUS (1,2); + |333333333333333333333 + |112222333444555666777 + |360369269258258148147 + |680246802468024680246 +test.c: 7|loc:287200| int b = PLUS (3,4); + |777777777777777777777 + |222333444555666777888 + |369269258258148147047 + |246802468024680246802 +test.c: 8|loc:291296| return 0; + |11111111111 + |33344455566 + |26925825814 + |80246802468 +test.c: 9|loc:295392|} + |5 + |4 + |2 + |4 +test.c: 10|loc:299488| + | + | + | + | UNALLOCATED LOCATIONS - source_location interval: 202016 <= loc < 2147483633 + source_location interval: 299520 <= loc < 2147483632 -MACRO 1: PLUS (7 tokens) - source_location interval: 2147483633 <= loc < 2147483640 -test.c:7:11: note: expansion point is location 194115 - int b = PLUS (3,4); - ^~~~ +MACRO 3: PLUS (7 tokens) + source_location interval: 2147483632 <= loc < 2147483639 +test.c:7:11: note: expansion point is location 287555 + 7 | int b = PLUS (3,4); + | ^~~~ + map->start_location: 2147483632 + macro_locations: + 0: 287744, 263200 +test.c:7:17: note: token 0 has x-location == 287744 + 7 | int b = PLUS (3,4); + | ^ +test.c:7:17: note: token 0 has y-location == 263200 + 1: 263264, 263264 +In file included from test.c:1: +test.h:3:22: note: token 1 has x-location == y-location == 263264 + 3 | #define PLUS(A, B) A + B + | ^ + 2: 287808, 263328 +test.c:7:19: note: token 2 has x-location == 287808 + 7 | int b = PLUS (3,4); + | ^ +test.c:7:19: note: token 2 has y-location == 263328 + 3: 0, 0 +cc1: note: token 3 has x-location == y-location == 0 + 4: 0, 0 +cc1: note: token 4 has x-location == y-location == 0 + 5: 0, 0 +cc1: note: token 5 has x-location == y-location == 0 + 6: 0, 0 +cc1: note: token 6 has x-location == y-location == 0 + +MACRO 2: PLUS (7 tokens) + source_location interval: 2147483639 <= loc < 2147483646 +test.c:6:11: note: expansion point is location 283459 + 6 | int a = PLUS (1,2); + | ^~~~ + map->start_location: 2147483639 + macro_locations: + 0: 283648, 263200 +test.c:6:17: note: token 0 has x-location == 283648 + 6 | int a = PLUS (1,2); + | ^ +test.c:6:17: note: token 0 has y-location == 263200 + 1: 263264, 263264 +In file included from test.c:1: +test.h:3:22: note: token 1 has x-location == y-location == 263264 + 3 | #define PLUS(A, B) A + B + | ^ + 2: 283712, 263328 +test.c:6:19: note: token 2 has x-location == 283712 + 6 | int a = PLUS (1,2); + | ^ +test.c:6:19: note: token 2 has y-location == 263328 + 3: 0, 0 +cc1: note: token 3 has x-location == y-location == 0 + 4: 0, 0 +cc1: note: token 4 has x-location == y-location == 0 + 5: 0, 0 +cc1: note: token 5 has x-location == y-location == 0 + 6: 0, 0 +cc1: note: token 6 has x-location == y-location == 0 - map->start_location: 2147483633 +MACRO 1: __GCC_IEC_559_COMPLEX (1 tokens) + source_location interval: 2147483646 <= loc < 2147483647 +In file included from <command-line>:31: +/usr/include/stdc-predef.h:45:6: note: expansion point is location 180564 + 45 | # if __GCC_IEC_559_COMPLEX > 0 + | ^~~~~~~~~~~~~~~~~~~~~ + map->start_location: 2147483646 macro_locations: - 0: 194304, 173088 -test.c:7:17: note: token 0 has x-location == 194304 - int b = PLUS (3,4); - ^ - -test.c:7:17: note: token 0 has y-location == 173088 - 1: 173152, 173152 -In file included from test.c:1:0: -test.h:3:22: note: token 1 has x-location == y-location == 173152 - #define PLUS(A, B) A + B - ^ - - 2: 194368, 173216 -test.c:7:19: note: token 2 has x-location == 194368 - int b = PLUS (3,4); - ^ - -test.c:7:19: note: token 2 has y-location == 173216 - 3: 0, 2947526575 -cc1: note: token 3 has x-location == 0 -cc1: note: token 3 has y-location == 2947526575 - 4: 2947526575, 2947526575 -x-location == y-location == 2947526575 encodes token # 800042942 - 5: 2947526575, 2947526575 -x-location == y-location == 2947526575 encodes token # 800042942 - 6: 2947526575, 2947526575 -x-location == y-location == 2947526575 encodes token # 800042942 - -MACRO 0: PLUS (7 tokens) - source_location interval: 2147483640 <= loc < 2147483647 -test.c:6:11: note: expansion point is location 190019 - int a = PLUS (1,2); - ^~~~ - - map->start_location: 2147483640 + 0: 1, 1 +<built-in>: note: token 0 has x-location == y-location == 1 + +MACRO 0: __GCC_IEC_559 (1 tokens) + source_location interval: 2147483647 <= loc < 2147483648 +/usr/include/stdc-predef.h:37:6: note: expansion point is location 147788 + 37 | # if __GCC_IEC_559 > 0 + | ^~~~~~~~~~~~~ + map->start_location: 2147483647 macro_locations: - 0: 190208, 173088 -test.c:6:17: note: token 0 has x-location == 190208 - int a = PLUS (1,2); - ^ - -test.c:6:17: note: token 0 has y-location == 173088 - 1: 173152, 173152 -In file included from test.c:1:0: -test.h:3:22: note: token 1 has x-location == y-location == 173152 - #define PLUS(A, B) A + B - ^ - - 2: 190272, 173216 -test.c:6:19: note: token 2 has x-location == 190272 - int a = PLUS (1,2); - ^ - -test.c:6:19: note: token 2 has y-location == 173216 - 3: 0, 2947526575 -cc1: note: token 3 has x-location == 0 -cc1: note: token 3 has y-location == 2947526575 - 4: 2947526575, 2947526575 -x-location == y-location == 2947526575 encodes token # 800042935 - 5: 2947526575, 2947526575 -x-location == y-location == 2947526575 encodes token # 800042935 - 6: 2947526575, 2947526575 -x-location == y-location == 2947526575 encodes token # 800042935 + 0: 1, 1 +<built-in>: note: token 0 has x-location == y-location == 1 MAX_SOURCE_LOCATION source_location interval: 2147483647 <= loc < 2147483648 -- 2.19.1 ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 3/3] PR preprocessor/83173: Enhance -fdump-internal-locations output 2018-11-01 15:58 ` [PATCH v3 3/3] PR preprocessor/83173: Enhance -fdump-internal-locations output Mike Gulick @ 2018-11-02 21:04 ` David Malcolm 2018-11-12 21:13 ` Mike Gulick 0 siblings, 1 reply; 34+ messages in thread From: David Malcolm @ 2018-11-02 21:04 UTC (permalink / raw) To: Mike Gulick, gcc-patches On Thu, 2018-11-01 at 11:56 -0400, Mike Gulick wrote: > 2017-10-31 Mike Gulick <mgulick@mathworks.com> > > PR preprocessor/83173 > * gcc/input.c (dump_location_info): Dump reason and > included_from fields from line_map_ordinary struct. Fix > indentation when location > 5 digits. > > * libcpp/location-example.txt: Update example > -fdump-internal-locations output. > --- > gcc/input.c | 49 +++++- > libcpp/location-example.txt | 333 +++++++++++++++++++++------------- > -- > 2 files changed, 241 insertions(+), 141 deletions(-) Sorry about the belated response. This is a nice enhancement; some nits below. > diff --git a/gcc/input.c b/gcc/input.c > index a94a010f353..f938a37f20e 100644 > --- a/gcc/input.c > +++ b/gcc/input.c > @@ -1075,6 +1075,17 @@ dump_labelled_location_range (FILE *stream, > fprintf (stream, "\n"); > } > > +#define NUM_DIGITS(x) ((x) >= 1000000000 ? 10 : \ > + (x) >= 100000000 ? 9 : \ > + (x) >= 10000000 ? 8 : \ > + (x) >= 1000000 ? 7 : \ > + (x) >= 100000 ? 6 : \ > + (x) >= 10000 ? 5 : \ > + (x) >= 1000 ? 4 : \ > + (x) >= 100 ? 3 : \ > + (x) >= 10 ? 2 : \ > + 1) diagnostic-show-locus.c has a function "num_digits" (currently static) and, fwiw, a unit test. It would be good to share the implementation. > /* Write a visualization of the locations in the line_table to > STREAM. */ > > void > @@ -1104,6 +1115,35 @@ dump_location_info (FILE *stream) > map->m_column_and_range_bits - map->m_range_bits); > fprintf (stream, " range bits: %i\n", > map->m_range_bits); > + const char * reason; > + switch (map->reason) { > + case LC_ENTER: > + reason = "LC_ENTER"; > + break; > + case LC_LEAVE: > + reason = "LC_LEAVE"; > + break; > + case LC_RENAME: > + reason = "LC_RENAME"; > + break; > + case LC_RENAME_VERBATIM: > + reason = "LC_RENAME_VERBATIM"; > + break; > + case LC_ENTER_MACRO: > + reason = "LC_RENAME_MACRO"; > + break; > + default: > + reason = "Unknown"; > + } > + fprintf (stream, " reason: %d (%s)\n", map->reason, reason); > + > + const line_map_ordinary *includer_map > + = linemap_included_from_linemap (line_table, map); > + fprintf (stream, " included from map: %d\n", > + includer_map ? int (includer_map - line_table- > >info_ordinary.maps) > + : -1); I'm not a fan of "-1" here; it's a NULL pointer in the original data. How about "n/a" for that case? > + fprintf (stream, " included from location: %d\n", > + linemap_included_from (map)); ...or merging it with this line, for something like: included from location: 127 (in ordinary map 2) vs: included from location: 0 [...snip...] Other than that, this is OK for trunk, assuming your contributor paperwork is in place. Dave ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 3/3] PR preprocessor/83173: Enhance -fdump-internal-locations output 2018-11-02 21:04 ` David Malcolm @ 2018-11-12 21:13 ` Mike Gulick 2018-11-13 0:56 ` David Malcolm 0 siblings, 1 reply; 34+ messages in thread From: Mike Gulick @ 2018-11-12 21:13 UTC (permalink / raw) To: David Malcolm, gcc-patches On 11/2/18 5:04 PM, David Malcolm wrote: > On Thu, 2018-11-01 at 11:56 -0400, Mike Gulick wrote: >> 2017-10-31 Mike Gulick <mgulick@mathworks.com> >> >> PR preprocessor/83173 >> * gcc/input.c (dump_location_info): Dump reason and >> included_from fields from line_map_ordinary struct. Fix >> indentation when location > 5 digits. >> >> * libcpp/location-example.txt: Update example >> -fdump-internal-locations output. >> --- >> gcc/input.c | 49 +++++- >> libcpp/location-example.txt | 333 +++++++++++++++++++++------------- >> -- >> 2 files changed, 241 insertions(+), 141 deletions(-) > > Sorry about the belated response. This is a nice enhancement; some > nits below. > >> diff --git a/gcc/input.c b/gcc/input.c >> index a94a010f353..f938a37f20e 100644 >> --- a/gcc/input.c >> +++ b/gcc/input.c >> @@ -1075,6 +1075,17 @@ dump_labelled_location_range (FILE *stream, >> fprintf (stream, "\n"); >> } >> >> +#define NUM_DIGITS(x) ((x) >= 1000000000 ? 10 : \ >> + (x) >= 100000000 ? 9 : \ >> + (x) >= 10000000 ? 8 : \ >> + (x) >= 1000000 ? 7 : \ >> + (x) >= 100000 ? 6 : \ >> + (x) >= 10000 ? 5 : \ >> + (x) >= 1000 ? 4 : \ >> + (x) >= 100 ? 3 : \ >> + (x) >= 10 ? 2 : \ >> + 1) > > diagnostic-show-locus.c has a function "num_digits" (currently static) > and, fwiw, a unit test. It would be good to share the implementation. > I initially tried to use this function by just adding "extern int num_digits(int);" into diagnostic-core.h, but that failed to link, so it seems like diagnostic-show-locus.c is not included in whatever library input.c gets linked with (I forget which library it was trying to link). Instead I moved num_digits and its unit test to diagnostic.c, and added the extern definition to diagnostic-core.h. That builds and tests successfully. Does that seem like a reasonable way to do this? >> /* Write a visualization of the locations in the line_table to >> STREAM. */ >> >> void >> @@ -1104,6 +1115,35 @@ dump_location_info (FILE *stream) >> map->m_column_and_range_bits - map->m_range_bits); >> fprintf (stream, " range bits: %i\n", >> map->m_range_bits); >> + const char * reason; >> + switch (map->reason) { >> + case LC_ENTER: >> + reason = "LC_ENTER"; >> + break; >> + case LC_LEAVE: >> + reason = "LC_LEAVE"; >> + break; >> + case LC_RENAME: >> + reason = "LC_RENAME"; >> + break; >> + case LC_RENAME_VERBATIM: >> + reason = "LC_RENAME_VERBATIM"; >> + break; >> + case LC_ENTER_MACRO: >> + reason = "LC_RENAME_MACRO"; >> + break; >> + default: >> + reason = "Unknown"; >> + } >> + fprintf (stream, " reason: %d (%s)\n", map->reason, reason); >> + >> + const line_map_ordinary *includer_map >> + = linemap_included_from_linemap (line_table, map); >> + fprintf (stream, " included from map: %d\n", >> + includer_map ? int (includer_map - line_table- >>> info_ordinary.maps) >> + : -1); > > I'm not a fan of "-1" here; it's a NULL pointer in the original data. > How about "n/a" for that case? > That's a good suggestion. Thanks. >> + fprintf (stream, " included from location: %d\n", >> + linemap_included_from (map)); > > ...or merging it with this line, for something like: > > included from location: 127 (in ordinary map 2) > > vs: > > included from location: 0 > > [...snip...] > > Other than that, this is OK for trunk, assuming your contributor > paperwork is in place. > > Dave > What is the preferred way to re-send this patch? Should I re-send the entire patch series as v4, or just an updated version of this single patch? Also, I'm waiting on FSF for assignment paperwork. I've re-pinged them after waiting a week. Thanks for the feedback and help. -Mike ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 3/3] PR preprocessor/83173: Enhance -fdump-internal-locations output 2018-11-12 21:13 ` Mike Gulick @ 2018-11-13 0:56 ` David Malcolm 2018-11-13 19:40 ` Mike Gulick 2018-11-13 19:55 ` [PATCH v4] " Mike Gulick 0 siblings, 2 replies; 34+ messages in thread From: David Malcolm @ 2018-11-13 0:56 UTC (permalink / raw) To: Mike Gulick, gcc-patches On Mon, 2018-11-12 at 21:13 +0000, Mike Gulick wrote: > On 11/2/18 5:04 PM, David Malcolm wrote: > > On Thu, 2018-11-01 at 11:56 -0400, Mike Gulick wrote: > > > 2017-10-31 Mike Gulick <mgulick@mathworks.com> > > > > > > PR preprocessor/83173 > > > * gcc/input.c (dump_location_info): Dump reason and > > > included_from fields from line_map_ordinary struct. Fix > > > indentation when location > 5 digits. > > > > > > * libcpp/location-example.txt: Update example > > > -fdump-internal-locations output. > > > --- > > > gcc/input.c | 49 +++++- > > > libcpp/location-example.txt | 333 +++++++++++++++++++++--------- > > > ---- > > > -- > > > 2 files changed, 241 insertions(+), 141 deletions(-) > > > > Sorry about the belated response. This is a nice enhancement; some > > nits below. > > > > > diff --git a/gcc/input.c b/gcc/input.c > > > index a94a010f353..f938a37f20e 100644 > > > --- a/gcc/input.c > > > +++ b/gcc/input.c > > > @@ -1075,6 +1075,17 @@ dump_labelled_location_range (FILE > > > *stream, > > > fprintf (stream, "\n"); > > > } > > > > > > +#define NUM_DIGITS(x) ((x) >= 1000000000 ? 10 : \ > > > + (x) >= 100000000 ? 9 : \ > > > + (x) >= 10000000 ? 8 : \ > > > + (x) >= 1000000 ? 7 : \ > > > + (x) >= 100000 ? 6 : \ > > > + (x) >= 10000 ? 5 : \ > > > + (x) >= 1000 ? 4 : \ > > > + (x) >= 100 ? 3 : \ > > > + (x) >= 10 ? 2 : \ > > > + 1) > > > > diagnostic-show-locus.c has a function "num_digits" (currently > > static) > > and, fwiw, a unit test. It would be good to share the > > implementation. > > > > I initially tried to use this function by just adding "extern int > num_digits(int);" into diagnostic-core.h, but that failed to link, so > it seems > like diagnostic-show-locus.c is not included in whatever library > input.c gets > linked with (I forget which library it was trying to link). Both input.o and diagnostic-show-locus.o are in OBJS-libcommon, so I'm not sure what went wrong. > Instead I moved > num_digits and its unit test to diagnostic.c, and added the extern > definition to > diagnostic-core.h. That builds and tests successfully. Does that > seem like a > reasonable way to do this? Thanks. That sounds good (maybe put the decl in diagnostic.h rather than diagnostic-core.h; the latter is used in lots of places, whereas the former is more about implementation details). > > > /* Write a visualization of the locations in the line_table to > > > STREAM. */ > > > > > > void > > > @@ -1104,6 +1115,35 @@ dump_location_info (FILE *stream) > > > map->m_column_and_range_bits - map- > > > >m_range_bits); > > > fprintf (stream, " range bits: %i\n", > > > map->m_range_bits); > > > + const char * reason; > > > + switch (map->reason) { > > > + case LC_ENTER: > > > + reason = "LC_ENTER"; > > > + break; > > > + case LC_LEAVE: > > > + reason = "LC_LEAVE"; > > > + break; > > > + case LC_RENAME: > > > + reason = "LC_RENAME"; > > > + break; > > > + case LC_RENAME_VERBATIM: > > > + reason = "LC_RENAME_VERBATIM"; > > > + break; > > > + case LC_ENTER_MACRO: > > > + reason = "LC_RENAME_MACRO"; > > > + break; > > > + default: > > > + reason = "Unknown"; > > > + } > > > + fprintf (stream, " reason: %d (%s)\n", map->reason, > > > reason); > > > + > > > + const line_map_ordinary *includer_map > > > + = linemap_included_from_linemap (line_table, map); > > > + fprintf (stream, " included from map: %d\n", > > > + includer_map ? int (includer_map - line_table- > > > > info_ordinary.maps) > > > > > > + : -1); > > > > I'm not a fan of "-1" here; it's a NULL pointer in the original > > data. > > How about "n/a" for that case? > > > > That's a good suggestion. Thanks. > > > > + fprintf (stream, " included from location: %d\n", > > > + linemap_included_from (map)); > > > > ...or merging it with this line, for something like: > > > > included from location: 127 (in ordinary map 2) > > > > vs: > > > > included from location: 0 > > > > [...snip...] > > > > Other than that, this is OK for trunk, assuming your contributor > > paperwork is in place. > > > > Dave > > > > What is the preferred way to re-send this patch? Should I re-send > the entire > patch series as v4, or just an updated version of this single patch? The latter: just an updated version of the changed patch. IIRC the rest is all approved. > > Also, I'm waiting on FSF for assignment paperwork. I've re-pinged > them after > waiting a week. Thanks. > Thanks for the feedback and help. > > -Mike ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 3/3] PR preprocessor/83173: Enhance -fdump-internal-locations output 2018-11-13 0:56 ` David Malcolm @ 2018-11-13 19:40 ` Mike Gulick 2018-11-13 19:55 ` [PATCH v4] " Mike Gulick 1 sibling, 0 replies; 34+ messages in thread From: Mike Gulick @ 2018-11-13 19:40 UTC (permalink / raw) To: David Malcolm, gcc-patches On 11/12/18 7:56 PM, David Malcolm wrote: > On Mon, 2018-11-12 at 21:13 +0000, Mike Gulick wrote: >> On 11/2/18 5:04 PM, David Malcolm wrote: >>> On Thu, 2018-11-01 at 11:56 -0400, Mike Gulick wrote: >>>> 2017-10-31 Mike Gulick <mgulick@mathworks.com> >>>> >>>> PR preprocessor/83173 >>>> * gcc/input.c (dump_location_info): Dump reason and >>>> included_from fields from line_map_ordinary struct. Fix >>>> indentation when location > 5 digits. >>>> >>>> * libcpp/location-example.txt: Update example >>>> -fdump-internal-locations output. >>>> --- >>>> gcc/input.c | 49 +++++- >>>> libcpp/location-example.txt | 333 +++++++++++++++++++++--------- >>>> ---- >>>> -- >>>> 2 files changed, 241 insertions(+), 141 deletions(-) >>> >>> Sorry about the belated response. This is a nice enhancement; some >>> nits below. >>> >>>> diff --git a/gcc/input.c b/gcc/input.c >>>> index a94a010f353..f938a37f20e 100644 >>>> --- a/gcc/input.c >>>> +++ b/gcc/input.c >>>> @@ -1075,6 +1075,17 @@ dump_labelled_location_range (FILE >>>> *stream, >>>> fprintf (stream, "\n"); >>>> } >>>> >>>> +#define NUM_DIGITS(x) ((x) >= 1000000000 ? 10 : \ >>>> + (x) >= 100000000 ? 9 : \ >>>> + (x) >= 10000000 ? 8 : \ >>>> + (x) >= 1000000 ? 7 : \ >>>> + (x) >= 100000 ? 6 : \ >>>> + (x) >= 10000 ? 5 : \ >>>> + (x) >= 1000 ? 4 : \ >>>> + (x) >= 100 ? 3 : \ >>>> + (x) >= 10 ? 2 : \ >>>> + 1) >>> >>> diagnostic-show-locus.c has a function "num_digits" (currently >>> static) >>> and, fwiw, a unit test. It would be good to share the >>> implementation. >>> >> >> I initially tried to use this function by just adding "extern int >> num_digits(int);" into diagnostic-core.h, but that failed to link, so >> it seems >> like diagnostic-show-locus.c is not included in whatever library >> input.c gets >> linked with (I forget which library it was trying to link). > > Both input.o and diagnostic-show-locus.o are in OBJS-libcommon, so I'm > not sure what went wrong. > After looking at libcommon.a with nm, I realized that diagnostic-show-locus.c wrapped everything inside an anonymous namespace, so that was why the symbol wasn't visible. >> Instead I moved >> num_digits and its unit test to diagnostic.c, and added the extern >> definition to >> diagnostic-core.h. That builds and tests successfully. Does that >> seem like a >> reasonable way to do this? > > Thanks. That sounds good (maybe put the decl in diagnostic.h rather > than diagnostic-core.h; the latter is used in lots of places, whereas > the former is more about implementation details). > No problem. >>>> /* Write a visualization of the locations in the line_table to >>>> STREAM. */ >>>> >>>> void >>>> @@ -1104,6 +1115,35 @@ dump_location_info (FILE *stream) >>>> map->m_column_and_range_bits - map- >>>>> m_range_bits); >>>> fprintf (stream, " range bits: %i\n", >>>> map->m_range_bits); >>>> + const char * reason; >>>> + switch (map->reason) { >>>> + case LC_ENTER: >>>> + reason = "LC_ENTER"; >>>> + break; >>>> + case LC_LEAVE: >>>> + reason = "LC_LEAVE"; >>>> + break; >>>> + case LC_RENAME: >>>> + reason = "LC_RENAME"; >>>> + break; >>>> + case LC_RENAME_VERBATIM: >>>> + reason = "LC_RENAME_VERBATIM"; >>>> + break; >>>> + case LC_ENTER_MACRO: >>>> + reason = "LC_RENAME_MACRO"; >>>> + break; >>>> + default: >>>> + reason = "Unknown"; >>>> + } >>>> + fprintf (stream, " reason: %d (%s)\n", map->reason, >>>> reason); >>>> + >>>> + const line_map_ordinary *includer_map >>>> + = linemap_included_from_linemap (line_table, map); >>>> + fprintf (stream, " included from map: %d\n", >>>> + includer_map ? int (includer_map - line_table- >>>>> info_ordinary.maps) >>>> >>>> + : -1); >>> >>> I'm not a fan of "-1" here; it's a NULL pointer in the original >>> data. >>> How about "n/a" for that case? >>> >> >> That's a good suggestion. Thanks. >> >>>> + fprintf (stream, " included from location: %d\n", >>>> + linemap_included_from (map)); >>> >>> ...or merging it with this line, for something like: >>> >>> included from location: 127 (in ordinary map 2) >>> >>> vs: >>> >>> included from location: 0 >>> >>> [...snip...] >>> >>> Other than that, this is OK for trunk, assuming your contributor >>> paperwork is in place. >>> >>> Dave >>> >> >> What is the preferred way to re-send this patch? Should I re-send >> the entire >> patch series as v4, or just an updated version of this single patch? > > The latter: just an updated version of the changed patch. IIRC the > rest is all approved. > Thanks, I'll reply with the updated patch. >> >> Also, I'm waiting on FSF for assignment paperwork. I've re-pinged >> them after >> waiting a week. > > Thanks. > >> Thanks for the feedback and help. >> >> -Mike > ^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v4] PR preprocessor/83173: Enhance -fdump-internal-locations output 2018-11-13 0:56 ` David Malcolm 2018-11-13 19:40 ` Mike Gulick @ 2018-11-13 19:55 ` Mike Gulick 2018-11-13 20:13 ` David Malcolm 1 sibling, 1 reply; 34+ messages in thread From: Mike Gulick @ 2018-11-13 19:55 UTC (permalink / raw) To: gcc-patches; +Cc: David Malcolm, Mike Gulick 2018-11-13 Mike Gulick <mgulick@mathworks.com> PR preprocessor/83173 * gcc/input.c (dump_location_info): Dump reason and included_from fields from line_map_ordinary struct. Fix indentation when location > 5 digits. * libcpp/location-example.txt: Update example -fdump-internal-locations output. * gcc/diagnostic-show-locus.c (num_digits, test_num_digits): Move to gcc/diagnostic.c to allow it to be utilized by gcc/input.c. * gcc/diagnostic.c (num_digits, test_num_digits): Moved here. (diagnostic_c_tests): Run test_num_digits. * gcc/diagnostic-core.h (num_digits): Add extern definition. --- gcc/diagnostic-show-locus.c | 51 ------ gcc/diagnostic.c | 46 +++++ gcc/diagnostic.h | 2 + gcc/input.c | 41 ++++- libcpp/location-example.txt | 333 +++++++++++++++++++++--------------- 5 files changed, 281 insertions(+), 192 deletions(-) diff --git a/gcc/diagnostic-show-locus.c b/gcc/diagnostic-show-locus.c index a42ff819512..08fe74a6136 100644 --- a/gcc/diagnostic-show-locus.c +++ b/gcc/diagnostic-show-locus.c @@ -819,56 +819,6 @@ fixit_cmp (const void *p_a, const void *p_b) return hint_a->get_start_loc () - hint_b->get_start_loc (); } -/* Get the number of digits in the decimal representation - of VALUE. */ - -static int -num_digits (int value) -{ - /* Perhaps simpler to use log10 for this, but doing it this way avoids - using floating point. */ - gcc_assert (value >= 0); - - if (value == 0) - return 1; - - int digits = 0; - while (value > 0) - { - digits++; - value /= 10; - } - return digits; -} - - -#if CHECKING_P - -/* Selftest for num_digits. */ - -static void -test_num_digits () -{ - ASSERT_EQ (1, num_digits (0)); - ASSERT_EQ (1, num_digits (9)); - ASSERT_EQ (2, num_digits (10)); - ASSERT_EQ (2, num_digits (99)); - ASSERT_EQ (3, num_digits (100)); - ASSERT_EQ (3, num_digits (999)); - ASSERT_EQ (4, num_digits (1000)); - ASSERT_EQ (4, num_digits (9999)); - ASSERT_EQ (5, num_digits (10000)); - ASSERT_EQ (5, num_digits (99999)); - ASSERT_EQ (6, num_digits (100000)); - ASSERT_EQ (6, num_digits (999999)); - ASSERT_EQ (7, num_digits (1000000)); - ASSERT_EQ (7, num_digits (9999999)); - ASSERT_EQ (8, num_digits (10000000)); - ASSERT_EQ (8, num_digits (99999999)); -} - -#endif /* #if CHECKING_P */ - /* Implementation of class layout. */ /* Constructor for class layout. @@ -3761,7 +3711,6 @@ void diagnostic_show_locus_c_tests () { test_line_span (); - test_num_digits (); test_layout_range_for_single_point (); test_layout_range_for_single_line (); diff --git a/gcc/diagnostic.c b/gcc/diagnostic.c index a572c084aac..08d40b87e2c 100644 --- a/gcc/diagnostic.c +++ b/gcc/diagnostic.c @@ -1024,6 +1024,27 @@ diagnostic_report_diagnostic (diagnostic_context *context, return true; } +/* Get the number of digits in the decimal representation of VALUE. */ + +int +num_digits (int value) +{ + /* Perhaps simpler to use log10 for this, but doing it this way avoids + using floating point. */ + gcc_assert (value >= 0); + + if (value == 0) + return 1; + + int digits = 0; + while (value > 0) + { + digits++; + value /= 10; + } + return digits; +} + /* Given a partial pathname as input, return another pathname that shares no directory elements with the pathname of __FILE__. This is used by fancy_abort() to print `Internal compiler error in expr.c' @@ -1774,6 +1795,29 @@ test_diagnostic_get_location_text () progname = old_progname; } +/* Selftest for num_digits. */ + +static void +test_num_digits () +{ + ASSERT_EQ (1, num_digits (0)); + ASSERT_EQ (1, num_digits (9)); + ASSERT_EQ (2, num_digits (10)); + ASSERT_EQ (2, num_digits (99)); + ASSERT_EQ (3, num_digits (100)); + ASSERT_EQ (3, num_digits (999)); + ASSERT_EQ (4, num_digits (1000)); + ASSERT_EQ (4, num_digits (9999)); + ASSERT_EQ (5, num_digits (10000)); + ASSERT_EQ (5, num_digits (99999)); + ASSERT_EQ (6, num_digits (100000)); + ASSERT_EQ (6, num_digits (999999)); + ASSERT_EQ (7, num_digits (1000000)); + ASSERT_EQ (7, num_digits (9999999)); + ASSERT_EQ (8, num_digits (10000000)); + ASSERT_EQ (8, num_digits (99999999)); +} + /* Run all of the selftests within this file. */ void @@ -1785,6 +1829,8 @@ diagnostic_c_tests () test_print_parseable_fixits_remove (); test_print_parseable_fixits_replace (); test_diagnostic_get_location_text (); + test_num_digits (); + } } // namespace selftest diff --git a/gcc/diagnostic.h b/gcc/diagnostic.h index 3498a9ba7bb..a48fe3f9a97 100644 --- a/gcc/diagnostic.h +++ b/gcc/diagnostic.h @@ -401,5 +401,7 @@ extern char *file_name_as_prefix (diagnostic_context *, const char *); extern char *build_message_string (const char *, ...) ATTRIBUTE_PRINTF_1; +/* Compute the number of digits in the decimal representation of an integer. */ +extern int num_digits (int); #endif /* ! GCC_DIAGNOSTIC_H */ diff --git a/gcc/input.c b/gcc/input.c index 9fb6e72421f..d63b5572db4 100644 --- a/gcc/input.c +++ b/gcc/input.c @@ -21,6 +21,7 @@ along with GCC; see the file COPYING3. If not see #include "system.h" #include "coretypes.h" #include "intl.h" +#include "diagnostic.h" #include "diagnostic-core.h" #include "selftest.h" #include "cpplib.h" @@ -1067,6 +1068,37 @@ dump_location_info (FILE *stream) map->m_column_and_range_bits - map->m_range_bits); fprintf (stream, " range bits: %i\n", map->m_range_bits); + const char * reason; + switch (map->reason) { + case LC_ENTER: + reason = "LC_ENTER"; + break; + case LC_LEAVE: + reason = "LC_LEAVE"; + break; + case LC_RENAME: + reason = "LC_RENAME"; + break; + case LC_RENAME_VERBATIM: + reason = "LC_RENAME_VERBATIM"; + break; + case LC_ENTER_MACRO: + reason = "LC_RENAME_MACRO"; + break; + default: + reason = "Unknown"; + } + fprintf (stream, " reason: %d (%s)\n", map->reason, reason); + + const line_map_ordinary *includer_map + = linemap_included_from_linemap (line_table, map); + fprintf (stream, " included from location: %d", + linemap_included_from (map)); + if (includer_map) { + fprintf (stream, " (in ordinary map %d)", + int (includer_map - line_table->info_ordinary.maps)); + } + fprintf (stream, "\n"); /* Render the span of source lines that this "map" covers. */ for (source_location loc = MAP_START_LOCATION (map); @@ -1100,7 +1132,14 @@ dump_location_info (FILE *stream) if (max_col > line_text.length ()) max_col = line_text.length () + 1; - int indent = 14 + strlen (exploc.file); + int len_lnum = num_digits (exploc.line); + if (len_lnum < 3) + len_lnum = 3; + int len_loc = num_digits (loc); + if (len_loc < 5) + len_loc = 5; + + int indent = 6 + strlen (exploc.file) + len_lnum + len_loc; /* Thousands. */ if (end_location > 999) diff --git a/libcpp/location-example.txt b/libcpp/location-example.txt index 14b5c2e284a..dc448b0493e 100644 --- a/libcpp/location-example.txt +++ b/libcpp/location-example.txt @@ -33,8 +33,12 @@ ORDINARY MAP: 0 source_location interval: 32 <= loc < 64 file: test.c starting at line: 1 - column bits: 12 + column and range bits: 12 + column bits: 7 range bits: 5 + reason: 0 (LC_ENTER) + included from map: -1 + included from location: 0 test.c: 1|loc: 32|#include "test.h" |69269258258148147 |46802468024680246 @@ -43,186 +47,235 @@ ORDINARY MAP: 1 source_location interval: 64 <= loc < 96 file: <built-in> starting at line: 0 + column and range bits: 0 column bits: 0 range bits: 0 + reason: 2 (LC_RENAME) + included from map: -1 + included from location: 0 ORDINARY MAP: 2 source_location interval: 96 <= loc < 128 file: <command-line> starting at line: 0 + column and range bits: 0 column bits: 0 range bits: 0 + reason: 2 (LC_RENAME) + included from map: -1 + included from location: 0 ORDINARY MAP: 3 - source_location interval: 128 <= loc < 160128 + source_location interval: 128 <= loc < 250240 file: /usr/include/stdc-predef.h starting at line: 1 - column bits: 12 + column and range bits: 12 + column bits: 7 range bits: 5 + reason: 0 (LC_ENTER) + included from map: 2 + included from location: 127 (contents of /usr/include/stdc-predef.h snipped for brevity) ORDINARY MAP: 4 - source_location interval: 160128 <= loc < 160160 + source_location interval: 250240 <= loc < 250272 file: <command-line> starting at line: 32 - column bits: 12 + column and range bits: 12 + column bits: 7 range bits: 5 + reason: 1 (LC_LEAVE) + included from map: -1 + included from location: 0 ORDINARY MAP: 5 - source_location interval: 160160 <= loc < 164256 + source_location interval: 250272 <= loc < 254368 file: test.c starting at line: 1 - column bits: 12 + column and range bits: 12 + column bits: 7 range bits: 5 -test.c: 1|loc:160160|#include "test.h" - |00000000000000000 - |12223334445556667 - |92582581481470470 - |24680246802468024 + reason: 2 (LC_RENAME) + included from map: -1 + included from location: 0 +test.c: 1|loc:250272|#include "test.h" + |00000000000000000 + |33344445556667778 + |03603692692582581 + |46802468024680246 ORDINARY MAP: 6 - source_location interval: 164256 <= loc < 173280 + source_location interval: 254368 <= loc < 266720 file: test.h starting at line: 1 - column bits: 12 + column and range bits: 12 + column bits: 7 range bits: 5 -test.h: 1|loc:164256|extern int foo (); - |444444444444444444 - |233344455566677788 - |825814814704703603 - |802468024680246802 -test.h: 2|loc:168352| - | - | - | - | -test.h: 3|loc:172448|#define PLUS(A, B) A + B - |222222222222222223333333 - |455566677788889990001112 - |814704703603692692582581 - |024680246802468024680246 + reason: 0 (LC_ENTER) + included from map: 5 + included from location: 250272 +test.h: 1|loc:254368|extern int foo (); + |444444444444444444 + |444455566677788899 + |036926925825814814 + |024680246802468024 +test.h: 2|loc:258464| + | + | + | + | +test.h: 3|loc:262560|#define PLUS(A, B) A + B + |222222222222233333333333 + |566677788899900011122223 + |925825814814704703603692 + |246802468024680246802468 +test.h: 4|loc:266656| + | + | + | + | ORDINARY MAP: 7 - source_location interval: 173280 <= loc < 202016 + source_location interval: 266720 <= loc < 299520 file: test.c starting at line: 2 - column bits: 12 + column and range bits: 12 + column bits: 7 range bits: 5 -test.c: 2|loc:173280| - | - | - | - | -test.c: 3|loc:177376|int - |777 - |444 - |047 - |802 -test.c: 4|loc:181472|main (int argc, char **argv) - |1111111111111111222222222222 - |5556666777888999000111222333 - |0360369269258258148147047036 - |4680246802468024680246802468 -test.c: 5|loc:185568|{ - |5 - |6 - |0 - |0 -test.c: 6|loc:189664| int a = PLUS (1,2); - |999999999900000000000 - |677788899900011122233 - |926925825814814704703 - |680246802468024680246 -test.c: 7|loc:193760| int b = PLUS (3,4); - |333333344444444444444 - |788899900011122233344 - |925825814814704703603 - |246802468024680246802 -test.c: 8|loc:197856| return 0; - |77778888888 - |89990001112 - |82581481470 - |80246802468 -test.c: 9|loc:201952|} - |1 - |9 - |8 - |4 + reason: 1 (LC_LEAVE) + included from map: -1 + included from location: 0 +test.c: 2|loc:266720| + | + | + | + | +test.c: 3|loc:270816|int + |000 + |889 + |481 + |802 +test.c: 4|loc:274912|main (int argc, char **argv) + |4455555555555555555555555555 + |9900011122223334445556667778 + |4704703603692692582581481470 + |4680246802468024680246802468 +test.c: 5|loc:279008|{ + |9 + |0 + |4 + |0 +test.c: 6|loc:283104| int a = PLUS (1,2); + |333333333333333333333 + |112222333444555666777 + |360369269258258148147 + |680246802468024680246 +test.c: 7|loc:287200| int b = PLUS (3,4); + |777777777777777777777 + |222333444555666777888 + |369269258258148147047 + |246802468024680246802 +test.c: 8|loc:291296| return 0; + |11111111111 + |33344455566 + |26925825814 + |80246802468 +test.c: 9|loc:295392|} + |5 + |4 + |2 + |4 +test.c: 10|loc:299488| + | + | + | + | UNALLOCATED LOCATIONS - source_location interval: 202016 <= loc < 2147483633 + source_location interval: 299520 <= loc < 2147483632 -MACRO 1: PLUS (7 tokens) - source_location interval: 2147483633 <= loc < 2147483640 -test.c:7:11: note: expansion point is location 194115 - int b = PLUS (3,4); - ^~~~ +MACRO 3: PLUS (7 tokens) + source_location interval: 2147483632 <= loc < 2147483639 +test.c:7:11: note: expansion point is location 287555 + 7 | int b = PLUS (3,4); + | ^~~~ + map->start_location: 2147483632 + macro_locations: + 0: 287744, 263200 +test.c:7:17: note: token 0 has x-location == 287744 + 7 | int b = PLUS (3,4); + | ^ +test.c:7:17: note: token 0 has y-location == 263200 + 1: 263264, 263264 +In file included from test.c:1: +test.h:3:22: note: token 1 has x-location == y-location == 263264 + 3 | #define PLUS(A, B) A + B + | ^ + 2: 287808, 263328 +test.c:7:19: note: token 2 has x-location == 287808 + 7 | int b = PLUS (3,4); + | ^ +test.c:7:19: note: token 2 has y-location == 263328 + 3: 0, 0 +cc1: note: token 3 has x-location == y-location == 0 + 4: 0, 0 +cc1: note: token 4 has x-location == y-location == 0 + 5: 0, 0 +cc1: note: token 5 has x-location == y-location == 0 + 6: 0, 0 +cc1: note: token 6 has x-location == y-location == 0 + +MACRO 2: PLUS (7 tokens) + source_location interval: 2147483639 <= loc < 2147483646 +test.c:6:11: note: expansion point is location 283459 + 6 | int a = PLUS (1,2); + | ^~~~ + map->start_location: 2147483639 + macro_locations: + 0: 283648, 263200 +test.c:6:17: note: token 0 has x-location == 283648 + 6 | int a = PLUS (1,2); + | ^ +test.c:6:17: note: token 0 has y-location == 263200 + 1: 263264, 263264 +In file included from test.c:1: +test.h:3:22: note: token 1 has x-location == y-location == 263264 + 3 | #define PLUS(A, B) A + B + | ^ + 2: 283712, 263328 +test.c:6:19: note: token 2 has x-location == 283712 + 6 | int a = PLUS (1,2); + | ^ +test.c:6:19: note: token 2 has y-location == 263328 + 3: 0, 0 +cc1: note: token 3 has x-location == y-location == 0 + 4: 0, 0 +cc1: note: token 4 has x-location == y-location == 0 + 5: 0, 0 +cc1: note: token 5 has x-location == y-location == 0 + 6: 0, 0 +cc1: note: token 6 has x-location == y-location == 0 - map->start_location: 2147483633 +MACRO 1: __GCC_IEC_559_COMPLEX (1 tokens) + source_location interval: 2147483646 <= loc < 2147483647 +In file included from <command-line>:31: +/usr/include/stdc-predef.h:45:6: note: expansion point is location 180564 + 45 | # if __GCC_IEC_559_COMPLEX > 0 + | ^~~~~~~~~~~~~~~~~~~~~ + map->start_location: 2147483646 macro_locations: - 0: 194304, 173088 -test.c:7:17: note: token 0 has x-location == 194304 - int b = PLUS (3,4); - ^ - -test.c:7:17: note: token 0 has y-location == 173088 - 1: 173152, 173152 -In file included from test.c:1:0: -test.h:3:22: note: token 1 has x-location == y-location == 173152 - #define PLUS(A, B) A + B - ^ - - 2: 194368, 173216 -test.c:7:19: note: token 2 has x-location == 194368 - int b = PLUS (3,4); - ^ - -test.c:7:19: note: token 2 has y-location == 173216 - 3: 0, 2947526575 -cc1: note: token 3 has x-location == 0 -cc1: note: token 3 has y-location == 2947526575 - 4: 2947526575, 2947526575 -x-location == y-location == 2947526575 encodes token # 800042942 - 5: 2947526575, 2947526575 -x-location == y-location == 2947526575 encodes token # 800042942 - 6: 2947526575, 2947526575 -x-location == y-location == 2947526575 encodes token # 800042942 - -MACRO 0: PLUS (7 tokens) - source_location interval: 2147483640 <= loc < 2147483647 -test.c:6:11: note: expansion point is location 190019 - int a = PLUS (1,2); - ^~~~ - - map->start_location: 2147483640 + 0: 1, 1 +<built-in>: note: token 0 has x-location == y-location == 1 + +MACRO 0: __GCC_IEC_559 (1 tokens) + source_location interval: 2147483647 <= loc < 2147483648 +/usr/include/stdc-predef.h:37:6: note: expansion point is location 147788 + 37 | # if __GCC_IEC_559 > 0 + | ^~~~~~~~~~~~~ + map->start_location: 2147483647 macro_locations: - 0: 190208, 173088 -test.c:6:17: note: token 0 has x-location == 190208 - int a = PLUS (1,2); - ^ - -test.c:6:17: note: token 0 has y-location == 173088 - 1: 173152, 173152 -In file included from test.c:1:0: -test.h:3:22: note: token 1 has x-location == y-location == 173152 - #define PLUS(A, B) A + B - ^ - - 2: 190272, 173216 -test.c:6:19: note: token 2 has x-location == 190272 - int a = PLUS (1,2); - ^ - -test.c:6:19: note: token 2 has y-location == 173216 - 3: 0, 2947526575 -cc1: note: token 3 has x-location == 0 -cc1: note: token 3 has y-location == 2947526575 - 4: 2947526575, 2947526575 -x-location == y-location == 2947526575 encodes token # 800042935 - 5: 2947526575, 2947526575 -x-location == y-location == 2947526575 encodes token # 800042935 - 6: 2947526575, 2947526575 -x-location == y-location == 2947526575 encodes token # 800042935 + 0: 1, 1 +<built-in>: note: token 0 has x-location == y-location == 1 MAX_SOURCE_LOCATION source_location interval: 2147483647 <= loc < 2147483648 -- 2.19.1 ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v4] PR preprocessor/83173: Enhance -fdump-internal-locations output 2018-11-13 19:55 ` [PATCH v4] " Mike Gulick @ 2018-11-13 20:13 ` David Malcolm 2018-11-26 22:17 ` Mike Gulick 2018-11-26 22:22 ` [PATCH v5] PR preprocessor/83173: Enhance -fdump-internal-locations output Mike Gulick 0 siblings, 2 replies; 34+ messages in thread From: David Malcolm @ 2018-11-13 20:13 UTC (permalink / raw) To: Mike Gulick, gcc-patches On Tue, 2018-11-13 at 14:54 -0500, Mike Gulick wrote: > 2018-11-13 Mike Gulick <mgulick@mathworks.com> [...] > * gcc/diagnostic-core.h (num_digits): Add extern definition. FWIW you moved the decl to diagnostic.h, but didn't update the above ChangeLog entry. [...] > diff --git a/libcpp/location-example.txt b/libcpp/location- > example.txt > index 14b5c2e284a..dc448b0493e 100644 > --- a/libcpp/location-example.txt > +++ b/libcpp/location-example.txt You're going to need to regenerate this file again; I touched many of the same lines as your patch does, in r266085 (sorry). Other than the nits above, this looks good to me (once you have your contribution paperwork in place). Thanks Dave ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v4] PR preprocessor/83173: Enhance -fdump-internal-locations output 2018-11-13 20:13 ` David Malcolm @ 2018-11-26 22:17 ` Mike Gulick 2018-11-27 1:29 ` David Malcolm 2018-11-26 22:22 ` [PATCH v5] PR preprocessor/83173: Enhance -fdump-internal-locations output Mike Gulick 1 sibling, 1 reply; 34+ messages in thread From: Mike Gulick @ 2018-11-26 22:17 UTC (permalink / raw) To: David Malcolm, gcc-patches On 11/13/18 3:12 PM, David Malcolm wrote: > On Tue, 2018-11-13 at 14:54 -0500, Mike Gulick wrote: >> 2018-11-13 Mike Gulick <mgulick@mathworks.com> > > [...] > >> * gcc/diagnostic-core.h (num_digits): Add extern definition. > > FWIW you moved the decl to diagnostic.h, but didn't update the above > ChangeLog entry. > > [...] > >> diff --git a/libcpp/location-example.txt b/libcpp/location- >> example.txt >> index 14b5c2e284a..dc448b0493e 100644 >> --- a/libcpp/location-example.txt >> +++ b/libcpp/location-example.txt > > You're going to need to regenerate this file again; I touched many of > the same lines as your patch does, in r266085 (sorry). > Thanks. I updated this file and fixed the changelog. I will send an updated patch after this email. The contents of this file were a little stale, so many of the locations in the file have changed in addition to the fields I added. I verified that the changed locations aren't due to any of these patches. > Other than the nits above, this looks good to me (once you have your > contribution paperwork in place). The contribution paperwork is now in place. There are no other changes to the previous patches (other than updating the changelog date). Please let me know if there is anything else I need to do. Thanks, Mike ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v4] PR preprocessor/83173: Enhance -fdump-internal-locations output 2018-11-26 22:17 ` Mike Gulick @ 2018-11-27 1:29 ` David Malcolm 2018-11-27 14:11 ` Mike Gulick 0 siblings, 1 reply; 34+ messages in thread From: David Malcolm @ 2018-11-27 1:29 UTC (permalink / raw) To: Mike Gulick, gcc-patches On Mon, 2018-11-26 at 22:17 +0000, Mike Gulick wrote: > On 11/13/18 3:12 PM, David Malcolm wrote: > > On Tue, 2018-11-13 at 14:54 -0500, Mike Gulick wrote: > > > 2018-11-13 Mike Gulick <mgulick@mathworks.com> > > > > [...] > > > > > * gcc/diagnostic-core.h (num_digits): Add extern definition. > > > > FWIW you moved the decl to diagnostic.h, but didn't update the > > above > > ChangeLog entry. > > > > [...] > > > > > diff --git a/libcpp/location-example.txt b/libcpp/location- > > > example.txt > > > index 14b5c2e284a..dc448b0493e 100644 > > > --- a/libcpp/location-example.txt > > > +++ b/libcpp/location-example.txt > > > > You're going to need to regenerate this file again; I touched many > > of > > the same lines as your patch does, in r266085 (sorry). > > > > Thanks. I updated this file and fixed the changelog. I will send an > updated patch after this email. The contents of this file were a > little > stale, so many of the locations in the file have changed in addition > to > the fields I added. I verified that the changed locations aren't due > to > any of these patches. Excellent; thanks. Did the latest patch go through a bootstrap and regression testing? > > Other than the nits above, this looks good to me (once you have > > your > > contribution paperwork in place). > > The contribution paperwork is now in place. There are no other > changes > to the previous patches (other than updating the changelog date). > Please let me know if there is anything else I need to do. Do you have an account with commit rights to the repository? Dave ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v4] PR preprocessor/83173: Enhance -fdump-internal-locations output 2018-11-27 1:29 ` David Malcolm @ 2018-11-27 14:11 ` Mike Gulick 2018-11-27 14:27 ` David Malcolm 0 siblings, 1 reply; 34+ messages in thread From: Mike Gulick @ 2018-11-27 14:11 UTC (permalink / raw) To: David Malcolm, gcc-patches On 11/26/18 8:29 PM, David Malcolm wrote: > On Mon, 2018-11-26 at 22:17 +0000, Mike Gulick wrote: >> On 11/13/18 3:12 PM, David Malcolm wrote: >>> On Tue, 2018-11-13 at 14:54 -0500, Mike Gulick wrote: >>>> 2018-11-13 Mike Gulick <mgulick@mathworks.com> >>> >>> [...] >>> >>>> * gcc/diagnostic-core.h (num_digits): Add extern definition. >>> >>> FWIW you moved the decl to diagnostic.h, but didn't update the >>> above >>> ChangeLog entry. >>> >>> [...] >>> >>>> diff --git a/libcpp/location-example.txt b/libcpp/location- >>>> example.txt >>>> index 14b5c2e284a..dc448b0493e 100644 >>>> --- a/libcpp/location-example.txt >>>> +++ b/libcpp/location-example.txt >>> >>> You're going to need to regenerate this file again; I touched many >>> of >>> the same lines as your patch does, in r266085 (sorry). >>> >> >> Thanks. I updated this file and fixed the changelog. I will send an >> updated patch after this email. The contents of this file were a >> little >> stale, so many of the locations in the file have changed in addition >> to >> the fields I added. I verified that the changed locations aren't due >> to >> any of these patches. > > Excellent; thanks. > > Did the latest patch go through a bootstrap and regression testing? > I built gcc with and without the patches and tested using ../gcc/configure --enable-languages=c,c++ --disable-multilib make make -k check I compared the test results using contrib/compare_tests, and the only difference between the two was the new test that was added. >>> Other than the nits above, this looks good to me (once you have >>> your >>> contribution paperwork in place). >> >> The contribution paperwork is now in place. There are no other >> changes >> to the previous patches (other than updating the changelog date). >> Please let me know if there is anything else I need to do. > > Do you have an account with commit rights to the repository? I do not. If you or someone else wants to commit these for me, that is obviously easier for me, but if you prefer me to do it just let me know where to start. I've been using the git mirror to create these patches, and haven't used subversion for a quite a few years. > > Dave > ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v4] PR preprocessor/83173: Enhance -fdump-internal-locations output 2018-11-27 14:11 ` Mike Gulick @ 2018-11-27 14:27 ` David Malcolm 2018-11-27 14:37 ` Mike Gulick 0 siblings, 1 reply; 34+ messages in thread From: David Malcolm @ 2018-11-27 14:27 UTC (permalink / raw) To: Mike Gulick, gcc-patches On Tue, 2018-11-27 at 14:10 +0000, Mike Gulick wrote: > On 11/26/18 8:29 PM, David Malcolm wrote: > > On Mon, 2018-11-26 at 22:17 +0000, Mike Gulick wrote: > > > On 11/13/18 3:12 PM, David Malcolm wrote: > > > > On Tue, 2018-11-13 at 14:54 -0500, Mike Gulick wrote: > > > > > 2018-11-13 Mike Gulick <mgulick@mathworks.com> > > > > > > > > [...] > > > > > > > > > * gcc/diagnostic-core.h (num_digits): Add extern > > > > > definition. > > > > > > > > FWIW you moved the decl to diagnostic.h, but didn't update the > > > > above > > > > ChangeLog entry. > > > > > > > > [...] > > > > > > > > > diff --git a/libcpp/location-example.txt b/libcpp/location- > > > > > example.txt > > > > > index 14b5c2e284a..dc448b0493e 100644 > > > > > --- a/libcpp/location-example.txt > > > > > +++ b/libcpp/location-example.txt > > > > > > > > You're going to need to regenerate this file again; I touched > > > > many > > > > of > > > > the same lines as your patch does, in r266085 (sorry). > > > > > > > > > > Thanks. I updated this file and fixed the changelog. I will > > > send an > > > updated patch after this email. The contents of this file were a > > > little > > > stale, so many of the locations in the file have changed in > > > addition > > > to > > > the fields I added. I verified that the changed locations aren't > > > due > > > to > > > any of these patches. > > > > Excellent; thanks. > > > > Did the latest patch go through a bootstrap and regression testing? > > > > I built gcc with and without the patches and tested using > > ../gcc/configure --enable-languages=c,c++ --disable-multilib > make > make -k check > > I compared the test results using contrib/compare_tests, and the only > difference between the two was the new test that was added. Thanks. > > > > Other than the nits above, this looks good to me (once you have > > > > your > > > > contribution paperwork in place). > > > > > > The contribution paperwork is now in place. There are no other > > > changes > > > to the previous patches (other than updating the changelog date). > > > Please let me know if there is anything else I need to do. > > > > Do you have an account with commit rights to the repository? > > I do not. If you or someone else wants to commit these for me, that > is > obviously easier for me, but if you prefer me to do it just let me > know > where to start. I've been using the git mirror to create these > patches, > and haven't used subversion for a quite a few years. I can commit them for you if you like. Please can you repost the latest version of the patches as one kit, for clarity, so I can commit them. Thanks Dave ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v4] PR preprocessor/83173: Enhance -fdump-internal-locations output 2018-11-27 14:27 ` David Malcolm @ 2018-11-27 14:37 ` Mike Gulick 2018-11-27 14:42 ` David Malcolm 0 siblings, 1 reply; 34+ messages in thread From: Mike Gulick @ 2018-11-27 14:37 UTC (permalink / raw) To: David Malcolm, gcc-patches On 11/27/18 9:27 AM, David Malcolm wrote: [...] > > I can commit them for you if you like. Please can you repost the > latest version of the patches as one kit, for clarity, so I can commit > them. > Do you want me to repost them as a patch series, or as a single patch? > Thanks > Dave > ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v4] PR preprocessor/83173: Enhance -fdump-internal-locations output 2018-11-27 14:37 ` Mike Gulick @ 2018-11-27 14:42 ` David Malcolm 2018-11-27 14:53 ` [PATCH v5 1/3] PR preprocessor/83173: Additional check before decrementing highest_location Mike Gulick 0 siblings, 1 reply; 34+ messages in thread From: David Malcolm @ 2018-11-27 14:42 UTC (permalink / raw) To: Mike Gulick, gcc-patches On Tue, 2018-11-27 at 14:37 +0000, Mike Gulick wrote: > On 11/27/18 9:27 AM, David Malcolm wrote: > > [...] > > > > > I can commit them for you if you like. Please can you repost the > > latest version of the patches as one kit, for clarity, so I can > > commit > > them. > > > > Do you want me to repost them as a patch series, or as a single > patch? Whatever's most convenient for you. Dave ^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v5 1/3] PR preprocessor/83173: Additional check before decrementing highest_location 2018-11-27 14:42 ` David Malcolm @ 2018-11-27 14:53 ` Mike Gulick 2018-11-27 14:53 ` [PATCH v5 3/3] PR preprocessor/83173: Enhance -fdump-internal-locations output Mike Gulick ` (2 more replies) 0 siblings, 3 replies; 34+ messages in thread From: Mike Gulick @ 2018-11-27 14:53 UTC (permalink / raw) To: gcc-patches; +Cc: David Malcolm, Mike Gulick 2018-11-27 Mike Gulick <mgulick@mathworks.com> PR preprocessor/83173 * libcpp/files.c (_cpp_stack_include): Check if line_table->highest_location is past current line before decrementing. --- libcpp/files.c | 32 +++++++++++++++++++++++--------- 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/libcpp/files.c b/libcpp/files.c index 3771fad75ec..fe80e84454b 100644 --- a/libcpp/files.c +++ b/libcpp/files.c @@ -1012,6 +1012,7 @@ _cpp_stack_include (cpp_reader *pfile, const char *fname, int angle_brackets, struct cpp_dir *dir; _cpp_file *file; bool stacked; + bool decremented = false; /* For -include command-line flags we have type == IT_CMDLINE. When the first -include file is processed we have the case, where @@ -1035,20 +1036,33 @@ _cpp_stack_include (cpp_reader *pfile, const char *fname, int angle_brackets, return false; /* 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 location_t 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. */ + _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 location_t 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. In the case that the #include is the last + line in the file, highest_location still points to the current + line, not the start of the next line, so we do not decrement in + this case. See plugin/location-overflow-test-pr83173.h for an + example. */ if (file->pchname == NULL && file->err_no == 0 && type != IT_CMDLINE && type != IT_DEFAULT) - pfile->line_table->highest_location--; + { + int highest_line = linemap_get_expansion_line (pfile->line_table, + pfile->line_table->highest_location); + int source_line = linemap_get_expansion_line (pfile->line_table, loc); + if (highest_line > source_line) + { + pfile->line_table->highest_location--; + decremented = true; + } + } stacked = _cpp_stack_file (pfile, file, type == IT_IMPORT, loc); - if (!stacked) + if (decremented && !stacked) /* _cpp_stack_file didn't stack the file, so let's rollback the compensation dance we performed above. */ pfile->line_table->highest_location++; -- 2.19.1 ^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v5 3/3] PR preprocessor/83173: Enhance -fdump-internal-locations output 2018-11-27 14:53 ` [PATCH v5 1/3] PR preprocessor/83173: Additional check before decrementing highest_location Mike Gulick @ 2018-11-27 14:53 ` Mike Gulick 2018-11-27 14:53 ` [PATCH v5 2/3] PR preprocessor/83173: New test Mike Gulick 2018-11-27 16:07 ` [PATCH v5 1/3] PR preprocessor/83173: Additional check before decrementing highest_location David Malcolm 2 siblings, 0 replies; 34+ messages in thread From: Mike Gulick @ 2018-11-27 14:53 UTC (permalink / raw) To: gcc-patches; +Cc: David Malcolm, Mike Gulick 2018-11-27 Mike Gulick <mgulick@mathworks.com> PR preprocessor/83173 * gcc/input.c (dump_location_info): Dump reason and included_from fields from line_map_ordinary struct. Fix indentation when location > 5 digits. * libcpp/location-example.txt: Update example -fdump-internal-locations output. * gcc/diagnostic-show-locus.c (num_digits, test_num_digits): Move to gcc/diagnostic.c to allow it to be utilized by gcc/input.c. * gcc/diagnostic.c (num_digits, test_num_digits): Moved here. (diagnostic_c_tests): Run test_num_digits. * gcc/diagnostic.h (num_digits): Add extern definition. --- gcc/diagnostic-show-locus.c | 51 ------ gcc/diagnostic.c | 46 +++++ gcc/diagnostic.h | 3 + gcc/input.c | 41 ++++- libcpp/location-example.txt | 325 ++++++++++++++++++++---------------- 5 files changed, 274 insertions(+), 192 deletions(-) diff --git a/gcc/diagnostic-show-locus.c b/gcc/diagnostic-show-locus.c index 278e17274e9..65fb102a817 100644 --- a/gcc/diagnostic-show-locus.c +++ b/gcc/diagnostic-show-locus.c @@ -819,56 +819,6 @@ fixit_cmp (const void *p_a, const void *p_b) return hint_a->get_start_loc () - hint_b->get_start_loc (); } -/* Get the number of digits in the decimal representation - of VALUE. */ - -static int -num_digits (int value) -{ - /* Perhaps simpler to use log10 for this, but doing it this way avoids - using floating point. */ - gcc_assert (value >= 0); - - if (value == 0) - return 1; - - int digits = 0; - while (value > 0) - { - digits++; - value /= 10; - } - return digits; -} - - -#if CHECKING_P - -/* Selftest for num_digits. */ - -static void -test_num_digits () -{ - ASSERT_EQ (1, num_digits (0)); - ASSERT_EQ (1, num_digits (9)); - ASSERT_EQ (2, num_digits (10)); - ASSERT_EQ (2, num_digits (99)); - ASSERT_EQ (3, num_digits (100)); - ASSERT_EQ (3, num_digits (999)); - ASSERT_EQ (4, num_digits (1000)); - ASSERT_EQ (4, num_digits (9999)); - ASSERT_EQ (5, num_digits (10000)); - ASSERT_EQ (5, num_digits (99999)); - ASSERT_EQ (6, num_digits (100000)); - ASSERT_EQ (6, num_digits (999999)); - ASSERT_EQ (7, num_digits (1000000)); - ASSERT_EQ (7, num_digits (9999999)); - ASSERT_EQ (8, num_digits (10000000)); - ASSERT_EQ (8, num_digits (99999999)); -} - -#endif /* #if CHECKING_P */ - /* Implementation of class layout. */ /* Constructor for class layout. @@ -3761,7 +3711,6 @@ void diagnostic_show_locus_c_tests () { test_line_span (); - test_num_digits (); test_layout_range_for_single_point (); test_layout_range_for_single_line (); diff --git a/gcc/diagnostic.c b/gcc/diagnostic.c index 27e98fa9434..1b572aec6de 100644 --- a/gcc/diagnostic.c +++ b/gcc/diagnostic.c @@ -1035,6 +1035,27 @@ diagnostic_report_diagnostic (diagnostic_context *context, return true; } +/* Get the number of digits in the decimal representation of VALUE. */ + +int +num_digits (int value) +{ + /* Perhaps simpler to use log10 for this, but doing it this way avoids + using floating point. */ + gcc_assert (value >= 0); + + if (value == 0) + return 1; + + int digits = 0; + while (value > 0) + { + digits++; + value /= 10; + } + return digits; +} + /* Given a partial pathname as input, return another pathname that shares no directory elements with the pathname of __FILE__. This is used by fancy_abort() to print `Internal compiler error in expr.c' @@ -1785,6 +1806,29 @@ test_diagnostic_get_location_text () progname = old_progname; } +/* Selftest for num_digits. */ + +static void +test_num_digits () +{ + ASSERT_EQ (1, num_digits (0)); + ASSERT_EQ (1, num_digits (9)); + ASSERT_EQ (2, num_digits (10)); + ASSERT_EQ (2, num_digits (99)); + ASSERT_EQ (3, num_digits (100)); + ASSERT_EQ (3, num_digits (999)); + ASSERT_EQ (4, num_digits (1000)); + ASSERT_EQ (4, num_digits (9999)); + ASSERT_EQ (5, num_digits (10000)); + ASSERT_EQ (5, num_digits (99999)); + ASSERT_EQ (6, num_digits (100000)); + ASSERT_EQ (6, num_digits (999999)); + ASSERT_EQ (7, num_digits (1000000)); + ASSERT_EQ (7, num_digits (9999999)); + ASSERT_EQ (8, num_digits (10000000)); + ASSERT_EQ (8, num_digits (99999999)); +} + /* Run all of the selftests within this file. */ void @@ -1796,6 +1840,8 @@ diagnostic_c_tests () test_print_parseable_fixits_remove (); test_print_parseable_fixits_replace (); test_diagnostic_get_location_text (); + test_num_digits (); + } } // namespace selftest diff --git a/gcc/diagnostic.h b/gcc/diagnostic.h index a926f9bdd0b..596717e331c 100644 --- a/gcc/diagnostic.h +++ b/gcc/diagnostic.h @@ -421,4 +421,7 @@ extern char *build_message_string (const char *, ...) ATTRIBUTE_PRINTF_1; extern void diagnostic_output_format_init (diagnostic_context *, enum diagnostics_output_format); +/* Compute the number of digits in the decimal representation of an integer. */ +extern int num_digits (int); + #endif /* ! GCC_DIAGNOSTIC_H */ diff --git a/gcc/input.c b/gcc/input.c index 237c0d58f07..6ce9782d3a8 100644 --- a/gcc/input.c +++ b/gcc/input.c @@ -21,6 +21,7 @@ along with GCC; see the file COPYING3. If not see #include "system.h" #include "coretypes.h" #include "intl.h" +#include "diagnostic.h" #include "diagnostic-core.h" #include "selftest.h" #include "cpplib.h" @@ -1067,6 +1068,37 @@ dump_location_info (FILE *stream) map->m_column_and_range_bits - map->m_range_bits); fprintf (stream, " range bits: %i\n", map->m_range_bits); + const char * reason; + switch (map->reason) { + case LC_ENTER: + reason = "LC_ENTER"; + break; + case LC_LEAVE: + reason = "LC_LEAVE"; + break; + case LC_RENAME: + reason = "LC_RENAME"; + break; + case LC_RENAME_VERBATIM: + reason = "LC_RENAME_VERBATIM"; + break; + case LC_ENTER_MACRO: + reason = "LC_RENAME_MACRO"; + break; + default: + reason = "Unknown"; + } + fprintf (stream, " reason: %d (%s)\n", map->reason, reason); + + const line_map_ordinary *includer_map + = linemap_included_from_linemap (line_table, map); + fprintf (stream, " included from location: %d", + linemap_included_from (map)); + if (includer_map) { + fprintf (stream, " (in ordinary map %d)", + int (includer_map - line_table->info_ordinary.maps)); + } + fprintf (stream, "\n"); /* Render the span of source lines that this "map" covers. */ for (location_t loc = MAP_START_LOCATION (map); @@ -1100,7 +1132,14 @@ dump_location_info (FILE *stream) if (max_col > line_text.length ()) max_col = line_text.length () + 1; - int indent = 14 + strlen (exploc.file); + int len_lnum = num_digits (exploc.line); + if (len_lnum < 3) + len_lnum = 3; + int len_loc = num_digits (loc); + if (len_loc < 5) + len_loc = 5; + + int indent = 6 + strlen (exploc.file) + len_lnum + len_loc; /* Thousands. */ if (end_location > 999) diff --git a/libcpp/location-example.txt b/libcpp/location-example.txt index 829ca53b89b..f6d98e2d228 100644 --- a/libcpp/location-example.txt +++ b/libcpp/location-example.txt @@ -33,8 +33,11 @@ ORDINARY MAP: 0 location_t interval: 32 <= loc < 64 file: test.c starting at line: 1 - column bits: 12 + column and range bits: 12 + column bits: 7 range bits: 5 + reason: 0 (LC_ENTER) + included from location: 0 test.c: 1|loc: 32|#include "test.h" |69269258258148147 |46802468024680246 @@ -43,186 +46,228 @@ ORDINARY MAP: 1 location_t interval: 64 <= loc < 96 file: <built-in> starting at line: 0 + column and range bits: 0 column bits: 0 range bits: 0 + reason: 2 (LC_RENAME) + included from location: 0 ORDINARY MAP: 2 location_t interval: 96 <= loc < 128 file: <command-line> starting at line: 0 + column and range bits: 0 column bits: 0 range bits: 0 + reason: 2 (LC_RENAME) + included from location: 0 ORDINARY MAP: 3 - location_t interval: 128 <= loc < 160128 + location_t interval: 128 <= loc < 250240 file: /usr/include/stdc-predef.h starting at line: 1 - column bits: 12 + column and range bits: 12 + column bits: 7 range bits: 5 + reason: 0 (LC_ENTER) + included from location: 127 (in ordinary map 2) (contents of /usr/include/stdc-predef.h snipped for brevity) ORDINARY MAP: 4 - location_t interval: 160128 <= loc < 160160 + location_t interval: 250240 <= loc < 250272 file: <command-line> starting at line: 32 - column bits: 12 + column and range bits: 12 + column bits: 7 range bits: 5 + reason: 1 (LC_LEAVE) + included from location: 0 ORDINARY MAP: 5 - location_t interval: 160160 <= loc < 164256 + location_t interval: 250272 <= loc < 254368 file: test.c starting at line: 1 - column bits: 12 + column and range bits: 12 + column bits: 7 range bits: 5 -test.c: 1|loc:160160|#include "test.h" - |00000000000000000 - |12223334445556667 - |92582581481470470 - |24680246802468024 + reason: 2 (LC_RENAME) + included from location: 0 +test.c: 1|loc:250272|#include "test.h" + |00000000000000000 + |33344445556667778 + |03603692692582581 + |46802468024680246 ORDINARY MAP: 6 - location_t interval: 164256 <= loc < 173280 + location_t interval: 254368 <= loc < 266720 file: test.h starting at line: 1 - column bits: 12 + column and range bits: 12 + column bits: 7 range bits: 5 -test.h: 1|loc:164256|extern int foo (); - |444444444444444444 - |233344455566677788 - |825814814704703603 - |802468024680246802 -test.h: 2|loc:168352| - | - | - | - | -test.h: 3|loc:172448|#define PLUS(A, B) A + B - |222222222222222223333333 - |455566677788889990001112 - |814704703603692692582581 - |024680246802468024680246 + reason: 0 (LC_ENTER) + included from location: 250272 (in ordinary map 5) +test.h: 1|loc:254368|extern int foo (); + |444444444444444444 + |444455566677788899 + |036926925825814814 + |024680246802468024 +test.h: 2|loc:258464| + | + | + | + | +test.h: 3|loc:262560|#define PLUS(A, B) A + B + |222222222222233333333333 + |566677788899900011122223 + |925825814814704703603692 + |246802468024680246802468 +test.h: 4|loc:266656| + | + | + | + | ORDINARY MAP: 7 - location_t interval: 173280 <= loc < 202016 + location_t interval: 266720 <= loc < 299520 file: test.c starting at line: 2 - column bits: 12 + column and range bits: 12 + column bits: 7 range bits: 5 -test.c: 2|loc:173280| - | - | - | - | -test.c: 3|loc:177376|int - |777 - |444 - |047 - |802 -test.c: 4|loc:181472|main (int argc, char **argv) - |1111111111111111222222222222 - |5556666777888999000111222333 - |0360369269258258148147047036 - |4680246802468024680246802468 -test.c: 5|loc:185568|{ - |5 - |6 - |0 - |0 -test.c: 6|loc:189664| int a = PLUS (1,2); - |999999999900000000000 - |677788899900011122233 - |926925825814814704703 - |680246802468024680246 -test.c: 7|loc:193760| int b = PLUS (3,4); - |333333344444444444444 - |788899900011122233344 - |925825814814704703603 - |246802468024680246802 -test.c: 8|loc:197856| return 0; - |77778888888 - |89990001112 - |82581481470 - |80246802468 -test.c: 9|loc:201952|} - |1 - |9 - |8 - |4 + reason: 1 (LC_LEAVE) + included from location: 0 +test.c: 2|loc:266720| + | + | + | + | +test.c: 3|loc:270816|int + |000 + |889 + |481 + |802 +test.c: 4|loc:274912|main (int argc, char **argv) + |4455555555555555555555555555 + |9900011122223334445556667778 + |4704703603692692582581481470 + |4680246802468024680246802468 +test.c: 5|loc:279008|{ + |9 + |0 + |4 + |0 +test.c: 6|loc:283104| int a = PLUS (1,2); + |333333333333333333333 + |112222333444555666777 + |360369269258258148147 + |680246802468024680246 +test.c: 7|loc:287200| int b = PLUS (3,4); + |777777777777777777777 + |222333444555666777888 + |369269258258148147047 + |246802468024680246802 +test.c: 8|loc:291296| return 0; + |11111111111 + |33344455566 + |26925825814 + |80246802468 +test.c: 9|loc:295392|} + |5 + |4 + |2 + |4 +test.c: 10|loc:299488| + | + | + | + | UNALLOCATED LOCATIONS - location_t interval: 202016 <= loc < 2147483633 + location_t interval: 299520 <= loc < 2147483632 -MACRO 1: PLUS (7 tokens) - location_t interval: 2147483633 <= loc < 2147483640 -test.c:7:11: note: expansion point is location 194115 - int b = PLUS (3,4); - ^~~~ +MACRO 3: PLUS (7 tokens) + location_t interval: 2147483632 <= loc < 2147483639 +test.c:7:11: note: expansion point is location 287555 + 7 | int b = PLUS (3,4); + | ^~~~ + map->start_location: 2147483632 + macro_locations: + 0: 287744, 263200 +test.c:7:17: note: token 0 has x-location == 287744 + 7 | int b = PLUS (3,4); + | ^ +test.c:7:17: note: token 0 has y-location == 263200 + 1: 263264, 263264 +In file included from test.c:1: +test.h:3:22: note: token 1 has x-location == y-location == 263264 + 3 | #define PLUS(A, B) A + B + | ^ + 2: 287808, 263328 +test.c:7:19: note: token 2 has x-location == 287808 + 7 | int b = PLUS (3,4); + | ^ +test.c:7:19: note: token 2 has y-location == 263328 + 3: 0, 0 +cc1: note: token 3 has x-location == y-location == 0 + 4: 0, 0 +cc1: note: token 4 has x-location == y-location == 0 + 5: 0, 0 +cc1: note: token 5 has x-location == y-location == 0 + 6: 0, 0 +cc1: note: token 6 has x-location == y-location == 0 + +MACRO 2: PLUS (7 tokens) + location_t interval: 2147483639 <= loc < 2147483646 +test.c:6:11: note: expansion point is location 283459 + 6 | int a = PLUS (1,2); + | ^~~~ + map->start_location: 2147483639 + macro_locations: + 0: 283648, 263200 +test.c:6:17: note: token 0 has x-location == 283648 + 6 | int a = PLUS (1,2); + | ^ +test.c:6:17: note: token 0 has y-location == 263200 + 1: 263264, 263264 +In file included from test.c:1: +test.h:3:22: note: token 1 has x-location == y-location == 263264 + 3 | #define PLUS(A, B) A + B + | ^ + 2: 283712, 263328 +test.c:6:19: note: token 2 has x-location == 283712 + 6 | int a = PLUS (1,2); + | ^ +test.c:6:19: note: token 2 has y-location == 263328 + 3: 0, 0 +cc1: note: token 3 has x-location == y-location == 0 + 4: 0, 0 +cc1: note: token 4 has x-location == y-location == 0 + 5: 0, 0 +cc1: note: token 5 has x-location == y-location == 0 + 6: 0, 0 +cc1: note: token 6 has x-location == y-location == 0 - map->start_location: 2147483633 +MACRO 1: __GCC_IEC_559_COMPLEX (1 tokens) + location_t interval: 2147483646 <= loc < 2147483647 +In file included from <command-line>:31: +/usr/include/stdc-predef.h:45:6: note: expansion point is location 180564 + 45 | # if __GCC_IEC_559_COMPLEX > 0 + | ^~~~~~~~~~~~~~~~~~~~~ + map->start_location: 2147483646 macro_locations: - 0: 194304, 173088 -test.c:7:17: note: token 0 has x-location == 194304 - int b = PLUS (3,4); - ^ - -test.c:7:17: note: token 0 has y-location == 173088 - 1: 173152, 173152 -In file included from test.c:1:0: -test.h:3:22: note: token 1 has x-location == y-location == 173152 - #define PLUS(A, B) A + B - ^ - - 2: 194368, 173216 -test.c:7:19: note: token 2 has x-location == 194368 - int b = PLUS (3,4); - ^ - -test.c:7:19: note: token 2 has y-location == 173216 - 3: 0, 2947526575 -cc1: note: token 3 has x-location == 0 -cc1: note: token 3 has y-location == 2947526575 - 4: 2947526575, 2947526575 -x-location == y-location == 2947526575 encodes token # 800042942 - 5: 2947526575, 2947526575 -x-location == y-location == 2947526575 encodes token # 800042942 - 6: 2947526575, 2947526575 -x-location == y-location == 2947526575 encodes token # 800042942 - -MACRO 0: PLUS (7 tokens) - location_t interval: 2147483640 <= loc < 2147483647 -test.c:6:11: note: expansion point is location 190019 - int a = PLUS (1,2); - ^~~~ - - map->start_location: 2147483640 + 0: 1, 1 +<built-in>: note: token 0 has x-location == y-location == 1 + +MACRO 0: __GCC_IEC_559 (1 tokens) + location_t interval: 2147483647 <= loc < 2147483648 +/usr/include/stdc-predef.h:37:6: note: expansion point is location 147788 + 37 | # if __GCC_IEC_559 > 0 + | ^~~~~~~~~~~~~ + map->start_location: 2147483647 macro_locations: - 0: 190208, 173088 -test.c:6:17: note: token 0 has x-location == 190208 - int a = PLUS (1,2); - ^ - -test.c:6:17: note: token 0 has y-location == 173088 - 1: 173152, 173152 -In file included from test.c:1:0: -test.h:3:22: note: token 1 has x-location == y-location == 173152 - #define PLUS(A, B) A + B - ^ - - 2: 190272, 173216 -test.c:6:19: note: token 2 has x-location == 190272 - int a = PLUS (1,2); - ^ - -test.c:6:19: note: token 2 has y-location == 173216 - 3: 0, 2947526575 -cc1: note: token 3 has x-location == 0 -cc1: note: token 3 has y-location == 2947526575 - 4: 2947526575, 2947526575 -x-location == y-location == 2947526575 encodes token # 800042935 - 5: 2947526575, 2947526575 -x-location == y-location == 2947526575 encodes token # 800042935 - 6: 2947526575, 2947526575 -x-location == y-location == 2947526575 encodes token # 800042935 + 0: 1, 1 +<built-in>: note: token 0 has x-location == y-location == 1 MAX_LOCATION_T location_t interval: 2147483647 <= loc < 2147483648 -- 2.19.1 ^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v5 2/3] PR preprocessor/83173: New test 2018-11-27 14:53 ` [PATCH v5 1/3] PR preprocessor/83173: Additional check before decrementing highest_location Mike Gulick 2018-11-27 14:53 ` [PATCH v5 3/3] PR preprocessor/83173: Enhance -fdump-internal-locations output Mike Gulick @ 2018-11-27 14:53 ` Mike Gulick 2018-11-27 16:07 ` [PATCH v5 1/3] PR preprocessor/83173: Additional check before decrementing highest_location David Malcolm 2 siblings, 0 replies; 34+ messages in thread From: Mike Gulick @ 2018-11-27 14:53 UTC (permalink / raw) To: gcc-patches; +Cc: David Malcolm, Mike Gulick 2018-11-27 Mike Gulick <mgulick@mathworks.com> 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. --- .../plugin/location-overflow-test-pr83173-1.h | 2 ++ .../plugin/location-overflow-test-pr83173-2.h | 2 ++ .../plugin/location-overflow-test-pr83173.c | 21 +++++++++++++++++++ .../plugin/location-overflow-test-pr83173.h | 2 ++ .../gcc.dg/plugin/location_overflow_plugin.c | 13 +++++++----- gcc/testsuite/gcc.dg/plugin/plugin.exp | 3 ++- 6 files changed, 37 insertions(+), 6 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173-1.h create mode 100644 gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173-2.h create mode 100644 gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173.c create mode 100644 gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173.h diff --git a/gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173-1.h b/gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173-1.h new file mode 100644 index 00000000000..f47dc3643c2 --- /dev/null +++ b/gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173-1.h @@ -0,0 +1,2 @@ +#pragma once +#define LOCATION_OVERFLOW_TEST_PR83173_1_H diff --git a/gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173-2.h b/gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173-2.h new file mode 100644 index 00000000000..bc23ed2a188 --- /dev/null +++ b/gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173-2.h @@ -0,0 +1,2 @@ +#pragma once +#define LOCATION_OVERFLOW_TEST_PR83173_2_H diff --git a/gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173.c b/gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173.c new file mode 100644 index 00000000000..2d581283474 --- /dev/null +++ b/gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173.c @@ -0,0 +1,21 @@ +/* + { dg-options "-fplugin-arg-location_overflow_plugin-value=0x60000001" } + { dg-do preprocess } +*/ + +#include "location-overflow-test-pr83173.h" + +int +main () +{ + return 0; +} + +/* + The preprocessor output should contain + '# 1 "path/to/location-overflow-test-pr83173-1.h" 1', but should not + contain any other references to location-overflow-test-pr83183-1.h. + + { dg-final { scan-file location-overflow-test-pr83173.i "# 1 \[^\r\n\]+location-overflow-test-pr83173-1\.h\" 1" } } + { dg-final { scan-file-not location-overflow-test-pr83173.i "# (?!1 \[^\r\n\]+location-overflow-test-pr83173-1\.h\" 1)\[0-9\]+ \[^\r\n\]+location-overflow-test-pr83173-1\.h\"" } } +*/ diff --git a/gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173.h b/gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173.h new file mode 100644 index 00000000000..49076f7ea3c --- /dev/null +++ b/gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173.h @@ -0,0 +1,2 @@ +#include "location-overflow-test-pr83173-1.h" +#include "location-overflow-test-pr83173-2.h" diff --git a/gcc/testsuite/gcc.dg/plugin/location_overflow_plugin.c b/gcc/testsuite/gcc.dg/plugin/location_overflow_plugin.c index 5c9d5bae77e..09bac50d2af 100644 --- a/gcc/testsuite/gcc.dg/plugin/location_overflow_plugin.c +++ b/gcc/testsuite/gcc.dg/plugin/location_overflow_plugin.c @@ -12,11 +12,14 @@ int plugin_is_GPL_compatible; static location_t base_location; -/* Callback handler for the PLUGIN_START_UNIT event; pretend - we parsed a very large include file. */ +/* Callback handler for the PLUGIN_PRAGMAS event; pretend we parsed a + very large include file. This is used to set the initial line table + offset for the preprocessor, to make it appear as if we had parsed a + very large file. PRAGMA_START_UNIT is not suitable here as is not + invoked during the preprocessor stage. */ static void -on_start_unit (void */*gcc_data*/, void */*user_data*/) +on_pragma_registration (void */*gcc_data*/, void */*user_data*/) { /* Act as if we've already parsed a large body of code; so that we can simulate various fallbacks in libcpp: @@ -81,8 +84,8 @@ plugin_init (struct plugin_name_args *plugin_info, error_at (UNKNOWN_LOCATION, "missing plugin argument"); register_callback (plugin_info->base_name, - PLUGIN_START_UNIT, - on_start_unit, + PLUGIN_PRAGMAS, + on_pragma_registration, NULL); /* void *user_data */ /* Hack in additional testing, based on the exact value supplied. */ diff --git a/gcc/testsuite/gcc.dg/plugin/plugin.exp b/gcc/testsuite/gcc.dg/plugin/plugin.exp index d92ede79c0d..f9e89c48140 100644 --- a/gcc/testsuite/gcc.dg/plugin/plugin.exp +++ b/gcc/testsuite/gcc.dg/plugin/plugin.exp @@ -96,7 +96,8 @@ set plugin_test_list [list \ diagnostic-test-inlining-4.c } \ { location_overflow_plugin.c \ location-overflow-test-1.c \ - location-overflow-test-2.c } \ + location-overflow-test-2.c \ + location-overflow-test-pr83173.c } \ { must_tail_call_plugin.c \ must-tail-call-1.c \ must-tail-call-2.c } \ -- 2.19.1 ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v5 1/3] PR preprocessor/83173: Additional check before decrementing highest_location 2018-11-27 14:53 ` [PATCH v5 1/3] PR preprocessor/83173: Additional check before decrementing highest_location Mike Gulick 2018-11-27 14:53 ` [PATCH v5 3/3] PR preprocessor/83173: Enhance -fdump-internal-locations output Mike Gulick 2018-11-27 14:53 ` [PATCH v5 2/3] PR preprocessor/83173: New test Mike Gulick @ 2018-11-27 16:07 ` David Malcolm 2018-11-27 16:18 ` Mike Gulick 2 siblings, 1 reply; 34+ messages in thread From: David Malcolm @ 2018-11-27 16:07 UTC (permalink / raw) To: Mike Gulick, gcc-patches On Tue, 2018-11-27 at 09:53 -0500, Mike Gulick wrote: > 2018-11-27 Mike Gulick <mgulick@mathworks.com> > > PR preprocessor/83173 > * libcpp/files.c (_cpp_stack_include): Check if > line_table->highest_location is past current line before > decrementing. I've committed these patches to trunk as r266516, r266518, and r266520 respectively. Thanks for all your work on this (and your patience; sorry about the delays in reviewing it) Dave ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v5 1/3] PR preprocessor/83173: Additional check before decrementing highest_location 2018-11-27 16:07 ` [PATCH v5 1/3] PR preprocessor/83173: Additional check before decrementing highest_location David Malcolm @ 2018-11-27 16:18 ` Mike Gulick 0 siblings, 0 replies; 34+ messages in thread From: Mike Gulick @ 2018-11-27 16:18 UTC (permalink / raw) To: David Malcolm, gcc-patches On 11/27/18 11:07 AM, David Malcolm wrote: > On Tue, 2018-11-27 at 09:53 -0500, Mike Gulick wrote: >> 2018-11-27 Mike Gulick <mgulick@mathworks.com> >> >> PR preprocessor/83173 >> * libcpp/files.c (_cpp_stack_include): Check if >> line_table->highest_location is past current line before >> decrementing. > > I've committed these patches to trunk as r266516, r266518, and r266520 > respectively. > > Thanks for all your work on this (and your patience; sorry about the > delays in reviewing it) > > Dave > Hi Dave, Thanks so much for your time and help in reviewing these patches. I'm glad to see this bug finally fixed (coincidentally exactly a year after I reported it)! Thanks, Mike ^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v5] PR preprocessor/83173: Enhance -fdump-internal-locations output 2018-11-13 20:13 ` David Malcolm 2018-11-26 22:17 ` Mike Gulick @ 2018-11-26 22:22 ` Mike Gulick 1 sibling, 0 replies; 34+ messages in thread From: Mike Gulick @ 2018-11-26 22:22 UTC (permalink / raw) To: gcc-patches; +Cc: David Malcolm, Mike Gulick 2018-11-26 Mike Gulick <mgulick@mathworks.com> PR preprocessor/83173 * gcc/input.c (dump_location_info): Dump reason and included_from fields from line_map_ordinary struct. Fix indentation when location > 5 digits. * libcpp/location-example.txt: Update example -fdump-internal-locations output. * gcc/diagnostic-show-locus.c (num_digits, test_num_digits): Move to gcc/diagnostic.c to allow it to be utilized by gcc/input.c. * gcc/diagnostic.c (num_digits, test_num_digits): Moved here. (diagnostic_c_tests): Run test_num_digits. * gcc/diagnostic.h (num_digits): Add extern definition. --- gcc/diagnostic-show-locus.c | 51 ------ gcc/diagnostic.c | 46 +++++ gcc/diagnostic.h | 3 + gcc/input.c | 41 ++++- libcpp/location-example.txt | 325 ++++++++++++++++++++---------------- 5 files changed, 274 insertions(+), 192 deletions(-) diff --git a/gcc/diagnostic-show-locus.c b/gcc/diagnostic-show-locus.c index 278e17274e9..65fb102a817 100644 --- a/gcc/diagnostic-show-locus.c +++ b/gcc/diagnostic-show-locus.c @@ -819,56 +819,6 @@ fixit_cmp (const void *p_a, const void *p_b) return hint_a->get_start_loc () - hint_b->get_start_loc (); } -/* Get the number of digits in the decimal representation - of VALUE. */ - -static int -num_digits (int value) -{ - /* Perhaps simpler to use log10 for this, but doing it this way avoids - using floating point. */ - gcc_assert (value >= 0); - - if (value == 0) - return 1; - - int digits = 0; - while (value > 0) - { - digits++; - value /= 10; - } - return digits; -} - - -#if CHECKING_P - -/* Selftest for num_digits. */ - -static void -test_num_digits () -{ - ASSERT_EQ (1, num_digits (0)); - ASSERT_EQ (1, num_digits (9)); - ASSERT_EQ (2, num_digits (10)); - ASSERT_EQ (2, num_digits (99)); - ASSERT_EQ (3, num_digits (100)); - ASSERT_EQ (3, num_digits (999)); - ASSERT_EQ (4, num_digits (1000)); - ASSERT_EQ (4, num_digits (9999)); - ASSERT_EQ (5, num_digits (10000)); - ASSERT_EQ (5, num_digits (99999)); - ASSERT_EQ (6, num_digits (100000)); - ASSERT_EQ (6, num_digits (999999)); - ASSERT_EQ (7, num_digits (1000000)); - ASSERT_EQ (7, num_digits (9999999)); - ASSERT_EQ (8, num_digits (10000000)); - ASSERT_EQ (8, num_digits (99999999)); -} - -#endif /* #if CHECKING_P */ - /* Implementation of class layout. */ /* Constructor for class layout. @@ -3761,7 +3711,6 @@ void diagnostic_show_locus_c_tests () { test_line_span (); - test_num_digits (); test_layout_range_for_single_point (); test_layout_range_for_single_line (); diff --git a/gcc/diagnostic.c b/gcc/diagnostic.c index 27e98fa9434..1b572aec6de 100644 --- a/gcc/diagnostic.c +++ b/gcc/diagnostic.c @@ -1035,6 +1035,27 @@ diagnostic_report_diagnostic (diagnostic_context *context, return true; } +/* Get the number of digits in the decimal representation of VALUE. */ + +int +num_digits (int value) +{ + /* Perhaps simpler to use log10 for this, but doing it this way avoids + using floating point. */ + gcc_assert (value >= 0); + + if (value == 0) + return 1; + + int digits = 0; + while (value > 0) + { + digits++; + value /= 10; + } + return digits; +} + /* Given a partial pathname as input, return another pathname that shares no directory elements with the pathname of __FILE__. This is used by fancy_abort() to print `Internal compiler error in expr.c' @@ -1785,6 +1806,29 @@ test_diagnostic_get_location_text () progname = old_progname; } +/* Selftest for num_digits. */ + +static void +test_num_digits () +{ + ASSERT_EQ (1, num_digits (0)); + ASSERT_EQ (1, num_digits (9)); + ASSERT_EQ (2, num_digits (10)); + ASSERT_EQ (2, num_digits (99)); + ASSERT_EQ (3, num_digits (100)); + ASSERT_EQ (3, num_digits (999)); + ASSERT_EQ (4, num_digits (1000)); + ASSERT_EQ (4, num_digits (9999)); + ASSERT_EQ (5, num_digits (10000)); + ASSERT_EQ (5, num_digits (99999)); + ASSERT_EQ (6, num_digits (100000)); + ASSERT_EQ (6, num_digits (999999)); + ASSERT_EQ (7, num_digits (1000000)); + ASSERT_EQ (7, num_digits (9999999)); + ASSERT_EQ (8, num_digits (10000000)); + ASSERT_EQ (8, num_digits (99999999)); +} + /* Run all of the selftests within this file. */ void @@ -1796,6 +1840,8 @@ diagnostic_c_tests () test_print_parseable_fixits_remove (); test_print_parseable_fixits_replace (); test_diagnostic_get_location_text (); + test_num_digits (); + } } // namespace selftest diff --git a/gcc/diagnostic.h b/gcc/diagnostic.h index a926f9bdd0b..596717e331c 100644 --- a/gcc/diagnostic.h +++ b/gcc/diagnostic.h @@ -421,4 +421,7 @@ extern char *build_message_string (const char *, ...) ATTRIBUTE_PRINTF_1; extern void diagnostic_output_format_init (diagnostic_context *, enum diagnostics_output_format); +/* Compute the number of digits in the decimal representation of an integer. */ +extern int num_digits (int); + #endif /* ! GCC_DIAGNOSTIC_H */ diff --git a/gcc/input.c b/gcc/input.c index 237c0d58f07..6ce9782d3a8 100644 --- a/gcc/input.c +++ b/gcc/input.c @@ -21,6 +21,7 @@ along with GCC; see the file COPYING3. If not see #include "system.h" #include "coretypes.h" #include "intl.h" +#include "diagnostic.h" #include "diagnostic-core.h" #include "selftest.h" #include "cpplib.h" @@ -1067,6 +1068,37 @@ dump_location_info (FILE *stream) map->m_column_and_range_bits - map->m_range_bits); fprintf (stream, " range bits: %i\n", map->m_range_bits); + const char * reason; + switch (map->reason) { + case LC_ENTER: + reason = "LC_ENTER"; + break; + case LC_LEAVE: + reason = "LC_LEAVE"; + break; + case LC_RENAME: + reason = "LC_RENAME"; + break; + case LC_RENAME_VERBATIM: + reason = "LC_RENAME_VERBATIM"; + break; + case LC_ENTER_MACRO: + reason = "LC_RENAME_MACRO"; + break; + default: + reason = "Unknown"; + } + fprintf (stream, " reason: %d (%s)\n", map->reason, reason); + + const line_map_ordinary *includer_map + = linemap_included_from_linemap (line_table, map); + fprintf (stream, " included from location: %d", + linemap_included_from (map)); + if (includer_map) { + fprintf (stream, " (in ordinary map %d)", + int (includer_map - line_table->info_ordinary.maps)); + } + fprintf (stream, "\n"); /* Render the span of source lines that this "map" covers. */ for (location_t loc = MAP_START_LOCATION (map); @@ -1100,7 +1132,14 @@ dump_location_info (FILE *stream) if (max_col > line_text.length ()) max_col = line_text.length () + 1; - int indent = 14 + strlen (exploc.file); + int len_lnum = num_digits (exploc.line); + if (len_lnum < 3) + len_lnum = 3; + int len_loc = num_digits (loc); + if (len_loc < 5) + len_loc = 5; + + int indent = 6 + strlen (exploc.file) + len_lnum + len_loc; /* Thousands. */ if (end_location > 999) diff --git a/libcpp/location-example.txt b/libcpp/location-example.txt index 829ca53b89b..f6d98e2d228 100644 --- a/libcpp/location-example.txt +++ b/libcpp/location-example.txt @@ -33,8 +33,11 @@ ORDINARY MAP: 0 location_t interval: 32 <= loc < 64 file: test.c starting at line: 1 - column bits: 12 + column and range bits: 12 + column bits: 7 range bits: 5 + reason: 0 (LC_ENTER) + included from location: 0 test.c: 1|loc: 32|#include "test.h" |69269258258148147 |46802468024680246 @@ -43,186 +46,228 @@ ORDINARY MAP: 1 location_t interval: 64 <= loc < 96 file: <built-in> starting at line: 0 + column and range bits: 0 column bits: 0 range bits: 0 + reason: 2 (LC_RENAME) + included from location: 0 ORDINARY MAP: 2 location_t interval: 96 <= loc < 128 file: <command-line> starting at line: 0 + column and range bits: 0 column bits: 0 range bits: 0 + reason: 2 (LC_RENAME) + included from location: 0 ORDINARY MAP: 3 - location_t interval: 128 <= loc < 160128 + location_t interval: 128 <= loc < 250240 file: /usr/include/stdc-predef.h starting at line: 1 - column bits: 12 + column and range bits: 12 + column bits: 7 range bits: 5 + reason: 0 (LC_ENTER) + included from location: 127 (in ordinary map 2) (contents of /usr/include/stdc-predef.h snipped for brevity) ORDINARY MAP: 4 - location_t interval: 160128 <= loc < 160160 + location_t interval: 250240 <= loc < 250272 file: <command-line> starting at line: 32 - column bits: 12 + column and range bits: 12 + column bits: 7 range bits: 5 + reason: 1 (LC_LEAVE) + included from location: 0 ORDINARY MAP: 5 - location_t interval: 160160 <= loc < 164256 + location_t interval: 250272 <= loc < 254368 file: test.c starting at line: 1 - column bits: 12 + column and range bits: 12 + column bits: 7 range bits: 5 -test.c: 1|loc:160160|#include "test.h" - |00000000000000000 - |12223334445556667 - |92582581481470470 - |24680246802468024 + reason: 2 (LC_RENAME) + included from location: 0 +test.c: 1|loc:250272|#include "test.h" + |00000000000000000 + |33344445556667778 + |03603692692582581 + |46802468024680246 ORDINARY MAP: 6 - location_t interval: 164256 <= loc < 173280 + location_t interval: 254368 <= loc < 266720 file: test.h starting at line: 1 - column bits: 12 + column and range bits: 12 + column bits: 7 range bits: 5 -test.h: 1|loc:164256|extern int foo (); - |444444444444444444 - |233344455566677788 - |825814814704703603 - |802468024680246802 -test.h: 2|loc:168352| - | - | - | - | -test.h: 3|loc:172448|#define PLUS(A, B) A + B - |222222222222222223333333 - |455566677788889990001112 - |814704703603692692582581 - |024680246802468024680246 + reason: 0 (LC_ENTER) + included from location: 250272 (in ordinary map 5) +test.h: 1|loc:254368|extern int foo (); + |444444444444444444 + |444455566677788899 + |036926925825814814 + |024680246802468024 +test.h: 2|loc:258464| + | + | + | + | +test.h: 3|loc:262560|#define PLUS(A, B) A + B + |222222222222233333333333 + |566677788899900011122223 + |925825814814704703603692 + |246802468024680246802468 +test.h: 4|loc:266656| + | + | + | + | ORDINARY MAP: 7 - location_t interval: 173280 <= loc < 202016 + location_t interval: 266720 <= loc < 299520 file: test.c starting at line: 2 - column bits: 12 + column and range bits: 12 + column bits: 7 range bits: 5 -test.c: 2|loc:173280| - | - | - | - | -test.c: 3|loc:177376|int - |777 - |444 - |047 - |802 -test.c: 4|loc:181472|main (int argc, char **argv) - |1111111111111111222222222222 - |5556666777888999000111222333 - |0360369269258258148147047036 - |4680246802468024680246802468 -test.c: 5|loc:185568|{ - |5 - |6 - |0 - |0 -test.c: 6|loc:189664| int a = PLUS (1,2); - |999999999900000000000 - |677788899900011122233 - |926925825814814704703 - |680246802468024680246 -test.c: 7|loc:193760| int b = PLUS (3,4); - |333333344444444444444 - |788899900011122233344 - |925825814814704703603 - |246802468024680246802 -test.c: 8|loc:197856| return 0; - |77778888888 - |89990001112 - |82581481470 - |80246802468 -test.c: 9|loc:201952|} - |1 - |9 - |8 - |4 + reason: 1 (LC_LEAVE) + included from location: 0 +test.c: 2|loc:266720| + | + | + | + | +test.c: 3|loc:270816|int + |000 + |889 + |481 + |802 +test.c: 4|loc:274912|main (int argc, char **argv) + |4455555555555555555555555555 + |9900011122223334445556667778 + |4704703603692692582581481470 + |4680246802468024680246802468 +test.c: 5|loc:279008|{ + |9 + |0 + |4 + |0 +test.c: 6|loc:283104| int a = PLUS (1,2); + |333333333333333333333 + |112222333444555666777 + |360369269258258148147 + |680246802468024680246 +test.c: 7|loc:287200| int b = PLUS (3,4); + |777777777777777777777 + |222333444555666777888 + |369269258258148147047 + |246802468024680246802 +test.c: 8|loc:291296| return 0; + |11111111111 + |33344455566 + |26925825814 + |80246802468 +test.c: 9|loc:295392|} + |5 + |4 + |2 + |4 +test.c: 10|loc:299488| + | + | + | + | UNALLOCATED LOCATIONS - location_t interval: 202016 <= loc < 2147483633 + location_t interval: 299520 <= loc < 2147483632 -MACRO 1: PLUS (7 tokens) - location_t interval: 2147483633 <= loc < 2147483640 -test.c:7:11: note: expansion point is location 194115 - int b = PLUS (3,4); - ^~~~ +MACRO 3: PLUS (7 tokens) + location_t interval: 2147483632 <= loc < 2147483639 +test.c:7:11: note: expansion point is location 287555 + 7 | int b = PLUS (3,4); + | ^~~~ + map->start_location: 2147483632 + macro_locations: + 0: 287744, 263200 +test.c:7:17: note: token 0 has x-location == 287744 + 7 | int b = PLUS (3,4); + | ^ +test.c:7:17: note: token 0 has y-location == 263200 + 1: 263264, 263264 +In file included from test.c:1: +test.h:3:22: note: token 1 has x-location == y-location == 263264 + 3 | #define PLUS(A, B) A + B + | ^ + 2: 287808, 263328 +test.c:7:19: note: token 2 has x-location == 287808 + 7 | int b = PLUS (3,4); + | ^ +test.c:7:19: note: token 2 has y-location == 263328 + 3: 0, 0 +cc1: note: token 3 has x-location == y-location == 0 + 4: 0, 0 +cc1: note: token 4 has x-location == y-location == 0 + 5: 0, 0 +cc1: note: token 5 has x-location == y-location == 0 + 6: 0, 0 +cc1: note: token 6 has x-location == y-location == 0 + +MACRO 2: PLUS (7 tokens) + location_t interval: 2147483639 <= loc < 2147483646 +test.c:6:11: note: expansion point is location 283459 + 6 | int a = PLUS (1,2); + | ^~~~ + map->start_location: 2147483639 + macro_locations: + 0: 283648, 263200 +test.c:6:17: note: token 0 has x-location == 283648 + 6 | int a = PLUS (1,2); + | ^ +test.c:6:17: note: token 0 has y-location == 263200 + 1: 263264, 263264 +In file included from test.c:1: +test.h:3:22: note: token 1 has x-location == y-location == 263264 + 3 | #define PLUS(A, B) A + B + | ^ + 2: 283712, 263328 +test.c:6:19: note: token 2 has x-location == 283712 + 6 | int a = PLUS (1,2); + | ^ +test.c:6:19: note: token 2 has y-location == 263328 + 3: 0, 0 +cc1: note: token 3 has x-location == y-location == 0 + 4: 0, 0 +cc1: note: token 4 has x-location == y-location == 0 + 5: 0, 0 +cc1: note: token 5 has x-location == y-location == 0 + 6: 0, 0 +cc1: note: token 6 has x-location == y-location == 0 - map->start_location: 2147483633 +MACRO 1: __GCC_IEC_559_COMPLEX (1 tokens) + location_t interval: 2147483646 <= loc < 2147483647 +In file included from <command-line>:31: +/usr/include/stdc-predef.h:45:6: note: expansion point is location 180564 + 45 | # if __GCC_IEC_559_COMPLEX > 0 + | ^~~~~~~~~~~~~~~~~~~~~ + map->start_location: 2147483646 macro_locations: - 0: 194304, 173088 -test.c:7:17: note: token 0 has x-location == 194304 - int b = PLUS (3,4); - ^ - -test.c:7:17: note: token 0 has y-location == 173088 - 1: 173152, 173152 -In file included from test.c:1:0: -test.h:3:22: note: token 1 has x-location == y-location == 173152 - #define PLUS(A, B) A + B - ^ - - 2: 194368, 173216 -test.c:7:19: note: token 2 has x-location == 194368 - int b = PLUS (3,4); - ^ - -test.c:7:19: note: token 2 has y-location == 173216 - 3: 0, 2947526575 -cc1: note: token 3 has x-location == 0 -cc1: note: token 3 has y-location == 2947526575 - 4: 2947526575, 2947526575 -x-location == y-location == 2947526575 encodes token # 800042942 - 5: 2947526575, 2947526575 -x-location == y-location == 2947526575 encodes token # 800042942 - 6: 2947526575, 2947526575 -x-location == y-location == 2947526575 encodes token # 800042942 - -MACRO 0: PLUS (7 tokens) - location_t interval: 2147483640 <= loc < 2147483647 -test.c:6:11: note: expansion point is location 190019 - int a = PLUS (1,2); - ^~~~ - - map->start_location: 2147483640 + 0: 1, 1 +<built-in>: note: token 0 has x-location == y-location == 1 + +MACRO 0: __GCC_IEC_559 (1 tokens) + location_t interval: 2147483647 <= loc < 2147483648 +/usr/include/stdc-predef.h:37:6: note: expansion point is location 147788 + 37 | # if __GCC_IEC_559 > 0 + | ^~~~~~~~~~~~~ + map->start_location: 2147483647 macro_locations: - 0: 190208, 173088 -test.c:6:17: note: token 0 has x-location == 190208 - int a = PLUS (1,2); - ^ - -test.c:6:17: note: token 0 has y-location == 173088 - 1: 173152, 173152 -In file included from test.c:1:0: -test.h:3:22: note: token 1 has x-location == y-location == 173152 - #define PLUS(A, B) A + B - ^ - - 2: 190272, 173216 -test.c:6:19: note: token 2 has x-location == 190272 - int a = PLUS (1,2); - ^ - -test.c:6:19: note: token 2 has y-location == 173216 - 3: 0, 2947526575 -cc1: note: token 3 has x-location == 0 -cc1: note: token 3 has y-location == 2947526575 - 4: 2947526575, 2947526575 -x-location == y-location == 2947526575 encodes token # 800042935 - 5: 2947526575, 2947526575 -x-location == y-location == 2947526575 encodes token # 800042935 - 6: 2947526575, 2947526575 -x-location == y-location == 2947526575 encodes token # 800042935 + 0: 1, 1 +<built-in>: note: token 0 has x-location == y-location == 1 MAX_LOCATION_T location_t interval: 2147483647 <= loc < 2147483648 -- 2.19.1 ^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v3 1/3] PR preprocessor/83173: Additional check before decrementing highest_location 2018-11-01 15:58 ` [PATCH v3 0/3] PR preprocessor/83173: C preprocessor generates incorrect linemarkers Mike Gulick 2018-11-01 15:58 ` [PATCH v3 3/3] PR preprocessor/83173: Enhance -fdump-internal-locations output Mike Gulick @ 2018-11-01 15:58 ` Mike Gulick 2018-11-02 21:13 ` David Malcolm 2018-11-01 15:58 ` [PATCH v3 2/3] PR preprocessor/83173: New test Mike Gulick 2 siblings, 1 reply; 34+ messages in thread From: Mike Gulick @ 2018-11-01 15:58 UTC (permalink / raw) To: gcc-patches; +Cc: David Malcolm, Mike Gulick 2018-10-31 Mike Gulick <mgulick@mathworks.com> PR preprocessor/83173 * libcpp/files.c (_cpp_stack_include): Check if line_table->highest_location is past current line before decrementing. --- libcpp/files.c | 32 +++++++++++++++++++++++--------- 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/libcpp/files.c b/libcpp/files.c index 08b7c647c91..c0165fe64e4 100644 --- a/libcpp/files.c +++ b/libcpp/files.c @@ -1012,6 +1012,7 @@ _cpp_stack_include (cpp_reader *pfile, const char *fname, int angle_brackets, struct cpp_dir *dir; _cpp_file *file; bool stacked; + bool decremented = false; /* For -include command-line flags we have type == IT_CMDLINE. When the first -include file is processed we have the case, where @@ -1035,20 +1036,33 @@ _cpp_stack_include (cpp_reader *pfile, const char *fname, int angle_brackets, return false; /* 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. */ + _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. In the case that the #include is the last + line in the file, highest_location still points to the current + line, not the start of the next line, so we do not decrement in + this case. See plugin/location-overflow-test-pr83173.h for an + example. */ if (file->pchname == NULL && file->err_no == 0 && type != IT_CMDLINE && type != IT_DEFAULT) - pfile->line_table->highest_location--; + { + int highest_line = linemap_get_expansion_line (pfile->line_table, + pfile->line_table->highest_location); + int source_line = linemap_get_expansion_line (pfile->line_table, loc); + if (highest_line > source_line) + { + pfile->line_table->highest_location--; + decremented = true; + } + } stacked = _cpp_stack_file (pfile, file, type == IT_IMPORT, loc); - if (!stacked) + if (decremented && !stacked) /* _cpp_stack_file didn't stack the file, so let's rollback the compensation dance we performed above. */ pfile->line_table->highest_location++; -- 2.19.1 ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 1/3] PR preprocessor/83173: Additional check before decrementing highest_location 2018-11-01 15:58 ` [PATCH v3 1/3] PR preprocessor/83173: Additional check before decrementing highest_location Mike Gulick @ 2018-11-02 21:13 ` David Malcolm 2018-11-02 21:34 ` Mike Gulick 0 siblings, 1 reply; 34+ messages in thread From: David Malcolm @ 2018-11-02 21:13 UTC (permalink / raw) To: Mike Gulick, gcc-patches On Thu, 2018-11-01 at 11:56 -0400, Mike Gulick wrote: > 2018-10-31 Mike Gulick <mgulick@mathworks.com> > > PR preprocessor/83173 > * libcpp/files.c (_cpp_stack_include): Check if > line_table->highest_location is past current line before > decrementing. > --- > libcpp/files.c | 32 +++++++++++++++++++++++--------- > 1 file changed, 23 insertions(+), 9 deletions(-) > > diff --git a/libcpp/files.c b/libcpp/files.c > index 08b7c647c91..c0165fe64e4 100644 > --- a/libcpp/files.c > +++ b/libcpp/files.c > @@ -1012,6 +1012,7 @@ _cpp_stack_include (cpp_reader *pfile, const > char *fname, int angle_brackets, > struct cpp_dir *dir; > _cpp_file *file; > bool stacked; > + bool decremented = false; > > /* For -include command-line flags we have type == IT_CMDLINE. > When the first -include file is processed we have the case, > where > @@ -1035,20 +1036,33 @@ _cpp_stack_include (cpp_reader *pfile, const > char *fname, int angle_brackets, > return false; > > /* 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. */ > + _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. In the case that the #include is the > last > + line in the file, highest_location still points to the current > + line, not the start of the next line, so we do not decrement in > + this case. See plugin/location-overflow-test-pr83173.h for an > + example. */ > if (file->pchname == NULL && file->err_no == 0 > && type != IT_CMDLINE && type != IT_DEFAULT) > - pfile->line_table->highest_location--; > + { > + int highest_line = linemap_get_expansion_line (pfile- > >line_table, > + pfile- > >line_table->highest_location); > + int source_line = linemap_get_expansion_line (pfile- > >line_table, loc); > + if (highest_line > source_line) > + { > + pfile->line_table->highest_location--; > + decremented = true; > + } > + } > > stacked = _cpp_stack_file (pfile, file, type == IT_IMPORT, loc); > > - if (!stacked) > + if (decremented && !stacked) > /* _cpp_stack_file didn't stack the file, so let's rollback the > compensation dance we performed above. */ > pfile->line_table->highest_location++; Sorry for the belated response. This is OK for trunk (assuming your contributor paperwork is in place). Thanks Dave ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 1/3] PR preprocessor/83173: Additional check before decrementing highest_location 2018-11-02 21:13 ` David Malcolm @ 2018-11-02 21:34 ` Mike Gulick 2018-11-02 22:50 ` Jeff Law 0 siblings, 1 reply; 34+ messages in thread From: Mike Gulick @ 2018-11-02 21:34 UTC (permalink / raw) To: David Malcolm, gcc-patches On 11/2/18 5:13 PM, David Malcolm wrote: > On Thu, 2018-11-01 at 11:56 -0400, Mike Gulick wrote: >> 2018-10-31 Mike Gulick <mgulick@mathworks.com> >> >> PR preprocessor/83173 >> * libcpp/files.c (_cpp_stack_include): Check if >> line_table->highest_location is past current line before >> decrementing. >> --- >> libcpp/files.c | 32 +++++++++++++++++++++++--------- >> 1 file changed, 23 insertions(+), 9 deletions(-) >> >> diff --git a/libcpp/files.c b/libcpp/files.c >> index 08b7c647c91..c0165fe64e4 100644 >> --- a/libcpp/files.c >> +++ b/libcpp/files.c >> @@ -1012,6 +1012,7 @@ _cpp_stack_include (cpp_reader *pfile, const >> char *fname, int angle_brackets, >> struct cpp_dir *dir; >> _cpp_file *file; >> bool stacked; >> + bool decremented = false; >> >> /* For -include command-line flags we have type == IT_CMDLINE. >> When the first -include file is processed we have the case, >> where >> @@ -1035,20 +1036,33 @@ _cpp_stack_include (cpp_reader *pfile, const >> char *fname, int angle_brackets, >> return false; >> >> /* 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. */ >> + _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. In the case that the #include is the >> last >> + line in the file, highest_location still points to the current >> + line, not the start of the next line, so we do not decrement in >> + this case. See plugin/location-overflow-test-pr83173.h for an >> + example. */ >> if (file->pchname == NULL && file->err_no == 0 >> && type != IT_CMDLINE && type != IT_DEFAULT) >> - pfile->line_table->highest_location--; >> + { >> + int highest_line = linemap_get_expansion_line (pfile- >>> line_table, >> + pfile- >>> line_table->highest_location); >> + int source_line = linemap_get_expansion_line (pfile- >>> line_table, loc); >> + if (highest_line > source_line) >> + { >> + pfile->line_table->highest_location--; >> + decremented = true; >> + } >> + } >> >> stacked = _cpp_stack_file (pfile, file, type == IT_IMPORT, loc); >> >> - if (!stacked) >> + if (decremented && !stacked) >> /* _cpp_stack_file didn't stack the file, so let's rollback the >> compensation dance we performed above. */ >> pfile->line_table->highest_location++; > > Sorry for the belated response. > > This is OK for trunk (assuming your contributor paperwork is in place). > > Thanks > Dave > Thanks Dave. I don't have contributor paperwork in place for gcc. I was under the impressed the request needed to be initiated by a maintainer, but if I can make the request myself let me know and I will do so. I do have an employer copyright disclaimer already approved which includes gcc, so the process should be quick. Thanks, Mike ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 1/3] PR preprocessor/83173: Additional check before decrementing highest_location 2018-11-02 21:34 ` Mike Gulick @ 2018-11-02 22:50 ` Jeff Law 0 siblings, 0 replies; 34+ messages in thread From: Jeff Law @ 2018-11-02 22:50 UTC (permalink / raw) To: Mike Gulick, David Malcolm, gcc-patches On 11/2/18 3:34 PM, Mike Gulick wrote: > On 11/2/18 5:13 PM, David Malcolm wrote: >> On Thu, 2018-11-01 at 11:56 -0400, Mike Gulick wrote: >>> 2018-10-31 Mike Gulick <mgulick@mathworks.com> >>> >>> PR preprocessor/83173 >>> * libcpp/files.c (_cpp_stack_include): Check if >>> line_table->highest_location is past current line before >>> decrementing. >>> --- >>> libcpp/files.c | 32 +++++++++++++++++++++++--------- >>> 1 file changed, 23 insertions(+), 9 deletions(-) >>> >>> diff --git a/libcpp/files.c b/libcpp/files.c >>> index 08b7c647c91..c0165fe64e4 100644 >>> --- a/libcpp/files.c >>> +++ b/libcpp/files.c >>> @@ -1012,6 +1012,7 @@ _cpp_stack_include (cpp_reader *pfile, const >>> char *fname, int angle_brackets, >>> struct cpp_dir *dir; >>> _cpp_file *file; >>> bool stacked; >>> + bool decremented = false; >>> >>> /* For -include command-line flags we have type == IT_CMDLINE. >>> When the first -include file is processed we have the case, >>> where >>> @@ -1035,20 +1036,33 @@ _cpp_stack_include (cpp_reader *pfile, const >>> char *fname, int angle_brackets, >>> return false; >>> >>> /* 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. */ >>> + _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. In the case that the #include is the >>> last >>> + line in the file, highest_location still points to the current >>> + line, not the start of the next line, so we do not decrement in >>> + this case. See plugin/location-overflow-test-pr83173.h for an >>> + example. */ >>> if (file->pchname == NULL && file->err_no == 0 >>> && type != IT_CMDLINE && type != IT_DEFAULT) >>> - pfile->line_table->highest_location--; >>> + { >>> + int highest_line = linemap_get_expansion_line (pfile- >>>> line_table, >>> + pfile- >>>> line_table->highest_location); >>> + int source_line = linemap_get_expansion_line (pfile- >>>> line_table, loc); >>> + if (highest_line > source_line) >>> + { >>> + pfile->line_table->highest_location--; >>> + decremented = true; >>> + } >>> + } >>> >>> stacked = _cpp_stack_file (pfile, file, type == IT_IMPORT, loc); >>> >>> - if (!stacked) >>> + if (decremented && !stacked) >>> /* _cpp_stack_file didn't stack the file, so let's rollback the >>> compensation dance we performed above. */ >>> pfile->line_table->highest_location++; >> >> Sorry for the belated response. >> >> This is OK for trunk (assuming your contributor paperwork is in place). >> >> Thanks >> Dave >> > Thanks Dave. I don't have contributor paperwork in place for gcc. I was under the impressed the request needed to be initiated by a maintainer, but if I can make the request myself let me know and I will do so. I do have an employer copyright disclaimer already approved which includes gcc, so the process should be quick. Contact assign@gnu.org and indicate to them you need a past and future copyright assignment for GCC. jeff ^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v3 2/3] PR preprocessor/83173: New test 2018-11-01 15:58 ` [PATCH v3 0/3] PR preprocessor/83173: C preprocessor generates incorrect linemarkers Mike Gulick 2018-11-01 15:58 ` [PATCH v3 3/3] PR preprocessor/83173: Enhance -fdump-internal-locations output Mike Gulick 2018-11-01 15:58 ` [PATCH v3 1/3] PR preprocessor/83173: Additional check before decrementing highest_location Mike Gulick @ 2018-11-01 15:58 ` Mike Gulick 2018-11-02 20:52 ` David Malcolm 2 siblings, 1 reply; 34+ messages in thread From: Mike Gulick @ 2018-11-01 15:58 UTC (permalink / raw) To: gcc-patches; +Cc: David Malcolm, Mike Gulick 2018-10-31 Mike Gulick <mgulick@mathworks.com> 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. --- .../plugin/location-overflow-test-pr83173-1.h | 2 ++ .../plugin/location-overflow-test-pr83173-2.h | 2 ++ .../plugin/location-overflow-test-pr83173.c | 21 +++++++++++++++++++ .../plugin/location-overflow-test-pr83173.h | 2 ++ .../gcc.dg/plugin/location_overflow_plugin.c | 13 +++++++----- gcc/testsuite/gcc.dg/plugin/plugin.exp | 3 ++- 6 files changed, 37 insertions(+), 6 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173-1.h create mode 100644 gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173-2.h create mode 100644 gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173.c create mode 100644 gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173.h diff --git a/gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173-1.h b/gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173-1.h new file mode 100644 index 00000000000..f47dc3643c2 --- /dev/null +++ b/gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173-1.h @@ -0,0 +1,2 @@ +#pragma once +#define LOCATION_OVERFLOW_TEST_PR83173_1_H diff --git a/gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173-2.h b/gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173-2.h new file mode 100644 index 00000000000..bc23ed2a188 --- /dev/null +++ b/gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173-2.h @@ -0,0 +1,2 @@ +#pragma once +#define LOCATION_OVERFLOW_TEST_PR83173_2_H diff --git a/gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173.c b/gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173.c new file mode 100644 index 00000000000..2d581283474 --- /dev/null +++ b/gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173.c @@ -0,0 +1,21 @@ +/* + { dg-options "-fplugin-arg-location_overflow_plugin-value=0x60000001" } + { dg-do preprocess } +*/ + +#include "location-overflow-test-pr83173.h" + +int +main () +{ + return 0; +} + +/* + The preprocessor output should contain + '# 1 "path/to/location-overflow-test-pr83173-1.h" 1', but should not + contain any other references to location-overflow-test-pr83183-1.h. + + { dg-final { scan-file location-overflow-test-pr83173.i "# 1 \[^\r\n\]+location-overflow-test-pr83173-1\.h\" 1" } } + { dg-final { scan-file-not location-overflow-test-pr83173.i "# (?!1 \[^\r\n\]+location-overflow-test-pr83173-1\.h\" 1)\[0-9\]+ \[^\r\n\]+location-overflow-test-pr83173-1\.h\"" } } +*/ diff --git a/gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173.h b/gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173.h new file mode 100644 index 00000000000..49076f7ea3c --- /dev/null +++ b/gcc/testsuite/gcc.dg/plugin/location-overflow-test-pr83173.h @@ -0,0 +1,2 @@ +#include "location-overflow-test-pr83173-1.h" +#include "location-overflow-test-pr83173-2.h" diff --git a/gcc/testsuite/gcc.dg/plugin/location_overflow_plugin.c b/gcc/testsuite/gcc.dg/plugin/location_overflow_plugin.c index 3644d9fd82e..317232bc19d 100644 --- a/gcc/testsuite/gcc.dg/plugin/location_overflow_plugin.c +++ b/gcc/testsuite/gcc.dg/plugin/location_overflow_plugin.c @@ -12,11 +12,14 @@ int plugin_is_GPL_compatible; static location_t base_location; -/* Callback handler for the PLUGIN_START_UNIT event; pretend - we parsed a very large include file. */ +/* Callback handler for the PLUGIN_PRAGMAS event; pretend we parsed a + very large include file. This is used to set the initial line table + offset for the preprocessor, to make it appear as if we had parsed a + very large file. PRAGMA_START_UNIT is not suitable here as is not + invoked during the preprocessor stage. */ static void -on_start_unit (void */*gcc_data*/, void */*user_data*/) +on_pragma_registration (void */*gcc_data*/, void */*user_data*/) { /* Act as if we've already parsed a large body of code; so that we can simulate various fallbacks in libcpp: @@ -79,8 +82,8 @@ plugin_init (struct plugin_name_args *plugin_info, error_at (UNKNOWN_LOCATION, "missing plugin argument"); register_callback (plugin_info->base_name, - PLUGIN_START_UNIT, - on_start_unit, + PLUGIN_PRAGMAS, + on_pragma_registration, NULL); /* void *user_data */ /* Hack in additional testing, based on the exact value supplied. */ diff --git a/gcc/testsuite/gcc.dg/plugin/plugin.exp b/gcc/testsuite/gcc.dg/plugin/plugin.exp index d92ede79c0d..f9e89c48140 100644 --- a/gcc/testsuite/gcc.dg/plugin/plugin.exp +++ b/gcc/testsuite/gcc.dg/plugin/plugin.exp @@ -96,7 +96,8 @@ set plugin_test_list [list \ diagnostic-test-inlining-4.c } \ { location_overflow_plugin.c \ location-overflow-test-1.c \ - location-overflow-test-2.c } \ + location-overflow-test-2.c \ + location-overflow-test-pr83173.c } \ { must_tail_call_plugin.c \ must-tail-call-1.c \ must-tail-call-2.c } \ -- 2.19.1 ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v3 2/3] PR preprocessor/83173: New test 2018-11-01 15:58 ` [PATCH v3 2/3] PR preprocessor/83173: New test Mike Gulick @ 2018-11-02 20:52 ` David Malcolm 0 siblings, 0 replies; 34+ messages in thread From: David Malcolm @ 2018-11-02 20:52 UTC (permalink / raw) To: Mike Gulick, gcc-patches On Thu, 2018-11-01 at 11:56 -0400, Mike Gulick wrote: > 2018-10-31 Mike Gulick <mgulick@mathworks.com> > > 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. (sorry for the belated response on this) This patch is OK once the other parts are approved, and assuming your contributor paperwork is in place. Dave ^ permalink raw reply [flat|nested] 34+ messages in thread
end of thread, other threads:[~2018-11-27 16:18 UTC | newest] Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-12-01 22:57 [RFC][PATCH] PR preprocessor/83173: Additional check before decrementing highest_location Mike Gulick 2018-01-12 23:33 ` David Malcolm 2018-01-14 17:32 ` Mike Gulick 2018-01-24 5:17 ` Mike Gulick 2018-02-09 22:55 ` [RFC][PATCH v2] " Mike Gulick 2018-03-04 19:27 ` Mike Gulick 2018-05-29 15:31 ` Mike Gulick 2018-08-10 14:04 ` Mike Gulick 2018-11-01 15:58 ` [PATCH v3 0/3] PR preprocessor/83173: C preprocessor generates incorrect linemarkers Mike Gulick 2018-11-01 15:58 ` [PATCH v3 3/3] PR preprocessor/83173: Enhance -fdump-internal-locations output Mike Gulick 2018-11-02 21:04 ` David Malcolm 2018-11-12 21:13 ` Mike Gulick 2018-11-13 0:56 ` David Malcolm 2018-11-13 19:40 ` Mike Gulick 2018-11-13 19:55 ` [PATCH v4] " Mike Gulick 2018-11-13 20:13 ` David Malcolm 2018-11-26 22:17 ` Mike Gulick 2018-11-27 1:29 ` David Malcolm 2018-11-27 14:11 ` Mike Gulick 2018-11-27 14:27 ` David Malcolm 2018-11-27 14:37 ` Mike Gulick 2018-11-27 14:42 ` David Malcolm 2018-11-27 14:53 ` [PATCH v5 1/3] PR preprocessor/83173: Additional check before decrementing highest_location Mike Gulick 2018-11-27 14:53 ` [PATCH v5 3/3] PR preprocessor/83173: Enhance -fdump-internal-locations output Mike Gulick 2018-11-27 14:53 ` [PATCH v5 2/3] PR preprocessor/83173: New test Mike Gulick 2018-11-27 16:07 ` [PATCH v5 1/3] PR preprocessor/83173: Additional check before decrementing highest_location David Malcolm 2018-11-27 16:18 ` Mike Gulick 2018-11-26 22:22 ` [PATCH v5] PR preprocessor/83173: Enhance -fdump-internal-locations output Mike Gulick 2018-11-01 15:58 ` [PATCH v3 1/3] PR preprocessor/83173: Additional check before decrementing highest_location Mike Gulick 2018-11-02 21:13 ` David Malcolm 2018-11-02 21:34 ` Mike Gulick 2018-11-02 22:50 ` Jeff Law 2018-11-01 15:58 ` [PATCH v3 2/3] PR preprocessor/83173: New test Mike Gulick 2018-11-02 20:52 ` David Malcolm
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).