This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[PATCH 2/2] PR preprocessor/69126: avoid comparing ad-hoc and non-ad-hoc locations
- From: David Malcolm <dmalcolm at redhat dot com>
- To: gcc-patches at gcc dot gnu dot org
- Cc: David Malcolm <dmalcolm at redhat dot com>
- Date: Sat, 20 Feb 2016 11:46:23 -0500
- Subject: [PATCH 2/2] PR preprocessor/69126: avoid comparing ad-hoc and non-ad-hoc locations
- Authentication-results: sourceware.org; auth=none
- References: <1455986783-7920-1-git-send-email-dmalcolm at redhat dot com>
Comment #18 of PR preprocessor/69126 reported a difficult-to-reproduce
re-occurrence of that bug, where attempts to suppress
-Wdeprecated-declarations
via a _Pragma could fail.
The root cause is a bug in linemap_compare_locations when comparing
certain macro expansions with certain non-macro expansions.
(gdb) call inform (pre, "pre")
test.cc:8:16: note: pre
#define IGNORE _Pragma("GCC diagnostic ignored \"-Wdeprecated-declarations\"")
^
test.cc:8:16: note: in definition of macro âIGNOREâ
#define IGNORE _Pragma("GCC diagnostic ignored \"-Wdeprecated-declarations\"")
^~~~~~~
(gdb) call inform (post, "post")
test.cc:12:5: note: post
f();
^
After macro expansion, we have (at the end of linemap_compare_locations):
(gdb) p /x l0
$13 = 0x800101ec
(gdb) p /x l1
$14 = 0x50684c05
and hence:
(gdb) p /x l1 - l0
$23 = 0xd0674a19
it's effectively negative, and so "before_p" is false; it's erroneously
treating the _Pragma as if it were *after* the diagnostic (and hence
it doesn't affect it).
But this is wrong: l0 is an ad-hoc loc, whereas l1 is a non-ad-hoc loc.
It's clearly insane to do pure numeric comparisons of ad-hoc
locations with non-ad-hoc locations, since doing so will make the
latter always be "after" the former. The fix is simple: resolve
ad-hoc locations at the end of linemap_compare_locations.
For this bug to occur, we need a location for the macro name that's an
ad-hoc location, and a location for the diagnostic that's *not* an
ad-hoc location.
The reason it triggered for the reporter of comment #18 of the PR is
that the sheer quantity of code in his reproducer meant that both
locations in question were above LINE_MAP_MAX_LOCATION_WITH_PACKED_RANGES
(but below LINE_MAP_MAX_LOCATION_WITH_COLS), so range-packing was
disabled, in particular for the "IGNORE" macro, giving an ad-hoc
location for the _Pragma; in contrast, the diagnostic a single
character, and thus used a non-ad-hoc location.
The attached patch fixes the issue, and adds test coverage, via a pair
of test cases c-c++-common/pr69126-2-{long|short}.c, where the "long"
version of the test case uses a macro name that's >=32 characters (thus
forcing the use of an ad-hoc location), which reproduced the issue.
Given that this adds calls to get_location_from_adhoc_loc to the end
of linemap_compare_locations, I went ahead and removed the hand-inlined
copies from the top of the function (but I can avoid that change if
that's too much for stage 4).
This also fixes the xfail in pr69543-1.c, as
"YY_IGNORE_MAYBE_UNINITIALIZED_BEGIN" is longer than 31 chars and
was thus affected by this.
Successfully bootstrapped®rtested on x86_64-pc-linux-gnu in
combination with the previous patch.
OK for trunk in stage 4?
gcc/testsuite/ChangeLog:
PR preprocessor/69126
PR preprocessor/69543
* c-c++-common/pr69126-2-long.c: New test.
* c-c++-common/pr69126-2-short.c: New test.
* c-c++-common/pr69543-1.c: Remove xfail.
libcpp/ChangeLog:
PR preprocessor/69126
PR preprocessor/69543
* line-map.c (linemap_compare_locations): At the function top,
replace inlined bodies of get_location_from_adhoc_loc with calls
to get_location_from_adhoc_loc. Add a pair of calls to
get_location_from_adhoc_loc at the bottom of the function, to
avoid meaningless comparisons of ad-hoc and non-ad-hoc locations.
---
gcc/testsuite/c-c++-common/pr69126-2-long.c | 11 +++++++++++
gcc/testsuite/c-c++-common/pr69126-2-short.c | 11 +++++++++++
gcc/testsuite/c-c++-common/pr69543-1.c | 4 +---
libcpp/line-map.c | 9 +++++++--
4 files changed, 30 insertions(+), 5 deletions(-)
create mode 100644 gcc/testsuite/c-c++-common/pr69126-2-long.c
create mode 100644 gcc/testsuite/c-c++-common/pr69126-2-short.c
diff --git a/gcc/testsuite/c-c++-common/pr69126-2-long.c b/gcc/testsuite/c-c++-common/pr69126-2-long.c
new file mode 100644
index 0000000..f4f1964
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/pr69126-2-long.c
@@ -0,0 +1,11 @@
+/* { dg-options "-Wdeprecated-declarations" } */
+
+/* The macro's name is >= 32 characters long, and hence its location
+ requires an ad-hoc location. */
+
+#define IGNORE_WHERE_MACRO_IS_LONGER_THAN_31_CHARS _Pragma("GCC diagnostic ignored \"-Wdeprecated-declarations\"")
+__attribute__((deprecated)) void f();
+int main() {
+ IGNORE_WHERE_MACRO_IS_LONGER_THAN_31_CHARS
+ f();
+}
diff --git a/gcc/testsuite/c-c++-common/pr69126-2-short.c b/gcc/testsuite/c-c++-common/pr69126-2-short.c
new file mode 100644
index 0000000..aee43e5
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/pr69126-2-short.c
@@ -0,0 +1,11 @@
+/* { dg-options "-Wdeprecated-declarations" } */
+
+/* IGNORE_SHORT_MACRO is < 32 characters long, and hence its location
+ can be stored without needing an ad-hoc location. */
+
+#define IGNORE_SHORT_MACRO _Pragma("GCC diagnostic ignored \"-Wdeprecated-declarations\"")
+__attribute__((deprecated)) void f();
+int main() {
+ IGNORE_SHORT_MACRO
+ f();
+}
diff --git a/gcc/testsuite/c-c++-common/pr69543-1.c b/gcc/testsuite/c-c++-common/pr69543-1.c
index bfb5270..bbf4759 100644
--- a/gcc/testsuite/c-c++-common/pr69543-1.c
+++ b/gcc/testsuite/c-c++-common/pr69543-1.c
@@ -3,8 +3,6 @@
/* Verify disabling a warning, where the _Pragma is within
a macro, but the affected code is *not* in a macro. */
-/* TODO: XFAIL: why does g++ still emit a warning here? (works for C). */
-
# define YY_IGNORE_MAYBE_UNINITIALIZED_BEGIN \
_Pragma ("GCC diagnostic push") \
_Pragma ("GCC diagnostic ignored \"-Wuninitialized\"")\
@@ -16,6 +14,6 @@ void test (char yylval)
{
char *yyvsp;
YY_IGNORE_MAYBE_UNINITIALIZED_BEGIN
- *++yyvsp = yylval; /* { dg-bogus "used uninitialized" "" { xfail { c++ } } } */
+ *++yyvsp = yylval;
YY_IGNORE_MAYBE_UNINITIALIZED_END
}
diff --git a/libcpp/line-map.c b/libcpp/line-map.c
index e9175df..c05a001 100644
--- a/libcpp/line-map.c
+++ b/libcpp/line-map.c
@@ -1328,9 +1328,9 @@ linemap_compare_locations (struct line_maps *set,
source_location l0 = pre, l1 = post;
if (IS_ADHOC_LOC (l0))
- l0 = set->location_adhoc_data_map.data[l0 & MAX_SOURCE_LOCATION].locus;
+ l0 = get_location_from_adhoc_loc (set, l0);
if (IS_ADHOC_LOC (l1))
- l1 = set->location_adhoc_data_map.data[l1 & MAX_SOURCE_LOCATION].locus;
+ l1 = get_location_from_adhoc_loc (set, l1);
if (l0 == l1)
return 0;
@@ -1365,6 +1365,11 @@ linemap_compare_locations (struct line_maps *set,
return i1 - i0;
}
+ if (IS_ADHOC_LOC (l0))
+ l0 = get_location_from_adhoc_loc (set, l0);
+ if (IS_ADHOC_LOC (l1))
+ l1 = get_location_from_adhoc_loc (set, l1);
+
return l1 - l0;
}
--
1.8.5.3