From b4f3232d6979022a36b4055d7d3aaba693a39938 Mon Sep 17 00:00:00 2001 From: David Malcolm Date: Wed, 9 Mar 2016 18:23:27 +0000 Subject: [PATCH] PR c++/70105: prevent nonsensical underline spew for macro expansions diagnostic_show_locus can sometimes do the wrong thing when handling expressions built up from macros. PR c++/70105 (currently marked as a P3 regression) has an example of a diagnostic where over 500 lines of irrelevant source are printed, and underlined, giving >1000 lines of useless spew to stderr. This patch adds extra sanitization to diagnostic-show-locus.c, so that we only attempt to print underlines and secondary locations if such locations are "sufficiently sane" relative to the primary location of a diagnostic. This "sufficiently sane" condition is implemented by a new helper function compatible_locations_p, which requires such locations to have the same macro expansion hierarchy as the primary location, using linemap_macro_map_loc_unwind_toward_spelling, effectively mimicing the expansion performed by LRK_SPELLING_LOCATION. This may be too strong a condition, but it effectively fixes PR c++/70105, without removing any underlines in my testing. Successfully bootstrapped®rtested in combination with the previous patch on x86_64-pc-linux-gnu; adds 15 new PASS results to g++.sum and 4 new PASS results to gcc.sum. gcc/ChangeLog: PR c/68473 PR c++/70105 * diagnostic-show-locus.c (compatible_locations_p): New function. (layout::layout): Sanitize ranges using compatible_locations_p. gcc/testsuite/ChangeLog: PR c/68473 PR c++/70105 * g++.dg/diagnostic/pr70105.C: New test. * gcc.dg/plugin/diagnostic-test-expressions-1.c (foo): New decl. (test_multiple_ordinary_maps): New test function. libcpp/ChangeLog: PR c/68473 PR c++/70105 * line-map.c (linemap_macro_map_loc_unwind_toward_spelling): Move decl... * include/line-map.h (linemap_macro_map_loc_unwind_toward_spelling): ...here, converting from static to extern. From-SVN: r234088 --- gcc/ChangeLog | 7 ++ gcc/diagnostic-show-locus.c | 89 +++++++++++++++++++++- gcc/testsuite/ChangeLog | 8 ++ gcc/testsuite/g++.dg/diagnostic/pr70105.C | 43 +++++++++++ .../gcc.dg/plugin/diagnostic-test-expressions-1.c | 36 +++++++++ libcpp/ChangeLog | 10 +++ libcpp/include/line-map.h | 8 ++ libcpp/line-map.c | 2 - 8 files changed, 199 insertions(+), 4 deletions(-) create mode 100644 gcc/testsuite/g++.dg/diagnostic/pr70105.C diff --git a/gcc/ChangeLog b/gcc/ChangeLog index ee8e208..d3323a6 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -2,6 +2,13 @@ PR c/68473 PR c++/70105 + * diagnostic-show-locus.c (compatible_locations_p): New function. + (layout::layout): Sanitize ranges using compatible_locations_p. + +2016-03-09 David Malcolm + + PR c/68473 + PR c++/70105 * diagnostic-show-locus.c (layout_range::layout_range): Replace location_range param with three const expanded_locations * and a bool. diff --git a/gcc/diagnostic-show-locus.c b/gcc/diagnostic-show-locus.c index a940f24..f10ade5 100644 --- a/gcc/diagnostic-show-locus.c +++ b/gcc/diagnostic-show-locus.c @@ -463,6 +463,76 @@ get_line_width_without_trailing_whitespace (const char *line, int line_width) return result; } +/* Helper function for layout's ctor, for sanitizing locations relative + to the primary location within a diagnostic. + + Compare LOC_A and LOC_B to see if it makes sense to print underlines + connecting their expanded locations. Doing so is only guaranteed to + make sense if the locations share the same macro expansion "history" + i.e. they can be traced through the same macro expansions, eventually + reaching an ordinary map. + + This may be too strong a condition, but it effectively sanitizes + PR c++/70105, which has an example of printing an expression where the + final location of the expression is in a different macro, which + erroneously was leading to hundreds of lines of irrelevant source + being printed. */ + +static bool +compatible_locations_p (location_t loc_a, location_t loc_b) +{ + if (IS_ADHOC_LOC (loc_a)) + loc_a = get_location_from_adhoc_loc (line_table, loc_a); + if (IS_ADHOC_LOC (loc_b)) + loc_b = get_location_from_adhoc_loc (line_table, loc_b); + + const line_map *map_a = linemap_lookup (line_table, loc_a); + linemap_assert (map_a); + + const line_map *map_b = linemap_lookup (line_table, loc_b); + linemap_assert (map_b); + + /* Are they within the same map? */ + if (map_a == map_b) + { + /* Are both within the same macro expansion? */ + if (linemap_macro_expansion_map_p (map_a)) + { + /* Expand each location towards the spelling location, and + recurse. */ + const line_map_macro *macro_map = linemap_check_macro (map_a); + source_location loc_a_toward_spelling + = linemap_macro_map_loc_unwind_toward_spelling (line_table, + macro_map, + loc_a); + source_location loc_b_toward_spelling + = linemap_macro_map_loc_unwind_toward_spelling (line_table, + macro_map, + loc_b); + return compatible_locations_p (loc_a_toward_spelling, + loc_b_toward_spelling); + } + + /* Otherwise they are within the same ordinary map. */ + return true; + } + else + { + /* Within different maps. */ + + /* If either is within a macro expansion, they are incompatible. */ + if (linemap_macro_expansion_map_p (map_a) + || linemap_macro_expansion_map_p (map_b)) + return false; + + /* Within two different ordinary maps; they are compatible iff they + are in the same file. */ + const line_map_ordinary *ord_map_a = linemap_check_ordinary (map_a); + const line_map_ordinary *ord_map_b = linemap_check_ordinary (map_b); + return ord_map_a->to_file == ord_map_b->to_file; + } +} + /* Implementation of class layout. */ /* Constructor for class layout. @@ -487,6 +557,8 @@ layout::layout (diagnostic_context * context, m_x_offset (0) { rich_location *richloc = diagnostic->richloc; + source_location primary_loc = richloc->get_range (0)->m_loc; + for (unsigned int idx = 0; idx < richloc->get_num_locations (); idx++) { /* This diagnostic printer can only cope with "sufficiently sane" ranges. @@ -514,6 +586,14 @@ layout::layout (diagnostic_context * context, if (caret.file != m_exploc.file) continue; + /* Sanitize the caret location for non-primary ranges. */ + if (m_layout_ranges.length () > 0) + if (loc_range->m_show_caret_p) + if (!compatible_locations_p (loc_range->m_loc, primary_loc)) + /* Discard any non-primary ranges that can't be printed + sanely relative to the primary location. */ + continue; + /* Everything is now known to be in the correct source file, but it may require further sanitization. */ layout_range ri (&start, &finish, loc_range->m_show_caret_p, &caret); @@ -521,8 +601,13 @@ layout::layout (diagnostic_context * context, /* If we have a range that finishes before it starts (perhaps from something built via macro expansion), printing the range is likely to be nonsensical. Also, attempting to do so - breaks assumptions within the printing code (PR c/68473). */ - if (start.line > finish.line) + breaks assumptions within the printing code (PR c/68473). + Similarly, don't attempt to print ranges if one or both ends + of the range aren't sane to print relative to the + primary location (PR c++/70105). */ + if (start.line > finish.line + || !compatible_locations_p (src_range.m_start, primary_loc) + || !compatible_locations_p (src_range.m_finish, primary_loc)) { /* Is this the primary location? */ if (m_layout_ranges.length () == 0) diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index 014ee42..14a2f67 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -2,6 +2,14 @@ PR c/68473 PR c++/70105 + * g++.dg/diagnostic/pr70105.C: New test. + * gcc.dg/plugin/diagnostic-test-expressions-1.c (foo): New decl. + (test_multiple_ordinary_maps): New test function. + +2016-03-09 David Malcolm + + PR c/68473 + PR c++/70105 * gcc.dg/plugin/diagnostic_plugin_show_trees.c (show_tree): Drop range information from call to inform_at_rich_loc. * gcc.dg/plugin/diagnostic_plugin_test_show_locus.c (add_range): diff --git a/gcc/testsuite/g++.dg/diagnostic/pr70105.C b/gcc/testsuite/g++.dg/diagnostic/pr70105.C new file mode 100644 index 0000000..9c9b02c --- /dev/null +++ b/gcc/testsuite/g++.dg/diagnostic/pr70105.C @@ -0,0 +1,43 @@ +// { dg-options "-Wsequence-point -fdiagnostics-show-caret" } + +void *libiberty_concat_ptr; +extern unsigned long concat_length (const char *, ...); +extern char *concat_copy2 (const char *, ...); + +#define ACONCAT(ACONCAT_PARAMS) \ + (libiberty_concat_ptr = (char *) ALLOCA (concat_length ACONCAT_PARAMS + 1), /* { dg-warning "may be undefined" } */ \ + concat_copy2 ACONCAT_PARAMS) + +/* Arbitrary content here. + In PR c++/70105, this was >500 lines of source. + This should not be printed. */ + +# define ALLOCA(x) __builtin_alloca(x) + +int strlen (const char *); +void *get_identifier (const char *); +void *get_identifier_with_length (const char *, int); + +#define GET_IDENTIFIER(STR) \ + (__builtin_constant_p (STR) \ + ? get_identifier_with_length ((STR), strlen (STR)) \ + : get_identifier (STR)) + +void *test(void) +{ + int *i; + return GET_IDENTIFIER (ACONCAT (("foo"))); +} + +/* { dg-begin-multiline-output "" } + (libiberty_concat_ptr = (char *) ALLOCA (concat_length ACONCAT_PARAMS + 1), + ^ + { dg-end-multiline-output "" } */ +/* { dg-begin-multiline-output "" } + ? get_identifier_with_length ((STR), strlen (STR)) \ + ^~~ + { dg-end-multiline-output "" } */ +/* { dg-begin-multiline-output "" } + return GET_IDENTIFIER (ACONCAT (("foo"))); + ^~~~~~~ + { dg-end-multiline-output "" } */ diff --git a/gcc/testsuite/gcc.dg/plugin/diagnostic-test-expressions-1.c b/gcc/testsuite/gcc.dg/plugin/diagnostic-test-expressions-1.c index 97426f6..170060f 100644 --- a/gcc/testsuite/gcc.dg/plugin/diagnostic-test-expressions-1.c +++ b/gcc/testsuite/gcc.dg/plugin/diagnostic-test-expressions-1.c @@ -635,3 +635,39 @@ void test_macro (long double xl) ^~~~ { dg-end-multiline-output "" } */ } + +/* Verify that we can underline expressions that span multiple + ordinary maps. */ + +extern int foo (int, ...); + +void test_multiple_ordinary_maps (void) +{ + /* The expression + foo (0, "very long string...") + below contains a transition between ordinary maps due to a very long + line (>127 "columns", treating tab characters as 1 column). */ + __emit_expression_range (0, foo (0, /* { dg-warning "range" } */ + "0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789")); + +/* { dg-begin-multiline-output "" } + __emit_expression_range (0, foo (0, + ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + "0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789")); + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + { dg-end-multiline-output "" } */ + + /* Another expression that transitions between ordinary maps; this + one due to an ordinary map for a very long line transitioning back to + one for a very short line. The policy in linemap_line_start + means that we need a transition from >10 bits of column + (i.e. 2048 columns) to a line with <= 80 columns. */ + __emit_expression_range (0, foo (0, "01234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789", /* { dg-warning "range" } */ + 0)); +/* { dg-begin-multiline-output "" } + __emit_expression_range (0, foo (0, "01234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789", + ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + 0)); + ~~ + { dg-end-multiline-output "" } */ +} diff --git a/libcpp/ChangeLog b/libcpp/ChangeLog index d583e98..19b9310 100644 --- a/libcpp/ChangeLog +++ b/libcpp/ChangeLog @@ -2,6 +2,16 @@ PR c/68473 PR c++/70105 + * line-map.c (linemap_macro_map_loc_unwind_toward_spelling): Move + decl... + * include/line-map.h + (linemap_macro_map_loc_unwind_toward_spelling): ...here, + converting from static to extern. + +2016-03-09 David Malcolm + + PR c/68473 + PR c++/70105 * include/line-map.h (source_range::debug): Delete. (struct location_range): Update comment. Replace expanded_location fields "m_start", "m_finish", and "m_caret" with diff --git a/libcpp/include/line-map.h b/libcpp/include/line-map.h index 13cc6f8..292abce 100644 --- a/libcpp/include/line-map.h +++ b/libcpp/include/line-map.h @@ -1066,6 +1066,14 @@ int linemap_location_in_system_header_p (struct line_maps *, bool linemap_location_from_macro_expansion_p (const struct line_maps *, source_location); +/* With the precondition that LOCATION is the locus of a token that is + an argument of a function-like macro MACRO_MAP and appears in the + expansion of MACRO_MAP, return the locus of that argument in the + context of the caller of MACRO_MAP. */ + +extern source_location linemap_macro_map_loc_unwind_toward_spelling + (line_maps *set, const line_map_macro *macro_map, source_location location); + /* source_location values from 0 to RESERVED_LOCATION_COUNT-1 will be reserved for libcpp user as special values, no token from libcpp will contain any of those locations. */ diff --git a/libcpp/line-map.c b/libcpp/line-map.c index de6eafc..1fb634a 100644 --- a/libcpp/line-map.c +++ b/libcpp/line-map.c @@ -54,8 +54,6 @@ static const line_map_macro* linemap_macro_map_lookup (struct line_maps *, source_location); static source_location linemap_macro_map_loc_to_def_point (const line_map_macro *, source_location); -static source_location linemap_macro_map_loc_unwind_toward_spelling -(line_maps *set, const line_map_macro *, source_location); static source_location linemap_macro_map_loc_to_exp_point (const line_map_macro *, source_location); static source_location linemap_macro_loc_to_spelling_point -- 2.7.4