[RFC][PATCH] PR preprocessor/83173: Additional check before decrementing highest_location

[Mike Gulick <mgulick at mathworks dot com>]

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

>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

>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

>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