[5/5] gimple-ssa-sprintf.c: rewrite overflow/truncation diagnostic (PR middle-end/77696)

Message ID 1536949038-34114-6-git-send-email-dmalcolm@redhat.com
State New
Headers show
Series
  • RFC: gimple-ssa-sprintf.c: a new approach (PR middle-end/77696)
Related show

Commit Message

David Malcolm Sept. 14, 2018, 6:17 p.m.
gcc/ChangeLog:
	PR middle-end/77696
	* gimple-ssa-sprintf.c: Include "gcc-rich-location.h".
	(struct directive_state): New struct.
	(directive_state::get_text): New function.
	(format_result::add_directive): New member function.
	(format_result::should_warn): New field.
	(format_result::m_directives): New field.
	(fmtwarn_n): Delete.
	(maybe_warn): Delete.
	(format_directive): Add param "is_nul_terminator".  Rename locals
	"start" and "length" to "start_idx" and "end_idx" respectively.
	Call res->add_directive.  Eliminate call to maybe_warn in favor of
	calling should_warn_p and updating res->should_warn.
	Eliminate notes.
	(class rich_format_location): New class.
	(rich_format_location::flyweight_range_label::get_text): New
	function.
	(rich_format_location::get_label_for_buffer): New function.
	(rich_format_location::get_label_for_directive): New function.
	(sprintf_dom_walker::compute_format_length): Add param "dst_ptr".
	Retain param "callloc".  Initialize res->should_warn.  Add local
	"is_nul_terminator" and pass to format_directive.  Rather than
	returning at the end of the format string, check for res->should_warn
	and emit warnings using rich_format_location.
	(sprintf_dom_walker::handle_gimple_call): Pass in dst_ptr to
	compute_format_length.

gcc/testsuite/ChangeLog:
	PR middle-end/77696
	* gcc.dg/sprintf-diagnostics-1.c: New test.
	* gcc.dg/sprintf-diagnostics-2.c: New test.
---
 gcc/gimple-ssa-sprintf.c                     | 646 ++++++++++++---------------
 gcc/testsuite/gcc.dg/sprintf-diagnostics-1.c | 252 +++++++++++
 gcc/testsuite/gcc.dg/sprintf-diagnostics-2.c |  57 +++
 3 files changed, 597 insertions(+), 358 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/sprintf-diagnostics-1.c
 create mode 100644 gcc/testsuite/gcc.dg/sprintf-diagnostics-2.c

-- 
1.8.5.3

Patch

diff --git a/gcc/gimple-ssa-sprintf.c b/gcc/gimple-ssa-sprintf.c
index ab430fe..fb6268c 100644
--- a/gcc/gimple-ssa-sprintf.c
+++ b/gcc/gimple-ssa-sprintf.c
@@ -83,6 +83,7 @@  along with GCC; see the file COPYING3.  If not see
 #include "alloc-pool.h"
 #include "vr-values.h"
 #include "gimple-ssa-evrp-analyze.h"
+#include "gcc-rich-location.h"
 
 /* The likely worst case value of MB_LEN_MAX for the target, large enough
    for UTF-8.  Ideally, this would be obtained by a target hook if it were
@@ -128,7 +129,7 @@  class sprintf_dom_walker : public dom_walker
   void after_dom_children (basic_block) FINAL OVERRIDE;
   bool handle_gimple_call (gimple_stmt_iterator *);
 
-  bool compute_format_length (call_info &, format_result *);
+  bool compute_format_length (call_info &, format_result *, tree);
   class evrp_range_analyzer evrp_range_analyzer;
 };
 
@@ -193,10 +194,67 @@  struct result_range
   unsigned HOST_WIDE_INT unlikely;
 };
 
+/* Information about a particular directive (or the NUL terminator)
+   within a format string, for use when emitting warnings.  */
+
+struct directive_state
+{
+  directive_state (size_t fmt_start_idx, size_t fmt_end_idx,
+		   bool is_nul_terminator,
+		   unsigned HOST_WIDE_INT min_size,
+		   unsigned HOST_WIDE_INT max_size)
+  : m_fmt_start_idx (fmt_start_idx), m_fmt_end_idx (fmt_end_idx),
+    m_is_nul_terminator (is_nul_terminator),
+    m_min_size (min_size), m_max_size (max_size)
+  {}
+
+  label_text get_text () const;
+
+  size_t m_fmt_start_idx;
+  size_t m_fmt_end_idx;
+  bool m_is_nul_terminator;
+  unsigned HOST_WIDE_INT m_min_size;
+  unsigned HOST_WIDE_INT m_max_size;
+};
+
+/* Generate a label for the directive within the format string, describing
+   the size consumed by the output, and highlighting the NUL terminator.  */
+
+label_text
+directive_state::get_text () const
+{
+  pretty_printer pp_range;
+  bool singular = pp_humanized_range (&pp_range, m_min_size, m_max_size);
+  pretty_printer pp;
+  // FIXME: i18n for singular/plural
+  if (singular)
+    pp_printf (&pp, G_("%s byte"),
+	       pp_formatted_text (&pp_range));
+  else
+    pp_printf (&pp, G_("%s bytes"),
+	       pp_formatted_text (&pp_range));
+  if (m_is_nul_terminator)
+    pp_string (&pp, G_(" (for NUL terminator)"));
+  return label_text (xstrdup (pp_formatted_text (&pp)), true);
+}
+
 /* The result of a call to a formatted function.  */
 
 struct format_result
 {
+  /* Record a directive from FMT_START_IDX through FMT_END_IDX,
+     and whether it's the nul terminator, consuming between
+     MIN_SIZE and MAX_SIZE bytes when output.  */
+  void add_directive (size_t fmt_start_idx, size_t fmt_end_idx,
+		      bool is_nul_terminator,
+		      unsigned HOST_WIDE_INT min_size,
+		      unsigned HOST_WIDE_INT max_size)
+  {
+    m_directives.safe_push (directive_state (fmt_start_idx, fmt_end_idx,
+					     is_nul_terminator,
+					     min_size, max_size));
+  }
+
   /* Range of characters written by the formatted function.
      Setting the minimum to HOST_WIDE_INT_MAX disables all
      length tracking for the remainder of the format string.  */
@@ -229,6 +287,13 @@  struct format_result
      of a call.  WARNED also disables the return value optimization.  */
   bool warned;
 
+  /* True when we have an overflow or truncation.  */
+  bool should_warn;
+
+  /* The directives we've seen: locations and sizes of output, in case
+     we need to emit a warning.  */
+  auto_vec<directive_state> m_directives;
+
   /* Preincrement the number of output characters by 1.  */
   format_result& operator++ ()
   {
@@ -475,23 +540,6 @@  fmtwarn (const substring_loc &fmt_loc, location_t param_loc,
   return warned;
 }
 
-static bool
-ATTRIBUTE_GCC_DIAG (6, 8) ATTRIBUTE_GCC_DIAG (7, 8)
-fmtwarn_n (const substring_loc &fmt_loc, location_t param_loc,
-	   const char *corrected_substring, int opt, unsigned HOST_WIDE_INT n,
-	   const char *singular_gmsgid, const char *plural_gmsgid, ...)
-{
-  format_string_diagnostic_t diag (fmt_loc, NULL, param_loc, NULL,
-				   corrected_substring);
-  va_list ap;
-  va_start (ap, plural_gmsgid);
-  bool warned = diag.emit_warning_n_va (opt, n, singular_gmsgid, plural_gmsgid,
-					&ap);
-  va_end (ap);
-
-  return warned;
-}
-
 /* Format length modifiers.  */
 
 enum format_lengths
@@ -2394,310 +2442,27 @@  should_warn_p (const call_info &info,
   return true;
 }
 
-/* At format string location describe by DIRLOC in a call described
-   by INFO, issue a warning for a directive DIR whose output may be
-   in excess of the available space AVAIL_RANGE in the destination
-   given the formatting result FMTRES.  This function does nothing
-   except decide whether to issue a warning for a possible write
-   past the end or truncation and, if so, format the warning.
-   Return true if a warning has been issued.  */
-
-static bool
-maybe_warn (substring_loc &dirloc, location_t argloc,
-	    const call_info &info,
-	    const result_range &avail_range, const result_range &res,
-	    const directive &dir)
-{
-  if (!should_warn_p (info, avail_range, res))
-    return false;
-
-  /* A warning will definitely be issued below.  */
-
-  /* The maximum byte count to reference in the warning.  Larger counts
-     imply that the upper bound is unknown (and could be anywhere between
-     RES.MIN + 1 and SIZE_MAX / 2) are printed as "N or more bytes" rather
-     than "between N and X" where X is some huge number.  */
-  unsigned HOST_WIDE_INT maxbytes = target_dir_max ();
-
-  /* True when there is enough room in the destination for the least
-     amount of a directive's output but not enough for its likely or
-     maximum output.  */
-  bool maybe = (res.min <= avail_range.max
-		&& (avail_range.min < res.likely
-		    || (res.max < HOST_WIDE_INT_MAX
-			&& avail_range.min < res.max)));
-
-  /* Buffer for the directive in the host character set (used when
-     the source character set is different).  */
-  char hostdir[32];
-
-  if (avail_range.min == avail_range.max)
-    {
-      /* The size of the destination region is exact.  */
-      unsigned HOST_WIDE_INT navail = avail_range.max;
-
-      if (target_to_host (*dir.beg) != '%')
-	{
-	  /* For plain character directives (i.e., the format string itself)
-	     but not others, point the caret at the first character that's
-	     past the end of the destination.  */
-	  if (navail < dir.len)
-	    dirloc.set_caret_index (dirloc.get_caret_idx () + navail);
-	}
-
-      if (*dir.beg == '\0')
-	{
-	  /* This is the terminating nul.  */
-	  gcc_assert (res.min == 1 && res.min == res.max);
-
-	  return fmtwarn (dirloc, UNKNOWN_LOCATION, NULL, info.warnopt (),
-			  info.bounded
-			  ? (maybe
-			     ? G_("%qE output may be truncated before the "
-				  "last format character")
-			     : G_("%qE output truncated before the last "
-				  "format character"))
-			  : (maybe
-			     ? G_("%qE may write a terminating nul past the "
-				  "end of the destination")
-			     : G_("%qE writing a terminating nul past the "
-				  "end of the destination")),
-			  info.func);
-	}
-
-      if (res.min == res.max)
-	{
-	  const char *d = target_to_host (hostdir, sizeof hostdir, dir.beg);
-	  if (!info.bounded)
-	    return fmtwarn_n (dirloc, argloc, NULL, info.warnopt (), res.min,
-			      "%<%.*s%> directive writing %wu byte into a "
-			      "region of size %wu",
-			      "%<%.*s%> directive writing %wu bytes into a "
-			      "region of size %wu",
-			      (int) dir.len, d, res.min, navail);
-	  else if (maybe)
-	    return fmtwarn_n (dirloc, argloc, NULL, info.warnopt (), res.min,
-			      "%<%.*s%> directive output may be truncated "
-			      "writing %wu byte into a region of size %wu",
-			      "%<%.*s%> directive output may be truncated "
-			      "writing %wu bytes into a region of size %wu",
-			      (int) dir.len, d, res.min, navail);
-	  else
-	    return fmtwarn_n (dirloc, argloc, NULL, info.warnopt (), res.min,
-			      "%<%.*s%> directive output truncated writing "
-			      "%wu byte into a region of size %wu",
-			      "%<%.*s%> directive output truncated writing "
-			      "%wu bytes into a region of size %wu",
-			      (int) dir.len, d, res.min, navail);
-	}
-      if (res.min == 0 && res.max < maxbytes)
-	return fmtwarn (dirloc, argloc, NULL,
-			info.warnopt (),
-			info.bounded
-			? (maybe
-			   ? G_("%<%.*s%> directive output may be truncated "
-				"writing up to %wu bytes into a region of "
-				"size %wu")
-			   : G_("%<%.*s%> directive output truncated writing "
-				"up to %wu bytes into a region of size %wu"))
-			: G_("%<%.*s%> directive writing up to %wu bytes "
-			     "into a region of size %wu"), (int) dir.len,
-			target_to_host (hostdir, sizeof hostdir, dir.beg),
-			res.max, navail);
-
-      if (res.min == 0 && maxbytes <= res.max)
-	/* This is a special case to avoid issuing the potentially
-	   confusing warning:
-	     writing 0 or more bytes into a region of size 0.  */
-	return fmtwarn (dirloc, argloc, NULL, info.warnopt (),
-			info.bounded
-			? (maybe
-			   ? G_("%<%.*s%> directive output may be truncated "
-				"writing likely %wu or more bytes into a "
-				"region of size %wu")
-			   : G_("%<%.*s%> directive output truncated writing "
-				"likely %wu or more bytes into a region of "
-				"size %wu"))
-			: G_("%<%.*s%> directive writing likely %wu or more "
-			     "bytes into a region of size %wu"), (int) dir.len,
-			target_to_host (hostdir, sizeof hostdir, dir.beg),
-			res.likely, navail);
-
-      if (res.max < maxbytes)
-	return fmtwarn (dirloc, argloc, NULL, info.warnopt (),
-			info.bounded
-			? (maybe
-			   ? G_("%<%.*s%> directive output may be truncated "
-				"writing between %wu and %wu bytes into a "
-				"region of size %wu")
-			   : G_("%<%.*s%> directive output truncated "
-				"writing between %wu and %wu bytes into a "
-				"region of size %wu"))
-			: G_("%<%.*s%> directive writing between %wu and "
-			     "%wu bytes into a region of size %wu"),
-			(int) dir.len,
-			target_to_host (hostdir, sizeof hostdir, dir.beg),
-			res.min, res.max, navail);
-
-      return fmtwarn (dirloc, argloc, NULL, info.warnopt (),
-		      info.bounded
-		      ? (maybe
-			 ? G_("%<%.*s%> directive output may be truncated "
-			      "writing %wu or more bytes into a region of "
-			      "size %wu")
-			 : G_("%<%.*s%> directive output truncated writing "
-			      "%wu or more bytes into a region of size %wu"))
-		      : G_("%<%.*s%> directive writing %wu or more bytes "
-			   "into a region of size %wu"), (int) dir.len,
-		      target_to_host (hostdir, sizeof hostdir, dir.beg),
-		      res.min, navail);
-    }
-
-  /* The size of the destination region is a range.  */
-
-  if (target_to_host (*dir.beg) != '%')
-    {
-      unsigned HOST_WIDE_INT navail = avail_range.max;
-
-      /* For plain character directives (i.e., the format string itself)
-	 but not others, point the caret at the first character that's
-	 past the end of the destination.  */
-      if (navail < dir.len)
-	dirloc.set_caret_index (dirloc.get_caret_idx () + navail);
-    }
-
-  if (*dir.beg == '\0')
-    {
-      gcc_assert (res.min == 1 && res.min == res.max);
-
-      return fmtwarn (dirloc, UNKNOWN_LOCATION, NULL, info.warnopt (),
-		      info.bounded
-		      ? (maybe
-			 ? G_("%qE output may be truncated before the last "
-			      "format character")
-			 : G_("%qE output truncated before the last format "
-			      "character"))
-		      : (maybe
-			 ? G_("%qE may write a terminating nul past the end "
-			      "of the destination")
-			 : G_("%qE writing a terminating nul past the end "
-			      "of the destination")), info.func);
-    }
-
-  if (res.min == res.max)
-    {
-      const char *d = target_to_host (hostdir, sizeof hostdir, dir.beg);
-      if (!info.bounded)
-	return fmtwarn_n (dirloc, argloc, NULL, info.warnopt (), res.min,
-			  "%<%.*s%> directive writing %wu byte into a region "
-			  "of size between %wu and %wu",
-			  "%<%.*s%> directive writing %wu bytes into a region "
-			  "of size between %wu and %wu", (int) dir.len, d,
-			  res.min, avail_range.min, avail_range.max);
-      else if (maybe)
-	return fmtwarn_n (dirloc, argloc, NULL, info.warnopt (), res.min,
-			  "%<%.*s%> directive output may be truncated writing "
-			  "%wu byte into a region of size between %wu and %wu",
-			  "%<%.*s%> directive output may be truncated writing "
-			  "%wu bytes into a region of size between %wu and "
-			  "%wu", (int) dir.len, d, res.min, avail_range.min,
-			  avail_range.max);
-      else
-	return fmtwarn_n (dirloc, argloc, NULL, info.warnopt (), res.min,
-			  "%<%.*s%> directive output truncated writing %wu "
-			  "byte into a region of size between %wu and %wu",
-			  "%<%.*s%> directive output truncated writing %wu "
-			  "bytes into a region of size between %wu and %wu",
-			  (int) dir.len, d, res.min, avail_range.min,
-			  avail_range.max);
-    }
-
-  if (res.min == 0 && res.max < maxbytes)
-    return fmtwarn (dirloc, argloc, NULL, info.warnopt (),
-		    info.bounded
-		    ? (maybe
-		       ? G_("%<%.*s%> directive output may be truncated "
-			    "writing up to %wu bytes into a region of size "
-			    "between %wu and %wu")
-		       : G_("%<%.*s%> directive output truncated writing "
-			    "up to %wu bytes into a region of size between "
-			    "%wu and %wu"))
-		    : G_("%<%.*s%> directive writing up to %wu bytes "
-			 "into a region of size between %wu and %wu"),
-		    (int) dir.len,
-		    target_to_host (hostdir, sizeof hostdir, dir.beg),
-		    res.max, avail_range.min, avail_range.max);
-
-  if (res.min == 0 && maxbytes <= res.max)
-    /* This is a special case to avoid issuing the potentially confusing
-       warning:
-	 writing 0 or more bytes into a region of size between 0 and N.  */
-    return fmtwarn (dirloc, argloc, NULL, info.warnopt (),
-		    info.bounded
-		    ? (maybe
-		       ? G_("%<%.*s%> directive output may be truncated "
-			    "writing likely %wu or more bytes into a region "
-			    "of size between %wu and %wu")
-		       : G_("%<%.*s%> directive output truncated writing "
-			    "likely %wu or more bytes into a region of size "
-			    "between %wu and %wu"))
-		    : G_("%<%.*s%> directive writing likely %wu or more bytes "
-			 "into a region of size between %wu and %wu"),
-		    (int) dir.len,
-		    target_to_host (hostdir, sizeof hostdir, dir.beg),
-		    res.likely, avail_range.min, avail_range.max);
-
-  if (res.max < maxbytes)
-    return fmtwarn (dirloc, argloc, NULL, info.warnopt (),
-		    info.bounded
-		    ? (maybe
-		       ? G_("%<%.*s%> directive output may be truncated "
-			    "writing between %wu and %wu bytes into a region "
-			    "of size between %wu and %wu")
-		       : G_("%<%.*s%> directive output truncated writing "
-			    "between %wu and %wu bytes into a region of size "
-			    "between %wu and %wu"))
-		    : G_("%<%.*s%> directive writing between %wu and "
-			 "%wu bytes into a region of size between %wu and "
-			 "%wu"), (int) dir.len,
-		    target_to_host (hostdir, sizeof hostdir, dir.beg),
-		    res.min, res.max, avail_range.min, avail_range.max);
-
-  return fmtwarn (dirloc, argloc, NULL, info.warnopt (),
-		  info.bounded
-		  ? (maybe
-		     ? G_("%<%.*s%> directive output may be truncated writing "
-			  "%wu or more bytes into a region of size between "
-			  "%wu and %wu")
-		     : G_("%<%.*s%> directive output truncated writing "
-			  "%wu or more bytes into a region of size between "
-			  "%wu and %wu"))
-		  : G_("%<%.*s%> directive writing %wu or more bytes "
-		       "into a region of size between %wu and %wu"),
-		  (int) dir.len,
-		  target_to_host (hostdir, sizeof hostdir, dir.beg),
-		  res.min, avail_range.min, avail_range.max);
-}
-
 /* Compute the length of the output resulting from the directive DIR
    in a call described by INFO and update the overall result of the call
-   in *RES.  Return true if the directive has been handled.  */
+   in *RES.  Return true if the directive has been handled.
+   IS_NUL_TERMINATOR is true iff the "directive" is the NUL terminator.  */
 
 static bool
 format_directive (const call_info &info,
 		  format_result *res, const directive &dir,
-		  class vr_values *vr_values)
+		  class vr_values *vr_values,
+		  bool is_nul_terminator)
 {
   /* Offset of the beginning of the directive from the beginning
      of the format string.  */
   size_t offset = dir.beg - info.fmtstr;
-  size_t start = offset;
-  size_t length = offset + dir.len - !!dir.len;
+  size_t start_idx = offset;
+  size_t end_idx = offset + dir.len - !!dir.len;
 
   /* Create a location for the whole directive from the % to the format
      specifier.  */
   substring_loc dirloc (info.fmtloc, TREE_TYPE (info.format),
-			offset, start, length);
+			offset, start_idx, end_idx);
 
   /* Also get the location of the argument if possible.
      This doesn't work for integer literals or function calls.  */
@@ -2713,6 +2478,10 @@  format_directive (const call_info &info,
   /* Compute the range of lengths of the formatted output.  */
   fmtresult fmtres = dir.fmtfunc (dir, dir.arg, vr_values);
 
+  // (FIXME: or maybe record the whole of fmtres?)
+  res->add_directive (start_idx, end_idx, is_nul_terminator,
+		      fmtres.range.min, fmtres.range.max);
+
   /* Record whether the output of all directives is known to be
      bounded by some maximum, implying that their arguments are
      either known exactly or determined to be in a known range
@@ -2776,11 +2545,8 @@  format_directive (const call_info &info,
      NUL that's appended after the format string has been processed.  */
   result_range avail_range = bytes_remaining (info.objsize, *res);
 
-  bool warned = res->warned;
-
-  if (!warned)
-    warned = maybe_warn (dirloc, argloc, info, avail_range,
-			 fmtres.range, dir);
+  if (should_warn_p (info, avail_range, fmtres.range))
+    res->should_warn = true;
 
   /* Bump up the total maximum if it isn't too big.  */
   if (res->range.max < HOST_WIDE_INT_MAX
@@ -2814,6 +2580,8 @@  format_directive (const call_info &info,
   if (fmtres.mayfail)
     res->posunder4k = false;
 
+  bool warned = res->warned;
+
   if (!warned
       /* Only warn at level 2.  */
       && warn_level > 1
@@ -2907,39 +2675,6 @@  format_directive (const call_info &info,
 
   res->warned |= warned;
 
-  if (!dir.beg[0] && res->warned && info.objsize < HOST_WIDE_INT_MAX)
-    {
-      /* If a warning has been issued for buffer overflow or truncation
-	 (but not otherwise) help the user figure out how big a buffer
-	 they need.  */
-
-      location_t callloc = gimple_location (info.callstmt);
-
-      unsigned HOST_WIDE_INT min = res->range.min;
-      unsigned HOST_WIDE_INT max = res->range.max;
-
-      if (min == max)
-	inform (callloc,
-		(min == 1
-		 ? G_("%qE output %wu byte into a destination of size %wu")
-		 : G_("%qE output %wu bytes into a destination of size %wu")),
-		info.func, min, info.objsize);
-      else if (max < HOST_WIDE_INT_MAX)
-	inform (callloc,
-		"%qE output between %wu and %wu bytes into "
-		"a destination of size %wu",
-		info.func, min, max, info.objsize);
-      else if (min < res->range.likely && res->range.likely < max)
-	inform (callloc,
-		"%qE output %wu or more bytes (assuming %wu) into "
-		"a destination of size %wu",
-		info.func, min, res->range.likely, info.objsize);
-      else
-	inform (callloc,
-		"%qE output %wu or more bytes into a destination of size %wu",
-		info.func, min, info.objsize);
-    }
-
   if (dump_file && *dir.beg)
     {
       fprintf (dump_file,
@@ -3396,20 +3131,109 @@  parse_directive (call_info &info,
   return dir.len;
 }
 
+/* Subclass of gcc_rich_location for emitting warnings about sprintf and
+   snprintf.  */
+
+class rich_format_location : public gcc_rich_location
+{
+ public:
+  rich_format_location (location_t dst_loc, const call_info &info,
+			const format_result &res)
+  : gcc_rich_location (dst_loc, &m_label),
+    m_info (info), m_res (res),
+    m_label (*this)
+  {
+    unsigned i;
+    directive_state *dir_state;
+    FOR_EACH_VEC_ELT (m_res.m_directives, i, dir_state)
+      {
+	substring_loc dirloc (m_info.fmtloc, TREE_TYPE (m_info.format),
+			      dir_state->m_fmt_start_idx,
+			      dir_state->m_fmt_start_idx,
+			      dir_state->m_fmt_end_idx);
+	location_t loc = UNKNOWN_LOCATION;
+	dirloc.get_location (&loc);
+	add_range (loc, SHOW_RANGE_WITH_CARET, &m_label);
+      }
+  }
+
+ private:
+  /* Subclass of range_label for labelling all of the various ranges.  */
+  class flyweight_range_label : public range_label
+  {
+  public:
+    flyweight_range_label (const rich_format_location &rich_loc)
+    : m_rich_loc (rich_loc) {}
+
+    label_text get_text (unsigned range_idx) const FINAL OVERRIDE;
+  private:
+    const rich_format_location &m_rich_loc;
+  };
+  friend class flyweight_range_label;
+
+  label_text get_label_for_buffer () const;
+  label_text get_label_for_directive (unsigned dir_idx) const;
+
+  const call_info &m_info;
+  const format_result &m_res;
+  flyweight_range_label m_label;
+};
+
+/* Implementation of range_label::get_text for
+   rich_format_location::flyweight_range_label.
+   Handle all of the various labels with one instance by dispatching based on
+   RANGE_IDX.  */
+
+label_text
+rich_format_location::flyweight_range_label::get_text (unsigned range_idx) const
+{
+  if (range_idx == 0)
+    return m_rich_loc.get_label_for_buffer ();
+  else
+    return m_rich_loc.get_label_for_directive (range_idx - 1);
+}
+
+/* Get a label for the underlined destination buffer, showing the capacity of
+   the buffer.  */
+
+label_text
+rich_format_location::get_label_for_buffer () const
+{
+  pretty_printer pp;
+  // FIXME: i18n for singular/plural
+  if (m_info.objsize == 1)
+    pp_printf (&pp, G_("capacity: %wu byte"), m_info.objsize);
+  else
+    pp_printf (&pp, G_("capacity: %wu bytes"), m_info.objsize);
+  return label_text (xstrdup (pp_formatted_text (&pp)), true);
+}
+
+/* Get a label for the underlined directive, showing the size of the output
+   from that directive.  */
+
+label_text
+rich_format_location::get_label_for_directive (unsigned dir_idx) const
+{
+  const directive_state &dir_state = m_res.m_directives[dir_idx];
+  return dir_state.get_text ();
+}
+
 /* Compute the length of the output resulting from the call to a formatted
    output function described by INFO and store the result of the call in
    *RES.  Issue warnings for detected past the end writes.  Return true
    if the complete format string has been processed and *RES can be relied
    on, false otherwise (e.g., when a unknown or unhandled directive was seen
-   that caused the processing to be terminated early).  */
+   that caused the processing to be terminated early).  DST_PTR is the destination
+   of the output.  */
 
 bool
 sprintf_dom_walker::compute_format_length (call_info &info,
-					   format_result *res)
+					   format_result *res,
+					   tree dst_ptr)
 {
+  location_t callloc = gimple_location (info.callstmt);
   if (dump_file)
     {
-      location_t callloc = gimple_location (info.callstmt);
       fprintf (dump_file, "%s:%i: ",
 	       LOCATION_FILE (callloc), LOCATION_LINE (callloc));
       print_generic_expr (dump_file, info.func, dump_flags);
@@ -3430,6 +3254,7 @@  sprintf_dom_walker::compute_format_length (call_info &info,
   res->posunder4k = true;
   res->floating = false;
   res->warned = false;
+  res->should_warn = false;
 
   /* 1-based directive counter.  */
   unsigned dirno = 1;
@@ -3437,6 +3262,8 @@  sprintf_dom_walker::compute_format_length (call_info &info,
   /* The variadic argument counter.  */
   unsigned argno = info.argidx;
 
+  bool is_nul_terminator = false;
+
   for (const char *pf = info.fmtstr; ; ++dirno)
     {
       directive dir = directive ();
@@ -3445,23 +3272,126 @@  sprintf_dom_walker::compute_format_length (call_info &info,
       size_t n = parse_directive (info, dir, res, pf, &argno,
 				  evrp_range_analyzer.get_vr_values ());
 
+      is_nul_terminator = !n && *pf == '\0';
+
+      fmtresult fmtres;
+
       /* Return failure if the format function fails.  */
       if (!format_directive (info, res, dir,
-			     evrp_range_analyzer.get_vr_values ()))
+			     evrp_range_analyzer.get_vr_values (),
+			     is_nul_terminator))
 	return false;
 
-      /* Return success the directive is zero bytes long and it's
-	 the last think in the format string (i.e., it's the terminating
-	 nul, which isn't really a directive but handling it as one makes
-	 things simpler).  */
+      /* Stop at the end of the format string.  */
       if (!n)
-	return *pf == '\0';
+	break;
 
       pf += n;
     }
 
-  /* The complete format string was processed (with or without warnings).  */
-  return true;
+  if (res->should_warn && info.objsize < HOST_WIDE_INT_MAX)
+    {
+      unsigned HOST_WIDE_INT min = res->range.min;
+      unsigned HOST_WIDE_INT max = res->range.max;
+      bool warned;
+
+      /* Build the rich location for diagnostics.  */
+      location_t dst_loc = UNKNOWN_LOCATION;
+      if (dst_ptr != NULL_TREE && EXPR_HAS_LOCATION (dst_ptr))
+	dst_loc = EXPR_LOCATION (dst_ptr);
+      location_t primary_loc;
+      if (dst_loc != UNKNOWN_LOCATION)
+	primary_loc = dst_loc;
+      else
+	primary_loc = get_start (callloc);
+      rich_format_location rich_loc (primary_loc, info, *res);
+
+      /* Truncation vs buffer overflow.  */
+      if (info.bounded)
+	{
+	  /* Emit a "truncated output" warning.  */
+	  if (min == max)
+	    warned = warning_at
+	      (&rich_loc, info.warnopt (),
+	       (min == 1
+		? G_("%qE will truncate the output (%wu byte) to size %wu")
+		: G_("%qE will truncate the output (%wu bytes) to size %wu")),
+	       info.func, min, info.objsize);
+	  else if (max < HOST_WIDE_INT_MAX)
+	    warned = warning_at
+	      (&rich_loc, info.warnopt (),
+	       "%qE will truncate the output (between %wu and %wu bytes) "
+	       "to size %wu",
+	       info.func, min, max, info.objsize);
+	  else if (min < res->range.likely && res->range.likely < max)
+	    warned = warning_at
+	      (&rich_loc, info.warnopt (),
+	       "%qE will truncate the output (%wu or more bytes, assuming %wu) "
+	       "to size %wu",
+	       info.func, min, res->range.likely, info.objsize);
+	  else
+	    warned = warning_at
+	      (&rich_loc, info.warnopt (),
+	       "FIXME: the other truncation case for %qE) ",
+	       info.func);
+	  // FIXME: what's the other case here?
+
+	  // FIXME: what about wrong size limit?
+	  // FIXME: should we highlight where the size comes for sizeof exprs?
+	}
+      else
+	{
+	  /* Emit a "buffer overflow" warning.  */
+	  auto_diagnostic_group d;
+	  if (min == max)
+	    warned = warning_at
+	      (&rich_loc, info.warnopt (),
+	       (min == 1
+		? G_("buffer overflow: %qE will write %wu byte"
+		     " into a destination of size %wu")
+		: G_("buffer overflow: %qE will write %wu bytes"
+		     " into a destination of size %wu")),
+	       info.func, min, info.objsize);
+	  else if (max < HOST_WIDE_INT_MAX)
+	    warned = warning_at
+	      (&rich_loc, info.warnopt (),
+	       "buffer overflow: %qE will write between %wu and %wu bytes into "
+	       "a destination of size %wu",
+	       info.func, min, max, info.objsize);
+	  else if (min < res->range.likely && res->range.likely < max)
+	    warned = warning_at
+	      (&rich_loc, info.warnopt (),
+	       "buffer overflow: %qE will write %wu or more bytes"
+	       " (assuming %wu) into a destination of size %wu",
+	       info.func, min, res->range.likely, info.objsize);
+	  else
+	    warned = warning_at
+	      (&rich_loc, info.warnopt (),
+	       "buffer overflow: %qE will write %wu or more bytes"
+	       " into a destination of size %wu",
+	       info.func, min, info.objsize);
+	}
+      if (warned)
+	{
+	  /* If possible, emit a note showing the declaration of the
+	     destination buffer.  */
+	  // FIXME: possibly use "if nearby" to add as another location to main warning?
+	  if (TREE_CODE (dst_ptr) == ADDR_EXPR)
+	    {
+	      tree pt_var = TREE_OPERAND (dst_ptr, 0);
+	      if (DECL_P (pt_var))
+		inform (DECL_SOURCE_LOCATION (pt_var),
+			"destination declared here");
+	    }
+	}
+    }
+
+  /* Return success if the final "directive" is zero bytes long and it's
+     the last think in the format string (i.e., it's the terminating
+     nul, which isn't really a directive but handling it as one makes
+     things simpler).
+     This isn't affected by whether we emitted warnings.  */
+  return is_nul_terminator;
 }
 
 /* Return the size of the object referenced by the expression DEST if
@@ -3932,7 +3862,7 @@  sprintf_dom_walker::handle_gimple_call (gimple_stmt_iterator *gsi)
      including the terminating NUL.  */
   format_result res = format_result ();
 
-  bool success = compute_format_length (info, &res);
+  bool success = compute_format_length (info, &res, dstptr);
 
   /* When optimizing and the printf return value optimization is enabled,
      attempt to substitute the computed result for the return value of
diff --git a/gcc/testsuite/gcc.dg/sprintf-diagnostics-1.c b/gcc/testsuite/gcc.dg/sprintf-diagnostics-1.c
new file mode 100644
index 0000000..c807b2d
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/sprintf-diagnostics-1.c
@@ -0,0 +1,252 @@ 
+/* { dg-do compile } */
+/* { dg-options "-Wformat-overflow=2 -Wformat-truncation=2 -O2 -fdiagnostics-show-caret" } */
+
+typedef __SIZE_TYPE__ size_t;
+extern int sprintf (char*, const char*, ...);
+extern int snprintf (char*, size_t, const char*, ...);
+
+/* Bounded, definite truncation in a directive.  */
+
+void test_1 (int i)
+{
+  char buf_1[4]; /* { dg-message "destination declared here" } */
+  snprintf (buf_1, sizeof buf_1, "%i", 1235); /* { dg-warning "'snprintf' will truncate the output \\(5 bytes\\) to size 4" } */
+  /* { dg-begin-multiline-output "" }
+   snprintf (buf_1, sizeof buf_1, "%i", 1235);
+             ^~~~~                 ^~^
+             |                     | |
+             |                     | 1 byte (for NUL terminator)
+             capacity: 4 bytes     4 bytes
+     { dg-end-multiline-output "" } */
+  /* { dg-begin-multiline-output "" }
+   char buf_1[4];
+        ^~~~~
+     { dg-end-multiline-output "" } */
+}
+
+/* Bounded, definite truncation copying format string.  */
+
+void test_2 (int i)
+{
+  char buf_2[4]; /* { dg-message "destination declared here" } */
+  snprintf (buf_2, sizeof buf_2, "%iAB", 123); /* { dg-warning "'snprintf' will truncate the output \\(6 bytes\\) to size 4" } */
+  /* { dg-begin-multiline-output "" }
+   snprintf (buf_2, sizeof buf_2, "%iAB", 123);
+             ^~~~~                 ^~^~^
+             |                     | | |
+             |                     | | 1 byte (for NUL terminator)
+             |                     | 2 bytes
+             capacity: 4 bytes     3 bytes
+     { dg-end-multiline-output "" } */
+  /* { dg-begin-multiline-output "" }
+   char buf_2[4];
+        ^~~~~
+     { dg-end-multiline-output "" } */
+}
+
+/* unbounded, definite overflow in a directive.  */
+
+void test_3 (int i)
+{
+  char buf_3[4]; /* { dg-message "destination declared here" } */
+  sprintf (buf_3, "%i", 1235); /* { dg-warning "buffer overflow: 'sprintf' will write 5 bytes into a destination of size 4" } */
+  /* { dg-begin-multiline-output "" }
+   sprintf (buf_3, "%i", 1235);
+            ^~~~~   ^~^
+            |       | |
+            |       | 1 byte (for NUL terminator)
+            |       4 bytes
+            capacity: 4 bytes
+     { dg-end-multiline-output "" } */
+  /* { dg-begin-multiline-output "" }
+   char buf_3[4];
+        ^~~~~
+     { dg-end-multiline-output "" } */
+}
+
+/* unbounded, definite overflow copying format string.  */
+
+void test_4 (int i)
+{
+  char buf_4[4]; /* { dg-message "destination declared here" } */
+  sprintf (buf_4, "%iAB", 123); /* { dg-warning "buffer overflow: 'sprintf' will write 6 bytes into a destination of size 4" } */
+  /* { dg-begin-multiline-output "" }
+   sprintf (buf_4, "%iAB", 123);
+            ^~~~~   ^~^~^
+            |       | | |
+            |       | | 1 byte (for NUL terminator)
+            |       | 2 bytes
+            |       3 bytes
+            capacity: 4 bytes
+     { dg-end-multiline-output "" } */
+  /* { dg-begin-multiline-output "" }
+   char buf_4[4];
+        ^~~~~
+     { dg-end-multiline-output "" } */
+}
+
+/* bounded, possible truncation a directve.  */
+
+void test_5 (int i)
+{
+  char buf_5[4]; /* { dg-message "destination declared here" } */
+  snprintf (buf_5, sizeof buf_5, "%i", i); /* { dg-warning "'snprintf' will truncate the output \\(between 2 and 12 bytes\\) to size 4" } */
+  /* { dg-begin-multiline-output "" }
+   snprintf (buf_5, sizeof buf_5, "%i", i);
+             ^~~~~                 ^~^
+             |                     | |
+             |                     | 1 byte (for NUL terminator)
+             capacity: 4 bytes     1...11 bytes
+     { dg-end-multiline-output "" } */
+  /* { dg-begin-multiline-output "" }
+   char buf_5[4];
+        ^~~~~
+     { dg-end-multiline-output "" } */
+}
+
+/* bounded, possible overflow copying format string.  */
+
+void test_6 (int i)
+{
+  char buf_6[4]; /* { dg-message "destination declared here" } */
+  snprintf (buf_6, sizeof buf_6, "%iAB", i); /* { dg-warning "'snprintf' will truncate the output \\(between 4 and 14 bytes\\) to size 4" } */
+  /* { dg-begin-multiline-output "" }
+   snprintf (buf_6, sizeof buf_6, "%iAB", i);
+             ^~~~~                 ^~^~^
+             |                     | | |
+             |                     | | 1 byte (for NUL terminator)
+             |                     | 2 bytes
+             capacity: 4 bytes     1...11 bytes
+     { dg-end-multiline-output "" } */
+  /* { dg-begin-multiline-output "" }
+   char buf_6[4];
+        ^~~~~
+     { dg-end-multiline-output "" } */
+}
+
+/* unbounded, possible overflow in a directve.  */
+
+void test_7 (int i)
+{
+  char buf_7[4]; /* { dg-message "destination declared here" } */
+  sprintf (buf_7, "%i", i); /* { dg-warning "buffer overflow: 'sprintf' will write between 2 and 12 bytes into a destination of size 4" } */
+  /* { dg-begin-multiline-output "" }
+   sprintf (buf_7, "%i", i);
+            ^~~~~   ^~^
+            |       | |
+            |       | 1 byte (for NUL terminator)
+            |       1...11 bytes
+            capacity: 4 bytes
+     { dg-end-multiline-output "" } */
+  /* { dg-begin-multiline-output "" }
+   char buf_7[4];
+        ^~~~~
+     { dg-end-multiline-output "" } */
+}
+
+/* unbounded, possible overflow copying format string.  */
+
+void test_8 (int i)
+{
+  char buf_8[4];
+  sprintf (buf_8, "%iAB", 123);
+  /* FIXME: why isn't this generating a warning?  */
+}
+
+/* unbounded, possible overflow copying format string.  */
+
+void test_9 (int i)
+{
+  char buf_9[4]; /* { dg-message "destination declared here" } */
+  const char *s = i ? "123" : "1234";
+  sprintf (buf_9, "%sAB", s); /* { dg-warning "buffer overflow: 'sprintf' will write between 6 and 7 bytes into a destination of size 4" } */
+  /* { dg-begin-multiline-output "" }
+   sprintf (buf_9, "%sAB", s);
+            ^~~~~   ^~^~^
+            |       | | |
+            |       | | 1 byte (for NUL terminator)
+            |       | 2 bytes
+            |       3...4 bytes
+            capacity: 4 bytes
+     { dg-end-multiline-output "" } */
+  /* { dg-begin-multiline-output "" }
+   char buf_9[4];
+        ^~~~~
+     { dg-end-multiline-output "" } */
+}
+
+extern char buf_10[80];
+extern char tmpdir[80];
+extern char fname[8];
+
+void test_10 (int num)
+{
+  sprintf (buf_10, "/%s/%s-%i.tmp", tmpdir, fname, num); /* { dg-warning "buffer overflow: 'sprintf' will write between 9 and 105 bytes into a destination of size 80" } */
+  /* { dg-begin-multiline-output "" }
+   sprintf (buf_10, "/%s/%s-%i.tmp", tmpdir, fname, num);
+            ^~~~~~   ^^~^^~^^~^~~~^
+            |        || || || |   |
+            |        || || || |   1 byte (for NUL terminator)
+            |        || || || 4 bytes
+            |        || || |1...11 bytes
+            |        || || 1 byte
+            |        || |0...7 bytes
+            |        || 1 byte
+            |        |0...79 bytes
+            |        1 byte
+            capacity: 80 bytes
+     { dg-end-multiline-output "" } */
+  /* { dg-begin-multiline-output "" }
+ extern char buf_10[80];
+             ^~~~~~
+     { dg-end-multiline-output "" } */
+}
+
+struct MyStrings {
+  char a[8], b[20];
+};
+
+const struct MyStrings ms[] = {
+  { "foo", "bar" }, { "abcd", "klmno" }
+};
+
+void test_11 (void)
+{
+  char buf_11[6];
+  sprintf (buf_11, "msg: %s\n", ms[1].b); /* { dg-warning "buffer overflow: 'sprintf' will write 12 bytes into a destination of size 6" } */
+  /* { dg-begin-multiline-output "" }
+   sprintf (buf_11, "msg: %s\n", ms[1].b);
+            ^~~~~~   ^~~~~^~^~^
+            |        |    | | |
+            |        |    | | 1 byte (for NUL terminator)
+            |        |    | 1 byte
+            |        |    5 bytes
+            |        5 bytes
+            capacity: 6 bytes
+     { dg-end-multiline-output "" } */
+  /* { dg-begin-multiline-output "" }
+   char buf_11[6];
+        ^~~~~~
+     { dg-end-multiline-output "" } */
+}
+
+void test_12 (int idx)
+{
+  char buf_12[6];
+  sprintf (buf_12, "msg: %s\n", ms[idx].b); /* { dg-warning "buffer overflow: 'sprintf' will write 7 or more bytes \\(assuming 8\\) into a destination of size 6" } */
+  // FIXME: this has a 64-bit assumption
+  /* { dg-begin-multiline-output "" }
+   sprintf (buf_12, "msg: %s\n", ms[idx].b);
+            ^~~~~~   ^~~~~^~^~^
+            |        |    | | |
+            |        |    | | 1 byte (for NUL terminator)
+            |        |    | 1 byte
+            |        |    0...(1<<63)-1 bytes
+            |        5 bytes
+            capacity: 6 bytes
+     { dg-end-multiline-output "" } */
+  /* { dg-begin-multiline-output "" }
+   char buf_12[6];
+        ^~~~~~
+     { dg-end-multiline-output "" } */
+}
diff --git a/gcc/testsuite/gcc.dg/sprintf-diagnostics-2.c b/gcc/testsuite/gcc.dg/sprintf-diagnostics-2.c
new file mode 100644
index 0000000..69482ee
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/sprintf-diagnostics-2.c
@@ -0,0 +1,57 @@ 
+/* TODO: finish these test cases.  */
+
+/* { dg-do compile } */
+/* { dg-options "-Wformat-overflow=2 -Wformat-truncation=2 -O2 -fdiagnostics-show-caret" } */
+
+typedef __SIZE_TYPE__ size_t;
+extern int sprintf (char*, const char*, ...);
+extern int snprintf (char*, size_t, const char*, ...);
+
+void test_13 (const char *msg)
+{
+  char buf[16];
+
+  const char *fmt = "msg: %s\n";
+  sprintf (buf, fmt, msg);
+  /* { dg-begin-multiline-output "" }
+     { dg-end-multiline-output "" } */
+}
+
+void test_14 (void)
+{
+#define INT_FMT "%i"
+  char buf_14[4]; /* { dg-message "destination declared here" } */
+  sprintf (buf_14, INT_FMT "AB", 123);
+  /* { dg-begin-multiline-output "" }
+     { dg-end-multiline-output "" } */
+}
+
+void test_15 (void)
+{
+#define FMT "%i"
+  char buf[16];
+  sprintf (buf, "i: " FMT " j: " FMT " k: " FMT "\n", 1066, 1215, 1649);
+  /* { dg-begin-multiline-output "" }
+     { dg-end-multiline-output "" } */
+}
+
+void test_non_contiguous_strings (void)
+{
+  char buf[4]; /* { dg-message "destination declared here" } */
+  sprintf(buf, " %" "i ", 65536);
+  /* { dg-begin-multiline-output "" }
+     { dg-end-multiline-output "" } */
+}
+
+void test_non_contiguous_strings_2 (void)
+{
+  char buf[4]; /* { dg-message "destination declared here" } */
+  sprintf(buf, " %"
+	  "i ", 65536);
+  /* FIXME: looks like ICF merges this with the above and gets the wrong location.  */
+  /* { dg-begin-multiline-output "" }
+     { dg-end-multiline-output "" } */
+}
+
+/* FIXME: see gcc.dg/format/diagnostic-ranges.c
+   FIXME: try with C++.  */