PR c++/70105: prevent nonsensical underline spew for macro expansions
authorDavid Malcolm <dmalcolm@redhat.com>
Wed, 9 Mar 2016 18:23:27 +0000 (18:23 +0000)
committerDavid Malcolm <dmalcolm@gcc.gnu.org>
Wed, 9 Mar 2016 18:23:27 +0000 (18:23 +0000)
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&regrtested 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
gcc/diagnostic-show-locus.c
gcc/testsuite/ChangeLog
gcc/testsuite/g++.dg/diagnostic/pr70105.C [new file with mode: 0644]
gcc/testsuite/gcc.dg/plugin/diagnostic-test-expressions-1.c
libcpp/ChangeLog
libcpp/include/line-map.h
libcpp/line-map.c

index ee8e208..d3323a6 100644 (file)
@@ -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  <dmalcolm@redhat.com>
+
+       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.
index a940f24..f10ade5 100644 (file)
@@ -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)
index 014ee42..14a2f67 100644 (file)
@@ -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  <dmalcolm@redhat.com>
+
+       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 (file)
index 0000000..9c9b02c
--- /dev/null
@@ -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 "" } */
index 97426f6..170060f 100644 (file)
@@ -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 "" } */
+}
index d583e98..19b9310 100644 (file)
@@ -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  <dmalcolm@redhat.com>
+
+       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
index 13cc6f8..292abce 100644 (file)
@@ -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.  */
index de6eafc..1fb634a 100644 (file)
@@ -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