From ed1c0a1c6fb2d0250e60b9ab69562c28d7b2b5b4 Mon Sep 17 00:00:00 2001
From: David Malcolm <dmalcolm@redhat.com>
Date: Tue, 1 Sep 2015 09:10:32 -0400
Subject: [PATCH 28/56] FIXME: WIP on c-family/c-format.c to use string literal
 location info

with:
  * FIXME: combine the format/ diagnostic-range tests into one
  * FIXME: Eliminate usage of weird gcc_rich_location ctor from c-format.c
  * FIXME: c-format: use cpp_string_location::get_range_between_indices
  * FIXME: WIP on showing argument range in c-format.c
  * FIXME: WIP on c-format captions
    (Disabled for now; they look too "busy", and captions don't work
    well enough)
---
 gcc/c-family/c-format.c                         | 144 +++++++++++++++++-------
 gcc/gcc-rich-location.c                         |  17 +++
 gcc/gcc-rich-location.h                         |   5 +
 gcc/testsuite/gcc.dg/format/diagnostic-ranges.c | 101 +++++++++++++++++
 4 files changed, 227 insertions(+), 40 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/format/diagnostic-ranges.c

diff --git a/gcc/c-family/c-format.c b/gcc/c-family/c-format.c
index ab58076..2f5a350 100644
--- a/gcc/c-family/c-format.c
+++ b/gcc/c-family/c-format.c
@@ -33,6 +33,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "c-format.h"
 #include "alloc-pool.h"
 #include "c-target.h"
+#include "gcc-rich-location.h"
 
 /* Handle attributes associated with format checking.  */
 
@@ -1023,6 +1024,7 @@ static void check_format_info (function_format_info *, tree);
 static void check_format_arg (void *, tree, unsigned HOST_WIDE_INT);
 static void check_format_info_main (format_check_results *,
 				    function_format_info *,
+				    tree,
 				    const char *, int, tree,
 				    unsigned HOST_WIDE_INT,
 				    object_allocator<format_wanted_type> &);
@@ -1036,8 +1038,12 @@ static void finish_dollar_format_checking (format_check_results *, int);
 static const format_flag_spec *get_flag_spec (const format_flag_spec *,
 					      int, const char *);
 
-static void check_format_types (location_t, format_wanted_type *);
-static void format_type_warning (location_t, format_wanted_type *, tree, tree);
+static void check_format_types (source_range fmt_source_range,
+				format_wanted_type *);
+static void format_type_warning (source_range fmt_source_range,
+				 source_range *param_range,
+				 format_wanted_type *, tree,
+				 tree);
 
 /* Decode a format type from a string, returning the type, or
    format_type_error if not valid, in which case the caller should print an
@@ -1689,12 +1695,14 @@ check_format_arg (void *ctx, tree format_tree,
   res->number_other++;
   object_allocator <format_wanted_type> fwt_pool ("format_wanted_type pool",
 						  10);
-  check_format_info_main (res, info, format_chars, format_length,
+  check_format_info_main (res, info, format_tree, format_chars, format_length,
 			  params, arg_num, fwt_pool);
 }
 
 
-/* Do the main part of checking a call to a format function.  FORMAT_CHARS
+/* Do the main part of checking a call to a format function.
+   FORMAT_STRING_CST is the STRING_CST format string.
+   FORMAT_CHARS
    is the NUL-terminated format string (which at this point may contain
    internal NUL characters); FORMAT_LENGTH is its length (excluding the
    terminating NUL character).  ARG_NUM is one less than the number of
@@ -1703,7 +1711,9 @@ check_format_arg (void *ctx, tree format_tree,
 
 static void
 check_format_info_main (format_check_results *res,
-			function_format_info *info, const char *format_chars,
+			function_format_info *info,
+			tree format_string_cst,
+			const char *format_chars,
 			int format_length, tree params,
 			unsigned HOST_WIDE_INT arg_num,
 			object_allocator <format_wanted_type> &fwt_pool)
@@ -1722,6 +1732,8 @@ check_format_info_main (format_check_results *res,
 
   init_dollar_format_checking (info->first_arg_num, first_fillin_param);
 
+  //const char *start_of_format_string = format_chars;
+
   while (*format_chars != 0)
     {
       int i;
@@ -1752,10 +1764,20 @@ check_format_info_main (format_check_results *res,
 	continue;
       if (*format_chars == 0)
 	{
+#if 1
           warning_at (location_from_offset (format_string_loc,
 					    format_chars - orig_format_chars),
 		      OPT_Wformat_,
 		      "spurious trailing %<%%%> in format");
+#else
+	  ptrdiff_t offset = (format_chars - 1) - start_of_format_string;
+	  gcc_rich_location richloc (format_string_cst,
+				     offset,
+				     offset,
+				     offset);
+	  warning_at_rich_loc (&richloc, OPT_Wformat_,
+			       "spurious trailing %<%%%> in format");
+#endif
 	  continue;
 	}
       if (*format_chars == '%')
@@ -1763,6 +1785,7 @@ check_format_info_main (format_check_results *res,
 	  ++format_chars;
 	  continue;
 	}
+      const char *start_of_this_format = format_chars;
       flag_chars[0] = 0;
 
       if ((fki->flags & (int) FMT_FLAG_USE_DOLLAR) && has_operand_number != 0)
@@ -2426,7 +2449,17 @@ check_format_info_main (format_check_results *res,
 	}
 
       if (first_wanted_type != 0)
-        check_format_types (format_string_loc, first_wanted_type);
+	{
+	  ptrdiff_t offset_to_format_start = (start_of_this_format - 1) - orig_format_chars;
+	  ptrdiff_t offset_to_format_end = (format_chars - 1) - orig_format_chars;
+	  cpp_string_location *strloc
+	    = TREE_STRING_LOCATION (format_string_cst);
+	  gcc_assert (strloc);
+	  source_range fmt_source_range
+	    = strloc->get_range_between_indices (offset_to_format_start,
+						 offset_to_format_end);
+	  check_format_types (fmt_source_range, first_wanted_type);
+	}
     }
 
   if (format_chars - orig_format_chars != format_length)
@@ -2446,10 +2479,11 @@ check_format_info_main (format_check_results *res,
 
 
 /* Check the argument types from a single format conversion (possibly
-   including width and precision arguments).  LOC is the location of
-   the format string.  */
+   including width and precision arguments).  FMT_SOURCE_RANGE is the
+   location of the format string.  */
 static void
-check_format_types (location_t loc, format_wanted_type *types)
+check_format_types (source_range fmt_source_range,
+		    format_wanted_type *types)
 {
   for (; types != 0; types = types->next)
     {
@@ -2476,7 +2510,7 @@ check_format_types (location_t loc, format_wanted_type *types)
       cur_param = types->param;
       if (!cur_param)
         {
-          format_type_warning (loc, types, wanted_type, NULL);
+          format_type_warning (fmt_source_range, NULL, types, wanted_type, NULL);
           continue;
         }
 
@@ -2486,6 +2520,7 @@ check_format_types (location_t loc, format_wanted_type *types)
       orig_cur_type = cur_type;
       char_type_flag = 0;
 
+      source_range param_range = EXPR_LOCATION_RANGE (cur_param);
       STRIP_NOPS (cur_param);
 
       /* Check the types of any additional pointer arguments
@@ -2550,7 +2585,8 @@ check_format_types (location_t loc, format_wanted_type *types)
 	    }
 	  else
 	    {
-              format_type_warning (loc, types, wanted_type, orig_cur_type);
+              format_type_warning (fmt_source_range, &param_range, types,
+				   wanted_type, orig_cur_type);
 	      break;
 	    }
 	}
@@ -2618,12 +2654,14 @@ check_format_types (location_t loc, format_wanted_type *types)
 	  && TYPE_PRECISION (cur_type) == TYPE_PRECISION (wanted_type))
 	continue;
       /* Now we have a type mismatch.  */
-      format_type_warning (loc, types, wanted_type, orig_cur_type);
+      format_type_warning (fmt_source_range, &param_range, types, wanted_type,
+			   orig_cur_type);
     }
 }
 
 
-/* Give a warning at LOC about a format argument of different type from that
+/* FIXME: update this comment.
+   Give a warning at LOC about a format argument of different type from that
    expected.  WANTED_TYPE is the type the argument should have, possibly
    stripped of pointer dereferences.  The description (such as "field
    precision"), the placement in the format string, a possibly more
@@ -2631,7 +2669,9 @@ check_format_types (location_t loc, format_wanted_type *types)
    are taken from TYPE.  ARG_TYPE is the type of the actual argument,
    or NULL if it is missing.  */
 static void
-format_type_warning (location_t loc, format_wanted_type *type,
+format_type_warning (source_range fmt_source_range,
+		     source_range *param_range,
+		     format_wanted_type *type,
 		     tree wanted_type, tree arg_type)
 {
   int kind = type->kind;
@@ -2640,9 +2680,12 @@ format_type_warning (location_t loc, format_wanted_type *type,
   int format_length = type->format_length;
   int pointer_count = type->pointer_count;
   int arg_num = type->arg_num;
-  unsigned int offset_loc = type->offset_loc;
+  //unsigned int offset_loc = type->offset_loc;
 
   char *p;
+
+  gcc_rich_location richloc (fmt_source_range);
+
   /* If ARG_TYPE is a typedef with a misleading name (for example,
      size_t but not the standard size_t expected by printf %zu), avoid
      printing the typedef name.  */
@@ -2674,41 +2717,62 @@ format_type_warning (location_t loc, format_wanted_type *type,
       p[pointer_count + 1] = 0;
     }
 
+#if 0
   loc = location_from_offset (loc, offset_loc);
-		      
+#endif
+
+#if 0
+  if (wanted_type_name)
+    richloc.set_caption (global_dc,
+			 "%<%s%>", wanted_type_name);
+  else
+    richloc.set_caption (global_dc,
+			 "%qT", wanted_type);
+#endif
+
+  if (param_range)
+#if 0
+    richloc.add_range_with_caption (param_range->m_start,
+				    param_range->m_finish,
+				    global_dc,
+				    "%qT", arg_type);
+#else
+  richloc.add_range (*param_range);
+#endif
+
   if (wanted_type_name)
     {
       if (arg_type)
-        warning_at (loc, OPT_Wformat_,
-		    "%s %<%s%.*s%> expects argument of type %<%s%s%>, "
-		    "but argument %d has type %qT",
-		    gettext (kind_descriptions[kind]),
-		    (kind == CF_KIND_FORMAT ? "%" : ""),
-		    format_length, format_start, 
-		    wanted_type_name, p, arg_num, arg_type);
+	warning_at_rich_loc (&richloc, OPT_Wformat_,
+			     "%s %<%s%.*s%> expects argument of type %<%s%s%>, "
+			     "but argument %d has type %qT",
+			     gettext (kind_descriptions[kind]),
+			     (kind == CF_KIND_FORMAT ? "%" : ""),
+			     format_length, format_start,
+			     wanted_type_name, p, arg_num, arg_type);
       else
-        warning_at (loc, OPT_Wformat_,
-		    "%s %<%s%.*s%> expects a matching %<%s%s%> argument",
-		    gettext (kind_descriptions[kind]),
-		    (kind == CF_KIND_FORMAT ? "%" : ""),
-		    format_length, format_start, wanted_type_name, p);
+	warning_at_rich_loc (&richloc, OPT_Wformat_,
+			     "%s %<%s%.*s%> expects a matching %<%s%s%> argument",
+			     gettext (kind_descriptions[kind]),
+			     (kind == CF_KIND_FORMAT ? "%" : ""),
+			     format_length, format_start, wanted_type_name, p);
     }
   else
     {
       if (arg_type)
-        warning_at (loc, OPT_Wformat_,
-		    "%s %<%s%.*s%> expects argument of type %<%T%s%>, "
-		    "but argument %d has type %qT",
-		    gettext (kind_descriptions[kind]),
-		    (kind == CF_KIND_FORMAT ? "%" : ""),
-		    format_length, format_start, 
-		    wanted_type, p, arg_num, arg_type);
+	warning_at_rich_loc (&richloc, OPT_Wformat_,
+			     "%s %<%s%.*s%> expects argument of type %<%T%s%>, "
+			     "but argument %d has type %qT",
+			     gettext (kind_descriptions[kind]),
+			     (kind == CF_KIND_FORMAT ? "%" : ""),
+			     format_length, format_start,
+			     wanted_type, p, arg_num, arg_type);
       else
-        warning_at (loc, OPT_Wformat_,
-		    "%s %<%s%.*s%> expects a matching %<%T%s%> argument",
-		    gettext (kind_descriptions[kind]),
-		    (kind == CF_KIND_FORMAT ? "%" : ""),
-		    format_length, format_start, wanted_type, p);
+	warning_at_rich_loc (&richloc, OPT_Wformat_,
+			     "%s %<%s%.*s%> expects a matching %<%T%s%> argument",
+			     gettext (kind_descriptions[kind]),
+			     (kind == CF_KIND_FORMAT ? "%" : ""),
+			     format_length, format_start, wanted_type, p);
     }
 }
 
diff --git a/gcc/gcc-rich-location.c b/gcc/gcc-rich-location.c
index 2fcbcbf..48ecf4e 100644
--- a/gcc/gcc-rich-location.c
+++ b/gcc/gcc-rich-location.c
@@ -67,6 +67,23 @@ get_range_for_expr (tree expr, location_range *r)
    by translating and formatting GMSGID and any variadic args.  */
 
 void
+gcc_rich_location::set_caption (diagnostic_context *context,
+				const char *gmsgid, ...)
+{
+  gcc_assert (context);
+  gcc_assert (gmsgid);
+  gcc_assert (m_num_ranges > 0);
+
+  va_list ap;
+  va_start (ap, gmsgid);
+
+  free (m_ranges[0].m_caption);
+  m_ranges[0].m_caption = expand_caption_va (context, gmsgid, &ap);
+
+  va_end (ap);
+}
+
+void
 gcc_rich_location::add_range_with_caption (location_t start, location_t finish,
 					   diagnostic_context *context,
 					   const char *gmsgid, ...)
diff --git a/gcc/gcc-rich-location.h b/gcc/gcc-rich-location.h
index a7822ac..4a07bb9 100644
--- a/gcc/gcc-rich-location.h
+++ b/gcc/gcc-rich-location.h
@@ -39,6 +39,11 @@ class gcc_rich_location : public rich_location
   /* Methods for adding additional details.  */
 
   void
+  set_caption (diagnostic_context *context,
+	       const char *gmsgid, ...)
+    ATTRIBUTE_GCC_DIAG(3,4);
+
+  void
   add_range_with_caption (location_t start, location_t finish,
 			  diagnostic_context *context,
 			  const char *gmsgid, ...)
diff --git a/gcc/testsuite/gcc.dg/format/diagnostic-ranges.c b/gcc/testsuite/gcc.dg/format/diagnostic-ranges.c
new file mode 100644
index 0000000..f890f6a
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/format/diagnostic-ranges.c
@@ -0,0 +1,101 @@
+/* { dg-options "-Wformat -fdiagnostics-show-caret" } */
+
+/* See PR 52952. */
+
+extern int printf (__const char *__restrict __format, ...);
+
+void test_mismatching_types (const char *msg)
+{
+  printf("hello %i", msg);  /* { dg-warning "format '%i' expects argument of type 'int', but argument 2 has type 'const char \\*' " } */
+
+/* { dg-begin-multiline-output "" }
+   printf("hello %i", msg);
+                 ^~   ~~~
+   { dg-end-multiline-output "" } */
+}
+
+void test_multiple_arguments (void)
+{
+  printf ("arg0: %i  arg1: %s arg 2: %i", /* { dg-warning "format '%s'" } */
+          100, 101, 102);
+/* { dg-begin-multiline-output "" }
+   printf ("arg0: %i  arg1: %s arg 2: %i",
+                            ^~
+           100, 101, 102);
+                ~~~          
+   { dg-end-multiline-output "" } */
+/* FIXME: why the trailing whitespace in the line above?  */
+}
+
+void multiline_format_string (void) {
+  printf ("before the fmt specifier"
+          "%"  /* { dg-warning "format '%d' expects a matching 'int' argument" } */
+          "d"
+          "after the fmt specifier");
+
+/* { dg-begin-multiline-output "" }
+           "%"
+            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+           "d"
+           ~~
+   { dg-end-multiline-output "" } */
+}
+
+void test_hex (const char *msg)
+{
+  /* "%" is \x25
+     "i" is \x69 */
+  printf("hello \x25\x69", msg);  /* { dg-warning "format '%i' expects argument of type 'int', but argument 2 has type 'const char \\*' " } */
+
+/* { dg-begin-multiline-output "" }
+   printf("hello \x25\x69", msg);
+                 ^~~~~~~~   ~~~
+   { dg-end-multiline-output "" } */
+}
+
+void test_oct (const char *msg)
+{
+  /* "%" is octal 045
+     "i" is octal 151.  */
+  printf("hello \045\151", msg);  /* { dg-warning "format '%i' expects argument of type 'int', but argument 2 has type 'const char \\*' " } */
+
+/* { dg-begin-multiline-output "" }
+   printf("hello \045\151", msg);
+                 ^~~~~~~~   ~~~
+   { dg-end-multiline-output "" } */
+}
+
+void test_multiple (const char *msg)
+{
+  /* "%" is \x25 in hex
+     "i" is \151 in octal.  */
+  printf("prefix"  "\x25"  "\151"  "suffix",  /* { dg-warning "format '%i'" } */
+         msg);
+
+/* { dg-begin-multiline-output "" }
+   printf("prefix"  "\x25"  "\151"  "suffix",
+                     ^~~~~~~~~~~~
+          msg);
+          ~~~         
+  { dg-end-multiline-output "" } */
+/* FIXME: why the trailing whitespace in the line above?  */
+}
+
+void test_u8 (const char *msg)
+{
+  printf(u8"hello %i", msg);/* { dg-warning "format '%i' expects argument of type 'int', but argument 2 has type 'const char \\*' " } */
+/* { dg-begin-multiline-output "" }
+   printf(u8"hello %i", msg);
+                   ^~   ~~~
+   { dg-end-multiline-output "" } */
+}
+
+void test_spurious_percent (void)
+{
+  printf("hello world %"); /* { dg-warning "spurious trailing" } */
+
+/* { dg-begin-multiline-output "" }
+   printf("hello world %");
+                       ^
+   { dg-end-multiline-output "" } */
+}
-- 
1.8.5.3

