diff mbox

[2/2] PR c++/70105: prevent nonsensical underline spew for macro expansions

Message ID 1457548621.9813.125.camel@redhat.com
State New
Headers show

Commit Message

David Malcolm March 9, 2016, 6:37 p.m. UTC
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.

Committed to trunk as r234088.

The new test cases contain lines > 2048 long in order to stress the
new code, hence I had to send the following "by hand" via an attachment, as
"git send-email" informs me that > 998 characters is too long to send
"inline", due to SMTP limits as described by
http://www.ietf.org/rfc/rfc2821.txt.

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.
diff mbox

Patch

Index: gcc/ChangeLog
===================================================================
--- gcc/ChangeLog	(revision 234087)
+++ gcc/ChangeLog	(revision 234088)
@@ -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: gcc/testsuite/gcc.dg/plugin/diagnostic-test-expressions-1.c
===================================================================
--- gcc/testsuite/gcc.dg/plugin/diagnostic-test-expressions-1.c	(revision 234087)
+++ gcc/testsuite/gcc.dg/plugin/diagnostic-test-expressions-1.c	(revision 234088)
@@ -635,3 +635,39 @@ 
                        ^~~~
    { 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: gcc/testsuite/ChangeLog
===================================================================
--- gcc/testsuite/ChangeLog	(revision 234087)
+++ gcc/testsuite/ChangeLog	(revision 234088)
@@ -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):
Index: gcc/testsuite/g++.dg/diagnostic/pr70105.C
===================================================================
--- gcc/testsuite/g++.dg/diagnostic/pr70105.C	(revision 0)
+++ gcc/testsuite/g++.dg/diagnostic/pr70105.C	(revision 234088)
@@ -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: gcc/diagnostic-show-locus.c
===================================================================
--- gcc/diagnostic-show-locus.c	(revision 234087)
+++ gcc/diagnostic-show-locus.c	(revision 234088)
@@ -463,6 +463,76 @@ 
   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 @@ 
   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 @@ 
 	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 @@ 
       /* 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: libcpp/line-map.c
===================================================================
--- libcpp/line-map.c	(revision 234087)
+++ libcpp/line-map.c	(revision 234088)
@@ -54,8 +54,6 @@ 
 						       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
Index: libcpp/include/line-map.h
===================================================================
--- libcpp/include/line-map.h	(revision 234087)
+++ libcpp/include/line-map.h	(revision 234088)
@@ -1066,6 +1066,14 @@ 
 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: libcpp/ChangeLog
===================================================================
--- libcpp/ChangeLog	(revision 234087)
+++ libcpp/ChangeLog	(revision 234088)
@@ -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