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
From a0320fc6e4292a194a8680b26f4951a320fceaf2 Mon Sep 17 00:00:00 2001
From: Mike Gulick <mgul...@mathworks.com>
Date: Thu, 30 Nov 2017 18:35:48 -0500
Subject: [PATCH v2] PR preprocessor/83173: New test
2017-12-01 Mike Gulick <mgul...@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
