public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [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 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   ` [PATCH v3 2/3] PR preprocessor/83173: New test 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
  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

* [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 2/3] PR preprocessor/83173: New test 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
  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

* [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   ` Mike Gulick
  2018-11-02 20:52     ` David Malcolm
  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
  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

* [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 2/3] PR preprocessor/83173: New test 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

* 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

* 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 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

* 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

* [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

* 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 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                             ` Mike Gulick
  2018-11-27 14:53                             ` [PATCH v5 3/3] PR preprocessor/83173: Enhance -fdump-internal-locations output 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

* [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 2/3] PR preprocessor/83173: New test 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                             ` [PATCH v5 2/3] PR preprocessor/83173: New test 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/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

* 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 2/3] PR preprocessor/83173: New test Mike Gulick
  2018-11-27 14:53                             ` [PATCH v5 3/3] PR preprocessor/83173: Enhance -fdump-internal-locations output 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

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 2/3] PR preprocessor/83173: New test Mike Gulick
2018-11-02 20:52     ` David Malcolm
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 2/3] PR preprocessor/83173: New test Mike Gulick
2018-11-27 14:53                             ` [PATCH v5 3/3] PR preprocessor/83173: Enhance -fdump-internal-locations output 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

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).